Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discovery of JAX-RS resources in MicroProfile #50

Open
NottyCode opened this issue Jul 18, 2018 · 67 comments
Open

Discovery of JAX-RS resources in MicroProfile #50

NottyCode opened this issue Jul 18, 2018 · 67 comments
Labels
Architecture board Issues across more MP specifications

Comments

@NottyCode
Copy link
Member

There was an issue raised against the Open API TCK to resolve how JAX-RS resources get discovered. The issue was closed with a reference to issue #32 based on a discussion on the hangout. I wasn't on the hangout, but reading issue 32 I'm concerned that this issue might be lost so I'm raising this to specifically cover it.

The example sited from section 11.2 of the JAX-RS specification is incomplete for how CDI discovery works, so perhaps the MicroProfile spec could provide a modern and complete example.

@kwsutter kwsutter added the Architecture board Issues across more MP specifications label Jul 18, 2018
@kenfinnigan
Copy link

Is this resolved by #49 setting all JAX-RS Resources to be @RequestScoped? In the architecture call we thought it would, but we wanted to verify that

@struberg
Copy link

struberg commented Oct 8, 2018

Strong -1
Note that the behaviour IS specified in EE8!

Auto-assigning @RequestScoped would introduce non-compatibility with EE servers!
The EE spec says that a class MUST be a CDI bean to be picked up at first. (Except you manually register the resource classes/Application via web.xml)
So either have a META-INF/beans.xml in that jar or have a bean-defining annotation.
In the later case the Scope is AUTOMATICALLY @dependent!
Yes, that doesn't fit well with JAX-RS, but that's CDI rules.

If we would now define that any JAX-RS resource which has no scope would automatically be @RequestScoped, then this would break CDI rules I fear.

@rmannibucau @antoinesd @mkarg @chkal plz review and also state an opinion. txs!

@rmannibucau
Copy link

Same here. Fix it in jaxrs and dont fork it in mp.

@mkarg
Copy link
Member

mkarg commented Oct 8, 2018

@struberg JAX-RS 2.1 spec says that JAX-RS applications and JAX-RS providers MUST be application scoped and JAX-RS resources MUST be request scoped, so it would be simply logical to make it @RequestScoped by default. We could add this requirement to the JAX-RS 3.0 specification, so it is unambiguously defined and the CDI- and MP-specs can be free of JAX-RS-special-cases. There is a common concesus by the JAX-RS committers anyways to mandate by JAX-RS 3.0 that all JAX-RS resources MUST be CDI beans (yes, this will be backwards-incompatible, that's why we don't do it in JAX-RS 2.2 already).

@rmannibucau
Copy link

Hmm, if true it mist be reverted at spec level since it is a breaking change in all impl. Only default scope is ambiguous and is request scope but any scope is valid. Also providers are dependent (of the app) and not application scoped by default.

At least it was like that in 2.0 and i cant believe it has been broken in 2.1.

@mkarg
Copy link
Member

mkarg commented Oct 8, 2018

As I said, we will break backwards compatibility in JAX-RS 3.x. This already is common sense of the JAX-RS committers. We actually will get rid of the outdated JAX-RS component model in favor of CDI wherever it stands in the way. CDI shall be the core of JAX-RS in future, not just an add-on. That's why we do it in 3.x but not 2.x.

JAX-RS Spec 2.1 chapter 11.2.3: Providers and Application subclasses MUST be singletons or use application scope.. See, I said application scope, not @ApplicationScoped.

@struberg
Copy link

struberg commented Oct 8, 2018

If it's backward incompatible in JAX-RS-3.x then you must not do it per JCP rules. And even for JakartaEE it's a bad idea.

But anyway, the initial question was about the situation right now. What is defined right now?

@rmannibucau
Copy link

Hmm @RequestScoped for resource by default is against CDI and if the goal is to align on CDI then just say the scope is the cdi one - of the resource bean. This is bckward compatible and works.

Side note: currznt recomlended scope for resource in apps is application scoped so request scoped sounds like a huge regression and will enforce code changes...+ it goes against cdi model

@chkal
Copy link

chkal commented Oct 9, 2018

