Add waypoints parameter to viaroute API (#5345)

danpat · web-flow · commit 381d492a8fbd · 2019-01-24T16:19:59.000-08:00
* Add silent waypoints to viaroute API.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,7 @@
 # Unreleased
+  - Changes from 5.21.0
+    - Features:
+      - ADDED: new waypoints parameter to the `route` plugin, enabling silent waypoints [#5345](https://github.com/Project-OSRM/osrm-backend/pull/5345)
 
 # 5.21.0
   - Changes from 5.20.0
diff --git a/docs/http.md b/docs/http.md
@@ -195,6 +195,7 @@ In addition to the [general options](#general-options) the following options are
 |geometries  |`polyline` (default), `polyline6`, `geojson` |Returned route geometry format (influences overview and per step)              |
 |overview    |`simplified` (default), `full`, `false`      |Add overview geometry either full, simplified according to highest zoom level it could be display on, or not at all.|
 |continue\_straight |`default` (default), `true`, `false` |Forces the route to keep going straight at waypoints constraining uturns there even if it would be faster. Default value depends on the profile. |
+|waypoints   | `{index};{index};{index}...`                   |Treats input coordinates indicated by given indices as waypoints in returned Match object. Default is to treat all input coordinates as waypoints.    |
 
 \* Please note that even if alternative routes are requested, a result cannot be guaranteed.
 
diff --git a/docs/nodejs/api.md b/docs/nodejs/api.md
@@ -57,6 +57,7 @@ Returns the fastest route between two or more coordinates while visiting the way
     -   `options.overview` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)** Add overview geometry either `full`, `simplified` according to highest zoom level it could be display on, or not at all (`false`). (optional, default `simplified`)
     -   `options.continue_straight` **[Boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)?** Forces the route to keep going straight at waypoints and don't do a uturn even if it would be faster. Default value depends on the profile.
     -   `options.approaches` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Keep waypoints on curb side. Can be `null` (unrestricted, default) or `curb`.
+    -   `options.waypoints` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Indices to coordinates to treat as waypoints.  If not supplied, all coordinates are waypoints.  Must include first and last coordinate index.
                          `null`/`true`/`false`
 -   `callback` **[Function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function)**
 
@@ -210,6 +211,7 @@ if they can not be matched successfully.
     -   `options.radiuses` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Standard deviation of GPS precision used for map matching. If applicable use GPS accuracy. Can be `null` for default value `5` meters or `double >= 0`.
     -   `options.gaps` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)?** Allows the input track splitting based on huge timestamp gaps between points. Either `split` or `ignore` (optional, default `split`).
     -   `options.tidy` **[Boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)?** Allows the input track modification to obtain better matching quality for noisy tracks (optional, default `false`).
+    -   `options.waypoints` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Indices to coordinates to treat as waypoints.  If not supplied, all coordinates are waypoints.  Must include first and last coordinate index.
 -   `callback` **[Function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function)**
 
 **Examples**
diff --git a/features/testbot/via.feature b/features/testbot/via.feature
@@ -18,6 +18,53 @@ Feature: Via points
             | waypoints | route           |
             | a,b,c     | abc,abc,abc,abc |
 
+    Scenario: Simple via point with waypoints collapsing
+        Given the node map
+            """
+                 a
+
+            b   1c    d
+                 2
+
+                 e
+            """
+
+        And the ways
+            | nodes |
+            | ace   |
+            | bcd   |
+
+       Given the query options
+            | waypoints | 0;2   |
+
+        When I route I should get
+            | waypoints | route       | turns                    |
+            | b,1,e     | bcd,ace,ace | depart,turn right,arrive |
+            | b,2,e     | bcd,ace,ace | depart,turn right,arrive |
+
+    Scenario: Simple via point with waypoints collapsing
+        Given the node map
+            """
+            a  2  b
+
+            c     d
+             1   3
+            """
+
+        And the ways
+            | nodes |
+            | ab   |
+            | bd   |
+            | cd   |
+            | ac   |
+
+       Given the query options
+            | waypoints | 0;2   |
+
+        When I route I should get
+            | waypoints | route          | turns                                                      |
+            | 1,2,3     | cd,ac,ab,bd,cd | depart,new name right,new name right,new name right,arrive |
+
     Scenario: Simple via point with core factor
         Given the contract extra arguments "--core 0.8"
         Given the node map
diff --git a/include/engine/api/match_parameters.hpp b/include/engine/api/match_parameters.hpp
@@ -63,7 +63,7 @@ struct MatchParameters : public RouteParameters
                           RouteParameters::GeometriesType::Polyline,
                           RouteParameters::OverviewType::Simplified,
                           {}),
