issue274 Unignore and fix TckTests#mixedMultiLevelNestedActivity

eclipse / microprofile-lra

microprofile-lra

Apache License 2.0

101 stars 30 forks source link

issue274 Unignore and fix TckTests#mixedMultiLevelNestedActivity #293

Closed xstefank closed 4 years ago

xstefank commented 4 years ago

resolves #274

xstefank commented 4 years ago

@rdebusscher Rudy, I would like to merge this later today. Can you take a look, please?

rdebusscher commented 4 years ago

I fear that some of the nested LRA scenarios aren't understood enough at this time and will cause great confusion

allowed to call endpoint again with already closed (nested) LRAid
Is call with and without LRA_HTTP_PARENT_CONTEXT_HEADER the same invocation?
which situation exactly trigger the invocation of @Complete and @Compensate (for the same participant)
the (almost required) definition of a @Forget, etc ...)
...

xstefank commented 4 years ago

@rdebusscher I agree that the nested scenarios are a little overcomplicated, however, they are specified so they should be consumable for users. But I myself had quite a hard time understanding all possible nesting outcomes. I don't expect that this feature will be heavily used in the beginning.

This PR is in accordance with the definition that we all agreed on in https://github.com/eclipse/microprofile-lra/pull/276.

rdebusscher commented 4 years ago

@xstefank But as I mentioned in my last comment on #276

I don't think you need to enlist in the case of the nested case going from Closed to cancelling.

I did not think about the user explicitly calling an endpoint which is already enlisted as a nested LRA again. Because in this example it does a cancel, but the endpoint can just be called successfully, and in that case, is the @Complete called a second time? (corresponding to the second enlistment) This is not specified in the spec if you ask me.

The spec says that participant can only be enlisted once, so only 1 set of calls to @Complete (because multiple calls are possible) This would mean we have a whole different set of rules related to nested LRA.

xstefank commented 4 years ago

@rdebusscher actually I mentioned this case in #276 and it is specified (if the method closes the LRA again the complete method is not called) -- https://github.com/eclipse/microprofile-lra/pull/276/files#diff-0d4b5f4f2a4183d5dfb813a9cc447a0eR301-R303.

xstefank commented 4 years ago

After the number of discussions in the public chat room (gitter) we agreed that we will include the current nested model and thus PR can be merged.