Let me share a few thoughts about this.

Auto-assigning @RequestScoped would introduce non-compatibility with EE servers!

AFAIK Wildfly is already doing it this way today. Please see this mail. Perhaps @asoldano can confirm this?

Only default scope is ambiguous and is request scope but any scope is valid. Also providers are dependent (of the app) and not application scoped by default.

I already brought up the topic of CDI resources and their lifecycle on the JAX-RS mailing list earlier this year. See this mail for details.

In a nutshell the JAX-RS spec states:

  • 3.1.1 Lifecycle and Environment:
    • "By default a new resource class instance is created for each request to that resource."
  • 11.2.3 Context and Dependency Injection (CDI):
    • "In a product that supports CDI, implementations MUST support the use of CDI-style Beans as root resource classes"

My first interpretation was actually that JAX-RS requires CDI resources to be request-scoped. But after some discussions @mkarg provided a great summary:

I think the misunderstanding is that JAX-RS's phrase "per request" is not necessarily @requestScope. CDI can reach the target of request scoping by many ways, using several annotations or even programmatic solutions. JAX-RS does not enforce "@RequestScoped" literally, only "effectively".

So basically it is fine that CDI resources and providers are dependent-scoped by default. It is up to the JAX-RS implementation to lookup/create instances either once per request or only once for the application.

So this is the current interpretation. But I agree that the spec is rather vague on the concrete integration requirements between CDI and JAX-RS. Especially I'm very unhappy about the term "CDI-style beans". What does this mean exactly? What is the difference between a "CDI-style bean" and a "CDI bean". I would really like to clarify this in JAX-RS.

@struberg
Copy link

struberg commented Oct 9, 2018

I think I can shed a light on "CDI-style bean" vs "CDI bean".

  • "CDI Bean" is a terminus tecnicus of the CDI specification. It basically means meta-information about a single factory. So it's a factory rule represented by a single instance of Bean<T> extends Contextual<T>. (And for the rest of the story please check the terms 'Contextual Instance' and 'Contextual Reference' as outlined by Pete and me in our ancient article on that topic [1])

  • "CDI style bean" most likely means any Bean picked up by the CDI Container and any Contextual Reference managed by that CDI Container. So anything you can resolve via BeanManager#getReference()

[1] https://jaxenter.com/tutorial-introduction-to-cdi-contexts-and-dependency-injection-for-java-ee-jsr-299-104536.html

@asoldano
Copy link

Auto-assigning @RequestScoped would introduce non-compatibility with EE servers!

AFAIK Wildfly is already doing it this way today. Please see this mail. Perhaps @asoldano can confirm this?

http://docs.jboss.org/resteasy/docs/3.6.1.Final/userguide/html_single/index.html#CDI

@chkal
Copy link

chkal commented Oct 11, 2018

The relevant part from this section:

48.2. Default scopes
A CDI bean that does not explicitly define a scope is @Dependent scoped by default. This pseudo scope means that the bean adapts to the lifecycle of the bean it is injected into. Normal scopes (request, session, application) are more suitable for JAX-RS components as they designate component's lifecycle boundaries explicitly. Therefore, the resteasy-cdi module alters the default scoping in the following way:

  • If a JAX-RS root resource does not define a scope explicitly, it is bound to the Request scope.
  • If a JAX-RS Provider or javax.ws.rs.Application subclass does not define a scope explicitly, it is bound to the Application scope.

@ljnelson
Copy link

ljnelson commented Nov 8, 2018

Just a passing note on this: I've seen a couple mentions here and elsewhere about what Java (or Jakarta) EE does or says with regard to the interaction between CDI and JAX-RS. But this is MicroProfile and there is no defined dependency in the MicroProfile effort on the Java (or Jakarta) EE Platform Specification that I am aware of, so surely such mentions are advisory at best. Indeed, at the moment, it seems to me that only the (not really extant, but planned) MicroProfile "umbrella" specification (see #49 and relevant discussion) together with a perhaps clarified section 11.2.3 of the JAX-RS specification governs this interaction.