-          gaps(GapsType::Split), tidy(false), waypoints()
+          gaps(GapsType::Split), tidy(false)
     {
     }
 
@@ -79,24 +79,19 @@ struct MatchParameters : public RouteParameters
                     bool tidy_,
                     std::vector<std::size_t> waypoints_,
                     Args... args_)
-        : RouteParameters{std::forward<Args>(args_)...}, timestamps{std::move(timestamps_)},
-          gaps(gaps_), tidy(tidy_), waypoints{std::move(waypoints_)}
+        : RouteParameters{std::forward<Args>(args_)..., waypoints_},
+          timestamps{std::move(timestamps_)}, gaps(gaps_), tidy(tidy_)
     {
     }
 
     std::vector<unsigned> timestamps;
     GapsType gaps;
     bool tidy;
-    std::vector<std::size_t> waypoints;
 
     bool IsValid() const
     {
-        const auto valid_waypoints =
-            std::all_of(waypoints.begin(), waypoints.end(), [this](const auto &w) {
-                return w < coordinates.size();
-            });
         return RouteParameters::IsValid() &&
-               (timestamps.empty() || timestamps.size() == coordinates.size()) && valid_waypoints;
+               (timestamps.empty() || timestamps.size() == coordinates.size());
     }
 };
 }
diff --git a/include/engine/api/route_api.hpp b/include/engine/api/route_api.hpp
@@ -44,8 +44,11 @@ class RouteAPI : public BaseAPI
     {
     }
 
-    void MakeResponse(const InternalManyRoutesResult &raw_routes,
-                      util::json::Object &response) const
+    void
+    MakeResponse(const InternalManyRoutesResult &raw_routes,
+                 const std::vector<PhantomNodes>
+                     &all_start_end_points, // all used coordinates, ignoring waypoints= parameter
+                 util::json::Object &response) const
     {
         BOOST_ASSERT(!raw_routes.routes.empty());
 
@@ -62,8 +65,7 @@ class RouteAPI : public BaseAPI
                                                 route.target_traversed_in_reverse));
         }
 
-        response.values["waypoints"] =
-            BaseAPI::MakeWaypoints(raw_routes.routes[0].segment_end_coordinates);
+        response.values["waypoints"] = BaseAPI::MakeWaypoints(all_start_end_points);
         response.values["routes"] = std::move(jsRoutes);
         response.values["code"] = "Ok";
     }
diff --git a/include/engine/api/route_parameters.hpp b/include/engine/api/route_parameters.hpp
@@ -98,7 +98,8 @@ struct RouteParameters : public BaseParameters
           annotations_type{AnnotationsType::None},
           geometries{geometries_},
           overview{overview_},
-          continue_straight{continue_straight_}
+          continue_straight{continue_straight_},
+          waypoints()
     {
     }
 
@@ -114,7 +115,9 @@ struct RouteParameters : public BaseParameters
         : BaseParameters{std::forward<Args>(args_)...}, steps{steps_}, alternatives{alternatives_},
           number_of_alternatives{alternatives_ ? 1u : 0u}, annotations{annotations_},
           annotations_type{annotations_ ? AnnotationsType::All : AnnotationsType::None},
-          geometries{geometries_}, overview{overview_}, continue_straight{continue_straight_}
+          geometries{geometries_}, overview{overview_}, continue_straight{continue_straight_},
+          waypoints()
+
     {
     }
 
