From bc4ba79cdf5a8dfcb65faaaca7069f42303f5c38 Mon Sep 17 00:00:00 2001 From: Michael Barry Date: Sat, 12 Oct 2024 12:16:58 -0400 Subject: [PATCH] Convert `${feature.source/source_layer}: [ ... ]` match expression to `MatchSource`/`MatchSourceLayer` (#1065) --- .../planetiler/expression/Expression.java | 13 ++-- .../expression/MultiExpression.java | 7 ++- .../planetiler/reader/SourceFeature.java | 4 +- .../planetiler/reader/WithSource.java | 5 ++ .../planetiler/reader/WithSourceLayer.java | 5 ++ planetiler-custommap/README.md | 3 +- planetiler-custommap/planetiler.schema.json | 2 +- .../custommap/BooleanExpressionParser.java | 17 +++++- .../custommap/ConfiguredFeature.java | 8 ++- .../custommap/ConfiguredProfile.java | 35 ++++------- .../planetiler/custommap/Contexts.java | 15 ++++- .../custommap/configschema/FeatureItem.java | 5 ++ .../custommap/ConfiguredFeatureTest.java | 26 ++++++++ .../expression/ConfigExpressionTest.java | 60 ++++++++++++++++++- 14 files changed, 162 insertions(+), 43 deletions(-) create mode 100644 planetiler-core/src/main/java/com/onthegomap/planetiler/reader/WithSource.java create mode 100644 planetiler-core/src/main/java/com/onthegomap/planetiler/reader/WithSourceLayer.java 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 311f2295..c4168f6a 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 @@ -50,7 +50,7 @@ public interface Expression extends Simplifiable { return and(List.of(children)); } - static And and(List children) { + static And and(List children) { return new And(children); } @@ -58,7 +58,7 @@ public interface Expression extends Simplifiable { return or(List.of(children)); } - static Or or(List children) { + static Or or(List children) { return new Or(children); } @@ -100,8 +100,7 @@ public interface Expression extends Simplifiable { *

* {@code values} can contain exact matches, "%text%" to match any value containing "text", or "" to match any value. */ - static MatchAny matchAnyTyped(String field, TypedGetter typeGetter, - List values) { + static MatchAny matchAnyTyped(String field, TypedGetter typeGetter, List values) { return MatchAny.from(field, typeGetter, values); } @@ -153,7 +152,7 @@ public interface Expression extends Simplifiable { return new MatchSourceLayer(layer); } - private static String generateJavaCodeList(List items) { + private static String generateJavaCodeList(List items) { return items.stream().map(Expression::generateJavaCode).collect(Collectors.joining(", ")); } @@ -268,7 +267,7 @@ public interface Expression extends Simplifiable { } } - record And(List children) implements Expression { + record And(List children) implements Expression { @Override public String generateJavaCode() { @@ -306,7 +305,7 @@ public interface Expression extends Simplifiable { } } - record Or(List children) implements Expression { + record Or(List children) implements Expression { @Override public String generateJavaCode() { 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 784d082a..db08f146 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 @@ -4,8 +4,9 @@ import static com.onthegomap.planetiler.expression.Expression.FALSE; import static com.onthegomap.planetiler.expression.Expression.TRUE; import static com.onthegomap.planetiler.expression.Expression.matchType; -import com.onthegomap.planetiler.reader.SourceFeature; import com.onthegomap.planetiler.reader.WithGeometryType; +import com.onthegomap.planetiler.reader.WithSource; +import com.onthegomap.planetiler.reader.WithSourceLayer; import com.onthegomap.planetiler.reader.WithTags; import java.util.ArrayList; import java.util.Comparator; @@ -436,7 +437,7 @@ public record MultiExpression(List> expressions) implements Simplifi @Override String extract(WithTags input) { - return input instanceof SourceFeature feature ? feature.getSourceLayer() : null; + return input instanceof WithSourceLayer feature ? feature.getSourceLayer() : null; } } @@ -451,7 +452,7 @@ public record MultiExpression(List> expressions) implements Simplifi @Override String extract(WithTags input) { - return input instanceof SourceFeature feature ? feature.getSource() : null; + return input instanceof WithSource feature ? feature.getSource() : null; } } diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/SourceFeature.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/SourceFeature.java index 68caa9d6..7dfdf6e8 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/SourceFeature.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/SourceFeature.java @@ -26,7 +26,7 @@ import org.locationtech.jts.geom.Polygon; * All geometries except for {@link #latLonGeometry()} return elements in world web mercator coordinates where (0,0) is * the northwest corner and (1,1) is the southeast corner of the planet. */ -public abstract class SourceFeature implements WithTags, WithGeometryType { +public abstract class SourceFeature implements WithTags, WithGeometryType, WithSource, WithSourceLayer { private final Map tags; private final String source; @@ -279,11 +279,13 @@ public abstract class SourceFeature implements WithTags, WithGeometryType { } /** Returns the ID of the source that this feature came from. */ + @Override public String getSource() { return source; } /** Returns the layer ID within a source that this feature comes from. */ + @Override public String getSourceLayer() { return sourceLayer; } diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/WithSource.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/WithSource.java new file mode 100644 index 00000000..34aaaae4 --- /dev/null +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/WithSource.java @@ -0,0 +1,5 @@ +package com.onthegomap.planetiler.reader; + +public interface WithSource { + String getSource(); +} diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/WithSourceLayer.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/WithSourceLayer.java new file mode 100644 index 00000000..3e043d9a --- /dev/null +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/reader/WithSourceLayer.java @@ -0,0 +1,5 @@ +package com.onthegomap.planetiler.reader; + +public interface WithSourceLayer { + String getSourceLayer(); +} diff --git a/planetiler-custommap/README.md b/planetiler-custommap/README.md index 03be03c6..d89d9d21 100644 --- a/planetiler-custommap/README.md +++ b/planetiler-custommap/README.md @@ -214,7 +214,8 @@ layers: A feature is a defined set of objects that meet a specified filter criteria. -- `source` - A string [source](#source) ID, or list of source IDs from which features should be extracted +- `source` - A string [source](#source) ID, or list of source IDs from which features should be extracted. If missing, + features from all sources are included. - `geometry` - A string enum that indicates which geometry types to include, and how to transform them. Can be one of: - `point` `line` or `polygon` to pass the original feature through diff --git a/planetiler-custommap/planetiler.schema.json b/planetiler-custommap/planetiler.schema.json index f763b8fd..437d691c 100644 --- a/planetiler-custommap/planetiler.schema.json +++ b/planetiler-custommap/planetiler.schema.json @@ -360,7 +360,7 @@ ] }, "source": { - "description": "A source ID or list of source IDs from which features should be extracted", + "description": "A source ID or list of source IDs from which features should be extracted. If unspecified, all sources are included", "oneOf": [ { "type": "string" diff --git a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/BooleanExpressionParser.java b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/BooleanExpressionParser.java index ac97bb5a..011df98d 100644 --- a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/BooleanExpressionParser.java +++ b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/BooleanExpressionParser.java @@ -3,8 +3,10 @@ package com.onthegomap.planetiler.custommap; import static com.onthegomap.planetiler.expression.Expression.matchAnyTyped; import static com.onthegomap.planetiler.expression.Expression.matchField; import static com.onthegomap.planetiler.expression.Expression.not; +import static com.onthegomap.planetiler.expression.Expression.or; import com.onthegomap.planetiler.custommap.expression.BooleanExpressionScript; +import com.onthegomap.planetiler.custommap.expression.ConfigExpression; import com.onthegomap.planetiler.custommap.expression.ConfigExpressionScript; import com.onthegomap.planetiler.custommap.expression.ParseException; import com.onthegomap.planetiler.custommap.expression.ScriptContext; @@ -119,11 +121,22 @@ public class BooleanExpressionParser { List values = (value instanceof Collection items ? items : value == null ? List.of() : List.of(value)) .stream().map(BooleanExpressionParser::unescape).toList(); if (ConfigExpressionScript.isScript(key)) { - var expression = ConfigExpressionScript.parse(ConfigExpressionScript.extractScript(key), context); + var expression = ConfigExpressionScript.parse(ConfigExpressionScript.extractScript(key), context).simplify(); if (isAny) { values = List.of(); } - return matchAnyTyped(null, expression, values); + var result = matchAnyTyped(null, expression, values); + if (!values.isEmpty() && result.pattern() == null && !result.isMatchAnything() && !result.matchWhenMissing() && + expression instanceof ConfigExpression.Variable(var ignored,var name)) { + if (name.equals("feature.source")) { + return or(values.stream().filter(String.class::isInstance).map(String.class::cast) + .map(Expression::matchSource).toList()); + } else if (name.equals("feature.source_layer")) { + return or(values.stream().filter(String.class::isInstance).map(String.class::cast) + .map(Expression::matchSourceLayer).toList()); + } + } + return result; } String field = unescape(key); if (isAny) { diff --git a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfiguredFeature.java b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfiguredFeature.java index a43e7f64..ed239688 100644 --- a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfiguredFeature.java +++ b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfiguredFeature.java @@ -64,6 +64,12 @@ public class ConfiguredFeature { BooleanExpressionParser.parse(feature.includeWhen(), tagValueProducer, processFeatureContext); } + if (!feature.source().isEmpty()) { + filter = Expression.and( + filter, + Expression.or(feature.source().stream().map(Expression::matchSource).toList()) + ); + } if (feature.excludeWhen() != null) { filter = Expression.and( filter, @@ -274,7 +280,7 @@ public class ConfiguredFeature { var sourceFeature = context.feature(); // Ensure that this feature is from the correct source (index should enforce this, so just check when assertions enabled) - assert sources.contains(sourceFeature.getSource()); + assert sources.isEmpty() || sources.contains(sourceFeature.getSource()); var f = geometryFactory.apply(features); for (var processor : featureProcessors) { diff --git a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfiguredProfile.java b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfiguredProfile.java index 1acd372f..02461761 100644 --- a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfiguredProfile.java +++ b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/ConfiguredProfile.java @@ -1,7 +1,6 @@ package com.onthegomap.planetiler.custommap; import static com.onthegomap.planetiler.expression.MultiExpression.Entry; -import static java.util.Map.entry; import com.onthegomap.planetiler.FeatureCollector; import com.onthegomap.planetiler.FeatureMerge; @@ -19,7 +18,6 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; /** * A profile configured from a yml file. @@ -27,10 +25,8 @@ import java.util.stream.Collectors; public class ConfiguredProfile implements Profile { private final SchemaConfig schema; - - private final Collection layers; private final Map layersById = new HashMap<>(); - private final Map> featureLayerMatcher; + private final Index featureLayerMatcher; private final TagValueProducer tagValueProducer; private final Contexts.Root rootContext; @@ -38,14 +34,14 @@ public class ConfiguredProfile implements Profile { this.schema = schema; this.rootContext = rootContext; - layers = schema.layers(); + Collection layers = schema.layers(); if (layers == null || layers.isEmpty()) { throw new IllegalArgumentException("No layers defined"); } tagValueProducer = new TagValueProducer(schema.inputMappings()); - Map>> configuredFeatureEntries = new HashMap<>(); + List> configuredFeatureEntries = new ArrayList<>(); for (var layer : layers) { String layerId = layer.id(); @@ -53,16 +49,12 @@ public class ConfiguredProfile implements Profile { for (var feature : layer.features()) { var configuredFeature = new ConfiguredFeature(layerId, tagValueProducer, feature, rootContext); var entry = new Entry<>(configuredFeature, configuredFeature.matchExpression()); - for (var source : feature.source()) { - var list = configuredFeatureEntries.computeIfAbsent(source, s -> new ArrayList<>()); - list.add(entry); - } + configuredFeatureEntries.add(entry); } } - featureLayerMatcher = configuredFeatureEntries.entrySet().stream() - .map(entry -> entry(entry.getKey(), MultiExpression.of(entry.getValue()).index())) - .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); + featureLayerMatcher = MultiExpression.of(configuredFeatureEntries).index(); + } @Override @@ -78,15 +70,12 @@ public class ConfiguredProfile implements Profile { @Override public void processFeature(SourceFeature sourceFeature, FeatureCollector featureCollector) { var context = rootContext.createProcessFeatureContext(sourceFeature, tagValueProducer); - var index = featureLayerMatcher.get(sourceFeature.getSource()); - if (index != null) { - var matches = index.getMatchesWithTriggers(context); - for (var configuredFeature : matches) { - configuredFeature.match().processFeature( - context.createPostMatchContext(configuredFeature.keys()), - featureCollector - ); - } + var matches = featureLayerMatcher.getMatchesWithTriggers(context); + for (var configuredFeature : matches) { + configuredFeature.match().processFeature( + context.createPostMatchContext(configuredFeature.keys()), + featureCollector + ); } } diff --git a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/Contexts.java b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/Contexts.java index f4977bfa..f01a8a41 100644 --- a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/Contexts.java +++ b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/Contexts.java @@ -13,6 +13,8 @@ import com.onthegomap.planetiler.custommap.expression.ScriptEnvironment; import com.onthegomap.planetiler.expression.DataType; import com.onthegomap.planetiler.reader.SourceFeature; import com.onthegomap.planetiler.reader.WithGeometryType; +import com.onthegomap.planetiler.reader.WithSource; +import com.onthegomap.planetiler.reader.WithSourceLayer; import com.onthegomap.planetiler.reader.WithTags; import com.onthegomap.planetiler.reader.osm.OsmElement; import com.onthegomap.planetiler.reader.osm.OsmSourceFeature; @@ -285,7 +287,8 @@ public class Contexts { * Makes nested contexts adhere to {@link WithTags} and {@link WithGeometryType} by recursively fetching source * feature from the root context. */ - private interface FeatureContext extends ScriptContext, WithTags, WithGeometryType, NestedContext { + private interface FeatureContext extends ScriptContext, WithTags, WithGeometryType, NestedContext, WithSourceLayer, + WithSource { default FeatureContext parent() { return null; @@ -325,6 +328,16 @@ public class Contexts { default boolean canBePolygon() { return feature().canBePolygon(); } + + @Override + default String getSource() { + return feature().getSource(); + } + + @Override + default String getSourceLayer() { + return feature().getSourceLayer(); + } } /** diff --git a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/configschema/FeatureItem.java b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/configschema/FeatureItem.java index 236a2f26..7f94bc87 100644 --- a/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/configschema/FeatureItem.java +++ b/planetiler-custommap/src/main/java/com/onthegomap/planetiler/custommap/configschema/FeatureItem.java @@ -25,4 +25,9 @@ public record FeatureItem( public FeatureGeometry geometry() { return geometry == null ? FeatureGeometry.ANY : geometry; } + + @Override + public List source() { + return source == null ? List.of() : source; + } } 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 270418cc..cbda42c3 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 @@ -1321,6 +1321,32 @@ class ConfiguredFeatureTest { }, 1); } + @ParameterizedTest + @ValueSource(strings = {"source: []", ""}) + void testAnySource(String expression) { + var config = """ + sources: + osm: + type: osm + url: geofabrik:rhode-island + local_path: data/rhode-island.osm.pbf + layers: + - id: testLayer + features: + - geometry: point + %s + """.formatted(expression).strip(); + this.planetilerConfig = PlanetilerConfig.from(Arguments.of(Map.of())); + testFeature(config, SimpleFeature.createFakeOsmFeature(newPoint(0, 0), Map.of( + ), "osm", null, 1, emptyList(), OSM_INFO), feature -> { + assertInstanceOf(Puntal.class, feature.getGeometry()); + }, 1); + testFeature(config, SimpleFeature.createFakeOsmFeature(newPoint(0, 0), Map.of( + ), "other", null, 1, emptyList(), OSM_INFO), feature -> { + assertInstanceOf(Puntal.class, feature.getGeometry()); + }, 1); + } + @Test void testWikidataParse() { var config = """ diff --git a/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/expression/ConfigExpressionTest.java b/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/expression/ConfigExpressionTest.java index d53f16f1..c6cce6db 100644 --- a/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/expression/ConfigExpressionTest.java +++ b/planetiler-custommap/src/test/java/com/onthegomap/planetiler/custommap/expression/ConfigExpressionTest.java @@ -5,13 +5,12 @@ import static com.onthegomap.planetiler.custommap.TestContexts.FEATURE_POST_MATC import static com.onthegomap.planetiler.custommap.TestContexts.PROCESS_FEATURE; import static com.onthegomap.planetiler.custommap.TestContexts.ROOT_CONTEXT; import static com.onthegomap.planetiler.custommap.expression.ConfigExpression.*; -import static com.onthegomap.planetiler.expression.Expression.matchAny; -import static com.onthegomap.planetiler.expression.Expression.matchAnyTyped; -import static com.onthegomap.planetiler.expression.Expression.or; +import static com.onthegomap.planetiler.expression.Expression.*; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import com.google.common.collect.Lists; import com.onthegomap.planetiler.config.Arguments; import com.onthegomap.planetiler.custommap.BooleanExpressionParser; import com.onthegomap.planetiler.custommap.Contexts; @@ -171,6 +170,61 @@ class ConfigExpressionTest { ); } + @Test + void testSimplifyExpressionToMatchSource() { + assertEquals( + match(FEATURE_SIGNATURE, MultiExpression.of(List.of( + MultiExpression.entry(constOf(1), + or(matchSource("source1"), matchSource("source2")) + )))), + + match(FEATURE_SIGNATURE, MultiExpression.of(List.of( + MultiExpression.entry(constOf(1), + BooleanExpressionParser.parse(Map.of("${feature.source}", List.of("source1", "source2")), null, + FEATURE_SIGNATURE.in()) + )))).simplify() + ); + } + + @Test + void testSimplifyExpressionToMatchSourceLayer() { + assertEquals( + match(FEATURE_SIGNATURE, MultiExpression.of(List.of( + MultiExpression.entry(constOf(1), + or(matchSourceLayer("layer1"), matchSourceLayer("layer2")) + )))), + + match(FEATURE_SIGNATURE, MultiExpression.of(List.of( + MultiExpression.entry(constOf(1), + BooleanExpressionParser.parse(Map.of("${feature.source_layer}", List.of("layer1", "layer2")), null, + FEATURE_SIGNATURE.in()) + )))).simplify() + ); + } + + @Test + void testDontSimplifyOtherSourceLayerCases() { + testDontSimplifyOtherSourceLayerCases("__any__", List.of()); + testDontSimplifyOtherSourceLayerCases("", List.of("")); + testDontSimplifyOtherSourceLayerCases("prefix%", List.of("prefix%")); + } + + private static void testDontSimplifyOtherSourceLayerCases(String in, List out) { + assertEquals( + match(FEATURE_SIGNATURE, MultiExpression.of(List.of( + MultiExpression.entry(constOf(1), + matchAnyTyped(null, variable(FEATURE_SIGNATURE.withOutput(Object.class), "feature.source_layer"), + out + ))))), + + match(FEATURE_SIGNATURE, MultiExpression.of(List.of( + MultiExpression.entry(constOf(1), + BooleanExpressionParser.parse(Map.of("${feature.source_layer}", Lists.newArrayList(in)), null, + FEATURE_SIGNATURE.in()) + )))).simplify() + ); + } + @Test void testSimplifyCoalesce() { assertEquals(