(Slightly related issue: jakartaee/rest#698.)

@rmannibucau
Copy link

@ljnelson the point was that if MP diverges from EE it is just a small library like a tons we find on the net. The only strength of MP is to give eagerly future EE features in a "close enough" shape to enable costless migrations (sed more or less). In that context it can't do anything ans must take care it will converge in case of any ambiguity.

@ljnelson
Copy link

ljnelson commented Nov 9, 2018

@rmannibucau I don't have an opinion on this one way or the other. I just wanted to point out that as of November 9, 2018 there is no platform specification for MicroProfile that defines behaviors arising from the interaction of its component specifications. There will, IMHO, need to be if there is a desire to standardize such behaviors.

@kenfinnigan
Copy link

In today's MP Architecture call we discussed again the resolution to #49 in light of the above comments.

As there was no agreement this was the appropriate approach, it was agreed that further discussion on this issue was warranted

@kenfinnigan
Copy link

Would it be worth considering marking all JAX-RS Resources as @ApplicationScoped if no other scope is defined?

@rmannibucau
Copy link

CDI spec defines them as @dependent already

@kenfinnigan
Copy link

Do you have a reference where it refers to that? My understanding any definition of that was at the Java EE specification level, not an individual spec.

Even so, there's nothing saying we couldn't prefer a different scope for MicroProfile.

@rmannibucau
Copy link

In cdi spec no expicit defined scope implies dependent, mp only uses cdi for scanning so all classses are cdi beans including jaxrs resources, put them together since you dont want to break user or go against jakartaee and you have your default well specified - even more than ee which opened the door to vendor specific mecanism preventing the portability in some (> cdi) cases.

Also this wouldnt be compatible with some config, jwtauth and failsafe feature so likely better to stick with already spec-ed defaults.

@kenfinnigan
Copy link

I didn't think JAX-RS Resource classes were automatically CDI beans unless a scope is defined.

I don't see any issues with MicroProfile diverging with what JavaEE/JakartaEE might define, if we think it's the right approach.

@rmannibucau
Copy link

Well for me it is a clear blocker since it means Microprofile does not respect the specifications it is based upon which should like not being consistent with itself and pretty much not usable (and doing reusable libraries will be impossible). So at the end Microprofile libs and code couldn't be deployed in a EE container without a huge impact. Lastly it does not bring any feature to the user even if I agree the default would have been better for perfs - not done cause the proxying rules are too restrictive to be general.

That said it is trivial to do with a CDI extension which just change the scope of any bean with @path so maybe do it as an extension and that's it?

@kenfinnigan
Copy link

You appear to argue against a change and then say it's easy to do, so I'm confused.

All I'm saying is that if we as a community feel something other than @Dependent should be the scope for JAX-RS Resources, then there's nothing saying we can't do that

@rmannibucau
Copy link

@kenfinnigan it is easy to provide as a custom extension user can import or not, but shouldn't be in the platform IMO

@kenfinnigan
Copy link

Ok, that's your opinion, but I'd like to have the wider community comment before we close this

@rdebusscher
Copy link
Member

I thought JAX-RS resources are non-contextual instances. http://www.next-presso.com/2017/06/non-contextual-instances-in-cdi/

Not created by CDI container (and thus not injectable somewhere else) but you can inject beans into it.

The JAX-RS spec also says "By default JAX-RS resource classes are per-request and all providers are singletons" which is comparable to @RequestScoped (but not the same as they aren't actual CDI beans)

@kenfinnigan
Copy link

I believe that's the current state.

I'm interested in seeing whether the community thinks @ApplicationScoped is a good default for JAX-RS Resources in MicroProfile

@mkarg
Copy link
Member

mkarg commented Feb 23, 2019

Quoting JAX-RS 2.1 specification chapter 3.1.1 Lifecycle and Environment: "By default a new resource class instance is created for each request to that resource". So @ApplicationScoped is not just a weird default, it is actually wrong default.

@chkal
Copy link

chkal commented Feb 23, 2019

I would really be interested in the reasoning behind the idea of defining a new default. What problem are we trying to solve?

@Emily-Jiang
Copy link
Member

I thought this a bit further. CDI spec does not say anything about JAX-RS, which is not a bad thing. Java EE spec only requires CDI-managed JAX-RS resources support injection and interceptor support. Since this minimal requirements, some application server provides more support, e.g automatically making JAX-RS as CDI beans to support injection. IIRC, we want to reach consensus among application servers. Should we spec clearly on the interaction between CDI and JAX-RS?
In MP, we have 2 options:

  1. we could leave as it is and let Jakarta EE to clarify or JAX-RS 3.0 to fix this as per @chkal
  2. Fix Java EE spec and changes JAX-RS to force each JAX-RS resources to be managed by CDI via the notion of defaulting JAX-RS resources with a bean defining annotations such as ‘RequestScoped’.
    Since JAX-RS 3.0 is also addressing this issue. I think it is much safer to leave to Jakarta EE to fix this. Option 1 is safer and it will work better in the long run. Option 2 might bring more confusion and introduce potential conflicts as per Romain’s concern.

@struberg
Copy link

struberg commented Feb 23, 2019

We need to go back a few meters and probably look where all this discussion started. I have the feeling that the completely side-tracked again.

The JAX-RS spec has a few discovery methods. Most of them do not apply to MicroProfile. The only one being a 'Managed Bean'. Within a jar without any META-INF/beans.xml this is only the case if it has a bean defining annotation as per the CDI spec. Which means you need to give this class a proper CDI scope. Like e.g. @ApplicationScoped or @RequestScoped. If you do have a beans.xml (explicit bean archive case), then almost every class gets picked up as CDI bean. In this case the default scope is @Dependent.

This is what the spec says right now. AND THIS IS PERFECTLY FINE - NO NEED TO CHANGE ANYTHING!

The only problem which happened is that Liberty seems to pick up classes without any CDI annotation nor beans.xml, as long as they have a @Path annotation. That's not by itself a problem, but someone wrote the TCK tests in some mp subproject to require this behaviour - which is simply not portable!

Of course Liberty can continue to pick those classes up. But we should stop writing non-portable TCK tests! For those we should either provide a proper META-INF/beans.xml or annotate the JAX-RS resources with some proper CDI scope. Case closed.

@rmannibucau
Copy link

If it helps: this is a common EE behavior to scan all classes outside CDI too but saner for MP to keep it vendor specific since impls diverge in the way they handle it - and it is fine.

@ljnelson
Copy link

Point 1 is not cause you reference the vendor extension point ignoring cdi references which delegate to cdi all the definitiin and MP is exactly there.

I am having a hard time understanding this sentence; I apologize.

Doesn't JAX-RS 2.1 section 2.3.3 give a non-Servlet-based vendor the freedom to "host a JAX-RS application in other types of container [sic]" in pretty much any way they see fit, given that (a) "such facilities are outside the scope of this specification" and (b) automatic discovery of root resource classes and provider classes is not required of non-Servlet-based vendors by the JAX-RS specification?

If indeed automatic discovery of root resource classes and provider classes is required of only Servlet-based implementations (2.3.2) but MicroProfile is not such a thing (2.3.3), and if MicroProfile is not Jakarta EE (it's not), and if "an implementation MAY offer other resource class lifecycles" (3.1.1) then if MicroProfile is going to integrate CDI with JAX-RS, can't MicroProfile do it any way it likes? That is my read of @kenfinnigan's proposal to have the (currently nonexistent but necessary) MicroProfile umbrella specification treat resource classes as though they were annotated with @ApplicationScoped for purposes of CDI integration. I am deliberately withholding judgment in this discussion so far on whether that's a good idea or a bad one—it seems like it must be a valid idea, however.

If you believe that MicroProfile is actually constrained in this regard, i.e. that MicroProfile cannot technically integrate JAX-RS 2.1 with CDI 2.0 in any way it likes, could you explain why, without referring to Jakarta EE?

@rmannibucau
Copy link

MP is not under 2.3.3 in jaxrs spec but all parts - there are multiple - referencing and relying on CDI. I dont refer about EE but only CDI and JAX RS saying scope is defined. EE just enforces it.

@ljnelson
Copy link

MP is not under 2.3.3 in jaxrs spec but all parts - there are multiple - referencing and relying on CDI.

Right; section 11.2.3 in particular, which says that "in a product that supports CDI" (that's MicroProfile) an implementation of JAX-RS "MUST support the use of CDI-style Beans [sic] as root resource classes, providers and Application subclasses. Providers and Application subclasses MUST be singletons or use application scope."

So fair enough, in the case of an Application subclass and any provider class that is instantiated by this implementation, or by the CDI-supporting product the implementation is subsumed under, such a class must be forced by the product or the implementation to be a singleton or to behave as though annotated with ApplicationScoped.

But there is no such restriction on root resource classes, right? So can't MicroProfile do what it wants here? I believe that is why Ken's proposal is not on the face of it invalid.

I dont refer about EE but only CDI and JAX RS saying scope is defined. EE just enforces it.

Could you point in the JAX-RS 2.1 specification where in fact the scope of a root resource class is defined in the context of a JAX-RS 2.1 implementation "belonging to" a product supporting CDI? I didn't find it in section 11.2.3 or section 11.2.8 which are the only two I could find that talk about "CDI-style Beans" (whatever those are, but that's a subject for a different day 😄 ).

@rmannibucau
Copy link

You answered yourself actually: the scope is not defined so it inherits from CDI @dependent. Also please keep in mind it is like that for users (libraries and app writers) since JAX-RS 1.0 and works well. It also guarantees microprogike works (metrics and jwt auth are in my mind).

Now as Mark stated this is not this issue topic so let's agree cdi resources are discovered and other discovery mecanism are vendor specific and let's move on on features with some more value ;).

@ljnelson
Copy link

You answered yourself actually: the scope is not defined so it inherits from CDI @dependent.

I think the issue that Ken is raising (and I might still be misinterpreting him) is that an umbrella specification can still change this default without breaking anything in any of its subordinate specifications.

I'll detach from the discussion so as not to add more noise!

@kenfinnigan
Copy link

Thanks @ljnelson, you've represented my thoughts very well.

CDI specification defines that beans without a scope are @Dependent.

JAX-RS says Resources that are not added via the Application class will be instantiated per request. Those added via Application are singletons.

As others have said above, I don't see anything in either CDI or JAX-RS specifications that prevents MicroProfile defining a different scope than @Dependent for JAX-RS Resources.

Though the issue of discovering JAX-RS Resources in MicroProfile may have begun due to an issue in a TCK, that doesn't mean it's not a valid issue to be discussed.

I don't consider an argument that it's always been done that way as a reason to keep doing it that way when there are alternatives that could suit better.

With MicroProfile we're attempting to develop pieces that are appropriate for microservices and cloud development. For that reason, I don't see treating every JAX-RS Resource as @Dependent or even @RequestScoped as appropriate. In an age where memory is critical, with us trying to eek out more throughput for the same memory utilization, can we honestly say that instantiating an instance per request is the best choice?

In addition, defining @ApplicationScoped as a default would not only improve memory utilization, but also force developers to be better at developing thread safe code.

@rmannibucau
Copy link

....microprofile itself is a reason to not change scopes, please check other specs as mentionned. Also except perf which are not impacted enough to discuss something breaking the platform, it breaks usages. MP can choose to be unstable but if you fo ensure to do as much comm on that as all other communications to ensure people know it cant be chosen gor professional projects.

Contrarly to what you say it is in jaxrs spec that cdi default scope is dependent if you read it strictly: the bean is looked up per request, with no explicit scope it is dependent and therefore behaves as request scope without the proxying constraints. Easy and written as you point out.

Finally app scope would make default not thread safe anymore - and break existing code and ecosystem - so I fear you are not true at all on that too.

@kenfinnigan
Copy link

MicroProfile is planning to break backwards compatibility once a year, why should that be limited to the specifications that MicroProfile creates? There's no reason it can't be applied to how other specifications interact within MicroProfile.

As I pointed out in my last message, I don't think it's a bad thing to push developers to be thread safe. It improves the programming model and is more performant than just creating state everywhere. Yes it requires some forethought, but in my view that's not a bad thing.

At any rate, you can have your opinion that it shouldn't change, just as I can have a differing opinion. Neither is wrong or right, it's all about particular choices and tradeoffs.

None of this should preclude a discussion being had as to whether the wider community thinks such a change is appropriate or not.

@rmannibucau
Copy link

You should read messages so let me repeat:

  • breaking changes cant affect any element of the stack which has its own ecosystem or you break that ecosystem which is very highly a blocker for users
  • it is good to be thread safe by default and it is ee default for years....this is not app scope though
  • it is fine we disagree but you opinion does not repredent the majority if you read that thread so community disagree with you for now

@struberg
Copy link

struberg commented Feb 24, 2019

Oh gosh, @kenfinnigan this sentence might be very controversial:

MicroProfile is planning to break backwards compatibility once a year,

We are fine to break backwards compatibility with our OWN specs. But we certainly will not break backward compatibility with official EE specs we refer to. At least that is my understanding. Feel free to bring this up to our next mp hangout agenda.

@kenfinnigan
Copy link

@rmannibucau, I don't see why breaking changes can't affect any element of the stack. That's precisely why they're called breaking changes. Furthermore, if we're not going to accept any breaking changes and stick with what's been done before, then MicroProfile might as well close up shop and merge with JakartaEE. The whole point of MicroProfile is to push the envelope faster than Java EE could before, sticking with what works doesn't fit that mandate in my view.

I never said my opinion represented the majority, I'm merely trying to open a discussion about whether we want to make such a change. Conversely, I wouldn't consider the few people who have commented here as creating a majority for your opinion either.

@struberg, well it's what we've decided to date, that there could be a breaking change release every year. Doesn't mean to say there will be, but we're planning for one a year.

I wouldn't consider the change I've been describing as breaking either the CDI or JAX-RS specifications. I consider it to be clearly defining behavior between those two specifications when executing in a MicroProfile environment.

@rmannibucau
Copy link

@kenfinnigan right, since now JakartaEE is @ eclipse then no more need of microprofile umbrella anymore. Personally I really hope it will bring more CDI-friendly API (vs copy of SE API).
Anyway, your change is breaking user code since today it is spec-ed as request scoped (whatever derived it is - proxied or not using @dependent) and therefore the bean does not have to handle any concurrency and you change would break that game and break jwt-auth injections (+ jaxrs params injection as it has been mentionned). So yes it is a big breaking change which shouldn't even enable to pass MP tck strictly speaking - I didn't check if there were covering on that point but since it is in the spec it should.

@struberg
Copy link

@kenfinnigan I attend the mp meetings as well. We decided that we can break OUR apis once a year. And also only if needed! There is a difference between evolving our stuff and breaking other stuff.

@kenfinnigan
Copy link

@rmannibucau I don't believe such a change would break any TCK. MicroProfile does not require passing the CDI or JAX-RS TCKs.

@struberg for sure it's not guaranteed to happen every year, but initially I could see that being the case is things are still evolving rapidly. In my view MicroProfile doesn't need to be compatible with Java EE applications today. My reason for that is because we just end up in a situation where no progress or advancements are made if we don't say "something needs to change".

Now I'm absolutely not saying the change I'm proposing has to be that change to break things, but we at least need to be prepared to make those decisions otherwise we become stale.

@rmannibucau
Copy link

@kenfinnigan I agree sometimes you must break things. Typically that MP state that anything outside CDI is not supported where JAX-RS has a SE mode would make sense but being based on CDI I strongly think MP can't go against its defaults.

@konbk
Copy link

konbk commented May 25, 2020

Hi, heavy WildFly user here,

@kenfinnigan, As I understand, you would like MicroProfile to specify different behavior than that already defined in Java EE/Jakarta EE for resources without any scope annotations. Currently, JAX-RS resources in WildFly are not CDI beans by default but behave mostly like request scoped beans. WildFly successfully implements Java EE/Jakarta EE and MicroProfile. But after the proposed change, how do you imagine WildFly implementing two contradicting specifications at the same time?

@arjantijms
Copy link

But after the proposed change, how do you imagine WildFly implementing two contradicting specifications at the same time?

Did anyone ever take this question into account? I'm curious about that too, and since in Piranha we implement both EE and MP I may face this exact same problem. @kenfinnigan any thoughts on this?

@kenfinnigan
Copy link

I don't believe it's been looked into, but I could imagine it behaves differently depending on the type of application.

By that I mean, as soon as MP dependencies are included, JAX-RS scope would switch to MP desired behavior

@ederks85
Copy link
Contributor

ederks85 commented Dec 14, 2020 via email

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Dec 14, 2020

Nothing right now blocks end users if you stick with Jakarta EE spec.

  • In order to enable your application compatible, stick to what Jakarta EE platform spec defined and don't use some additional features provided by some runtime, why means:

Annotate explicitly with CDI bean defining annotation on your JAX-RS resources, such as RequestScoped etc.

If you don't use CDI annotation and you have beans.xml, the resources will be defaulted to Dependent Scoped.
Anything else you will run into the defined behaviour by different runtime. Jakarta EE platform spec said very clearly:

Only CDI-managed JAX-RS resources supports injection and interceptors, which means you need to explicitly make JAX-RS resources to be CDI beans via bean-defining annotations or beans.xml.

In the future JAX-RS spec, it might be better to update @Path to automatically be a bean defining annotations with the similar behaviour of RequestScoped. In this case, there won't be any ambiguity.

@rmannibucau
Copy link

@Emily-Jiang not sure where the Only CDI-managed JAX-RS resources supports injection and interceptors, which means you need to explicitly make JAX-RS resources to be CDI beans via bean-defining annotations or beans.xml. comes from but it is clearly not the case in Jakarta due to EJB, war etc specifications and TCK. Overall point is that any microprofile specification can only rely on CDI discovery, so only CDI beans can be JAX-RS resources by construction - anything else is vendor dependent which is fine but not usable by the specs. Maybe @Path defining means CDI would know about JAX-RS which is not an option if we want things to stay simple, clear and usable (why would we duplicate discovery in 2 spec, in particular when one rely on the other). So at the end Microprofile only defines CDI for the discovery, in an EE container it can be defined as such (Microprofile integration will only be active for CDI beans, including EJB) and we are good. All other options are broken at some point and will need to fork Microprofile way more than needed for what it does IMHO.

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Dec 14, 2020

@rmannibucau see here , quoted from Java EE8 platform spec doc
image
I did not argue about the CDI discovery. If you re-read my earlier comments again, I mentioned CDI bean-defining annotation or beans.xml, which covers the ground of CDI discovering all beans.

@rmannibucau
Copy link

@Emily-Jiang this document is highly likely wrongly phrased since any web component must support EE injections for current deployment (whatever it is for the app, either CDI or actual EE as @Resource/@EJB) and also support EJB which have its enriched injection set too. This is in TCK as well so guess it is an "unlucky" phrasing/note. Now, the overall point of this ticket - and original one to be very concrete - was to write in microprofile spec:

  1. Microprofile implementation are only required to support CDI discovery
  2. EE vendors (or other vendors) can extend that to EE discovery mecanism - including vendor specific ones - but it is outside of the scope of Microprofile to define those
  3. bis: note that it includes GraalVM build time discoveries which bypasses CDI extensions at runtime - but once again it is outside of the scope of MP
  4. probably write that CDI discovery can depend on Microprofile container since there is nothing done at Microprofile on that area today. Concretely it means a servlet based CDI container will use web discovery (with WEB-INF/beans.xml support to give an example of a specific difference you can encounter), a CDI SE based container will use SeContainer discovery etc...

Side note: I'm not asking to define 3 in MP since it already belongs to CDI and there is no real point aligning it between vendors since user knows in which context he is and it does not impact the code at all. It is just about being clear of what is in the scope of the spec and what is not + clearly define how user code is discovered (issue being: not scanned/discovered code it silently ignored which is surprising and sometimes hard to debug for end users).

Hope it makes more sense this way.

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Dec 14, 2020

@Emily-Jiang this document is highly likely wrongly phrased since any web component must support EE injections for current deployment (whatever it is for the app, either CDI or actual EE as @Resource/@EJB) and also support EJB which have its enriched injection set too. This is in TCK as well so guess it is an "unlucky" phrasing/note.

I disagree with the above statement. End users relies on the Java EE/Jakarta EE spec to develop their applications. If you think the spec is wrong, the right approach is to raise an issue to get it corrected. From my understanding, the spec stated very clearly. Each Java EE component classes have the different requirements, so there was further clarification below the table.

Now, the overall point of this ticket - and original one to be very concrete - was to write in microprofile spec:

1. Microprofile implementation are only required to support CDI discovery

2. EE vendors (or other vendors) can extend that to EE discovery mecanism - including vendor specific ones - but it is outside of the scope of Microprofile to define those

3. bis: note that it includes GraalVM build time discoveries which bypasses CDI extensions at runtime - but once again it is outside of the scope of MP

4. probably write that CDI discovery can depend on Microprofile container since there is nothing done at Microprofile on that area today. Concretely it means a servlet based CDI container will use web discovery (with WEB-INF/beans.xml support to give an example of a specific difference you can encounter), a CDI SE based container will use SeContainer discovery etc...

Side note: I'm not asking to define 3 in MP since it already belongs to CDI and there is no real point aligning it between vendors since user knows in which context he is and it does not impact the code at all. It is just about being clear of what is in the scope of the spec and what is not + clearly define how user code is discovered (issue being: not scanned/discovered code it silently ignored which is surprising and sometimes hard to debug for end users).

Hope it makes more sense this way.

My take is that it is best for JAX-RS spec to fix this. I searched JAX-RS issues and found this issue is exactly what needed. I suggeset to close this very issue in MP and pursuit a fix from JAX-RS because JAX-RS is under active development. In my view, it is wrong to define a behaviour for Jakarta EE as this will lead to problems in the long run since Jakarta EE specs do not reside in MP and it might define a complete different behaviour.

@rmannibucau
Copy link

@Emily-Jiang

I disagree with the above statement.

Sadly there is nothing to agree or disagree, factually EE does not do what you write - take any container from liberty to tomee without forgetting wildfly and friends. It is also in the EE spec as well (from TCK to PDF/textual specs). Don't forget you use "CDI => injection" but you don't have the "Not CDI => x" part of the statement which is covered by the specs and does what I described.

So back to the point that for microprofile only CDI model exists, EE model is possible but out of official support - until EE integrated MP which is unlikely by design.

My take is that it is best for JAX-RS spec to fix this.

Here again it is just not possible without JAX-RS integrating MP which is by design not possible and not desired. The issue is "what is a bean for the spec" and since the spec can only rely on CDI as an IoC (= what sees beans/model) then the spec can only explicitly define what is supported for CDI, nothing to discuss there again.
Once again it does not prevent any larger/enhanced support, it is just defining what is the spec about and what does mean portability for the spec.

Also note that the issue you mention - jakartaee/rest#556 - does not solve that issue at all, it is only for one particular case where you don't have a beans.xml which is not more common than the case you have one or the case you have one with trimming enabled etc...All these cases are covered by CDI so MP relies on CDI scanning for all its specs, including the ones using JAX-RS. It is by construction and complies with all vendors until you abuse the wording testing a EE bean (but not a CDI bean) is ignored - which shouldn't be tested, it should be let to vendors to decide. However, if a bean is a CDI bean and is not vetoed then it is taken into account by MP platform (CDI => MP is aware of it, anything else => vendor specific).

This enables EE and CDI to work together properly in all cases and all platform. Last note on that integration is that EE app not using CDI are legacy, must be MP "integrable" - to add metrics, tracing etc in a vendor fashion - but there is no point defining anything else at MP level except making the platform inconsistent as explained in the previous ticket and broken by a very fragile definition - note that EE has this issue already between spec duplicating the IoC in pre-CDI times. This is why, since MP has a native and natural definition of the discovery MP should just stick with it and not try to either not do its job - defining explicitly it - or do more - defining EE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture board Issues across more MP specifications
Projects
None yet
Development

No branches or pull requests