[osgi] Solutions for https://github.com/eclipse/microprofile/issues/33

[rotty3000]

See: eclipse/microprofile#33

[arthurdm]

@rotty3000 - is there a change needed to OASFactoryResolver at all?

[arthurdm]

hey @tjwatson and @Emily-Jiang - can you please help me review this PR? I am not really sure what it does =)

[tjwatson]

@Emily-Jiang I need to discuss this with you. We consume the micro-profile jars into OpenLiberty (which has an OSGi based runtime) but I am pretty sure we do not want to require the osgi.serviceloader usage in OpenLiberty do we?

[tjwatson]

Especially since we do setup a TCCL which the ServiceLoader would use to lookup services and would probably want to continue that behavior, while the osgi.serviceloader will do some weaving tricks to change the call sights of the ServiceLoader.load calls to do something with OSGi services.

[rotty3000]

Service loader mediator is an OSGi spec which open Liberty could implement any way they like. Are you suggesting that everyone else suffer for Open Liberty's implementation details?

…

On Mon, Oct 22, 2018, 19:41 Thomas Watson, ***@***.***> wrote: Especially since we do setup a TCCL which the ServiceLoader would use to lookup services and would probably want to continue that behavior, while the osgi.serviceloader will do some weaving tricks to change the call sights of the ServiceLoader.load calls to do something with OSGi services. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#265 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAI9TIXfcmSB_YmvJ5Rik7vVQ8rFuEycks5ungNCgaJpZM4WTzcZ> .

[rotty3000]

Let me demonstrate how things get broken when you only do OSGi part way. The latest umbrella release is already broken. Consider https://github.com/eclipse/microprofile/blob/master/spec/src/main/asciidoc/architecture.asciidoc Notice that it expects MP-Config 1.3 which imports CDI packages at version 1.2, but CDI version 2.0 is required by the umbrella. So you cannot even run the bundles as is I OSGi. This is willful proliferation of broken OSGi bundles from an organization that a) has the know-how and b) should know better. It's making OSGi look bad to anyone willing to try it. I'm sorry to be bearer of bad news but that's where we are.

…

On Tue, Oct 23, 2018, 00:15 Raymond Auge, ***@***.***> wrote: Service loader mediator is an OSGi spec which open Liberty could implement any way they like. Are you suggesting that everyone else suffer for Open Liberty's implementation details? On Mon, Oct 22, 2018, 19:41 Thomas Watson, ***@***.***> wrote: > Especially since we do setup a TCCL which the ServiceLoader would use to > lookup services and would probably want to continue that behavior, while > the osgi.serviceloader will do some weaving tricks to change the call > sights of the ServiceLoader.load calls to do something with OSGi services. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#265 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAI9TIXfcmSB_YmvJ5Rik7vVQ8rFuEycks5ungNCgaJpZM4WTzcZ> > . >

[tjwatson]

That does sound broken, but also unrelated to this PR. I assume you have a separate issue about proper import ranges?

[tjwatson]

Service loader mediator is an OSGi spec which open Liberty could implement any way they like. Are you suggesting that everyone else suffer for Open Liberty's implementation details?

Not at all, I'm having an open discussion about the additional requirements this dependency on an OSGi specification will have on MicroProfile (while trying not to be dramatic with all the suffering that will occur :)

[rotty3000]

This IS related to this PR in that I've attempted to correct package import versions in all the linked PRs as part of the effort to make them good OSGi citizens.

…

