Merge pull request #1531 from joto/input-check-versions

Fix input data check: Two versions of same object are not allowed

Author: lonvia

src/input.cpp

@@ -21,7 +21,7 @@
 #include "osmdata.hpp"
 #include "progress-display.hpp"
 
-type_id_version check_input(type_id_version const &last, type_id_version curr)
+type_id check_input(type_id const &last, type_id curr)
 {
     if (curr.id < 0) {
         throw std::runtime_error{
@@ -40,14 +40,10 @@ type_id_version check_input(type_id_version const &last, type_id_version curr)
                     osmium::item_type_to_name(last.type), curr.id, last.id)};
         }
 
-        if (last.version < curr.version) {
-            return curr;
-        }
-
         throw std::runtime_error{
-            "Input data is not ordered: {} id {} version {} after {}."_format(
-                osmium::item_type_to_name(last.type), curr.id, curr.version,
-                last.version)};
+            "Input data is not ordered:"
+            " {} id {} appears more than once."_format(
+                osmium::item_type_to_name(last.type), curr.id)};
     }
 
     if (item_type_to_nwr_index(last.type) <=
@@ -60,10 +56,9 @@ type_id_version check_input(type_id_version const &last, type_id_version curr)
         osmium::item_type_to_name(last.type))};
 }
 
-type_id_version check_input(type_id_version const &last,
-                            osmium::OSMObject const &object)
+type_id check_input(type_id const &last, osmium::OSMObject const &object)
 {
-    return check_input(last, {object.type(), object.id(), object.version()});
+    return check_input(last, {object.type(), object.id()});
 }
 
 /**
@@ -130,7 +125,7 @@ class data_source_t
     osmium::memory::Buffer m_buffer{};
     iterator m_it{};
     iterator m_end{};
-    type_id_version m_last = {osmium::item_type::node, 0, 0};
+    type_id m_last = {osmium::item_type::node, 0};
 
 }; // class data_source_t
 
@@ -274,7 +269,7 @@ static void process_single_file(osmium::io::File const &file,
                                 progress_display_t *progress, bool append)
 {
     osmium::io::Reader reader{file};
-    type_id_version last{osmium::item_type::node, 0, 0};
+    type_id last{osmium::item_type::node, 0};
 
     input_context_t ctx{osmdata, progress, append};
     while (osmium::memory::Buffer buffer = reader.read()) {

src/input.hpp

@@ -28,21 +28,19 @@
 
 class osmdata_t;
 
-struct type_id_version
+struct type_id
 {
     osmium::item_type type;
     osmid_t id;
-    osmium::object_version_type version;
 };
 
 /**
- * Compare two tuples (type, id, version) throw a descriptive error if either
- * the curr id is negative or if the data is not ordered.
+ * Compare two tuples (type, id). Throw a descriptive error if either the
+ * curr id is negative or if the data is not ordered.
  */
-type_id_version check_input(type_id_version const &last, type_id_version curr);
+type_id check_input(type_id const &last, type_id curr);
 
