From fee3dd3539c87b3920302685c7fd709780244804 Mon Sep 17 00:00:00 2001 From: Matt Coley Date: Mon, 26 Feb 2024 14:18:54 -0500 Subject: [PATCH 1/2] Mark plugin manager impl as EagerInitialization so plugins will load on startup --- .../services/plugin/BasicPluginManager.java | 19 ++++++++++++++++--- .../recaf/services/plugin/PluginManager.java | 12 ++++++++++++ .../services/plugin/PluginManagerConfig.java | 19 +++++++------------ .../software/coley/recaf/test/TestBase.java | 4 ---- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/recaf-core/src/main/java/software/coley/recaf/services/plugin/BasicPluginManager.java b/recaf-core/src/main/java/software/coley/recaf/services/plugin/BasicPluginManager.java index d20645227..aa8f83992 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/plugin/BasicPluginManager.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/plugin/BasicPluginManager.java @@ -7,8 +7,10 @@ import jakarta.inject.Inject; import org.slf4j.Logger; import software.coley.recaf.analytics.logging.Logging; +import software.coley.recaf.cdi.EagerInitialization; import software.coley.recaf.plugin.*; import software.coley.recaf.services.file.RecafDirectoriesConfig; +import software.coley.recaf.util.TestEnvironment; import software.coley.recaf.util.io.ByteSource; import software.coley.recaf.util.io.ByteSources; @@ -24,6 +26,7 @@ * @author xDark */ @ApplicationScoped +@EagerInitialization public class BasicPluginManager implements PluginManager { private static final Logger logger = Logging.get(BasicPluginManager.class); private final List loaders = new ArrayList<>(); @@ -45,7 +48,11 @@ public BasicPluginManager(PluginManagerConfig config, CdiClassAllocator classAll @PostConstruct @SuppressWarnings("unused") private void setup(RecafDirectoriesConfig directoriesConfig) { - if (config.isAllowLocalScan()) + // Do not load plugins in a test environment + if (TestEnvironment.isTestEnv()) + return; + + if (config.doScanOnStartup()) scanPlugins(directoriesConfig.getPluginDirectory()); } @@ -135,14 +142,20 @@ public void removeLoader(@Nonnull PluginLoader loader) { loaders.remove(loader); } + @Override + public boolean isPluginLoaded(@Nonnull String name) { + return nameMap.get(name) != null; + } + @Nonnull @Override public PluginContainer loadPlugin(@Nonnull PluginContainer container) throws PluginLoadException { String name = container.getInformation().getName(); if (nameMap.putIfAbsent(name.toLowerCase(Locale.ROOT), container) != null) { - // Plugin already exists, we do not allow - // multiple plugins with the same name. + // Plugin already exists, we do not allow multiple plugins with the same name. + // The passed in plugin container will be disabled since it shouldn't be used. try { + logger.warn("Attempted to load duplicate instance of plugin '{}'", name); container.getLoader().disablePlugin(container); } catch (Exception ignored) { } diff --git a/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManager.java b/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManager.java index 1ef4deb6a..d1086695a 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManager.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManager.java @@ -129,6 +129,8 @@ default PluginContainer loadPlugin(@Nonnull ByteSource sou // Load and record plugin container PluginContainer container = loader.load(getAllocator(), source); + PluginContainer existingPlugin = getPlugin(container.getInformation().getName()); + PluginContainer loadedContainer = loadPlugin(container); if (shouldEnablePluginOnLoad(loadedContainer)) loader.enablePlugin(loadedContainer); @@ -140,6 +142,16 @@ default PluginContainer loadPlugin(@Nonnull ByteSource sou throw new PluginLoadException("Plugin manager was unable to locate suitable loader for the source."); } + /** + * Checks if a plugin is loaded. + * + * @param name + * Name of plugin to check for. + * + * @return {@code true} if the plugin has been registered/loaded by this manager. + */ + boolean isPluginLoaded(@Nonnull String name); + /** * Loads and registers a plugin. * diff --git a/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManagerConfig.java b/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManagerConfig.java index 8a1a007c6..de1034808 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManagerConfig.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManagerConfig.java @@ -1,8 +1,11 @@ package software.coley.recaf.services.plugin; +import jakarta.annotation.Nonnull; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; +import software.coley.observables.ObservableBoolean; import software.coley.recaf.config.BasicConfigContainer; +import software.coley.recaf.config.BasicConfigValue; import software.coley.recaf.config.ConfigGroups; import software.coley.recaf.services.ServiceConfig; @@ -13,27 +16,19 @@ */ @ApplicationScoped public class PluginManagerConfig extends BasicConfigContainer implements ServiceConfig { - private boolean allowLocalScan = true; + private final ObservableBoolean scanOnStartup = new ObservableBoolean(true); @Inject public PluginManagerConfig() { super(ConfigGroups.SERVICE_PLUGIN, PluginManager.SERVICE_ID + CONFIG_SUFFIX); + addValue(new BasicConfigValue<>("scan-on-start", boolean.class, scanOnStartup)); } /** * @return {@code true} when local plugins should be scanned when the plugin manager implementation initializes. * {@code false} to disable local automatic plugin loading. */ - public boolean isAllowLocalScan() { - return allowLocalScan; - } - - /** - * @param allowLocalScan - * {@code true} when local plugins should be scanned when the plugin manager implementation initializes. - * {@code false} to disable local automatic plugin loading. - */ - public void setAllowLocalScan(boolean allowLocalScan) { - this.allowLocalScan = allowLocalScan; + public boolean doScanOnStartup() { + return scanOnStartup.getValue(); } } diff --git a/recaf-core/src/testFixtures/java/software/coley/recaf/test/TestBase.java b/recaf-core/src/testFixtures/java/software/coley/recaf/test/TestBase.java index 7a7b8150a..75a76d8ae 100644 --- a/recaf-core/src/testFixtures/java/software/coley/recaf/test/TestBase.java +++ b/recaf-core/src/testFixtures/java/software/coley/recaf/test/TestBase.java @@ -27,10 +27,6 @@ public static void setupWorkspaceManager() { // We'll use this a lot so may as well grab it workspaceManager = recaf.get(WorkspaceManager.class); workspaceManager.setCurrent(null); - - // Disable plugin scanning in tests - PluginManagerConfig pluginConfig = recaf.get(PluginManagerConfig.class); - pluginConfig.setAllowLocalScan(false); } /** From 0429889509b82372187120f802993bd3fe98adfd Mon Sep 17 00:00:00 2001 From: Matt Coley Date: Mon, 26 Feb 2024 17:35:57 -0500 Subject: [PATCH 2/2] Move plugin enabling to common load call, so plugins will always be checked for loading --- .../recaf/services/plugin/BasicPluginManager.java | 12 +++++++++++- .../coley/recaf/services/plugin/PluginManager.java | 12 +++++------- .../recaf/services/plugin/PluginManagerTest.java | 11 ++++++++++- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/recaf-core/src/main/java/software/coley/recaf/services/plugin/BasicPluginManager.java b/recaf-core/src/main/java/software/coley/recaf/services/plugin/BasicPluginManager.java index aa8f83992..bae55b993 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/plugin/BasicPluginManager.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/plugin/BasicPluginManager.java @@ -151,17 +151,27 @@ public boolean isPluginLoaded(@Nonnull String name) { @Override public PluginContainer loadPlugin(@Nonnull PluginContainer container) throws PluginLoadException { String name = container.getInformation().getName(); + + // Throw if a plugin with the same name is already loaded. + PluginLoader loader = container.getLoader(); if (nameMap.putIfAbsent(name.toLowerCase(Locale.ROOT), container) != null) { // Plugin already exists, we do not allow multiple plugins with the same name. // The passed in plugin container will be disabled since it shouldn't be used. try { logger.warn("Attempted to load duplicate instance of plugin '{}'", name); - container.getLoader().disablePlugin(container); + loader.disablePlugin(container); } catch (Exception ignored) { } throw new PluginLoadException("Duplicate plugin: " + name); } + + // Update the instance registry. instanceMap.put(container.getPlugin(), container); + + // If configured, we'll want to load the plugin immediately. + if (shouldEnablePluginOnLoad(container)) + loader.enablePlugin(container); + return container; } diff --git a/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManager.java b/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManager.java index d1086695a..9b80146be 100644 --- a/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManager.java +++ b/recaf-core/src/main/java/software/coley/recaf/services/plugin/PluginManager.java @@ -121,20 +121,18 @@ default Collection getPluginsOfType(@Nonnull Class type) { */ @Nonnull default PluginContainer loadPlugin(@Nonnull ByteSource source) throws PluginLoadException { + ClassAllocator allocator = getAllocator(); for (PluginLoader loader : getLoaders()) { try { // Skip unsupported sources if (!loader.isSupported(source)) continue; - // Load and record plugin container - PluginContainer container = loader.load(getAllocator(), source); - PluginContainer existingPlugin = getPlugin(container.getInformation().getName()); + // Load the plugin container from the source with the current allocator. + PluginContainer container = loader.load(allocator, source); - PluginContainer loadedContainer = loadPlugin(container); - if (shouldEnablePluginOnLoad(loadedContainer)) - loader.enablePlugin(loadedContainer); - return loadedContainer; + // Register the plugin with the manager. + return loadPlugin(container); } catch (IOException | UnsupportedSourceException ex) { throw new PluginLoadException("Could not load plugin due to an error", ex); } diff --git a/recaf-core/src/test/java/software/coley/recaf/services/plugin/PluginManagerTest.java b/recaf-core/src/test/java/software/coley/recaf/services/plugin/PluginManagerTest.java index 32d0f456d..d0fa0dfbe 100644 --- a/recaf-core/src/test/java/software/coley/recaf/services/plugin/PluginManagerTest.java +++ b/recaf-core/src/test/java/software/coley/recaf/services/plugin/PluginManagerTest.java @@ -8,6 +8,7 @@ import software.coley.recaf.plugin.*; import software.coley.recaf.test.TestBase; import software.coley.recaf.util.ZipCreationUtils; +import software.coley.recaf.util.io.ByteSource; import software.coley.recaf.util.io.ByteSources; import java.io.IOException; @@ -70,7 +71,8 @@ public String description() { try { // Load the plugin - PluginContainer container = pluginManager.loadPlugin(ByteSources.wrap(zip)); + ByteSource pluginSource = ByteSources.wrap(zip); + PluginContainer container = pluginManager.loadPlugin(pluginSource); // Assert the information stuck PluginInfo information = container.getInformation(); @@ -83,6 +85,13 @@ public String description() { assertEquals(1, pluginManager.getPlugins().size()); assertSame(container, pluginManager.getPlugin(name)); + // Assert that loading the same plugin twice throws an exception, and does + // not actually register a 2nd instance of the plugin. + assertThrows(PluginLoadException.class, () -> pluginManager.loadPlugin(pluginSource), + "Duplicate plugin loading should fail"); + assertEquals(1, pluginManager.getPlugins().size()); + assertSame(container, pluginManager.getPlugin(name)); + // Now unload it pluginManager.unloadPlugin(container);