From 5b308ef147a21278fffa6b907495f9c06af592cb Mon Sep 17 00:00:00 2001 From: guqing Date: Mon, 8 Jan 2024 17:44:54 +0800 Subject: [PATCH] refactor: supports combining custom matchers for field selector --- .../extension/DefaultExtensionMatcher.java | 9 +- .../run/halo/app/extension/ListOptions.java | 2 +- .../router/selector/AndSelectorMatcher.java | 18 +++ .../router/selector/AnySelectorMatcher.java | 9 ++ .../router/selector/FieldSelector.java | 112 ++++++++++++++---- .../router/selector/LogicalMatcher.java | 9 ++ .../router/selector/OrSelectorMatcher.java | 24 ++++ .../router/selector/SelectorUtil.java | 9 +- .../router/selector/FieldSelectorTest.java | 104 ++++++++++++++++ .../index/IndexedQueryEngineImpl.java | 65 ++++++---- 10 files changed, 308 insertions(+), 53 deletions(-) create mode 100644 api/src/main/java/run/halo/app/extension/router/selector/AndSelectorMatcher.java create mode 100644 api/src/main/java/run/halo/app/extension/router/selector/AnySelectorMatcher.java create mode 100644 api/src/main/java/run/halo/app/extension/router/selector/LogicalMatcher.java create mode 100644 api/src/main/java/run/halo/app/extension/router/selector/OrSelectorMatcher.java create mode 100644 api/src/test/java/run/halo/app/extension/router/selector/FieldSelectorTest.java diff --git a/api/src/main/java/run/halo/app/extension/DefaultExtensionMatcher.java b/api/src/main/java/run/halo/app/extension/DefaultExtensionMatcher.java index 25b32d07fa..9ca69c2c40 100644 --- a/api/src/main/java/run/halo/app/extension/DefaultExtensionMatcher.java +++ b/api/src/main/java/run/halo/app/extension/DefaultExtensionMatcher.java @@ -41,13 +41,8 @@ public boolean match(Extension extension) { } if (fieldSelector != null) { - for (var matcher : fieldSelector.getMatchers()) { - var fieldValue = PARSER.parseRaw(matcher.getKey()) - .getValue(extension, String.class); - if (!matcher.test(fieldValue)) { - return false; - } - } + return fieldSelector.test(key -> PARSER.parseRaw(key) + .getValue(extension, String.class)); } return true; } diff --git a/api/src/main/java/run/halo/app/extension/ListOptions.java b/api/src/main/java/run/halo/app/extension/ListOptions.java index ca599ecbf1..efb0a286f2 100644 --- a/api/src/main/java/run/halo/app/extension/ListOptions.java +++ b/api/src/main/java/run/halo/app/extension/ListOptions.java @@ -10,4 +10,4 @@ public class ListOptions { private LabelSelector labelSelector; private FieldSelector fieldSelector; -} +} \ No newline at end of file diff --git a/api/src/main/java/run/halo/app/extension/router/selector/AndSelectorMatcher.java b/api/src/main/java/run/halo/app/extension/router/selector/AndSelectorMatcher.java new file mode 100644 index 0000000000..c12090b12d --- /dev/null +++ b/api/src/main/java/run/halo/app/extension/router/selector/AndSelectorMatcher.java @@ -0,0 +1,18 @@ +package run.halo.app.extension.router.selector; + +import lombok.Getter; + +@Getter +public class AndSelectorMatcher extends LogicalMatcher { + private final SelectorMatcher left; + private final SelectorMatcher right; + + public AndSelectorMatcher(SelectorMatcher left, SelectorMatcher right) { + this.left = left; + this.right = right; + } + + public boolean test(String s) { + return left.test(s) && right.test(s); + } +} diff --git a/api/src/main/java/run/halo/app/extension/router/selector/AnySelectorMatcher.java b/api/src/main/java/run/halo/app/extension/router/selector/AnySelectorMatcher.java new file mode 100644 index 0000000000..06b1094a86 --- /dev/null +++ b/api/src/main/java/run/halo/app/extension/router/selector/AnySelectorMatcher.java @@ -0,0 +1,9 @@ +package run.halo.app.extension.router.selector; + +public class AnySelectorMatcher extends LogicalMatcher { + + @Override + public boolean test(String s) { + return true; + } +} diff --git a/api/src/main/java/run/halo/app/extension/router/selector/FieldSelector.java b/api/src/main/java/run/halo/app/extension/router/selector/FieldSelector.java index 4f3990bf9f..b82e6f5299 100644 --- a/api/src/main/java/run/halo/app/extension/router/selector/FieldSelector.java +++ b/api/src/main/java/run/halo/app/extension/router/selector/FieldSelector.java @@ -1,49 +1,117 @@ package run.halo.app.extension.router.selector; -import java.util.ArrayList; -import java.util.List; -import java.util.function.Predicate; +import java.util.Objects; +import java.util.function.UnaryOperator; import lombok.Data; import lombok.experimental.Accessors; +import org.springframework.util.Assert; @Data @Accessors(chain = true) -public class FieldSelector implements Predicate { - private List matchers; +public class FieldSelector { + private SelectorMatcher matcher; - @Override - public boolean test(String fieldValue) { - if (matchers == null || matchers.isEmpty()) { - return true; - } - return matchers.stream() - .allMatch(matcher -> matcher.test(fieldValue)); + public static FieldSelectorBuilder builder() { + return new FieldSelectorBuilder(null); } - public static FieldSelectorBuilder builder() { - return new FieldSelectorBuilder(); + public static FieldSelectorBuilder builder(SelectorMatcher rootMatcher) { + return new FieldSelectorBuilder(rootMatcher); + } + + public boolean test(UnaryOperator valueForKeyFunc) { + return evaluate(matcher, valueForKeyFunc); + } + + boolean evaluate(SelectorMatcher matcher, UnaryOperator valueForKeyFunc) { + if (matcher instanceof LogicalMatcher) { + if (matcher instanceof AndSelectorMatcher andNode) { + return evaluate(andNode.getLeft(), valueForKeyFunc) + && evaluate(andNode.getRight(), valueForKeyFunc); + } else if (matcher instanceof OrSelectorMatcher orNode) { + return evaluate(orNode.getLeft(), valueForKeyFunc) + || evaluate(orNode.getRight(), valueForKeyFunc); + } else if (matcher instanceof AnySelectorMatcher) { + return true; + } + } + String valueToTest = valueForKeyFunc.apply(matcher.getKey()); + return matcher.test(valueToTest); } public static class FieldSelectorBuilder { - private final List matchers = new ArrayList<>(); + private SelectorMatcher rootMatcher; + + public FieldSelectorBuilder(SelectorMatcher rootMatcher) { + this.rootMatcher = rootMatcher; + } + + public FieldSelectorBuilder eq(String fieldName, String fieldValue) { + return and(EqualityMatcher.equal(fieldName, fieldValue)); + } + + public FieldSelectorBuilder notEq(String fieldName, String fieldValue) { + return and(EqualityMatcher.notEqual(fieldName, fieldValue)); + } - public FieldSelectorBuilder eq(String fieldPath, String value) { - matchers.add(EqualityMatcher.equal(fieldPath, value)); + public FieldSelectorBuilder in(String fieldName, String... fieldValues) { + return and(SetMatcher.in(fieldName, fieldValues)); + } + + public FieldSelectorBuilder notIn(String fieldName, String... fieldValues) { + return and(SetMatcher.notIn(fieldName, fieldValues)); + } + + public FieldSelectorBuilder exists(String fieldName) { + return and(SetMatcher.exists(fieldName)); + } + + public FieldSelectorBuilder notExists(String fieldName) { + return and(SetMatcher.notExists(fieldName)); + } + + /** + * Combine the current selector matcher with another one with AND. + * + * @param other another selector matcher to be combined with the current one with AND + * @return the current selector matcher builder + */ + public FieldSelectorBuilder and(SelectorMatcher other) { + Assert.notNull(other, "Other selector matcher must not be null"); + if (rootMatcher == null) { + this.rootMatcher = other; + return this; + } + this.rootMatcher = new AndSelectorMatcher(rootMatcher, other); return this; } - public FieldSelectorBuilder notEq(String fieldPath, String value) { - matchers.add(EqualityMatcher.notEqual(fieldPath, value)); + /** + * Combine the current selector matcher with another one with OR. + * + * @param other another selector matcher to be combined with the current one with OR + * @return the current selector matcher builder + */ + public FieldSelectorBuilder or(SelectorMatcher other) { + Assert.notNull(other, "Other selector matcher must not be null"); + if (rootMatcher == null) { + rootMatcher = other; + } + rootMatcher = new OrSelectorMatcher(rootMatcher, other); return this; } /** - * Build a field selector. + * Build the selector matcher. */ public FieldSelector build() { - FieldSelector fieldSelector = new FieldSelector(); - fieldSelector.setMatchers(matchers); + var fieldSelector = new FieldSelector(); + fieldSelector.setMatcher(buildMatcher()); return fieldSelector; } + + public SelectorMatcher buildMatcher() { + return Objects.requireNonNullElseGet(rootMatcher, AnySelectorMatcher::new); + } } } diff --git a/api/src/main/java/run/halo/app/extension/router/selector/LogicalMatcher.java b/api/src/main/java/run/halo/app/extension/router/selector/LogicalMatcher.java new file mode 100644 index 0000000000..62ee67419d --- /dev/null +++ b/api/src/main/java/run/halo/app/extension/router/selector/LogicalMatcher.java @@ -0,0 +1,9 @@ +package run.halo.app.extension.router.selector; + +public abstract class LogicalMatcher implements SelectorMatcher { + + @Override + public String getKey() { + throw new UnsupportedOperationException(); + } +} diff --git a/api/src/main/java/run/halo/app/extension/router/selector/OrSelectorMatcher.java b/api/src/main/java/run/halo/app/extension/router/selector/OrSelectorMatcher.java new file mode 100644 index 0000000000..0fd41bb56a --- /dev/null +++ b/api/src/main/java/run/halo/app/extension/router/selector/OrSelectorMatcher.java @@ -0,0 +1,24 @@ +package run.halo.app.extension.router.selector; + +import lombok.Getter; + +@Getter +public class OrSelectorMatcher extends LogicalMatcher { + private final SelectorMatcher left; + private final SelectorMatcher right; + + public OrSelectorMatcher(SelectorMatcher left, SelectorMatcher right) { + this.left = left; + this.right = right; + } + + @Override + public String getKey() { + return null; + } + + @Override + public boolean test(String s) { + return left.test(s) || right.test(s); + } +} diff --git a/api/src/main/java/run/halo/app/extension/router/selector/SelectorUtil.java b/api/src/main/java/run/halo/app/extension/router/selector/SelectorUtil.java index f090faf7f5..f1ba20106f 100644 --- a/api/src/main/java/run/halo/app/extension/router/selector/SelectorUtil.java +++ b/api/src/main/java/run/halo/app/extension/router/selector/SelectorUtil.java @@ -88,11 +88,14 @@ public static ListOptions labelAndFieldSelectorToListOptions( .map(selectorConverter::convert) .filter(Objects::nonNull) .map(fieldConverter::convert) - .toList()) - .orElse(List.of()); + .toList() + ) + .orElse(List.of()) + .stream() + .reduce(AndSelectorMatcher::new); return new ListOptions() .setLabelSelector(new LabelSelector().setMatchers(labelMatchers)) - .setFieldSelector(new FieldSelector().setMatchers(fieldMatchers)); + .setFieldSelector(new FieldSelector().setMatcher(fieldMatchers.orElse(null))); } } diff --git a/api/src/test/java/run/halo/app/extension/router/selector/FieldSelectorTest.java b/api/src/test/java/run/halo/app/extension/router/selector/FieldSelectorTest.java new file mode 100644 index 0000000000..2f209fcecb --- /dev/null +++ b/api/src/test/java/run/halo/app/extension/router/selector/FieldSelectorTest.java @@ -0,0 +1,104 @@ +package run.halo.app.extension.router.selector; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** + * Tests for {@link FieldSelector}. + * + * @author guqing + * @since 2.12.0 + */ +class FieldSelectorTest { + + @Test + void testEq() { + var fieldSelector = FieldSelector.builder() + .eq("name", "guqing").build(); + assertThat(fieldSelector.test(key -> "guqing")).isTrue(); + assertThat(fieldSelector.test(key -> "halo")).isFalse(); + } + + @Test + void testNotEq() { + var fieldSelector = FieldSelector.builder() + .notEq("name", "guqing").build(); + assertThat(fieldSelector.test(key -> "guqing")).isFalse(); + assertThat(fieldSelector.test(key -> "halo")).isTrue(); + } + + @Test + void testIn() { + var fieldSelector = FieldSelector.builder() + .in("name", "guqing", "guqing1").build(); + assertThat(fieldSelector.test(key -> "guqing")).isTrue(); + assertThat(fieldSelector.test(key -> "halo")).isFalse(); + assertThat(fieldSelector.test(key -> "blog")).isFalse(); + } + + @Test + void testNotIn() { + var fieldSelector = FieldSelector.builder() + .notIn("name", "guqing", "guqing1").build(); + assertThat(fieldSelector.test(key -> "guqing")).isFalse(); + assertThat(fieldSelector.test(key -> "halo")).isTrue(); + assertThat(fieldSelector.test(key -> "blog")).isTrue(); + } + + @Test + void testExists() { + var fieldSelector = FieldSelector.builder() + .exists("name").build(); + assertThat(fieldSelector.test(key -> "guqing")).isTrue(); + assertThat(fieldSelector.test(key -> "halo")).isTrue(); + assertThat(fieldSelector.test(key -> "blog")).isTrue(); + } + + @Test + void testNotExists() { + var fieldSelector = FieldSelector.builder() + .notExists("name").build(); + assertThat(fieldSelector.test(key -> "guqing")).isFalse(); + assertThat(fieldSelector.test(key -> "halo")).isFalse(); + assertThat(fieldSelector.test(key -> "blog")).isFalse(); + } + + @Test + void testAnd() { + var fieldSelector = FieldSelector.builder() + .eq("name", "guqing") + .and(FieldSelector.builder().eq("age", "18").build().getMatcher()) + .build(); + assertThat(fieldSelector.test(key -> "guqing")).isFalse(); + assertThat(fieldSelector.test(key -> "halo")).isFalse(); + assertThat(fieldSelector.test(key -> "18")).isFalse(); + assertThat(fieldSelector.test(key -> "guqing18")).isFalse(); + assertThat(fieldSelector.test(key -> { + var map = Map.of("name", "guqing", "age", "18"); + return map.get(key); + })).isTrue(); + assertThat(fieldSelector.test(key -> { + var map = Map.of("name", "guqing", "age", "19"); + return map.get(key); + })).isFalse(); + } + + @Test + void testOr() { + var fieldSelector = FieldSelector.builder() + .eq("name", "guqing") + .or(FieldSelector.builder().eq("age", "18").build().getMatcher()) + .build(); + assertThat(fieldSelector.test(key -> "guqing")).isTrue(); + assertThat(fieldSelector.test(key -> "halo")).isFalse(); + assertThat(fieldSelector.test(key -> "blog")).isFalse(); + assertThat(fieldSelector.test(key -> "18")).isTrue(); + assertThat(fieldSelector.test(key -> "guqing18")).isFalse(); + assertThat(fieldSelector.test(key -> { + var map = Map.of("name", "guqing", "age", "18"); + return map.get(key); + })).isTrue(); + } +} \ No newline at end of file diff --git a/application/src/main/java/run/halo/app/extension/index/IndexedQueryEngineImpl.java b/application/src/main/java/run/halo/app/extension/index/IndexedQueryEngineImpl.java index 9778916737..ecca1f5d88 100644 --- a/application/src/main/java/run/halo/app/extension/index/IndexedQueryEngineImpl.java +++ b/application/src/main/java/run/halo/app/extension/index/IndexedQueryEngineImpl.java @@ -22,7 +22,10 @@ import run.halo.app.extension.ListOptions; import run.halo.app.extension.ListResult; import run.halo.app.extension.PageRequest; -import run.halo.app.extension.router.selector.FieldSelector; +import run.halo.app.extension.router.selector.AndSelectorMatcher; +import run.halo.app.extension.router.selector.AnySelectorMatcher; +import run.halo.app.extension.router.selector.LogicalMatcher; +import run.halo.app.extension.router.selector.OrSelectorMatcher; import run.halo.app.extension.router.selector.SelectorMatcher; /** @@ -85,6 +88,12 @@ static List intersection(List list1, List list2) { return intersection; } + static List union(List list1, List list2) { + Set set = new LinkedHashSet<>(list1); + set.addAll(list2); + return new ArrayList<>(set); + } + static void throwNotIndexedException(String fieldPath) { throw new IllegalArgumentException( "No index found for fieldPath: " + fieldPath @@ -147,25 +156,13 @@ List doRetrieve(Indexer indexer, ListOptions options, Sort sort) { stopWatch.start("retrieve matched metadata names by fields"); Optional> matchedByFields = Optional.ofNullable(options.getFieldSelector()) - .filter(selector -> !CollectionUtils.isEmpty(selector.getMatchers())) - .map(FieldSelector::getMatchers) - .map(matchers -> matchers.stream() - .map(matcher -> { - var fieldPath = matcher.getKey(); - if (!fieldPathEntryMap.containsKey(fieldPath)) { - throwNotIndexedException(fieldPath); - } - var indexEntry = fieldPathEntryMap.get(fieldPath); - var indexedKeys = indexEntry.indexedKeys(); - return indexedKeys.stream() - .filter(matcher::test) - .map(indexEntry::getByIndexKey) - .flatMap(List::stream) - .toList(); - }) - .reduce(IndexedQueryEngineImpl::intersection) - .orElse(List.of()) - ); + .filter(fieldSelector -> fieldSelector.getMatcher() != null) + .map(fieldSelector -> { + var values = evaluate(fieldSelector.getMatcher(), fieldPathEntryMap, + allMetadataNames); + var uniqueValues = new LinkedHashSet<>(values); + return new ArrayList<>(uniqueValues); + }); stopWatch.stop(); stopWatch.start("merge result"); @@ -189,6 +186,34 @@ List doRetrieve(Indexer indexer, ListOptions options, Sort sort) { return result; } + List evaluate(SelectorMatcher matcher, Map fieldPathEntryMap, + List allNames) { + if (matcher == null) { + return allNames; + } + if (matcher instanceof LogicalMatcher) { + if (matcher instanceof AndSelectorMatcher andNode) { + var left = evaluate(andNode.getLeft(), fieldPathEntryMap, allNames); + var right = evaluate(andNode.getRight(), fieldPathEntryMap, allNames); + return intersection(left, right); + } else if (matcher instanceof OrSelectorMatcher orNode) { + var left = evaluate(orNode.getLeft(), fieldPathEntryMap, allNames); + var right = evaluate(orNode.getRight(), fieldPathEntryMap, allNames); + return union(left, right); + } else if (matcher instanceof AnySelectorMatcher) { + return allNames; + } + } + var indexEntry = getIndexEntry(matcher.getKey(), fieldPathEntryMap); + var indexedKeys = indexEntry.indexedKeys(); + return indexedKeys.stream() + .filter(matcher::test) + .map(indexEntry::getByIndexKey) + .flatMap(List::stream) + .toList(); + } + + /** * Sort the given list by the given {@link Sort}. */