jakartaee / nosql

The specification in Jakarta EE to help Jakarta EE developers create enterprise-grade applications using Java® and NoSQL technologies.
https://projects.eclipse.org/projects/ee4j.nosql
Eclipse Public License 2.0
92 stars 28 forks source link

Add mechanism to ServiceLoaderProvider to set a custom loader implementation #84

Closed jesse-gallagher closed 2 years ago

jesse-gallagher commented 2 years ago

The intent here is to add a way for developers to specify a custom service-loading mechanism that can be used instead of ServiceLoader. This is useful for environments like what I'm working with at the moment, which is OSGi but without a ServiceLoader bridge implementation.

eclipse-nosql-bot commented 2 years ago

Can one of the admins verify this patch?

otaviojava commented 2 years ago

I have a concern about it, mainly because it is not stateless. It might get a race condition and not safe.

jesse-gallagher commented 2 years ago

I can see that. I figure something like this is useful, but I'm not married to this specific implementation. It's somewhat modeled after the pattern that shows up in some MicroProfile specs like in Config, but made a bit weirder here because of the sheer number of ServiceLoader-found classes NoSQL uses. Looking at that made me realize now I should at least mark the member volatile, which I've done now.

Could it suffice to expand the Javadoc to include a warning similar to in the Config API to further explain that it's intended just for exceptional, managed circumstances?

jesse-gallagher commented 2 years ago

I can see that. How about the notion of going the GlassFish hk2 osgi-resource-locator route? While it ends up a little ungainly in practice, it'd be in line with other existing JEE specs like Mail and JAX-B, while still being customizable via the HK2 side in cases like mine. I could investigate switching this over to that route in a reflective way like those cases.

otaviojava commented 2 years ago

@jesse-gallagher I like this approach. Maybe, create a project in the extension repository.

jesse-gallagher commented 2 years ago

I've modified this commit to use the HK2 ServiceLoader in the same fashion as the older JEE specs. To my knowledge, this pretty much has to happen in this class (as opposed to in an extension), as OSGi doesn't let you override a class in-place without getting into the complicated business of bytecode weaving.

jesse-gallagher commented 2 years ago

Hmm, this is a bit of a mess with the unrelated Jakarta commits. For cleanliness, I'm going to close this one and make a separate PR with only this file changed.