From e3b1ef4ca7da7b2146fd71e51fae873c76f6511c Mon Sep 17 00:00:00 2001 From: Hisham Alrashdan Date: Fri, 13 Nov 2026 07:12:41 +0300 Subject: [PATCH] Warn when a server plugin class implements no plugin type initPlugins registers a server class once for each plugin type interface it implements. A class that implements ServerPlugin but none of those interfaces was instantiated and then ignored: it was never added to serverPlugins, so start() and stop() were never called, and nothing in the log said why. Log a warning that names the plugin and the class. Which plugins are registered, started and stopped does not change. Fixes #398 Assisted-by: Claude:claude-fable-5-2 Signed-off-by: Hisham Alrashdan --- .../DefaultExtensionController.java | 9 ++ .../DefaultExtensionControllerTest.java | 85 +++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/server/src/main/java/com/mirth/connect/server/controllers/DefaultExtensionController.java b/server/src/main/java/com/mirth/connect/server/controllers/DefaultExtensionController.java index 77b02c1874..deac44706f 100644 --- a/server/src/main/java/com/mirth/connect/server/controllers/DefaultExtensionController.java +++ b/server/src/main/java/com/mirth/connect/server/controllers/DefaultExtensionController.java @@ -309,6 +309,15 @@ public void initPlugins() { serverPlugins.add(multiFactorAuthenticationPlugin); logger.debug("sucessfully loaded server multi-factor authentication plugin: " + serverPlugin.getPluginPointName()); } + + /* + * A class that implements none of the plugin type interfaces above is never + * added to serverPlugins, so it is never started or stopped. Warn about it + * instead of ignoring it silently. + */ + if (!serverPlugins.contains(serverPlugin)) { + logger.warn("Server class \"" + clazzName + "\" of plugin \"" + pluginName + "\" does not implement any supported plugin type interface (such as ServicePlugin or ChannelPlugin), so it will not be started or stopped."); + } } catch (Exception e) { logger.error("Error instantiating plugin: " + pluginName, e); } diff --git a/server/src/test/java/com/mirth/connect/server/controllers/DefaultExtensionControllerTest.java b/server/src/test/java/com/mirth/connect/server/controllers/DefaultExtensionControllerTest.java index 984607dcdf..7079a7930a 100644 --- a/server/src/test/java/com/mirth/connect/server/controllers/DefaultExtensionControllerTest.java +++ b/server/src/test/java/com/mirth/connect/server/controllers/DefaultExtensionControllerTest.java @@ -9,21 +9,37 @@ package com.mirth.connect.server.controllers; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import java.io.File; +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.Map; import java.util.zip.ZipEntry; import java.util.zip.ZipException; import java.util.zip.ZipFile; +import org.apache.logging.log4j.Logger; import org.junit.After; import org.junit.Before; import org.junit.Test; +import com.mirth.connect.model.PluginClass; +import com.mirth.connect.model.PluginMetaData; +import com.mirth.connect.plugins.ResourcePlugin; +import com.mirth.connect.plugins.ServerPlugin; import com.mirth.connect.util.ZipTestUtils; public class DefaultExtensionControllerTest { + private static final String TEST_PLUGIN_NAME = "Test Plugin"; + @Test(expected = ZipException.class) public void testExtractZipEntryZipSlipWithRelativePath() throws Exception { DefaultExtensionController extensionController = new DefaultExtensionController(); @@ -49,6 +65,29 @@ public void testExtractZipEntryValidPath() throws Exception { assertTrue(extractedFile.exists()); } + @Test + public void testInitPluginsWarnsAboutServerClassWithoutPluginType() throws Exception { + Logger logger = mock(Logger.class); + DefaultExtensionController extensionController = createControllerWithPlugin(NoTypeServerPlugin.class, logger); + + extensionController.initPlugins(); + + // The plugin is still not registered, but it is no longer ignored silently + assertTrue(extensionController.getServerPlugins().isEmpty()); + verify(logger).warn(contains(NoTypeServerPlugin.class.getName())); + } + + @Test + public void testInitPluginsDoesNotWarnAboutServerClassWithPluginType() throws Exception { + Logger logger = mock(Logger.class); + DefaultExtensionController extensionController = createControllerWithPlugin(ResourceServerPlugin.class, logger); + + extensionController.initPlugins(); + + assertEquals(1, extensionController.getServerPlugins().size()); + verify(logger, never()).warn(anyString()); + } + @Before public void createTestFolder() { File installTempDir = new File("tests/zipextraction/"); @@ -72,4 +111,50 @@ public void cleanupTestFolder() { private ZipFile createTempZipFile(String fileName) throws Exception { return new ZipFile(ZipTestUtils.createTempZipFile(fileName)); } + + /* + * Creates a controller that knows a single enabled plugin with the given server class, and + * that writes its log messages to the given logger. + */ + private DefaultExtensionController createControllerWithPlugin(Class serverPluginClass, Logger logger) throws Exception { + PluginClass serverClass = new PluginClass(); + serverClass.setName(serverPluginClass.getName()); + + final PluginMetaData pluginMetaData = new PluginMetaData(); + pluginMetaData.setName(TEST_PLUGIN_NAME); + pluginMetaData.setServerClasses(Collections.singletonList(serverClass)); + + DefaultExtensionController extensionController = new DefaultExtensionController() { + @Override + public Map getPluginMetaData() { + return Collections.singletonMap(TEST_PLUGIN_NAME, pluginMetaData); + } + + @Override + public boolean isExtensionEnabled(String extensionName) { + return true; + } + }; + + Field loggerField = DefaultExtensionController.class.getDeclaredField("logger"); + loggerField.setAccessible(true); + loggerField.set(extensionController, logger); + + return extensionController; + } + + public static class NoTypeServerPlugin implements ServerPlugin { + @Override + public String getPluginPointName() { + return TEST_PLUGIN_NAME; + } + + @Override + public void start() {} + + @Override + public void stop() {} + } + + public static class ResourceServerPlugin extends NoTypeServerPlugin implements ResourcePlugin {} } \ No newline at end of file