ARTEMIS-2314 Improving Test and compatibility check on FQQN

apache · Apr 22, 2019 · 33d6721 · 33d6721
1 parent a2cb444
commit 33d6721
Show file tree

Hide file tree

Showing 11 changed files with 75 additions and 33 deletions.
diff --git a/.../src/main/java/org/apache/activemq/artemis/core/protocol/core/CoreRemotingConnection.java b/.../src/main/java/org/apache/activemq/artemis/core/protocol/core/CoreRemotingConnection.java
@@ -46,6 +46,11 @@ default boolean isVersionSupportConsumerPriority() {
       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/...-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/PacketImpl.java b/...-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
@@ -71,6 +71,7 @@
 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 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
              * 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 @@ private ActiveMQMessageConsumer createConsumer(final ActiveMQDestination dest,
       }
    }
 
+   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
@@ -500,7 +500,11 @@ void createSharedQueue(SimpleString address,
 
    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/...server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/...server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
@@ -886,7 +886,7 @@ public boolean isAddressBound(String address) throws Exception {
    }
 
    @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 BindingQueryResult bindingQuery(SimpleString address) throws Exception {
       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/...-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java b/...-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java
@@ -60,6 +60,7 @@
 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 AddressQueryResult executeAddressQuery(SimpleString name) throws Exceptio
 
    @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
@@ -349,7 +349,28 @@
                      </libList>
                      <variableName>ARTEMIS-263</variableName>
                   </configuration>
-               </execution>
+               </execution>               <execution>
+               <phase>compile</phase>
+               <goals>
+                  <goal>dependency-scan</goal>
+               </goals>
+               <id>270-check</id>
+               <configuration>
+                  <libListWithDeps>
+                     <arg>org.apache.activemq:artemis-jms-server:2.7.0</arg>
+                     <arg>org.apache.activemq:artemis-jms-client:2.7.0</arg>
+                     <arg>org.apache.activemq:artemis-cli:2.7.0</arg>
+                     <arg>org.apache.activemq:artemis-hornetq-protocol:2.7.0</arg>
+                     <arg>org.apache.activemq:artemis-amqp-protocol:2.7.0</arg>
+                     <arg>org.apache.activemq:artemis-hornetq-protocol:2.7.0</arg>
+                     <arg>org.codehaus.groovy:groovy-all:${groovy.version}</arg>
+                  </libListWithDeps>
+                  <libList>
+                     <arg>org.apache.activemq.tests:compatibility-tests:${project.version}</arg>
+                  </libList>
+                  <variableName>ARTEMIS-270</variableName>
+               </configuration>
+            </execution>
                <execution>
                   <phase>compile</phase>
                   <goals>

diff --git a/...bility-tests/src/main/java/org/apache/activemq/artemis/tests/compatibility/GroovyRun.java b/...bility-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
@@ -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/src/test/java/org/apache/activemq/artemis/tests/compatibility/FQQNConsumerTest.java b/...tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/FQQNConsumerTest.java
@@ -32,24 +32,21 @@
 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<Object[]> 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/...sts/src/test/java/org/apache/activemq/artemis/tests/compatibility/base/VersionedBase.java b/...sts/src/test/java/org/apache/activemq/artemis/tests/compatibility/base/VersionedBase.java
@@ -63,6 +63,16 @@ protected static List<Object[]> combinatory(Object required,
                                                Object[] sideRight) {
       LinkedList<Object[]> combinations = new LinkedList<>();
 
+      addCombinations(combinations, required, rootSide, sideLeft, sideRight);
+
+      return combinations;
+   }
+
+   protected static void addCombinations(List<Object[]> 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 @@ protected static List<Object[]> combinatory(Object required,
             }
          }
       }
-
-      return combinations;
    }
 
    public void startServer(File folder, ClassLoader loader, String serverName) throws Throwable {