From 19e5a8fd6355517311cef0fc7b8ee6944c5bcd30 Mon Sep 17 00:00:00 2001 From: brharrington Date: Tue, 29 Aug 2023 14:42:24 -0500 Subject: [PATCH] additional tests for contains and trigrams (#1078) Put in some additional test cases for extracting contained strings and trigrams from patterns. Fixes some cases with trigram extraction for repeats that can be absent from the string and still match. --- .../impl/matcher/CharSeqMatcher.java | 5 +++ .../spectator/impl/matcher/RepeatMatcher.java | 8 +++- .../spectator/impl/matcher/SeqMatcher.java | 7 +++- .../impl/matcher/ZeroOrMoreMatcher.java | 11 ++--- .../impl/matcher/ZeroOrOneMatcher.java | 11 ++--- .../matcher/AbstractPatternMatcherTest.java | 8 ++++ .../matcher/ContainsPatternMatcherTest.java | 34 ++++++++++++++++ .../spectator/impl/matcher/ContainsTest.java | 36 +++++++++++++++++ .../matcher/TrigramsPatternMatcherTest.java | 40 +++++++++++++++++++ .../spectator/impl/matcher/TrigramsTest.java | 10 ++++- 10 files changed, 156 insertions(+), 14 deletions(-) create mode 100644 spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ContainsPatternMatcherTest.java create mode 100644 spectator-api/src/test/java/com/netflix/spectator/impl/matcher/TrigramsPatternMatcherTest.java diff --git a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/CharSeqMatcher.java b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/CharSeqMatcher.java index ff8fd2385..91f32fa8a 100644 --- a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/CharSeqMatcher.java +++ b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/CharSeqMatcher.java @@ -48,6 +48,11 @@ String pattern() { return pattern; } + @Override + public String containedString() { + return pattern; + } + @Override public SortedSet trigrams() { return PatternUtils.computeTrigrams(pattern); diff --git a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/RepeatMatcher.java b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/RepeatMatcher.java index cc40b36d7..06d3e2d67 100644 --- a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/RepeatMatcher.java +++ b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/RepeatMatcher.java @@ -16,6 +16,7 @@ package com.netflix.spectator.impl.matcher; import java.io.Serializable; +import java.util.Collections; import java.util.Objects; import java.util.SortedSet; import java.util.function.Function; @@ -48,9 +49,14 @@ int max() { return max; } + @Override + public String containedString() { + return min == 0 ? null : repeated.containedString(); + } + @Override public SortedSet trigrams() { - return repeated.trigrams(); + return min == 0 ? Collections.emptySortedSet() : repeated.trigrams(); } @Override diff --git a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/SeqMatcher.java b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/SeqMatcher.java index 688838fdd..39bee622e 100644 --- a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/SeqMatcher.java +++ b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/SeqMatcher.java @@ -80,7 +80,12 @@ public String prefix() { @Override public String containedString() { - return matchers[0].containedString(); + String str = null; + for (Matcher m : matchers) { + str = m.containedString(); + if (str != null) break; + } + return str; } @Override diff --git a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/ZeroOrMoreMatcher.java b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/ZeroOrMoreMatcher.java index b4b8d45e8..6cebb1996 100644 --- a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/ZeroOrMoreMatcher.java +++ b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/ZeroOrMoreMatcher.java @@ -20,7 +20,6 @@ import java.io.Serializable; import java.util.Objects; import java.util.SortedSet; -import java.util.TreeSet; import java.util.function.Function; /** @@ -49,12 +48,14 @@ Matcher next() { return next; } + @Override + public String containedString() { + return next.containedString(); + } + @Override public SortedSet trigrams() { - SortedSet ts = new TreeSet<>(); - ts.addAll(repeated.trigrams()); - ts.addAll(next.trigrams()); - return ts; + return next.trigrams(); } @Override diff --git a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/ZeroOrOneMatcher.java b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/ZeroOrOneMatcher.java index 246d3d519..9c1f225c3 100644 --- a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/ZeroOrOneMatcher.java +++ b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/ZeroOrOneMatcher.java @@ -20,7 +20,6 @@ import java.io.Serializable; import java.util.Objects; import java.util.SortedSet; -import java.util.TreeSet; import java.util.function.Function; /** @@ -49,12 +48,14 @@ Matcher next() { return next; } + @Override + public String containedString() { + return next.containedString(); + } + @Override public SortedSet trigrams() { - SortedSet ts = new TreeSet<>(); - ts.addAll(repeated.trigrams()); - ts.addAll(next.trigrams()); - return ts; + return next.trigrams(); } @Override diff --git a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/AbstractPatternMatcherTest.java b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/AbstractPatternMatcherTest.java index 69d8fd8de..9f43d57f0 100644 --- a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/AbstractPatternMatcherTest.java +++ b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/AbstractPatternMatcherTest.java @@ -421,6 +421,14 @@ public void zeroOrMoreWithOr() { testRE("(a|b|c)*d", "ababcd"); } + @Test + public void repeatZero() { + testRE("(abc){0,3}def", "def"); + testRE("(abc){0,3}def", "abcdef"); + testRE("(abc){1,3}def", "def"); + testRE("(abc){1,3}def", "abcdef"); + } + @Test public void repeatWithOr() { testRE("(a|b|c){1,3}d", "aaad"); diff --git a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ContainsPatternMatcherTest.java b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ContainsPatternMatcherTest.java new file mode 100644 index 000000000..ff541acff --- /dev/null +++ b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ContainsPatternMatcherTest.java @@ -0,0 +1,34 @@ +/* + * Copyright 2014-2023 Netflix, Inc. + * + * Licensed 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 com.netflix.spectator.impl.matcher; + +import com.netflix.spectator.impl.PatternMatcher; +import org.junit.jupiter.api.Assertions; + +public class ContainsPatternMatcherTest extends AbstractPatternMatcherTest { + + @Override + protected void testRE(String regex, String value) { + PatternMatcher matcher = PatternMatcher.compile(regex); + String str = matcher.containedString(); + + // Contains should be more lenient than the actual pattern, so anything that is matched + // by the pattern must be a possible match with the trigrams. + if (str != null && matcher.matches(value)) { + Assertions.assertTrue(value.contains(str)); + } + } +} diff --git a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ContainsTest.java b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ContainsTest.java index d0af495f1..5b2b0b2b8 100644 --- a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ContainsTest.java +++ b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ContainsTest.java @@ -68,10 +68,46 @@ public void notContains() { public void containedString() { Assertions.assertEquals("abc", re("abc").containedString()); Assertions.assertEquals("abc", re(".*abc").containedString()); + Assertions.assertEquals("abc", re(".*abc$").containedString()); Assertions.assertEquals("ab", re(".*ab[cd]").containedString()); Assertions.assertEquals("abc", re("abc.def").containedString()); Assertions.assertEquals("abc.def", re("abc\\.def").containedString()); Assertions.assertEquals("abc", re("^abc.def").containedString()); Assertions.assertEquals("abc.def", re("^abc\\.def").containedString()); } + + @Test + public void containsZeroOrMore() { + Assertions.assertEquals("def", re("(abc)*def").containedString()); + } + + @Test + public void containsZeroOrOne() { + Assertions.assertEquals("def", re("(abc)?def").containedString()); + } + + @Test + public void containsOneOrMore() { + Assertions.assertEquals("abc", re("(abc)+def").containedString()); + } + + @Test + public void containsRepeat() { + Assertions.assertEquals("def", re("(abc){0,5}def").containedString()); + } + + @Test + public void containsRepeatAtLeastOne() { + Assertions.assertEquals("abc", re("(abc){1,5}def").containedString()); + } + + @Test + public void containsPartOfSequence() { + Assertions.assertEquals("abcd", re(".*[0-9]abcd[efg]hij").containedString()); + } + + @Test + public void containsMultiple() { + Assertions.assertEquals("abc", re("^abc.*def.*ghi").containedString()); + } } diff --git a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/TrigramsPatternMatcherTest.java b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/TrigramsPatternMatcherTest.java new file mode 100644 index 000000000..b4668b1ab --- /dev/null +++ b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/TrigramsPatternMatcherTest.java @@ -0,0 +1,40 @@ +/* + * Copyright 2014-2023 Netflix, Inc. + * + * Licensed 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 com.netflix.spectator.impl.matcher; + +import com.netflix.spectator.impl.PatternMatcher; +import org.junit.jupiter.api.Assertions; + +import java.util.SortedSet; + +public class TrigramsPatternMatcherTest extends AbstractPatternMatcherTest { + + @Override + protected void testRE(String regex, String value) { + PatternMatcher matcher = PatternMatcher.compile(regex); + SortedSet trigrams = matcher.trigrams(); + + // Trigrams should be more lenient than the actual pattern, so anything that is matched + // by the pattern must be a possible match with the trigrams. + if (matcher.matches(value)) { + Assertions.assertTrue(couldMatch(trigrams, value)); + } + } + + private boolean couldMatch(SortedSet trigrams, String value) { + return trigrams.stream().allMatch(value::contains); + } +} diff --git a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/TrigramsTest.java b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/TrigramsTest.java index 169dc87b7..8b744059c 100644 --- a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/TrigramsTest.java +++ b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/TrigramsTest.java @@ -48,13 +48,13 @@ public void endsWith() { @Test public void zeroOrMore() { PatternMatcher m = PatternMatcher.compile("(abc)*def"); - Assertions.assertEquals(sortedSet("abc", "def"), m.trigrams()); + Assertions.assertEquals(sortedSet("def"), m.trigrams()); } @Test public void zeroOrOne() { PatternMatcher m = PatternMatcher.compile("(abc)?def"); - Assertions.assertEquals(sortedSet("abc", "def"), m.trigrams()); + Assertions.assertEquals(sortedSet("def"), m.trigrams()); } @Test @@ -66,6 +66,12 @@ public void oneOrMore() { @Test public void repeat() { PatternMatcher m = PatternMatcher.compile("(abc){0,5}def"); + Assertions.assertEquals(sortedSet("def"), m.trigrams()); + } + + @Test + public void repeatAtLeastOne() { + PatternMatcher m = PatternMatcher.compile("(abc){1,5}def"); Assertions.assertEquals(sortedSet("abc", "def"), m.trigrams()); }