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 70581bccad..aa4af0e297 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 routingTypes) throws Exception; - AddressInfo removeAddressInfo(SimpleString address); + AddressInfo removeAddressInfo(SimpleString address) throws Exception; AddressInfo getAddressInfo(SimpleString address); 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 e2f5eae7b3..f1ca5aef26 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); } 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 8ff1a38b89..190ae69456 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); + } } } } @@ -126,6 +129,15 @@ public class WildcardAddressManager extends SimpleAddressManager { return binding; } + @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(); 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 93229e1354..d0250b9261 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(); + } } 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 f628fa095b..ef0d67300a 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 wildcardAddresses = (Map)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