From 68d36c7198fdb22caa185c6c4b035b4f5d8a7525 Mon Sep 17 00:00:00 2001 From: Michael Bell Date: Wed, 30 Sep 2020 23:08:40 +0100 Subject: [PATCH] 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. --- CHANGELOG.md | 1 + include/engine/plugins/plugin_base.hpp | 17 +++++++++++++++++ src/engine/plugins/table.cpp | 6 ++---- src/engine/plugins/trip.cpp | 3 +-- src/engine/plugins/viaroute.cpp | 3 +-- unit_tests/library/table.cpp | 2 ++ 6 files changed, 24 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c44f9ab8e65..c2f7561947f 100644 --- 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 index 28e3db72fdc..4550722fdc1 100644 --- a/include/engine/plugins/plugin_base.hpp +++ b/include/engine/plugins/plugin_base.hpp @@ -371,6 +371,23 @@ class BasePlugin } return phantom_node_pairs; } + + std::string MissingPhantomErrorMessage(const std::vector &phantom_nodes, + const std::vector &coordinates) const + { + BOOST_ASSERT(phantom_nodes.size() < coordinates.size()); + size_t missing_index = phantom_nodes.size(); + for (size_t i : util::irange(0, phantom_nodes.size())) + { + if (phantom_nodes[i].first.input_location != coordinates[i]) + { + missing_index = i; + break; + } + } + 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 index 190a0138c42..5b517ac17e8 100644 --- 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 index 75a38d50a1f..74a4452aa23 100644 --- 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 index 8c4e3e8914d..9b0f3ebfc04 100644 --- 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 index 139830856fa..26fbdec7838 100644 --- 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().value; BOOST_CHECK_EQUAL(code, "NoSegment"); + const auto message = json_result.values.at("message").get().value; + BOOST_CHECK_EQUAL(message, "Could not find a matching segment for coordinate 0"); } BOOST_AUTO_TEST_CASE(test_table_serialiaze_fb)