From 0429889509b82372187120f802993bd3fe98adfd Mon Sep 17 00:00:00 2001
From: Matt Coley <mcoley@ctic.us>
Date: Mon, 26 Feb 2024 17:35:57 -0500
Subject: [PATCH] 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 <T extends Plugin> PluginContainer<T> loadPlugin(@Nonnull PluginContainer<T> 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 <T> Collection<T> getPluginsOfType(@Nonnull Class<T> type) {
 	 */
 	@Nonnull
 	default <T extends Plugin> PluginContainer<T> 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<T> container = loader.load(getAllocator(), source);
-				PluginContainer<Plugin> existingPlugin = getPlugin(container.getInformation().getName());
+				// Load the plugin container from the source with the current allocator.
+				PluginContainer<T> container = loader.load(allocator, source);
 
-				PluginContainer<T> 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<Plugin> container = pluginManager.loadPlugin(ByteSources.wrap(zip));
+			ByteSource pluginSource = ByteSources.wrap(zip);
+			PluginContainer<Plugin> 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);