ARTEMIS-4020 Fixing Management DTO Parsing with custom ETC

This is fixing an issue introduced by the logging changes where a custome ETC would not work correctly.

artemis-boot/src/main/java/org/apache/activemq/artemis/boot/Artemis.java

@@ -26,8 +26,6 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 /**
  * <p>
@@ -37,8 +35,6 @@ import java.util.logging.Logger;
  */
 public class Artemis {
 
-   private static final Logger logger = Logger.getLogger(Artemis.class.getName());
-
    public static void main(String[] args) throws Throwable {
       String home = System.getProperty("artemis.home");
 
@@ -47,7 +43,22 @@ public class Artemis {
       String instance = System.getProperty("artemis.instance");
       File fileInstance = instance != null ? new File(instance) : null;
 
-      Object result = execute(fileHome, fileInstance, true, args);
+
+      String brokerEtc = System.getProperty("artemis.instance.etc");
+      File fileBrokerETC = null;
+      if (brokerEtc != null) {
+         brokerEtc = brokerEtc.replace("\\", "/");
+         fileBrokerETC = new File(brokerEtc);
+      } else {
+         if (instance != null) {
+            brokerEtc = instance + "/etc";
+            fileBrokerETC = new File(brokerEtc);
+         }
+      }
+
+
+
+      Object result = execute(fileHome, fileInstance, fileBrokerETC, true, args);
       if (result instanceof Exception) {
          // Set a nonzero status code for the exceptions caught and printed by org.apache.activemq.artemis.cli.Artemis.execute
          System.exit(1);
@@ -57,14 +68,14 @@ public class Artemis {
    /**
     * This is a good method for booting an embedded command
     */
-   public static Object execute(File artemisHome, File artemisInstance, boolean useSystemOut, List<String> args) throws Throwable {
+   public static Object execute(File artemisHome, File artemisInstance, File fileBrokerETC, boolean useSystemOut, List<String> args) throws Throwable {
-      return execute(artemisHome, artemisInstance, useSystemOut, args.toArray(new String[args.size()]));
+      return execute(artemisHome, artemisInstance, fileBrokerETC, useSystemOut, args.toArray(new String[args.size()]));
    }
 
    /**
     * This is a good method for booting an embedded command
     */
-   public static Object execute(File fileHome, File fileInstance, boolean useSystemOut, String... args) throws Throwable {
+   public static Object execute(File fileHome, File fileInstance, File fileBrokerETC, boolean useSystemOut, String... args) throws Throwable {
       ArrayList<File> dirs = new ArrayList<>();
       if (fileHome != null) {
          dirs.add(new File(fileHome, "lib"));
@@ -76,11 +87,16 @@ public class Artemis {
       ArrayList<URL> urls = new ArrayList<>();
 
       // Without the etc on the config, things like JGroups configuration wouldn't be loaded
-      if (fileInstance != null) {
+      if (fileBrokerETC == null && fileInstance != null) {
-         File etcFile = new File(fileInstance, "etc");
+         // the fileBrokerETC could be null if execute is called directly from an external module
-         // Adding etc to the classLoader so modules can lookup for their configs
+         fileBrokerETC = new File(fileInstance, "etc");
-         urls.add(etcFile.toURI().toURL());
       }
+
+      if (fileBrokerETC != null) {
+         // Adding etc to the classLoader so modules can lookup for their configs
+         urls.add(fileBrokerETC.toURI().toURL());
+      }
+
       if (fileHome != null) {
          File etcFile = new File(fileHome, "etc");
          // Adding etc to the classLoader so modules can lookup for their configs
@@ -122,10 +138,10 @@ public class Artemis {
       URLClassLoader loader = new URLClassLoader(urls.toArray(new URL[urls.size()]));
       Thread.currentThread().setContextClassLoader(loader);
       Class<?> clazz = loader.loadClass("org.apache.activemq.artemis.cli.Artemis");
-      Method method = clazz.getMethod("execute", Boolean.TYPE, Boolean.TYPE, File.class, File.class, args.getClass());
+      Method method = clazz.getMethod("execute", Boolean.TYPE, Boolean.TYPE, File.class, File.class, File.class, args.getClass());
 
       try {
-         return method.invoke(null, useSystemOut, useSystemOut, fileHome, fileInstance, args);
+         return method.invoke(null, useSystemOut, useSystemOut, fileHome, fileInstance, fileBrokerETC, args);
       } catch (InvocationTargetException e) {
          throw e.getTargetException();
       } finally {
@@ -138,7 +154,7 @@ public class Artemis {
       try {
          urls.add(file.toURI().toURL());
       } catch (MalformedURLException e) {
-         logger.log(Level.WARNING, e.getMessage(), e);
+         e.printStackTrace();
       }
    }
 

artemis-cli/src/main/java/org/apache/activemq/artemis/cli/Artemis.java

@@ -93,18 +93,26 @@ public class Artemis {
       String instance = System.getProperty("artemis.instance");
       File fileInstance = instance != null ? new File(instance) : null;
 
-      verifyManagementDTO(fileInstance);
 
-      execute(true, true, fileHome, fileInstance, args);
+      String brokerEtc = System.getProperty("artemis.instance.etc");
+      if (brokerEtc != null) {
+         brokerEtc = brokerEtc.replace("\\", "/");
+      } else {
+         brokerEtc = instance + "/etc";
+      }
+
+      File fileBrokerETC = new File(brokerEtc);
+
+      verifyManagementDTO(fileBrokerETC);
+
+      execute(true, true, fileHome, fileInstance, fileBrokerETC, args);
    }
 
 
    // Notice this has to happen before any Log4j is used.
    //        otherwise Log4j's JMX will start the JMX before this property was able to tbe set
-   public static void verifyManagementDTO(File fileInstance) {
+   public static void verifyManagementDTO(File etc) {
-      if (fileInstance != null) {
+      if (etc != null) {
-
-         File etc = new File(fileInstance, "etc");
          File management = new File(etc, "management.xml");
 
          try {
@@ -119,16 +127,21 @@ public class Artemis {
    }
 
    public static Object internalExecute(String... args) throws Exception {
-      return internalExecute(null, null, args);
+      return internalExecute(null, null, null, args);
    }
 
-   public static Object execute(File artemisHome, File artemisInstance, List<String> args) throws Exception {
+   public static Object execute(File artemisHome, File artemisInstance, File etcFolder, List<String> args) throws Exception {
-      return execute(false, false, artemisHome, artemisInstance, args.toArray(new String[args.size()]));
+      return execute(false, false, artemisHome, artemisInstance, etcFolder, args.toArray(new String[args.size()]));
    }
 
-   public static Object execute(boolean inputEnabled, boolean useSystemOut, File artemisHome, File artemisInstance, String... args) throws Exception {
+   public static Object execute(boolean inputEnabled, boolean useSystemOut, File artemisHome, File artemisInstance, File etcFolder, String... args) throws Exception {
 
-      verifyManagementDTO(artemisInstance);
+      // using a default etc in case that is not set in the variables
+      if (etcFolder == null && artemisInstance != null) {
+         etcFolder = new File(artemisInstance, "etc");
+      }
+
+      verifyManagementDTO(etcFolder);
 
       if (inputEnabled) {
          InputAbstract.enableInput();
@@ -145,7 +158,7 @@ public class Artemis {
       ActionContext.setSystem(context);
 
       try {
-         return internalExecute(artemisHome, artemisInstance, args, context);
+         return internalExecute(artemisHome, artemisInstance, etcFolder, args, context);
       } catch (ConfigurationException configException) {
          context.err.println(configException.getMessage());
          context.out.println();
@@ -177,13 +190,13 @@ public class Artemis {
     * This method is used to validate exception returns.
     * Useful on test cases
     */
-   private static Object internalExecute(File artemisHome, File artemisInstance, String[] args) throws Exception {
+   private static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args) throws Exception {
-      return internalExecute(artemisHome, artemisInstance, args, ActionContext.system());
+      return internalExecute(artemisHome, artemisInstance, etcFolder, args, ActionContext.system());
    }
 
-   public static Object internalExecute(File artemisHome, File artemisInstance, String[] args, ActionContext context) throws Exception {
+   public static Object internalExecute(File artemisHome, File artemisInstance, File etcFolder, String[] args, ActionContext context) throws Exception {
       Action action = builder(artemisInstance).build().parse(args);
-      action.setHomeValues(artemisHome, artemisInstance);
+      action.setHomeValues(artemisHome, artemisInstance, etcFolder);
 
       if (action.isVerbose()) {
          context.out.print("Executing " + action.getClass().getName() + " ");

artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Action.java

@@ -22,7 +22,7 @@ public interface Action {
 
    boolean isVerbose();
 
-   void setHomeValues(File brokerHome, File brokerInstance);
+   void setHomeValues(File brokerHome, File brokerInstance, File etcFolder);
 
    Object execute(ActionContext context) throws Exception;
 

artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/ActionAbstract.java

@@ -63,13 +63,16 @@ public abstract class ActionAbstract implements Action {
    }
 
    @Override
-   public void setHomeValues(File brokerHome, File brokerInstance) {
+   public void setHomeValues(File brokerHome, File brokerInstance, File etcFolder) {
       if (brokerHome != null) {
          this.brokerHome = brokerHome.getAbsolutePath();
       }
       if (brokerInstance != null) {
          this.brokerInstance = brokerInstance.getAbsolutePath();
       }
+      if (etcFolder != null) {
+         this.brokerEtc = etcFolder.getAbsolutePath();
+      }
    }
 
    @Override

artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/HelpAction.java

@@ -28,7 +28,7 @@ public class HelpAction extends Help implements Action {
    }
 
    @Override
-   public void setHomeValues(File brokerHome, File brokerInstance) {
+   public void setHomeValues(File brokerHome, File brokerInstance, File etcFolder) {
 
    }
 

artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/address/HelpAddress.java

@@ -35,7 +35,7 @@ public class HelpAddress extends Help implements Action {
    }
 
    @Override
-   public void setHomeValues(File brokerHome, File brokerInstance) {
+   public void setHomeValues(File brokerHome, File brokerInstance, File etc) {
    }
 
    @Override

artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/check/HelpCheck.java

@@ -35,7 +35,7 @@ public class HelpCheck extends Help implements Action {
    }
 
    @Override
-   public void setHomeValues(File brokerHome, File brokerInstance) {
+   public void setHomeValues(File brokerHome, File brokerInstance, File etc) {
    }
 
    @Override

artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/queue/HelpQueue.java

@@ -35,7 +35,7 @@ public class HelpQueue extends Help implements Action {
    }
 
    @Override
-   public void setHomeValues(File brokerHome, File brokerInstance) {
+   public void setHomeValues(File brokerHome, File brokerInstance, File etc) {
    }
 
    @Override

artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/HelpData.java

@@ -35,7 +35,7 @@ public class HelpData extends Help implements Action {
    }
 
    @Override
-   public void setHomeValues(File brokerHome, File brokerInstance) {
+   public void setHomeValues(File brokerHome, File brokerInstance, File etc) {
 
    }
 

artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/HelpUser.java

@@ -34,7 +34,7 @@ public class HelpUser extends Help implements Action {
    }
 
    @Override
-   public void setHomeValues(File brokerHome, File brokerInstance) {
+   public void setHomeValues(File brokerHome, File brokerInstance, File etcFolder) {
    }
 
    @Override

artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstractTest.java

@@ -34,7 +34,7 @@ public class ConnectionAbstractTest {
 
       System.setProperty("artemis.instance.etc", brokerInstanceEtc.getAbsolutePath());
       try {
-         connectionAbstract.setHomeValues(null, brokerInstanceEtc.getParentFile());
+         connectionAbstract.setHomeValues(null, brokerInstanceEtc.getParentFile(), null);
 
          connectionAbstract.execute(new TestActionContext());
 
@@ -54,7 +54,7 @@ public class ConnectionAbstractTest {
 
       System.setProperty("artemis.instance.etc", brokerInstanceEtc.getAbsolutePath());
       try {
-         connectionAbstract.setHomeValues(null, brokerInstanceEtc.getParentFile());
+         connectionAbstract.setHomeValues(null, brokerInstanceEtc.getParentFile(), null);
          connectionAbstract.setAcceptor("amqp");
 
          connectionAbstract.execute(new TestActionContext());

artemis-cli/src/test/java/org/apache/activemq/artemis/cli/commands/messages/TransferTest.java

@@ -35,7 +35,7 @@ public class TransferTest {
 
       System.setProperty("artemis.instance.etc", brokerInstanceEtc.getAbsolutePath());
       try {
-         transfer.setHomeValues(null, brokerInstanceEtc.getParentFile());
+         transfer.setHomeValues(null, brokerInstanceEtc.getParentFile(), null);
 
          try {
             transfer.execute(new TestActionContext());
@@ -58,7 +58,7 @@ public class TransferTest {
 
       System.setProperty("artemis.instance.etc", brokerInstanceEtc.getAbsolutePath());
       try {
-         transfer.setHomeValues(null, brokerInstanceEtc.getParentFile());
+         transfer.setHomeValues(null, brokerInstanceEtc.getParentFile(), null);
          transfer.setSourceAcceptor("amqp");
 
          try {

artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java

@@ -1287,14 +1287,14 @@ public class ArtemisTest extends CliTestBase {
           * it will read from the InputStream in the ActionContext. It can't read the password since it's using
           * System.console.readPassword() for that.
           */
-         assertEquals(Integer.valueOf(100), Artemis.internalExecute(null, null, new String[] {"producer", "--destination", "queue://q1", "--message-count", "100", "--password", "admin"}, context));
+         assertEquals(Integer.valueOf(100), Artemis.internalExecute(null, null, null, new String[] {"producer", "--destination", "queue://q1", "--message-count", "100", "--password", "admin"}, context));
 
          /*
           * This is the same as above except it will prompt the user to re-enter both the URL and the username.
           */
          in = new ByteArrayInputStream("tcp://localhost:61616\nadmin\n".getBytes());
          context = new ActionContext(in, System.out, System.err);
-         assertEquals(Integer.valueOf(100), Artemis.internalExecute(null, null, new String[] {"producer", "--destination", "queue://q1", "--message-count", "100", "--password", "admin", "--url", "tcp://badhost:11111"}, context));
+         assertEquals(Integer.valueOf(100), Artemis.internalExecute(null, null, null, new String[] {"producer", "--destination", "queue://q1", "--message-count", "100", "--password", "admin", "--url", "tcp://badhost:11111"}, context));
       } finally {
          stopServer();
       }
@@ -1318,14 +1318,14 @@ public class ArtemisTest extends CliTestBase {
           * it will read from the InputStream in the ActionContext. It can't read the password since it's using
           * System.console.readPassword() for that.
           */
-         assertTrue((int) Artemis.internalExecute(null, null, new String[] {"queue", "stat", "--password", "admin"}, context) > 0);
+         assertTrue((int) Artemis.internalExecute(null, null, null, new String[] {"queue", "stat", "--password", "admin"}, context) > 0);
 
          /*
           * This is the same as above except it will prompt the user to re-enter both the URL and the username.
           */
          in = new ByteArrayInputStream("tcp://localhost:61616\nadmin\n".getBytes());
          context = new ActionContext(in, System.out, System.err);
-         assertTrue((int) Artemis.internalExecute(null, null, new String[] {"queue", "stat", "--password", "admin", "--url", "tcp://badhost:11111"}, context) > 0);
+         assertTrue((int) Artemis.internalExecute(null, null, null, new String[] {"queue", "stat", "--password", "admin", "--url", "tcp://badhost:11111"}, context) > 0);
       } finally {
          stopServer();
       }

artemis-cli/src/test/java/org/apache/activemq/cli/test/CheckTest.java

@@ -154,7 +154,7 @@ public class CheckTest extends CliTestBase {
                    "--replicated", "--host", "127.0.0.1", "--default-port", "61626", "--silent", "--no-autotune", "--no-web", "--require-login", "--slave");
 
       System.setProperty("artemis.instance", masterInstance.getAbsolutePath());
-      Object master = Artemis.execute(false, false, null, masterInstance, "run");
+      Object master = Artemis.execute(false, false, null, masterInstance, null, "run");
       ActiveMQServerImpl masterServer = (ActiveMQServerImpl)((Pair)master).getB();
 
       try {
@@ -181,7 +181,7 @@ public class CheckTest extends CliTestBase {
          }
 
          LockAbstract.unlock();
-         Object slave = Artemis.execute(false, false, null, slaveInstance, "run");
+         Object slave = Artemis.execute(false, false, null, slaveInstance, null, "run");
          ActiveMQServerImpl slaveServer = (ActiveMQServerImpl)((Pair)slave).getB();
 
          Wait.assertTrue("Backup isn't announced", () -> slaveServer.getBackupManager() != null &&
@@ -197,7 +197,7 @@ public class CheckTest extends CliTestBase {
             nodeCheck.setPeers(2);
             Assert.assertEquals(3, nodeCheck.execute(context));
          } finally {
-            Artemis.internalExecute(null, slaveInstance, new String[] {"stop"}, ActionContext.system());
+            Artemis.internalExecute(null, slaveInstance, null, new String[] {"stop"}, ActionContext.system());
          }
       } finally {
          stopServer();

artemis-cli/src/test/java/org/apache/activemq/cli/test/OptionsValidationTest.java

@@ -103,7 +103,7 @@ public class OptionsValidationTest extends CliTestBase {
          invalidArgs = new String[] {group, command, "--blahblah-" + command, "--rubbish-" + command + "=" + "more-rubbish", "--input=blahblah"};
       }
       try {
-         Artemis.internalExecute(null, needInstance ? this.artemisInstance : null, invalidArgs, context);
+         Artemis.internalExecute(null, needInstance ? this.artemisInstance : null, null, invalidArgs, context);
          fail("cannot detect invalid options");
       } catch (InvalidOptionsError e) {
          assertTrue(e.getMessage().contains("Found unexpected parameters"));

artemis-maven-plugin/src/main/java/org/apache/activemq/artemis/maven/ArtemisCLIPlugin.java

@@ -50,6 +50,10 @@ public class ArtemisCLIPlugin extends ArtemisAbstractPlugin {
    @Parameter(defaultValue = "${basedir}/target/server0", required = true)
    private File location;
 
+
+   @Parameter
+   private File etc;
+
    @Parameter
    private String[] args;
 
@@ -127,7 +131,7 @@ public class ArtemisCLIPlugin extends ArtemisAbstractPlugin {
                }
             }
          } else {
-            Artemis.execute(home, location, useSystemOutput, args);
+            Artemis.execute(home, location, etc, useSystemOutput, args);
          }
 
          Thread.sleep(600);

artemis-maven-plugin/src/main/java/org/apache/activemq/artemis/maven/ArtemisCreatePlugin.java

@@ -264,7 +264,7 @@ public class ArtemisCreatePlugin extends ArtemisAbstractPlugin {
          commandLineStream.println("# These are the commands used to create " + instance.getName());
          commandLineStream.println(getCommandline(listCommands));
 
-         Artemis.execute(home, null, useSystemOutput, listCommands);
+         Artemis.execute(home, null, null, useSystemOutput, listCommands);
 
          if (configuration != null) {
             String[] list = configuration.list();

tests/smoke-tests/pom.xml

@@ -1180,7 +1180,6 @@
                      </args>
                   </configuration>
                </execution>
-
                <execution>
                   <phase>test-compile</phase>
                   <id>create-create-nettynative</id>
@@ -1197,6 +1196,22 @@
                      <configuration>${basedir}/target/classes/servers/nettynative</configuration>
                   </configuration>
                </execution>
+               <execution>
+                  <phase>test-compile</phase>
+                  <id>create-custom-etc</id>
+                  <goals>
+                     <goal>create</goal>
+                  </goals>
+                  <configuration>
+                     <allowAnonymous>true</allowAnonymous>
+                     <noWeb>true</noWeb>
+                     <instance>${basedir}/target/customETC/server</instance>
+                     <args>
+                        <arg>--etc</arg>
+                        <arg>${basedir}/target/customETC/theCustomETC</arg>
+                     </args>
+                  </configuration>
+               </execution>
             </executions>
             <dependencies>
                <dependency>

tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/custometc/CustomETCTest.java

@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.tests.smoke.custometc;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.activemq.artemis.tests.smoke.common.SmokeTestBase;
+import org.apache.activemq.artemis.tests.util.CFUtil;
+import org.apache.activemq.artemis.utils.RandomUtil;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class CustomETCTest extends SmokeTestBase {
+
+   public static final String SERVER_NAME_0 = "customETC/server";
+
+   public CustomETCTest() {
+   }
+
+   @Before
+   public void before() throws Exception {
+      cleanupData(SERVER_NAME_0);
+      disableCheckThread();
+      startServer(SERVER_NAME_0, 0, 30000);
+   }
+
+   @Test
+   public void testSimpleSendReceive() throws Exception {
+
+      ConnectionFactory cf = CFUtil.createConnectionFactory("core", "tcp://localhost:61616");
+      try (Connection connection = cf.createConnection()) {
+         Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+         Queue queue = session.createQueue(getName());
+         MessageConsumer consumer = session.createConsumer(queue);
+         MessageProducer producer = session.createProducer(queue);
+         String text = RandomUtil.randomString();
+         connection.start();
+         producer.send(session.createTextMessage(text));
+         TextMessage txtMessage = (TextMessage) consumer.receive(5000);
+         Assert.assertNotNull(txtMessage);
+         Assert.assertEquals(text, txtMessage.getText());
+      }
+
+      File logLocation = new File(getServerLocation(SERVER_NAME_0) + "/log/artemis.log");
+      Assert.assertTrue(logLocation.exists());
+
+      AtomicBoolean started = new AtomicBoolean(false);
+      Files.lines(logLocation.toPath()).forEach(line -> {
+         if (line.contains("AMQ221007")) { // server started
+            started.set(true);
+         }
+      });
+
+      Assert.assertTrue(started.get());
+   }
+
+}