Fix parameter parsing tests

include/engine/api/table_parameters.hpp

@@ -21,6 +21,16 @@ struct TableParameters : public BaseParameters
     std::vector<std::size_t> sources;
     std::vector<std::size_t> destinations;
 
+    TableParameters() = default;
+    template <typename... Args>
+    TableParameters(std::vector<std::size_t> sources_,
+                    std::vector<std::size_t> destinations_,
+                    Args... args_)
+        : BaseParameters{std::forward<Args>(args_)...}, sources{std::move(sources_)},
+          destinations{std::move(destinations_)}
+    {
+    }
+
     bool IsValid() const
     {
         if (!BaseParameters::IsValid())

include/server/api/parameters_parser.hpp

@@ -0,0 +1,35 @@
+#ifndef SERVER_API_ROUTE_PARAMETERS_PARSER_HPP
+#define SERVER_API_ROUTE_PARAMETERS_PARSER_HPP
+
+#include "engine/api/route_parameters.hpp"
+#include "engine/api/table_parameters.hpp"
+
+namespace osrm
+{
+namespace server
+{
+namespace api
+{
+
+// Starts parsing and iter and modifies it until iter == end or parsing failed
+template<typename ParameterT>
+boost::optional<ParameterT> parseParameters(std::string::iterator& iter, std::string::iterator end);
+
+// copy on purpose because we need mutability
+template<typename ParameterT>
+inline boost::optional<ParameterT> parseParameters(std::string options_string)
+{
+    auto iter = options_string.begin();
+    return parseParameters<ParameterT>(iter, options_string.end());
+}
+
+template<>
+boost::optional<engine::api::RouteParameters> parseParameters(std::string::iterator& iter, std::string::iterator end);
+template<>
+boost::optional<engine::api::TableParameters> parseParameters(std::string::iterator& iter, std::string::iterator end);
+
+}
+}
+}
+
+#endif

include/server/api/route_parameters_grammar.hpp

@@ -29,40 +29,40 @@ struct RouteParametersGrammar : public BaseParametersGrammar
     using UturnsT = std::vector<boost::optional<bool>>;
 
     RouteParametersGrammar()
-        : BaseParametersGrammar(root_rule, route_parameters)
+        : BaseParametersGrammar(root_rule, parameters)
     {
         const auto set_geojson_type = [this]()
         {
-            route_parameters.geometries = engine::api::RouteParameters::GeometriesType::GeoJSON;
+            parameters.geometries = engine::api::RouteParameters::GeometriesType::GeoJSON;
         };
         const auto set_polyline_type = [this]()
         {
-            route_parameters.geometries = engine::api::RouteParameters::GeometriesType::Polyline;
+            parameters.geometries = engine::api::RouteParameters::GeometriesType::Polyline;
         };
 
         const auto set_simplified_type = [this]()
         {
-            route_parameters.overview = engine::api::RouteParameters::OverviewType::Simplified;
+            parameters.overview = engine::api::RouteParameters::OverviewType::Simplified;
         };
         const auto set_full_type = [this]()
         {
-            route_parameters.overview = engine::api::RouteParameters::OverviewType::Full;
+            parameters.overview = engine::api::RouteParameters::OverviewType::Full;
         };
         const auto set_false_type = [this]()
         {
-            route_parameters.overview = engine::api::RouteParameters::OverviewType::False;
+            parameters.overview = engine::api::RouteParameters::OverviewType::False;
         };
         const auto set_steps = [this](const StepsT steps)
         {
-            route_parameters.steps = steps;
+            parameters.steps = steps;
         };
         const auto set_alternative = [this](const AlternativeT alternative)
         {
-            route_parameters.alternative = alternative;
+            parameters.alternative = alternative;
         };
         const auto set_uturns = [this](UturnsT &uturns)
         {
-            route_parameters.uturns = std::move(uturns);
+            parameters.uturns = std::move(uturns);
         };
 
         alternative_rule = qi::lit("alternative=") >> qi::bool_;
@@ -78,7 +78,7 @@ struct RouteParametersGrammar : public BaseParametersGrammar
         root_rule = -((base_rule | route_rule) % '&');
     }
 
-    engine::api::RouteParameters route_parameters;
+    engine::api::RouteParameters parameters;
   private:
     qi::rule<Iterator> root_rule, route_rule, geometries_rule, overview_rule;
     qi::rule<Iterator, UturnsT()> uturns_rule;

include/server/api/route_parameters_parser.hpp

@@ -1,28 +0,0 @@
-#ifndef SERVER_API_ROUTE_PARAMETERS_PARSER_HPP
-#define SERVER_API_ROUTE_PARAMETERS_PARSER_HPP
-
-#include "engine/api/route_parameters.hpp"
-#include "server/api/route_parameters_parser.hpp"
-
-namespace osrm
-{
-namespace server
-{
-namespace api
-{
-
-// Starts parsing and iter and modifies it until iter == end or parsing failed
-boost::optional<engine::api::RouteParameters> parseRouteParameters(std::string::iterator& iter, std::string::iterator end);
-
-// copy on purpose because we need mutability
-inline boost::optional<engine::api::RouteParameters> parseRouteParameters(std::string options_string)
-{
-    auto iter = options_string.begin();
-    return parseRouteParameters(iter, options_string.end());
-}
-
-}
-}
-}
-
-#endif

include/server/api/table_parameter_grammar.hpp

@@ -0,0 +1,55 @@
+#ifndef TABLE_PARAMETERS_GRAMMAR_HPP
+#define TABLE_PARAMETERS_GRAMMAR_HPP
+
+#include "engine/api/table_parameters.hpp"
+
+#include "server/api/base_parameters_grammar.hpp"
+
+#include <boost/spirit/include/qi_lit.hpp>
+#include <boost/spirit/include/qi_uint.hpp>
+#include <boost/spirit/include/qi_grammar.hpp>
+#include <boost/spirit/include/qi_action.hpp>
+#include <boost/spirit/include/qi_optional.hpp>
+
+namespace osrm
+{
+namespace server
+{
+namespace api
+{
+
+namespace qi = boost::spirit::qi;
+struct TableParametersGrammar : public BaseParametersGrammar
+{
+    using Iterator = std::string::iterator;
+    using SourcesT = std::vector<std::size_t>;
+    using DestinationsT = std::vector<std::size_t>;
+
+    TableParametersGrammar() : BaseParametersGrammar(root_rule, parameters)
+    {
+        const auto set_destiantions = [this](DestinationsT &dests)
+        {
+            parameters.destinations = std::move(dests);
+        };
+        const auto set_sources = [this](SourcesT &sources)
+        {
+            parameters.sources = std::move(sources);
+        };
+        destinations_rule = qi::lit("destinations=") >> -qi::uint_ % ";";
+        sources_rule = qi::lit("sources=") >> -qi::uint_ % ";";
+        table_rule = destinations_rule[set_destiantions] | sources_rule[set_sources];
+        root_rule = -((base_rule | table_rule) % '&');
+    }
+
+    engine::api::TableParameters parameters;
+
+  private:
+    qi::rule<Iterator> root_rule, table_rule;
+    qi::rule<Iterator, SourcesT()> sources_rule;
+    qi::rule<Iterator, DestinationsT()> destinations_rule;
+};
+}
+}
+}
+
+#endif

include/server/request_handler.hpp

@@ -20,7 +20,7 @@ class RequestHandler
 {
 
   public:
-    RequestHandler();
+    RequestHandler() = default;
     RequestHandler(const RequestHandler &) = delete;
     RequestHandler &operator=(const RequestHandler &) = delete;
 

src/server/api/parameters_parser.cpp

@@ -0,0 +1,46 @@
+#include "server/api/parameters_parser.hpp"
+
+#include "server/api/route_parameters_grammar.hpp"
+#include "server/api/table_parameter_grammar.hpp"
+
+namespace osrm
+{
+namespace server
+{
+namespace api
+{
+
+template<>
+boost::optional<engine::api::RouteParameters> parseParameters(std::string::iterator& iter, std::string::iterator end)
+{
+    RouteParametersGrammar grammar;
+    const auto result = boost::spirit::qi::parse(iter, end, grammar);
+
+    boost::optional<engine::api::RouteParameters> parameters;
+    if (result && iter == end)
+    {
+        parameters = std::move(grammar.parameters);
+    }
+
+    return parameters;
+}
+
+template<>
+boost::optional<engine::api::TableParameters> parseParameters(std::string::iterator& iter, std::string::iterator end)
+{
+    TableParametersGrammar grammar;
+    const auto result = boost::spirit::qi::parse(iter, end, grammar);
+
+    boost::optional<engine::api::TableParameters> parameters;
+    if (result && iter == end)
+    {
+        parameters = std::move(grammar.parameters);
+    }
+
+    return parameters;
+}
+
+}
+}
+}
+

src/server/api/route_parameters_parser.cpp

@@ -1,28 +0,0 @@
-#include "server/api/route_parameters_parser.hpp"
-#include "server/api/route_parameters_grammar.hpp"
-
-namespace osrm
-{
-namespace server
-{
-namespace api
-{
-
-boost::optional<engine::api::RouteParameters> parseRouteParameters(std::string::iterator& iter, std::string::iterator end)
-{
-    RouteParametersGrammar grammar;
-    const auto result = boost::spirit::qi::parse(iter, end, grammar);
-
-    boost::optional<engine::api::RouteParameters> parameters;
-    if (result && iter == end)
-    {
-        parameters = std::move(grammar.route_parameters);
-    }
-
-    return parameters;
-}
-
-}
-}
-}
-

src/server/service/route_service.cpp

@@ -1,7 +1,7 @@
 #include "server/service/route_service.hpp"
 
 #include "engine/api/route_parameters.hpp"
-#include "server/api/route_parameters_parser.hpp"
+#include "server/api/parameters_parser.hpp"
 
 #include "util/json_container.hpp"
 
@@ -55,7 +55,7 @@ engine::Status RouteService::RunQuery(std::vector<util::FixedPointCoordinate> co
 {
 
     auto options_iterator = options.begin();
-    auto parameters = api::parseRouteParameters(options_iterator, options.end());
+    auto parameters = api::parseParameters<engine::api::RouteParameters>(options_iterator, options.end());
     if (!parameters || options_iterator != options.end())
     {
         const auto position = std::distance(options.begin(), options_iterator);

unit_tests/server/parameters_parser.cpp

@@ -1,4 +1,4 @@
-#include "server/api/route_parameters_parser.hpp"
+#include "server/api/parameters_parser.hpp"
 
 #include <fstream>
 
@@ -57,36 +57,53 @@ std::ostream &operator<<(std::ostream &out, api::RouteParameters::Bearing bearin
 #define CHECK_EQUAL_RANGE(R1, R2)                                                                  \
     BOOST_CHECK_EQUAL_COLLECTIONS(R1.begin(), R1.end(), R2.begin(), R2.end());
 
-BOOST_AUTO_TEST_SUITE(api_route_parameters_parser)
+BOOST_AUTO_TEST_SUITE(api_parameters_parser)
 
 using namespace osrm;
 using namespace osrm::server;
 
 // returns distance to front
-std::size_t testInvalidOptions(std::string options)
+template <typename ParameterT> std::size_t testInvalidOptions(std::string options)
 {
     auto iter = options.begin();
-    auto result = api::parseRouteParameters(iter, options.end());
+    auto result = api::parseParameters<ParameterT>(iter, options.end());
     BOOST_CHECK(!result);
     return std::distance(options.begin(), iter);
 }
 
-BOOST_AUTO_TEST_CASE(invalid_urls)
+BOOST_AUTO_TEST_CASE(invalid_route_urls)
 {
-    BOOST_CHECK_EQUAL(testInvalidOptions("overview=false&bla=foo"), 14UL);
+    BOOST_CHECK_EQUAL(testInvalidOptions<engine::api::RouteParameters>("overview=false&bla=foo"),
-    BOOST_CHECK_EQUAL(testInvalidOptions("overview=false&bearings=foo"), 24UL);
+                      14UL);
-    BOOST_CHECK_EQUAL(testInvalidOptions("overview=false&uturns=foo"), 22UL);
+    BOOST_CHECK_EQUAL(
-    BOOST_CHECK_EQUAL(testInvalidOptions("overview=false&radiuses=foo"), 24UL);
+        testInvalidOptions<engine::api::RouteParameters>("overview=false&bearings=foo"), 24UL);
-    BOOST_CHECK_EQUAL(testInvalidOptions("overview=false&hints=foo"), 14UL);
+    BOOST_CHECK_EQUAL(testInvalidOptions<engine::api::RouteParameters>("overview=false&uturns=foo"),
-    BOOST_CHECK_EQUAL(testInvalidOptions("overview=false&geometries=foo"), 14UL);
+                      22UL);
-    BOOST_CHECK_EQUAL(testInvalidOptions("overview=false&overview=foo"), 14UL);
+    BOOST_CHECK_EQUAL(
-    BOOST_CHECK_EQUAL(testInvalidOptions("overview=false&alternative=foo"), 14UL);
+        testInvalidOptions<engine::api::RouteParameters>("overview=false&radiuses=foo"), 24UL);
+    BOOST_CHECK_EQUAL(testInvalidOptions<engine::api::RouteParameters>("overview=false&hints=foo"),
+                      14UL);
+    BOOST_CHECK_EQUAL(
+        testInvalidOptions<engine::api::RouteParameters>("overview=false&geometries=foo"), 14UL);
+    BOOST_CHECK_EQUAL(
+        testInvalidOptions<engine::api::RouteParameters>("overview=false&overview=foo"), 14UL);
+    BOOST_CHECK_EQUAL(
+        testInvalidOptions<engine::api::RouteParameters>("overview=false&alternative=foo"), 14UL);
 }
 
-BOOST_AUTO_TEST_CASE(valid_urls)
+BOOST_AUTO_TEST_CASE(invalid_table_urls)
+{
+    BOOST_CHECK_EQUAL(testInvalidOptions<engine::api::TableParameters>("sources=1&bla=foo"), 9UL);
+    BOOST_CHECK_EQUAL(testInvalidOptions<engine::api::TableParameters>("destinations=1&bla=foo"), 14UL);
+    BOOST_CHECK_EQUAL(testInvalidOptions<engine::api::TableParameters>("sources=1&destinations=1&bla=foo"), 24UL);
+    BOOST_CHECK_EQUAL(testInvalidOptions<engine::api::TableParameters>("sources=foo"), 8UL);
+    BOOST_CHECK_EQUAL(testInvalidOptions<engine::api::TableParameters>("destinations=foo"), 13UL);
+}
+
+BOOST_AUTO_TEST_CASE(valid_route_urls)
 {
     engine::api::RouteParameters reference_1{};
-    auto result_1 = api::parseRouteParameters("");
+    auto result_1 = api::parseParameters<engine::api::RouteParameters>("");
     BOOST_CHECK(result_1);
     BOOST_CHECK_EQUAL(reference_1.steps, result_1->steps);
     BOOST_CHECK_EQUAL(reference_1.alternative, result_1->alternative);
@@ -97,14 +114,8 @@ BOOST_AUTO_TEST_CASE(valid_urls)
     CHECK_EQUAL_RANGE(reference_1.radiuses, result_1->radiuses);
     CHECK_EQUAL_RANGE(reference_1.coordinates, result_1->coordinates);
 
-    // bool steps = true;
-    // bool alternative = true;
-    // GeometriesType geometries = GeometriesType::Polyline;
-    // OverviewType overview = OverviewType::False;
-    // std::vector<boost::optional<bool>> uturns;
-
     engine::api::RouteParameters reference_2{};
-    auto result_2 = api::parseRouteParameters(
+    auto result_2 = api::parseParameters<engine::api::RouteParameters>(
         "steps=true&alternative=true&geometries=polyline&overview=simplified");
     BOOST_CHECK(result_2);
     BOOST_CHECK_EQUAL(reference_2.steps, result_2->steps);
@@ -120,7 +131,7 @@ BOOST_AUTO_TEST_CASE(valid_urls)
     engine::api::RouteParameters reference_3{
         false, false, engine::api::RouteParameters::GeometriesType::GeoJSON,
         engine::api::RouteParameters::OverviewType::False, uturns_3};
-    auto result_3 = api::parseRouteParameters(
+    auto result_3 = api::parseParameters<engine::api::RouteParameters>(
         "steps=false&alternative=false&geometries=geojson&overview=false&uturns=true;false;");
     BOOST_CHECK(result_3);
     BOOST_CHECK_EQUAL(reference_3.steps, result_3->steps);
@@ -139,17 +150,17 @@ BOOST_AUTO_TEST_CASE(valid_urls)
             "_4ghA4JuzAD_IAAAo28BAOYAAAAzAAAAAgAAAEwAAAAAAAAAdIwAAJ4AAAAXiSEDfm7MAAEAAQGLSzmR"),
         engine::Hint::FromBase64(
             "03AhA0vnzAA_SAAA_____3wEAAAYAAAAQAAAAB4AAABAAAAAoUYBAJ4AAADlcCEDSefMAAMAAQGLSzmR")};
-    engine::api::RouteParameters reference_4{false,
+    engine::api::RouteParameters reference_4{
-                                             true,
+        false,
-                                             engine::api::RouteParameters::GeometriesType::Polyline,
+        true,
-                                             engine::api::RouteParameters::OverviewType::Simplified,
+        engine::api::RouteParameters::GeometriesType::Polyline,
-                                             std::vector<boost::optional<bool>>{},
+        engine::api::RouteParameters::OverviewType::Simplified,
-                                             std::vector<util::FixedPointCoordinate>{},
+        std::vector<boost::optional<bool>>{},
-                                             hints_4,
+        std::vector<util::FixedPointCoordinate>{},
-                                             std::vector<boost::optional<double>>{},
+        hints_4,
-                                             std::vector<boost::optional<engine::api::BaseParameters::Bearing>>{}
+        std::vector<boost::optional<double>>{},
-                                             };
+        std::vector<boost::optional<engine::api::BaseParameters::Bearing>>{}};
-    auto result_4 = api::parseRouteParameters(
+    auto result_4 = api::parseParameters<engine::api::RouteParameters>(
         "steps=false&hints=rVghAzxMzABMAwAA5h4CAKMIAAAQAAAAGAAAAAYAAAAAAAAAch8BAJ4AAACpWCED_"
         "0vMAAEAAQGLSzmR;_4ghA4JuzAD_"
         "IAAAo28BAOYAAAAzAAAAAgAAAEwAAAAAAAAAdIwAAJ4AAAAXiSEDfm7MAAEAAQGLSzmR;03AhA0vnzAA_SAAA_____"
@@ -165,4 +176,27 @@ BOOST_AUTO_TEST_CASE(valid_urls)
     CHECK_EQUAL_RANGE(reference_4.coordinates, result_4->coordinates);
 }
 
+BOOST_AUTO_TEST_CASE(valid_table_urls)
+{
+    engine::api::TableParameters reference_1{};
+    auto result_1 = api::parseParameters<engine::api::TableParameters>("");
+    BOOST_CHECK(result_1);
+    CHECK_EQUAL_RANGE(reference_1.sources, result_1->sources);
+    CHECK_EQUAL_RANGE(reference_1.destinations, result_1->destinations);
+    CHECK_EQUAL_RANGE(reference_1.bearings, result_1->bearings);
+    CHECK_EQUAL_RANGE(reference_1.radiuses, result_1->radiuses);
+    CHECK_EQUAL_RANGE(reference_1.coordinates, result_1->coordinates);
+
+    std::vector<std::size_t> sources_2 = {1, 2, 3};
+    std::vector<std::size_t> destinations_2 = {4, 5};
+    engine::api::TableParameters reference_2{sources_2, destinations_2};
+    auto result_2 = api::parseParameters<engine::api::TableParameters>("sources=1;2;3&destinations=4;5");
+    BOOST_CHECK(result_2);
+    CHECK_EQUAL_RANGE(reference_2.sources,      result_2->sources);
+    CHECK_EQUAL_RANGE(reference_2.destinations, result_2->destinations);
+    CHECK_EQUAL_RANGE(reference_2.bearings,     result_2->bearings);
+    CHECK_EQUAL_RANGE(reference_2.radiuses,     result_2->radiuses);
+    CHECK_EQUAL_RANGE(reference_2.coordinates,  result_2->coordinates);
+}
+
 BOOST_AUTO_TEST_SUITE_END()