ThreadContext.REQUEST

[gavinking]

Currently it's possible to control propagation of contexts via passing of Stringly-typed identifiers. There are some standard identifiers defined by ThreadContext. For example, ThreadContext.TRANSACTION is useful to control exactly when transaction contexts are propagated to long-running asynchronous jobs.

However, there is no standard identifier for the very important request context. As I understand it, the current behavior is that if a JAX-RS client calls a ManagedExecutor to start a job, then:

1. the JAX-RS request context (the thread-bound request and response objects) is propagated to the asynchronous thread, but
2. the CDI request scope is not, unless by some accident of the underlying implementation, CDI happens to be collaborating with JAX-RS in a way that is currently undefined by the spec.

(If my understanding here is wrong, please could someone correct me.)

Assuming that my understanding is correct, well, it's broken.

• CDI request scope propagation should be consistent with how JAX-RS propagates its context, and
• by default, the JAX-RS request context should not be propagated by a ManagedExecutor.

And we don't even give the user a very good way to control request context propagation, since there is no standard way to identify the JAX-RS contexts.

I therefore propose that:

• we add ThreadContext.REQUEST, and specify
• that it is not propagated by default when the user explicitly calls ManagedExecutor, and
• that CDI propagates its request scope whenever iff the REQUEST context is propagated.

We don't need to limit ThreadContext.REQUEST to always mean the JAX-RS request, just as the CDI request scope doesn't always mean a servlet request.

Note that the required behavior for ThreadContext.REQUEST is extremely similar to ThreadContext.TRANSACTION. It should be propagated along a reactive stream, but not (by default) to a ManagedExecutor job.

Hope I'm not misunderstanding something fundamental here.

[manovotn]

You are talking about CDI req scope., but CDI has multiple scopes you need to consider. There is session and conversation as well. These are all incorporated in ThreadContext.CDI.

So what exactly would happen if you tried to propagate ThreadContext.CDI and clear ThreadContext.REQUEST?

[gavinking]

You are talking about CDI req scope., but CDI has multiple scopes you need to consider. There is session and conversation as well. These are all incorporated in ThreadContext.CDI.

I assume that if the (narrower) request scope is propagated, then the (wider) conversation and session scopes are going to also be propagated. Is there some reason to think this wouldn't work?

(Note, you need to consider how this works in the context of a reactive stream processing a single request.)

[manovotn]

Well, the spec doesn't (yet) define what exactly propagates but it should state that with CDI all three are.
My point is that your ThreadContext.REQUEST is basically CDI context + something else on top.
We should not have overlapping contexts (aggregations of several) defined IMO.

[FroMage]

• the JAX-RS request context (the thread-bound request and response objects) is propagated to the asynchronous thread, but
• the CDI request scope is not, unless by some accident of the underlying implementation, CDI happens to be collaborating with JAX-RS in a way that is currently undefined by the spec.

Actually, if you propagate all contexts then both those (request) contexts will be propagated.

[gavinking]

@FroMage

Sure, but as I understand it, "all contexts" is not the default behavior.

[gavinking]

@manovotn

Well, the spec doesn't (yet) define what exactly propagates but it should state that with CDI all three are.

Sure, I agree. Sorry I left that implicit. Certainly the spec should make it explicit.

[FroMage]

Sure, but as I understand it, "all contexts" is not the default behavior.

There is no default anymore. When there was a default, it was "all contexts except transaction" so it would have worked. Using ALL for the propagated would make this work.

[njr-11]

Well, the spec doesn't (yet) define what exactly propagates but it should state that with CDI all three are.

Sure, I agree. Sorry I left that implicit. Certainly the spec should make it explicit.

@aguibert is currently working on the pull that explicitly defines in the spec what is propagated by CDI context. There was another issue already open for it and it is being done under that issue.

[gavinking]

@FroMage well, OK, fine, if that's indeed the resolution of #124.

Using ALL for the propagated would make this work.

