Adds generate_hints=true for dropping hints in response, resolves #…

…1789. Adds an `generate_hints=false` option which lets us skip generating and emitting hints for Waypoints. This can be used to decrease the response size when the user does not need hints anyway. We should think about making `false` the default here in v6.
Project-OSRM · Dec 19, 2016 · b1f6797 · b1f6797
1 parent f04d146
commit b1f6797
Show file tree

Hide file tree

Showing 13 changed files with 83 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,7 @@
 # 5.5.1
   - Changes from 5.5.0
+    - API:
+      - Adds `generate_hints=true` (`true` by default) which lets user disable `Hint` generating in the response. Use if you don't need `Hint`s!
     - Bugfixes
       - Fix #3418 and ensure we only return bearings in the range 0-359 in API responses
       - Fixed a bug that could lead to emitting false instructions for staying on a roundabout

diff --git a/docs/http.md b/docs/http.md
@@ -24,11 +24,12 @@ To pass parameters to each location some options support an array like encoding:
 
 **Request options**
 
-| Option     | Values                                                 | Description                                                                                           |
-|------------|--------------------------------------------------------|-------------------------------------------------------------------------------------------------------|
-|bearings    |`{bearing};{bearing}[;{bearing} ...]`                   |Limits the search to segments with given bearing in degrees towards true north in clockwise direction. |
-|radiuses    |`{radius};{radius}[;{radius} ...]`                      |Limits the search to given radius in meters.                                                           |
-|hints       |`{hint};{hint}[;{hint} ...]`                            |Hint from previous request to derive position in street network.                                       |
+| Option         | Values                                                 | Description                                                                                           |
+|----------------|--------------------------------------------------------|-------------------------------------------------------------------------------------------------------|
+|bearings        |`{bearing};{bearing}[;{bearing} ...]`                   |Limits the search to segments with given bearing in degrees towards true north in clockwise direction. |
+|radiuses        |`{radius};{radius}[;{radius} ...]`                      |Limits the search to given radius in meters.                                                           |
+|generate\_hints |`true` (default), `false`                               |Adds a Hint to the response which can be used in subsequent requests, see `hints` parameter.           |
+|hints           |`{hint};{hint}[;{hint} ...]`                            |Hint from previous request to derive position in street network.                                       |
 
 Where the elements follow the following format:
 

diff --git a/include/engine/api/base_api.hpp b/include/engine/api/base_api.hpp
@@ -44,13 +44,19 @@ class BaseAPI
         return waypoints;
     }
 
-    // FIXME gcc 4.8 doesn't support for lambdas to call protected member functions
-    //  protected:
+  protected:
     util::json::Object MakeWaypoint(const PhantomNode &phantom) const
     {
-        return json::makeWaypoint(phantom.location,
-                                  facade.GetNameForID(phantom.name_id),
-                                  Hint{phantom, facade.GetCheckSum()});
+        if (parameters.generate_hints)
+        {
+            return json::makeWaypoint(phantom.location,
+                                      facade.GetNameForID(phantom.name_id),
+                                      Hint{phantom, facade.GetCheckSum()});
+        }
+        else
+        {
+            return json::makeWaypoint(phantom.location, facade.GetNameForID(phantom.name_id));
+        }
     }
 
     const datafacade::BaseDataFacade &facade;

diff --git a/include/engine/api/base_parameters.hpp b/include/engine/api/base_parameters.hpp
@@ -66,6 +66,9 @@ struct BaseParameters
     std::vector<boost::optional<double>> radiuses;
     std::vector<boost::optional<Bearing>> bearings;
 
