diff --git a/CHANGELOG.md b/CHANGELOG.md index b356c795..ddee11a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,9 +3,13 @@ Changelog ## 1.11.0-SNAPSHOT (current main) +### Bug Fixes +* Fix contributions extractions endpoints which were missing _deletion_ contributions that were later reverted ([#324]) + ### Other * Fix filter documentation ([#326]) +[#324]: https://github.com/GIScience/ohsome-api/pull/324 [#326]: https://github.com/GIScience/ohsome-api/pull/326 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 04fa160b..e99dc914 100644 --- a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java +++ b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/DataExtractionTransformer.java @@ -18,6 +18,7 @@ import org.heigit.ohsome.oshdb.util.mappable.OSMContribution; import org.heigit.ohsome.oshdb.util.mappable.OSMEntitySnapshot; import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.GeometryFactory; import org.wololo.geojson.Feature; /** @@ -113,6 +114,43 @@ public DataExtractionTransformer(String startTimestamp, String endTimestamp, * @return list of GeoJSON features corresponding to the given OSM entity's modifications. */ public List buildChangedFeatures(List contributions) { + if (isContributionsEndpoint) { + return buildChangedFeaturesContributions(contributions); + } else { + return buildChangedFeaturesFullHistory(contributions); + } + } + + private List buildChangedFeaturesContributions(List contributions) { + List output = new LinkedList<>(); + + for (int i = 0; i < contributions.size(); i++) { + if (isContributionsLatestEndpoint && i < contributions.size() - 1) { + // skip to end the loop when using contributions/latest endpoint + continue; + } + var contribution = contributions.get(i); + var geometryAfter = ExecutionUtils.getGeometry(contribution, clipGeometries, false); + if (geometryAfter == null) { + geometryAfter = new GeometryFactory().createEmpty(-1); + } + var entityAfter = contribution.getEntityAfter(); + var timestamp = TimestampFormatter.getInstance().isoDateTime(contribution.getTimestamp()); + + Map properties = new TreeMap<>(); + + properties.put(TIMESTAMP_PROPERTY, timestamp); + properties.put(CONTRIBUTION_CHANGESET_ID_PROPERTY, contribution.getChangesetId()); + output.add(exeUtils.createOSMFeature(entityAfter, geometryAfter, properties, keysInt, + includeTags, includeOSMMetadata, includeContributionTypes, isContributionsEndpoint, + outputGeometry, contribution::getContributionTypes, + contribution.is(ContributionType.DELETION))); + } + + return output; + } + + private List buildChangedFeaturesFullHistory(List contributions) { List output = new LinkedList<>(); Map properties; Geometry currentGeom = null; @@ -120,59 +158,46 @@ public List buildChangedFeatures(List contributions) { String validFrom = null; String validTo; boolean skipNext = false; - if (!isContributionsLatestEndpoint) { - // first contribution: - OSMContribution firstContribution = contributions.get(0); - if (firstContribution.is(ContributionType.CREATION) && !isContributionsEndpoint) { + + // first contribution: + OSMContribution firstContribution = contributions.get(0); + if (firstContribution.is(ContributionType.CREATION)) { + skipNext = true; + } else { + // if not "creation": take "before" as starting "row" (geom, tags), valid_from = t_start + currentEntity = firstContribution.getEntityBefore(); + currentGeom = ExecutionUtils.getGeometry(firstContribution, clipGeometries, true); + validFrom = startTimestamp; + } + + // then for each contribution: + for (OSMContribution contribution : contributions) { + // set valid_to of previous row + validTo = TimestampFormatter.getInstance().isoDateTime(contribution.getTimestamp()); + if (!skipNext && currentGeom != null && !currentGeom.isEmpty()) { + boolean addToOutput = addEntityToOutput(currentEntity, currentGeom); + if (addToOutput) { + properties = new TreeMap<>(); + properties.put(VALID_FROM_PROPERTY, validFrom); + properties.put(VALID_TO_PROPERTY, validTo); + output.add(exeUtils.createOSMFeature(currentEntity, currentGeom, properties, keysInt, + includeTags, includeOSMMetadata, includeContributionTypes, isContributionsEndpoint, + outputGeometry, contribution::getContributionTypes, + contribution.is(ContributionType.DELETION))); + } + } + skipNext = false; + if (contribution.is(ContributionType.DELETION)) { + // if deletion: skip output of next row skipNext = true; } else { - // if not "creation": take "before" as starting "row" (geom, tags), valid_from = t_start - currentEntity = firstContribution.getEntityBefore(); - currentGeom = ExecutionUtils.getGeometry(firstContribution, clipGeometries, true); - validFrom = startTimestamp; - } - // then for each contribution: - for (int i = 0; i < contributions.size(); i++) { - if (i == contributions.size() - 1 && isContributionsEndpoint) { - // end the loop when last contribution is reached as it gets added later on - break; - } - OSMContribution contribution = contributions.get(i); - if (isContributionsEndpoint) { - currentEntity = contribution.getEntityAfter(); - currentGeom = ExecutionUtils.getGeometry(contribution, clipGeometries, false); - validFrom = TimestampFormatter.getInstance().isoDateTime(contribution.getTimestamp()); - } - // set valid_to of previous row - validTo = TimestampFormatter.getInstance().isoDateTime(contribution.getTimestamp()); - if (!skipNext && currentGeom != null && !currentGeom.isEmpty()) { - boolean addToOutput = addEntityToOutput(currentEntity, currentGeom); - if (addToOutput) { - properties = new TreeMap<>(); - if (!isContributionsEndpoint) { - properties.put(VALID_FROM_PROPERTY, validFrom); - properties.put(VALID_TO_PROPERTY, validTo); - } else { - properties.put(TIMESTAMP_PROPERTY, validTo); - properties.put(CONTRIBUTION_CHANGESET_ID_PROPERTY, contribution.getChangesetId()); - } - output.add(exeUtils.createOSMFeature(currentEntity, currentGeom, properties, keysInt, - includeTags, includeOSMMetadata, includeContributionTypes, isContributionsEndpoint, - outputGeometry, contribution.getContributionTypes())); - } - } - skipNext = false; - if (contribution.is(ContributionType.DELETION)) { - // if deletion: skip output of next row - skipNext = true; - } else if (!isContributionsEndpoint) { - // else: take "after" as next row - currentEntity = contribution.getEntityAfter(); - currentGeom = ExecutionUtils.getGeometry(contribution, clipGeometries, false); - validFrom = TimestampFormatter.getInstance().isoDateTime(contribution.getTimestamp()); - } + // else: take "after" as next row + currentEntity = contribution.getEntityAfter(); + currentGeom = ExecutionUtils.getGeometry(contribution, clipGeometries, false); + validFrom = TimestampFormatter.getInstance().isoDateTime(contribution.getTimestamp()); } } + // after loop: OSMContribution lastContribution = contributions.get(contributions.size() - 1); currentGeom = ExecutionUtils.getGeometry(lastContribution, clipGeometries, false); @@ -181,33 +206,19 @@ public List buildChangedFeatures(List contributions) { // if last contribution was not "deletion": set valid_to = t_end validTo = endTimestamp; properties = new TreeMap<>(); - if (!isContributionsEndpoint) { - properties.put(VALID_FROM_PROPERTY, validFrom); - properties.put(VALID_TO_PROPERTY, validTo); - } else { - properties.put(TIMESTAMP_PROPERTY, - TimestampFormatter.getInstance().isoDateTime(lastContribution.getTimestamp())); - properties.put(CONTRIBUTION_CHANGESET_ID_PROPERTY, lastContribution.getChangesetId()); - } + properties.put(VALID_FROM_PROPERTY, validFrom); + properties.put(VALID_TO_PROPERTY, validTo); if (!currentGeom.isEmpty()) { boolean addToOutput = addEntityToOutput(currentEntity, currentGeom); if (addToOutput) { output.add(exeUtils.createOSMFeature(currentEntity, currentGeom, properties, keysInt, includeTags, includeOSMMetadata, includeContributionTypes, isContributionsEndpoint, - outputGeometry, lastContribution.getContributionTypes())); + outputGeometry, lastContribution::getContributionTypes, + lastContribution.is(ContributionType.DELETION))); } } - } else if (isContributionsEndpoint) { - // adds the deletion feature for a /contributions request - currentGeom = ExecutionUtils.getGeometry(lastContribution, clipGeometries, true); - properties = new TreeMap<>(); - properties.put(TIMESTAMP_PROPERTY, - TimestampFormatter.getInstance().isoDateTime(lastContribution.getTimestamp())); - properties.put(CONTRIBUTION_CHANGESET_ID_PROPERTY, lastContribution.getChangesetId()); - output.add(exeUtils.createOSMFeature(currentEntity, currentGeom, properties, keysInt, false, - includeOSMMetadata, includeContributionTypes, isContributionsEndpoint, outputGeometry, - lastContribution.getContributionTypes())); } + return output; } @@ -239,7 +250,8 @@ public List buildUnchangedFeatures(OSMEntitySnapshot snapshot) { if (addToOutput) { return Collections.singletonList(exeUtils.createOSMFeature(entity, geom, properties, keysInt, includeTags, includeOSMMetadata, includeContributionTypes, - isContributionsEndpoint, outputGeometry, EnumSet.noneOf(ContributionType.class))); + isContributionsEndpoint, outputGeometry, + () -> EnumSet.noneOf(ContributionType.class), false)); } else { return Collections.emptyList(); } diff --git a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/ElementsRequestExecutor.java b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/ElementsRequestExecutor.java index 0347b4be..ff1eaad0 100644 --- a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/ElementsRequestExecutor.java +++ b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/ElementsRequestExecutor.java @@ -130,7 +130,7 @@ public static void extract(RequestResource requestResource, ElementsGeometry ele } return exeUtils.createOSMFeature(snapshot.getEntity(), geom, properties, keysInt, includeTags, includeOSMMetadata, false, false, elemGeom, - EnumSet.noneOf(ContributionType.class)); + () -> EnumSet.noneOf(ContributionType.class), false); }).filter(Objects::nonNull); Metadata metadata = null; if (processingData.isShowMetadata()) { diff --git a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/ExecutionUtils.java b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/ExecutionUtils.java index 471b4b02..f7ec51db 100644 --- a/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/ExecutionUtils.java +++ b/src/main/lombok/org/heigit/ohsome/ohsomeapi/executor/ExecutionUtils.java @@ -33,6 +33,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Supplier; import java.util.stream.Stream; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletResponse; @@ -388,8 +389,9 @@ public static List createCsvTopComments(String url, String text, Strin public org.wololo.geojson.Feature createOSMFeature(OSMEntity entity, Geometry geometry, Map properties, Set keysInt, boolean includeTags, boolean includeOSMMetadata, boolean includeContributionTypes, boolean isContributionsEndpoint, - ElementsGeometry elemGeom, EnumSet contributionTypes) { - if (geometry.isEmpty() && !contributionTypes.contains(ContributionType.DELETION)) { + ElementsGeometry elemGeom, Supplier> contributionTypes, + boolean isDeletion) { + if (geometry.isEmpty() && !isDeletion) { // skip invalid geometries (e.g. ways with 0 nodes) return null; } @@ -404,15 +406,15 @@ public org.wololo.geojson.Feature createOSMFeature(OSMEntity entity, Geometry ge properties.put("@osmType", entity.getType().toString()); properties.put("@changesetId", entity.getChangesetId()); if (isContributionsEndpoint) { - properties = addContributionTypes(properties, contributionTypes); + properties = addContributionTypes(properties, contributionTypes.get()); } } if (includeContributionTypes && !includeOSMMetadata) { - properties = addContributionTypes(properties, contributionTypes); + properties = addContributionTypes(properties, contributionTypes.get()); } properties.put("@osmId", entity.getType().toString().toLowerCase() + "/" + entity.getId()); GeoJSONWriter gjw = new GeoJSONWriter(); - if (isContributionsEndpoint && contributionTypes.contains(ContributionType.DELETION)) { + if (isContributionsEndpoint && isDeletion) { return new org.wololo.geojson.Feature(null, properties); } Geometry outputGeometry; 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 feb31cdd..ee199ccb 100644 --- a/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java +++ b/src/test/java/org/heigit/ohsome/ohsomeapi/controller/DataExtractionTest.java @@ -1,6 +1,8 @@ package org.heigit.ohsome.ohsomeapi.controller; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -474,6 +476,38 @@ public void contributionsChangesetFilterTest() { features.get(0).get("properties").get("@contributionChangesetId").asText()); } + @Test + public void contributionsDeletedEntity() { + TestRestTemplate restTemplate = new TestRestTemplate(); + ResponseEntity response = restTemplate.getForEntity(server + port + + "/contributions/bbox?bboxes=8.67,49.39,8.72,49.42" + + "&filter=type:node and id:1639495193&time=2012-01-01,2019-01-01&properties=metadata&clipGeometry=false", + JsonNode.class); + var features = response.getBody().get("features"); + assertEquals(2, features.size()); + assertEquals(1, features.get(0).get("properties").get("@version").asInt()); + assertFalse(features.get(0).get("geometry").isNull()); + assertEquals(2, features.get(1).get("properties").get("@version").asInt()); + assertTrue(features.get(1).get("geometry").isNull()); + } + + @Test + public void contributionsUndeletedEntity() { + TestRestTemplate restTemplate = new TestRestTemplate(); + ResponseEntity response = restTemplate.getForEntity(server + port + + "/contributions/bbox?bboxes=8.67,49.39,8.72,49.42" + + "&filter=type:node and id:3451640407&time=2015-01-01,2016-01-01&properties=metadata&clipGeometry=false", + JsonNode.class); + var features = response.getBody().get("features"); + assertEquals(3, features.size()); + assertEquals(1, features.get(0).get("properties").get("@version").asInt()); + assertFalse(features.get(0).get("geometry").isNull()); + assertEquals(2, features.get(1).get("properties").get("@version").asInt()); + assertTrue(features.get(1).get("geometry").isNull()); + assertEquals(3, features.get(2).get("properties").get("@version").asInt()); + assertFalse(features.get(2).get("geometry").isNull()); + } + /* * ./contributions/latest tests */