Merge pull request #324 from GIScience/fix-contribution-deletions

fix contributions extraction endpoints for non-final deletions

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
 
 

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,66 +114,90 @@ public DataExtractionTransformer(String startTimestamp, String endTimestamp,
    * @return list of GeoJSON features corresponding to the given OSM entity's modifications.
    */
   public List<Feature> buildChangedFeatures(List<OSMContribution> contributions) {
+    if (isContributionsEndpoint) {
+      return buildChangedFeaturesContributions(contributions);
+    } else {
+      return buildChangedFeaturesFullHistory(contributions);
+    }
+  }
+
+  private List<Feature> buildChangedFeaturesContributions(List<OSMContribution> contributions) {
+    List<Feature> 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<String, Object> 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<Feature> buildChangedFeaturesFullHistory(List<OSMContribution> contributions) {
     List<Feature> output = new LinkedList<>();
     Map<String, Object> properties;
     Geometry currentGeom = null;
     OSMEntity currentEntity = null;
     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<Feature> buildChangedFeatures(List<OSMContribution> 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<Feature> 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();
     }

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()) {

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<String[]> createCsvTopComments(String url, String text, Strin
   public org.wololo.geojson.Feature createOSMFeature(OSMEntity entity, Geometry geometry,
       Map<String, Object> properties, Set<Integer> keysInt, boolean includeTags,
       boolean includeOSMMetadata, boolean includeContributionTypes, boolean isContributionsEndpoint,
-      ElementsGeometry elemGeom, EnumSet<ContributionType> contributionTypes) {
-    if (geometry.isEmpty() && !contributionTypes.contains(ContributionType.DELETION)) {
+      ElementsGeometry elemGeom, Supplier<EnumSet<ContributionType>> 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;

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<JsonNode> 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<JsonNode> 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
    */