Lets the user specify us dropping hints in response, resolves #1789.

Adds an `?emit_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 15, 2016 · dc8b75b · dc8b75b
1 parent 9832825
commit dc8b75b
Show file tree

Hide file tree

Showing 11 changed files with 74 additions and 17 deletions.
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.emit_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 emit_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)] %
                          ';';
 
+        emit_hints_rule =
+            qi::lit("emit_hints=") >
+            qi::bool_[ph::bind(&engine::api::BaseParameters::emit_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) //
+                    | emit_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> emit_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_emitting_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.emit_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,7 @@ 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?emit_hints=notboolean"), 19UL);
     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 +297,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 emit Hints when they are explicitely disabled
+    auto result_11 = parseParameters<RouteParameters>("1,2;3,4?emit_hints=false");
+    BOOST_CHECK(result_11);
+    BOOST_CHECK_EQUAL(result_11->emit_hints, false);
+
+    auto result_12 = parseParameters<RouteParameters>("1,2;3,4?emit_hints=true");
+    BOOST_CHECK(result_12);
+    BOOST_CHECK_EQUAL(result_12->emit_hints, true);
+
+    auto result_13 = parseParameters<RouteParameters>("1,2;3,4");
+    BOOST_CHECK(result_13);
+    BOOST_CHECK_EQUAL(result_13->emit_hints, true);
 }
 
 BOOST_AUTO_TEST_CASE(valid_table_urls)