Nevertheless, I it seems to me that there's still a need to be able to easily control request context propagation individually from other contexts. (For essentially the same reasons that we have a need to control TRANSACTION separately.)

[gavinking]

@njr-11

@aguibert is currently working on the pull that explicitly defines in the spec what is propagated by CDI context.

I bet that it would be easier to write down this definition if it were written in terms of ThreadContext.REQUEST.

[manovotn]

I wouldn't say so, introducing ThreadContext.REQUEST and having ThreadContext.CDI makes it error prone because:

• You don't define what exactly is a REQUEST here, different perceptions may apply
  • For instance the usual HTTP request or a custom view of request as a time period in which you sort out a task (e.g. how request scoped is often used in SE env)
• You now have two different contexts, both of which include CDI context propagation in it
• You could now request to clear CDI and propagate REQUEST in which case you get into undefined state
  • I do not think we should promote similar "composite contexts" as one of those basic in the spec

[gavinking]

@manovotn I don't understand.

You don't define what exactly is a REQUEST here, different perceptions may apply

I could make the exact same criticism of @RequestScoped in CDI. But in practice everyone seems to use it correctly.

For instance the usual HTTP request or a custom view of request as a time period in which you sort out a task

These are both perfectly valid interpretations of @RequestScoped in CDI and that doesn't seem to cause a problem.

You now have two different contexts, both of which include CDI context propagation in it

Nope.

• CDI context propagation is defined by ThreadContext.CDI end of story.
• ThreadContext.REQUEST refers to propagation of thread-bound context objects which are not managed by CDI, such as the JAX-RS request and response objects. These objects are not bound to any CDI context, and are propagated by MP Concurrency directly, not indirectly via CDI.

You could now request to clear CDI and propagate REQUEST in which case you get into undefined state

On the contrary, you're in a completely well-defined state, with fresh CDI contexts, but the JAX-RS request and response objects propagated from the thread that was processing the JAX-RS request.

[njr-11]

Am I correct in assuming that JAX-RS request context refers to the javax.ws.rs.core.Context annotation being used at class level to inject an instance that might be used across different request, that information possibly being lost when accessed from a completion stage action? I checked with one of our JAX-RS experts @andymc12 on this who informed me this pattern is non-strategic and being replaced by CDI @Inject in a future version of JAX-RS, after which I expect it would be automatically covered by CDI context propagation. In the meantime, there is the option of declaring @Context on a request method parameter, which it ought to be fine to access from a completion stage method. Is this understanding correct, or are there additional parts of this that I am missing.

[gavinking]

Am I correct in assuming that JAX-RS request context refers to the javax.ws.rs.core.Context annotation being used at class level to inject an instance that might be used across different request, that information possibly being lost when accessed from a completion stage action?

I think so. But @FroMage can clarify this.

after which I expect it would be automatically covered by CDI context propagation

That's not clear to me. The fact that you're using @Inject rather than @Context to inject something doesn't say anything at all about the semantics surrounding propagation of the corresponding injected objects between threads.

[gavinking]

I mean, look, if there's a need for ThreadContext.TRANSACTION, it's very difficult for me to understand how there's not a corresponding, equivalent, need for ThreadContext.REQUEST, since in both cases we have some threadbound context objects being managed by some infrastructure that is external to CDI.

I mean, you can write @Inject UserTransaction too, so does that mean we can just get rid of ThreadContext.TRANSACTION? Not as far as I can tell.

So what then is the difference between the two cases?

[gavinking]

In the meantime, there is the option of declaring @Context on a request method parameter, which it ought to be fine to access from a completion stage method.

But they wouldn't be accessible from anything the completion stage method calls. So the CDI @RequestScoped stuff would be propagated, but not the JAX-RS context objects. Which is inconsistent behavior and confusing to the user.

[manovotn]

Then I misunderstood your initial comments which seemed to me that REQUEST implied propagating CDI behind the scenes.

Now the question is how many contexts we want to place into the spec for its 1.0. Just note that the model we have is extensible and we can experiment with additional contexts in impls and pull them into specs later on.

[gavinking]

