From 6cb68f2ce923ea3c2400c21967b2003175c8a0f4 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Wed, 13 May 2015 15:06:11 -0400 Subject: [PATCH] Make tools more friendly to user errors --- .../artemis/cli/commands/Configurable.java | 37 ++++++++++++++++++- .../activemq/artemis/cli/commands/Run.java | 12 ++++++ .../cli/commands/tools/DataAbstract.java | 18 +++++---- .../cli/commands/tools/DecodeJournal.java | 2 +- .../cli/commands/tools/EncodeJournal.java | 2 +- .../artemis/cli/commands/tools/PrintData.java | 4 +- .../cli/commands/tools/XmlDataExporter.java | 9 ++++- .../bootstrap/ActiveMQBootstrapBundle.java | 3 -- .../bootstrap/ActiveMQBootstrapLogger.java | 4 -- 9 files changed, 70 insertions(+), 21 deletions(-) diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Configurable.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Configurable.java index b635903063..b378697835 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Configurable.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Configurable.java @@ -17,10 +17,15 @@ package org.apache.activemq.artemis.cli.commands; +import javax.inject.Inject; import java.io.File; import io.airlift.airline.Arguments; +import io.airlift.airline.Help; import io.airlift.airline.Option; +import io.airlift.airline.model.CommandGroupMetadata; +import io.airlift.airline.model.CommandMetadata; +import io.airlift.airline.model.GlobalMetadata; import org.apache.activemq.artemis.core.config.FileDeploymentManager; import org.apache.activemq.artemis.core.config.impl.FileConfiguration; import org.apache.activemq.artemis.dto.BrokerDTO; @@ -39,6 +44,10 @@ public abstract class Configurable @Option(name = "--broker", description = "This would override the broker configuration from the bootstrap") String brokerConfig; + + @Inject + public GlobalMetadata global; + private BrokerDTO brokerDTO = null; private String brokerInstance; @@ -47,6 +56,15 @@ public abstract class Configurable private FileConfiguration fileConfiguration; + protected void treatError(Exception e, String group, String command) + { + ActiveMQBootstrapLogger.LOGGER.debug(e.getMessage(), e); + System.err.println(); + System.err.println("Error:" + e.getMessage()); + System.err.println(); + helpGroup(group, command); + } + protected String getBrokerInstance() { if (brokerInstance == null) @@ -64,6 +82,24 @@ public abstract class Configurable return brokerInstance; } + protected void helpGroup(String groupName, String commandName) + { + for (CommandGroupMetadata group: global.getCommandGroups()) + { + if (group.getName().equals(groupName)) + { + for (CommandMetadata command: group.getCommands()) + { + if (command.getName().equals(commandName)) + { + Help.help(command); + } + } + break; + } + } + } + protected String getBrokerHome() { if (brokerHome == null) @@ -89,7 +125,6 @@ public abstract class Configurable if (getBrokerInstance() == null) { final String defaultLocation = "../data"; - ActiveMQBootstrapLogger.LOGGER.brokerConfigNotFound(defaultLocation); fileConfiguration = new FileConfiguration(); // These will be the default places in case the file can't be loaded fileConfiguration.setBindingsDirectory(defaultLocation + "/bindings"); diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java index 2ae98c4907..584911b22a 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Run.java @@ -25,6 +25,7 @@ import io.airlift.airline.Command; import io.airlift.airline.Option; import org.apache.activemq.artemis.cli.Artemis; import org.apache.activemq.artemis.components.ExternalComponent; +import org.apache.activemq.artemis.core.config.impl.FileConfiguration; import org.apache.activemq.artemis.core.server.ActiveMQComponent; import org.apache.activemq.artemis.dto.BrokerDTO; import org.apache.activemq.artemis.dto.ComponentDTO; @@ -50,6 +51,8 @@ public class Run extends Configurable implements Action Artemis.printBanner(); + createDirectories(getFileConfiguration()); + BrokerDTO broker = getBrokerDTO(); addShutdownHook(broker.server.getConfigurationFile().getParentFile()); @@ -77,6 +80,15 @@ public class Run extends Configurable implements Action return null; } + + private void createDirectories(FileConfiguration fileConfiguration) + { + new File(fileConfiguration.getBindingsDirectory()).mkdirs(); + new File(fileConfiguration.getJournalDirectory()).mkdirs(); + new File(fileConfiguration.getPagingDirectory()).mkdirs(); + new File(fileConfiguration.getLargeMessagesDirectory()).mkdirs(); + } + /** * Add a simple shutdown hook to stop the server. * @param configurationDir diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DataAbstract.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DataAbstract.java index d222eed58e..717a9de97a 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DataAbstract.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DataAbstract.java @@ -17,11 +17,10 @@ package org.apache.activemq.artemis.cli.commands.tools; +import java.io.File; + import io.airlift.airline.Option; import org.apache.activemq.artemis.cli.commands.Configurable; -import org.apache.activemq.artemis.integration.bootstrap.ActiveMQBootstrapBundle; - -import java.io.File; /** Abstract class for places where you need bindings, journal paging and large messages configuration */ public abstract class DataAbstract extends Configurable @@ -44,9 +43,10 @@ public abstract class DataAbstract extends Configurable if (largeMessges == null) { largeMessges = getFileConfiguration().getLargeMessagesDirectory(); - checkIfDirectoryExists(largeMessges); } + checkIfDirectoryExists(largeMessges); + return largeMessges; } @@ -56,9 +56,10 @@ public abstract class DataAbstract extends Configurable if (binding == null) { binding = getFileConfiguration().getBindingsDirectory(); - checkIfDirectoryExists(binding); } + checkIfDirectoryExists(binding); + return binding; } @@ -67,9 +68,10 @@ public abstract class DataAbstract extends Configurable if (journal == null) { journal = getFileConfiguration().getJournalDirectory(); - checkIfDirectoryExists(journal); } + checkIfDirectoryExists(journal); + return journal; } @@ -80,6 +82,8 @@ public abstract class DataAbstract extends Configurable paging = getFileConfiguration().getPagingDirectory(); } + checkIfDirectoryExists(paging); + return paging; } @@ -88,7 +92,7 @@ public abstract class DataAbstract extends Configurable File f = new File(directory); if (!f.exists()) { - throw ActiveMQBootstrapBundle.BUNDLE.directoryDoesNotExist(directory); + throw new IllegalStateException("Could not find folder: " + directory + ", please pass --bindings, --journal and --paging as arguments"); } } diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DecodeJournal.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DecodeJournal.java index 106412fd26..352bffde04 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DecodeJournal.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/DecodeJournal.java @@ -69,7 +69,7 @@ public class DecodeJournal extends Configurable implements Action } catch (Exception e) { - e.printStackTrace(); + treatError(e, "data", "decode"); } return null; diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/EncodeJournal.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/EncodeJournal.java index 426016efcd..d81773f5b0 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/EncodeJournal.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/EncodeJournal.java @@ -64,7 +64,7 @@ public class EncodeJournal extends Configurable implements Action } catch (Exception e) { - e.printStackTrace(); + treatError(e, "data", "encode"); } return null; diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/PrintData.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/PrintData.java index 077afe0831..2d2d004b75 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/PrintData.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/PrintData.java @@ -54,13 +54,11 @@ import org.apache.activemq.artemis.core.server.impl.FileLockNodeManager; import org.apache.activemq.artemis.core.settings.HierarchicalRepository; import org.apache.activemq.artemis.core.settings.impl.AddressSettings; import org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository; -import org.apache.activemq.artemis.integration.bootstrap.ActiveMQBootstrapLogger; import org.apache.activemq.artemis.utils.ExecutorFactory; @Command(name = "print", description = "Print data records information (WARNING: don't use while a production server is running)") public class PrintData extends DataAbstract implements Action { - @Override public Object execute(ActionContext context) throws Exception { @@ -70,7 +68,7 @@ public class PrintData extends DataAbstract implements Action } catch (Exception e) { - ActiveMQBootstrapLogger.LOGGER.printDataFailed(e.getMessage()); + treatError(e, "data", "print"); } return null; } diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/XmlDataExporter.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/XmlDataExporter.java index b8366d9b3e..c887dfb8b8 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/XmlDataExporter.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/tools/XmlDataExporter.java @@ -126,7 +126,14 @@ public final class XmlDataExporter extends DataAbstract implements Action @Override public Object execute(ActionContext context) throws Exception { - process(System.out, getBinding(), getJournal(), getPaging(), getLargeMessages()); + try + { + process(System.out, getBinding(), getJournal(), getPaging(), getLargeMessages()); + } + catch (Exception e) + { + treatError(e, "data", "exp"); + } return null; } diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/bootstrap/ActiveMQBootstrapBundle.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/bootstrap/ActiveMQBootstrapBundle.java index a7d14acd52..568b98e080 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/bootstrap/ActiveMQBootstrapBundle.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/bootstrap/ActiveMQBootstrapBundle.java @@ -18,7 +18,6 @@ package org.apache.activemq.artemis.integration.bootstrap; import org.jboss.logging.Messages; -import org.jboss.logging.annotations.Message; import org.jboss.logging.annotations.MessageBundle; /** @@ -33,6 +32,4 @@ public interface ActiveMQBootstrapBundle { ActiveMQBootstrapBundle BUNDLE = Messages.getBundle(ActiveMQBootstrapBundle.class); - @Message(id = 109000, value = "Directory does not exist: {0}", format = Message.Format.MESSAGE_FORMAT) - IllegalStateException directoryDoesNotExist(String directory); } diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/bootstrap/ActiveMQBootstrapLogger.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/bootstrap/ActiveMQBootstrapLogger.java index 2d0b311deb..0b3a467589 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/bootstrap/ActiveMQBootstrapLogger.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/integration/bootstrap/ActiveMQBootstrapLogger.java @@ -61,10 +61,6 @@ public interface ActiveMQBootstrapLogger extends BasicLogger @Message(id = 101003, value = "Halting ActiveMQ Artemis Server after user request", format = Message.Format.MESSAGE_FORMAT) void serverKilled(); - @LogMessage(level = Logger.Level.INFO) - @Message(id = 101004, value = "Broker configuration not found. Looking for data files in the ''{0}'' directory.", format = Message.Format.MESSAGE_FORMAT) - void brokerConfigNotFound(String defaultLocation); - @LogMessage(level = Logger.Level.INFO) @Message(id = 101005, value = "Using broker configuration: {0}", format = Message.Format.MESSAGE_FORMAT) void usingBrokerConfig(String location);