+    // Adds hints to response which can be included in subsequent requests, see `hints` above.
+    bool generate_hints = true;
+
     // FIXME add validation for invalid bearing values
     bool IsValid() const
     {

diff --git a/include/engine/api/json_factory.hpp b/include/engine/api/json_factory.hpp
@@ -89,6 +89,10 @@ util::json::Object makeRoute(const guidance::Route &route,
                              util::json::Array legs,
                              boost::optional<util::json::Value> geometry);
 
+// Creates a Waypoint without Hint, see the Hint overload below
+util::json::Object makeWaypoint(const util::Coordinate location, std::string name);
+
+// Creates a Waypoint with Hint, see the overload above when Hint is not needed
 util::json::Object
 makeWaypoint(const util::Coordinate location, std::string name, const Hint &hint);
 

diff --git a/include/engine/api/match_api.hpp b/include/engine/api/match_api.hpp
@@ -48,9 +48,7 @@ class MatchAPI final : public RouteAPI
         response.values["code"] = "Ok";
     }
 
-    // FIXME gcc 4.8 doesn't support for lambdas to call protected member functions
-    //  protected:
-
+  protected:
     // FIXME this logic is a little backwards. We should change the output format of the
     // map_matching
     // routing algorithm to be easier to consume here.

diff --git a/include/engine/api/route_api.hpp b/include/engine/api/route_api.hpp
@@ -61,8 +61,7 @@ class RouteAPI : public BaseAPI
         response.values["code"] = "Ok";
     }
 
-    // FIXME gcc 4.8 doesn't support for lambdas to call protected member functions
-    //  protected:
+  protected:
     template <typename ForwardIter>
     util::json::Value MakeGeometry(ForwardIter begin, ForwardIter end) const
     {

diff --git a/include/engine/api/table_api.hpp b/include/engine/api/table_api.hpp
@@ -70,8 +70,7 @@ class TableAPI final : public BaseAPI
         response.values["code"] = "Ok";
     }
 
-    // FIXME gcc 4.8 doesn't support for lambdas to call protected member functions
-    //  protected:
+  protected:
     virtual util::json::Array MakeWaypoints(const std::vector<PhantomNode> &phantoms) const
     {
         util::json::Array json_waypoints;

diff --git a/include/engine/api/trip_api.hpp b/include/engine/api/trip_api.hpp
@@ -47,9 +47,7 @@ class TripAPI final : public RouteAPI
         response.values["code"] = "Ok";
     }
 
-    // FIXME gcc 4.8 doesn't support for lambdas to call protected member functions
-    //  protected:
-
+  protected:
     // FIXME this logic is a little backwards. We should change the output format of the
     // trip plugin routing algorithm to be easier to consume here.
     util::json::Array MakeWaypoints(const std::vector<std::vector<NodeID>> &sub_trips,

diff --git a/include/server/api/base_parameters_grammar.hpp b/include/server/api/base_parameters_grammar.hpp
@@ -141,11 +141,18 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar<Iterator, Signature>
                          add_hint, qi::_r1, qi::_1)] %
                          ';';
 
+        generate_hints_rule =
+            qi::lit("generate_hints=") >
+            qi::bool_[ph::bind(&engine::api::BaseParameters::generate_hints, qi::_r1) = qi::_1];
+
         bearings_rule =
             qi::lit("bearings=") >
             (-(qi::short_ > ',' > qi::short_))[ph::bind(add_bearing, qi::_r1, qi::_1)] % ';';
 
-        base_rule = radiuses_rule(qi::_r1) | hints_rule(qi::_r1) | bearings_rule(qi::_r1);
+        base_rule = radiuses_rule(qi::_r1)   //
+                    | hints_rule(qi::_r1)    //
+                    | bearings_rule(qi::_r1) //
+                    | generate_hints_rule(qi::_r1);
     }
 
   protected:
@@ -157,6 +164,8 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar<Iterator, Signature>
     qi::rule<Iterator, Signature> radiuses_rule;
     qi::rule<Iterator, Signature> hints_rule;
 
+    qi::rule<Iterator, Signature> generate_hints_rule;
+
     qi::rule<Iterator, osrm::engine::Bearing()> bearing_rule;
     qi::rule<Iterator, osrm::util::Coordinate()> location_rule;
     qi::rule<Iterator, std::vector<osrm::util::Coordinate>()> polyline_rule;

