From 33d6721ffcd4f12c7dcd8a131c4ee5cf8140fb84 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Mon, 22 Apr 2019 12:04:48 -0400 Subject: [PATCH] ARTEMIS-2314 Improving Test and compatibility check on FQQN --- .../protocol/core/CoreRemotingConnection.java | 5 ++++ .../core/protocol/core/impl/PacketImpl.java | 1 + .../artemis/jms/client/ActiveMQSession.java | 11 ++++++++- .../artemis/core/server/ActiveMQServer.java | 6 ++++- .../core/server/impl/ActiveMQServerImpl.java | 4 ++-- .../core/server/impl/ServerSessionImpl.java | 12 +++++++++- tests/compatibility-tests/pom.xml | 23 ++++++++++++++++++- .../tests/compatibility/GroovyRun.java | 1 + .../fqqnConsumerProducer.groovy | 20 +++------------- .../tests/compatibility/FQQNConsumerTest.java | 13 ++++------- .../compatibility/base/VersionedBase.java | 12 ++++++++-- 11 files changed, 75 insertions(+), 33 deletions(-) diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/CoreRemotingConnection.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/CoreRemotingConnection.java index fac76e4dcc..377b1b5901 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/CoreRemotingConnection.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/CoreRemotingConnection.java @@ -46,6 +46,11 @@ public interface CoreRemotingConnection extends RemotingConnection { return version >= PacketImpl.CONSUMER_PRIORITY_CHANGE_VERSION; } + default boolean isVersionNewFQQN() { + int version = getChannelVersion(); + return version >= PacketImpl.ARTEMIS_2_7_0_VERSION; + } + /** * Sets the client protocol used on the communication. This will determine if the client has * support for certain packet types diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/PacketImpl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/PacketImpl.java index 41c2dc495d..f8f85e8a42 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/PacketImpl.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/PacketImpl.java @@ -36,6 +36,7 @@ public class PacketImpl implements Packet { public static final int ARTEMIS_2_7_0_VERSION = 130; public static final int ASYNC_RESPONSE_CHANGE_VERSION = ARTEMIS_2_7_0_VERSION; public static final int CONSUMER_PRIORITY_CHANGE_VERSION = ARTEMIS_2_7_0_VERSION; + public static final int FQQN_CHANGE_VERSION = ARTEMIS_2_7_0_VERSION; public static final SimpleString OLD_QUEUE_PREFIX = new SimpleString("jms.queue."); diff --git a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java index 7154abd6f1..f6223a658f 100644 --- a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java +++ b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java @@ -71,6 +71,7 @@ import org.apache.activemq.artemis.jms.client.compatible1X.ActiveMQStreamCompati import org.apache.activemq.artemis.jms.client.compatible1X.ActiveMQTextCompabileMessage; import org.apache.activemq.artemis.selector.filter.FilterException; import org.apache.activemq.artemis.selector.impl.SelectorParser; +import org.apache.activemq.artemis.utils.CompositeAddress; import org.apache.activemq.artemis.utils.SelectorTranslator; /** @@ -798,7 +799,7 @@ public class ActiveMQSession implements QueueSession, TopicSession { * Therefore, we must check if the queue names list contains the exact name of the address to know whether or * not a LOCAL binding for the address exists. If no LOCAL binding exists then it should be created here. */ - if (!response.isExists() || !response.getQueueNames().contains(dest.getSimpleAddress())) { + if (!response.isExists() || !response.getQueueNames().contains(getCoreQueueName(dest))) { if (response.isAutoCreateQueues()) { try { createQueue(dest, RoutingType.ANYCAST, dest.getSimpleAddress(), null, true, true, response); @@ -906,6 +907,14 @@ public class ActiveMQSession implements QueueSession, TopicSession { } } + private SimpleString getCoreQueueName(ActiveMQDestination dest) { + if (session.getVersion() < PacketImpl.FQQN_CHANGE_VERSION) { + return dest.getSimpleAddress(); + } else { + return CompositeAddress.extractQueueName(dest.getSimpleAddress()); + } + } + private ClientConsumer createClientConsumer(ActiveMQDestination destination, SimpleString queueName, SimpleString coreFilterString) throws ActiveMQException { QueueAttributes queueAttributes = destination.getQueueAttributes() == null ? new QueueAttributes() : destination.getQueueAttributes(); int priority = queueAttributes.getConsumerPriority() == null ? ActiveMQDefaultConfiguration.getDefaultConsumerPriority() : queueAttributes.getConsumerPriority(); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java index c7318bbf3a..cdb852075a 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java @@ -500,7 +500,11 @@ public interface ActiveMQServer extends ServiceComponent { Queue locateQueue(SimpleString queueName); - BindingQueryResult bindingQuery(SimpleString address) throws Exception; + default BindingQueryResult bindingQuery(SimpleString address) throws Exception { + return bindingQuery(address, true); + } + + BindingQueryResult bindingQuery(SimpleString address, boolean newFQQN) throws Exception; QueueQueryResult queueQuery(SimpleString name) throws Exception; 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 fd232e7c99..3320816ceb 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 @@ -886,7 +886,7 @@ public class ActiveMQServerImpl implements ActiveMQServer { } @Override - public BindingQueryResult bindingQuery(SimpleString address) throws Exception { + public BindingQueryResult bindingQuery(SimpleString address, boolean newFQQN) throws Exception { if (address == null) { throw ActiveMQMessageBundle.BUNDLE.addressIsNull(); } @@ -920,7 +920,7 @@ public class ActiveMQServerImpl implements ActiveMQServer { for (Binding binding : bindings.getBindings()) { if (binding.getType() == BindingType.LOCAL_QUEUE || binding.getType() == BindingType.REMOTE_QUEUE) { SimpleString name; - if (CompositeAddress.isFullyQualified(address.toString())) { + if (!newFQQN && CompositeAddress.isFullyQualified(address.toString())) { // need to use the FQQN here for backwards compatibility with core JMS client name = CompositeAddress.toFullyQualified(realAddress, binding.getUniqueName()); } else { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java index 2f83d358b1..f6d845529b 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java @@ -60,6 +60,7 @@ import org.apache.activemq.artemis.core.postoffice.BindingType; import org.apache.activemq.artemis.core.postoffice.PostOffice; import org.apache.activemq.artemis.core.postoffice.QueueBinding; import org.apache.activemq.artemis.core.postoffice.RoutingStatus; +import org.apache.activemq.artemis.core.protocol.core.CoreRemotingConnection; import org.apache.activemq.artemis.core.remoting.CertificateUtil; import org.apache.activemq.artemis.core.remoting.CloseListener; import org.apache.activemq.artemis.core.remoting.FailureListener; @@ -1052,7 +1053,16 @@ public class ServerSessionImpl implements ServerSession, FailureListener { @Override public BindingQueryResult executeBindingQuery(final SimpleString address) throws Exception { - return server.bindingQuery(removePrefix(address)); + + boolean newFQQN = true; + + // remotingConnection could be null on UnitTests + // that's why I'm checking for null here, and it's best to do so + if (remotingConnection != null && remotingConnection instanceof CoreRemotingConnection) { + newFQQN = ((CoreRemotingConnection) remotingConnection).isVersionNewFQQN(); + } + + return server.bindingQuery(removePrefix(address), newFQQN); } @Override diff --git a/tests/compatibility-tests/pom.xml b/tests/compatibility-tests/pom.xml index 2fd5d1b835..fa5e78530f 100644 --- a/tests/compatibility-tests/pom.xml +++ b/tests/compatibility-tests/pom.xml @@ -349,7 +349,28 @@ ARTEMIS-263 - + + compile + + dependency-scan + + 270-check + + + org.apache.activemq:artemis-jms-server:2.7.0 + org.apache.activemq:artemis-jms-client:2.7.0 + org.apache.activemq:artemis-cli:2.7.0 + org.apache.activemq:artemis-hornetq-protocol:2.7.0 + org.apache.activemq:artemis-amqp-protocol:2.7.0 + org.apache.activemq:artemis-hornetq-protocol:2.7.0 + org.codehaus.groovy:groovy-all:${groovy.version} + + + org.apache.activemq.tests:compatibility-tests:${project.version} + + ARTEMIS-270 + + compile diff --git a/tests/compatibility-tests/src/main/java/org/apache/activemq/artemis/tests/compatibility/GroovyRun.java b/tests/compatibility-tests/src/main/java/org/apache/activemq/artemis/tests/compatibility/GroovyRun.java index 9f28770003..b573b11cbb 100644 --- a/tests/compatibility-tests/src/main/java/org/apache/activemq/artemis/tests/compatibility/GroovyRun.java +++ b/tests/compatibility-tests/src/main/java/org/apache/activemq/artemis/tests/compatibility/GroovyRun.java @@ -34,6 +34,7 @@ public class GroovyRun { public static final String TWO_ONE = "ARTEMIS-210"; public static final String TWO_FOUR = "ARTEMIS-240"; public static final String TWO_SIX_THREE = "ARTEMIS-263"; + public static final String TWO_SEVEN_ZERO = "ARTEMIS-270"; public static final String HORNETQ_235 = "HORNETQ-235"; public static final String HORNETQ_247 = "HORNETQ-247"; diff --git a/tests/compatibility-tests/src/main/resources/fqqnconsumertest/fqqnConsumerProducer.groovy b/tests/compatibility-tests/src/main/resources/fqqnconsumertest/fqqnConsumerProducer.groovy index ebfe05fb13..bc5b81f3b4 100644 --- a/tests/compatibility-tests/src/main/resources/fqqnconsumertest/fqqnConsumerProducer.groovy +++ b/tests/compatibility-tests/src/main/resources/fqqnconsumertest/fqqnConsumerProducer.groovy @@ -44,32 +44,18 @@ if (clientType.startsWith("ARTEMIS")) { Connection connection = cf.createConnection(); -Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); +Session session = connection.createSession(true, Session.SESSION_TRANSACTED); Queue queue = session.createQueue(queueName); if (operation.equals("sendMessage")) { - CountDownLatch latch = new CountDownLatch(10); - - CompletionListener completionListener = new CompletionListener() { - @Override - void onCompletion(Message message) { - latch.countDown(); - } - - @Override - void onException(Message message, Exception exception) { - - } - } - MessageProducer producer = session.createProducer(queue); producer.setDeliveryMode(DeliveryMode.PERSISTENT); for (int i = 0; i < 10; i++) { - producer.send(session.createTextMessage(textBody + i), completionListener); + producer.send(session.createTextMessage(textBody + i)); } - GroovyRun.assertTrue(latch.await(10, TimeUnit.SECONDS)); + session.commit(); connection.close(); } else if (operation.equals("receiveMessage")) { diff --git a/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/FQQNConsumerTest.java b/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/FQQNConsumerTest.java index 338890cbb3..c40682d6b4 100644 --- a/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/FQQNConsumerTest.java +++ b/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/FQQNConsumerTest.java @@ -32,24 +32,21 @@ import org.junit.runners.Parameterized; import static org.apache.activemq.artemis.tests.compatibility.GroovyRun.SNAPSHOT; import static org.apache.activemq.artemis.tests.compatibility.GroovyRun.TWO_FOUR; import static org.apache.activemq.artemis.tests.compatibility.GroovyRun.TWO_ONE; +import static org.apache.activemq.artemis.tests.compatibility.GroovyRun.TWO_SEVEN_ZERO; import static org.apache.activemq.artemis.tests.compatibility.GroovyRun.TWO_SIX_THREE; import static org.apache.activemq.artemis.tests.compatibility.GroovyRun.TWO_ZERO; @RunWith(Parameterized.class) public class FQQNConsumerTest extends ServerBase { - // this will ensure that all tests in this class are run twice, - // once with "true" passed to the class' constructor and once with "false" @Parameterized.Parameters(name = "server={0}, producer={1}, consumer={2}") public static Collection getParameters() { - // FQQN support was added in 2.0 so testing several 2.x versions before 2.7 List combinations = new ArrayList<>(); - combinations.add(new Object[]{SNAPSHOT, TWO_SIX_THREE, TWO_SIX_THREE}); - combinations.add(new Object[]{SNAPSHOT, TWO_ZERO, TWO_ZERO}); - combinations.add(new Object[]{SNAPSHOT, TWO_ONE, TWO_ONE}); - combinations.add(new Object[]{SNAPSHOT, TWO_FOUR, TWO_FOUR}); - return combinations; + // FQQN was added into 2.7.0, hence we only test the server as SNAPSHOT or TWO_SEVEN_ZERO + List testsList = combinatory(new Object[]{SNAPSHOT}, new Object[]{SNAPSHOT, TWO_ZERO, TWO_FOUR, TWO_ONE, TWO_SIX_THREE, TWO_SEVEN_ZERO}, new Object[]{SNAPSHOT, TWO_ZERO, TWO_FOUR, TWO_ONE, TWO_SIX_THREE, TWO_SEVEN_ZERO}); + addCombinations(testsList, null, new Object[] {TWO_SEVEN_ZERO}, new Object[]{SNAPSHOT, TWO_SEVEN_ZERO}, new Object[]{SNAPSHOT, TWO_SEVEN_ZERO}); + return testsList; } public FQQNConsumerTest(String server, String sender, String receiver) throws Exception { diff --git a/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/base/VersionedBase.java b/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/base/VersionedBase.java index 1c5b2442ee..1ccbf303f2 100644 --- a/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/base/VersionedBase.java +++ b/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/base/VersionedBase.java @@ -63,6 +63,16 @@ public abstract class VersionedBase extends ClasspathBase { Object[] sideRight) { LinkedList combinations = new LinkedList<>(); + addCombinations(combinations, required, rootSide, sideLeft, sideRight); + + return combinations; + } + + protected static void addCombinations(List combinations, + Object required, + Object[] rootSide, + Object[] sideLeft, + Object[] sideRight) { for (Object root : rootSide) { for (Object left : sideLeft) { for (Object right : sideRight) { @@ -72,8 +82,6 @@ public abstract class VersionedBase extends ClasspathBase { } } } - - return combinations; } public void startServer(File folder, ClassLoader loader, String serverName) throws Throwable {