Support optional to optional extension dependencies #7729
Conversation
Allow optional extensions to depend on other optional extensions in the same add-on. Signed-off-by: ricekot <github@ricekot.com>
| @@ -126,6 +126,8 @@ public class AddOnLoader extends URLClassLoader { | |||
| */ | |||
| private Map<String, AddOnClassLoader> addOnLoaders = new HashMap<>(); | |||
|
|
|||
| private Map<String, Collection<AddOnClassLoader>> extensionLoaders = new HashMap<>(); | |||
There was a problem hiding this comment.
The class loaders are being added but never removed from the map, when the extension/add-on is unloaded.
| DEPENDENCIES_ELEMENT + "." + DEPENDENCIES_ADDONS_ALL_ELEMENTS; | ||
| private static final String EXTENSION_EXTENSION_DEPENDENCIES = |
There was a problem hiding this comment.
The JavaDoc could be updated to mention the new element.
| List<HierarchicalConfiguration> fields = | ||
| List<HierarchicalConfiguration> addOnFields = | ||
| node.configurationsAt(DEPENDENCIES_ADDONS_ALL_ELEMENTS); | ||
| if (fields.isEmpty()) { | ||
| return new Dependencies(javaVersion, Collections.<AddOnDep>emptyList()); | ||
| var extensionFields = node.configurationsAt(DEPENDENCIES_EXTENSIONS_ALL_ELEMENTS); | ||
| if (addOnFields.isEmpty() && extensionFields.isEmpty()) { | ||
| return new Dependencies(javaVersion, List.of(), List.of()); | ||
| } | ||
|
|
||
| List<AddOnDep> addOns = readAddOnDependencies(fields); | ||
| return new Dependencies(javaVersion, addOns); | ||
| List<AddOnDep> addOns = readAddOnDependencies(addOnFields); | ||
| List<ExtensionDep> extensions = readExtensionDependencies(extensionFields); | ||
| return new Dependencies(javaVersion, addOns, extensions); |
There was a problem hiding this comment.
These changes (and in Dependencies) should not be needed, the add-on will not depend on extensions(?).
| if (classname == null) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
This should not be needed, the name is required.
| @@ -320,16 +328,12 @@ protected Class<?> findClass(String name) throws ClassNotFoundException { | |||
| } | |||
| } | |||
|
|
|||
| try { | |||
There was a problem hiding this comment.
I think we should keep this the default behaviour (classes of the add-on should have priority over the ones in the dependencies) and allow to change it when needed.
| if (classname == null) { | ||
| continue; | ||
| } | ||
| if (addOn.getExtensionsWithDeps().contains(classname)) { |
There was a problem hiding this comment.
It should also check that it can be loaded (i.e. the transitive dependencies are all met).
Allow optional extensions to depend on other optional extensions in the same add-on.
Signed-off-by: ricekot github@ricekot.com