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 db08f14607..d70d14c833 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 @@ -40,12 +40,27 @@ public record MultiExpression(List> expressions) implements Simplifi private static final Logger LOGGER = LoggerFactory.getLogger(MultiExpression.class); private static final Comparator BY_ID = Comparator.comparingInt(WithId::id); + /** + * Returns a new multi-expression from {@code expressions} where multiple expressions for the same key get OR'd + * together. + *

+ * If the order in which expresions match matter, use {@link #ofOrdered(List)} instead. + */ public static MultiExpression of(List> expressions) { LinkedHashMap map = new LinkedHashMap<>(); for (var expression : expressions) { map.merge(expression.result, expression.expression, Expression::or); } - return new MultiExpression<>(map.entrySet().stream().map(e -> entry(e.getKey(), e.getValue())).collect(Collectors.toList())); + return new MultiExpression<>( + map.entrySet().stream().map(e -> entry(e.getKey(), e.getValue())).collect(Collectors.toList())); + } + + /** + * Returns a new multi-expression from {@code expressions} where multiple expressions for the same key stay separate, + * in cases where the order in which expressions matches is important. + */ + public static MultiExpression ofOrdered(List> expressions) { + return new MultiExpression<>(new ArrayList<>(expressions)); } public static Entry entry(T result, Expression expression) { @@ -63,8 +78,8 @@ private static boolean mustAlwaysEvaluate(Expression expression) { case Expression.Not(var child) -> !mustAlwaysEvaluate(child); case Expression.MatchAny any when any.mustAlwaysEvaluate() -> true; case null, default -> !(expression instanceof Expression.MatchAny) && - !(expression instanceof Expression.MatchField) && - !FALSE.equals(expression); + !(expression instanceof Expression.MatchField) && + !FALSE.equals(expression); }; } 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 e7f5b678e2..83fdf36a0e 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 @@ -729,33 +729,24 @@ private static SourceFeature point(String source, String layer, Map void assertSameElements(List a, List b) { diff --git a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfigExpressionParser.java b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfigExpressionParser.java index 1137bfffe9..8db83addf1 100644 --- a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfigExpressionParser.java +++ b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfigExpressionParser.java @@ -146,7 +146,7 @@ private ConfigExpression.Match parseMatch(Object match, boolean allowE } else { throw new ParseException("Invalid match block. Expected a list or map, but got: " + match); } - return ConfigExpression.match(signature(output), MultiExpression.of(List.copyOf(conditions)), fallback); + return ConfigExpression.match(signature(output), MultiExpression.ofOrdered(List.copyOf(conditions)), fallback); } private Signature signature(Class outputClass) { diff --git a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/expression/ConfigExpression.java b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/expression/ConfigExpression.java index 4e4853bbd2..65cb28700c 100644 --- a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/expression/ConfigExpression.java +++ b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/expression/ConfigExpression.java @@ -152,7 +152,7 @@ public ConfigExpression simplifyOnce() { if (Expression.TRUE.equals(expression.expression())) { return new Match<>( signature, - MultiExpression.of(expressions.stream().limit(i).toList()), + MultiExpression.ofOrdered(expressions.stream().limit(i).toList()), expression.result() ); } diff --git a/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/ConfiguredFeatureTest.java b/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/ConfiguredFeatureTest.java index 4a9d2fd82e..3ad107562c 100644 --- a/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/ConfiguredFeatureTest.java +++ b/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/ConfiguredFeatureTest.java @@ -1602,4 +1602,38 @@ void testGeometryAttributesArea(String expression, double expected) { testFeature(config, sfMatch, any -> assertEquals(expected, (Double) any.getAttrsAtZoom(14).get("attr"), expected / 1e3), 1); } + + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testMatchOrdering(boolean withFallback) { + var config = """ + sources: + osm: + type: osm + url: geofabrik:rhode-island + local_path: data/rhode-island.osm.pbf + layers: + - id: testLayer + features: + - source: osm + attributes: + - key: attr + value: + - if: {natural: tree} + value: green + - if: {historic: memorial} + value: black + - if: {tourism: viewpoint} + value: green + - if: ${%s} + value: fallback + + """.formatted(withFallback ? "true" : "false"); + testFeature(config, SimpleFeature.createFakeOsmFeature(newPoint(0, 0), Map.of( + "historic", "memorial", + "tourism", "viewpoint" + ), "osm", null, 1, emptyList(), OSM_INFO), feature -> assertEquals("black", feature.getAttrsAtZoom(14).get("attr")), + 1); + } } diff --git a/pom.xml b/pom.xml index 6cca7720b6..f07f5c1ba7 100644 --- a/pom.xml +++ b/pom.xml @@ -186,7 +186,7 @@ - 4.29 + 4.33 ${maven.multiModuleProjectDirectory}/eclipse-formatter.xml