Skip to content

Commit

Permalink
Undo libosrm API break by adding old interface as method overload (Pr…
Browse files Browse the repository at this point in the history 
…oject-OSRM#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.
  • Loading branch information
@mjjbell
mjjbell authored Jan 27, 2021
1 parent b6557b8 commit bd3eb65
Show file tree
Hide file tree
Showing 10 changed files with 578 additions and 248 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
- Windows:
- FIXED: Fix bit-shift overflow in MLD partition step. [#5878](https://github.com/Project-OSRM/osrm-backend/pull/5878)
- FIXED: Fix vector bool permutation in graph contraction step [#5882](https://github.com/Project-OSRM/osrm-backend/pull/5882)

- API:
- FIXED: Undo libosrm API break by adding old interface as method overload [#5861](https://github.com/Project-OSRM/osrm-backend/pull/5861)

# 5.23.0
- Changes from 5.22.0
Expand Down
18 changes: 12 additions & 6 deletions include/osrm/osrm.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ class OSRM final
* \return Status indicating success for the query or failure
* \see Status, RouteParameters and json::Object
*/
Status Route(const RouteParameters &parameters, osrm::engine::api::ResultT &result) const;
Status Route(const RouteParameters &parameters, json::Object &result) const;
Status Route(const RouteParameters &parameters, engine::api::ResultT &result) const;

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

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

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

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

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

private:
std::unique_ptr<engine::EngineInterface> engine_;
Expand Down
30 changes: 24 additions & 6 deletions src/nodejs/node_osrm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,10 @@ inline void asyncForTiles(const Nan::FunctionCallbackInfo<v8::Value> &info,
// clang-format on
NAN_METHOD(Engine::route) //
{
async(info, &argumentsToRouteParameter, &osrm::OSRM::Route, true);
osrm::Status (osrm::OSRM::*route_fn)(const osrm::RouteParameters &params,
osrm::engine::api::ResultT &result) const =
&osrm::OSRM::Route;
async(info, &argumentsToRouteParameter, route_fn, true);
}

// clang-format off
Expand Down Expand Up @@ -346,7 +349,10 @@ NAN_METHOD(Engine::route) //
// clang-format on
NAN_METHOD(Engine::nearest) //
{
async(info, &argumentsToNearestParameter, &osrm::OSRM::Nearest, false);
osrm::Status (osrm::OSRM::*nearest_fn)(const osrm::NearestParameters &params,
osrm::engine::api::ResultT &result) const =
&osrm::OSRM::Nearest;
async(info, &argumentsToNearestParameter, nearest_fn, false);
}

// clang-format off
Expand Down Expand Up @@ -398,7 +404,10 @@ NAN_METHOD(Engine::nearest) //
// clang-format on
NAN_METHOD(Engine::table) //
{
async(info, &argumentsToTableParameter, &osrm::OSRM::Table, true);
osrm::Status (osrm::OSRM::*table_fn)(const osrm::TableParameters &params,
osrm::engine::api::ResultT &result) const =
&osrm::OSRM::Table;
async(info, &argumentsToTableParameter, table_fn, true);
}

// clang-format off
Expand Down Expand Up @@ -429,7 +438,10 @@ NAN_METHOD(Engine::table) //
// clang-format on
NAN_METHOD(Engine::tile)
{
asyncForTiles(info, &argumentsToTileParameters, &osrm::OSRM::Tile, {/*unused*/});
osrm::Status (osrm::OSRM::*tile_fn)(const osrm::TileParameters &params,
osrm::engine::api::ResultT &result) const =
&osrm::OSRM::Tile;
asyncForTiles(info, &argumentsToTileParameters, tile_fn, {/*unused*/});
}

// clang-format off
Expand Down Expand Up @@ -483,7 +495,10 @@ NAN_METHOD(Engine::tile)
// clang-format on
NAN_METHOD(Engine::match) //
{
async(info, &argumentsToMatchParameter, &osrm::OSRM::Match, true);
osrm::Status (osrm::OSRM::*match_fn)(const osrm::MatchParameters &params,
osrm::engine::api::ResultT &result) const =
&osrm::OSRM::Match;
async(info, &argumentsToMatchParameter, match_fn, true);
}

// clang-format off
Expand Down Expand Up @@ -553,7 +568,10 @@ NAN_METHOD(Engine::match) //
// clang-format on
NAN_METHOD(Engine::trip) //
{
async(info, &argumentsToTripParameter, &osrm::OSRM::Trip, true);
osrm::Status (osrm::OSRM::*trip_fn)(const osrm::TripParameters &params,
osrm::engine::api::ResultT &result) const =
&osrm::OSRM::Trip;
async(info, &argumentsToTripParameter, trip_fn, true);
}

/**
Expand Down
65 changes: 54 additions & 11 deletions src/osrm/osrm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,38 +56,81 @@ OSRM &OSRM::operator=(OSRM &&) noexcept = default;

// Forward to implementation

engine::Status OSRM::Route(const engine::api::RouteParameters &params,
osrm::engine::api::ResultT &result) const
Status OSRM::Route(const engine::api::RouteParameters &params, json::Object &json_result) const
{
osrm::engine::api::ResultT result = json::Object();
auto status = engine_->Route(params, result);
json_result = std::move(result.get<json::Object>());
return status;
}

Status OSRM::Route(const RouteParameters &params, engine::api::ResultT &result) const
{
return engine_->Route(params, result);
}

engine::Status OSRM::Table(const engine::api::TableParameters &params,
osrm::engine::api::ResultT &result) const
Status OSRM::Table(const engine::api::TableParameters &params, json::Object &json_result) const
{
osrm::engine::api::ResultT result = json::Object();
auto status = engine_->Table(params, result);
json_result = std::move(result.get<json::Object>());
return status;
}

Status OSRM::Table(const TableParameters &params, engine::api::ResultT &result) const
{
return engine_->Table(params, result);
}

engine::Status OSRM::Nearest(const engine::api::NearestParameters &params,
osrm::engine::api::ResultT &result) const
Status OSRM::Nearest(const engine::api::NearestParameters &params, json::Object &json_result) const
{
osrm::engine::api::ResultT result = json::Object();
auto status = engine_->Nearest(params, result);
json_result = std::move(result.get<json::Object>());
return status;
}

Status OSRM::Nearest(const NearestParameters &params, engine::api::ResultT &result) const
{
return engine_->Nearest(params, result);
}

Status OSRM::Trip(const engine::api::TripParameters &params, json::Object &json_result) const
{
osrm::engine::api::ResultT result = json::Object();
auto status = engine_->Trip(params, result);
json_result = std::move(result.get<json::Object>());
return status;
}

engine::Status OSRM::Trip(const engine::api::TripParameters &params,
osrm::engine::api::ResultT &result) const
engine::api::ResultT &result) const
{
return engine_->Trip(params, result);
}

engine::Status OSRM::Match(const engine::api::MatchParameters &params,
osrm::engine::api::ResultT &result) const
Status OSRM::Match(const engine::api::MatchParameters &params, json::Object &json_result) const
{
osrm::engine::api::ResultT result = json::Object();
auto status = engine_->Match(params, result);
json_result = std::move(result.get<json::Object>());
return status;
}

Status OSRM::Match(const MatchParameters &params, engine::api::ResultT &result) const
{
return engine_->Match(params, result);
}

engine::Status OSRM::Tile(const engine::api::TileParameters &params,
osrm::engine::api::ResultT &result) const
Status OSRM::Tile(const engine::api::TileParameters &params, std::string &str_result) const
{
osrm::engine::api::ResultT result = std::string();
auto status = engine_->Tile(params, result);
str_result = std::move(result.get<std::string>());
return status;
}

Status OSRM::Tile(const engine::api::TileParameters &params, engine::api::ResultT &result) const
{
return engine_->Tile(params, result);
}
Expand Down
46 changes: 30 additions & 16 deletions unit_tests/library/match.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,28 @@
#include "osrm/match_parameters.hpp"

#include "osrm/coordinate.hpp"
#include "osrm/engine_config.hpp"
#include "osrm/json_container.hpp"
#include "osrm/osrm.hpp"
#include "osrm/status.hpp"

osrm::Status run_match_json(const osrm::OSRM &osrm,
const MatchParameters &params,
json::Object &json_result,
bool use_json_only_api)
{
if (use_json_only_api)
{
return osrm.Match(params, json_result);
}
engine::api::ResultT result = json::Object();
auto rc = osrm.Match(params, result);
json_result = result.get<json::Object>();
return rc;
}

BOOST_AUTO_TEST_SUITE(match)

BOOST_AUTO_TEST_CASE(test_match)
void test_match(bool use_json_only_api)
{
using namespace osrm;

Expand All @@ -25,11 +39,9 @@ BOOST_AUTO_TEST_CASE(test_match)
params.coordinates.push_back(get_dummy_location());
params.coordinates.push_back(get_dummy_location());

engine::api::ResultT result = json::Object();
json::Object json_result;
const auto rc = run_match_json(osrm, params, json_result, use_json_only_api);

const auto rc = osrm.Match(params, result);

auto &json_result = result.get<json::Object>();
BOOST_CHECK(rc == Status::Ok || rc == Status::Error);
const auto code = json_result.values.at("code").get<json::String>().value;
BOOST_CHECK_EQUAL(code, "Ok");
Expand Down Expand Up @@ -63,8 +75,10 @@ BOOST_AUTO_TEST_CASE(test_match)
}
}
}
BOOST_AUTO_TEST_CASE(test_match_new_api) { test_match(false); }
BOOST_AUTO_TEST_CASE(test_match_old_api) { test_match(true); }

BOOST_AUTO_TEST_CASE(test_match_skip_waypoints)
void test_match_skip_waypoints(bool use_json_only_api)
{
using namespace osrm;

Expand All @@ -76,19 +90,19 @@ BOOST_AUTO_TEST_CASE(test_match_skip_waypoints)
params.coordinates.push_back(get_dummy_location());
params.coordinates.push_back(get_dummy_location());

engine::api::ResultT result = json::Object();

const auto rc = osrm.Match(params, result);
json::Object json_result;
const auto rc = run_match_json(osrm, params, json_result, use_json_only_api);

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

BOOST_CHECK(json_result.values.find("tracepoints") == json_result.values.end());
}
BOOST_AUTO_TEST_CASE(test_match_skip_waypoints_old_api) { test_match_skip_waypoints(true); }
BOOST_AUTO_TEST_CASE(test_match_skip_waypoints_new_api) { test_match_skip_waypoints(false); }

BOOST_AUTO_TEST_CASE(test_match_split)
void test_match_split(bool use_json_only_api)
{
using namespace osrm;

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

engine::api::ResultT result = json::Object();

const auto rc = osrm.Match(params, result);
json::Object json_result;
const auto rc = run_match_json(osrm, params, json_result, use_json_only_api);

auto &json_result = result.get<json::Object>();
BOOST_CHECK(rc == Status::Ok || rc == Status::Error);
const auto code = json_result.values.at("code").get<json::String>().value;
BOOST_CHECK_EQUAL(code, "Ok");
Expand Down Expand Up @@ -140,6 +152,8 @@ BOOST_AUTO_TEST_CASE(test_match_split)
}
}
}
BOOST_AUTO_TEST_CASE(test_match_split_old_api) { test_match_split(true); }
BOOST_AUTO_TEST_CASE(test_match_split_new_api) { test_match_split(false); }

BOOST_AUTO_TEST_CASE(test_match_fb_serialization)
{
Expand Down
Loading

0 comments on commit bd3eb65

Please sign in to comment.