You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cl...@apache.org on 2018/01/17 18:22:17 UTC

[1/2] activemq-artemis git commit: ARTEMIS-1610 Properly remove an address from the WildcardAddressManager

Repository: activemq-artemis
Updated Branches:
  refs/heads/master 14f149d75 -> 9e781be44


ARTEMIS-1610 Properly remove an address from the WildcardAddressManager

When an address is removed from the address manager its linked addresses
also need to be removed if there are no more bindings for the address.
Also adding a null check on bindings of linked addresses when a new
binding is added


Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo
Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/67a9220d
Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/67a9220d
Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/67a9220d

Branch: refs/heads/master
Commit: 67a9220d24bb530996d4268bfc7b8033f8210ea4
Parents: 14f149d
Author: Christopher L. Shannon (cshannon) <ch...@gmail.com>
Authored: Tue Jan 16 13:03:38 2018 -0500
Committer: Clebert Suconic <cl...@apache.org>
Committed: Wed Jan 17 13:20:04 2018 -0500

----------------------------------------------------------------------
 .../artemis/core/postoffice/AddressManager.java |  2 +-
 .../postoffice/impl/SimpleAddressManager.java   |  2 +-
 .../postoffice/impl/WildcardAddressManager.java | 16 ++++-
 .../jms/consumer/JmsConsumerTest.java           | 73 ++++++++++++++++++++
 .../impl/WildcardAddressManagerUnitTest.java    | 36 ++++++++++
 5 files changed, 125 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/67a9220d/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/AddressManager.java
