don't silently ignore invalid timestamps supplied to mapreducer (#469)

GIScience · Nov 2, 2022 · 8604ac8 · 8604ac8
1 parent a7bcef0
commit 8604ac8
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,7 +29,9 @@ Changelog
 * update jts dependency to version 1.18.2
 * update ignite dependency to version 2.14.0 ([#459], [#467])
 * add natural order to `OSHDBTag` ([#454])
+* throw exception when invalid timestamp strings are supplied to the MapReducer ([#260])
 
+[#260]: https://github.com/GIScience/oshdb/issues/260
 [#419]: https://github.com/GIScience/oshdb/pull/419
 [#433]: https://github.com/GIScience/oshdb/issues/433
 [#438]: https://github.com/GIScience/oshdb/pull/438

diff --git a/oshdb-api/src/main/java/org/heigit/ohsome/oshdb/api/mapreducer/MapReducer.java b/oshdb-api/src/main/java/org/heigit/ohsome/oshdb/api/mapreducer/MapReducer.java
@@ -413,17 +413,13 @@ public MapReducer<X> timestamps(String isoDateStart, String isoDateEnd) {
   public MapReducer<X> timestamps(
       String isoDateFirst, String isoDateSecond, String... isoDateMore) {
     SortedSet<OSHDBTimestamp> timestamps = new TreeSet<>();
-    try {
-      timestamps.add(
-          new OSHDBTimestamp(IsoDateTimeParser.parseIsoDateTime(isoDateFirst).toEpochSecond()));
+    timestamps.add(
+        new OSHDBTimestamp(IsoDateTimeParser.parseIsoDateTime(isoDateFirst).toEpochSecond()));
+    timestamps.add(
+        new OSHDBTimestamp(IsoDateTimeParser.parseIsoDateTime(isoDateSecond).toEpochSecond()));
+    for (String isoDate : isoDateMore) {
       timestamps.add(
-          new OSHDBTimestamp(IsoDateTimeParser.parseIsoDateTime(isoDateSecond).toEpochSecond()));
-      for (String isoDate : isoDateMore) {
-        timestamps.add(
-            new OSHDBTimestamp(IsoDateTimeParser.parseIsoDateTime(isoDate).toEpochSecond()));
-      }
-    } catch (Exception e) {
-      LOG.error("unable to parse ISO date string: " + e.getMessage());
+          new OSHDBTimestamp(IsoDateTimeParser.parseIsoDateTime(isoDate).toEpochSecond()));
     }
     return this.timestamps(() -> timestamps);
   }

diff --git a/oshdb-api/src/test/java/org/heigit/ohsome/oshdb/api/tests/TestMapReducerTimestamps.java b/oshdb-api/src/test/java/org/heigit/ohsome/oshdb/api/tests/TestMapReducerTimestamps.java
@@ -0,0 +1,81 @@
+package org.heigit.ohsome.oshdb.api.tests;
+
+import static org.heigit.ohsome.oshdb.OSHDBBoundingBox.bboxWgs84Coordinates;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.time.format.DateTimeParseException;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.SortedMap;
+import org.heigit.ohsome.oshdb.OSHDBBoundingBox;
+import org.heigit.ohsome.oshdb.api.db.OSHDBDatabase;
+import org.heigit.ohsome.oshdb.api.db.OSHDBH2;
+import org.heigit.ohsome.oshdb.api.generic.OSHDBCombinedIndex;
+import org.heigit.ohsome.oshdb.api.mapreducer.MapReducer;
+import org.heigit.ohsome.oshdb.api.mapreducer.OSMContributionView;
+import org.heigit.ohsome.oshdb.api.mapreducer.OSMEntitySnapshotView;
+import org.heigit.ohsome.oshdb.osm.OSMType;
+import org.heigit.ohsome.oshdb.util.mappable.OSMContribution;
+import org.heigit.ohsome.oshdb.util.mappable.OSMEntitySnapshot;
+import org.heigit.ohsome.oshdb.util.time.OSHDBTimestampIllegalArgumentException;
+import org.heigit.ohsome.oshdb.util.time.OSHDBTimestamps;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test aggregate by custom index method of the OSHDB API.
+ */
+class TestMapReducerTimestamps {
+  private final OSHDBDatabase oshdb;
+
+  private final OSHDBBoundingBox bbox = bboxWgs84Coordinates(8.0, 49.0, 9.0, 50.0);
+
+  TestMapReducerTimestamps() throws Exception {
+    oshdb = new OSHDBH2("../data/test-data");
+  }
+
+  private MapReducer<OSMContribution> createMapReducerOSMContribution() throws Exception {
+    return OSMContributionView
+        .on(oshdb)
+        .areaOfInterest(bbox)
+        .filter("type:node and highway=*");
+  }
+
+  private MapReducer<OSMEntitySnapshot> createMapReducerOSMEntitySnapshot() throws Exception {
+    return OSMEntitySnapshotView
+        .on(oshdb)
+        .areaOfInterest(bbox)
+        .filter("type:node and highway=*");
+  }
+
+  @Test
+  void testInvalidTimestamps() {
+    assertThrows(DateTimeParseException.class, this::invalidTimestamps1);
+    assertThrows(OSHDBTimestampIllegalArgumentException.class, this::invalidTimestamps2);
+  }
+
+  private void invalidTimestamps1() throws Exception {
+    // contribution view query
+    var ignored = createMapReducerOSMContribution()
+        .timestamps("invalid1", "invalid2")
+        .map(OSMContribution::getContributorUserId)
+        .uniq();
+    // snapshot view query
+    var ignored2 = createMapReducerOSMEntitySnapshot()
+        .timestamps("invalid")
+        .count();
+  }
+
+  @SuppressWarnings("UnusedAssignment")
+  private void invalidTimestamps2() throws Exception {
+    // invalid time zone
+    var ignored = createMapReducerOSMEntitySnapshot()
+        .timestamps("2020-01-01T00:00:00+00")
+        .count();
+    // invalid sign
+    ignored = createMapReducerOSMEntitySnapshot()
+        .timestamps("-2020-01-01T00:00:00Z")
+        .count();
+  }
+}