ARTEMIS-5997 refactor SimpleAddressManager.getMatchingQueue

jbertram · tabish121 · commit 93eaeadf925f · 2026-04-14T17:03:52.000-04:00
The getMatchingQueue method on SimpleAddressManager is overloaded, but
only one of them is actually necessary. The three-parameter version is
only used once and the method that uses it has already performed all
the same work.

Furthermore, the two-parameter version doesn't actually check the
routing-type and the first if statement omits the wild check.

This commit:
 - Removes the unnecessary version of getMatchingQueue
 - Adds missing checks to the remaining getMatchingQueue
 - Adds JavaDoc
 - Adds tests
diff --git a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java
@@ -832,12 +832,6 @@ public SimpleString getMatchingQueue(SimpleString address, RoutingType routingTy
       return serverSession.getMatchingQueue(address, routingType);
    }
 
-   public SimpleString getMatchingQueue(SimpleString address,
-                                        SimpleString queueName,
-                                        RoutingType routingType) throws Exception {
-      return serverSession.getMatchingQueue(address, queueName, routingType);
-   }
-
    public AddressInfo getAddress(SimpleString address) {
       return serverSession.getAddress(address);
    }
diff --git a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/DefaultSenderController.java b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/DefaultSenderController.java
@@ -527,7 +527,7 @@ protected SimpleString getMatchingQueue(SimpleString queueName, SimpleString add
                throw new ActiveMQIllegalStateException("Queue: " + queueName + " filter mismatch [" + filter + "] is different than existing filter [" + result.getFilterString() + "]");
 
             }
-            return sessionSPI.getMatchingQueue(address, queueName, routingType);
+            return queueName;
          }
       }
 
diff --git a/artemis-server/pom.xml b/artemis-server/pom.xml
@@ -290,6 +290,12 @@
          <artifactId>mockserver-client-java</artifactId>
          <scope>test</scope>
       </dependency>
