From cff8ff428287aaf501ddb0928a479d014dd5fdd9 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 14 Dec 2024 10:18:18 +0100 Subject: [PATCH] Order deleted objects after visible ones in reverse id order osmium::object_order_type_id_reverse_version is used to order OSM objects for merging or applying diffs. If the diffs are from extracts, it can happen that there are multiple objects with the same type, id, version, and timestamp but different deleted flag. In that case the merged diff should contain the visible object, not the deleted one, because the deleted one isn't really deleted, just outside the area of the extract. To achieve this, osmium::object_order_type_id_reverse_version must order visible objects before deleted ones if all else stays the same. See https://github.com/osmcode/osmium-tool/issues/282 --- include/osmium/osm/object_comparisons.hpp | 6 ++++-- test/t/osm/test_object_comparisons.cpp | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/include/osmium/osm/object_comparisons.hpp b/include/osmium/osm/object_comparisons.hpp index f77df6ab..2205ae4c 100644 --- a/include/osmium/osm/object_comparisons.hpp +++ b/include/osmium/osm/object_comparisons.hpp @@ -160,9 +160,11 @@ namespace osmium { bool operator()(const osmium::OSMObject& lhs, const osmium::OSMObject& rhs) const noexcept { return const_tie(lhs.type(), lhs.id() > 0, lhs.positive_id(), rhs.version(), - ((lhs.timestamp().valid() && rhs.timestamp().valid()) ? rhs.timestamp() : osmium::Timestamp())) < + ((lhs.timestamp().valid() && rhs.timestamp().valid()) ? rhs.timestamp() : osmium::Timestamp()), + rhs.visible()) < const_tie(rhs.type(), rhs.id() > 0, rhs.positive_id(), lhs.version(), - ((lhs.timestamp().valid() && rhs.timestamp().valid()) ? lhs.timestamp() : osmium::Timestamp())); + ((lhs.timestamp().valid() && rhs.timestamp().valid()) ? lhs.timestamp() : osmium::Timestamp()), + lhs.visible()); } /// @pre lhs and rhs must not be nullptr diff --git a/test/t/osm/test_object_comparisons.cpp b/test/t/osm/test_object_comparisons.cpp index 2254ce41..2ff41276 100644 --- a/test/t/osm/test_object_comparisons.cpp +++ b/test/t/osm/test_object_comparisons.cpp @@ -195,6 +195,12 @@ TEST_CASE("Node comparisons") { REQUIRE(std::is_sorted(nodes.cbegin(), nodes.cend(), osmium::object_order_type_id_reverse_version{})); } + SECTION("reverse version ordering should order objects with deleted flag last") { + nodes.emplace_back(buffer.get(osmium::builder::add_node(buffer, _id( 1), _version(2), _timestamp("2016-01-01T00:00:00Z"), _deleted(false)))); + nodes.emplace_back(buffer.get(osmium::builder::add_node(buffer, _id( 1), _version(2), _timestamp("2016-01-01T00:00:00Z"), _deleted(true)))); + + REQUIRE(std::is_sorted(nodes.cbegin(), nodes.cend(), osmium::object_order_type_id_reverse_version{})); + } } TEST_CASE("Object comparisons: types are ordered nodes, then ways, then relations") {