-type_id_version check_input(type_id_version const &last,
-                            osmium::OSMObject const &object);
+type_id check_input(type_id const &last, osmium::OSMObject const &object);
 
 /**
  * Prepare input file(s). Does format checks as far as this is possible

tests/test-check-input.cpp

@@ -13,27 +13,25 @@
 
 TEST_CASE("it's good if input data is ordered", "[NoDB]")
 {
-    type_id_version const tiv1{osmium::item_type::node, 1, 1};
-    type_id_version const tiv2{osmium::item_type::node, 1, 2};
-    type_id_version const tiv3{osmium::item_type::node, 2, 1};
-    type_id_version const tiv4{osmium::item_type::way, 1, 1};
-    type_id_version const tiv5{osmium::item_type::way, 2, 1};
-    type_id_version const tiv6{osmium::item_type::relation, 1, 1};
-    type_id_version const tiv7{osmium::item_type::relation, 1, 2};
+    type_id const tiv1{osmium::item_type::node, 1};
+    type_id const tiv2{osmium::item_type::node, 2};
+    type_id const tiv3{osmium::item_type::way, 1};
+    type_id const tiv4{osmium::item_type::way, 2};
+    type_id const tiv5{osmium::item_type::relation, 1};
+    type_id const tiv6{osmium::item_type::relation, 2};
 
     REQUIRE_NOTHROW(check_input(tiv1, tiv2));
     REQUIRE_NOTHROW(check_input(tiv2, tiv3));
     REQUIRE_NOTHROW(check_input(tiv3, tiv4));
     REQUIRE_NOTHROW(check_input(tiv4, tiv5));
     REQUIRE_NOTHROW(check_input(tiv5, tiv6));
-    REQUIRE_NOTHROW(check_input(tiv6, tiv7));
 }
 
 TEST_CASE("negative OSM object ids are not allowed", "[NoDB]")
 {
-    type_id_version const tivn{osmium::item_type::node, -17, 1};
-    type_id_version const tivw{osmium::item_type::way, -1, 1};
-    type_id_version const tivr{osmium::item_type::relation, -999, 17};
+    type_id const tivn{osmium::item_type::node, -17};
+    type_id const tivw{osmium::item_type::way, -1};
+    type_id const tivr{osmium::item_type::relation, -999};
 
     REQUIRE_THROWS_WITH(
         check_input(tivn, tivn),
@@ -47,18 +45,18 @@ TEST_CASE("negative OSM object ids are not allowed", "[NoDB]")
 
 TEST_CASE("objects of the same type must be ordered", "[NoDB]")
 {
-    type_id_version const tiv1{osmium::item_type::node, 42, 1};
-    type_id_version const tiv2{osmium::item_type::node, 3, 1};
+    type_id const tiv1{osmium::item_type::node, 42};
+    type_id const tiv2{osmium::item_type::node, 3};
 
     REQUIRE_THROWS_WITH(check_input(tiv1, tiv2),
                         "Input data is not ordered: node id 3 after 42.");
 }
 
 TEST_CASE("a node after a way or relation is not allowed", "[NoDB]")
 {
-    type_id_version const tiv1w{osmium::item_type::way, 42, 1};
-    type_id_version const tiv1r{osmium::item_type::relation, 42, 1};
-    type_id_version const tiv2{osmium::item_type::node, 100, 1};
+    type_id const tiv1w{osmium::item_type::way, 42};
+    type_id const tiv1r{osmium::item_type::relation, 42};
+    type_id const tiv2{osmium::item_type::node, 100};
 
     REQUIRE_THROWS_WITH(check_input(tiv1w, tiv2),
                         "Input data is not ordered: node after way.");
@@ -68,20 +66,20 @@ TEST_CASE("a node after a way or relation is not allowed", "[NoDB]")
 
 TEST_CASE("a way after a relation is not allowed", "[NoDB]")
 {
-    type_id_version const tiv1{osmium::item_type::relation, 42, 1};
-    type_id_version const tiv2{osmium::item_type::way, 100, 1};
+    type_id const tiv1{osmium::item_type::relation, 42};
+    type_id const tiv2{osmium::item_type::way, 100};
 
     REQUIRE_THROWS_WITH(check_input(tiv1, tiv2),
                         "Input data is not ordered: way after relation.");
 }
 
-TEST_CASE("versions must be ordered", "[NoDB]")
+TEST_CASE("no object may appear twice", "[NoDB]")
 {
-    type_id_version const tiv1{osmium::item_type::way, 42, 2};
-    type_id_version const tiv2{osmium::item_type::way, 42, 1};
+    type_id const tiv1{osmium::item_type::way, 42};
+    type_id const tiv2{osmium::item_type::way, 42};
 
     REQUIRE_THROWS_WITH(
         check_input(tiv1, tiv2),
-        "Input data is not ordered: way id 42 version 1 after 2.");
+        "Input data is not ordered: way id 42 appears more than once.");
 }