ARTEMIS-4488 Merge settings with wildcards after literal matches

artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java

@@ -20,14 +20,12 @@
 import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -258,10 +256,9 @@ public T getMatch(final String match) {
       }
       lock.readLock().lock();
       try {
-         T actualMatch;
-         Map<String, Match<T>> possibleMatches = getPossibleMatches(modifiedMatch);
-         Collection<Match<T>> orderedMatches = sort(possibleMatches);
-         actualMatch = merge(orderedMatches);
+         List<Match<T>> matches =
+            getMatches(modifiedMatch);
+         T actualMatch = merge(matches);
          T value = actualMatch != null ? actualMatch : defaultmatch;
          if (value != null) {
             cache.put(modifiedMatch, value);
@@ -299,32 +296,12 @@ private T merge(final Collection<Match<T>> orderedMatches) {
          while (matchIterator.hasNext() && Mergeable.class.isAssignableFrom(result.getClass())) {
             match = matchIterator.next();
             result = ((Mergeable<T>)result).mergeCopy(match.getValue());
-            if (match.isLiteral()) {
-               break;
-            }
          }
       }
 
       return result;
    }
 
-   /**
-    * Sort the matches according to their precedence (that is, according to the precedence of their
-    * keys).
-    *
-    * @param possibleMatches
-    * @return
-    */
-   private List<Match<T>> sort(final Map<String, Match<T>> possibleMatches) {
-      List<String> keys = new ArrayList<>(possibleMatches.keySet());
-      Collections.sort(keys, matchComparator);
-      List<Match<T>> matches1 = new ArrayList<>(possibleMatches.size());
-      for (String key : keys) {
-         matches1.add(possibleMatches.get(key));
-      }
-      return matches1;
-   }
-
    /**
     * remove a match from the repository
     *
@@ -463,30 +440,30 @@ private void onChange() {
    }
 
    /**
-    * return any possible matches
+    * return matches
     *
     * @param match
     * @return
     */
-   private Map<String, Match<T>> getPossibleMatches(final String match) {
-      HashMap<String, Match<T>> possibleMatches = new HashMap<>();
+   private List<Match<T>> getMatches(final String match) {
+      List<Match<T>> matches = new ArrayList<>();
 
-      if (exactMatches.containsKey(match)) {
-         possibleMatches.put(match, exactMatches.get(match));
+      Match exactMatch = exactMatches.get(match);
+      if (exactMatch != null) {
+         matches.add(exactMatch);
       }
 
-      for (Entry<String, Match<T>> entry : wildcardMatches.entrySet()) {
-         Match<T> entryMatch = entry.getValue();
-         if (entryMatch.getPattern().matcher(match).matches()) {
-            possibleMatches.put(entry.getKey(), entryMatch);
-         }
+      Match literalMatch = literalMatches.get(match);
+      if (literalMatch != null) {
+         matches.add(literalMatch);
       }
 
-      if (literalMatches.containsKey(match)) {
-         possibleMatches.put(match, literalMatches.get(match));
-      }
+      wildcardMatches.values().stream().
+         filter(m -> m.getPattern().matcher(match).matches()).
+         sorted((m1, m2) -> matchComparator.compare(m1.getMatch(), m2.getMatch())).
+         forEach(m -> matches.add(m));
 
-      return possibleMatches;
+      return matches;
    }
 
    /**

artemis-server/src/test/java/org/apache/activemq/artemis/core/settings/RepositoryTest.java

@@ -80,17 +80,21 @@ public void testLiteral() {
       repo.addMatch("(a.#)", new DummyMergeable(1));
       repo.addMatch("a.#", new DummyMergeable(2));
       repo.addMatch("a.b", new DummyMergeable(3));
+      repo.addMatch("a.*", new DummyMergeable(4));
 
       DummyMergeable abDummyMatch = repo.getMatch("a.b");
-      Assert.assertEquals(2, abDummyMatch.getMergedItems().size());
+      Assert.assertEquals(3, abDummyMatch.getMergedItems().size());
       Assert.assertEquals(3, abDummyMatch.getId());
-      Assert.assertEquals(2, abDummyMatch.getMergedItems().get(0).getId());
-      Assert.assertEquals(0, abDummyMatch.getMergedItems().get(1).getId());
+      Assert.assertEquals(4, abDummyMatch.getMergedItems().get(0).getId());
+      Assert.assertEquals(2, abDummyMatch.getMergedItems().get(1).getId());
+      Assert.assertEquals(0, abDummyMatch.getMergedItems().get(2).getId());
 
       DummyMergeable aDummyMatch = repo.getMatch("a.#");
-      Assert.assertEquals(1, aDummyMatch.getMergedItems().size());
+      Assert.assertEquals(3, aDummyMatch.getMergedItems().size());
       Assert.assertEquals(1, aDummyMatch.getId());
-      Assert.assertEquals(0, aDummyMatch.getMergedItems().get(0).getId());
+      Assert.assertEquals(4, aDummyMatch.getMergedItems().get(0).getId());
+      Assert.assertEquals(2, aDummyMatch.getMergedItems().get(1).getId());
+      Assert.assertEquals(0, aDummyMatch.getMergedItems().get(2).getId());
    }
 
    @Test

docs/user-manual/address-settings.adoc

@@ -13,7 +13,7 @@ A match on the any-words delimiter (`#` by default) is considered less specific
 A match with a single word delimiter (`*` by default) is considered less specific than a match on an exact queue name.
 In this way settings can be "layered" so that configuration details don't need to be repeated.
 
-Address setting matches can also be "literal" which can be used to interrupt the hierarchy in useful ways.
+Address setting matches can also be "literal" which can be used to match wildcards literally, for further details see <<literal-matches,literal matches>>.
 
 The meaning of the specific settings are explained fully throughout the user manual, however here is a brief description with a link to the appropriate chapter if available.
 

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/AddressSettingsTest.java

@@ -117,22 +117,29 @@ public void testSimpleHierarchyWithDLA() throws Exception {
 
    @Test
    public void testLiteralMatch() throws Exception {
-      final SimpleString literal = RandomUtil.randomSimpleString();
-      final SimpleString nonLiteral = RandomUtil.randomSimpleString();
+      final SimpleString defaultDLA = RandomUtil.randomSimpleString();
+      final SimpleString defaultEA = RandomUtil.randomSimpleString();
+      final SimpleString fooDefaultDLA = RandomUtil.randomSimpleString();
+      final SimpleString fooChildrenDLA = RandomUtil.randomSimpleString();
+      final SimpleString fooLiteralDLA = RandomUtil.randomSimpleString();
 
       Configuration configuration = createDefaultConfig(false);
       configuration.setLiteralMatchMarkers("()");
       ActiveMQServer server = createServer(false, configuration);
       server.start();
       HierarchicalRepository<AddressSettings> repo = server.getAddressSettingsRepository();
-      repo.addMatch("(foo.#)", new AddressSettings().setDeadLetterAddress(literal));
-      repo.addMatch("foo.#", new AddressSettings().setDeadLetterAddress(nonLiteral));
+      repo.addMatch("#", new AddressSettings().setDeadLetterAddress(defaultDLA).setExpiryAddress(defaultEA));
+      repo.addMatch("foo.#", new AddressSettings().setDeadLetterAddress(fooDefaultDLA));
+      repo.addMatch("foo.*", new AddressSettings().setDeadLetterAddress(fooChildrenDLA));
+      repo.addMatch("(foo.#)", new AddressSettings().setDeadLetterAddress(fooLiteralDLA));
 
       // should be the DLA from foo.# - the literal match
-      Assert.assertEquals(literal, repo.getMatch("foo.#").getDeadLetterAddress());
-
-      Assert.assertEquals(nonLiteral, repo.getMatch("foo.bar").getDeadLetterAddress());
+      Assert.assertEquals(fooLiteralDLA, repo.getMatch("foo.#").getDeadLetterAddress());
+      Assert.assertEquals(defaultEA, repo.getMatch("foo.#").getExpiryAddress());
 
+      Assert.assertEquals(fooChildrenDLA, repo.getMatch("foo.bar").getDeadLetterAddress());
+      Assert.assertEquals(fooDefaultDLA, repo.getMatch("foo.bar.too").getDeadLetterAddress());
+      Assert.assertEquals(defaultDLA, repo.getMatch("too.#").getDeadLetterAddress());
    }
 
    @Test