----------------------------------------------------------------------
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/AddressManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/AddressManager.java
index 70581bc..aa4af0e 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/AddressManager.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/AddressManager.java
@@ -70,7 +70,7 @@ public interface AddressManager {
     *  it will throw an exception if the address doesn't exist */
    AddressInfo updateAddressInfo(SimpleString addressName, Collection<RoutingType> routingTypes) throws Exception;
 
-   AddressInfo removeAddressInfo(SimpleString address);
+   AddressInfo removeAddressInfo(SimpleString address) throws Exception;
 
    AddressInfo getAddressInfo(SimpleString address);
 

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/67a9220d/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java
----------------------------------------------------------------------
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java
index e2f5eae..f1ca5ae 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java
@@ -325,7 +325,7 @@ public class SimpleAddressManager implements AddressManager {
    }
 
    @Override
-   public AddressInfo removeAddressInfo(SimpleString address) {
+   public AddressInfo removeAddressInfo(SimpleString address) throws Exception {
       return addressInfoMap.remove(address);
    }
 

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/67a9220d/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/WildcardAddressManager.java
----------------------------------------------------------------------
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/WildcardAddressManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/WildcardAddressManager.java
index 8ff1a38..190ae69 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/WildcardAddressManager.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/WildcardAddressManager.java
@@ -23,6 +23,7 @@ import org.apache.activemq.artemis.core.postoffice.Address;
 import org.apache.activemq.artemis.core.postoffice.Binding;
 import org.apache.activemq.artemis.core.postoffice.Bindings;
 import org.apache.activemq.artemis.core.postoffice.BindingsFactory;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
 import org.apache.activemq.artemis.core.transaction.Transaction;
 
 import java.util.Collection;
@@ -95,8 +96,10 @@ public class WildcardAddressManager extends SimpleAddressManager {
          } else {
             for (Address destAdd : add.getLinkedAddresses()) {
                Bindings bindings = super.getBindingsForRoutingAddress(destAdd.getAddress());
-               for (Binding b : bindings.getBindings()) {
-                  super.addMappingInternal(binding.getAddress(), b);
+               if (bindings != null) {
+                  for (Binding b : bindings.getBindings()) {
+                     super.addMappingInternal(binding.getAddress(), b);
+                  }
                }
             }
          }
@@ -127,6 +130,15 @@ public class WildcardAddressManager extends SimpleAddressManager {
    }
 
    @Override
+   public AddressInfo removeAddressInfo(SimpleString address) throws Exception {
+      final AddressInfo removed = super.removeAddressInfo(address);
+      if (removed != null) {
+         removeAndUpdateAddressMap(new AddressImpl(removed.getName(), wildcardConfiguration));
+      }
+      return removed;
+   }
+
+   @Override
    public void clear() {
       super.clear();
       addresses.clear();

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/67a9220d/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/consumer/JmsConsumerTest.java
----------------------------------------------------------------------
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/consumer/JmsConsumerTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/consumer/JmsConsumerTest.java
index 93229e1..d0250b9 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/consumer/JmsConsumerTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/consumer/JmsConsumerTest.java
@@ -31,12 +31,14 @@ import javax.jms.TextMessage;
 import java.util.Enumeration;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.apache.activemq.artemis.api.core.RoutingType;
 import org.apache.activemq.artemis.api.core.SimpleString;
 import org.apache.activemq.artemis.api.core.TransportConfiguration;
 import org.apache.activemq.artemis.api.jms.ActiveMQJMSClient;
 import org.apache.activemq.artemis.api.jms.ActiveMQJMSConstants;
 import org.apache.activemq.artemis.api.jms.JMSFactoryType;
 import org.apache.activemq.artemis.core.server.Queue;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
 import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
 import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory;
 import org.apache.activemq.artemis.tests.integration.IntegrationTestLogger;
@@ -815,4 +817,75 @@ public class JmsConsumerTest extends JMSTestBase {
 
       connection.close();
    }
+
+   /**
+    * Test for ARTEMIS-1610
+    * @throws Exception
+    */
+   @Test
+   public void testConsumerAfterWildcardAddressRemoval() throws Exception {
+      String queue1 = "queue.#";
+      String topic1 = "durable.#";
+      String topic2 = "durable.test";
+
+      //Create a new address along with 1 queue for it (this cases a wildcard address to get registered
+      //inside the WildcardAddressManager manager when the binding is created)
+      server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(queue1), RoutingType.ANYCAST));
+      server.createQueue(SimpleString.toSimpleString(queue1), RoutingType.ANYCAST,
+            SimpleString.toSimpleString(queue1), null, false, false);
+
+      //create addresses for both topics
+      server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(topic1), RoutingType.MULTICAST));
+      server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(topic2), RoutingType.MULTICAST));
+
+      //Remove the wildcard address associated with topic2
+      server.removeAddressInfo(SimpleString.toSimpleString(topic1), null);
+
+      conn = cf.createConnection();
+      conn.setClientID("clientId");
+      conn.start();
+      Session sess = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+      //Verify consumer can be created without issue - this caused a NPE
+      //before ARTEMIS-1610
+      sess.createConsumer(sess.createTopic(topic2));
+      sess.close();
+   }
+
+   /**
+    * Test for ARTEMIS-1610
+    * @throws Exception
+    */
+   @Test
+   public void testConsumerAfterWildcardConsumer() throws Exception {
+      String queue1 = "queue.#";
+      String topic1 = "durable.#";
+      String topic2 = "durable.test";
+
+      //Create a new address along with 1 queue for it (this cases a wildcard address to get registered
+      //inside the WildcardAddressManager manager when the binding is created)
+      server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(queue1), RoutingType.ANYCAST));
+      server.createQueue(SimpleString.toSimpleString(queue1), RoutingType.ANYCAST,
+            SimpleString.toSimpleString(queue1), null, false, false);
+
+      //create addresses for both topics
+      server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(topic1), RoutingType.MULTICAST));
+      server.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(topic2), RoutingType.MULTICAST));
+
+      conn = cf.createConnection();
+      conn.setClientID("clientId");
+      conn.start();
+      Session sess = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
+
+      //create and close consumer on wildcard
+      MessageConsumer c = sess.createConsumer(sess.createTopic(topic1));
+      c.close();
+
+      //Verify consumer can be created without issue - this caused a NPE
+      //before ARTEMIS-1610
+      //will create binding for this topic and will link addresses with durable.# so need to do a null check
+      //on binding add of linked addresses
+      sess.createConsumer(sess.createTopic(topic2));
+      sess.close();
+   }
 }

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/67a9220d/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/WildcardAddressManagerUnitTest.java
----------------------------------------------------------------------
diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/WildcardAddressManagerUnitTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/WildcardAddressManagerUnitTest.java
index f628fa0..ef0d673 100644
--- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/WildcardAddressManagerUnitTest.java
+++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/core/postoffice/impl/WildcardAddressManagerUnitTest.java
@@ -16,12 +16,16 @@
  */
 package org.apache.activemq.artemis.tests.unit.core.postoffice.impl;
 
