Add a new thin abstraction layer over the creation of resolver instances

[laeubi]

Currently Equinox directly reference the implementation of the (felix) resolver even though it never uses any specific API (except the logger), this makes it currently hard to allow other implementations especially as the is an unfortunate relation between the startup of launches and tests, so if another implementation is used and might be buggy the framework possibly not start up and tests can not be executed or debugged.

This now adds a thin abstraction layer over the creation of resolver instances that basically distinguish the three cases we have where an instance is created allowing to later e.g. return dynamically based on the context (e.g. testrun) a different resolver instance.

Background

Of the past months I struggled with analyze and optimize the current resolver implementation that works fast most of the time but in some cases can badly fail, see for example:

• Add a long running testcase using BC #1148
• Add a new testcase to demonstrate a long running resolve operation #1125
• Additional testcase that show some unexpected behavior of resolver #469

I made several attempts already (and many failed) to improve this and sometimes it is possible to improve for a certain case but often it reveals then other corner cases and I identified some blockers that hinder making progress:

1. There is a high inherited risk to break everything, doing it "wrong" not only break tests it sometimes prevents the whole SDK and test launches from startup, this makes it hard to develop and hard to merge incremental improvements
2. Due to the Felix Impl is (intentionally) a separated place some useful methods in Equinox can only be indirectly called.
3. Some of the problems seem inherit by the used datastructure but major refactorings are hard to accomplish
4. Some limitations are imposed by the OSGi specification that do not matter for internal use cases.

To mitigate I would like to get this abstraction in place what will allow:

• Let dynamically exchange the resolver at runtime, so it is possible to run tests multiple times with different resolver implementations
• We can create different resolver instances per usecase (and maybe even optimized for a certain use-case!), e.g. think about a resolver that is not capable of resolve dynamic or violates some contracts of what the Resolver service imposes
• It would be possible to ship with a "classic" and a "new" resolver implementation so there is less of a risk for releases as users can choose or use a fallback.

[github-actions]

Test Results

  810 files  ±0    810 suites  ±0   1h 34m 38s ⏱️ + 4m 37s
2 222 tests ±0  2 173 ✅ ±0   49 💤 ±0  0 ❌ ±0 
6 678 runs  ±0  6 529 ✅ ±0  149 💤 ±0  0 ❌ ±0 

Results for commit b6a1d06. ± Comparison against base commit e3c0da8.

[tjwatson]

I don't understand why the abstraction uses static methods instead of an interface that can be swapped out more easily with services.

Why does the abstraction contain inputs that are still Felix resolver specific (Logger). Should that not also be abstracted?

[laeubi]

The static methods are needed because wen need to perform this even before the framework starts (e.g. you need a resolved state to register services). So my (intermediate) plan would be to use the properties of the container to specify what class should be loaded so I can give it when the container starts (similar to system-packages).

Regarding the logger, that would generally be possible / useful I just don't want to make it to complicated here.

[tjwatson]

The static methods are needed because wen need to perform this even before the framework starts (e.g. you need a resolved state to register services).

I was using the term "service" generally. For example, you could use the ServiceLoader like we do for the FrameworkUtilHelper

equinox/bundles/org.eclipse.osgi/osgi/src/org/osgi/framework/FrameworkUtil.java

Lines 245 to 251 in e3c0da8

	ServiceLoader<FrameworkUtilHelper> helperLoader = AccessController
	.doPrivileged(
	(PrivilegedAction<ServiceLoader<FrameworkUtilHelper>>) () -> ServiceLoader
	.load(FrameworkUtilHelper.class,
	FrameworkUtilHelper.class
	.getClassLoader()));
	helperLoader.forEach(l::add);

[laeubi]

I first though about that, but it makes the code a lot more work as it should, e.g. we need to define an interface, then load all implementations and then choose if there are many what to use.

So for now I want it really stupid simple and maybe we later make it more flexible e.g. to allow open it up to other (non internal) use-cases where service loader seems quite a good approach.

[tjwatson]

I don't understand how the current PR can be used since everything is hardcoded to use the existing resolver implementation.

[tjwatson]

I need more context on the final solution. Right now this just adds some static methods that requires a Felix specific Logger type in order to return a generic Resolver implementation and therefore I don't see the value. I would rather see an interface used for the factory so it is easier to swap implementations. For now you can hard-code the factory implementation. Then later have some way to switch the factory implementation class (ServiceLoader, system prop etc.).

[laeubi]

Alright I'll try to came up with a more complete proposal then...

One thing that currently bugs me a bit that we even have EquinoxContainer + ModuleContainerAdaptor in the sigature thats not so nice, but the do not seem to share a common interface to access config keys.

[tjwatson]

One thing that currently bugs me a bit that we even have EquinoxContainer + ModuleContainerAdaptor in the sigature

Right, why do you pass the ModuleContainerAdaptor here? It doesn't seem to be used.

[laeubi]

I want to use a property to decide what implementation to load but maybe I just use Function<String, String> or similar...

[tjwatson]

I see, yes a Function<String, String> could be used for that to get configuration from the framework launch properties.

[laeubi]

@tjwatson I though more about the "logger problem" and think not only internally but also other consumers have the problem that there is no (standard) way to get log messages.

I therefore proposed an enhancement to the spec to have a general support for logging in the resolver:

• Add Logging Support to Resolver Service osgi/osgi#883

in Equinox it then can work in a way that for emitting "special" messages as of today we do an instanceof check and otherwise use a default log message. That the would completely remove the need for passing the logger to the constructor at all.

[laeubi]

For the second part I now prose the following:

• Enhance Resolver Service with SPI Support osgi/osgi#884