-
Notifications
You must be signed in to change notification settings - Fork 100
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
Fix630 2 #633
Fix630 2 #633
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SPI Fly is not available in GlassFish (yet?), so the API jar will stop working there after this change. Is there a way for both approaches to co-exist? Alternatively what changes need to be done on the GlassFish side to support this approach, preferably with keeping backward compatibility?
I don't know enough about GlassFish. I always thought it is a "J2EE server" (old name, sorry, I stopped using J2EE a while ago). The change does not require SPI Fly to be available in a non-OSGi environment.There is no dependency whatsoever. It' just a few declarations that take effect in OSGi environments. Is GlassFish itself built on OSGi? |
Yes, it is (it used to support felix, equinox and knopplerfish runtimes) and that is the main reason behind HK2 resource loader support in many APIs as a way to support service loader look up. |
Okay, I just found that GlassFish "includes" Apache Felix (quite a lot of 3rd party libraries...) So if you use GlassFisch (actually Felix) as your OSGi environment then currently, you have to add the hk2-bundle to your runtime configuration. Else it won't work (it actually doesn't even work then because of #629 (comment) -- this will have to be fixed if this PR isn't accepted). After merging this pull request, all you have to do is add the SPI Fly Bundle(s) to your list of runtime bundles. I suppose GlassFish doesn't use only the Apache Felix OSGi "core" framework, it also uses additional bundles from the "Apache Felix Ecosystem" that provide features such as SCR or the metadata service, config admin etc. The only difference between adding these bundles at runtime and adding the SPI Fly bundles is that the SPI Fly bundles aren't provided by "Apache Felix" but by by "Apache Aries". Don't ask me why this particular aspect of implementing OSGi specifications has it's own name instead of being an Apache Felix subproject. |
HK2 is there "by default", it is one of the core libraries around which GF was originally built (in v3, I think)
see the core pom for what is there and what is not
GF would need to support (=include) Apache Aries "by default" first to allow APIs to remove integration with the HK2 resource loader |
Really, I don't care about politics. I think using the "standard" OSGi solution is the better way to go. But the only point that I'm really interested in is that the mail API continues to work in OSGi environments. So if you close this PR, please consider giving appropriate priority to the problem mentioned in #629 (comment). This PR would have solved the problem mentioned there as a side effect. But as this PR does not seem to happen, I've added an issue for the problem mentioned in the comment so that it get the necessary attention (#635). |
I'm not saying that I disagree with the support for the better way. All I'm saying is - can both co-exist together? If so, why not just add what is required while keeping the rest untouched? |
They can co-exist if you do a "find first (any)" search for services and if you keep strictly to the usage pattern as found in mail-api/api/src/main/java/jakarta/mail/util/FactoryFinder.java Lines 52 to 54 in 82dae3a
isHk2Available() , not isOsgi() ).
Keep in mind, however, that using the service loader mediator must be actively supported by the provider of the service as well, because it must add the |
not only angus-mail, but also activation/angus-activation at least (since mail depends on that). Back to this particular PR - to accept it, update it as follows, please:
so if service loader mediator is not supported, hk2 resource locator is kicked in as a fallback thanks! |
Changed as requested. |
Thank you! The only missing part now is ECA which needs to be signed in order to accept this, see CONTRIBUTING.md for details. |
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Implements #630 (API side).
A PR without using annotations. It removes the hk2 code and adds to the pom.xml only.
Things look a bit complicated because the service mediator uses the general OSGi require/provide capability mechanism (not something "service loader" specific) for its configuration. Here's an explanation of the Require-Capability instruction added:
Tells the OSGi runtime that "something" must provide (in the namespace "osgi.extender") the capability "osgi.serviceloader.processor" with a version 1.x.x. This is the most general form of requiring something in OSGi. Usually, you require bundles with specific symbolic names (mostly through imports) because bundles provide an API or implementation that you call in your program and the symbolic name reflects the package name. But in this case we require a "feature" to be added to the OSGi runtime that we don't invoke (it has no API). It just should be there and do its job in the background. The feature in question is "properly handling service loader usage in OSGi" ("osgi.serviceloader.processor" for short) which will at runtime be provided to the OSGi runtime environment e.g. by adding the SPI Fly jars.
The service loader (mediator) is available now, so let's configure it:
We tell the service loader (addressing it by using the namespace "osgi.serviceloader") that it should intercept all calls of
ServiceLoader.load
withjakarta.mail.util.StreamProvider.class
as argument and do its magic (i.e. return the implementing classes despite the usual classloader isolation in OSGi). Note that special handling of the service loader is not specific to OSGi. You need a similar mechanism in other module systems as well. `cardinality:=multiple' states that there may be several service implementations and the application code must choose (or use them all).Finally we have:
because the service loader is used for two interfaces (two kinds of service) in jakarta-mail-api.
In order for this to work, the providers of services have to add some information to their
MANIFEST.MF
as well. I cannot get angus-mail to build (maven package
results in an error and I know almost nothing about maven). So here's what must be added to the manifests of the provider bundles by adding instructions to pom.xml as above (tested by patchingMANIFEST.MF
in the angus-mail jar).First, we (again) need a
<Require-Capability>
:This makes sure that the bundle is processed by the platform "addition"
osgi.serviceloader.registrar
(provided by the SPI Fly jars). The registrar looks for service implementations and makes them available to the (intercepted)ServiceLoader.load
invocations. We also have to specify which interfaces or classes actually define services to be consumed by the service loader, so we need a<Provide-Capability>
instruction with:There's one entry for each file in
META-INF/services
. However, while you must explicitly specify the providing classes in the files inMETA-INF/service/...
, the registrar finds them automatically while processing the bundle.