ARTEMIS-4001 - add properties url to the reload manager watch list, v…

…alidate queue creation on reload with new test
apache · Sep 20, 2022 · d7d1c2e · d7d1c2e
1 parent 1974ec5
commit d7d1c2e
Show file tree

Hide file tree

Showing 7 changed files with 153 additions and 4 deletions.
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java
@@ -23,6 +23,7 @@
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
+import org.apache.activemq.artemis.api.config.ActiveMQDefaultConfiguration;
 import org.apache.activemq.artemis.api.core.QueueConfiguration;
 import org.apache.activemq.artemis.core.config.amqpBrokerConnectivity.AMQPBrokerConnectConfiguration;
 import org.apache.activemq.artemis.core.config.routing.ConnectionRouterConfiguration;
@@ -1426,4 +1427,7 @@ default boolean isJDBC() {
 
    Configuration setSuppressSessionNotifications(boolean suppressSessionNotifications);
 
+   default String resolvePropertiesSources(String propertiesFileUrl) {
+      return System.getProperty(ActiveMQDefaultConfiguration.BROKER_PROPERTIES_SYSTEM_PROPERTY_NAME, propertiesFileUrl);
+   }
 }
diff --git a/...erver/src/main/java/org/apache/activemq/artemis/core/config/CoreAddressConfiguration.java b/...erver/src/main/java/org/apache/activemq/artemis/core/config/CoreAddressConfiguration.java
@@ -20,6 +20,7 @@
 import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.List;
+import java.util.Set;
 
 import org.apache.activemq.artemis.api.core.QueueConfiguration;
 import org.apache.activemq.artemis.api.core.RoutingType;
@@ -48,6 +49,12 @@ public EnumSet<RoutingType> getRoutingTypes() {
       return routingTypes;
    }
 
