Bump version fields, 5.15.3

* expose waypoints parameter in match interface

* Sync target_traversed_in_reverse with target_phantom

CHANGELOG.md

@@ -1,3 +1,14 @@
+# 5.15.3
+  - Changes from 5.15.2:
+    - Bugfixes: fix deduplication of route steps when waypoints are used [#4909](https://github.com/Project-OSRM/osrm-backend/issues/4909)
+
+# 5.15.2
+  - Changes from 5.15.1:
+    - Features:
+        - Exposed the waypoints parameter in the node bindings interface
+    - Bugfixes:
+        - Segfault causing bug in leg collapsing map matching when traversing edges in reverse
+
 # 5.15.1
   - Changes from 5.15.0:
     - Bugfixes:

CMakeLists.txt

@@ -62,7 +62,7 @@ endif()
 project(OSRM C CXX)
 set(OSRM_VERSION_MAJOR 5)
 set(OSRM_VERSION_MINOR 15)
-set(OSRM_VERSION_PATCH 1)
+set(OSRM_VERSION_PATCH 3)
 set(OSRM_VERSION "${OSRM_VERSION_MAJOR}.${OSRM_VERSION_MINOR}.${OSRM_VERSION_PATCH}")
 
 add_definitions(-DOSRM_PROJECT_DIR="${CMAKE_CURRENT_SOURCE_DIR}")

features/testbot/matching.feature

@@ -626,3 +626,61 @@ Feature: Basic Map Matching
         | trace    | timestamps                                   | matchings  | code |
         | abbecd   | 10 11 27 1516914902 1516914913 1516914952    | ab,ecd     | Ok   |
 
+    Scenario: Regression test - waypoints trimming too much geometry
+        # fixes bug in map matching collapsing that was dropping path geometries
+        # after segments that had 0 distance in internal route results
+        Given the node map
+            """
+            ad
+            |
+            |
+            |
+            |
+            |e   g
+            b--------------c
+            f              h
+            """
+
+        And the ways
+            | nodes |
+            | ab    |
+            | bc    |
+
+        Given the query options
+            | waypoints | 0;4   |
+            | overview  | full  |
+
+        When I match I should get
+            | trace    | geometry                           | code |
+            | defgh    | 1,1,1,0.999461,1.000674,0.999461   | Ok   |
+
+    @match @testbot
+    Scenario: Regression test - waypoints trimming too much geometry
+        Given the profile "testbot"
+        Given a grid size of 10 meters
+        Given the query options
+          | geometries | geojson |
+        Given the node map
+          """
+            bh
+             |
+             |
+             |
+             c
+             g\
+               \k
+                \
+                 \
+                  \
+                 j f
+          """
+        And the ways
+          | nodes |
+          | hc    |
+          | cf    |
+        Given the query options
+          | waypoints | 0;3  |
+          | overview  | full |
+        When I match I should get
+          | trace | geometry                                      | code |
+          | bgkj  | 1.000135,1,1.000135,0.99964,1.000387,0.999137 | Ok   |

include/engine/internal_route_result.hpp

@@ -136,14 +136,11 @@ inline InternalRouteResult CollapseInternalRouteResult(const InternalRouteResult
         {
             BOOST_ASSERT(!collapsed.unpacked_path_segments.empty());
             auto &last_segment = collapsed.unpacked_path_segments.back();
-            // deduplicate last segment (needs to be checked for empty for the same node query edge
-            // case)
-            if (!last_segment.empty())
-                last_segment.pop_back();
-            // update target phantom node of leg
             BOOST_ASSERT(!collapsed.segment_end_coordinates.empty());
             collapsed.segment_end_coordinates.back().target_phantom =
                 leggy_result.segment_end_coordinates[i].target_phantom;
+            collapsed.target_traversed_in_reverse.back() =
+                leggy_result.target_traversed_in_reverse[i];
             // copy path segments into current leg
             last_segment.insert(last_segment.end(),
                                 leggy_result.unpacked_path_segments[i].begin(),

include/nodejs/node_osrm_support.hpp

@@ -1171,7 +1171,7 @@ argumentsToMatchParameter(const Nan::FunctionCallbackInfo<v8::Value> &args,
                 Nan::ThrowError("Timestamps array items must be numbers");
                 return match_parameters_ptr();
             }
-            params->timestamps.emplace_back(static_cast<unsigned>(timestamp->NumberValue()));
+            params->timestamps.emplace_back(static_cast<std::size_t>(timestamp->NumberValue()));
         }
     }
 
@@ -1220,6 +1220,60 @@ argumentsToMatchParameter(const Nan::FunctionCallbackInfo<v8::Value> &args,
         params->tidy = tidy->BooleanValue();
     }
 
+    if (obj->Has(Nan::New("waypoints").ToLocalChecked()))
+    {
+        v8::Local<v8::Value> waypoints = obj->Get(Nan::New("waypoints").ToLocalChecked());
+        if (waypoints.IsEmpty())
+            return match_parameters_ptr();
+
+        // must be array
+        if (!waypoints->IsArray())
+        {
+            Nan::ThrowError(
+                "Waypoints must be an array of integers corresponding to the input coordinates.");
+            return match_parameters_ptr();
+        }
+
+        auto waypoints_array = v8::Local<v8::Array>::Cast(waypoints);
+        // must have at least two elements
+        if (waypoints_array->Length() < 2)
+        {
+            Nan::ThrowError("At least two waypoints must be provided");
+            return match_parameters_ptr();
+        }
+        auto coords_size = params->coordinates.size();
+        auto waypoints_array_size = waypoints_array->Length();
+
+        const auto first_index = Nan::To<std::uint32_t>(waypoints_array->Get(0)).FromJust();
+        const auto last_index =
+            Nan::To<std::uint32_t>(waypoints_array->Get(waypoints_array_size - 1)).FromJust();
+        if (first_index != 0 || last_index != coords_size - 1)
+        {
+            Nan::ThrowError("First and last waypoints values must correspond to first and last "
+                            "coordinate indices");
+            return match_parameters_ptr();
+        }
+
+        for (uint32_t i = 0; i < waypoints_array_size; ++i)
+        {
+            v8::Local<v8::Value> waypoint_value = waypoints_array->Get(i);
+            // all elements must be numbers
+            if (!waypoint_value->IsNumber())
+            {
+                Nan::ThrowError("Waypoint values must be an array of integers");
+                return match_parameters_ptr();
+            }
+            // check that the waypoint index corresponds with an inpute coordinate
+            const auto index = Nan::To<std::uint32_t>(waypoint_value).FromJust();
+            if (index >= coords_size)
+            {
+                Nan::ThrowError("Waypoints must correspond with the index of an input coordinate");
+                return match_parameters_ptr();
+            }
+            params->waypoints.emplace_back(static_cast<unsigned>(waypoint_value->NumberValue()));
+        }
+    }
+
     bool parsedSuccessfully = parseCommonParameters(obj, params);
     if (!parsedSuccessfully)
     {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "osrm",
-  "version": "5.15.1",
+  "version": "5.15.3",
   "private": false,
   "description": "The Open Source Routing Machine is a high performance routing engine written in C++14 designed to run on OpenStreetMap data.",
   "dependencies": {

test/nodejs/match.js

@@ -238,3 +238,89 @@ test('match: match in Monaco without motorways', function(assert) {
         assert.equal(response.matchings.length, 1);
     });
 });
+
+test('match: throws on invalid waypoints values needs at least two', function(assert) {
+    assert.plan(1);
+    var osrm = new OSRM(data_path);
+    var options = {
+        steps: true,
+        coordinates: three_test_coordinates,
+        waypoints: [0]
+    };
+    assert.throws(function() { osrm.match(options, function(err, response) {}); },
+        'At least two waypoints must be provided');
+});
+
+test('match: throws on invalid waypoints values, needs first and last coordinate indices', function(assert) {
+    assert.plan(1);
+    var osrm = new OSRM(data_path);
+    var options = {
+        steps: true,
+        coordinates: three_test_coordinates,
+        waypoints: [1, 2]
+    };
+    assert.throws(function() { osrm.match(options, function(err, response) {console.log(err);}); },
+        'First and last waypoints values must correspond to first and last coordinate indices');
+});
+
+test('match: throws on invalid waypoints values, order matters', function(assert) {
+    assert.plan(1);
+    var osrm = new OSRM(data_path);
+    var options = {
+        steps: true,
+        coordinates: three_test_coordinates,
+        waypoints: [2, 0]
+    };
+    assert.throws(function() { osrm.match(options, function(err, response) {console.log(err);}); },
+        'First and last waypoints values must correspond to first and last coordinate indices');
+});
+
+test('match: throws on invalid waypoints values, waypoints must correspond with a coordinate index', function(assert) {
+    assert.plan(1);
+    var osrm = new OSRM(data_path);
+    var options = {
+        steps: true,
+        coordinates: three_test_coordinates,
+        waypoints: [0, 3, 2]
+    };
+    assert.throws(function() { osrm.match(options, function(err, response) {console.log(err);}); },
+        'Waypoints must correspond with the index of an input coordinate');
+});
+
+test('match: error on split trace', function(assert) {
+    assert.plan(1);
+    var osrm = new OSRM(data_path);
+    var four_coords = Array.from(three_test_coordinates);
+    four_coords.push([7.41902,43.73487]);
+    var options = {
+        steps: true,
+        coordinates: four_coords,
+        timestamps: [1700, 1750, 1424684616, 1424684620],
+        waypoints: [0,3]
+    };
+    osrm.match(options, function(err, response) {
+        assert.ok(err, 'Errors with NoMatch');
+    });
+});
+
+test('match: match in Monaco with waypoints', function(assert) {
+    assert.plan(6);
+    var osrm = new OSRM(data_path);
+    var options = {
+        steps: true,
+        coordinates: three_test_coordinates,
+        waypoints: [0,2]
+    };
+    osrm.match(options, function(err, response) {
+        assert.ifError(err);
+        assert.equal(response.matchings.length, 1);
+        assert.equal(response.matchings[0].legs.length, 1);
+        assert.ok(response.matchings.every(function(m) {
+            return !!m.distance && !!m.duration && Array.isArray(m.legs) && !!m.geometry && m.confidence > 0;
+        }))
+        assert.equal(response.tracepoints.length, 3);
+        assert.ok(response.tracepoints.every(function(t) {
+            return !!t.hint && !isNaN(t.matchings_index) && !isNaN(t.waypoint_index) && !!t.name;
+        }));
+    });
+});

unit_tests/engine/collapse_internal_route_result.cpp

@@ -38,6 +38,10 @@ BOOST_AUTO_TEST_CASE(unchanged_collapse_route_result)
 
 BOOST_AUTO_TEST_CASE(two_legs_to_one_leg)
 {
+    // turn_via_node, name_id, is_segregated, weight_until_turn,
+    // weight_of_turn,
+    // duration_until_turn, duration_of_turn, turn_instruction, lane_data, travel_mode, classes,
+    // entry_class, datasource_id, pre_turn_bearing, post_turn_bearing, left_hand
     PathData pathy{2, 17, false, 2, 3, 4, 5, 0, {}, 4, 2, {}, 2, {1.0}, {1.0}, false};
     PathData kathy{1, 16, false, 1, 2, 3, 4, 1, {}, 3, 1, {}, 1, {2.0}, {3.0}, false};
     PathData cathy{3, 16, false, 1, 2, 3, 4, 1, {}, 3, 1, {}, 1, {2.0}, {3.0}, false};
@@ -61,10 +65,11 @@ BOOST_AUTO_TEST_CASE(two_legs_to_one_leg)
     BOOST_CHECK_EQUAL(collapsed.segment_end_coordinates[0].target_phantom.forward_segment_id.id,
                       12);
     BOOST_CHECK_EQUAL(collapsed.segment_end_coordinates[0].source_phantom.forward_segment_id.id, 1);
-    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0].size(), 3);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0].size(), 4);
     BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][0].turn_via_node, 2);
     BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][1].turn_via_node, 1);
-    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][2].turn_via_node, 3);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][2].turn_via_node, 1);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][3].turn_via_node, 3);
 }
 
 BOOST_AUTO_TEST_CASE(three_legs_to_two_legs)
@@ -101,13 +106,14 @@ BOOST_AUTO_TEST_CASE(three_legs_to_two_legs)
     BOOST_CHECK_EQUAL(collapsed.segment_end_coordinates[1].target_phantom.forward_segment_id.id,
                       18);
     BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0].size(), 2);
-    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1].size(), 4);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1].size(), 5);
     BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][0].turn_via_node, 2);
     BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[0][1].turn_via_node, 1);
     BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][0].turn_via_node, 1);
     BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][1].turn_via_node, 5);
     BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][2].turn_via_node, 3);
-    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][3].turn_via_node, 4);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][3].turn_via_node, 3);
+    BOOST_CHECK_EQUAL(collapsed.unpacked_path_segments[1][4].turn_via_node, 4);
 }
 
 BOOST_AUTO_TEST_CASE(two_legs_to_two_legs)