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

Clarification on auto-discovery of singletons #605

Closed
mkarg opened this issue Mar 24, 2018 · 18 comments
Closed

Clarification on auto-discovery of singletons #605

mkarg opened this issue Mar 24, 2018 · 18 comments
Assignees

Comments

@mkarg
Copy link
Contributor

mkarg commented Mar 24, 2018

A JAX-RS application can explicitly declare singleton instances by implementing Application#getSingletons. There should be an unambiguous way to also declare singletons when not implementing this method, i. e. when using auto-discovery and annotations.

Example: One class is annotated as @javax.inject.Singleton and it implements Feature, ContainerRequestFilter and ContainerResponseFilter. A short test using Payara/Jersey discovered that actually two (not one, not three, but really two) instances of that class had been created. This is "near" to a singleton, but not exactly a singleton.

Implementors of JAX-RS extensions need a clear API-level way to ensure that their extension is always exactly one class at runtime, on any JAX-RS implementation, being used by any JAX-RS application. For example, a filter which gets configured (hence is a Feature) once and that puts infos at requests and analyzes this infos at responses MUST be exactly one single instance.

This clearly cannot be solved by the API but in fact is a spec issue. As there is no spec issue tracker right now, I put it here. My proposal is that we agree upon adding a sentence like: "A compliant implementation guarantees that for a class annotated with javax.inject.Singleton exactly one single instance is created, and that particular instance is treated as if it would have been returned by Application#getSingletons".

@spericas
Copy link
Contributor

spericas commented Mar 26, 2018 via email

@mkarg
Copy link
Contributor Author

mkarg commented Mar 26, 2018

@spericas The problem is that the JAX-RS spec does not say how to declare a singleton using annotations. So we both share the intention that it is a bug, but "formally" it is not. The spec completely leaves this question open to EJB and CDI. But what if neither is used? Also I would not say that it is clearly a bug that Jersey / Payara created two instances instead of one -- maybe this is by intention due to a good reason we simply do not see (I doubt that)?

What we should do in JAX-RS 2.2 is to explicitly say in the spec that @javax.inject.Singleton MUST produce exactly one single instance, even when neither EJB nor CDI is not used. Using this phrase, it is 100% clear that Jersey / Payara's current behaviour is definitively a bug and MUST fail the TCK.

@chkal
Copy link
Contributor

chkal commented Mar 26, 2018

I agree with @mkarg on this. There is currently no way to declare a singleton using annotations in the JAX-RS component model. Using @javax.inject.Singleton would be a way to solve this.

@spericas
Copy link
Contributor

spericas commented Mar 26, 2018 via email

@spericas
Copy link
Contributor

spericas commented Mar 26, 2018 via email

@mkarg
Copy link
Contributor Author

mkarg commented Mar 26, 2018

@spericas I really do share the vision to use CDI in 3.0 for the singleton topic, but I dislike the idea to wait for long time until we come up with a clear solution for the issue. In fact I want a solution for 2.2, and for non-CDI applications, too. Adding an annotation is not breaking compatibility, so there is no other technical force to wait for 3.0 besides you personally not wanting to invest the effort. Also you might call me nitpicking, but there is no singleton annotation in CDI, but only ApplicationScoped, which does not explicitly enforce exactly one single instance. IMHO the CDI spec does not say that it is forbidden to have more than one instance in the same scope (maybe I'm wrong here)?

@mkarg
Copy link
Contributor Author

mkarg commented Mar 26, 2018

@deki @andymc12 Please describe your opinions how to deal with annotation-based singleton declaration.

@spericas
Copy link
Contributor

spericas commented Mar 27, 2018 via email

@jansupol
Copy link
Contributor

There is @Singleton used in Section 9.4 of the Spec., on a resource. Having this precedence, it make sense to clarify. From the example, it is not clear whether it could be used on any resource, in general, whether it has a connection just to SSE (obviously not, but it is confusing), or whether the annotation makes sense on other JAX-RS classes, not just on a resource. It is possible that the example only expressed that the resource was defined in the Application#getSingletons().

@ggam
Copy link
Contributor

ggam commented Mar 27, 2018 via email

@andymc12
Copy link
Contributor

andymc12 commented Mar 27, 2018 via email

@ggam
Copy link
Contributor

ggam commented Mar 27, 2018 via email

@mkarg
Copy link
Contributor Author

mkarg commented Mar 27, 2018

@spericas

By default, providers are singletons in JAX-RS already.

Can you please point me to the place where this is mandated by the JAX-RS specification document and / or API? I just can't find that. It would actually solve this issue, because if it is explicitly told, then Jersey is definitively broken, as it creates one instance per interface, not one instance per class.

@mkarg
Copy link
Contributor Author

mkarg commented Mar 27, 2018

@andymc12 Actually this does not help. The question of this issue is: How to annotate singletons without CDI and without EJB. Your link is the reverse of what this thread tries to clarify.

@spericas
Copy link
Contributor

spericas commented Mar 27, 2018 via email

@mkarg
Copy link
Contributor Author

mkarg commented Mar 27, 2018

@spericas You mean By default a single instance of each provider class is instantiated for each JAX-RS application., right? Right, seems I missed that line. So Jersey definitively has a bug and I will file an issue once the project is migrated to eclipse-ee4j. Also, I do not see the need for a @Singleton clarification in the spec anymore due to that. Thanks! :-)

@chkal As my question is resolved, I would close this issue. You voted +1, so do you want to keept this issue open?

@chkal
Copy link
Contributor

chkal commented Mar 27, 2018

@spericas Thanks for pointing us to the corresponding statement in the spec. In this case we don't need any clarification. Seems like I also missed that one. Sorry about that.

@mkarg Feel free to close this issue.

@mkarg
Copy link
Contributor Author

mkarg commented Mar 27, 2018

Closing as requested clarification is already part of the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants