Skip to content

Commit bd3eb65

Browse files
mjjbellmjjbell
authored
Undo libosrm API break by adding old interface as method overload (#5861)
Removes the breaking libosrm API change by adding the old interface to the new. This does not introduce any new breaks. The downside of this is that it allows for multiple ways to return JSON responses.
1 parent b6557b8 commit bd3eb65

File tree

10 files changed

+578
-248
lines changed

10 files changed

+578
-248
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
- Windows:
1717
- FIXED: Fix bit-shift overflow in MLD partition step. [#5878](https://github.com/Project-OSRM/osrm-backend/pull/5878)
1818
- FIXED: Fix vector bool permutation in graph contraction step [#5882](https://github.com/Project-OSRM/osrm-backend/pull/5882)
19-
19+
- API:
20+
- FIXED: Undo libosrm API break by adding old interface as method overload [#5861](https://github.com/Project-OSRM/osrm-backend/pull/5861)
2021

2122
# 5.23.0
2223
- Changes from 5.22.0

‎include/osrm/osrm.hpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ class OSRM final
8484
* \return Status indicating success for the query or failure
8585
* \see Status, RouteParameters and json::Object
8686
*/
87-
Status Route(const RouteParameters &parameters, osrm::engine::api::ResultT &result) const;
87+
Status Route(const RouteParameters &parameters, json::Object &result) const;
88+
Status Route(const RouteParameters &parameters, engine::api::ResultT &result) const;
8889

8990
/**
9091
* Distance tables for coordinates.
@@ -93,7 +94,8 @@ class OSRM final
9394
* \return Status indicating success for the query or failure
9495
* \see Status, TableParameters and json::Object
9596
*/
96-
Status Table(const TableParameters &parameters, osrm::engine::api::ResultT &result) const;
97+
Status Table(const TableParameters &parameters, json::Object &result) const;
98+
Status Table(const TableParameters &parameters, engine::api::ResultT &result) const;
9799

98100
/**
99101
* Nearest street segment for coordinate.
@@ -102,7 +104,8 @@ class OSRM final
102104
* \return Status indicating success for the query or failure
103105
* \see Status, NearestParameters and json::Object
104106
*/
105-
Status Nearest(const NearestParameters &parameters, osrm::engine::api::ResultT &result) const;
107+
Status Nearest(const NearestParameters &parameters, json::Object &result) const;
108+
Status Nearest(const NearestParameters &parameters, engine::api::ResultT &result) const;
106109

107110
/**
108111
* Trip: shortest round trip between coordinates.
@@ -111,7 +114,8 @@ class OSRM final
111114
* \return Status indicating success for the query or failure
112115
* \see Status, TripParameters and json::Object
113116
*/
114-
Status Trip(const TripParameters &parameters, osrm::engine::api::ResultT &result) const;
117+
Status Trip(const TripParameters &parameters, json::Object &result) const;
118+
Status Trip(const TripParameters &parameters, engine::api::ResultT &result) const;
115119

116120
/**
117121
* Match: snaps noisy coordinate traces to the road network
@@ -120,7 +124,8 @@ class OSRM final
120124
* \return Status indicating success for the query or failure
121125
* \see Status, MatchParameters and json::Object
122126
*/
123-
Status Match(const MatchParameters &parameters, osrm::engine::api::ResultT &result) const;
127+
Status Match(const MatchParameters &parameters, json::Object &result) const;
128+
Status Match(const MatchParameters &parameters, engine::api::ResultT &result) const;
124129

125130
/**
126131
* Tile: vector tiles with internal graph representation
@@ -129,7 +134,8 @@ class OSRM final
129134
* \return Status indicating success for the query or failure
130135
* \see Status, TileParameters and json::Object
131136
*/
132-
Status Tile(const TileParameters &parameters, osrm::engine::api::ResultT &result) const;
137+
Status Tile(const TileParameters &parameters, std::string &result) const;
138+
Status Tile(const TileParameters &parameters, engine::api::ResultT &result) const;
133139

134140
private:
135141
std::unique_ptr<engine::EngineInterface> engine_;

‎src/nodejs/node_osrm.cpp‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,10 @@ inline void asyncForTiles(const Nan::FunctionCallbackInfo<v8::Value> &info,
306306
// clang-format on
307307
NAN_METHOD(Engine::route) //
308308
{
309-
async(info, &argumentsToRouteParameter, &osrm::OSRM::Route, true);
309+
osrm::Status (osrm::OSRM::*route_fn)(const osrm::RouteParameters &params,
310+
osrm::engine::api::ResultT &result) const =
311+
&osrm::OSRM::Route;
312+
async(info, &argumentsToRouteParameter, route_fn, true);
310313
}
311314

312315
// clang-format off
@@ -346,7 +349,10 @@ NAN_METHOD(Engine::route) //
346349
// clang-format on
347350
NAN_METHOD(Engine::nearest) //
348351
{
349-
async(info, &argumentsToNearestParameter, &osrm::OSRM::Nearest, false);
352+
osrm::Status (osrm::OSRM::*nearest_fn)(const osrm::NearestParameters &params,
353+
osrm::engine::api::ResultT &result) const =
354+
&osrm::OSRM::Nearest;
355+
async(info, &argumentsToNearestParameter, nearest_fn, false);
350356
}
351357

352358
// clang-format off
@@ -398,7 +404,10 @@ NAN_METHOD(Engine::nearest) //
398404
// clang-format on
399405
NAN_METHOD(Engine::table) //
400406
{
401-
async(info, &argumentsToTableParameter, &osrm::OSRM::Table, true);
407+
osrm::Status (osrm::OSRM::*table_fn)(const osrm::TableParameters &params,
408+
osrm::engine::api::ResultT &result) const =
409+
&osrm::OSRM::Table;
410+
async(info, &argumentsToTableParameter, table_fn, true);
402411
}
403412

404413
// clang-format off
@@ -429,7 +438,10 @@ NAN_METHOD(Engine::table) //
429438
// clang-format on
430439
NAN_METHOD(Engine::tile)
431440
{
432-
asyncForTiles(info, &argumentsToTileParameters, &osrm::OSRM::Tile, {/*unused*/});
441+
osrm::Status (osrm::OSRM::*tile_fn)(const osrm::TileParameters &params,
442+
osrm::engine::api::ResultT &result) const =
443+
&osrm::OSRM::Tile;
444+
asyncForTiles(info, &argumentsToTileParameters, tile_fn, {/*unused*/});
433445
}
434446

435447
// clang-format off
@@ -483,7 +495,10 @@ NAN_METHOD(Engine::tile)
483495
// clang-format on
484496
NAN_METHOD(Engine::match) //
485497
{
486-
async(info, &argumentsToMatchParameter, &osrm::OSRM::Match, true);
498+
osrm::Status (osrm::OSRM::*match_fn)(const osrm::MatchParameters &params,
499+
osrm::engine::api::ResultT &result) const =
500+
&osrm::OSRM::Match;
501+
async(info, &argumentsToMatchParameter, match_fn, true);
487502
}
488503

489504
// clang-format off
@@ -553,7 +568,10 @@ NAN_METHOD(Engine::match) //
553568
// clang-format on
554569
NAN_METHOD(Engine::trip) //
555570
{
556-
async(info, &argumentsToTripParameter, &osrm::OSRM::Trip, true);
571+
osrm::Status (osrm::OSRM::*trip_fn)(const osrm::TripParameters &params,
572+
osrm::engine::api::ResultT &result) const =
573+
&osrm::OSRM::Trip;
574+
async(info, &argumentsToTripParameter, trip_fn, true);
557575
}
558576

559577
/**

‎src/osrm/osrm.cpp‎

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,38 +56,81 @@ OSRM &OSRM::operator=(OSRM &&) noexcept = default;
5656

5757
// Forward to implementation
5858

59-
engine::Status OSRM::Route(const engine::api::RouteParameters &params,
60-
osrm::engine::api::ResultT &result) const
59+
Status OSRM::Route(const engine::api::RouteParameters &params, json::Object &json_result) const
60+
{
61+
osrm::engine::api::ResultT result = json::Object();
62+
auto status = engine_->Route(params, result);
63+
json_result = std::move(result.get<json::Object>());
64+
return status;
65+
}
66+
67+
Status OSRM::Route(const RouteParameters &params, engine::api::ResultT &result) const
6168
{
6269
return engine_->Route(params, result);
6370
}
6471

65-
engine::Status OSRM::Table(const engine::api::TableParameters &params,
66-
osrm::engine::api::ResultT &result) const
72+
Status OSRM::Table(const engine::api::TableParameters &params, json::Object &json_result) const
73+
{
74+
osrm::engine::api::ResultT result = json::Object();
75+
auto status = engine_->Table(params, result);
76+
json_result = std::move(result.get<json::Object>());
77+
return status;
78+
}
79+
80+
Status OSRM::Table(const TableParameters &params, engine::api::ResultT &result) const
6781
{
6882
return engine_->Table(params, result);
6983
}
7084

71-
engine::Status OSRM::Nearest(const engine::api::NearestParameters &params,
72-
osrm::engine::api::ResultT &result) const
85+
Status OSRM::Nearest(const engine::api::NearestParameters &params, json::Object &json_result) const
86+
{
87+
osrm::engine::api::ResultT result = json::Object();
88+
auto status = engine_->Nearest(params, result);
89+
json_result = std::move(result.get<json::Object>());
90+
return status;
91+
}
92+
93+
Status OSRM::Nearest(const NearestParameters &params, engine::api::ResultT &result) const
7394
{
7495
return engine_->Nearest(params, result);
7596
}
7697

98+
Status OSRM::Trip(const engine::api::TripParameters &params, json::Object &json_result) const
99+
{
100+
osrm::engine::api::ResultT result = json::Object();
101+
auto status = engine_->Trip(params, result);
102+
json_result = std::move(result.get<json::Object>());
103+
return status;
104+
}
105+
77106
engine::Status OSRM::Trip(const engine::api::TripParameters &params,
78-
osrm::engine::api::ResultT &result) const
107+
engine::api::ResultT &result) const
79108
{
80109
return engine_->Trip(params, result);
81110
}
82111

83-
engine::Status OSRM::Match(const engine::api::MatchParameters &params,
84-
osrm::engine::api::ResultT &result) const
112+
Status OSRM::Match(const engine::api::MatchParameters &params, json::Object &json_result) const
113+
{
114+
osrm::engine::api::ResultT result = json::Object();
115+
auto status = engine_->Match(params, result);
116+
json_result = std::move(result.get<json::Object>());
117+
return status;
118+
}
119+
120+
Status OSRM::Match(const MatchParameters &params, engine::api::ResultT &result) const
85121
{
86122
return engine_->Match(params, result);
87123
}
88124

89-
engine::Status OSRM::Tile(const engine::api::TileParameters &params,
90-
osrm::engine::api::ResultT &result) const
125+
Status OSRM::Tile(const engine::api::TileParameters &params, std::string &str_result) const
126+
{
127+
osrm::engine::api::ResultT result = std::string();
128+
auto status = engine_->Tile(params, result);
129+
str_result = std::move(result.get<std::string>());
130+
return status;
131+
}
132+
133+
Status OSRM::Tile(const engine::api::TileParameters &params, engine::api::ResultT &result) const
91134
{
92135
return engine_->Tile(params, result);
93136
}

‎unit_tests/library/match.cpp‎

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,28 @@
77
#include "osrm/match_parameters.hpp"
88

99
#include "osrm/coordinate.hpp"
10-
#include "osrm/engine_config.hpp"
1110
#include "osrm/json_container.hpp"
1211
#include "osrm/osrm.hpp"
1312
#include "osrm/status.hpp"
1413

14+
osrm::Status run_match_json(const osrm::OSRM &osrm,
15+
const MatchParameters &params,
16+
json::Object &json_result,
17+
bool use_json_only_api)
18+
{
19+
if (use_json_only_api)
20+
{
21+
return osrm.Match(params, json_result);
22+
}
23+
engine::api::ResultT result = json::Object();
24+
auto rc = osrm.Match(params, result);
25+
json_result = result.get<json::Object>();
26+
return rc;
27+
}
28+
1529
BOOST_AUTO_TEST_SUITE(match)
1630

17-
BOOST_AUTO_TEST_CASE(test_match)
31+
void test_match(bool use_json_only_api)
1832
{
1933
using namespace osrm;
2034

@@ -25,11 +39,9 @@ BOOST_AUTO_TEST_CASE(test_match)
2539
params.coordinates.push_back(get_dummy_location());
2640
params.coordinates.push_back(get_dummy_location());
2741

28-
engine::api::ResultT result = json::Object();
42+
json::Object json_result;
43+
const auto rc = run_match_json(osrm, params, json_result, use_json_only_api);
2944

30-
const auto rc = osrm.Match(params, result);
31-
32-
auto &json_result = result.get<json::Object>();
3345
BOOST_CHECK(rc == Status::Ok || rc == Status::Error);
3446
const auto code = json_result.values.at("code").get<json::String>().value;
3547
BOOST_CHECK_EQUAL(code, "Ok");
@@ -63,8 +75,10 @@ BOOST_AUTO_TEST_CASE(test_match)
6375
}
6476
}
6577
}
78+
BOOST_AUTO_TEST_CASE(test_match_new_api) { test_match(false); }
79+
BOOST_AUTO_TEST_CASE(test_match_old_api) { test_match(true); }
6680

67-
BOOST_AUTO_TEST_CASE(test_match_skip_waypoints)
81+
void test_match_skip_waypoints(bool use_json_only_api)
6882
{
6983
using namespace osrm;
7084

@@ -76,19 +90,19 @@ BOOST_AUTO_TEST_CASE(test_match_skip_waypoints)
7690
params.coordinates.push_back(get_dummy_location());
7791
params.coordinates.push_back(get_dummy_location());
7892

79-
engine::api::ResultT result = json::Object();
80-
81-
const auto rc = osrm.Match(params, result);
93+
json::Object json_result;
94+
const auto rc = run_match_json(osrm, params, json_result, use_json_only_api);
8295

83-
auto &json_result = result.get<json::Object>();
8496
BOOST_CHECK(rc == Status::Ok || rc == Status::Error);
8597
const auto code = json_result.values.at("code").get<json::String>().value;
8698
BOOST_CHECK_EQUAL(code, "Ok");
8799

88100
BOOST_CHECK(json_result.values.find("tracepoints") == json_result.values.end());
89101
}
102+
BOOST_AUTO_TEST_CASE(test_match_skip_waypoints_old_api) { test_match_skip_waypoints(true); }
103+
BOOST_AUTO_TEST_CASE(test_match_skip_waypoints_new_api) { test_match_skip_waypoints(false); }
90104

91-
BOOST_AUTO_TEST_CASE(test_match_split)
105+
void test_match_split(bool use_json_only_api)
92106
{
93107
using namespace osrm;
94108

@@ -98,11 +112,9 @@ BOOST_AUTO_TEST_CASE(test_match_split)
98112
params.coordinates = get_split_trace_locations();
99113
params.timestamps = {1, 2, 1700, 1800};
100114

101-
engine::api::ResultT result = json::Object();
102-
103-
const auto rc = osrm.Match(params, result);
115+
json::Object json_result;
116+
const auto rc = run_match_json(osrm, params, json_result, use_json_only_api);
104117

105-
auto &json_result = result.get<json::Object>();
106118
BOOST_CHECK(rc == Status::Ok || rc == Status::Error);
107119
const auto code = json_result.values.at("code").get<json::String>().value;
108120
BOOST_CHECK_EQUAL(code, "Ok");
@@ -140,6 +152,8 @@ BOOST_AUTO_TEST_CASE(test_match_split)
140152
}
141153
}
142154
}
155+
BOOST_AUTO_TEST_CASE(test_match_split_old_api) { test_match_split(true); }
156+
BOOST_AUTO_TEST_CASE(test_match_split_new_api) { test_match_split(false); }
143157

144158
BOOST_AUTO_TEST_CASE(test_match_fb_serialization)
145159
{

0 commit comments

Comments
 (0)