From 741fd433c0b729f5beb69fae1c55649e3dfe8e52 Mon Sep 17 00:00:00 2001 From: Shailendra Kumar Singh Date: Wed, 26 Jun 2019 12:37:30 +0530 Subject: [PATCH 1/2] ARTEMIS-2403 Consumer command, clientID is ignored on CLI during retry connection with stdin input --- .../commands/messages/ConnectionAbstract.java | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java index 90882e6c0b..a4a1f1e128 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java @@ -72,13 +72,21 @@ public class ConnectionAbstract extends InputAbstract { // if a security exception will get the user and password through an input context.err.println("Connection failed::" + e.getMessage()); userPassword(); - return new JmsConnectionFactory(user, password, brokerURL); + cf = new JmsConnectionFactory(user, password, brokerURL); + if (clientID != null) { + cf.setClientID(clientID); + } + return cf; } catch (JMSException e) { // if a connection exception will ask for the URL, user and password context.err.println("Connection failed::" + e.getMessage()); brokerURL = input("--url", "Type in the broker URL for a retry (e.g. tcp://localhost:61616)", brokerURL); userPassword(); - return new JmsConnectionFactory(user, password, brokerURL); + cf = new JmsConnectionFactory(user, password, brokerURL); + if (clientID != null) { + cf.setClientID(clientID); + } + return cf; } } @@ -97,13 +105,21 @@ public class ConnectionAbstract extends InputAbstract { // if a security exception will get the user and password through an input context.err.println("Connection failed::" + e.getMessage()); userPassword(); - return new ActiveMQConnectionFactory(brokerURL, user, password); + cf = new ActiveMQConnectionFactory(brokerURL, user, password); + if (clientID != null) { + cf.setClientID(clientID); + } + return cf; } catch (JMSException e) { // if a connection exception will ask for the URL, user and password context.err.println("Connection failed::" + e.getMessage()); brokerURL = input("--url", "Type in the broker URL for a retry (e.g. tcp://localhost:61616)", brokerURL); userPassword(); - return new ActiveMQConnectionFactory(brokerURL, user, password); + cf = new ActiveMQConnectionFactory(brokerURL, user, password); + if (clientID != null) { + cf.setClientID(clientID); + } + return cf; } } From 705f1db17b7fd198bf5bdf48a181cd173ce7334d Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Wed, 26 Jun 2019 15:22:23 -0400 Subject: [PATCH 2/2] ARTEMIS-2403 Adding test with clientID on retry after exceptions --- .../artemis/cli/commands/InputAbstract.java | 6 +- .../commands/messages/ConnectionAbstract.java | 40 +++++++++++- .../artemis/cli/commands/queue/StatQueue.java | 8 --- .../cli/test/RetryCLIClientIDTest.java | 64 +++++++++++++++++++ 4 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 artemis-cli/src/test/java/org/apache/activemq/cli/test/RetryCLIClientIDTest.java diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/InputAbstract.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/InputAbstract.java index cadb702cc3..05a6035464 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/InputAbstract.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/InputAbstract.java @@ -37,10 +37,14 @@ public class InputAbstract extends ActionAbstract { @Option(name = "--silent", description = "It will disable all the inputs, and it would make a best guess for any required input") private boolean silentInput = false; - private boolean isSilentInput() { + public boolean isSilentInput() { return silentInput || !inputEnabled; } + public void setSilentInput(boolean isSilent) { + this.silentInput = isSilent; + } + protected boolean inputBoolean(String propertyName, String prompt, boolean silentDefault) { if (isSilentInput()) { return silentDefault; diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java index a4a1f1e128..c170259e64 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/messages/ConnectionAbstract.java @@ -44,6 +44,38 @@ public class ConnectionAbstract extends InputAbstract { @Option(name = "--protocol", description = "Protocol used. Valid values are amqp or core. Default=core.") String protocol = "core"; + public String getUser() { + return user; + } + + public void setUser(String user) { + this.user = user; + } + + public String getPassword() { + return password; + } + + public void setPassword(String password) { + this.password = password; + } + + public String getClientID() { + return clientID; + } + + public void setClientID(String clientID) { + this.clientID = clientID; + } + + public String getProtocol() { + return protocol; + } + + public void setProtocol(String protocol) { + this.protocol = protocol; + } + protected ConnectionFactory createConnectionFactory() throws Exception { if (protocol.equals("core")) { return createCoreConnectionFactory(); @@ -103,7 +135,9 @@ public class ConnectionAbstract extends InputAbstract { return cf; } catch (JMSSecurityException e) { // if a security exception will get the user and password through an input - context.err.println("Connection failed::" + e.getMessage()); + if (context != null) { + context.err.println("Connection failed::" + e.getMessage()); + } userPassword(); cf = new ActiveMQConnectionFactory(brokerURL, user, password); if (clientID != null) { @@ -112,7 +146,9 @@ public class ConnectionAbstract extends InputAbstract { return cf; } catch (JMSException e) { // if a connection exception will ask for the URL, user and password - context.err.println("Connection failed::" + e.getMessage()); + if (context != null) { + context.err.println("Connection failed::" + e.getMessage()); + } brokerURL = input("--url", "Type in the broker URL for a retry (e.g. tcp://localhost:61616)", brokerURL); userPassword(); cf = new ActiveMQConnectionFactory(brokerURL, user, password); diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/queue/StatQueue.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/queue/StatQueue.java index 9f3e85676e..2d74fc44e9 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/queue/StatQueue.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/queue/StatQueue.java @@ -90,14 +90,6 @@ public class StatQueue extends AbstractAction { this.maxRows = maxRows; } - public void setUser(String user) { - this.user = user; - } - - public void setPassword(String password) { - this.password = password; - } - public void setverbose(boolean verbose) { this.verbose = verbose; } diff --git a/artemis-cli/src/test/java/org/apache/activemq/cli/test/RetryCLIClientIDTest.java b/artemis-cli/src/test/java/org/apache/activemq/cli/test/RetryCLIClientIDTest.java new file mode 100644 index 0000000000..5043b93612 --- /dev/null +++ b/artemis-cli/src/test/java/org/apache/activemq/cli/test/RetryCLIClientIDTest.java @@ -0,0 +1,64 @@ +/* + * 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * 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.cli.test; + +import java.io.File; + +import org.apache.activemq.artemis.api.core.Pair; +import org.apache.activemq.artemis.api.core.management.ActiveMQServerControl; +import org.apache.activemq.artemis.cli.Artemis; +import org.apache.activemq.artemis.cli.commands.Run; +import org.apache.activemq.artemis.cli.commands.messages.ConnectionAbstract; +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.core.server.management.ManagementContext; +import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory; +import org.junit.Assert; +import org.junit.Test; + +public class RetryCLIClientIDTest extends CliTestBase { + + @Test + public void testWrongUserAndPass() throws Exception { + try { + Run.setEmbedded(true); + File instance1 = new File(temporaryFolder.getRoot(), "instance_user"); + System.setProperty("java.security.auth.login.config", instance1.getAbsolutePath() + "/etc/login.config"); + Artemis.main("create", instance1.getAbsolutePath(), "--silent", "--no-autotune", "--no-web", "--no-amqp-acceptor", "--no-mqtt-acceptor", "--no-stomp-acceptor", "--no-hornetq-acceptor", "--user", "dumb", "--password", "dumber", "--require-login"); + System.setProperty("artemis.instance", instance1.getAbsolutePath()); + Object result = Artemis.internalExecute("run"); + ActiveMQServer activeMQServer = ((Pair) result).getB(); + ActiveMQServerControl activeMQServerControl = activeMQServer.getActiveMQServerControl(); + + ConnectionTest test = new ConnectionTest(); + test.setSilentInput(true); + test.setClientID("someClientID"); + ActiveMQConnectionFactory cf = test.newCF(); + Assert.assertEquals("someClientID", cf.getClientID()); + + } finally { + stopServer(); + } + } + + private class ConnectionTest extends ConnectionAbstract { + + public ActiveMQConnectionFactory newCF() { + return createCoreConnectionFactory(); + } + } +}