Skip to content

Commit 563571d

Browse files
DennisOSRMDennisOSRM
authored
refactor(hints): replace PhantomNode in SegmentHint with fixed placeholder (#7649)
1 parent 9d24e4a commit 563571d

14 files changed

Lines changed: 71 additions & 91 deletions

‎include/engine/api/base_api.hpp‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,14 @@ class BaseAPI
6363
const auto &input_location = candidatesInputLocation(candidates);
6464
if (parameters.generate_hints)
6565
{
66-
std::vector<SegmentHint> seg_hints(candidates.size());
67-
std::transform(candidates.begin(),
68-
candidates.end(),
69-
seg_hints.begin(),
70-
[this](const auto &phantom)
71-
{ return SegmentHint{phantom, facade.GetCheckSum()}; });
66+
// Hints are deprecated; generate fixed-size placeholder segment hints
67+
const auto num_candidates = candidates.size();
68+
std::vector<SegmentHint> seg_hints;
69+
seg_hints.reserve(num_candidates);
70+
for (std::size_t i = 0; i < num_candidates; ++i)
71+
{
72+
seg_hints.push_back({.data_checksum = facade.GetCheckSum()});
73+
}
7274

7375
return json::makeWaypoint(
7476
snapped_location,
@@ -130,12 +132,14 @@ class BaseAPI
130132
flatbuffers::Offset<flatbuffers::String> hint_string;
131133
if (parameters.generate_hints)
132134
{
133-
std::vector<SegmentHint> seg_hints(candidates.size());
134-
std::transform(candidates.begin(),
135-
candidates.end(),
136-
seg_hints.begin(),
137-
[this](const auto &phantom)
138-
{ return SegmentHint{phantom, facade.GetCheckSum()}; });
135+
// Hints are deprecated; generate fixed-size placeholder segment hints
136+
const auto num_candidates = candidates.size();
137+
std::vector<SegmentHint> seg_hints;
138+
seg_hints.reserve(num_candidates);
139+
for (std::size_t i = 0; i < num_candidates; ++i)
140+
{
141+
seg_hints.push_back({.data_checksum = facade.GetCheckSum()});
142+
}
139143
Hint hint{std::move(seg_hints)};
140144
hint_string = builder->CreateString(hint.ToBase64());
141145
}

‎include/engine/api/table_parameters.hpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
2929
#define ENGINE_API_TABLE_PARAMETERS_HPP
3030

3131
#include "engine/api/base_parameters.hpp"
32+
#include "util/typedefs.hpp"
3233

3334
#include <cstddef>
3435

‎include/engine/hint.hpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
2828
#ifndef ENGINE_HINT_HPP
2929
#define ENGINE_HINT_HPP
3030

31-
#include "engine/phantom_node.hpp"
32-
3331
#include "util/coordinate.hpp"
3432

33+
#include <array>
3534
#include <cstdint>
3635
#include <iosfwd>
3736
#include <string>
37+
#include <vector>
3838

3939
namespace osrm::engine
4040
{
@@ -45,12 +45,13 @@ namespace datafacade
4545
class BaseDataFacade;
4646
}
4747

48-
// SegmentHint represents an individual segment position that could be used
49-
// as the waypoint for a given input location
48+
// SegmentHint is deprecated. It is kept as a fixed-size placeholder for
49+
// backward-compatible hint serialization in API responses.
5050
struct SegmentHint
5151
{
52-
PhantomNode phantom;
53-
std::uint32_t data_checksum;
52+
// Placeholder bytes (was PhantomNode, now deprecated)
53+
std::array<unsigned char, 80> deprecated_data{};
54+
std::uint32_t data_checksum = 0;
5455

5556
bool IsValid(const util::Coordinate new_input_coordinates,
5657
const datafacade::BaseDataFacade &facade) const;
@@ -63,8 +64,7 @@ struct SegmentHint
6364
friend std::ostream &operator<<(std::ostream &, const SegmentHint &);
6465
};
6566

66-
// Hint represents the suggested segment positions that could be used
67-
// as the waypoint for a given input location
67+
// Hint is deprecated. It is kept for backward-compatible serialization.
6868
struct Hint
6969
{
7070
std::vector<SegmentHint> segment_hints;
@@ -76,7 +76,7 @@ struct Hint
7676
static Hint FromBase64(const std::string &base64Hint);
7777
};
7878

79-
static_assert(sizeof(SegmentHint) == 80 + 4, "Hint is bigger than expected");
79+
static_assert(sizeof(SegmentHint) == 80 + 4, "SegmentHint size must stay at 84 bytes");
8080
constexpr std::size_t ENCODED_SEGMENT_HINT_SIZE = 112;
8181
static_assert(ENCODED_SEGMENT_HINT_SIZE / 4 * 3 >= sizeof(SegmentHint),
8282
"ENCODED_SEGMENT_HINT_SIZE does not match size of SegmentHint");

‎src/engine/hint.cpp‎

Lines changed: 13 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,17 @@
66

77
#include <algorithm>
88
#include <iterator>
9-
#include <tuple>
10-
#include <unordered_set>
9+
#include <string>
1110

1211
namespace osrm::engine
1312
{
1413

15-
bool SegmentHint::IsValid(const util::Coordinate new_input_coordinates,
14+
bool SegmentHint::IsValid(const util::Coordinate /*new_input_coordinates*/,
1615
const datafacade::BaseDataFacade &facade) const
1716
{
18-
auto is_same_input_coordinate = new_input_coordinates.lon == phantom.input_location.lon &&
19-
new_input_coordinates.lat == phantom.input_location.lat;
20-
// FIXME this does not use the number of nodes to validate the phantom because
21-
// GetNumberOfNodes()
22-
// depends on the graph which is algorithm dependent
23-
return is_same_input_coordinate && phantom.IsValid() && facade.GetCheckSum() == data_checksum;
17+
// Hints are deprecated; only validate via dataset checksum to avoid rejecting
18+
// requests that still include hints from older responses.
19+
return facade.GetCheckSum() == data_checksum;
2420
}
2521

2622
std::string SegmentHint::ToBase64() const
@@ -50,7 +46,7 @@ SegmentHint SegmentHint::FromBase64(const std::string &base64Hint)
5046

5147
bool operator==(const SegmentHint &lhs, const SegmentHint &rhs)
5248
{
53-
return std::tie(lhs.phantom, lhs.data_checksum) == std::tie(rhs.phantom, rhs.data_checksum);
49+
return lhs.deprecated_data == rhs.deprecated_data && lhs.data_checksum == rhs.data_checksum;
5450
}
5551

5652
bool operator!=(const SegmentHint &lhs, const SegmentHint &rhs) { return !(lhs == rhs); }
@@ -72,7 +68,6 @@ std::string Hint::ToBase64() const
7268

7369
Hint Hint::FromBase64(const std::string &base64Hint)
7470
{
75-
7671
BOOST_ASSERT_MSG(base64Hint.size() % ENCODED_SEGMENT_HINT_SIZE == 0,
7772
"SegmentHint has invalid size");
7873

@@ -90,36 +85,15 @@ Hint Hint::FromBase64(const std::string &base64Hint)
9085
return {std::move(res)};
9186
}
9287

93-
bool Hint::IsValid(const util::Coordinate new_input_coordinates,
88+
bool Hint::IsValid(const util::Coordinate /*new_input_coordinates*/,
9489
const datafacade::BaseDataFacade &facade) const
9590
{
96-
const auto all_valid = std::all_of(segment_hints.begin(),
97-
segment_hints.end(),
98-
[&](const auto &seg_hint)
99-
{ return seg_hint.IsValid(new_input_coordinates, facade); });
100-
if (!all_valid)
101-
{
102-
return false;
103-
}
104-
105-
// Check hints do not contain duplicate segment pairs
106-
// We can't allow duplicates as search heaps do not support it.
107-
std::unordered_set<NodeID> forward_segments;
108-
std::unordered_set<NodeID> reverse_segments;
109-
for (const auto &seg_hint : segment_hints)
110-
{
111-
const auto forward_res = forward_segments.insert(seg_hint.phantom.forward_segment_id.id);
112-
if (!forward_res.second)
113-
{
114-
return false;
115-
}
116-
const auto backward_res = reverse_segments.insert(seg_hint.phantom.reverse_segment_id.id);
117-
if (!backward_res.second)
118-
{
119-
return false;
120-
}
121-
}
122-
return true;
91+
// Hints are deprecated; validate each segment hint via checksum-only check.
92+
const auto all_valid =
93+
std::all_of(segment_hints.begin(),
94+
segment_hints.end(),
95+
[&](const auto &seg_hint) { return seg_hint.IsValid({}, facade); });
96+
return all_valid;
12397
}
12498

12599
} // namespace osrm::engine

‎src/server/request_handler.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
#include "server/request_handler.hpp"
2+
3+
#include <boost/assert.hpp>
4+
25
#include "server/service_handler.hpp"
36

47
#include "server/api/url_parser.hpp"

‎src/server/service/match_service.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "server/service/match_service.hpp"
22

3+
#include <boost/assert.hpp>
4+
35
#include "server/api/parameters_parser.hpp"
46
#include "server/service/utils.hpp"
57
#include "engine/api/match_parameters.hpp"

‎src/server/service/nearest_service.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
#include "server/service/nearest_service.hpp"
2+
3+
#include <boost/assert.hpp>
4+
25
#include "server/service/utils.hpp"
36

47
#include "server/api/parameters_parser.hpp"

‎src/server/service/route_service.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
#include "server/service/route_service.hpp"
2+
3+
#include <boost/assert.hpp>
4+
25
#include "server/service/utils.hpp"
36

47
#include "server/api/parameters_parser.hpp"

‎src/server/service/table_service.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "server/service/table_service.hpp"
22

3+
#include <boost/assert.hpp>
4+
35
#include "server/api/parameters_parser.hpp"
46
#include "engine/api/table_parameters.hpp"
57

‎src/server/service/tile_service.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
#include "server/service/tile_service.hpp"
2+
3+
#include <boost/assert.hpp>
4+
25
#include "server/service/utils.hpp"
36

47
#include "server/api/parameters_parser.hpp"

0 commit comments

Comments
 (0)