This is an automated email from the ASF dual-hosted git repository. timothyjward pushed a commit to branch fix/tck in repository https://gitbox.apache.org/repos/asf/aries-typedevent.git
commit b1612aa8d890af5e6cee75e8e627641f1a1c75f1 Author: Tim Ward <[email protected]> AuthorDate: Thu Jan 15 16:34:15 2026 +0000 Fix the ordering priority of filters when configuring event history The specification defines a strict order to determine which range policy applies when multiple filters match. This is enforced by the implementation using sort ordering, which is now better tested and has been fixed Signed-off-by: Tim Ward <[email protected]> --- .../aries/typedevent/bus/impl/EventSelector.java | 60 +++++++++------ .../typedevent/bus/impl/EventSelectorTest.java | 88 ++++++++++++++++++++++ 2 files changed, 126 insertions(+), 22 deletions(-) diff --git a/org.apache.aries.typedevent.bus/src/main/java/org/apache/aries/typedevent/bus/impl/EventSelector.java b/org.apache.aries.typedevent.bus/src/main/java/org/apache/aries/typedevent/bus/impl/EventSelector.java index 3712066..cd12b76 100644 --- a/org.apache.aries.typedevent.bus/src/main/java/org/apache/aries/typedevent/bus/impl/EventSelector.java +++ b/org.apache.aries.typedevent.bus/src/main/java/org/apache/aries/typedevent/bus/impl/EventSelector.java @@ -207,46 +207,62 @@ public class EventSelector implements Comparable<EventSelector> { } @Override + /** + * A lower value indicates a more specific match (according to the algorithm + * in the specification) + */ public int compareTo(EventSelector o) { - if(isWildcard()) { - if(!o.isWildcard()) { - return 1; - } - } else if(o.isWildcard()) { - return -1; + int result; + if(isWildcard()) { + // Either both wildcards, or the non wildcard is more specific (lower) + result = o.isWildcard() ? compareWildcards(o) : 1; } else { - return initial.compareTo(o.initial); + // If o is a wildcard this is more specific (lower), + // otherwise lexical comparison of filter strings + result = o.isWildcard() ? -1 : initial.compareTo(o.initial); } + return result; + } + + private int compareWildcards(EventSelector o) { + // The longer initial number of tokens is more specific int compare = tokenCount(o.initial) - tokenCount(initial); + // This loop only executes if the initial tokens were the same length for(int i = 0; compare == 0 && i < additionalSegments.size(); i++) { if(o.additionalSegments.size() > i) { + // Both have a next (i.e. post '+') segment + // the bigger number of tokens is more specific (lower) compare = tokenCount(o.additionalSegments.get(i)) - tokenCount(additionalSegments.get(i)); } else { - // other is out of segments before we are - return 1; + // other is out of segments before this is, so this is more specific + compare = -1; } } if(compare == 0) { - if(o.additionalSegments.size() > additionalSegments.size()) { - return -1; - } - - if(isMultiLevelWildcard) { + // o has more segments overall so it is more specific + compare = 1; + } else if(isMultiLevelWildcard) { if(!o.isMultiLevelWildcard) { - return 1; + // Multi-level wildcards are the least specific + compare = 1; + } else { + // Both multi-level with the same number of segments + // fall back to lexical + compare = initial.compareTo(o.initial); } } else if(o.isMultiLevelWildcard) { - return -1; - } - - compare = initial.compareTo(o.initial); - - for(int i = 0; compare == 0 && i < additionalSegments.size(); i++) { - compare = additionalSegments.get(i).compareTo(o.additionalSegments.get(i)); + // Multi Level is least specific + compare = -1; + } else { + // Neither multilevel with the same number of segments + // use the first segment which compares in a non zero way + for(int i = 0; compare == 0 && i < additionalSegments.size(); i++) { + compare = additionalSegments.get(i).compareTo(o.additionalSegments.get(i)); + } } } diff --git a/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/impl/EventSelectorTest.java b/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/impl/EventSelectorTest.java index 21d4e0f..79e396d 100644 --- a/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/impl/EventSelectorTest.java +++ b/org.apache.aries.typedevent.bus/src/test/java/org/apache/aries/typedevent/bus/impl/EventSelectorTest.java @@ -17,12 +17,19 @@ package org.apache.aries.typedevent.bus.impl; +import static java.util.function.Function.identity; +import static java.util.stream.Collectors.toMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.stream.Stream; import org.apache.aries.typedevent.bus.impl.EventConverterTest.NestedEventHolderNotAProperDTO; @@ -125,5 +132,86 @@ public class EventSelectorTest { assertFalse(new EventSelector("foo/bar", mockFilter).matches("foo/bar", eventConverter)); Mockito.verify(eventConverter).applyFilter(mockFilter); } + + @Test + public void testOrdering() { + List<String> filters = Arrays.asList("*", "+", "event/*", "event/+/data", "event/foo/data", "event/foo/+", + "event/bar/*", "event/foo/*", "event/+", "event/+/+", "event/+/*", "bigevent/+/*", "bigevent/foo/data"); + + Map<String, EventSelector> selectors = filters.stream().collect(toMap(identity(), f -> new EventSelector(f, null))); + + // Work from the highest sort (lowest priority) to the beginning + doTest(selectors, "*", Arrays.asList("+", "event/*", "event/+/data", "event/foo/data", "event/foo/+", + "event/bar/*", "event/foo/*", "event/+", "event/+/+", "event/+/*", "bigevent/+/*", "bigevent/foo/data"), + Collections.emptyList()); + doTest(selectors, "+", Arrays.asList("event/*", "event/+/data", "event/foo/data", "event/foo/+", + "event/bar/*", "event/foo/*", "event/+", "event/+/+", "event/+/*", "bigevent/+/*", "bigevent/foo/data"), + Arrays.asList("*")); + doTest(selectors, "event/*", Arrays.asList("event/+/data", "event/foo/data", "event/foo/+", + "event/bar/*", "event/foo/*", "event/+", "event/+/+", "event/+/*", "bigevent/+/*", "bigevent/foo/data"), + Arrays.asList("*", "+")); + doTest(selectors, "event/+/*", Arrays.asList("event/+/data", "event/foo/data", "event/foo/+", + "event/bar/*", "event/foo/*", "event/+", "event/+/+", "bigevent/+/*", "bigevent/foo/data"), + Arrays.asList("*", "+", "event/*")); + doTest(selectors, "bigevent/+/*", Arrays.asList("event/+/data", "event/foo/data", "event/foo/+", + "event/bar/*", "event/foo/*", "event/+", "event/+/+", "bigevent/foo/data"), + Arrays.asList("*", "+", "event/*", "event/+/*")); + doTest(selectors, "event/+/+", Arrays.asList("event/+/data", "event/+", "event/foo/data", "event/foo/+", + "event/bar/*", "event/foo/*", "bigevent/foo/data"), + Arrays.asList("*", "+", "event/*", "event/+/*", "bigevent/+/*")); + doTest(selectors, "event/+/data", Arrays.asList("event/+", "event/foo/data", "event/foo/+", + "event/bar/*", "event/foo/*", "bigevent/foo/data"), + Arrays.asList("*", "+", "event/*", "event/+/*", "event/+/+", "bigevent/+/*")); + doTest(selectors, "event/+", Arrays.asList("event/foo/data", "event/foo/+", + "event/bar/*", "event/foo/*", "bigevent/foo/data"), + Arrays.asList("*", "+", "event/*", "event/+/*", "event/+/+", "bigevent/+/*", "event/+/data")); + doTest(selectors, "event/foo/*", Arrays.asList("event/foo/data", "event/foo/+", "event/bar/*", "bigevent/foo/data"), + Arrays.asList("*", "+", "event/*", "event/+/*", "event/+/+", "bigevent/+/*", "event/+/data", + "event/+")); + doTest(selectors, "event/bar/*", Arrays.asList("event/foo/data", "event/foo/+", "bigevent/foo/data"), + Arrays.asList("*", "+", "event/*", "event/+/*", "event/+/+", "bigevent/+/*", "event/+/data", + "event/+", "event/foo/*")); + doTest(selectors, "event/foo/+", Arrays.asList("event/foo/data", "bigevent/foo/data"), + Arrays.asList("*", "+", "event/*", "event/+/*", "event/+/+", "bigevent/+/*", "event/+/data", + "event/+", "event/foo/*", "event/bar/*")); + doTest(selectors, "event/foo/data", Arrays.asList("bigevent/foo/data"), + Arrays.asList("*", "+", "event/*", "event/+/*", "event/+/+", "bigevent/+/*", "event/+/data", + "event/+", "event/foo/*", "event/bar/*", "event/foo/+")); + doTest(selectors, "bigevent/foo/data", Collections.emptyList(), + Arrays.asList("*", "+", "event/*", "event/+/*", "event/+/+", "bigevent/+/*", "event/+/data", + "event/+", "event/foo/*", "event/bar/*", "event/foo/+", "event/foo/data")); + } + + private void doTest(Map<String, EventSelector> selectors, String toTest, List<String> lower, List<String> higher) { + EventSelector es = new EventSelector(toTest, null); + EventSelector check = selectors.get(toTest); + assertEquals(0, es.compareTo(check)); + assertEquals(0, es.compareTo(check)); + + Set<String> tested = new HashSet<>(); + doCheck(es, check, 0, 0); + tested.add(toTest); + + for(String s : lower) { + doCheck(es, selectors.get(s), 1, -1); + tested.add(s); + } + for(String s : higher) { + doCheck(es, selectors.get(s), -1, 1); + tested.add(s); + } + + assertEquals(selectors.keySet(), tested); + } + private void doCheck(EventSelector a, EventSelector b, int ab, int ba) { + int aToB = a.compareTo(b); + int bToA = b.compareTo(a); + // Scale + aToB = aToB == 0 ? 0 : aToB / Math.abs(aToB); + bToA = bToA == 0 ? 0 : bToA / Math.abs(bToA); + + assertEquals(ab, aToB, "Error comparing " + a.getTopicFilter() + " with " + b.getTopicFilter()); + assertEquals(ba, bToA, "Error comparing " + b.getTopicFilter() + " with " + a.getTopicFilter()); + } }