+import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Map;
 
 import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.api.core.RoutingType;
 import org.apache.activemq.artemis.api.core.SimpleString;
 import org.apache.activemq.artemis.core.filter.Filter;
+import org.apache.activemq.artemis.core.postoffice.Address;
 import org.apache.activemq.artemis.core.postoffice.Binding;
 import org.apache.activemq.artemis.core.postoffice.BindingType;
 import org.apache.activemq.artemis.core.postoffice.Bindings;
@@ -31,6 +35,7 @@ import org.apache.activemq.artemis.core.server.Bindable;
 import org.apache.activemq.artemis.core.server.Queue;
 import org.apache.activemq.artemis.core.server.RoutingContext;
 import org.apache.activemq.artemis.core.server.cluster.impl.MessageLoadBalancingType;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
 import org.junit.Test;
 
@@ -99,6 +104,37 @@ public class WildcardAddressManagerUnitTest extends ActiveMQTestBase {
       assertEquals("Exception happened during the process", 0, errors);
    }
 
+   /**
+    * Test for ARTEMIS-1610
+    * @throws Exception
+    */
+   @SuppressWarnings("unchecked")
+   @Test
+   public void testWildCardAddressRemoval() throws Exception {
+
+      WildcardAddressManager ad = new WildcardAddressManager(new BindingFactoryFake(), null);
+      ad.addAddressInfo(new AddressInfo(SimpleString.toSimpleString("Queue1.#"), RoutingType.ANYCAST));
+      ad.addAddressInfo(new AddressInfo(SimpleString.toSimpleString("Topic1.#"), RoutingType.MULTICAST));
+      ad.addBinding(new BindingFake("Topic1.topic", "two"));
+      ad.addBinding(new BindingFake("Queue1.#", "one"));
+
+      Field wildcardAddressField = WildcardAddressManager.class.getDeclaredField("wildCardAddresses");
+      wildcardAddressField.setAccessible(true);
+      Map<SimpleString, Address> wildcardAddresses = (Map<SimpleString, Address>)wildcardAddressField.get(ad);
+
+      //Calling this method will trigger the wildcard to be added to the wildcard map internal
+      //to WildcardAddressManager
+      ad.getBindingsForRoutingAddress(SimpleString.toSimpleString("Topic1.#"));
+
+      //Remove the address
+      ad.removeAddressInfo(SimpleString.toSimpleString("Topic1.#"));
+
+      //Verify the address was cleaned up properly
+      assertEquals(1, wildcardAddresses.size());
+      assertNull(ad.getAddressInfo(SimpleString.toSimpleString("Topic1.#")));
+      assertNull(wildcardAddresses.get(SimpleString.toSimpleString("Topic1.#")));
+   }
+
    class BindingFactoryFake implements BindingsFactory {
 
       @Override


[2/2] activemq-artemis git commit: This closes #1780

Posted by cl...@apache.org.
This closes #1780


Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo
Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/9e781be4
Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/9e781be4
Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/9e781be4

Branch: refs/heads/master
Commit: 9e781be44e7fc3cb816e9fa095add6bda791b7c8
Parents: 14f149d 67a9220
Author: Clebert Suconic <cl...@apache.org>
Authored: Wed Jan 17 13:22:08 2018 -0500
Committer: Clebert Suconic <cl...@apache.org>
Committed: Wed Jan 17 13:22:08 2018 -0500

----------------------------------------------------------------------
 .../artemis/core/postoffice/AddressManager.java |  2 +-
 .../postoffice/impl/SimpleAddressManager.java   |  2 +-
 .../postoffice/impl/WildcardAddressManager.java | 16 ++++-
 .../jms/consumer/JmsConsumerTest.java           | 73 ++++++++++++++++++++
 .../impl/WildcardAddressManagerUnitTest.java    | 36 ++++++++++
 5 files changed, 125 insertions(+), 4 deletions(-)
----------------------------------------------------------------------