diff --git a/planetiler-basemap/src/main/java/com/onthegomap/planetiler/basemap/BasemapProfile.java b/planetiler-basemap/src/main/java/com/onthegomap/planetiler/basemap/BasemapProfile.java index 7d4c4a4975..2e976a8799 100644 --- a/planetiler-basemap/src/main/java/com/onthegomap/planetiler/basemap/BasemapProfile.java +++ b/planetiler-basemap/src/main/java/com/onthegomap/planetiler/basemap/BasemapProfile.java @@ -116,7 +116,7 @@ public BasemapProfile(Translations translations, PlanetilerConfig config, Stats }) .toList(); return new RowDispatch(constructor.create(), handlers); - }).simplify().index(); + }).simplify().indexAndWarn(); wikidataMappings = Tables.MAPPINGS .mapResults(constructor -> handlerMap.getOrDefault(constructor.rowClass(), List.of()).stream() .anyMatch(handler -> !IgnoreWikidata.class.isAssignableFrom(handler.handlerClass())) diff --git a/planetiler-benchmarks/src/main/java/com/onthegomap/planetiler/benchmarks/BasemapMapping.java b/planetiler-benchmarks/src/main/java/com/onthegomap/planetiler/benchmarks/BasemapMapping.java index 02e3b8cf92..e969dbf276 100644 --- a/planetiler-benchmarks/src/main/java/com/onthegomap/planetiler/benchmarks/BasemapMapping.java +++ b/planetiler-benchmarks/src/main/java/com/onthegomap/planetiler/benchmarks/BasemapMapping.java @@ -33,7 +33,7 @@ public static void main(String[] args) { try (var reader = OsmInputFile.readFrom(Path.of("data", "sources", "massachusetts.osm.pbf"))) { reader.forEachBlock(block -> { for (var element : block.decodeElements()) { - if (random.nextDouble() < 0.2) { + if (random.nextDouble() < 0.9) { if (inputs.size() % 1_000_000 == 0) { logger.log(); } diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/Expression.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/Expression.java index b75bfd7ef5..082afb670f 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/Expression.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/Expression.java @@ -13,6 +13,8 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A framework for defining and manipulating boolean expressions that match on input element. @@ -27,6 +29,7 @@ * */ public interface Expression { + Logger LOGGER = LoggerFactory.getLogger(Expression.class); String LINESTRING_TYPE = "linestring"; String POINT_TYPE = "point"; @@ -132,9 +135,12 @@ private static Expression simplify(Expression initial) { seen.add(simplified); while (true) { simplified = simplifyOnce(simplified); - if (seen.contains(simplified) || seen.size() > 100) { + if (seen.contains(simplified)) { return simplified; } + if (seen.size() > 1000) { + throw new IllegalStateException("Infinite loop while simplifying expression " + initial); + } seen.add(simplified); } } @@ -151,6 +157,8 @@ private static Expression simplifyOnce(Expression expression) { return FALSE; } else if (not.child == FALSE) { return TRUE; + } else if (not.child instanceof MatchAny any && any.values.equals(List.of(""))) { + return matchField(any.field); } return not; } else if (expression instanceof Or or) { @@ -166,11 +174,11 @@ private static Expression simplifyOnce(Expression expression) { return or(or.children.stream() // hoist children .flatMap(child -> child instanceof Or childOr ? childOr.children.stream() : Stream.of(child)) - .filter(child -> child != FALSE) + .filter(child -> child != FALSE) // or() == or(FALSE) == or(FALSE, FALSE) == FALSE, so safe to remove all here .map(Expression::simplifyOnce).toList()); } else if (expression instanceof And and) { if (and.children.isEmpty()) { - return FALSE; + return TRUE; } if (and.children.size() == 1) { return simplifyOnce(and.children.get(0)); @@ -181,7 +189,7 @@ private static Expression simplifyOnce(Expression expression) { return and(and.children.stream() // hoist children .flatMap(child -> child instanceof And childAnd ? childAnd.children.stream() : Stream.of(child)) - .filter(child -> child != TRUE) + .filter(child -> child != TRUE) // and() == and(TRUE) == and(TRUE, TRUE) == TRUE, so safe to remove all here .map(Expression::simplifyOnce).toList()); } else { return expression; @@ -283,12 +291,7 @@ public String generateJavaCode() { @Override public boolean evaluate(WithTags input, List matchKeys) { - int size = children.size(); - // Optimization: this method consumes the most time when matching against input elements, and - // iterating through this list by index is slightly faster than an enhanced for loop - // noinspection ForLoopReplaceableByForEach - for intellij - for (int i = 0; i < size; i++) { - Expression child = children.get(i); + for (Expression child : children) { if (child.evaluate(input, matchKeys)) { return true; } diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/MultiExpression.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/MultiExpression.java index 87a438e37b..e39715dae1 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/MultiExpression.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/expression/MultiExpression.java @@ -19,6 +19,8 @@ import java.util.function.Predicate; import java.util.function.UnaryOperator; import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A list of {@link Expression Expressions} to evaluate on input elements. @@ -33,6 +35,7 @@ */ public record MultiExpression (List> expressions) { + private static final Logger LOGGER = LoggerFactory.getLogger(MultiExpression.class); private static final Comparator BY_ID = Comparator.comparingInt(WithId::id); public static MultiExpression of(List> expressions) { @@ -62,45 +65,64 @@ private static void visitExpressions(SourceFeature input, List> res } } + /** + * Returns true if {@code expression} only contains "not filter" so we can't limit evaluating this expression to only + * when a particular key is present on the input. + */ + private static boolean mustAlwaysEvaluate(Expression expression) { + if (expression instanceof Expression.Or or) { + return or.children().stream().anyMatch(MultiExpression::mustAlwaysEvaluate); + } else if (expression instanceof Expression.And and) { + return and.children().stream().allMatch(MultiExpression::mustAlwaysEvaluate); + } else if (expression instanceof Expression.Not not) { + return !mustAlwaysEvaluate(not.child()); + } else if (expression instanceof Expression.MatchAny any && any.matchWhenMissing()) { + return true; + } else { + return TRUE.equals(expression); + } + } + /** Calls {@code acceptKey} for every tag that could possibly cause {@code exp} to match an input element. */ private static void getRelevantKeys(Expression exp, Consumer acceptKey) { - if (exp instanceof Expression.And and) { - and.children().forEach(child -> getRelevantKeys(child, acceptKey)); - } else if (exp instanceof Expression.Or or) { - or.children().forEach(child -> getRelevantKeys(child, acceptKey)); - } else if (exp instanceof Expression.Not not) { - getRelevantMissingKeys(not.child(), acceptKey); - } else if (exp instanceof Expression.MatchField field) { - acceptKey.accept(field.field()); - } else if (exp instanceof Expression.MatchAny any) { - acceptKey.accept(any.field()); + // if a sub-expression must always be evaluated, then either the whole expression must always be evaluated + // or there is another part of the expression that limits the elements on which it must be evaluated, so we can + // ignore keys from this sub-expression. + if (!mustAlwaysEvaluate(exp)) { + if (exp instanceof Expression.And and) { + and.children().forEach(child -> getRelevantKeys(child, acceptKey)); + } else if (exp instanceof Expression.Or or) { + or.children().forEach(child -> getRelevantKeys(child, acceptKey)); + } else if (exp instanceof Expression.MatchField field) { + acceptKey.accept(field.field()); + } else if (exp instanceof Expression.MatchAny any && !any.matchWhenMissing()) { + acceptKey.accept(any.field()); + } + // ignore not case since not(matchAny("field", "")) should track "field" as a relevant key, but that gets + // simplified to matchField("field") so don't need to handle that here } } + /** Returns an optimized index for matching {@link #expressions()} against each input element. */ + public Index index() { + return index(false); + } + /** - * Calls {@code acceptKey} for every tag that, when missing, could possibly cause {@code exp} to match an input - * element. + * Same as {@link #index()} but logs a warning when there are degenerate expressions that must be evaluated on every + * input. */ - private static void getRelevantMissingKeys(Expression exp, Consumer acceptKey) { - if (exp instanceof Expression.And and) { - and.children().forEach(child -> getRelevantMissingKeys(child, acceptKey)); - } else if (exp instanceof Expression.Or or) { - or.children().forEach(child -> getRelevantMissingKeys(child, acceptKey)); - } else if (exp instanceof Expression.Not not) { - getRelevantKeys(not.child(), acceptKey); - } else if (exp instanceof Expression.MatchAny any && any.matchWhenMissing()) { - acceptKey.accept(any.field()); - } + public Index indexAndWarn() { + return index(true); } - /** Returns an optimized index for matching {@link #expressions()} against each input element. */ - public Index index() { + private Index index(boolean warn) { if (expressions.isEmpty()) { return new EmptyIndex<>(); } boolean caresAboutGeometryType = expressions.stream().anyMatch(entry -> entry.expression.contains(exp -> exp instanceof Expression.MatchType)); - return caresAboutGeometryType ? new GeometryTypeIndex<>(this) : new KeyIndex<>(simplify()); + return caresAboutGeometryType ? new GeometryTypeIndex<>(this, warn) : new KeyIndex<>(simplify(), warn); } /** Returns a copy of this multi-expression that replaces every expression using {@code mapper}. */ @@ -220,38 +242,38 @@ private static class KeyIndex implements Index { private final Map>> keyToExpressionsMap; // same as keyToExpressionsMap but as a list (optimized for iteration when # source feature keys > # tags we care about) private final List>>> keyToExpressionsList; - // expressions that should match when certain tags are *not* present on an input element - private final List>>> missingKeyToExpressionList; - // expressions that match a constant true input element - private final List> constantTrueExpressionList; + // expressions that must always be evaluated on each input element + private final List> alwaysEvaluateExpressionList; - private KeyIndex(MultiExpression expressions) { + private KeyIndex(MultiExpression expressions, boolean warn) { int id = 1; // build the indexes Map>> keyToExpressions = new HashMap<>(); - Map>> missingKeyToExpressions = new HashMap<>(); - List> constants = new ArrayList<>(); + List> always = new ArrayList<>(); for (var entry : expressions.expressions) { Expression expression = entry.expression; EntryWithId expressionValue = new EntryWithId<>(entry.result, expression, id++); - getRelevantKeys(expression, - key -> keyToExpressions.computeIfAbsent(key, k -> new HashSet<>()).add(expressionValue)); - getRelevantMissingKeys(expression, - key -> missingKeyToExpressions.computeIfAbsent(key, k -> new HashSet<>()).add(expressionValue)); - if (expression.equals(TRUE)) { - constants.add(expressionValue); + if (mustAlwaysEvaluate(expression)) { + always.add(expressionValue); + } else { + getRelevantKeys(expression, + key -> keyToExpressions.computeIfAbsent(key, k -> new HashSet<>()).add(expressionValue)); } } // create immutable copies for fast iteration at matching time - constantTrueExpressionList = List.copyOf(constants); + if (warn && !always.isEmpty()) { + LOGGER.warn("{} expressions will be evaluated for every element:", always.size()); + for (var expression : always) { + LOGGER.warn(" {}: {}", expression.result, expression.expression); + } + } + alwaysEvaluateExpressionList = List.copyOf(always); keyToExpressionsMap = keyToExpressions.entrySet().stream().collect(Collectors.toUnmodifiableMap( Map.Entry::getKey, entry -> entry.getValue().stream().toList() )); keyToExpressionsList = List.copyOf(keyToExpressionsMap.entrySet()); - missingKeyToExpressionList = missingKeyToExpressions.entrySet().stream() - .map(entry -> Map.entry(entry.getKey(), entry.getValue().stream().toList())).toList(); numExpressions = id; } @@ -260,14 +282,7 @@ private KeyIndex(MultiExpression expressions) { public List> getMatchesWithTriggers(SourceFeature input) { List> result = new ArrayList<>(); boolean[] visited = new boolean[numExpressions]; - for (var entry : constantTrueExpressionList) { - result.add(new Match<>(entry.result, List.of(), entry.id)); - } - for (var entry : missingKeyToExpressionList) { - if (!input.hasTag(entry.getKey())) { - visitExpressions(input, result, visited, entry.getValue()); - } - } + visitExpressions(input, result, visited, alwaysEvaluateExpressionList); Map tags = input.tags(); if (tags.size() < keyToExpressionsMap.size()) { for (String inputKey : tags.keySet()) { @@ -291,19 +306,22 @@ private static class GeometryTypeIndex implements Index { private final KeyIndex lineIndex; private final KeyIndex polygonIndex; - private GeometryTypeIndex(MultiExpression expressions) { + private GeometryTypeIndex(MultiExpression expressions, boolean warn) { // build an index per type then search in each of those indexes based on the geometry type of each input element // this narrows the search space substantially, improving matching performance - pointIndex = indexForType(expressions, Expression.POINT_TYPE); - lineIndex = indexForType(expressions, Expression.LINESTRING_TYPE); - polygonIndex = indexForType(expressions, Expression.POLYGON_TYPE); + pointIndex = indexForType(expressions, Expression.POINT_TYPE, warn); + lineIndex = indexForType(expressions, Expression.LINESTRING_TYPE, warn); + polygonIndex = indexForType(expressions, Expression.POLYGON_TYPE, warn); } - private KeyIndex indexForType(MultiExpression expressions, String type) { - return new KeyIndex<>(expressions - .replace(matchType(type), TRUE) - .replace(e -> e instanceof Expression.MatchType, FALSE) - .simplify()); + private KeyIndex indexForType(MultiExpression expressions, String type, boolean warn) { + return new KeyIndex<>( + expressions + .replace(matchType(type), TRUE) + .replace(e -> e instanceof Expression.MatchType, FALSE) + .simplify(), + warn + ); } /** diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/expression/ExpressionTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/expression/ExpressionTest.java index a31fb3a94d..01366c7f31 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/expression/ExpressionTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/expression/ExpressionTest.java @@ -1,12 +1,18 @@ package com.onthegomap.planetiler.expression; +import static com.onthegomap.planetiler.TestUtils.newPoint; import static com.onthegomap.planetiler.expression.Expression.*; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.onthegomap.planetiler.reader.SimpleFeature; +import com.onthegomap.planetiler.reader.SourceFeature; import com.onthegomap.planetiler.reader.WithTags; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; import java.util.Set; import org.junit.jupiter.api.Test; @@ -16,6 +22,14 @@ class ExpressionTest { public static final Expression.MatchAny matchCD = matchAny("c", "d"); public static final Expression.MatchAny matchBC = matchAny("b", "c"); + static SourceFeature featureWithTags(String... tags) { + Map map = new HashMap<>(); + for (int i = 0; i < tags.length; i += 2) { + map.put(tags[i], tags[i + 1]); + } + return SimpleFeature.create(newPoint(0, 0), map); + } + @Test void testSimplify() { assertEquals(matchAB, matchAB.simplify()); @@ -80,6 +94,39 @@ void testSimplifyTrue() { assertEquals(TRUE, or(TRUE, matchAB).simplify()); } + @Test + void testSimplifyAndCases() { + assertEquals(TRUE, and(TRUE).simplify()); + assertEquals(TRUE, and(TRUE, TRUE).simplify()); + assertEquals(TRUE, and(TRUE, and()).simplify()); + assertEquals(TRUE, and(and(TRUE)).simplify()); + assertEquals(TRUE, and(and(TRUE), TRUE).simplify()); + assertEquals(TRUE, and(TRUE, and(TRUE), TRUE).simplify()); + assertEquals(matchAB, and(TRUE, and(TRUE), matchAB).simplify()); + } + + @Test + void testSimplifyOrCases() { + assertEquals(FALSE, or(or(FALSE)).simplify()); + assertEquals(FALSE, or(or(FALSE), FALSE).simplify()); + assertEquals(FALSE, or(FALSE, or(FALSE), FALSE).simplify()); + assertEquals(matchAB, or(FALSE, or(FALSE), matchAB).simplify()); + } + + @Test + void testSimplifyNotCases() { + assertEquals(FALSE, not(TRUE).simplify()); + assertEquals(TRUE, not(FALSE).simplify()); + } + + @Test + void testEvaluateEmptyAnd() { + assertEquals( + and().evaluate(featureWithTags(), new ArrayList<>()), + and().simplify().evaluate(featureWithTags(), new ArrayList<>()) + ); + } + @Test void testReplace() { assertEquals( diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/expression/MultiExpressionTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/expression/MultiExpressionTest.java index 31845e7695..54c14a22d8 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/expression/MultiExpressionTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/expression/MultiExpressionTest.java @@ -4,6 +4,7 @@ import static com.onthegomap.planetiler.TestUtils.newPoint; import static com.onthegomap.planetiler.TestUtils.rectangle; import static com.onthegomap.planetiler.expression.Expression.*; +import static com.onthegomap.planetiler.expression.ExpressionTest.featureWithTags; import static com.onthegomap.planetiler.expression.MultiExpression.entry; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -15,21 +16,13 @@ import com.onthegomap.planetiler.reader.WithTags; import java.util.ArrayList; import java.util.Comparator; -import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import org.junit.jupiter.api.Test; class MultiExpressionTest { - private static SourceFeature featureWithTags(String... tags) { - Map map = new HashMap<>(); - for (int i = 0; i < tags.length; i += 2) { - map.put(tags[i], tags[i + 1]); - } - return SimpleFeature.create(newPoint(0, 0), map); - } - @Test void testEmpty() { var index = MultiExpression.of(List.of()).index(); @@ -263,6 +256,35 @@ void testAnd() { assertSameElements(List.of(), index.getMatches(featureWithTags())); } + @Test + void testAndConstant() { + var index = MultiExpression.of(List.of( + entry("a", and( + Expression.TRUE, + matchAny("key1", "val1") + )) + )).index(); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1", "key2", "val2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "no", "key2", "val2"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1", "key2", "no"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key2", "val2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags())); + + index = MultiExpression.of(List.of( + entry("a", and( + Expression.FALSE, + matchAny("key1", "val1") + )) + )).index(); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "val1", "key2", "val2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "no", "key2", "val2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "val1", "key2", "no"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "val1"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key2", "val2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags())); + } + @Test void testOr() { var index = MultiExpression.of(List.of( @@ -281,6 +303,52 @@ void testOr() { assertSameElements(List.of(), index.getMatches(featureWithTags())); } + @Test + void testMapResult() { + var index = MultiExpression.of(List.of( + entry("a", matchField("key")) + )).mapResults(result -> result.toUpperCase(Locale.ROOT)).index(); + assertSameElements(List.of("A"), index.getMatches(featureWithTags("key", "value"))); + } + + @Test + void testFilterResults() { + var index = MultiExpression.of(List.of( + entry("a", matchField("key")), + entry("b", matchField("key")) + )).filterResults(result -> !result.equals("b")).index(); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key", "value"))); + } + + @Test + void testOrConstant() { + var index = MultiExpression.of(List.of( + entry("a", or( + Expression.TRUE, + matchAny("key1", "val1") + )) + )).indexAndWarn(); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1", "key2", "val2"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "no", "key2", "val2"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1", "key2", "no"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key2", "val2"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags())); + + index = MultiExpression.of(List.of( + entry("a", or( + Expression.FALSE, + matchAny("key1", "val1") + )) + )).index(); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1", "key2", "val2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "no", "key2", "val2"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1", "key2", "no"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key2", "val2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags())); + } + @Test void testNot() { var index = MultiExpression.of(List.of( @@ -298,6 +366,41 @@ void testNot() { assertSameElements(List.of(), index.getMatches(featureWithTags())); } + @Test + void testNor() { + var index = MultiExpression.of(List.of( + entry("a", not(or( + matchAny("key1", "val1"), + matchAny("key2", "val2") + ))) + )).index(); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "val1", "key2", "val2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "val1", "key2", "val2", "key3", "val3"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "no", "key2", "val2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "val1", "key2", "no"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "val1"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key2", "val2"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags())); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "no", "key2", "no"))); + } + + @Test + void testNand() { + var index = MultiExpression.of(List.of( + entry("a", not(and( + matchAny("key1", "val1"), + matchAny("key2", "val2") + ))) + )).index(); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "val1", "key2", "val2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "val1", "key2", "val2", "key3", "val3"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags())); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key2", "val2"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "val1", "key2", "no"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "no", "key2", "val2"))); + } + @Test void testXor() { var index = MultiExpression.of(List.of( @@ -434,6 +537,86 @@ void testMatchDifferentTypes() { assertEquals("polygon", index.getOrElse(polygon, null)); } + @Test + void testMatchMissing() { + // Test logic: match if either key1 or key2 is missing + var index1 = MultiExpression.of(List.of( + entry("a", or( + matchAny("key1", ""), + matchAny("key2", "") + )) + )).index(); + + var index2 = MultiExpression.of(List.of( + entry("a", not(and( + matchField("key1"), + matchField("key2") + ))) + )).index(); + + List.of(index1, index2).forEach(index -> { + assertSameElements(List.of("a"), index.getMatches(featureWithTags())); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "value1"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key2", "value2"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key1", "value1", "key2", "value2"))); + }); + } + + @Test + void testMatchNotMissing() { + //Test logic: match if key1 is present (not missing) + var index1 = MultiExpression.of(List.of( + entry("a", matchField("key1")) + )).index(); + + var index2 = MultiExpression.of(List.of( + entry("a", not( + matchAny("key1", "") + )) + )).index(); + + List.of(index1, index2).forEach(index -> { + assertSameElements(List.of(), index.getMatches(featureWithTags())); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "value1"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key2", "value2"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key1", "value1", "key2", "value2"))); + }); + } + + void testAndOrMatch() { + var expr = and( + or( + matchAny("key", "val") + ), + TRUE + ); + var index = MultiExpression.of(List.of(entry("a", expr))).index(); + + //Ensure MultiExpression and regular Expression work the same + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key", "val"))); + assertSameElements(List.of("a"), index.getMatches(featureWithTags("key", "val", "otherkey", "otherval"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("otherkey", "otherval"))); + assertSameElements(List.of(), index.getMatches(featureWithTags("key", "otherval"))); + assertSameElements(List.of(), index.getMatches(featureWithTags())); + + var list = new ArrayList(); + + assertTrue(expr.evaluate(featureWithTags("key", "val"), list)); + assertTrue(expr.evaluate(featureWithTags("key", "val", "otherkey", "otherval"), list)); + assertFalse(expr.evaluate(featureWithTags("otherkey", "otherval"), list)); + assertFalse(expr.evaluate(featureWithTags("key", "otherval"), list)); + assertFalse(expr.evaluate(featureWithTags(), list)); + + expr.simplify(); + + //Ensure Expression works the same after a simplify + assertTrue(expr.evaluate(featureWithTags("key", "val"), list)); + assertTrue(expr.evaluate(featureWithTags("key", "val", "otherkey", "otherval"), list)); + assertFalse(expr.evaluate(featureWithTags("otherkey", "otherval"), list)); + assertFalse(expr.evaluate(featureWithTags("key", "otherval"), list)); + assertFalse(expr.evaluate(featureWithTags(), list)); + } + private static void assertSameElements(List a, List b) { assertEquals( a.stream().sorted(Comparator.comparing(Object::toString)).toList(),