From fb1bb7a15b839f76230a7c797ac6fd6131d25280 Mon Sep 17 00:00:00 2001 From: Siarhei Fedartsou Date: Wed, 19 Oct 2022 08:35:18 +0200 Subject: [PATCH] Fix annotations=true handling in NodeJS bindings & libosrm (#6415) --- CHANGELOG.md | 1 + include/engine/api/route_api.hpp | 4 ++-- include/nodejs/node_osrm_support.hpp | 6 ++++++ test/nodejs/route.js | 14 ++++++++++---- unit_tests/library/route.cpp | 2 +- 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0db1f24f6..49ffcfbc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased - Changes from 5.27.1 - Misc: + - FIXED: Fix annotations=true handling in NodeJS bindings & libosrm. [#6415](https://github.com/Project-OSRM/osrm-backend/pull/6415/) - FIXED: Fix bindings compilation issue on the latest Node. Update NAN to 2.17.0. [#6416](https://github.com/Project-OSRM/osrm-backend/pull/6416) # 5.27.1 - Changes from 5.27.0 diff --git a/include/engine/api/route_api.hpp b/include/engine/api/route_api.hpp index 619c2624d..044ea0b1b 100644 --- a/include/engine/api/route_api.hpp +++ b/include/engine/api/route_api.hpp @@ -439,7 +439,7 @@ class RouteAPI : public BaseAPI { // AnnotationsType uses bit flags, & operator checks if a property is set flatbuffers::Offset> speed; - if (parameters.annotations_type & RouteParameters::AnnotationsType::Speed) + if (requested_annotations & RouteParameters::AnnotationsType::Speed) { double prev_speed = 0; speed = @@ -778,7 +778,7 @@ class RouteAPI : public BaseAPI util::json::Object annotation; // AnnotationsType uses bit flags, & operator checks if a property is set - if (parameters.annotations_type & RouteParameters::AnnotationsType::Speed) + if (requested_annotations & RouteParameters::AnnotationsType::Speed) { double prev_speed = 0; annotation.values["speed"] = GetAnnotations( diff --git a/include/nodejs/node_osrm_support.hpp b/include/nodejs/node_osrm_support.hpp index f1e288226..c9db2372d 100644 --- a/include/nodejs/node_osrm_support.hpp +++ b/include/nodejs/node_osrm_support.hpp @@ -799,6 +799,9 @@ inline bool parseCommonParameters(const v8::Local &obj, ParamType &p if (annotations->IsBoolean()) { params->annotations = Nan::To(annotations).FromJust(); + params->annotations_type = params->annotations + ? osrm::RouteParameters::AnnotationsType::All + : osrm::RouteParameters::AnnotationsType::None; } else if (annotations->IsArray()) { @@ -845,6 +848,9 @@ inline bool parseCommonParameters(const v8::Local &obj, ParamType &p Nan::ThrowError("this 'annotations' param is not supported"); return false; } + + params->annotations = + params->annotations_type != osrm::RouteParameters::AnnotationsType::None; } } else diff --git a/test/nodejs/route.js b/test/nodejs/route.js index 0e4033843..d0f1ddf49 100644 --- a/test/nodejs/route.js +++ b/test/nodejs/route.js @@ -286,9 +286,9 @@ test('route: routes Monaco with several (duration, distance, nodes) annotations assert.ok(first.routes[0].legs.every(l => { return l.annotation.distance;}), 'every leg has annotations for distance'); assert.ok(first.routes[0].legs.every(l => { return l.annotation.duration;}), 'every leg has annotations for durations'); assert.ok(first.routes[0].legs.every(l => { return l.annotation.nodes;}), 'every leg has annotations for nodes'); - assert.notOk(first.routes[0].legs.every(l => { return l.annotation.weight; }), 'has no annotations for weight') - assert.notOk(first.routes[0].legs.every(l => { return l.annotation.datasources; }), 'has no annotations for datasources') - assert.notOk(first.routes[0].legs.every(l => { return l.annotation.speed; }), 'has no annotations for speed') + assert.notOk(first.routes[0].legs.every(l => { return l.annotation.weight; }), 'has no annotations for weight'); + assert.notOk(first.routes[0].legs.every(l => { return l.annotation.datasources; }), 'has no annotations for datasources'); + assert.notOk(first.routes[0].legs.every(l => { return l.annotation.speed; }), 'has no annotations for speed'); options.overview = 'full'; osrm.route(options, function(err, full) { @@ -303,7 +303,7 @@ test('route: routes Monaco with several (duration, distance, nodes) annotations }); test('route: routes Monaco with options', function(assert) { - assert.plan(11); + assert.plan(17); var osrm = new OSRM(monaco_path); var options = { coordinates: two_test_coordinates, @@ -322,6 +322,12 @@ test('route: routes Monaco with options', function(assert) { assert.ok(first.routes[0].legs[0]); assert.ok(first.routes[0].legs.every(l => { return l.steps.length > 0; }), 'every leg has steps'); assert.ok(first.routes[0].legs.every(l => { return l.annotation;}), 'every leg has annotations'); + assert.ok(first.routes[0].legs.every(l => { return l.annotation.distance;}), 'every leg has annotations for distance'); + assert.ok(first.routes[0].legs.every(l => { return l.annotation.duration;}), 'every leg has annotations for durations'); + assert.ok(first.routes[0].legs.every(l => { return l.annotation.nodes;}), 'every leg has annotations for nodes'); + assert.ok(first.routes[0].legs.every(l => { return l.annotation.weight; }), 'every leg has annotations for weight'); + assert.ok(first.routes[0].legs.every(l => { return l.annotation.datasources; }), 'every leg has annotations for datasources'); + assert.ok(first.routes[0].legs.every(l => { return l.annotation.speed; }), 'every leg has annotations for speed'); options.overview = 'full'; osrm.route(options, function(err, full) { diff --git a/unit_tests/library/route.cpp b/unit_tests/library/route.cpp index 0e4798523..cc823816d 100644 --- a/unit_tests/library/route.cpp +++ b/unit_tests/library/route.cpp @@ -584,7 +584,7 @@ void test_manual_setting_of_annotations_property(bool use_json_only_api) .values["annotation"] .get() .values; - BOOST_CHECK_EQUAL(annotations.size(), 6); + BOOST_CHECK_EQUAL(annotations.size(), 7); } BOOST_AUTO_TEST_CASE(test_manual_setting_of_annotations_property_old_api) {