@@ -131,7 +134,43 @@ struct RouteParameters : public BaseParameters
           number_of_alternatives{alternatives_ ? 1u : 0u},
           annotations{annotations_ == AnnotationsType::None ? false : true},
           annotations_type{annotations_}, geometries{geometries_}, overview{overview_},
-          continue_straight{continue_straight_}
+          continue_straight{continue_straight_}, waypoints()
+    {
+    }
+
+    // RouteParameters constructor adding the `waypoints` parameter
+    template <typename... Args>
+    RouteParameters(const bool steps_,
+                    const bool alternatives_,
+                    const bool annotations_,
+                    const GeometriesType geometries_,
+                    const OverviewType overview_,
+                    const boost::optional<bool> continue_straight_,
+                    std::vector<std::size_t> waypoints_,
+                    const Args... args_)
+        : BaseParameters{std::forward<Args>(args_)...}, steps{steps_}, alternatives{alternatives_},
+          number_of_alternatives{alternatives_ ? 1u : 0u}, annotations{annotations_},
+          annotations_type{annotations_ ? AnnotationsType::All : AnnotationsType::None},
+          geometries{geometries_}, overview{overview_}, continue_straight{continue_straight_},
+          waypoints{waypoints_}
+    {
+    }
+
+    // RouteParameters constructor adding the `waypoints` parameter
+    template <typename... Args>
+    RouteParameters(const bool steps_,
+                    const bool alternatives_,
+                    const AnnotationsType annotations_,
+                    const GeometriesType geometries_,
+                    const OverviewType overview_,
+                    const boost::optional<bool> continue_straight_,
+                    std::vector<std::size_t> waypoints_,
+                    Args... args_)
+        : BaseParameters{std::forward<Args>(args_)...}, steps{steps_}, alternatives{alternatives_},
+          number_of_alternatives{alternatives_ ? 1u : 0u},
+          annotations{annotations_ == AnnotationsType::None ? false : true},
+          annotations_type{annotations_}, geometries{geometries_}, overview{overview_},
+          continue_straight{continue_straight_}, waypoints{waypoints_}
     {
     }
 
@@ -144,12 +183,17 @@ struct RouteParameters : public BaseParameters
     GeometriesType geometries = GeometriesType::Polyline;
     OverviewType overview = OverviewType::Simplified;
     boost::optional<bool> continue_straight;
+    std::vector<std::size_t> waypoints;
 
     bool IsValid() const
     {
         const auto coordinates_ok = coordinates.size() >= 2;
         const auto base_params_ok = BaseParameters::IsValid();
-        return coordinates_ok && base_params_ok;
+        const auto valid_waypoints =
+            std::all_of(waypoints.begin(), waypoints.end(), [this](const auto &w) {
+                return w < coordinates.size();
+            });
+        return coordinates_ok && base_params_ok && valid_waypoints;
     }
 };
 
@@ -173,8 +217,8 @@ inline RouteParameters::AnnotationsType operator|=(RouteParameters::AnnotationsT
 {
     return lhs = lhs | rhs;
 }
-}
-}
-}
+} // ns api
+} // ns engine
+} // ns osrm
 
 #endif
diff --git a/include/nodejs/node_osrm_support.hpp b/include/nodejs/node_osrm_support.hpp
@@ -944,6 +944,72 @@ argumentsToRouteParameter(const Nan::FunctionCallbackInfo<v8::Value> &args,
         }
     }
 
