From 10f929230b397c42676d3fd506281a6b63e04643 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Wed, 17 Mar 2021 17:44:16 +0100 Subject: [PATCH 1/7] apply geometry checks to all stages in elementsFullHistory requests fixes #109 --- .../executor/DataExtractionTransformer.java | 30 ++++++++----------- .../executor/DataRequestExecutor.java | 2 +- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java index 919515f8..0f68d919 100644 --- a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java +++ b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java @@ -5,6 +5,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.function.Supplier; @@ -13,6 +14,7 @@ import org.heigit.bigspatialdata.oshdb.osm.OSMEntity; import org.heigit.bigspatialdata.oshdb.util.celliterator.ContributionType; import org.heigit.bigspatialdata.oshdb.util.time.TimestampFormatter; +import org.heigit.ohsome.filter.FilterExpression; import org.heigit.ohsome.ohsomeapi.controller.dataextraction.elements.ElementsGeometry; import org.heigit.ohsome.ohsomeapi.inputprocessing.InputProcessingUtils; import org.heigit.ohsome.ohsomeapi.inputprocessing.SimpleFeatureType; @@ -28,6 +30,7 @@ public class DataExtractionTransformer implements Serializable { private final String startTimestamp; private final InputProcessingUtils utils; private final Set simpleFeatureTypes; + private Optional filter; private final Set keysInt; private final boolean includeTags; private final boolean includeOSMMetadata; @@ -38,8 +41,9 @@ public class DataExtractionTransformer implements Serializable { public DataExtractionTransformer(boolean isContributionsLatestEndpoint, boolean isContributionsEndpoint, ExecutionUtils exeUtils, boolean clipGeometries, String startTimestamp, InputProcessingUtils utils, - Set simpleFeatureTypes, Set keysInt, boolean includeTags, - boolean includeOSMMetadata, ElementsGeometry elementsGeometry, String endTimestamp, + Set simpleFeatureTypes, Optional filter, + Set keysInt, boolean includeTags, boolean includeOSMMetadata, + ElementsGeometry elementsGeometry, String endTimestamp, boolean isContainingSimpleFeatureTypes) { this.isContributionsLatestEndpoint = isContributionsLatestEndpoint; this.isContributionsEndpoint = isContributionsEndpoint; @@ -48,6 +52,7 @@ public DataExtractionTransformer(boolean isContributionsLatestEndpoint, this.startTimestamp = startTimestamp; this.utils = utils; this.simpleFeatureTypes = simpleFeatureTypes; + this.filter = filter; this.keysInt = keysInt; this.includeTags = includeTags; this.includeOSMMetadata = includeOSMMetadata; @@ -91,8 +96,7 @@ public List buildChangedFeatures(List contributions) { validTo = TimestampFormatter.getInstance().isoDateTime(contribution.getTimestamp()); if (!skipNext && currentGeom != null && !currentGeom.isEmpty()) { final Geometry geomToCheck = currentGeom; - boolean addToOutput = addEntityToOutput(isContainingSimpleFeatureTypes, utils, - simpleFeatureTypes, () -> geomToCheck); + boolean addToOutput = addEntityToOutput(currentEntity, () -> geomToCheck); if (addToOutput) { properties = new TreeMap<>(); if (!isContributionsEndpoint) { @@ -135,8 +139,7 @@ public List buildChangedFeatures(List contributions) { } if (!currentGeom.isEmpty()) { final Geometry geomToCheck = currentGeom; - boolean addToOutput = addEntityToOutput(isContainingSimpleFeatureTypes, utils, - simpleFeatureTypes, () -> geomToCheck); + boolean addToOutput = addEntityToOutput(currentEntity, () -> geomToCheck); if (addToOutput) { output.add(exeUtils.createOSMFeature(currentEntity, currentGeom, properties, keysInt, includeTags, includeOSMMetadata, isContributionsEndpoint, elementsGeometry, @@ -172,8 +175,7 @@ public List buildUnchangedFeatures(OSMEntitySnapshot snapshot) { TimestampFormatter.getInstance().isoDateTime(snapshot.getTimestamp())); properties.put("@validFrom", startTimestamp); properties.put("@validTo", endTimestamp); - boolean addToOutput = addEntityToOutput(isContainingSimpleFeatureTypes, utils, - simpleFeatureTypes, geom); + boolean addToOutput = addEntityToOutput(entity, geom); if (addToOutput) { return Collections.singletonList( exeUtils.createOSMFeature(entity, geom.get(), properties, keysInt, includeTags, @@ -184,17 +186,11 @@ public List buildUnchangedFeatures(OSMEntitySnapshot snapshot) { } /** Checks whether the given entity should be added to the output (true) or not (false). */ - public static boolean addEntityToOutput( - boolean isContainingSimpleFeatureTypes, - InputProcessingUtils utils, - final Set simpleFeatureTypes, - Supplier currentGeom) { - boolean addToOutput; + public boolean addEntityToOutput(OSMEntity currentEntity, Supplier currentGeom) { if (isContainingSimpleFeatureTypes) { - addToOutput = utils.checkGeometryOnSimpleFeatures(currentGeom.get(), simpleFeatureTypes); + return utils.checkGeometryOnSimpleFeatures(currentGeom.get(), simpleFeatureTypes); } else { - addToOutput = true; + return filter.map(f -> f.applyOSMGeometry(currentEntity, currentGeom)).orElse(true); } - return addToOutput; } } diff --git a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataRequestExecutor.java b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataRequestExecutor.java index 44442b01..d0b8c12b 100644 --- a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataRequestExecutor.java +++ b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataRequestExecutor.java @@ -110,7 +110,7 @@ public void extract() throws Exception { final boolean isContainingSimpleFeatureTypes = processingData.isContainingSimpleFeatureTypes(); DataExtractionTransformer dataExtractionTransformer = new DataExtractionTransformer( isContributionsLatestEndpoint, isContributionsEndpoint, exeUtils, clipGeometries, - startTimestamp, utils, simpleFeatureTypes, keysInt, includeTags, includeOSMMetadata, + startTimestamp, utils, simpleFeatureTypes, filter, keysInt, includeTags, includeOSMMetadata, elementsGeometry, endTimestamp, isContainingSimpleFeatureTypes); MapReducer contributionPreResult = mapRedContributions .flatMap(dataExtractionTransformer::buildChangedFeatures) From 49898a3fa8e7314136bc9461b44a19807242e33b Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Wed, 17 Mar 2021 17:49:26 +0100 Subject: [PATCH 2/7] simplify implementation slightly --- .../executor/DataExtractionTransformer.java | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java index 0f68d919..4e751b5f 100644 --- a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java +++ b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java @@ -8,7 +8,6 @@ import java.util.Optional; import java.util.Set; import java.util.TreeMap; -import java.util.function.Supplier; import org.heigit.bigspatialdata.oshdb.api.object.OSMContribution; import org.heigit.bigspatialdata.oshdb.api.object.OSMEntitySnapshot; import org.heigit.bigspatialdata.oshdb.osm.OSMEntity; @@ -30,7 +29,7 @@ public class DataExtractionTransformer implements Serializable { private final String startTimestamp; private final InputProcessingUtils utils; private final Set simpleFeatureTypes; - private Optional filter; + private final Optional filter; private final Set keysInt; private final boolean includeTags; private final boolean includeOSMMetadata; @@ -95,8 +94,7 @@ public List buildChangedFeatures(List contributions) { // set valid_to of previous row validTo = TimestampFormatter.getInstance().isoDateTime(contribution.getTimestamp()); if (!skipNext && currentGeom != null && !currentGeom.isEmpty()) { - final Geometry geomToCheck = currentGeom; - boolean addToOutput = addEntityToOutput(currentEntity, () -> geomToCheck); + boolean addToOutput = addEntityToOutput(currentEntity, currentGeom); if (addToOutput) { properties = new TreeMap<>(); if (!isContributionsEndpoint) { @@ -138,8 +136,7 @@ public List buildChangedFeatures(List contributions) { TimestampFormatter.getInstance().isoDateTime(lastContribution.getTimestamp())); } if (!currentGeom.isEmpty()) { - final Geometry geomToCheck = currentGeom; - boolean addToOutput = addEntityToOutput(currentEntity, () -> geomToCheck); + boolean addToOutput = addEntityToOutput(currentEntity, currentGeom); if (addToOutput) { output.add(exeUtils.createOSMFeature(currentEntity, currentGeom, properties, keysInt, includeTags, includeOSMMetadata, isContributionsEndpoint, elementsGeometry, @@ -165,11 +162,11 @@ public List buildUnchangedFeatures(OSMEntitySnapshot snapshot) { if (includeOSMMetadata) { properties.put("@lastEdit", entity.getTimestamp().toString()); } - Supplier geom; + Geometry geom; if (clipGeometries) { - geom = snapshot::getGeometry; + geom = snapshot.getGeometry(); } else { - geom = snapshot::getGeometryUnclipped; + geom = snapshot.getGeometryUnclipped(); } properties.put("@snapshotTimestamp", TimestampFormatter.getInstance().isoDateTime(snapshot.getTimestamp())); @@ -178,7 +175,7 @@ public List buildUnchangedFeatures(OSMEntitySnapshot snapshot) { boolean addToOutput = addEntityToOutput(entity, geom); if (addToOutput) { return Collections.singletonList( - exeUtils.createOSMFeature(entity, geom.get(), properties, keysInt, includeTags, + exeUtils.createOSMFeature(entity, geom, properties, keysInt, includeTags, includeOSMMetadata, isContributionsEndpoint, elementsGeometry, null)); } else { return Collections.emptyList(); @@ -186,9 +183,9 @@ public List buildUnchangedFeatures(OSMEntitySnapshot snapshot) { } /** Checks whether the given entity should be added to the output (true) or not (false). */ - public boolean addEntityToOutput(OSMEntity currentEntity, Supplier currentGeom) { + public boolean addEntityToOutput(OSMEntity currentEntity, Geometry currentGeom) { if (isContainingSimpleFeatureTypes) { - return utils.checkGeometryOnSimpleFeatures(currentGeom.get(), simpleFeatureTypes); + return utils.checkGeometryOnSimpleFeatures(currentGeom, simpleFeatureTypes); } else { return filter.map(f -> f.applyOSMGeometry(currentEntity, currentGeom)).orElse(true); } From b1a38ff1b08a28e429e35df2d8d5f1eed1440e5e Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Wed, 17 Mar 2021 17:58:19 +0100 Subject: [PATCH 3/7] avoid Optional as field type in serializable class --- .../ohsomeapi/executor/DataExtractionTransformer.java | 7 +++---- .../ohsome/ohsomeapi/executor/DataRequestExecutor.java | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java index 4e751b5f..39e4d1ba 100644 --- a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java +++ b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java @@ -5,7 +5,6 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.TreeMap; import org.heigit.bigspatialdata.oshdb.api.object.OSMContribution; @@ -29,7 +28,7 @@ public class DataExtractionTransformer implements Serializable { private final String startTimestamp; private final InputProcessingUtils utils; private final Set simpleFeatureTypes; - private final Optional filter; + private final FilterExpression filter; private final Set keysInt; private final boolean includeTags; private final boolean includeOSMMetadata; @@ -40,7 +39,7 @@ public class DataExtractionTransformer implements Serializable { public DataExtractionTransformer(boolean isContributionsLatestEndpoint, boolean isContributionsEndpoint, ExecutionUtils exeUtils, boolean clipGeometries, String startTimestamp, InputProcessingUtils utils, - Set simpleFeatureTypes, Optional filter, + Set simpleFeatureTypes, FilterExpression filter, Set keysInt, boolean includeTags, boolean includeOSMMetadata, ElementsGeometry elementsGeometry, String endTimestamp, boolean isContainingSimpleFeatureTypes) { @@ -187,7 +186,7 @@ public boolean addEntityToOutput(OSMEntity currentEntity, Geometry currentGeom) if (isContainingSimpleFeatureTypes) { return utils.checkGeometryOnSimpleFeatures(currentGeom, simpleFeatureTypes); } else { - return filter.map(f -> f.applyOSMGeometry(currentEntity, currentGeom)).orElse(true); + return filter == null || filter.applyOSMGeometry(currentEntity, currentGeom); } } } diff --git a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataRequestExecutor.java b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataRequestExecutor.java index d0b8c12b..8276f221 100644 --- a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataRequestExecutor.java +++ b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataRequestExecutor.java @@ -110,8 +110,8 @@ public void extract() throws Exception { final boolean isContainingSimpleFeatureTypes = processingData.isContainingSimpleFeatureTypes(); DataExtractionTransformer dataExtractionTransformer = new DataExtractionTransformer( isContributionsLatestEndpoint, isContributionsEndpoint, exeUtils, clipGeometries, - startTimestamp, utils, simpleFeatureTypes, filter, keysInt, includeTags, includeOSMMetadata, - elementsGeometry, endTimestamp, isContainingSimpleFeatureTypes); + startTimestamp, utils, simpleFeatureTypes, filter.orElse(null), keysInt, includeTags, + includeOSMMetadata, elementsGeometry, endTimestamp, isContainingSimpleFeatureTypes); MapReducer contributionPreResult = mapRedContributions .flatMap(dataExtractionTransformer::buildChangedFeatures) .filter(Objects::nonNull); From 9ad2bddc13c912a8fccc4184e7df5ba62a382e19 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Wed, 17 Mar 2021 18:03:40 +0100 Subject: [PATCH 4/7] add to changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4290d755..ded5b5c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Changelog * fix returning invalid GeoJSON using empty coordinates for deletion contributions ([#129], [#131]) * fix using a proper boolean data type instead of a string for contributionType in response ([#135]) * fix NPE with createOSMFeature ([#141]) +* make sure geometry filters are applied to all returned features of elementsFullHistory requests ([#109]) ### Performance and Code Quality @@ -25,6 +26,7 @@ Changelog [#83]: https://github.com/GIScience/ohsome-api/issues/83 [#98]: https://github.com/GIScience/ohsome-api/issues/98 +[#109]: https://github.com/GIScience/ohsome-api/issues/109 [#111]: https://github.com/GIScience/ohsome-api/issues/111 [#113]: https://github.com/GIScience/ohsome-api/issues/113 [#114]: https://github.com/GIScience/ohsome-api/pull/114 From 702b04dc3cd14f768dbc887c830af0e8a75a8e6a Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Wed, 17 Mar 2021 19:16:26 +0100 Subject: [PATCH 5/7] add test for issue #109 --- .../ohsomeapi/controller/DataExtractionTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java index ad71f8f2..762821c0 100644 --- a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java +++ b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java @@ -515,4 +515,16 @@ public void contributionsLatestCreationTest() { assertTrue( response.getBody().get("features").get(0).get("properties").get("@creation").asBoolean()); } + + @Test + public void issue109Test() { + // see https://github.com/GIScience/ohsome-api/issues/109 + TestRestTemplate restTemplate = new TestRestTemplate(); + ResponseEntity response = restTemplate.getForEntity( + server + port + + "/elementsFullHistory/centroid?bboxes=8.69525,49.40938,8.70461,49.41203&" + + "time=2011-09-05,2011-09-06&filter=geometry:polygon and id:relation/1391838", + JsonNode.class); + assertEquals(1, response.getBody().get("features").size()); + } } From b3d51120f6204976b454d76971dc1af313a51c4a Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Wed, 17 Mar 2021 19:13:09 +0100 Subject: [PATCH 6/7] make other test method names uniform --- .../heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java | 2 +- .../heigit/ohsome/ohsomeapi/controller/GetControllerTest.java | 2 +- .../heigit/ohsome/ohsomeapi/controller/PostControllerTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java index 762821c0..11f7eba0 100644 --- a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java +++ b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java @@ -426,7 +426,7 @@ public void contributionsVersionTest() { } @Test - public void contributionsAssociationChangeSetIdWithOsmIdAndVersion() { + public void contributionsAssociationChangeSetIdWithOsmIdAndVersionTest() { TestRestTemplate restTemplate = new TestRestTemplate(); ResponseEntity response = restTemplate.getForEntity(server + port + "/contributions/bbox?bboxes=8.70606,49.412150,8.70766,49.413686" diff --git a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/GetControllerTest.java b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/GetControllerTest.java index ed247eda..decf45a6 100644 --- a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/GetControllerTest.java +++ b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/GetControllerTest.java @@ -1238,7 +1238,7 @@ public void getOrFilterTest() { } @Test - public void getRequestEndsByQuestionMark() { + public void getRequestEndsByQuestionMarkTest() { TestRestTemplate restTemplate = new TestRestTemplate(); ResponseEntity response = restTemplate.getForEntity(server + port + "/users/count?", JsonNode.class); diff --git a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/PostControllerTest.java b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/PostControllerTest.java index 3aa97b85..9bcc7fe7 100644 --- a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/PostControllerTest.java +++ b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/PostControllerTest.java @@ -1129,7 +1129,7 @@ public void postUmlautFilterTest() { } @Test - public void postQueryRequestEndsByQuestionMark() { + public void postQueryRequestEndsByQuestionMarkTest() { TestRestTemplate restTemplate = new TestRestTemplate(); MultiValueMap map = new LinkedMultiValueMap<>(); ResponseEntity response = From 2209c8114e38c909614c2a2a5b5b02d80082a7d6 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Thu, 18 Mar 2021 10:38:54 +0100 Subject: [PATCH 7/7] clarify that the centroids endpoint is used on purpose --- .../heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java index 11f7eba0..6b496363 100644 --- a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java +++ b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java @@ -520,6 +520,8 @@ public void contributionsLatestCreationTest() { public void issue109Test() { // see https://github.com/GIScience/ohsome-api/issues/109 TestRestTemplate restTemplate = new TestRestTemplate(); + // this uses the centroid endpoint to make sure that geometry filters are even applied to + // the geometries before being transformed to, e.g., centroid points ResponseEntity response = restTemplate.getForEntity( server + port + "/elementsFullHistory/centroid?bboxes=8.69525,49.40938,8.70461,49.41203&"