From d7d1c2ee42f91e37e5d34fcad1dd18411a19ce23 Mon Sep 17 00:00:00 2001 From: Gary Tully Date: Mon, 19 Sep 2022 11:33:16 +0100 Subject: [PATCH] ARTEMIS-4001 - add properties url to the reload manager watch list, validate queue creation on reload with new test --- .../artemis/core/config/Configuration.java | 4 ++ .../core/config/CoreAddressConfiguration.java | 7 ++ .../core/config/impl/ConfigurationImpl.java | 4 +- .../core/server/impl/ActiveMQServerImpl.java | 17 ++++- .../core/server/reload/ReloadManagerImpl.java | 4 +- .../core/reload/ReloadManagerTest.java | 56 ++++++++++++++++ .../integration/server/ConfigurationTest.java | 65 +++++++++++++++++++ 7 files changed, 153 insertions(+), 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 index af35a66210..ad2a5c9f26 100644 --- 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.Map; 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 @@ public interface Configuration { Configuration setSuppressSessionNotifications(boolean suppressSessionNotifications); + default String resolvePropertiesSources(String propertiesFileUrl) { + return System.getProperty(ActiveMQDefaultConfiguration.BROKER_PROPERTIES_SYSTEM_PROPERTY_NAME, propertiesFileUrl); + } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/CoreAddressConfiguration.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/CoreAddressConfiguration.java index 0dad2b4a40..8c7487d10a 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/CoreAddressConfiguration.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/CoreAddressConfiguration.java @@ -20,6 +20,7 @@ import java.io.Serializable; 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 class CoreAddressConfiguration implements Serializable { return routingTypes; } + public void setRoutingTypes(Set rawRootingTypes) { + for (String routingTypeString : rawRootingTypes) { + routingTypes.add(RoutingType.valueOf(routingTypeString)); + } + } + public CoreAddressConfiguration addRoutingType(RoutingType routingType) { routingTypes.add(routingType); return this; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java index 4b16756888..62a13627ad 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java @@ -470,7 +470,7 @@ public class ConfigurationImpl implements Configuration, Serializable { @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 class ConfigurationImpl implements Configuration, Serializable { } } - static class InsertionOrderedProperties extends Properties { + public static class InsertionOrderedProperties extends Properties { final LinkedHashMap orderedMap = new LinkedHashMap<>(); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index 2b9a32aff5..0f792661ea 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-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.ActiveMQServerMessagePlugi 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 @@ public class ActiveMQServerImpl implements ActiveMQServer { 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/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java index 62ad093c38..0ae7358037 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java @@ -117,7 +117,9 @@ public class ReloadManagerImpl extends ActiveMQScheduledComponent implements Rel 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 index fa82ae0221..fbffb72eb0 100644 --- 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 class ReloadManagerTest extends ActiveMQTestBase { 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/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java index cd8219a458..269dc53c94 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java +++ b/tests/integration-tests/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.ActiveMQJAASSecurityManager 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 class ConfigurationTest extends ActiveMQTestBase { } } + @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();