diff --git a/src/engine/api/json_factory.cpp b/src/engine/api/json_factory.cpp
@@ -288,11 +288,17 @@ util::json::Object makeRoute(const guidance::Route &route,
     return json_route;
 }
 
-util::json::Object makeWaypoint(const util::Coordinate location, std::string name, const Hint &hint)
+util::json::Object makeWaypoint(const util::Coordinate location, std::string name)
 {
     util::json::Object waypoint;
     waypoint.values["location"] = detail::coordinateToLonLat(location);
     waypoint.values["name"] = std::move(name);
+    return waypoint;
+}
+
+util::json::Object makeWaypoint(const util::Coordinate location, std::string name, const Hint &hint)
+{
+    auto waypoint = makeWaypoint(location, name);
     waypoint.values["hint"] = hint.ToBase64();
     return waypoint;
 }

diff --git a/unit_tests/library/route.cpp b/unit_tests/library/route.cpp
@@ -361,4 +361,25 @@ BOOST_AUTO_TEST_CASE(test_route_response_for_locations_across_components)
     }
 }
 
+BOOST_AUTO_TEST_CASE(test_route_user_disables_generating_hints)
+{
+    const auto args = get_args();
+    auto osrm = getOSRM(args.at(0));
+
+    using namespace osrm;
+
+    RouteParameters params;
+    params.steps = true;
+    params.coordinates.push_back(get_dummy_location());
+    params.coordinates.push_back(get_dummy_location());
+    params.generate_hints = false;
+
+    json::Object result;
+    const auto rc = osrm.Route(params, result);
+    BOOST_CHECK(rc == Status::Ok);
+
+    for (auto waypoint : result.values["waypoints"].get<json::Array>().values)
+        BOOST_CHECK_EQUAL(waypoint.get<json::Object>().values.count("hint"), 0);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/unit_tests/server/parameters_parser.cpp b/unit_tests/server/parameters_parser.cpp
@@ -49,6 +49,8 @@ BOOST_AUTO_TEST_CASE(invalid_route_urls)
                       29UL);
     BOOST_CHECK_EQUAL(testInvalidOptions<RouteParameters>("1,2;3,4?overview=false&hints=;;; ;"),
                       32UL);
+    BOOST_CHECK_EQUAL(testInvalidOptions<RouteParameters>("1,2;3,4?generate_hints=notboolean"),
+                      23UL);
     BOOST_CHECK_EQUAL(testInvalidOptions<RouteParameters>("1,2;3,4?overview=false&geometries=foo"),
                       34UL);
     BOOST_CHECK_EQUAL(testInvalidOptions<RouteParameters>("1,2;3,4?overview=false&overview=foo"),
@@ -296,6 +298,19 @@ BOOST_AUTO_TEST_CASE(valid_route_urls)
     CHECK_EQUAL_RANGE(reference_10.radiuses, result_10->radiuses);
     CHECK_EQUAL_RANGE(reference_10.coordinates, result_10->coordinates);
     CHECK_EQUAL_RANGE(reference_10.hints, result_10->hints);
+
+    // Do not generate Hints when they are explicitely disabled
+    auto result_11 = parseParameters<RouteParameters>("1,2;3,4?generate_hints=false");
+    BOOST_CHECK(result_11);
+    BOOST_CHECK_EQUAL(result_11->generate_hints, false);
+
+    auto result_12 = parseParameters<RouteParameters>("1,2;3,4?generate_hints=true");
+    BOOST_CHECK(result_12);
+    BOOST_CHECK_EQUAL(result_12->generate_hints, true);
+
+    auto result_13 = parseParameters<RouteParameters>("1,2;3,4");
+    BOOST_CHECK(result_13);
+    BOOST_CHECK_EQUAL(result_13->generate_hints, true);
 }
 
 BOOST_AUTO_TEST_CASE(valid_table_urls)