+    if (obj->Has(Nan::New("waypoints").ToLocalChecked()))
+    {
+        v8::Local<v8::Value> waypoints = obj->Get(Nan::New("waypoints").ToLocalChecked());
+        if (waypoints.IsEmpty())
+            return route_parameters_ptr();
+
+        // must be array
+        if (!waypoints->IsArray())
+        {
+            Nan::ThrowError(
+                "Waypoints must be an array of integers corresponding to the input coordinates.");
+            return route_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 route_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 route_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 route_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 route_parameters_ptr();
+            }
+            params->waypoints.emplace_back(static_cast<unsigned>(waypoint_value->NumberValue()));
+        }
+
+        if (!params->waypoints.empty())
+        {
+            for (std::size_t i = 0; i < params->waypoints.size() - 1; i++)
+            {
+                if (params->waypoints[i] >= params->waypoints[i + 1])
+                {
+                    Nan::ThrowError("Waypoints must be supplied in increasing order");
+                    return route_parameters_ptr();
+                }
+            }
+        }
+    }
+
     bool parsedSuccessfully = parseCommonParameters(obj, params);
     if (!parsedSuccessfully)
     {
diff --git a/include/server/api/match_parameter_grammar.hpp b/include/server/api/match_parameter_grammar.hpp
@@ -42,17 +42,12 @@ struct MatchParametersGrammar final : public RouteParametersGrammar<Iterator, Si
             (qi::uint_ %
              ';')[ph::bind(&engine::api::MatchParameters::timestamps, qi::_r1) = qi::_1];
 
-        waypoints_rule =
-            qi::lit("waypoints=") >
-            (size_t_ % ';')[ph::bind(&engine::api::MatchParameters::waypoints, qi::_r1) = qi::_1];
-
         gaps_type.add("split", engine::api::MatchParameters::GapsType::Split)(
             "ignore", engine::api::MatchParameters::GapsType::Ignore);
 
         root_rule =
             BaseGrammar::query_rule(qi::_r1) > -qi::lit(".json") >
             -('?' > (timestamps_rule(qi::_r1) | BaseGrammar::base_rule(qi::_r1) |
-                     waypoints_rule(qi::_r1) |
                      (qi::lit("gaps=") >
                       gaps_type[ph::bind(&engine::api::MatchParameters::gaps, qi::_r1) = qi::_1]) |
                      (qi::lit("tidy=") >
@@ -63,7 +58,6 @@ struct MatchParametersGrammar final : public RouteParametersGrammar<Iterator, Si
   private:
     qi::rule<Iterator, Signature> root_rule;
     qi::rule<Iterator, Signature> timestamps_rule;
-    qi::rule<Iterator, Signature> waypoints_rule;
     qi::rule<Iterator, std::size_t()> size_t_;
 
     qi::symbols<char, engine::api::MatchParameters::GapsType> gaps_type;
diff --git a/include/server/api/route_parameters_grammar.hpp b/include/server/api/route_parameters_grammar.hpp
@@ -48,6 +48,14 @@ struct RouteParametersGrammar : public BaseParametersGrammar<Iterator, Signature
 
     RouteParametersGrammar(qi::rule<Iterator, Signature> &root_rule_) : BaseGrammar(root_rule_)
     {
+#ifdef BOOST_HAS_LONG_LONG
+        if (std::is_same<std::size_t, unsigned long long>::value)
+            size_t_ = qi::ulong_long;
+        else
+            size_t_ = qi::ulong_;
+#else
+        size_t_ = qi::ulong_;
+#endif
         using AnnotationsType = engine::api::RouteParameters::AnnotationsType;
 
         const auto add_annotation = [](engine::api::RouteParameters &route_parameters,
@@ -70,8 +78,12 @@ struct RouteParametersGrammar : public BaseParametersGrammar<Iterator, Signature
             "distance", AnnotationsType::Distance)("weight", AnnotationsType::Weight)(
             "datasources", AnnotationsType::Datasources)("speed", AnnotationsType::Speed);
 
+        waypoints_rule =
+            qi::lit("waypoints=") >
+            (size_t_ % ';')[ph::bind(&engine::api::RouteParameters::waypoints, qi::_r1) = qi::_1];
+
         base_rule =
-            BaseGrammar::base_rule(qi::_r1) |
+            BaseGrammar::base_rule(qi::_r1) | waypoints_rule(qi::_r1) |
             (qi::lit("steps=") >
              qi::bool_[ph::bind(&engine::api::RouteParameters::steps, qi::_r1) = qi::_1]) |
             (qi::lit("geometries=") >
@@ -94,6 +106,8 @@ struct RouteParametersGrammar : public BaseParametersGrammar<Iterator, Signature
   private:
     qi::rule<Iterator, Signature> root_rule;
     qi::rule<Iterator, Signature> route_rule;
+    qi::rule<Iterator, Signature> waypoints_rule;
+    qi::rule<Iterator, std::size_t()> size_t_;
 
     qi::symbols<char, engine::api::RouteParameters::GeometriesType> geometries_type;
     qi::symbols<char, engine::api::RouteParameters::OverviewType> overview_type;
diff --git a/src/engine/plugins/viaroute.cpp b/src/engine/plugins/viaroute.cpp
diff --git a/test/nodejs/match.js b/test/nodejs/match.js
diff --git a/test/nodejs/route.js b/test/nodejs/route.js

Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,7 @@ struct MatchParameters : public RouteParameters`
`63`	`63`	`RouteParameters::GeometriesType::Polyline,`
`64`	`64`	`RouteParameters::OverviewType::Simplified,`
`65`	`65`	`{}),`
`66`		`- gaps(GapsType::Split), tidy(false), waypoints()`
	`66`	`+ gaps(GapsType::Split), tidy(false)`
`67`	`67`	`{`
`68`	`68`	`}`
`69`	`69`
`@@ -79,24 +79,19 @@ struct MatchParameters : public RouteParameters`
`79`	`79`	`bool tidy_,`
`80`	`80`	`std::vector<std::size_t> waypoints_,`
`81`	`81`	`Args... args_)`
`82`		`- : RouteParameters{std::forward<Args>(args_)...}, timestamps{std::move(timestamps_)},`
`83`		`- gaps(gaps_), tidy(tidy_), waypoints{std::move(waypoints_)}`
	`82`	`+ : RouteParameters{std::forward<Args>(args_)..., waypoints_},`
	`83`	`+ timestamps{std::move(timestamps_)}, gaps(gaps_), tidy(tidy_)`
`84`	`84`	`{`
`85`	`85`	`}`
`86`	`86`
`87`	`87`	`std::vector<unsigned> timestamps;`
`88`	`88`	`GapsType gaps;`
`89`	`89`	`bool tidy;`
`90`		`- std::vector<std::size_t> waypoints;`
`91`	`90`
`92`	`91`	`bool IsValid() const`
`93`	`92`	`{`
`94`		`- const auto valid_waypoints =`
`95`		`- std::all_of(waypoints.begin(), waypoints.end(), [this](const auto &w) {`
`96`		`- return w < coordinates.size();`
`97`		`- });`
`98`	`93`	`return RouteParameters::IsValid() &&`
`99`		`- (timestamps.empty() \|\| timestamps.size() == coordinates.size()) && valid_waypoints;`
	`94`	`+ (timestamps.empty() \|\| timestamps.size() == coordinates.size());`
`100`	`95`	`}`
`101`	`96`	`};`
`102`	`97`	`}`
Original file line number	Diff line number	Diff line change
`@@ -44,8 +44,11 @@ class RouteAPI : public BaseAPI`
`44`	`44`	`{`
`45`	`45`	`}`
`46`	`46`
`47`		`- void MakeResponse(const InternalManyRoutesResult &raw_routes,`
`48`		`- util::json::Object &response) const`
	`47`	`+ void`
	`48`	`+ MakeResponse(const InternalManyRoutesResult &raw_routes,`
	`49`	`+ const std::vector<PhantomNodes>`
	`50`	`+ &all_start_end_points, // all used coordinates, ignoring waypoints= parameter`
	`51`	`+ util::json::Object &response) const`
`49`	`52`	`{`
`50`	`53`	`BOOST_ASSERT(!raw_routes.routes.empty());`
`51`	`54`
`@@ -62,8 +65,7 @@ class RouteAPI : public BaseAPI`
`62`	`65`	`route.target_traversed_in_reverse));`
`63`	`66`	`}`
`64`	`67`
`65`		`- response.values["waypoints"] =`
`66`		`- BaseAPI::MakeWaypoints(raw_routes.routes[0].segment_end_coordinates);`
	`68`	`+ response.values["waypoints"] = BaseAPI::MakeWaypoints(all_start_end_points);`
`67`	`69`	`response.values["routes"] = std::move(jsRoutes);`
`68`	`70`	`response.values["code"] = "Ok";`
`69`	`71`	`}`