Incorrect error message when unable to snap all input coordinates

In cases where we are unable to find a phantom node for an input coordinate, we return an error indicating which coordinate failed. This would always refer to the coordinate with index equal to the number of valid phantom nodes found. We fix this by instead returning the first index for which a phantom node could not be found.
Project-OSRM · Sep 30, 2020 · 838d289 · 838d289
1 parent 523d9e9
commit 838d289
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@
       - CHANGED: Reduce memory usage for raster source handling. [#5572](https://github.com/Project-OSRM/osrm-backend/pull/5572)
       - CHANGED: Add cmake option `ENABLE_DEBUG_LOGGING` to control whether output debug logging. [#3427](https://github.com/Project-OSRM/osrm-backend/issues/3427)
       - CHANGED: updated extent of Hong Kong as left hand drive country. [#5535](https://github.com/Project-OSRM/osrm-backend/issues/5535)
+      - FIXED: corrected error message when failing to snap input coordinates [#5846](https://github.com/Project-OSRM/osrm-backend/pull/5846)
     - Infrastructure
       - REMOVED: STXXL support removed as STXXL became abandonware. [#5760](https://github.com/Project-OSRM/osrm-backend/pull/5760)
 # 5.21.0

diff --git a/include/engine/plugins/plugin_base.hpp b/include/engine/plugins/plugin_base.hpp
@@ -371,6 +371,22 @@ class BasePlugin
         }
         return phantom_node_pairs;
     }
+
+    std::string MissingPhantomErrorMessage(const std::vector<PhantomNodePair> &phantom_nodes,
+                                           const std::vector<util::Coordinate> &coordinates) const
+    {
+        BOOST_ASSERT(phantom_nodes.size() < coordinates.size());
+        auto mismatch = std::mismatch(phantom_nodes.begin(),
+                                      phantom_nodes.end(),
+                                      coordinates.begin(),
+                                      coordinates.end(),
+                                      [](const auto &phantom_node, const auto &coordinate) {
+                                          return phantom_node.first.input_location == coordinate;
+                                      });
+        std::size_t missing_index = std::distance(phantom_nodes.begin(), mismatch.first);
+        return std::string("Could not find a matching segment for coordinate ") +
+               std::to_string(missing_index);
+    }
 };
 }
 }

diff --git a/src/engine/plugins/table.cpp b/src/engine/plugins/table.cpp
@@ -75,10 +75,8 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
 
     if (phantom_nodes.size() != params.coordinates.size())
     {
-        return Error("NoSegment",
-                     std::string("Could not find a matching segment for coordinate ") +
-                         std::to_string(phantom_nodes.size()),
-                     result);
+        return Error(
+            "NoSegment", MissingPhantomErrorMessage(phantom_nodes, params.coordinates), result);
     }
 
     auto snapped_phantoms = SnapPhantomNodes(phantom_nodes);

diff --git a/src/engine/plugins/trip.cpp b/src/engine/plugins/trip.cpp
@@ -199,8 +199,7 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
     if (phantom_node_pairs.size() != number_of_locations)
     {
         return Error("NoSegment",
-                     std::string("Could not find a matching segment for coordinate ") +
-                         std::to_string(phantom_node_pairs.size()),
+                     MissingPhantomErrorMessage(phantom_node_pairs, parameters.coordinates),
                      result);
     }
     BOOST_ASSERT(phantom_node_pairs.size() == number_of_locations);

diff --git a/src/engine/plugins/viaroute.cpp b/src/engine/plugins/viaroute.cpp
@@ -90,8 +90,7 @@ Status ViaRoutePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithm
     if (phantom_node_pairs.size() != route_parameters.coordinates.size())
     {
         return Error("NoSegment",
-                     std::string("Could not find a matching segment for coordinate ") +
-                         std::to_string(phantom_node_pairs.size()),
+                     MissingPhantomErrorMessage(phantom_node_pairs, route_parameters.coordinates),
                      result);
     }
     BOOST_ASSERT(phantom_node_pairs.size() == route_parameters.coordinates.size());

diff --git a/unit_tests/library/table.cpp b/unit_tests/library/table.cpp
@@ -242,6 +242,8 @@ BOOST_AUTO_TEST_CASE(test_table_no_segment_for_some_coordinates)
     BOOST_CHECK(rc == Status::Error);
     const auto code = json_result.values.at("code").get<json::String>().value;
     BOOST_CHECK_EQUAL(code, "NoSegment");
+    const auto message = json_result.values.at("message").get<json::String>().value;
+    BOOST_CHECK_EQUAL(message, "Could not find a matching segment for coordinate 0");
 }
 
 BOOST_AUTO_TEST_CASE(test_table_serialiaze_fb)