[GitHub] [activemq-artemis] gtully commented on a diff in pull request #4634: ARTEMIS-4443 support broker plugin config via broker properties

["gtully (via GitHub)" <gi...@apache.org>]

gtully commented on code in PR #4634:
URL: https://github.com/apache/activemq-artemis/pull/4634#discussion_r1340173331


##########
artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java:
##########
@@ -2057,6 +2061,57 @@ public void testStatusOnErrorApplyingProperties() throws Exception {
       assertTrue(jsonStatus.contains("alder32"));
    }
 
+   @Test
+   public void testPlugin() throws Exception {
+
+      final ConfigurationImpl configuration = new ConfigurationImpl();
+
+      Properties insertionOrderedProperties = new ConfigurationImpl.InsertionOrderedProperties();
+
+      insertionOrderedProperties.put("brokerPlugins.\"org.apache.activemq.artemis.core.server.plugin.impl.LoggingActiveMQServerPlugin.class\".init", "LOG_ALL_EVENTS=true,LOG_SESSION_EVENTS=false");
+
+      configuration.parsePrefixedProperties(insertionOrderedProperties, null);
+
+      Assert.assertEquals(1, configuration.getBrokerPlugins().size());
+      Assert.assertTrue(((LoggingActiveMQServerPlugin)(configuration.getBrokerPlugins().get(0))).isLogAll());
+      Assert.assertFalse(((LoggingActiveMQServerPlugin)(configuration.getBrokerPlugins().get(0))).isLogSessionEvents());
+      Assert.assertTrue(configuration.getBrokerPlugins().get(0).getName().contains(DOT_CLASS));
+
+      // serer initialisePart1
+      configuration.registerBrokerPlugins(configuration.getBrokerPlugins());
+
+      Assert.assertEquals(1, configuration.getBrokerPlugins().size());
+      Assert.assertEquals(1, configuration.getBrokerMessagePlugins().size());
+      Assert.assertEquals(1, configuration.getBrokerConnectionPlugins().size());
+
+      insertionOrderedProperties = new ConfigurationImpl.InsertionOrderedProperties();
+
+      // possible to change any attribute, but plugins only registered on start

Review Comment:
   I think this is invalid, on reload, a new configuration impl will be created. And the pattern on a plugin is a single call to init, which is one line of properties config



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org