From 89f6e2d55b938c13ec99867cf5d044683be8a50d Mon Sep 17 00:00:00 2001 From: Kajari Ghosh Date: Tue, 24 Apr 2018 11:05:35 -0400 Subject: [PATCH] Parse table annotations param correctly (#5050) * fix incorrect parameter parsing for node osrm and add tests * fix boost spirit grammar parsing for annotations * return NotImplemented when distance annotation is requested for MLD in table plugin * update docs --- docs/http.md | 12 +++-- docs/nodejs/api.md | 2 - features/support/hooks.js | 2 +- include/engine/algorithm.hpp | 9 ++++ include/engine/api/table_parameters.hpp | 4 +- include/engine/routing_algorithms.hpp | 6 +++ include/nodejs/node_osrm_support.hpp | 2 + .../server/api/table_parameter_grammar.hpp | 11 ++--- src/engine/plugins/table.cpp | 10 +++- src/nodejs/node_osrm.cpp | 15 +++--- test/nodejs/table.js | 49 +++++++++++++++++-- 11 files changed, 95 insertions(+), 27 deletions(-) diff --git a/docs/http.md b/docs/http.md index 4eb9b60d4..480019473 100644 --- a/docs/http.md +++ b/docs/http.md @@ -222,7 +222,7 @@ curl 'http://router.project-osrm.org/route/v1/driving/13.388860,52.517037;13.397 ### Table service -Computes the duration of the fastest route between all pairs of supplied coordinates. Optionally, also returns the distances between the coordinate pairs. Note that the distances are not the shortest distance between two coordinates, but rather the distances of the fastest routes. +Computes the duration of the fastest route between all pairs of supplied coordinates. Returns the durations or distances or both between the coordinate pairs. Note that the distances are not the shortest distance between two coordinates, but rather the distances of the fastest routes. Duration is in seconds and distances is in meters. ```endpoint GET /table/v1/{profile}/{coordinates}?{sources}=[{elem}...];&{destinations}=[{elem}...]&annotations={duration|distance|duration,distance} @@ -236,7 +236,8 @@ In addition to the [general options](#general-options) the following options are |------------|--------------------------------------------------|---------------------------------------------| |sources |`{index};{index}[;{index} ...]` or `all` (default)|Use location with given index as source. | |destinations|`{index};{index}[;{index} ...]` or `all` (default)|Use location with given index as destination.| -|annotations |`duration` (default), `distance`, or `duration,distance`|Return additional table with distances to the response. Whether requested or not, the duration table is always returned.| +|annotations |`duration` (default), `distance`, or `duration,distance`|Return the requested table or tables in response. Note that computing the `distances` table is currently only implemented for CH. If `annotations=distance` or `annotations=duration,distance` is requested when running a MLD router, a `NotImplemented` error will be returned. +| Unlike other array encoded options, the length of `sources` and `destinations` can be **smaller or equal** to number of input locations; @@ -266,10 +267,10 @@ curl 'http://router.project-osrm.org/table/v1/driving/polyline(egs_Iq_aqAppHzbHu # Returns a 3x3 duration matrix: curl 'http://router.project-osrm.org/table/v1/driving/13.388860,52.517037;13.397634,52.529407;13.428555,52.523219&annotations=duration' -# Returns a 3x3 distance matrix: +# Returns a 3x3 distance matrix for CH: curl 'http://router.project-osrm.org/table/v1/driving/13.388860,52.517037;13.397634,52.529407;13.428555,52.523219&annotations=distance' -# Returns a 3x3 duration matrix and a 3x3 distance matrix: +# Returns a 3x3 duration matrix and a 3x3 distance matrix for CH: curl 'http://router.project-osrm.org/table/v1/driving/13.388860,52.517037;13.397634,52.529407;13.428555,52.523219&annotations=distance,duration' ``` @@ -279,7 +280,7 @@ curl 'http://router.project-osrm.org/table/v1/driving/13.388860,52.517037;13.397 - `durations` array of arrays that stores the matrix in row-major order. `durations[i][j]` gives the travel time from the i-th waypoint to the j-th waypoint. Values are given in seconds. Can be `null` if no route between `i` and `j` can be found. - `distances` array of arrays that stores the matrix in row-major order. `distances[i][j]` gives the travel distance from - the i-th waypoint to the j-th waypoint. Values are given in meters. Can be `null` if no route between `i` and `j` can be found. + the i-th waypoint to the j-th waypoint. Values are given in meters. Can be `null` if no route between `i` and `j` can be found. Note that computing the `distances` table is currently only implemented for CH. If `annotations=distance` or `annotations=duration,distance` is requested when running a MLD router, a `NotImplemented` error will be returned. - `sources` array of `Waypoint` objects describing all sources in order - `destinations` array of `Waypoint` objects describing all destinations in order @@ -288,6 +289,7 @@ In case of error the following `code`s are supported in addition to the general | Type | Description | |------------------|-----------------| | `NoTable` | No route found. | +| `NotImplemented` | This request is not supported | All other properties might be undefined. diff --git a/docs/nodejs/api.md b/docs/nodejs/api.md index 8e864ce1a..01ef653cb 100644 --- a/docs/nodejs/api.md +++ b/docs/nodejs/api.md @@ -126,7 +126,6 @@ tables. Optionally returns distance table. location with given index as source. Default is to use all. - `options.destinations` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** An array of `index` elements (`0 <= integer < #coordinates`) to use location with given index as destination. Default is to use all. - - `options.annotations` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** An array of the table types to return. Values can be `duration` or `distance` or both. Default is to return only duration table. - `options.approaches` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Keep waypoints on curb side. Can be `null` (unrestricted, default) or `curb`. - `callback` **[Function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function)** @@ -143,7 +142,6 @@ var options = { }; osrm.table(options, function(err, response) { console.log(response.durations); // array of arrays, matrix in row-major order - console.log(response.distances); // array of arrays, matrix in row-major order console.log(response.sources); // array of Waypoint objects console.log(response.destinations); // array of Waypoint objects }); diff --git a/features/support/hooks.js b/features/support/hooks.js index 7bd8116cb..01c2e6e89 100644 --- a/features/support/hooks.js +++ b/features/support/hooks.js @@ -51,7 +51,7 @@ module.exports = function () { .defer(rimraf, this.scenarioLogFile) .awaitAll(callback); // uncomment to get path to logfile - console.log(' Writing logging output to ' + this.scenarioLogFile); + // console.log(' Writing logging output to ' + this.scenarioLogFile); }); this.After((scenario, callback) => { diff --git a/include/engine/algorithm.hpp b/include/engine/algorithm.hpp index 8f48b3730..760f78e80 100644 --- a/include/engine/algorithm.hpp +++ b/include/engine/algorithm.hpp @@ -50,6 +50,9 @@ template struct HasMapMatching final : std::false_type template struct HasManyToManySearch final : std::false_type { }; +template struct SupportsDistanceAnnotationType final : std::false_type +{ +}; template struct HasGetTileTurns final : std::false_type { }; @@ -73,6 +76,9 @@ template <> struct HasMapMatching final : std::true_type template <> struct HasManyToManySearch final : std::true_type { }; +template <> struct SupportsDistanceAnnotationType final : std::true_type +{ +}; template <> struct HasGetTileTurns final : std::true_type { }; @@ -96,6 +102,9 @@ template <> struct HasMapMatching final : std::true_type template <> struct HasManyToManySearch final : std::true_type { }; +template <> struct SupportsDistanceAnnotationType final : std::false_type +{ +}; template <> struct HasGetTileTurns final : std::true_type { }; diff --git a/include/engine/api/table_parameters.hpp b/include/engine/api/table_parameters.hpp index 0e4cb12ae..4f1a496bd 100644 --- a/include/engine/api/table_parameters.hpp +++ b/include/engine/api/table_parameters.hpp @@ -135,8 +135,8 @@ inline TableParameters::AnnotationsType operator|(TableParameters::AnnotationsTy static_cast>(rhs)); } -inline TableParameters::AnnotationsType operator|=(TableParameters::AnnotationsType lhs, - TableParameters::AnnotationsType rhs) +inline TableParameters::AnnotationsType &operator|=(TableParameters::AnnotationsType &lhs, + TableParameters::AnnotationsType rhs) { return lhs = lhs | rhs; } diff --git a/include/engine/routing_algorithms.hpp b/include/engine/routing_algorithms.hpp index b2a3d9a3f..aec19f753 100644 --- a/include/engine/routing_algorithms.hpp +++ b/include/engine/routing_algorithms.hpp @@ -55,6 +55,7 @@ class RoutingAlgorithmsInterface virtual bool HasDirectShortestPathSearch() const = 0; virtual bool HasMapMatching() const = 0; virtual bool HasManyToManySearch() const = 0; + virtual bool SupportsDistanceAnnotationType() const = 0; virtual bool HasGetTileTurns() const = 0; virtual bool HasExcludeFlags() const = 0; virtual bool IsValid() const = 0; @@ -128,6 +129,11 @@ template class RoutingAlgorithms final : public RoutingAlgo return routing_algorithms::HasManyToManySearch::value; } + bool SupportsDistanceAnnotationType() const final override + { + return routing_algorithms::SupportsDistanceAnnotationType::value; + } + bool HasGetTileTurns() const final override { return routing_algorithms::HasGetTileTurns::value; diff --git a/include/nodejs/node_osrm_support.hpp b/include/nodejs/node_osrm_support.hpp index 72bee7ce7..6ffea408b 100644 --- a/include/nodejs/node_osrm_support.hpp +++ b/include/nodejs/node_osrm_support.hpp @@ -1077,6 +1077,8 @@ argumentsToTableParameter(const Nan::FunctionCallbackInfo &args, return table_parameters_ptr(); } + params->annotations = osrm::TableParameters::AnnotationsType::None; + v8::Local annotations_array = v8::Local::Cast(annotations); for (std::size_t i = 0; i < annotations_array->Length(); ++i) { diff --git a/include/server/api/table_parameter_grammar.hpp b/include/server/api/table_parameter_grammar.hpp index e922f965b..5be793645 100644 --- a/include/server/api/table_parameter_grammar.hpp +++ b/include/server/api/table_parameter_grammar.hpp @@ -58,17 +58,15 @@ struct TableParametersGrammar : public BaseParametersGrammar= 0`. * @param {Array} [options.hints] Hints for the coordinate snapping. Array of base64 encoded strings. - * @param {Array} [options.sources] An array of `index` elements (`0 <= integer < #coordinates`) to - * use - * location with given index as source. Default is to use all. - * @param {Array} [options.destinations] An array of `index` elements (`0 <= integer < - * #coordinates`) to use location with given index as destination. Default is to use all. + * @param {Array} [options.sources] An array of `index` elements (`0 <= integer < #coordinates`) to use + * location with given index as source. Default is to use all. + * @param {Array} [options.destinations] An array of `index` elements (`0 <= integer < #coordinates`) to use location with given index as destination. Default is to use all. * @param {Array} [options.approaches] Keep waypoints on curb side. Can be `null` (unrestricted, default) or `curb`. + * @param {Array} [options.annotations] An array of the table types to return. Values can be `duration` or `distance` or both. If no annotations parameter is added, the default is to return the `durations` table. If `annotations=distance` or `annotations=duration,distance` is requested when running a MLD router, a `NotImplemented` error will be returned. + * @param {Function} callback * * @returns {Object} containing `durations`, `sources`, and `destinations`. * **`durations`**: array of arrays that stores the matrix in row-major order. `durations[i][j]` gives the travel time from the i-th waypoint to the j-th waypoint. * Values are given in seconds. + * **`distances`**: array of arrays that stores the matrix in row-major order. `distances[i][j]` gives the travel time from the i-th waypoint to the j-th waypoint. + * Values are given in meters. Note that computing the `distances` table is currently only implemented for CH. If `annotations=distance` or + * `annotations=duration,distance` is requested when running a MLD router, a `NotImplemented` error will be returned. * **`sources`**: array of [`Ẁaypoint`](#waypoint) objects describing all sources in order. * **`destinations`**: array of [`Ẁaypoint`](#waypoint) objects describing all destinations in order. * @@ -299,7 +302,7 @@ NAN_METHOD(Engine::nearest) // * }; * osrm.table(options, function(err, response) { * console.log(response.durations); // array of arrays, matrix in row-major order - * console.log(response.distances); // array of arrays, matrix in row-major order + * console.log(response.distances); // array of arrays, matrix in row-major order (currently only implemented for CH router) * console.log(response.sources); // array of Waypoint objects * console.log(response.destinations); // array of Waypoint objects * }); diff --git a/test/nodejs/table.js b/test/nodejs/table.js index f053001ec..b053443a7 100644 --- a/test/nodejs/table.js +++ b/test/nodejs/table.js @@ -5,6 +5,48 @@ var mld_data_path = require('./constants').mld_data_path; var three_test_coordinates = require('./constants').three_test_coordinates; var two_test_coordinates = require('./constants').two_test_coordinates; +test('table: test annotations paramater combination', function(assert) { + assert.plan(12); + var osrm = new OSRM(data_path); + var options = { + coordinates: [three_test_coordinates[0], three_test_coordinates[1]], + annotations: ['distance'] + }; + osrm.table(options, function(err, table) { + assert.ifError(err); + assert.ok(table['distances'], 'distances table result should exist'); + assert.notOk(table['durations'], 'durations table result should not exist'); + }); + + options = { + coordinates: [three_test_coordinates[0], three_test_coordinates[1]], + annotations: ['duration'] + }; + osrm.table(options, function(err, table) { + assert.ifError(err); + assert.ok(table['durations'], 'durations table result should exist'); + assert.notOk(table['distances'], 'distances table result should not exist'); + }); + + options = { + coordinates: [three_test_coordinates[0], three_test_coordinates[1]], + annotations: ['duration', 'distance'] + }; + osrm.table(options, function(err, table) { + assert.ifError(err); + assert.ok(table['durations'], 'durations table result should exist'); + assert.ok(table['distances'], 'distances table result should exist'); + }); + + options = { + coordinates: [three_test_coordinates[0], three_test_coordinates[1]] + }; + osrm.table(options, function(err, table) { + assert.ifError(err); + assert.ok(table['durations'], 'durations table result should exist'); + assert.notOk(table['distances'], 'distances table result should not exist'); + }); +}); var tables = ['distances', 'durations']; @@ -143,7 +185,6 @@ tables.forEach(function(annotation) { annotations: [annotation.slice(0,-1)] }; osrm.table(options, function(err, table) { - console.log(table); assert.ifError(err); function assertHasHints(waypoint) { @@ -176,7 +217,7 @@ tables.forEach(function(annotation) { }); test('table: ' + annotation + ' table in Monaco without motorways', function(assert) { - assert.plan(2); + assert.plan(1); var osrm = new OSRM({path: mld_data_path, algorithm: 'MLD'}); var options = { coordinates: two_test_coordinates, @@ -184,8 +225,8 @@ tables.forEach(function(annotation) { annotations: [annotation.slice(0,-1)] }; osrm.table(options, function(err, response) { - assert.ifError(err); - assert.equal(response[annotation].length, 2); + if (annotation === 'durations') assert.equal(response[annotation].length, 2); + else assert.error(response, 'NotImplemented'); }); }); });