On Tue, Oct 23, 2018 at 3:37 AM Thomas Watson ***@***.***> wrote: Service loader mediator is an OSGi spec which open Liberty could implement any way they like. Are you suggesting that everyone else suffer for Open Liberty's implementation details? Not at all, I'm having an open discussion about the additional requirements this dependency on an OSGi specification will have on MicroProfile (while trying not to be dramatic with all the suffering that will occur :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#265 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAI9TDdwnWpIYxdhKQKbW1iAg34BOjFPks5unnLVgaJpZM4WTzcZ> .

-- *Raymond Augé* <http://www.liferay.com/web/raymond.auge/profile> (@rotty3000) Senior Software Architect *Liferay, Inc.* <http://www.liferay.com> (@liferay) Board Member & EEG Co-Chair, OSGi Alliance <http://osgi.org> (@OSGiAlliance)

[rotty3000]

Specifically related to Service Loader Mediator, MicroProfile APIs would only be enhanced by following a standard specifically designed for adapting the use of ServiceLoader in OSGi as opposed to leaving it unaddressed, essentially forcing consumers to fork the artifact which has further negative ramifications on the perception of OSGi.

[tjwatson]

This IS related to this PR in that I've attempted to correct package import versions in all the linked PRs as part of the effort to make them good OSGi citizens.

OK, but this particular PR does not address the import issues. I want avoid conflating the import resolution issue with the service loader mediator discussion.

Specifically related to Service Loader Mediator, MicroProfile APIs would only be enhanced by following a standard specifically designed for adapting the use of ServiceLoader in OSGi as opposed to leaving it unaddressed, essentially forcing consumers to fork the artifact which has further negative ramifications on the perception of OSGi.

I should mention that up to now I have not really touched the MicroProfile implementation nor the specification (I'm not a committer of the project at Eclipse). If others with more concrete knowledge of the MicroProfile project agree that the approach of adding a hard dependency on the service loader mediator is the way to go for the spec'ed JARs then I am fine with that. I only can advise some caution here because I have been bitten in the past by libraries putting a hard dependency on service loader mediator. Ray, you should be familiar with such a case in Jetty, there is a part of Jetty that needs the mediator to function, but the Jetty bundles can live without the functionality. At least that is true for the use of Jetty by the Eclipse platform for the Http Service implementation. It seems each time we update Jetty versions for the Eclipse Platform we have to deal with another of these hard dependencies that really should be viewed as optional (but I know how your feel about optional requirements also).

I'm not even sure how the OSGI based runtimes that implement MicroProfile deal with these ServiceLoader calls. I had assumed MicroProfile defines a behavior for the TCCL and that makes it just work without the need for the mediator. But we really need the input from the likes of @Emily-Jiang or others to provide input on that.

[rotty3000]

.. and I'm not just here to be a hard case unwilling to compromise. But each issue I've brought up is a real concern for any OSGi consumer. Personally I feel that the only way that Open Libery could possibly be using these things is by passing them through the system bundle as extra package exports, and doing the wiring to the provider manually. That, or they force bundle start ordering in order to wire the provider before any consumer touches the API which is the most likely scenario based on this code reference

https://github.com/eclipse/microprofile-config/blob/master/api/src/main/java/org/eclipse/microprofile/config/spi/ConfigProviderResolver.java#L166-L175

which I have to say... it's less than ideal. This effectively is a race condition between consumers and providers.

[rotty3000]

I should add that Service Loader Mediator can totally handle the ordering issue by making sure it doesn't let the provider or consumer resolve (the required capabilities on either side) until it has performed whatever it needs to do. In fact Aries SpiFly actually provides a Framework Extension [1] fragment (which has no external dependencies) specifically designed to eliminate any start ordering concern between providers and consumers.

Furthermore, I am waving away the fact that microprofile-config's usage of ServiceLoader is not compatible with SpiFly at this time. but that's an entirely different issue that is a defect we need to address on the SpiFly side.

[1] https://search.maven.org/artifact/org.apache.aries.spifly/org.apache.aries.spifly.dynamic.framework.extension/1.0.14/bundle

[tjwatson]

.. and I'm not just here to be a hard case unwilling to compromise. But each issue I've brought up is a real concern for any OSGi consumer. Personally I feel that the only way that Open Libery could possibly be using these things is by passing them through the system bundle as extra package exports, and doing the wiring to the impl manually. That, or they force bundle start ordering in order to wire the impl before any consumer touches the API which is the most likely scenario based on this code reference

https://github.com/eclipse/microprofile-config/blob/master/api/src/main/java/org/eclipse/microprofile/config/spi/ConfigProviderResolver.java#L166-L175

which I have to say... it's less than ideal. This effectively is a race condition between consumers and providers.

Will the proposal to only use service loader mediator for the SPI implementation load also include the removal of this setter method? If the use of service loader mediator is mandatory then this method is not really very useful since the bundle will never resolve without the mediator implementation. I can see this being a race condition for applications that are also based on OSGi and the application bundles live in the same space as the implementation and there is no control over application bundles activating in the mix of the implementation bundles. I imagine that is not the case for implementations of MicroProfile today where they likely have strict control over when application code will start to be executed. But at this point I am only guessing. Will need others to provide input.

[rotty3000]

So after discussing this use case with others I think I've come up with a compromise; a weak SLM requirement.

A weak requirement allows:

• build time resolving works; finds an implementation of SLM and a provider when a consumer is part of the requirements (whether initial or transitive)
• optional at run time, so that if you didn't use a resolver to assemble AND you don't have SML implementation at runtime the bundles still resolve
• run time wires are created because proper wiring of requirements at run time is essential

This is implemented by specifying the same requirement twice; once as effective:=active (for build resolving) and the other as resolution:=optional (for non-mandatory run-time resolving)

e.g.

    osgi.serviceloader;\
        filter:="(osgi.serviceloader=org.eclipse.microprofile.config.spi.ConfigProviderResolver)";\
        cardinality:=multiple;\
        resolution:=optional,\
    osgi.serviceloader;\
        filter:="(osgi.serviceloader=org.eclipse.microprofile.config.spi.ConfigProviderResolver)";\
        cardinality:=multiple;\
        effective:=active,\
    osgi.extender;\
        filter:="(osgi.extender=osgi.serviceloader.processor)";\
        resolution:=optional,\
    osgi.extender;\
        filter:="(osgi.extender=osgi.serviceloader.processor)",\
        effective:=active

OR since R7 + bnd-maven-plugin 4.1.0 using the bundle annotations which I can provide an example for in a bit.

[tjwatson]

This approach seems reasonable to me.

[rotty3000]

@tjwatson, how do you feel about stripping the versions off of the non-semantically versioned package imports and adding the appropriate portable java contract in their place? Again, the requirements could be made weak for the same reason as above.

[eclipse-microprofile-bot]

Can one of the admins verify this patch?

[arthurdm]

The umbrella MP issue (eclipse/microprofile#33) where this PR originated from was closed as won't do, so I am inclined to close this PR.

@jmini / @EricWittmann - any objections?