Then I misunderstood your initial comments which seemed to me that REQUEST implied propagating CDI behind the scenes.

Well it's my understanding that the other context identifiers on ThreadContext (APPLICATION, SECURITY, TRANSACTION) don't have any side-effect on CDI context propagation, which is completely controlled by ThreadContext.CDI.

So no, that's not what I meant.

Now the question is how many contexts we want to place into the spec for its 1.0. Just note that the model we have is extensible and we can experiment with additional contexts in impls and pull them into specs later on.

Well I guess at least naively that seems like a very easy question to answer: there should be an appropriate context identifier whenever any of the Microprofile specs require that information be propagated with the current thread.

I understand that the model is extensible, but the set of Microprofile specs is finite. In fact, it's rather small.

[njr-11]

after which I expect it would be automatically covered by CDI context propagation

That's not clear to me. The fact that you're using @Inject rather than @Context to inject something doesn't say anything at all about the semantics surrounding propagation of the corresponding injected objects between threads.

I went back to the JAX-RS expert group member and asked the direct question about whether they foresee creating a scope/scopes of their own or whether they expect to reuse existing CDI scopes for the injectable JAX-RS objects. His reply, while qualified that this hasn't been sufficiently discussed in the JAX-RS community, is that he foresees the use of existing CDI scopes, which would vary by type, with most being request scoped, and a few being application scoped or possibly session scoped. Given that the probable direction for JAX-RS is that @Context will be replaced by CDI @Inject with built-in CDI scopes that are either propagated by ThreadContext.CDI or are always available (CDI Application scope), there does not seem to be justification for introducing a JAX-RS request scoped constant, destined to become obsolete and adding confusion to the end user.

[njr-11]

I mean, look, if there's a need for ThreadContext.TRANSACTION, it's very difficult for me to understand how there's not a corresponding, equivalent, need for ThreadContext.REQUEST, since in both cases we have some threadbound context objects being managed by some infrastructure that is external to CDI.

I mean, you can write @Inject UserTransaction too, so does that mean we can just get rid of ThreadContext.TRANSACTION? Not as far as I can tell.

So what then is the difference between the two cases?

Despite the name, @Inject UserTransaction doesn't inject a transaction. CDI neither starts a transaction for you, nor do you get an instance of a transaction with a life cycle that is tied to any managed bean. What CDI is injecting here (UserTransaction) is just the opportunity to manage your own transactions. The JTA spec defines a model where a transaction is tied to the current thread, not the UserTransaction interface. MP Concurrency extends that with the ability to optionally transfer such a transaction (or absence thereof) to other threads, which means you need propagation/clearing of the actual transaction, not just the ability to access the same management interface.

The difference with JAX-RS usage of CDI context is in the details of the yet-to-be-officially-defined proposal. We can only speculate on it, albeit with the guidance and help of the JAX-RS community hinting at the future direction. And I'm told that we should prepare for injectable instances with life cycle that is tied to built-in CDI scopes. If that's how it is defined, then it is natural that by switching CDI scopes, the JAX-RS context that is tied to it must change as well. Ultimately, we need to wait and see. We should at least not do anything contradictory to this in the meantime.

[gavinking]

I agree that if the JAX-RS implementations refrain from storing any state in its own threadlocals, and stores everything in CDI contexts instead, then this feature is unnecessary.

However, this behavior would have to be mandated by the JAX-RS spec. Because I'm pretty sure that currently all JAX-RS implementations associate some state with the thread via their own threadlocals, and if the spec doesn't explicitly prohibit this, then they will most likely continue to do that, even after introducing support for @Inject.

[njr-11]

With this being the last issue remaining open against the MP Concurrency initial release, and it being unlikely to see an answer with 100% certainty from JAX-RS in the near term, I would like to propose that we keep this issue open and defer it to vNext of MP Concurrency. This will ensure that we don't put anything into the spec that ends up being incompatible/inconsistent with where JAX-RS is headed. Is this approach acceptable to everyone else?

[gavinking]

OK.