Skip to content

Fix json rendering of large osm ids #7142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions include/engine/api/nearest_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ class NearestAPI final : public BaseAPI
auto &phantom_node = phantom_with_distance.phantom_node;

auto node_values = MakeNodes(phantom_node);
fbresult::Uint64Pair nodes{node_values.first, node_values.second};
fbresult::Uint64Pair nodes{from_alias<uint64_t>(node_values.first),
from_alias<uint64_t>(node_values.second)};

auto waypoint = MakeWaypoint(&fb_result, {phantom_node});
waypoint->add_nodes(&nodes);
Expand Down Expand Up @@ -123,10 +124,10 @@ class NearestAPI final : public BaseAPI
const NearestParameters &parameters;

protected:
std::pair<uint64_t, uint64_t> MakeNodes(const PhantomNode &phantom_node) const
std::pair<OSMNodeID, OSMNodeID> MakeNodes(const PhantomNode &phantom_node) const
{
std::uint64_t from_node = 0;
std::uint64_t to_node = 0;
OSMNodeID from_node = OSMNodeID{0};
OSMNodeID to_node = OSMNodeID{0};

datafacade::BaseDataFacade::NodeForwardRange forward_geometry;
if (phantom_node.forward_segment_id.enabled)
Expand All @@ -135,26 +136,22 @@ class NearestAPI final : public BaseAPI
const auto geometry_id = facade.GetGeometryIndex(segment_id).id;
forward_geometry = facade.GetUncompressedForwardGeometry(geometry_id);

auto osm_node_id =
to_node =
facade.GetOSMNodeIDOfNode(forward_geometry[phantom_node.fwd_segment_position]);
to_node = static_cast<std::uint64_t>(osm_node_id);
}

if (phantom_node.reverse_segment_id.enabled)
{
auto segment_id = phantom_node.reverse_segment_id.id;
const auto geometry_id = facade.GetGeometryIndex(segment_id).id;
const auto geometry = facade.GetUncompressedForwardGeometry(geometry_id);
auto osm_node_id =
facade.GetOSMNodeIDOfNode(geometry[phantom_node.fwd_segment_position + 1]);
from_node = static_cast<std::uint64_t>(osm_node_id);
from_node = facade.GetOSMNodeIDOfNode(geometry[phantom_node.fwd_segment_position + 1]);
}
else if (phantom_node.forward_segment_id.enabled && phantom_node.fwd_segment_position > 0)
{
// In the case of one way, rely on forward segment only
auto osm_node_id =
from_node =
facade.GetOSMNodeIDOfNode(forward_geometry[phantom_node.fwd_segment_position - 1]);
from_node = static_cast<std::uint64_t>(osm_node_id);
}

return std::make_pair(from_node, to_node);
Expand Down
3 changes: 1 addition & 2 deletions include/engine/api/route_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -842,8 +842,7 @@ class RouteAPI : public BaseAPI
nodes.values.reserve(leg_geometry.node_ids.size());
for (const auto node_id : leg_geometry.node_ids)
{
nodes.values.push_back(
static_cast<std::uint64_t>(facade.GetOSMNodeIDOfNode(node_id)));
nodes.values.push_back(facade.GetOSMNodeIDOfNode(node_id));
}
annotation.values.emplace("nodes", std::move(nodes));
}
Expand Down
5 changes: 5 additions & 0 deletions include/nodejs/json_v8_renderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ struct V8Renderer
out = Napi::Number::New(env, number.value);
}

void operator()(const osrm::json::OSMID &osmid) const
{
out = Napi::BigInt::New(env, osmid.value);
}

void operator()(const osrm::json::Object &object) const
{
Napi::Object obj = Napi::Object::New(env);
Expand Down
15 changes: 14 additions & 1 deletion include/util/json_container.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#ifndef JSON_CONTAINER_HPP
#define JSON_CONTAINER_HPP

#include "util/typedefs.hpp"

#include <string>
#include <unordered_map>
#include <utility>
Expand Down Expand Up @@ -69,6 +71,17 @@ struct Number
double value;
};

/**
* Typed OSM id.
*/
struct OSMID
{
OSMID() = default;
OSMID(OSMNodeID id) : value{from_alias<uint64_t>(id)} {};
OSMID(OSMWayID id) : value{from_alias<uint64_t>(id)} {};
uint64_t value;
};

/**
* Typed True.
*/
Expand All @@ -95,7 +108,7 @@ struct Null
*
* Dispatch on its type by either by using apply_visitor or its get function.
*/
using Value = std::variant<String, Number, Object, Array, True, False, Null>;
using Value = std::variant<String, Number, Object, Array, True, False, Null, OSMID>;

/**
* Typed Object.
Expand Down
11 changes: 11 additions & 0 deletions include/util/json_deep_compare.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ struct Comparator
return is_same;
}

bool operator()(const OSMID &lhs, const OSMID &rhs) const
{
bool is_same = lhs.value == rhs.value;
if (!is_same)
{
reason = lhs_path + " (= " + std::to_string(lhs.value) + ") != " + rhs_path +
" (= " + std::to_string(rhs.value) + ")";
}
return is_same;
}

bool operator()(const Object &lhs, const Object &rhs) const
{
std::set<std::string_view> lhs_keys;
Expand Down
8 changes: 8 additions & 0 deletions include/util/json_renderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ template <typename Out> struct Renderer
write(buffer.data(), buffer.size());
}

void operator()(const OSMID &osmid)
{
fmt::memory_buffer buffer;
fmt::format_to(std::back_inserter(buffer), FMT_COMPILE("{}"), osmid.value);

write(buffer.data(), buffer.size());
}

void operator()(const Object &object)
{
write('{');
Expand Down
4 changes: 2 additions & 2 deletions unit_tests/library/nearest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ void test_nearest_response_for_location_in_small_component(bool use_json_only_ap

const auto &nodes = std::get<json::Array>(waypoint_object.values.at("nodes")).values;
BOOST_CHECK(nodes.size() == 2);
BOOST_CHECK(std::get<util::json::Number>(nodes[0]).value != 0);
BOOST_CHECK(std::get<util::json::Number>(nodes[1]).value != 0);
BOOST_CHECK(std::get<util::json::OSMID>(nodes[0]).value != 0);
BOOST_CHECK(std::get<util::json::OSMID>(nodes[1]).value != 0);
}
}
BOOST_AUTO_TEST_CASE(test_nearest_response_for_location_in_small_component_old_api)
Expand Down
14 changes: 14 additions & 0 deletions unit_tests/util/json_render.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "util/json_container.hpp"
#include "util/json_renderer.hpp"
#include "util/typedefs.hpp"

#include <boost/test/unit_test.hpp>

Expand All @@ -25,6 +26,19 @@ BOOST_AUTO_TEST_CASE(integer)
BOOST_CHECK_EQUAL(str, "42");
}

BOOST_AUTO_TEST_CASE(osmid)
{
std::string str;
Renderer<std::string> renderer(str);

renderer(OSMNodeID{std::numeric_limits<uint64_t>::min()});
BOOST_CHECK_EQUAL(str, "0");
str.clear();

renderer(OSMNodeID{std::numeric_limits<uint64_t>::max()});
BOOST_CHECK_EQUAL(str, "18446744073709551615");
}

BOOST_AUTO_TEST_CASE(test_json_issue_6531)
{
std::string output;
Expand Down
Loading