Hi devs,
I’d like to fix XCOMMONS-85 (opened in 2012), which changes the behavior of component registration in EmbeddableComponentManager.
The problem
Today, a role type + role hint holds a single component. When a second component is registered with the same role type and hint:
- if its priority is better or equal, the existing one is removed (disposed + unregistered event) and lost for good;
- if its priority is worse, the new one is silently ignored.
So uninstalling an extension overriding a standard component leaves nothing behind until a restart. Concrete case: uninstalling the Prism.js Code Macro Replacement makes the code macro unknown until XWiki is restarted (XPRISMJS-13). The same happens when deleting a wiki component or a wiki macro overriding another one.
Proposal
When a component is registered at a role type + hint where components already are:
- Same registration (equal descriptor and same provided instance): nothing happens (today the instance is disposed, even though it’s still the one registered). A descriptor registered again without instance keeps replacing the existing one as today (dropping its cached singleton), since some code relies on it.
- Same component, i.e. same implementation class name, including a class loaded again by another class loader (new version of an extension, script executed again): it’s replaced, as today, so that these don’t pile up. Exception: different instances provided for the same class, or without any implementation class, are different components (wiki components share a generic implementation class, wiki macros have none).
- Otherwise the components are stacked: the one with the best priority is active (the new one in case of same priority, as today), the other ones are kept hidden instead of being dropped or ignored.
Unregistration:
unregisterComponent(ComponentDescriptor)(used when uninstalling a JAR extension) removes that component, even if hidden. If it was active, the hidden one with the best priority becomes active again (unregistered event, then registered event).- New
unregisterComponent(ComponentDescriptor, instance)(@Unstable) removes exactly that instance, for components which only differ by their instance (wiki components, wiki macros). unregisterComponent(Type, String)is unchanged: nothing is left at that role type and hint (hidden components included), since the caller only knows the role and hint.
A hidden component’s instance is disposed (as today when overwritten) and created again lazily when it becomes active again. An instance provided at registration time is kept, since it couldn’t be created again.
What could go wrong
- Code registering a different component at an existing role/hint, unregistering it by descriptor and expecting nothing left, will now get the previous component back.
- Listeners receive an extra “registered” event when a hidden component is restored. When uninstalling a JAR extension, those events are sent once all its components are unregistered (as already done when installing).
- Smaller behavior changes: a jakarta/javax
Provideris now always registered, even when theProviderbridged from the other side has a better priority (before, it was ignored and lost when the bridged one was unregistered). - Limitation (no regression): wiki components and wiki macros still unregister by role and hint, so deleting one doesn’t restore what it was overriding yet. A platform follow-up can switch them to the new unregistration by instance.
The implementation is in XCOMMONS-85: Components should be stacked instead of overwritten by vmassol · Pull Request #2076 · xwiki/xwiki-commons · GitHub, updated after @MichaelHamann’s review (thanks!). It fixes XPRISMJS-13 (checked on a live instance with and without the change), and the xwiki-platform unit tests and a few functional tests pass against it (details in the PR).
Edit: updated after the review on the PR: instances of the same class (wiki components, wiki macros) now stack instead of replacing each other, registering the same thing again is a no-op, and there’s a new unregisterComponent(ComponentDescriptor, instance).
Edit 2: the no-op is limited to the same provided instance: registering again a descriptor without instance keeps replacing it as today, since xwiki-platform relies on it to get a new singleton instance (found by running the platform tests against the PR).
WDYT?
Thanks