Revert previous commits for cleaner approach

This commit is contained in:
whytro 2023-04-28 02:00:54 +09:00
parent 7dfa2bdd1f
commit 09ff160dcc
18 changed files with 81 additions and 228 deletions

View File

@ -1,7 +1,5 @@
# Unreleased # Unreleased
- Changes from 5.27.1 - Changes from 5.27.1
- API:
- CHANGED: Require a `radius` parameter when using `bearings`. [#6572](https://github.com/Project-OSRM/osrm-backend/pull/6572)
- Build: - Build:
- ADDED: Add CI job which builds OSRM with gcc 12. [#6455](https://github.com/Project-OSRM/osrm-backend/pull/6455) - ADDED: Add CI job which builds OSRM with gcc 12. [#6455](https://github.com/Project-OSRM/osrm-backend/pull/6455)
- CHANGED: Upgrade to clang-tidy 15. [#6439](https://github.com/Project-OSRM/osrm-backend/pull/6439) - CHANGED: Upgrade to clang-tidy 15. [#6439](https://github.com/Project-OSRM/osrm-backend/pull/6439)

View File

@ -31,7 +31,7 @@ To pass parameters to each location some options support an array-like encoding:
| Option | Values | Description | | Option | Values | Description |
|----------------|--------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| |----------------|--------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
|bearings |`{bearing};{bearing}[;{bearing} ...]` |Limits the search to segments with given bearing in degrees towards true north in a clockwise direction. Requires a corresponding radius. | |bearings |`{bearing};{bearing}[;{bearing} ...]` |Limits the search to segments with given bearing in degrees towards true north in a clockwise direction. |
|radiuses |`{radius};{radius}[;{radius} ...]` |Limits the search to given radius in meters. | |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. | |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. | |hints |`{hint};{hint}[;{hint} ...]` |Hint from previous request to derive position in street network. |

View File

@ -111,8 +111,7 @@ var osrm = new OSRM('network.osrm');
var options = { var options = {
coordinates: [[13.388860,52.517037]], coordinates: [[13.388860,52.517037]],
number: 3, number: 3,
bearings: [[0,20]], bearings: [[0,20]]
radiuses: [null]
}; };
osrm.nearest(options, function(err, response) { osrm.nearest(options, function(err, response) {
console.log(response.waypoints); // array of Waypoint objects console.log(response.waypoints); // array of Waypoint objects

View File

@ -70,7 +70,6 @@ Feature: Car - Allowed start/end modes
Given the query options Given the query options
| snapping | any | | snapping | any |
| bearings | 90,180; | | bearings | 90,180; |
| radiuses | unlimited; |
And the ways And the ways
| nodes | highway | access | | nodes | highway | access |
@ -113,7 +112,6 @@ Feature: Car - Allowed start/end modes
Given the query options Given the query options
| snapping | any | | snapping | any |
| bearings | 90,180;0,180;; | | bearings | 90,180;0,180;; |
| radiuses | unlimited;;; |
And the ways And the ways
| nodes | highway | access | | nodes | highway | access |

View File

@ -66,9 +66,6 @@ module.exports = function () {
if (bs.length === 2) return b; if (bs.length === 2) return b;
else return b += ',10'; else return b += ',10';
}).join(';'); }).join(';');
params.radiuses = bearings.map(() => {
return 'unlimited';
}).join(';');
} }
if (approaches.length) { if (approaches.length) {

View File

@ -112,7 +112,6 @@ Feature: Annotations
And the query options And the query options
| annotations | speed,distance,duration,nodes | | annotations | speed,distance,duration,nodes |
| bearings | 90,5;180,5 | | bearings | 90,5;180,5 |
| radiuses | unlimited;unlimited |
When I route I should get When I route I should get
| from | to | route | a:speed | a:distance | a:duration | a:nodes | | from | to | route | a:speed | a:distance | a:duration | a:nodes |

View File

@ -106,7 +106,7 @@ struct BaseParameters
bool IsValid() const bool IsValid() const
{ {
return (hints.empty() || hints.size() == coordinates.size()) && return (hints.empty() || hints.size() == coordinates.size()) &&
(bearings.empty() || bearings.size() == radiuses.size()) && (bearings.empty() || bearings.size() == coordinates.size()) &&
(radiuses.empty() || radiuses.size() == coordinates.size()) && (radiuses.empty() || radiuses.size() == coordinates.size()) &&
(approaches.empty() || approaches.size() == coordinates.size()) && (approaches.empty() || approaches.size() == coordinates.size()) &&
std::all_of(bearings.begin(), std::all_of(bearings.begin(),

View File

@ -522,12 +522,6 @@ inline bool argumentsToParameter(const Napi::CallbackInfo &args,
if (bearings.IsEmpty()) if (bearings.IsEmpty())
return false; return false;
if (!obj.Has("radiuses"))
{
ThrowError(args.Env(), "Bearings must be accompanied with radiuses");
return false;
}
if (!bearings.IsArray()) if (!bearings.IsArray())
{ {
ThrowError(args.Env(), "Bearings must be an array of arrays of numbers"); ThrowError(args.Env(), "Bearings must be an array of arrays of numbers");

View File

@ -4,20 +4,18 @@ namespace osrm::server::service
{ {
const constexpr char PARAMETER_SIZE_MISMATCH_MSG[] = const constexpr char PARAMETER_SIZE_MISMATCH_MSG[] =
"Number of elements in %1% size %2% does not match %3% size %4%"; "Number of elements in %1% size %2% does not match coordinate size %3%";
template <typename ParamT> template <typename ParamT>
bool constrainParamSize(const char *msg_template, bool constrainParamSize(const char *msg_template,
const char *param_name, const char *name,
const ParamT &param, const ParamT &param,
const char *target_name,
const std::size_t target_size, const std::size_t target_size,
std::string &help) std::string &help)
{ {
if (param.size() > 0 && param.size() != target_size) if (param.size() > 0 && param.size() != target_size)
{ {
help = (boost::format(msg_template) % param_name % param.size() % target_name % target_size) help = (boost::format(msg_template) % name % param.size() % target_size).str();
.str();
return true; return true;
} }
return false; return false;

View File

@ -43,12 +43,6 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
"InvalidOptions", "Number of bearings does not match number of coordinates", result); "InvalidOptions", "Number of bearings does not match number of coordinates", result);
} }
if (!params.bearings.empty() && params.radiuses.size() != params.bearings.size())
{
return Error(
"InvalidOptions", "Number of radiuses does not match number of bearings", result);
}
// Empty sources or destinations means the user wants all of them included, respectively // Empty sources or destinations means the user wants all of them included, respectively
// The ManyToMany routing algorithm we dispatch to below already handles this perfectly. // The ManyToMany routing algorithm we dispatch to below already handles this perfectly.
const auto num_sources = const auto num_sources =

View File

@ -349,8 +349,7 @@ Napi::Value Engine::route(const Napi::CallbackInfo &info)
* var options = { * var options = {
* coordinates: [[13.388860,52.517037]], * coordinates: [[13.388860,52.517037]],
* number: 3, * number: 3,
* bearings: [[0,20]], * bearings: [[0,20]]
* radiuses: [null]
* }; * };
* osrm.nearest(options, function(err, response) { * osrm.nearest(options, function(err, response) {
* console.log(response.waypoints); // array of Waypoint objects * console.log(response.waypoints); // array of Waypoint objects

View File

@ -1,11 +1,13 @@
#include "server/service/match_service.hpp" #include "server/service/match_service.hpp"
#include "server/service/utils.hpp"
#include "server/api/parameters_parser.hpp" #include "server/api/parameters_parser.hpp"
#include "server/service/utils.hpp"
#include "engine/api/match_parameters.hpp" #include "engine/api/match_parameters.hpp"
#include "util/json_container.hpp" #include "util/json_container.hpp"
#include <boost/format.hpp>
namespace osrm::server::service namespace osrm::server::service
{ {
namespace namespace
@ -15,38 +17,16 @@ std::string getWrongOptionHelp(const engine::api::MatchParameters &parameters)
std::string help; std::string help;
const auto coord_size = parameters.coordinates.size(); const auto coord_size = parameters.coordinates.size();
const auto bearings_size = parameters.bearings.size();
const bool param_size_mismatch = constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG, const bool param_size_mismatch =
"hints", constrainParamSize(
parameters.hints, PARAMETER_SIZE_MISMATCH_MSG, "hints", parameters.hints, coord_size, help) ||
"coordinates", constrainParamSize(
coord_size, PARAMETER_SIZE_MISMATCH_MSG, "bearings", parameters.bearings, coord_size, help) ||
help) || constrainParamSize(
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG, PARAMETER_SIZE_MISMATCH_MSG, "radiuses", parameters.radiuses, coord_size, help) ||
"bearings", constrainParamSize(
parameters.bearings, PARAMETER_SIZE_MISMATCH_MSG, "timestamps", parameters.timestamps, coord_size, help);
"coordinates",
coord_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"radiuses",
parameters.radiuses,
"bearings",
bearings_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"radiuses",
parameters.radiuses,
"coordinates",
coord_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"timestamps",
parameters.timestamps,
"coordinates",
coord_size,
help);
if (!param_size_mismatch && parameters.coordinates.size() < 2) if (!param_size_mismatch && parameters.coordinates.size() < 2)
{ {

View File

@ -6,8 +6,11 @@
#include "util/json_container.hpp" #include "util/json_container.hpp"
#include <boost/format.hpp>
namespace osrm::server::service namespace osrm::server::service
{ {
namespace namespace
{ {
std::string getWrongOptionHelp(const engine::api::NearestParameters &parameters) std::string getWrongOptionHelp(const engine::api::NearestParameters &parameters)
@ -15,34 +18,14 @@ std::string getWrongOptionHelp(const engine::api::NearestParameters &parameters)
std::string help; std::string help;
const auto coord_size = parameters.coordinates.size(); const auto coord_size = parameters.coordinates.size();
const auto bearings_size = parameters.bearings.size();
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG, "hints", parameters.hints, coord_size, help);
constrainParamSize( constrainParamSize(
PARAMETER_SIZE_MISMATCH_MSG, "hints", parameters.hints, "coordinates", coord_size, help); PARAMETER_SIZE_MISMATCH_MSG, "bearings", parameters.bearings, coord_size, help);
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG, constrainParamSize(
"bearings", PARAMETER_SIZE_MISMATCH_MSG, "radiuses", parameters.radiuses, coord_size, help);
parameters.bearings, constrainParamSize(
"coordinates", PARAMETER_SIZE_MISMATCH_MSG, "approaches", parameters.approaches, coord_size, help);
coord_size,
help);
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"radiuses",
parameters.radiuses,
"bearings",
bearings_size,
help);
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"radiuses",
parameters.radiuses,
"coordinates",
coord_size,
help);
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"approaches",
parameters.approaches,
"coordinates",
coord_size,
help);
return help; return help;
} }

View File

@ -15,38 +15,16 @@ std::string getWrongOptionHelp(const engine::api::RouteParameters &parameters)
std::string help; std::string help;
const auto coord_size = parameters.coordinates.size(); const auto coord_size = parameters.coordinates.size();
const auto bearings_size = parameters.bearings.size();
const bool param_size_mismatch = constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG, const bool param_size_mismatch =
"hints", constrainParamSize(
parameters.hints, PARAMETER_SIZE_MISMATCH_MSG, "hints", parameters.hints, coord_size, help) ||
"coordinates", constrainParamSize(
coord_size, PARAMETER_SIZE_MISMATCH_MSG, "bearings", parameters.bearings, coord_size, help) ||
help) || constrainParamSize(
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG, PARAMETER_SIZE_MISMATCH_MSG, "radiuses", parameters.radiuses, coord_size, help) ||
"bearings", constrainParamSize(
parameters.bearings, PARAMETER_SIZE_MISMATCH_MSG, "approaches", parameters.approaches, coord_size, help);
"coordinates",
coord_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"radiuses",
parameters.radiuses,
"bearings",
bearings_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"radiuses",
parameters.radiuses,
"coordinates",
coord_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"approaches",
parameters.approaches,
"coordinates",
coord_size,
help);
if (!param_size_mismatch && parameters.coordinates.size() < 2) if (!param_size_mismatch && parameters.coordinates.size() < 2)
{ {

View File

@ -1,52 +1,51 @@
#include "server/service/table_service.hpp" #include "server/service/table_service.hpp"
#include "server/service/utils.hpp"
#include "server/api/parameters_parser.hpp" #include "server/api/parameters_parser.hpp"
#include "engine/api/table_parameters.hpp" #include "engine/api/table_parameters.hpp"
#include "util/json_container.hpp" #include "util/json_container.hpp"
#include <boost/format.hpp>
namespace osrm::server::service namespace osrm::server::service
{ {
namespace namespace
{ {
const constexpr char PARAMETER_SIZE_MISMATCH_MSG[] =
"Number of elements in %1% size %2% does not match coordinate size %3%";
template <typename ParamT>
bool constrainParamSize(const char *msg_template,
const char *name,
const ParamT &param,
const std::size_t target_size,
std::string &help)
{
if (param.size() > 0 && param.size() != target_size)
{
help = (boost::format(msg_template) % name % param.size() % target_size).str();
return true;
}
return false;
}
std::string getWrongOptionHelp(const engine::api::TableParameters &parameters) std::string getWrongOptionHelp(const engine::api::TableParameters &parameters)
{ {
std::string help; std::string help;
const auto coord_size = parameters.coordinates.size(); const auto coord_size = parameters.coordinates.size();
const auto bearings_size = parameters.bearings.size();
const bool param_size_mismatch = constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG, const bool param_size_mismatch =
"hints", constrainParamSize(
parameters.hints, PARAMETER_SIZE_MISMATCH_MSG, "hints", parameters.hints, coord_size, help) ||
"coordinates", constrainParamSize(
coord_size, PARAMETER_SIZE_MISMATCH_MSG, "bearings", parameters.bearings, coord_size, help) ||
help) || constrainParamSize(
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG, PARAMETER_SIZE_MISMATCH_MSG, "radiuses", parameters.radiuses, coord_size, help) ||
"bearings", constrainParamSize(
parameters.bearings, PARAMETER_SIZE_MISMATCH_MSG, "approaches", parameters.approaches, coord_size, help);
"coordinates",
coord_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"radiuses",
parameters.radiuses,
"bearings",
bearings_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"radiuses",
parameters.radiuses,
"coordinates",
coord_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"approaches",
parameters.approaches,
"coordinates",
coord_size,
help);
if (!param_size_mismatch && parameters.coordinates.size() < 2) if (!param_size_mismatch && parameters.coordinates.size() < 2)
{ {

View File

@ -6,6 +6,8 @@
#include "util/json_container.hpp" #include "util/json_container.hpp"
#include <boost/format.hpp>
namespace osrm::server::service namespace osrm::server::service
{ {
namespace namespace
@ -15,38 +17,16 @@ std::string getWrongOptionHelp(const engine::api::TripParameters &parameters)
std::string help; std::string help;
const auto coord_size = parameters.coordinates.size(); const auto coord_size = parameters.coordinates.size();
const auto bearings_size = parameters.bearings.size();
const bool param_size_mismatch = constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG, const bool param_size_mismatch =
"hints", constrainParamSize(
parameters.hints, PARAMETER_SIZE_MISMATCH_MSG, "hints", parameters.hints, coord_size, help) ||
"coordinates", constrainParamSize(
coord_size, PARAMETER_SIZE_MISMATCH_MSG, "bearings", parameters.bearings, coord_size, help) ||
help) || constrainParamSize(
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG, PARAMETER_SIZE_MISMATCH_MSG, "radiuses", parameters.radiuses, coord_size, help) ||
"bearings", constrainParamSize(
parameters.bearings, PARAMETER_SIZE_MISMATCH_MSG, "approaches", parameters.approaches, coord_size, help);
"coordinates",
coord_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"radiuses",
parameters.radiuses,
"bearings",
bearings_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"radiuses",
parameters.radiuses,
"coordinates",
coord_size,
help) ||
constrainParamSize(PARAMETER_SIZE_MISMATCH_MSG,
"approaches",
parameters.approaches,
"coordinates",
coord_size,
help);
if (!param_size_mismatch && parameters.coordinates.size() < 2) if (!param_size_mismatch && parameters.coordinates.size() < 2)
{ {

View File

@ -424,7 +424,6 @@ test('route: integer bearing values no longer supported', function(assert) {
var options = { var options = {
coordinates: two_test_coordinates, coordinates: two_test_coordinates,
bearings: [200, 250], bearings: [200, 250],
radiuses: [null],
}; };
assert.throws(function() { osrm.route(options, function(err, route) {}); }, assert.throws(function() { osrm.route(options, function(err, route) {}); },
/Bearing must be an array of \[bearing, range\] or null/); /Bearing must be an array of \[bearing, range\] or null/);
@ -436,7 +435,6 @@ test('route: valid bearing values', function(assert) {
var options = { var options = {
coordinates: two_test_coordinates, coordinates: two_test_coordinates,
bearings: [[200, 180], [250, 180]], bearings: [[200, 180], [250, 180]],
radiuses: [null, null],
}; };
osrm.route(options, function(err, route) { osrm.route(options, function(err, route) {
assert.ifError(err); assert.ifError(err);
@ -450,49 +448,38 @@ test('route: valid bearing values', function(assert) {
}); });
test('route: invalid bearing values', function(assert) { test('route: invalid bearing values', function(assert) {
assert.plan(7); assert.plan(6);
var osrm = new OSRM(monaco_path); var osrm = new OSRM(monaco_path);
assert.throws(function() { osrm.route({ assert.throws(function() { osrm.route({
coordinates: two_test_coordinates, coordinates: two_test_coordinates,
bearings: [[400, 180], [-250, 180]], bearings: [[400, 180], [-250, 180]],
radiuses: [null, null],
}, function(err, route) {}) }, }, function(err, route) {}) },
/Bearing values need to be in range 0..360, 0..180/); /Bearing values need to be in range 0..360, 0..180/);
assert.throws(function() { osrm.route({ assert.throws(function() { osrm.route({
coordinates: two_test_coordinates, coordinates: two_test_coordinates,
bearings: [[200], [250, 180]], bearings: [[200], [250, 180]],
radiuses: [null, null],
}, function(err, route) {}) }, }, function(err, route) {}) },
/Bearing must be an array of/); /Bearing must be an array of/);
assert.throws(function() { osrm.route({ assert.throws(function() { osrm.route({
coordinates: two_test_coordinates, coordinates: two_test_coordinates,
bearings: [[400, 109], [100, 720]], bearings: [[400, 109], [100, 720]],
radiuses: [null, null],
}, function(err, route) {}) }, }, function(err, route) {}) },
/Bearing values need to be in range 0..360, 0..180/); /Bearing values need to be in range 0..360, 0..180/);
assert.throws(function() { osrm.route({ assert.throws(function() { osrm.route({
coordinates: two_test_coordinates, coordinates: two_test_coordinates,
bearings: 400, bearings: 400,
radiuses: [null],
}, function(err, route) {}) }, }, function(err, route) {}) },
/Bearings must be an array of arrays of numbers/); /Bearings must be an array of arrays of numbers/);
assert.throws(function() { osrm.route({ assert.throws(function() { osrm.route({
coordinates: two_test_coordinates, coordinates: two_test_coordinates,
bearings: [[100, 100]], bearings: [[100, 100]],
radiuses: [null, null],
}, function(err, route) {}) }, }, function(err, route) {}) },
/Bearings array must have the same length as coordinates array/); /Bearings array must have the same length as coordinates array/);
assert.throws(function() { osrm.route({ assert.throws(function() { osrm.route({
coordinates: two_test_coordinates, coordinates: two_test_coordinates,
bearings: [Infinity, Infinity], bearings: [Infinity, Infinity],
radiuses: [null, null],
}, function(err, route) {}) }, }, function(err, route) {}) },
/Bearing must be an array of \[bearing, range\] or null/); /Bearing must be an array of \[bearing, range\] or null/);
assert.throws(function() { osrm.route({
coordinates: two_test_coordinates,
bearings: [[0, 180], [0, 180]],
}, function(err, route) {}) },
/Bearings must be accompanied with radiuses/);
}); });
test('route: routes Monaco with hints', function(assert) { test('route: routes Monaco with hints', function(assert) {

View File

@ -387,34 +387,4 @@ BOOST_AUTO_TEST_CASE(test_table_serialiaze_fb_no_waypoints)
BOOST_CHECK(fb->waypoints() == nullptr); BOOST_CHECK(fb->waypoints() == nullptr);
} }
void test_table_bearings_without_radius(bool use_json_only_api)
{
using namespace osrm;
auto osrm = getOSRM(OSRM_TEST_DATA_DIR "/ch/monaco.osrm");
TableParameters params;
params.coordinates.push_back(get_dummy_location());
params.coordinates.push_back(get_dummy_location());
params.bearings.push_back(engine::Bearing{100, 100});
params.bearings.push_back(engine::Bearing{100, 100});
json::Object json_result;
const auto rc = run_table_json(osrm, params, json_result, use_json_only_api);
BOOST_CHECK(rc == Status::Error);
const auto code = json_result.values.at("code").get<json::String>().value;
BOOST_CHECK_EQUAL(code, "InvalidOptions");
const auto message = json_result.values.at("message").get<json::String>().value;
BOOST_CHECK_EQUAL(message, "Number of radiuses does not match number of bearings");
}
BOOST_AUTO_TEST_CASE(test_table_bearings_without_radius_old_api)
{
test_table_bearings_without_radius(true);
}
BOOST_AUTO_TEST_CASE(test_table_bearings_without_radius_new_api)
{
test_table_bearings_without_radius(false);
}
BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE_END()