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?