From e3c7995b00409f3da335300f7ca3d212a69602a4 Mon Sep 17 00:00:00 2001 From: Rafael Guglielmetti Date: Mon, 22 Aug 2022 08:32:25 +0200 Subject: [PATCH 1/3] Add missing files in exception message (#5360) --- CHANGELOG.md | 1 + include/storage/io_config.hpp | 1 + src/osrm/osrm.cpp | 9 +++++++-- src/storage/io_config.cpp | 18 ++++++++++++++++-- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d9c9d58f..ed93059d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - FIXED: Use Boost.Beast to parse HTTP request. [#6294](https://github.com/Project-OSRM/osrm-backend/pull/6294) - FIXED: Fix inefficient osrm-routed connection handling [#6113](https://github.com/Project-OSRM/osrm-backend/pull/6113) - Misc: + - CHANGED: missing files list is included in exception message. [#5360](https://github.com/Project-OSRM/osrm-backend/pull/5360) - CHANGED: Do not use deprecated Callback::Call overload in Node bindings. [#6318](https://github.com/Project-OSRM/osrm-backend/pull/6318) - FIXED: Fix distance calculation consistency. [#6315](https://github.com/Project-OSRM/osrm-backend/pull/6315) - FIXED: Fix performance issue after migration to sol2 3.3.0. [#6304](https://github.com/Project-OSRM/osrm-backend/pull/6304) diff --git a/include/storage/io_config.hpp b/include/storage/io_config.hpp index 577291ee1..831382df7 100644 --- a/include/storage/io_config.hpp +++ b/include/storage/io_config.hpp @@ -25,6 +25,7 @@ struct IOConfig } bool IsValid() const; + std::vector GetMissingFiles() const; boost::filesystem::path GetPath(const std::string &fileName) const { if (!IsConfigured(fileName, required_input_files) && diff --git a/src/osrm/osrm.cpp b/src/osrm/osrm.cpp index 8871aff45..83e1076f7 100644 --- a/src/osrm/osrm.cpp +++ b/src/osrm/osrm.cpp @@ -10,6 +10,8 @@ #include "engine/engine_config.hpp" #include "engine/status.hpp" +#include + #include namespace osrm @@ -25,8 +27,11 @@ OSRM::OSRM(engine::EngineConfig &config) // First, check that necessary core data is available if (!config.use_shared_memory && !config.storage_config.IsValid()) { - throw util::exception("Required files are missing, cannot continue. Have all the " - "pre-processing steps been run?"); + const auto &missingFiles = config.storage_config.GetMissingFiles(); + throw util::exception("Required files are missing, cannot continue. Have all the " + "pre-processing steps been run? " + "Missing files: " + + boost::algorithm::join(missingFiles, ", ")); } // Now, check that the algorithm requested can be used with the data diff --git a/src/storage/io_config.cpp b/src/storage/io_config.cpp index e100a9317..cd05d2d76 100644 --- a/src/storage/io_config.cpp +++ b/src/storage/io_config.cpp @@ -10,10 +10,11 @@ namespace osrm { namespace storage { + +namespace fs = boost::filesystem; + bool IOConfig::IsValid() const { - namespace fs = boost::filesystem; - bool success = true; for (auto &fileName : required_input_files) { @@ -26,5 +27,18 @@ bool IOConfig::IsValid() const } return success; } + +std::vector IOConfig::GetMissingFiles() const +{ + std::vector missingFiles; + for (auto &fileName : required_input_files) + { + if (!fs::is_regular_file(fs::path(base_path.string() + fileName.string()))) + { + missingFiles.push_back(base_path.string() + fileName.string()); + } + } + return missingFiles; +} } // namespace storage } // namespace osrm From b4b1aea567db245c7336ed5a7fd1eb0ddd0c70e5 Mon Sep 17 00:00:00 2001 From: Michael Bell Date: Mon, 22 Aug 2022 12:56:47 +0100 Subject: [PATCH 2/3] Add support for non-round-trips with a single fixed endpoint (#6050) Currently /trip supports finding round-trip routes where only the start or end location is fixed. This PR extends this feature to non-round-trip requests. We do this by a new table manipulation that simulates non-round-trip fixed endpoint requests as a round-trip request. --- CHANGELOG.md | 2 + docs/http.md | 4 +- features/step_definitions/trip.js | 6 +- features/testbot/trip.feature | 130 ++++++++++++++------ features/testbot/zero-speed-updates.feature | 4 +- src/engine/plugins/trip.cpp | 73 +++++++---- test/nodejs/trip.js | 55 +++++---- unit_tests/library/trip.cpp | 8 +- 8 files changed, 188 insertions(+), 94 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed93059d5..8b577c71f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ - FIXED: Fixed Node docs generation check in CI. [#6058](https://github.com/Project-OSRM/osrm-backend/pull/6058) - CHANGED: Docker build, enabled arm64 build layer [#6172](https://github.com/Project-OSRM/osrm-backend/pull/6172) - CHANGED: Docker build, enabled apt-get update/install caching in separate layer for build phase [#6175](https://github.com/Project-OSRM/osrm-backend/pull/6175) + - Routing: + - ADDED: Add support for non-round-trips with a single fixed endpoint. [#6050](https://github.com/Project-OSRM/osrm-backend/pull/6050) # 5.26.0 - Changes from 5.25.0 diff --git a/docs/http.md b/docs/http.md index 60149856b..acdc1cd4f 100644 --- a/docs/http.md +++ b/docs/http.md @@ -507,8 +507,8 @@ Right now, the following combinations are possible: | true | any | last | **yes** | | true | any | any | **yes** | | false | first | last | **yes** | -| false | first | any | no | -| false | any | last | no | +| false | first | any | **yes** | +| false | any | last | **yes** | | false | any | any | no | #### Example Requests diff --git a/features/step_definitions/trip.js b/features/step_definitions/trip.js index e72044018..ab2f7c35e 100644 --- a/features/step_definitions/trip.js +++ b/features/step_definitions/trip.js @@ -61,7 +61,8 @@ module.exports = function () { var subTrips; var trip_durations; var trip_distance; - if (res.statusCode === 200) { + var ok = res.statusCode === 200; + if (ok) { if (headers.has('trips')) { subTrips = json.trips.filter(t => !!t).map(t => t.legs).map(tl => Array.prototype.concat.apply([], tl.map((sl, i) => { var toAdd = []; @@ -84,8 +85,7 @@ module.exports = function () { } } - var ok = true, - encodedResult = ''; + var encodedResult = ''; if (json.trips) row.trips.split(',').forEach((sub, si) => { if (si >= subTrips.length) { diff --git a/features/testbot/trip.feature b/features/testbot/trip.feature index 9fe3c23ae..840aadb9c 100644 --- a/features/testbot/trip.feature +++ b/features/testbot/trip.feature @@ -5,7 +5,7 @@ Feature: Basic trip planning Given the profile "testbot" Given a grid size of 10 meters - Scenario: Testbot - Trip: Roundtrip with one waypoint + Scenario: Testbot - Trip: Roundtrip between same waypoint Given the node map """ a b @@ -21,7 +21,7 @@ Feature: Basic trip planning When I plan a trip I should get | waypoints | trips | - | a | aa | + | a,a | aa | Scenario: Testbot - Trip: Roundtrip with waypoints (less than 10) Given the node map @@ -69,36 +69,37 @@ Feature: Basic trip planning | waypoints | trips | | a,b,c,d,e,f,g,h,i,j,k,l | alkjihgfedcba | - Scenario: Testbot - Trip: Roundtrip FS waypoints (more than 10) - Given the node map - """ - a b c d - l e - k f - j i h g - """ - - And the ways - | nodes | - | ab | - | bc | - | de | - | ef | - | fg | - | gh | - | hi | - | ij | - | jk | - | kl | - | la | - - When I plan a trip I should get - | waypoints | source | trips | - | a,b,c,d,e,f,g,h,i,j,k,l | first | alkjihgfedcba | - - Scenario: Testbot - Trip: Roundtrip FE waypoints (more than 10) + Scenario: Testbot - Trip: FS waypoints (less than 10) Given the query options - | source | last | + | source | first | + Given the node map + """ + a b c d + l e + + j i g + """ + + And the ways + | nodes | + | ab | + | bc | + | de | + | eg | + | gi | + | ij | + | jl | + | la | + + When I plan a trip I should get + | waypoints | trips | roundtrip | durations | + | a,b,c,d,e,g,i,j,l | abcdegijla | true | 22 | + | a,b,c,d,e,g,i,j,l | abcljiged | false | 13 | + + + Scenario: Testbot - Trip: FS waypoints (more than 10) + Given the query options + | source | first | Given the node map """ a b c d @@ -122,8 +123,67 @@ Feature: Basic trip planning | la | When I plan a trip I should get - | waypoints | trips | - | a,b,c,d,e,f,g,h,i,j,k,l | lkjihgfedcbal | + | waypoints | trips | roundtrip | durations | + | a,b,c,d,e,f,g,h,i,j,k,l | alkjihgfedcba | true | 22 | + | a,b,c,d,e,f,g,h,i,j,k,l | acblkjihgfed | false | 13 | + + + Scenario: Testbot - Trip: FE waypoints (less than 10) + Given the query options + | destination | last | + Given the node map + """ + a b c d + l e + + j i g + """ + + And the ways + | nodes | + | ab | + | bc | + | de | + | eg | + | gi | + | ij | + | jl | + | la | + + When I plan a trip I should get + | waypoints | trips | roundtrip | durations | + | a,b,c,d,e,g,i,j,l | labcdegijl | true | 22 | + | a,b,c,d,e,g,i,j,l | degijabcl | false | 14 | + + Scenario: Testbot - Trip: FE waypoints (more than 10) + Given the query options + | destination | last | + Given the node map + """ + a b c d + l e + k f + j i h g + """ + + And the ways + | nodes | + | ab | + | bc | + | de | + | ef | + | fg | + | gh | + | hi | + | ij | + | jk | + | kl | + | la | + + When I plan a trip I should get + | waypoints | trips | roundtrip | durations | + | a,b,c,d,e,f,g,h,i,j,k,l | lkjihgfedcbal | true | 22 | + | a,b,c,d,e,f,g,h,i,j,k,l | cbakjihgfedl | false | 19 | Scenario: Testbot - Trip: Unroutable roundtrip with waypoints (less than 10) Given the node map @@ -274,7 +334,7 @@ Feature: Basic trip planning | a,b,d,e,c | first | last | true | abedca | - Scenario: Testbot - Trip: midway points in isoldated roads should return no trips + Scenario: Testbot - Trip: midway points in isolated roads should return no trips Given the node map """ a 1 b @@ -370,4 +430,4 @@ Feature: Basic trip planning When I plan a trip I should get | waypoints | trips | durations | geometry | | a,b,c,d | abcda | 7.6 | 1,1,1,1.00009,0.99991,1,1,1.00009,1,1,0.99991,1.00009,1,1 | - | d,b,c,a | dbcad | 7.6 | 0.99991,1.00009,1,1,1,1.00009,0.99991,1,1,1.00009,1,1,0.99991,1.00009 | \ No newline at end of file + | d,b,c,a | dbcad | 7.6 | 0.99991,1.00009,1,1,1,1.00009,0.99991,1,1,1.00009,1,1,0.99991,1.00009 | diff --git a/features/testbot/zero-speed-updates.feature b/features/testbot/zero-speed-updates.feature index c2bc82c94..ae5a99c7f 100644 --- a/features/testbot/zero-speed-updates.feature +++ b/features/testbot/zero-speed-updates.feature @@ -187,5 +187,5 @@ Feature: Check zero speed updates When I plan a trip I should get | waypoints | trips | code | - | a,b,c,d | abcda | NoTrips | - | d,b,c,a | dbcad | NoTrips | + | a,b,c,d | | NoTrips | + | d,b,c,a | | NoTrips | diff --git a/src/engine/plugins/trip.cpp b/src/engine/plugins/trip.cpp index d949fcb60..20daf95d9 100644 --- a/src/engine/plugins/trip.cpp +++ b/src/engine/plugins/trip.cpp @@ -36,18 +36,7 @@ bool IsSupportedParameterCombination(const bool fixed_start, const bool fixed_end, const bool roundtrip) { - if (fixed_start && fixed_end && !roundtrip) - { - return true; - } - else if (roundtrip) - { - return true; - } - else - { - return false; - } + return roundtrip || fixed_start || fixed_end; } // given the node order in which to visit, compute the actual route (with geometry, travel time and @@ -142,6 +131,32 @@ void ManipulateTableForFSE(const std::size_t source_id, //********* End of changes to table ************************************* } +void ManipulateTableForNonRoundtripFS(const std::size_t source_id, + util::DistTableWrapper &result_table) +{ + // We can use the round-trip calculation to simulate non-round-trip fixed start + // by making all paths to the source location zero. Effectively finding an 'optimal' + // round-trip path that ignores the cost of getting back from any destination to the + // source. + for (const auto i : util::irange(0, result_table.GetNumberOfNodes())) + { + result_table.SetValue(i, source_id, 0); + } +} + +void ManipulateTableForNonRoundtripFE(const std::size_t destination_id, + util::DistTableWrapper &result_table) +{ + // We can use the round-trip calculation to simulate non-round-trip fixed end + // by making all paths from the destination to other locations zero. + // Effectively, finding an 'optimal' round-trip path that ignores the cost of getting + // from the destination to any source. + for (const auto i : util::irange(0, result_table.GetNumberOfNodes())) + { + result_table.SetValue(destination_id, i, 0); + } +} + Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms, const api::TripParameters ¶meters, osrm::engine::api::ResultT &result) const @@ -225,7 +240,7 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms, return Status::Error; } - const constexpr std::size_t BF_MAX_FEASABLE = 10; + const constexpr std::size_t BF_MAX_FEASIBLE = 10; BOOST_ASSERT_MSG(result_duration_table.size() == number_of_locations * number_of_locations, "Distance Table has wrong size"); @@ -238,11 +253,19 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms, { ManipulateTableForFSE(source_id, destination_id, result_duration_table); } + else if (!parameters.roundtrip && fixed_start) + { + ManipulateTableForNonRoundtripFS(source_id, result_duration_table); + } + else if (!parameters.roundtrip && fixed_end) + { + ManipulateTableForNonRoundtripFE(destination_id, result_duration_table); + } std::vector duration_trip; duration_trip.reserve(number_of_locations); // get an optimized order in which the destinations should be visited - if (number_of_locations < BF_MAX_FEASABLE) + if (number_of_locations < BF_MAX_FEASIBLE) { duration_trip = trip::BruteForceTrip(number_of_locations, result_duration_table); } @@ -251,20 +274,28 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms, duration_trip = trip::FarthestInsertionTrip(number_of_locations, result_duration_table); } - // rotate result such that roundtrip starts at node with index 0 - // thist first if covers scenarios: !fixed_end || fixed_start || (fixed_start && fixed_end) if (!fixed_end || fixed_start) { + // rotate result such that trip starts at node with index 0 auto desired_start_index = std::find(std::begin(duration_trip), std::end(duration_trip), 0); BOOST_ASSERT(desired_start_index != std::end(duration_trip)); std::rotate(std::begin(duration_trip), desired_start_index, std::end(duration_trip)); } - else if (fixed_end && !fixed_start && parameters.roundtrip) - { - auto desired_start_index = + else + { // fixed_end + auto destination_index = std::find(std::begin(duration_trip), std::end(duration_trip), destination_id); - BOOST_ASSERT(desired_start_index != std::end(duration_trip)); - std::rotate(std::begin(duration_trip), desired_start_index, std::end(duration_trip)); + BOOST_ASSERT(destination_index != std::end(duration_trip)); + if (!parameters.roundtrip) + { + // We want the location after destination to be at the front + std::advance(destination_index, 1); + if (destination_index == std::end(duration_trip)) + { + destination_index = std::begin(duration_trip); + } + } + std::rotate(std::begin(duration_trip), destination_index, std::end(duration_trip)); } // get the route when visiting all destinations in optimized order diff --git a/test/nodejs/trip.js b/test/nodejs/trip.js index f26d97b83..683032b3c 100644 --- a/test/nodejs/trip.js +++ b/test/nodejs/trip.js @@ -262,33 +262,19 @@ test('trip: routes Monaco with null hints', function(assert) { }); test('trip: service combinations that are not implemented', function(assert) { - assert.plan(3); + assert.plan(1); var osrm = new OSRM(data_path); - // fixed start, non-roundtrip + // no fixed start, no fixed end, non-roundtrip var options = { coordinates: two_test_coordinates, - source: 'first', + source: 'any', + destination: 'any', roundtrip: false }; osrm.trip(options, function(err, second) { assert.equal('NotImplemented', err.message); }); - - // fixed start, fixed end, non-roundtrip - options.source = 'any'; - options.destination = 'any'; - osrm.trip(options, function(err, second) { - assert.equal('NotImplemented', err.message); - }); - - // fixed end, non-roundtrip - delete options.source; - options.destination = 'last'; - osrm.trip(options, function(err, second) { - assert.equal('NotImplemented', err.message); - }); - }); test('trip: fixed start and end combinations', function(assert) { @@ -302,16 +288,17 @@ test('trip: fixed start and end combinations', function(assert) { geometries: 'geojson' }; - // fixed start and end, non-roundtrip - osrm.trip(options, function(err, fseTrip) { - assert.ifError(err); - assert.equal(1, fseTrip.trips.length); - var coordinates = fseTrip.trips[0].geometry.coordinates; - assert.notEqual(JSON.stringify(coordinates[0]), JSON.stringify(coordinates[coordinates.length - 1])); - }); + // variations of non roundtrip + var nonRoundtripChecks = function(options) { + osrm.trip(options, function(err, fseTrip) { + assert.ifError(err); + assert.equal(1, fseTrip.trips.length); + var coordinates = fseTrip.trips[0].geometry.coordinates; + assert.notEqual(JSON.stringify(coordinates[0]), JSON.stringify(coordinates[coordinates.length - 1])); + }); + }; // variations of roundtrip - var roundtripChecks = function(options) { osrm.trip(options, function(err, trip) { assert.ifError(err); @@ -319,7 +306,20 @@ test('trip: fixed start and end combinations', function(assert) { var coordinates = trip.trips[0].geometry.coordinates; assert.equal(JSON.stringify(coordinates[0]), JSON.stringify(coordinates[coordinates.length - 1])); }); - } + }; + + // fixed start and end, non-roundtrip + nonRoundtripChecks(options); + + // fixed start, non-roundtrip + delete options.destination; + options.source = 'first'; + nonRoundtripChecks(options); + + // fixed end, non-roundtrip + delete options.source; + options.destination = 'last'; + nonRoundtripChecks(options); // roundtrip, source and destination not specified roundtripChecks({coordinates: options.coordinates, geometries: options.geometries}); @@ -327,6 +327,7 @@ test('trip: fixed start and end combinations', function(assert) { // roundtrip, fixed destination options.roundtrip = true; delete options.source; + options.destination = 'last'; roundtripChecks(options); //roundtrip, fixed source diff --git a/unit_tests/library/trip.cpp b/unit_tests/library/trip.cpp index 6eb500e81..52308f2e0 100644 --- a/unit_tests/library/trip.cpp +++ b/unit_tests/library/trip.cpp @@ -362,7 +362,7 @@ void test_tfse_illegal_parameters(bool use_json_only_api) ResetParams(locations, params); params.source = TripParameters::SourceType::First; params.roundtrip = false; - CheckNotImplemented(osrm, params, use_json_only_api); + CheckOk(osrm, params, use_json_only_api); ResetParams(locations, params); params.destination = TripParameters::DestinationType::Any; @@ -372,7 +372,7 @@ void test_tfse_illegal_parameters(bool use_json_only_api) ResetParams(locations, params); params.destination = TripParameters::DestinationType::Last; params.roundtrip = false; - CheckNotImplemented(osrm, params, use_json_only_api); + CheckOk(osrm, params, use_json_only_api); // three parameters set params.source = TripParameters::SourceType::Any; @@ -383,12 +383,12 @@ void test_tfse_illegal_parameters(bool use_json_only_api) params.source = TripParameters::SourceType::Any; params.destination = TripParameters::DestinationType::Last; params.roundtrip = false; - CheckNotImplemented(osrm, params, use_json_only_api); + CheckOk(osrm, params, use_json_only_api); params.source = TripParameters::SourceType::First; params.destination = TripParameters::DestinationType::Any; params.roundtrip = false; - CheckNotImplemented(osrm, params, use_json_only_api); + CheckOk(osrm, params, use_json_only_api); } BOOST_AUTO_TEST_CASE(test_tfse_illegal_parameters_old_api) { test_tfse_illegal_parameters(true); } BOOST_AUTO_TEST_CASE(test_tfse_illegal_parameters_new_api) { test_tfse_illegal_parameters(false); } From 3cfd0e833405f34474ff73706b4c67758fe4a061 Mon Sep 17 00:00:00 2001 From: Michael Bell Date: Mon, 22 Aug 2022 12:58:16 +0100 Subject: [PATCH 3/3] Complete support for no_entry and no_exit turn restrictions (#5988) The internal representation of turn restrictions expects only one `from` way and only one `to` way. `no_entry` and `no_exit` turn restrictions can have multiple `from` and `to` ways respectively. This means they are not fully supported by OSRM's restriction parser. We complete support for these turn restriction types by parsing all ways and converting a valid restriction with multiple `from`/`to` members into multiple internal restrictions. --- CHANGELOG.md | 1 + features/car/restrictions.feature | 120 ++++++++++++++++++++ include/extractor/restriction_parser.hpp | 2 +- src/extractor/restriction_parser.cpp | 84 +++++++++----- src/extractor/scripting_environment_lua.cpp | 8 +- 5 files changed, 184 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b577c71f..dc873de0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ - CHANGED: Docker build, enabled arm64 build layer [#6172](https://github.com/Project-OSRM/osrm-backend/pull/6172) - CHANGED: Docker build, enabled apt-get update/install caching in separate layer for build phase [#6175](https://github.com/Project-OSRM/osrm-backend/pull/6175) - Routing: + - FIXED: Completed support for no_entry and no_exit turn restrictions. [#5988](https://github.com/Project-OSRM/osrm-backend/pull/5988) - ADDED: Add support for non-round-trips with a single fixed endpoint. [#6050](https://github.com/Project-OSRM/osrm-backend/pull/6050) # 5.26.0 diff --git a/features/car/restrictions.feature b/features/car/restrictions.feature index da76f4ded..b9c878b00 100644 --- a/features/car/restrictions.feature +++ b/features/car/restrictions.feature @@ -1008,3 +1008,123 @@ Feature: Car - Turn restrictions | from | to | route | | d | x | bd,abc,xa,xa | | d | z | bd,abc,cz,cz | + + + Scenario: Multiple restricted entrances + Given the node map + """ + b + | + a----e----c + | + d + """ + + And the ways + | nodes | + | ae | + | be | + | ce | + | de | + + And the relations + | type | way:from | way:to | node:via | restriction | + | restriction | ae,be | ed | e | no_entry | + + When I route I should get + | from | to | route | + | a | d | ae,ce,ce,de,de | + | b | d | be,ce,ce,de,de | + | c | d | ce,de,de | + + + Scenario: Multiple restricted exits + Given the node map + """ + b + | + a----e----c + | + d + """ + + And the ways + | nodes | + | ae | + | be | + | ce | + | de | + + And the relations + | type | way:from | way:to | node:via | restriction | + | restriction | ae | ce,de | e | no_exit | + + When I route I should get + | from | to | route | + | a | b | ae,be,be | + | a | c | ae,be,be,ce,ce | + | a | d | ae,be,be,de,de | + + + Scenario: Invalid restricted entrances/exits + Given the node map + """ + b + | + a----e----c + | + d + """ + + And the ways + | nodes | + | ae | + | be | + | ce | + | de | + + And the relations + | type | way:from | way:to | node:via | restriction | + | restriction | ae | ce,de | e | no_entry | + | restriction | ae,be | ed | e | no_exit | + + When I route I should get + | from | to | route | + | a | b | ae,be,be | + | a | c | ae,ce,ce | + | a | d | ae,de,de | + | b | d | be,de,de | + | c | d | ce,de,de | + + + Scenario: Invalid multi from/to restrictions + Given the node map + """ + b + | + a----e----c + | + d + """ + + And the ways + | nodes | + | ae | + | be | + | ce | + | de | + + And the relations + | type | way:from | way:to | node:via | restriction | + | restriction | ae,de | ce,de | e | no_right_turn | + | restriction | ae,be | ce,de | e | no_straight_on | + | restriction | ae,be | be,ce | e | only_left_turn | + | restriction | ae,be | ce,de | e | only_straight_on | + + When I route I should get + | from | to | route | + | a | b | ae,be,be | + | a | c | ae,ce,ce | + | a | d | ae,de,de | + | b | d | be,de,de | + | c | d | ce,de,de | diff --git a/include/extractor/restriction_parser.hpp b/include/extractor/restriction_parser.hpp index 33367ae14..585396857 100644 --- a/include/extractor/restriction_parser.hpp +++ b/include/extractor/restriction_parser.hpp @@ -44,7 +44,7 @@ class RestrictionParser RestrictionParser(bool use_turn_restrictions, bool parse_conditionals, std::vector &restrictions); - boost::optional TryParse(const osmium::Relation &relation) const; + std::vector TryParse(const osmium::Relation &relation) const; private: bool ShouldIgnoreRestriction(const std::string &except_tag_string) const; diff --git a/src/extractor/restriction_parser.cpp b/src/extractor/restriction_parser.cpp index c81f4331d..a135214f5 100644 --- a/src/extractor/restriction_parser.cpp +++ b/src/extractor/restriction_parser.cpp @@ -50,13 +50,13 @@ RestrictionParser::RestrictionParser(bool use_turn_restrictions_, * in the corresponding profile. We use it for both namespacing restrictions, as in * restriction:motorcar as well as whitelisting if its in except:motorcar. */ -boost::optional +std::vector RestrictionParser::TryParse(const osmium::Relation &relation) const { // return if turn restrictions should be ignored if (!use_turn_restrictions) { - return boost::none; + return {}; } osmium::tags::KeyFilter filter(false); @@ -85,17 +85,19 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const // if it's not a restriction, continue; if (std::distance(fi_begin, fi_end) == 0) { - return boost::none; + return {}; } // check if the restriction should be ignored const char *except = relation.get_value_by_key("except"); if (except != nullptr && ShouldIgnoreRestriction(except)) { - return boost::none; + return {}; } bool is_only_restriction = false; + bool is_multi_from = false; + bool is_multi_to = false; for (; fi_begin != fi_end; ++fi_begin) { @@ -111,21 +113,26 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const else if (value.find("no_") == 0 && !boost::algorithm::ends_with(value, "_on_red")) { is_only_restriction = false; + if (boost::algorithm::starts_with(value, "no_exit")) + { + is_multi_to = true; + } + else if (boost::algorithm::starts_with(value, "no_entry")) + { + is_multi_from = true; + } } else // unrecognized value type { - return boost::none; + return {}; } } - InputTurnRestriction restriction_container; - restriction_container.is_only = is_only_restriction; - constexpr auto INVALID_OSM_ID = std::numeric_limits::max(); - auto from = INVALID_OSM_ID; + std::vector from_ways; auto via_node = INVALID_OSM_ID; std::vector via_ways; - auto to = INVALID_OSM_ID; + std::vector to_ways; bool is_node_restriction = true; for (const auto &member : relation.members()) @@ -157,11 +164,11 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const 0 == strcmp("via", role)); if (0 == strcmp("from", role)) { - from = static_cast(member.ref()); + from_ways.push_back({static_cast(member.ref())}); } else if (0 == strcmp("to", role)) { - to = static_cast(member.ref()); + to_ways.push_back({static_cast(member.ref())}); } else if (0 == strcmp("via", role)) { @@ -178,6 +185,7 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const } } + std::vector condition; // parse conditional tags if (parse_conditionals) { @@ -199,32 +207,54 @@ RestrictionParser::TryParse(const osmium::Relation &relation) const std::vector hours = util::ParseOpeningHours(p.condition); // found unrecognized condition, continue if (hours.empty()) - return boost::none; + return {}; - restriction_container.condition = std::move(hours); + condition = std::move(hours); } } } - if (from != INVALID_OSM_ID && (via_node != INVALID_OSM_ID || !via_ways.empty()) && - to != INVALID_OSM_ID) + std::vector restriction_containers; + if (!from_ways.empty() && (via_node != INVALID_OSM_ID || !via_ways.empty()) && !to_ways.empty()) { - if (is_node_restriction) + if (from_ways.size() > 1 && !is_multi_from) { - // template struct requires bracket for ID initialisation :( - restriction_container.node_or_way = InputNodeRestriction{{from}, {via_node}, {to}}; + util::Log(logDEBUG) << "Parsed restriction " << relation.id() + << " unexpectedly contains " << from_ways.size() + << " from ways, skipping..."; + return {}; } - else + if (to_ways.size() > 1 && !is_multi_to) { - // template struct requires bracket for ID initialisation :( - restriction_container.node_or_way = InputWayRestriction{{from}, via_ways, {to}}; + util::Log(logDEBUG) << "Parsed restriction " << relation.id() + << " unexpectedly contains " << to_ways.size() + << " to ways, skipping..."; + return {}; + } + // Internally restrictions are represented with one 'from' and one 'to' way. + // Therefore we need to convert a multi from/to restriction into multiple restrictions. + for (const auto &from : from_ways) + { + for (const auto &to : to_ways) + { + InputTurnRestriction restriction; + restriction.is_only = is_only_restriction; + restriction.condition = condition; + if (is_node_restriction) + { + // template struct requires bracket for ID initialisation :( + restriction.node_or_way = InputNodeRestriction{{from}, {via_node}, {to}}; + } + else + { + // template struct requires bracket for ID initialisation :( + restriction.node_or_way = InputWayRestriction{{from}, via_ways, {to}}; + } + restriction_containers.push_back(std::move(restriction)); + } } - return restriction_container; - } - else - { - return boost::none; } + return restriction_containers; } bool RestrictionParser::ShouldIgnoreRestriction(const std::string &except_tag_string) const diff --git a/src/extractor/scripting_environment_lua.cpp b/src/extractor/scripting_environment_lua.cpp index 2d0978d51..744140022 100644 --- a/src/extractor/scripting_environment_lua.cpp +++ b/src/extractor/scripting_environment_lua.cpp @@ -909,13 +909,15 @@ void Sol2ScriptingEnvironment::ProcessElements( case osmium::item_type::relation: { const auto &relation = static_cast(*entity); - if (auto result_res = restriction_parser.TryParse(relation)) + auto results = restriction_parser.TryParse(relation); + if (!results.empty()) { - resulting_restrictions.push_back(*result_res); + std::move( + results.begin(), results.end(), std::back_inserter(resulting_restrictions)); } else if (auto result_res = maneuver_override_parser.TryParse(relation)) { - resulting_maneuver_overrides.push_back(*result_res); + resulting_maneuver_overrides.push_back(std::move(*result_res)); } } break;