+   public void setRoutingTypes(Set<String> rawRootingTypes) {
+      for (String routingTypeString : rawRootingTypes) {
+         routingTypes.add(RoutingType.valueOf(routingTypeString));
+      }
+   }
+
    public CoreAddressConfiguration addRoutingType(RoutingType routingType) {
       routingTypes.add(routingType);
       return this;

diff --git a/...-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java b/...-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java
@@ -470,7 +470,7 @@ public void setBrokerPropertiesKeySurround(String brokerPropertiesKeySurround) {
    @Override
    public Configuration parseProperties(String fileUrlToProperties) throws Exception {
       // system property overrides location of file(s)
-      fileUrlToProperties = System.getProperty(ActiveMQDefaultConfiguration.BROKER_PROPERTIES_SYSTEM_PROPERTY_NAME, fileUrlToProperties);
+      fileUrlToProperties = resolvePropertiesSources(fileUrlToProperties);
       if (fileUrlToProperties != null) {
          for (String fileUrl : fileUrlToProperties.split(",")) {
             Properties brokerProperties = new InsertionOrderedProperties();
@@ -3051,7 +3051,7 @@ public String getProperty(final String expression) {
       }
    }
 
-   static class InsertionOrderedProperties extends Properties {
+   public static class InsertionOrderedProperties extends Properties {
 
       final LinkedHashMap<Object, Object> orderedMap = new LinkedHashMap<>();
 

diff --git a/...server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/...server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
@@ -173,6 +173,7 @@
 import org.apache.activemq.artemis.core.server.plugin.ActiveMQServerQueuePlugin;
 import org.apache.activemq.artemis.core.server.plugin.ActiveMQServerResourcePlugin;
 import org.apache.activemq.artemis.core.server.plugin.ActiveMQServerSessionPlugin;
+import org.apache.activemq.artemis.core.server.reload.ReloadCallback;
 import org.apache.activemq.artemis.core.server.routing.ConnectionRouterManager;
 import org.apache.activemq.artemis.core.server.reload.ReloadManager;
 import org.apache.activemq.artemis.core.server.reload.ReloadManagerImpl;
@@ -3252,7 +3253,21 @@ synchronized boolean initialisePart1(boolean scalingDown) throws Exception {
          this.reloadManager = new ReloadManagerImpl(getScheduledPool(), executorFactory.getExecutor(), configurationFileRefreshPeriod);
 
          if (configuration.getConfigurationUrl() != null && getScheduledPool() != null) {
-            reloadManager.addCallback(configuration.getConfigurationUrl(), uri -> reloadConfigurationFile(uri));
+            final URL configUrl = configuration.getConfigurationUrl();
+            ReloadCallback xmlConfigReload = uri -> {
+               // ignore the argument from the callback such that we can respond
+               // to property file locations with a full reload
+               reloadConfigurationFile(configUrl);
+            };
+            reloadManager.addCallback(configUrl, xmlConfigReload);
+
+            // watch properties and reload xml config
+            String propsLocations = configuration.resolvePropertiesSources(propertiesFileUrl);
+            if (propsLocations != null) {
+               for (String fileUrl : propsLocations.split(",")) {
+                  reloadManager.addCallback(new File(fileUrl).toURI().toURL(), xmlConfigReload);
+               }
+            }
          }
 
          if (System.getProperty("logging.configuration") != null) {

diff --git a/...erver/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java b/...erver/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java
@@ -117,7 +117,9 @@ public void check() {
             logger.debug("Validating lastModified " + lastModified + " modified = " + fileModified + " on " + uri);
          }
 
-         if (lastModified > 0 && fileModified > lastModified) {
+         if ((lastModified > 0 && fileModified > lastModified) ||
+            // newly created file, first valid modified time
+            (fileModified > 0 && lastModified == 0)) {
 
             for (ReloadCallback callback : callbacks) {
                try {

diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/reload/ReloadManagerTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/reload/ReloadManagerTest.java
@@ -74,6 +74,62 @@ public void testUpdateWithSpace() throws Exception {
       internalTest(manager, file);
    }
 
+   @Test
+   public void testUpdateOnDirectory() throws Exception {
+      File nested = new File(getTemporaryDir(), "./sub/nested.txt");
+      nested.mkdirs();
+      nested.createNewFile();
+
+      File parentDir = nested.getParentFile();
+
+      Assert.assertTrue(parentDir.isDirectory());
+
+      final ReusableLatch latch = new ReusableLatch(1);
+
+      ReloadCallback reloadCallback = new ReloadCallback() {
+         @Override
+         public void reload(URL uri) {
+            latch.countDown();
+         }
+      };
+      manager.addCallback(parentDir.toURI().toURL(), reloadCallback);
+
+      Assert.assertFalse(latch.await(1, TimeUnit.SECONDS));
+
+      parentDir.setLastModified(System.currentTimeMillis());
+
+      Assert.assertTrue(latch.await(1, TimeUnit.SECONDS));
+
+   }
+
+   @Test
+   public void testUpdateOnNewNotExistingDirectory() throws Exception {
+      final ReusableLatch latch = new ReusableLatch(1);
+
+      ReloadCallback reloadCallback = new ReloadCallback() {
+         @Override
+         public void reload(URL uri) {
+            latch.countDown();
+         }
+      };
+
+      // verify not existing dir is not a problem
+      File notExistFile = new File(getTemporaryDir(), "./sub2/not-there");
+      File notExistDir = notExistFile.getParentFile();
+
+      Assert.assertFalse(notExistDir.exists());
+
+      manager.addCallback(notExistDir.toURI().toURL(), reloadCallback);
+
+      Assert.assertFalse(latch.await(1, TimeUnit.SECONDS));
+
+      // create that non-existent file now
+      notExistFile.mkdirs();
+      notExistFile.createNewFile();
+
+      Assert.assertTrue(latch.await(1, TimeUnit.SECONDS));
+   }
+
    private void internalTest(ReloadManagerImpl manager, File file) throws IOException, InterruptedException {
       file.createNewFile();
 

diff --git a/...src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java b/...src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java
@@ -16,9 +16,13 @@
  */
 package org.apache.activemq.artemis.tests.integration.server;
 
+import java.io.File;
+import java.io.FileOutputStream;
+
 import org.apache.activemq.artemis.api.core.SimpleString;
 import org.apache.activemq.artemis.core.config.CoreQueueConfiguration;
 import org.apache.activemq.artemis.core.config.FileDeploymentManager;
+import org.apache.activemq.artemis.core.config.impl.ConfigurationImpl;
 import org.apache.activemq.artemis.core.config.impl.FileConfiguration;
 import org.apache.activemq.artemis.core.config.impl.SecurityConfiguration;
 import org.apache.activemq.artemis.core.postoffice.Bindings;
@@ -29,6 +33,8 @@
 import org.apache.activemq.artemis.spi.core.security.jaas.InVMLoginModule;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
 import org.apache.activemq.artemis.tests.util.RandomUtil;
+import org.apache.activemq.artemis.tests.util.Wait;
+import org.junit.Assert;
 import org.junit.Test;
 
 public class ConfigurationTest extends ActiveMQTestBase {
@@ -65,6 +71,65 @@ public void testQueueWithoutAddressName() throws Exception {
       }
    }
 
+   @Test
+   public void testPropertiesConfigReload() throws Exception {
+
+      File propsFile = new File(getTestDirfile(),"some.props");
+      propsFile.createNewFile();
+
+      ConfigurationImpl.InsertionOrderedProperties config = new ConfigurationImpl.InsertionOrderedProperties();
+      config.put("configurationFileRefreshPeriod", "500");
+
+      config.put("addressConfigurations.mytopic_3.routingTypes", "MULTICAST");
+
+      config.put("addressConfigurations.mytopic_3.queueConfigs.\"queue.A3\".address", "mytopic_3");
+      config.put("addressConfigurations.mytopic_3.queueConfigs.\"queue.A3\".routingType", "MULTICAST");
+
+      config.put("addressConfigurations.mytopic_3.queueConfigs.\"queue.B3\".address", "mytopic_3");
+      config.put("addressConfigurations.mytopic_3.queueConfigs.\"queue.B3\".routingType", "MULTICAST");
+
+      try (FileOutputStream outStream = new FileOutputStream(propsFile)) {
+         config.store(outStream, null);
+      }
+
+      Assert.assertTrue(propsFile.exists());
+
+      System.out.println("props: " + propsFile.getAbsolutePath());
+
+      ActiveMQServer server = getActiveMQServer("duplicate-queues.xml");
+      server.setProperties(propsFile.getAbsolutePath());
+      try {
+
+         server.start();
+         Bindings mytopic_1 = server.getPostOffice().getBindingsForAddress(new SimpleString("mytopic_1"));
+         assertEquals(mytopic_1.getBindings().size(), 0);
+         Bindings mytopic_2 = server.getPostOffice().getBindingsForAddress(new SimpleString("mytopic_2"));
+         assertEquals(mytopic_2.getBindings().size(), 3);
+
+         Bindings mytopic_3 = server.getPostOffice().getBindingsForAddress(new SimpleString("mytopic_3"));
+         assertEquals(mytopic_3.getBindings().size(), 2);
+
+
+         // add new binding from props update
+         config.put("addressConfigurations.mytopic_3.queueConfigs.\"queue.C3\".address", "mytopic_3");
+
+         try (FileOutputStream outStream = new FileOutputStream(propsFile)) {
+            config.store(outStream, null);
+         }
+
+         Wait.assertTrue(() -> {
+            Bindings mytopic_31 = server.getPostOffice().getBindingsForAddress(new SimpleString("mytopic_3"));
+            return mytopic_31.getBindings().size() == 3;
+         });
+
+      } finally {
+         try {
+            server.stop();
+         } catch (Exception e) {
+         }
+      }
+   }
+
    protected ActiveMQServer getActiveMQServer(String brokerConfig) throws Exception {
       FileConfiguration fc = new FileConfiguration();
       FileJMSConfiguration fileConfiguration = new FileJMSConfiguration();