+      <dependency>
+         <groupId>org.hamcrest</groupId>
+         <artifactId>hamcrest</artifactId>
+         <version>${hamcrest.version}</version>
+         <scope>test</scope>
+      </dependency>
    </dependencies>
 
 <build>
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
@@ -51,8 +51,6 @@ public interface AddressManager {
 
    SimpleString getMatchingQueue(SimpleString address, RoutingType routingType) throws Exception;
 
-   SimpleString getMatchingQueue(SimpleString address, SimpleString queueName, RoutingType routingType) throws Exception;
-
    LocalQueueBinding findLocalBinding(long id);
 
    void clear();
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/PostOffice.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/PostOffice.java
@@ -149,8 +149,6 @@ default Queue findQueue(final long bindingID) {
 
    SimpleString getMatchingQueue(SimpleString address, RoutingType routingType) throws Exception;
 
-   SimpleString getMatchingQueue(SimpleString address, SimpleString queueName, RoutingType routingType) throws Exception;
-
    RoutingStatus route(Message message, boolean direct) throws Exception;
 
    RoutingStatus route(Message message,
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
@@ -1538,13 +1538,6 @@ public SimpleString getMatchingQueue(SimpleString address, RoutingType routingTy
       return addressManager.getMatchingQueue(address, routingType);
    }
 
-   @Override
-   public SimpleString getMatchingQueue(SimpleString address,
-                                        SimpleString queueName,
-                                        RoutingType routingType) throws Exception {
-      return addressManager.getMatchingQueue(address, queueName, routingType);
-   }
-
    @Override
    public void sendQueueInfoToQueue(final SimpleString queueName, final SimpleString address) throws Exception {
       // We send direct to the queue so we can send it to the same queue that is bound to the notifications 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
@@ -75,17 +75,12 @@ public class SimpleAddressManager implements AddressManager {
 
    protected final WildcardConfiguration wildcardConfiguration;
 
-   public SimpleAddressManager(final BindingsFactory bindingsFactory, final StorageManager storageManager,
-                               final MetricsManager metricsManager) {
-      this(bindingsFactory, new WildcardConfiguration(), storageManager, metricsManager);
-   }
-
    public SimpleAddressManager(final BindingsFactory bindingsFactory,
                                final WildcardConfiguration wildcardConfiguration,
                                final StorageManager storageManager,
                                final MetricsManager metricsManager) {
-      this.wildcardConfiguration = wildcardConfiguration;
       this.bindingsFactory = bindingsFactory;
+      this.wildcardConfiguration = wildcardConfiguration;
       this.storageManager = storageManager;
       this.metricsManager = metricsManager;
    }
@@ -190,37 +185,49 @@ public Collection<Binding> getDirectBindings(final SimpleString address) throws
       return Collections.unmodifiableCollection(outputList);
    }
 
+   /**
+    * This method looks at the local queue bindings of an address to find one with a matching routing type that also is
+    * not wild. A match with the same name as the input address is preferred.
+    * <p>
+    * If a match is found, the unique name of the corresponding binding is returned. If no match is found, the method
+    * returns {@code null}.
+    *
+    * @param address     the address to match against
+    * @param routingType the routing type to use for matching the binding
+    * @return the unique name of the matching binding, or {@code null} if no match is found
+    * @throws Exception if an error occurs while attempting to find a matching binding
+    */
    @Override
    public SimpleString getMatchingQueue(final SimpleString address, RoutingType routingType) throws Exception {
       SimpleString realAddress = CompositeAddress.extractAddressName(address);
-      Binding binding = getBinding(realAddress);
 
-      if (binding == null || !(binding instanceof LocalQueueBinding) || !binding.getAddress().equals(realAddress)) {
-         Bindings bindings = mappings.get(realAddress);
-         if (bindings != null) {
-            for (Binding theBinding : bindings.getBindings()) {
-               if (theBinding instanceof LocalQueueBinding && !wildcardConfiguration.isWild(theBinding.getUniqueName())) {
-                  binding = theBinding;
-                  break;
-               }
+      Binding potentialMatch = getBinding(realAddress);
+      if (checkBindingForMatch(potentialMatch, realAddress, routingType)) {
+         // a local queue binding with the same name as the input address is preferred
+         return potentialMatch.getUniqueName();
+      }
+
+      Bindings bindings = mappings.get(realAddress);
+      if (bindings != null) {
+         for (Binding b : bindings.getBindings()) {
+            if (checkBindingForMatch(b, realAddress, routingType)) {
+               return b.getUniqueName();
             }
          }
       }
 
-      return binding != null ? binding.getUniqueName() : null;
+      return null;
    }
 
-   @Override
-   public SimpleString getMatchingQueue(final SimpleString address,
-                                        final SimpleString queueName,
-                                        RoutingType routingType) throws Exception {
-      SimpleString realAddress = CompositeAddress.extractAddressName(address);
-      Binding binding = getBinding(queueName);
-
-      if (binding != null && !binding.getAddress().equals(realAddress) && !realAddress.toString().isEmpty()) {
-         throw new IllegalStateException("queue belongs to address" + binding.getAddress());
+   private boolean checkBindingForMatch(Binding binding, SimpleString address, RoutingType routingType) {
+      if (binding != null &&
+         binding instanceof LocalQueueBinding lqb &&
+         lqb.getAddress().equals(address) &&
+         lqb.getQueue().getRoutingType() == routingType &&
+         !wildcardConfiguration.isWild(binding.getUniqueName())) {
+         return true;
       }
-      return binding != null ? binding.getUniqueName() : null;
+      return false;
    }
 
    @Override
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
@@ -41,12 +41,6 @@ public WildcardAddressManager(final BindingsFactory bindingsFactory,
       super(bindingsFactory, wildcardConfiguration, storageManager, metricsManager);
    }
 
-   public WildcardAddressManager(final BindingsFactory bindingsFactory,
-                                 final StorageManager storageManager,
-                                 final MetricsManager metricsManager) {
-      super(bindingsFactory, storageManager, metricsManager);
-   }
-
    // publish, may be a new address that needs wildcard bindings added
    // won't contain a wildcard because we don't ever route to a wildcards at this time
    @Override
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ServerSession.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ServerSession.java
@@ -497,10 +497,6 @@ void createSharedQueue(SimpleString address,
 
    SimpleString getMatchingQueue(SimpleString address, RoutingType routingType) throws Exception;
 
-   SimpleString getMatchingQueue(SimpleString address,
-                                 SimpleString queueName,
-                                 RoutingType routingType) throws Exception;
-
    AddressInfo getAddress(SimpleString address);
 
    /**
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
@@ -1935,7 +1935,7 @@ public AutoCreateResult checkAutoCreate(final QueueConfiguration queueConfig) th
          Queue q = server.locateQueue(unPrefixedQueue);
          if (q == null) {
             // The queue doesn't exist.
-            if (!queueConfig.isFqqn() && server.getPostOffice().getMatchingQueue(unPrefixedAddress, queueConfig.getRoutingType()) != null) {
+            if (!queueConfig.isFqqn() && getMatchingQueue(unPrefixedAddress, queueConfig.getRoutingType()) != null) {
                // The address has a local, non-wildcard queue with a different name, which is fine. Just ignore it.
                result = AutoCreateResult.EXISTED;
             } else if (addressSettings.isAutoCreateQueues() || queueConfig.isTemporary()) {
@@ -2226,13 +2226,6 @@ public SimpleString getMatchingQueue(SimpleString address, RoutingType routingTy
       return server.getPostOffice().getMatchingQueue(address, routingType);
    }
 
-   @Override
-   public SimpleString getMatchingQueue(SimpleString address,
-                                        SimpleString queueName,
-                                        RoutingType routingType) throws Exception {
-      return server.getPostOffice().getMatchingQueue(address, queueName, routingType);
-   }
-
    @Override
    public AddressInfo getAddress(SimpleString address) {
       return server.getPostOffice().getAddressInfo(removePrefix(address));
diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/postoffice/impl/AddressManagerGetMatchingQueueTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/postoffice/impl/AddressManagerGetMatchingQueueTest.java
@@ -0,0 +1,165 @@
+/*
+ * 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.artemis.core.postoffice.impl;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.config.WildcardConfiguration;
+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.Queue;
+import org.junit.jupiter.api.Test;
+
+import static org.apache.activemq.artemis.utils.RandomUtil.randomUUIDSimpleString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class AddressManagerGetMatchingQueueTest {
+
+   @Test
+   public void testCorrectNameCorrectRoutingTypeSingleQueue() throws Exception {
+      SimpleString addressName = SimpleString.of("myAddress");
+      SimpleString queueName = randomUUIDSimpleString();
+      SimpleAddressManager am = getSimpleAddressManager();
+      am.addBinding(getLocalQueueBinding(addressName, queueName, RoutingType.ANYCAST));
+
+      assertEquals(queueName, am.getMatchingQueue(addressName, RoutingType.ANYCAST));
+   }
+
+   @Test
+   public void testCorrectNameCorrectRoutingTypeMultipleQueues() throws Exception {
+      SimpleString addressName = SimpleString.of("myAddress");
+      SimpleString queueName1 = randomUUIDSimpleString();
+      SimpleString queueName2 = randomUUIDSimpleString();
+      SimpleAddressManager am = getSimpleAddressManager();
+      am.addBinding(getLocalQueueBinding(addressName, queueName1, RoutingType.ANYCAST));
+      am.addBinding(getLocalQueueBinding(addressName, queueName2, RoutingType.ANYCAST));
+
+      assertThat(am.getMatchingQueue(addressName, RoutingType.ANYCAST), anyOf(is(queueName1), is(queueName2)));
+   }
+
+   @Test
+   public void testCorrectNameCorrectRoutingTypeMultipleQueuesMixedRoutingTypes() throws Exception {
+      SimpleString addressName = SimpleString.of("myAddress");
+      SimpleString queueName1 = randomUUIDSimpleString();
+      SimpleString queueName2 = randomUUIDSimpleString();
+      SimpleAddressManager am = getSimpleAddressManager();
+      am.addBinding(getLocalQueueBinding(addressName, queueName1, RoutingType.ANYCAST));
+      am.addBinding(getLocalQueueBinding(addressName, queueName2, RoutingType.MULTICAST));
+
+      assertEquals(queueName1, am.getMatchingQueue(addressName, RoutingType.ANYCAST));
+   }
+
+   @Test
+   public void testCorrectNameIncorrectRoutingTypeSingleQueue() throws Exception {
+      SimpleString addressName = SimpleString.of("myAddress");
+      SimpleAddressManager am = getSimpleAddressManager();
+      am.addBinding(getLocalQueueBinding(addressName, RoutingType.MULTICAST));
+
+      assertNull(am.getMatchingQueue(addressName, RoutingType.ANYCAST));
+   }
+
+   @Test
+   public void testCorrectNameIncorrectRoutingTypeMultipleQueues() throws Exception {
+      SimpleString addressName = SimpleString.of("myAddress");
+      SimpleAddressManager am = getSimpleAddressManager();
+      am.addBinding(getLocalQueueBinding(addressName, RoutingType.MULTICAST));
+      am.addBinding(getLocalQueueBinding(addressName, RoutingType.MULTICAST));
+
+      assertNull(am.getMatchingQueue(addressName, RoutingType.ANYCAST));
+   }
+
+   @Test
+   public void testIncorrectNameCorrectRoutingTypeSingleQueue() throws Exception {
+      SimpleAddressManager am = getSimpleAddressManager();
+      am.addBinding(getLocalQueueBinding(randomUUIDSimpleString(), RoutingType.ANYCAST));
+
+      assertNull(am.getMatchingQueue(randomUUIDSimpleString(), RoutingType.ANYCAST));
+   }
+
+   @Test
+   public void testIncorrectNameCorrectRoutingTypeMultipleQueues() throws Exception {
+      SimpleAddressManager am = getSimpleAddressManager();
+      am.addBinding(getLocalQueueBinding(randomUUIDSimpleString(), RoutingType.ANYCAST));
+      am.addBinding(getLocalQueueBinding(randomUUIDSimpleString(), RoutingType.ANYCAST));
+
+      assertNull(am.getMatchingQueue(randomUUIDSimpleString(), RoutingType.ANYCAST));
+   }
+
+   @Test
+   public void testIncorrectNameIncorrectRoutingTypeSingleQueue() throws Exception {
+      SimpleAddressManager am = getSimpleAddressManager();
+      am.addBinding(getLocalQueueBinding(randomUUIDSimpleString(), RoutingType.MULTICAST));
+
+      assertNull(am.getMatchingQueue(randomUUIDSimpleString(), RoutingType.ANYCAST));
+   }
+
+   @Test
+   public void testIncorrectNameIncorrectRoutingTypeMultipleQueues() throws Exception {
+      SimpleAddressManager am = getSimpleAddressManager();
+      am.addBinding(getLocalQueueBinding(randomUUIDSimpleString(), RoutingType.MULTICAST));
+      am.addBinding(getLocalQueueBinding(randomUUIDSimpleString(), RoutingType.MULTICAST));
+
+      assertNull(am.getMatchingQueue(randomUUIDSimpleString(), RoutingType.ANYCAST));
+   }
+
+   private static SimpleAddressManager getSimpleAddressManager() throws Exception {
+      return new SimpleAddressManager(getBindingsFactory(), new WildcardConfiguration(), null, null);
+   }
+
+   private static LocalQueueBinding getLocalQueueBinding(SimpleString addressName, RoutingType routingType) {
+      return getLocalQueueBinding(addressName, randomUUIDSimpleString(), routingType);
+   }
+
+   private static LocalQueueBinding getLocalQueueBinding(SimpleString addressName, SimpleString queueName, RoutingType routingType) {
+      return new LocalQueueBinding(addressName, getQueue(queueName, routingType), randomUUIDSimpleString());
+   }
+
+   private static BindingsFactory getBindingsFactory() throws Exception {
+      List<Binding> bindingsList = new ArrayList<>();
+      Bindings bindings = mock(Bindings.class);
+      doAnswer(invocation -> {
+         bindingsList.add(invocation.getArgument(0));
+         return null;
+      }).when(bindings).addBinding(any(Binding.class));
+      when(bindings.getBindings()).thenReturn(bindingsList);
+
+      BindingsFactory bindingsFactory = mock(BindingsFactory.class);
+      when(bindingsFactory.createBindings(any(SimpleString.class))).thenReturn(bindings);
+      return bindingsFactory;
+   }
+
+   private static Queue getQueue(SimpleString queueName, RoutingType anycast) {
+      Queue q = mock(Queue.class);
+      when(q.getName()).thenReturn(queueName);
+      when(q.getID()).thenReturn(1L);
+      when(q.getRoutingType()).thenReturn(anycast);
+      return q;
+   }
+}
diff --git a/tests/artemis-test-support/src/main/java/org/apache/activemq/artemis/tests/unit/core/server/impl/fakes/FakePostOffice.java b/tests/artemis-test-support/src/main/java/org/apache/activemq/artemis/tests/unit/core/server/impl/fakes/FakePostOffice.java
@@ -116,11 +116,6 @@ public SimpleString getMatchingQueue(SimpleString address, RoutingType routingTy
       return null;
    }
 
-   @Override
-   public SimpleString getMatchingQueue(SimpleString address, SimpleString queueName, RoutingType routingType) {
-      return null;
-   }
-
    @Override
    public void start() throws Exception {
 
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

Original file line number	Diff line number	Diff line change
`@@ -832,12 +832,6 @@ public SimpleString getMatchingQueue(SimpleString address, RoutingType routingTy`
`832`	`832`	`return serverSession.getMatchingQueue(address, routingType);`
`833`	`833`	`}`
`834`	`834`
`835`		`- public SimpleString getMatchingQueue(SimpleString address,`
`836`		`- SimpleString queueName,`
`837`		`- RoutingType routingType) throws Exception {`
`838`		`- return serverSession.getMatchingQueue(address, queueName, routingType);`
`839`		`- }`
`840`		`-`
`841`	`835`	`public AddressInfo getAddress(SimpleString address) {`
`842`	`836`	`return serverSession.getAddress(address);`
`843`	`837`	`}`
Original file line number	Diff line number	Diff line change
`@@ -527,7 +527,7 @@ protected SimpleString getMatchingQueue(SimpleString queueName, SimpleString add`
`527`	`527`	`throw new ActiveMQIllegalStateException("Queue: " + queueName + " filter mismatch [" + filter + "] is different than existing filter [" + result.getFilterString() + "]");`
`528`	`528`
`529`	`529`	`}`
`530`		`- return sessionSPI.getMatchingQueue(address, queueName, routingType);`
	`530`	`+ return queueName;`
`531`	`531`	`}`
`532`	`532`	`}`
`533`	`533`