diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/ForwardingProfile.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/ForwardingProfile.java index 0cdc5f88e9..57e67c7a01 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/ForwardingProfile.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/ForwardingProfile.java @@ -1,5 +1,7 @@ package com.onthegomap.planetiler; +import static com.onthegomap.planetiler.util.MutableCollections.makeMutable; + import com.onthegomap.planetiler.config.PlanetilerConfig; import com.onthegomap.planetiler.expression.Expression; import com.onthegomap.planetiler.expression.MultiExpression; @@ -8,6 +10,7 @@ import com.onthegomap.planetiler.reader.SourceFeature; import com.onthegomap.planetiler.reader.osm.OsmElement; import com.onthegomap.planetiler.reader.osm.OsmRelationInfo; +import com.onthegomap.planetiler.util.MutableCollections; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -216,12 +219,12 @@ public List postProcessLayerFeatures(String layer, int zoom, throws GeometryException { // delegate feature post-processing to each layer, if it implements FeaturePostProcessor List postProcessers = layerPostProcessors.get(layer); - List result = items; + List result = makeMutable(items); if (postProcessers != null) { for (var handler : postProcessers) { var thisResult = handler.postProcess(zoom, result); - if (thisResult != null) { - result = thisResult; + if (thisResult != null && result != thisResult) { + result = makeMutable(thisResult); } } } @@ -231,12 +234,12 @@ public List postProcessLayerFeatures(String layer, int zoom, @Override public Map> postProcessTileFeatures(TileCoord tileCoord, Map> layers) throws GeometryException { - var result = layers; + var result = MutableCollections.makeMutableMultimap(layers); for (TilePostProcessor postProcessor : tilePostProcessors) { // TODO catch failures to isolate from other tile postprocessors? var thisResult = postProcessor.postProcessTile(tileCoord, result); - if (thisResult != null) { - result = thisResult; + if (thisResult != null && result != thisResult) { + result = MutableCollections.makeMutableMultimap(thisResult); } } return result; diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/collection/FeatureGroup.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/collection/FeatureGroup.java index 15f1784a81..51404062b3 100644 --- a/planetiler-core/src/main/java/com/onthegomap/planetiler/collection/FeatureGroup.java +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/collection/FeatureGroup.java @@ -1,5 +1,7 @@ package com.onthegomap.planetiler.collection; +import static com.onthegomap.planetiler.util.MutableCollections.makeMutable; + import com.carrotsearch.hppc.LongLongHashMap; import com.onthegomap.planetiler.Profile; import com.onthegomap.planetiler.VectorTile; @@ -492,8 +494,8 @@ private void postProcessAndAddLayerFeatures(VectorTile encoder, String layer, return; } try { - List postProcessed = profile - .postProcessLayerFeatures(layer, tileCoord.z(), features); + List postProcessed = makeMutable(profile + .postProcessLayerFeatures(layer, tileCoord.z(), makeMutable(features))); features = postProcessed == null ? features : postProcessed; // lines are stored using a higher precision so that rounding does not // introduce artificial intersections between endpoints to confuse line merging, diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/util/MutableCollections.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/MutableCollections.java new file mode 100644 index 0000000000..212ebce21e --- /dev/null +++ b/planetiler-core/src/main/java/com/onthegomap/planetiler/util/MutableCollections.java @@ -0,0 +1,59 @@ +package com.onthegomap.planetiler.util; + +import java.util.AbstractSequentialList; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.NavigableMap; +import java.util.SequencedMap; +import java.util.TreeMap; + +/** Utilities for converting immutable collections to mutable ones. */ +public class MutableCollections { + private MutableCollections() {} + + /** Return a mutable copy of {@code list} or the original list if it is already mutable. */ + public static List makeMutable(List list) { + return switch (list) { + case ArrayList l -> l; + case LinkedList l -> l; + case AbstractSequentialList l -> new LinkedList<>(l); + case null -> list; + default -> new ArrayList<>(list); + }; + } + + /** + * Return a mutable copy of {@code map} with mutable list values or the original collections if they are already + * mutable. + */ + public static Map> makeMutableMultimap(Map> map) { + var mutableMap = makeMutableMap(map); + if (mutableMap != null) { + for (var entry : map.entrySet()) { + var key = entry.getKey(); + var value = entry.getValue(); + var mutableList = makeMutable(value); + if (mutableList != value) { + mutableMap.put(key, mutableList); + } + } + } + return mutableMap; + } + + /** Return a mutable copy of {@code map} or the original list if it is already mutable. */ + public static Map makeMutableMap(Map map) { + return switch (map) { + case HashMap m -> m; + case TreeMap m -> m; + case NavigableMap m -> new TreeMap<>(m); + case SequencedMap m -> new LinkedHashMap<>(m); + case null -> map; + default -> new HashMap<>(map); + }; + } +} diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/ForwardingProfileTests.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/ForwardingProfileTests.java index 28081e1dfb..76305f614f 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/ForwardingProfileTests.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/ForwardingProfileTests.java @@ -179,6 +179,21 @@ public String name() { }); assertEquals(List.of(feature), profile.postProcessLayerFeatures("a", 0, List.of(feature))); + // allow mutations on initial input + profile.registerHandler(new ForwardingProfile.LayerPostProcesser() { + @Override + public List postProcess(int zoom, List items) { + items.set(0, items.getFirst()); + return null; + } + + @Override + public String name() { + return "a"; + } + }); + assertEquals(List.of(feature), profile.postProcessLayerFeatures("a", 0, List.of(feature))); + // empty list removes profile.registerHandler(new ForwardingProfile.LayerPostProcesser() { @Override @@ -195,6 +210,23 @@ public String name() { // doesn't touch elements in another layer assertEquals(List.of(feature), profile.postProcessLayerFeatures("b", 0, List.of(feature))); + // allow mutations on subsequent input + profile.registerHandler(new ForwardingProfile.LayerPostProcesser() { + @Override + public List postProcess(int zoom, List items) { + items.add(null); + items.removeLast(); + return items; + } + + @Override + public String name() { + return "a"; + } + }); + assertEquals(List.of(), profile.postProcessLayerFeatures("a", 0, List.of(feature))); + assertEquals(List.of(), profile.postProcessLayerFeatures("a", 0, new ArrayList<>(List.of(feature)))); + // 2 handlers for same layer run one after another var skip1 = new ForwardingProfile.LayerPostProcesser() { @Override @@ -243,10 +275,36 @@ void testTilePostProcesser() throws GeometryException { assertEquals(Map.of("a", List.of(feature)), profile.postProcessTileFeatures(TileCoord.ofXYZ(0, 0, 0), Map.of("a", List.of(feature)))); + // allow mutation on initial input + profile.registerHandler((ForwardingProfile.TilePostProcessor) (tileCoord, layers) -> { + if (layers.containsKey("a")) { + var list = layers.get("a"); + var item = list.getFirst(); + list.set(0, item); + layers.put("a", list); + } + return layers; + }); + assertEquals(Map.of("a", List.of(feature)), + profile.postProcessTileFeatures(TileCoord.ofXYZ(0, 0, 0), Map.of("a", List.of(feature)))); + assertEquals(Map.of("a", List.of(feature)), + profile.postProcessTileFeatures(TileCoord.ofXYZ(0, 0, 0), + new HashMap<>(Map.of("a", new ArrayList<>(List.of(feature)))))); + // empty map removes profile.registerHandler((ForwardingProfile.TilePostProcessor) (tileCoord, layers) -> Map.of()); assertEquals(Map.of(), profile.postProcessTileFeatures(TileCoord.ofXYZ(0, 0, 0), Map.of("a", List.of(feature)))); + + // allow mutation on subsequent inputs + profile.registerHandler((ForwardingProfile.TilePostProcessor) (tileCoord, layers) -> { + layers.put("a", List.of()); + layers.remove("a"); + return layers; + }); + assertEquals(Map.of(), + profile.postProcessTileFeatures(TileCoord.ofXYZ(0, 0, 0), Map.of("a", List.of(feature)))); + // also touches elements in another layer assertEquals(Map.of(), profile.postProcessTileFeatures(TileCoord.ofXYZ(0, 0, 0), Map.of("b", List.of(feature)))); @@ -285,6 +343,7 @@ public Map> postProcessTile(TileCoord tileCoord Map.of("c", List.of(feature, feature, feature, feature)))); } + @Test void testCaresAboutSource() { profile.registerSourceHandler("a", (x, y) -> { diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/util/MutableCollectionsTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/util/MutableCollectionsTest.java new file mode 100644 index 0000000000..733b221e04 --- /dev/null +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/util/MutableCollectionsTest.java @@ -0,0 +1,105 @@ +package com.onthegomap.planetiler.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; +import org.junit.jupiter.api.Test; + +class MutableCollectionsTest { + @Test + void testListOf() { + var mutable = MutableCollections.makeMutable(List.of(1, 2, 3)); + mutable.add(4); + assertEquals(List.of(1, 2, 3, 4), mutable); + } + + @Test + void testListOf0() { + var mutable = MutableCollections.makeMutable(List.of()); + mutable.add(1); + mutable.add(2); + mutable.add(3); + mutable.add(4); + assertEquals(List.of(1, 2, 3, 4), mutable); + } + + @Test + void testArrayList() { + var mutable = MutableCollections.makeMutable(new ArrayList<>(List.of(1, 2, 3))); + mutable.add(4); + assertEquals(List.of(1, 2, 3, 4), mutable); + } + + @Test + void testLinkedList() { + var mutable = MutableCollections.makeMutable(new LinkedList<>(List.of(1, 2, 3))); + mutable.add(4); + assertEquals(List.of(1, 2, 3, 4), mutable); + } + + @Test + void testUnmodifiableCollection() { + var mutable = MutableCollections.makeMutable(Collections.unmodifiableList(new ArrayList<>(List.of(1, 2, 3)))); + mutable.add(4); + assertEquals(List.of(1, 2, 3, 4), mutable); + } + + @Test + void testGuavaList() { + var mutable = MutableCollections.makeMutable(ImmutableList.builder().add(1, 2, 3).build()); + mutable.add(4); + assertEquals(List.of(1, 2, 3, 4), mutable); + } + + @Test + void testMapOs() { + var mutable = MutableCollections.makeMutableMap(Map.of(1, 2, 3, 4)); + mutable.put(5, 6); + assertEquals(Map.of(1, 2, 3, 4, 5, 6), mutable); + } + + @Test + void testHashMap() { + var mutable = MutableCollections.makeMutableMap(new HashMap<>(Map.of(1, 2, 3, 4))); + mutable.put(5, 6); + assertEquals(Map.of(1, 2, 3, 4, 5, 6), mutable); + } + + @Test + void testTreeMap() { + var mutable = MutableCollections.makeMutableMap(new TreeMap<>(Map.of(1, 2, 3, 4))); + mutable.put(5, 6); + assertEquals(Map.of(1, 2, 3, 4, 5, 6), mutable); + } + + @Test + void testUnmodifiableMap() { + var mutable = MutableCollections.makeMutableMap(Collections.unmodifiableMap(new TreeMap<>(Map.of(1, 2, 3, 4)))); + mutable.put(5, 6); + assertEquals(Map.of(1, 2, 3, 4, 5, 6), mutable); + } + + @Test + void testGuavaMap() { + var mutable = MutableCollections.makeMutableMap(ImmutableMap.builder().put(1, 2).put(3, 4).build()); + mutable.put(5, 6); + assertEquals(Map.of(1, 2, 3, 4, 5, 6), mutable); + } + + @Test + void testMultimap() { + var mutable = MutableCollections.makeMutableMultimap(Map.of(1, List.of(2, 3), 4, List.of(5, 6))); + var map = mutable.get(1); + map.add(3); + mutable.put(7, map); + assertEquals(Map.of(1, List.of(2, 3, 3), 4, List.of(5, 6), 7, List.of(2, 3, 3)), mutable); + } +}