Skip to content

Commit bee8a87

Browse files
mykhailo-kuchmamykhailo-kuchma
authored
Fix the QuadTreeIndex CRC binary compatibility (#1328)
The recently introduced CRC field breaks the binary compatibility with old data, the read method should check for CRC existence before reading it. Relates-To: OAM-1496 Signed-off-by: Mykhailo Kuchma <[email protected]>
1 parent d088091 commit bee8a87

File tree

3 files changed

+181
-13
lines changed

3 files changed

+181
-13
lines changed

‎olp-cpp-sdk-dataservice-read/src/repositories/QuadTreeIndex.cpp‎

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ bool WriteIndexData(const QuadTreeIndex::IndexData& data,
8585
success &= writer.Write(data.compressed_data_size);
8686
success &= writer.Write(data.data_handle);
8787
success &= writer.Write(data.checksum);
88-
success &= writer.Write(data.crc);
8988
success &= writer.Write(data.additional_metadata);
89+
success &= writer.Write(data.crc);
9090
return success;
9191
}
9292
} // namespace
@@ -160,7 +160,7 @@ QuadTreeIndex::QuadTreeIndex(const olp::geo::TileKey& root, int depth,
160160
}
161161

162162
bool QuadTreeIndex::ReadIndexData(QuadTreeIndex::IndexData& data,
163-
uint32_t offset) const {
163+
uint32_t offset, uint32_t limit) const {
164164
BlobDataReader reader(*raw_data_);
165165
reader.SetOffset(offset);
166166

@@ -169,8 +169,13 @@ bool QuadTreeIndex::ReadIndexData(QuadTreeIndex::IndexData& data,
169169
success &= reader.Read(data.compressed_data_size);
170170
success &= reader.Read(data.data_handle);
171171
success &= reader.Read(data.checksum);
172-
success &= reader.Read(data.crc);
173172
success &= reader.Read(data.additional_metadata);
173+
// The CRC field was added after the initial QuadTreeIndex implementation, and
174+
// to maintain the backwards compatibility we must check that we do not read
175+
// the crc from the next index block.
176+
if (reader.GetOffset() < limit) {
177+
success &= reader.Read(data.crc);
178+
}
174179
return success;
175180
}
176181

@@ -275,7 +280,24 @@ boost::optional<QuadTreeIndex::IndexData> QuadTreeIndex::Find(
275280
if (entry == end || entry->sub_quadkey != sub) {
276281
return aggregated ? FindNearestParent(tile_key) : boost::none;
277282
}
278-
if (!ReadIndexData(data, entry->tag_offset)) {
283+
const auto offset = entry->tag_offset;
284+
285+
// The limit is the offset for the next entry, or the beginning of the
286+
// parents entries. (in case there are no parents use the size of the index
287+
// block.)
288+
const auto limit = [&]() {
289+
auto next = entry + 1;
290+
if (next == end) {
291+
if (data_->parent_count == 0) {
292+
return static_cast<uint32_t>(raw_data_->size());
293+
} else {
294+
return ParentEntryBegin()->tag_offset;
295+
}
296+
}
297+
return next->tag_offset;
298+
}();
299+
300+
if (!ReadIndexData(data, offset, limit)) {
279301
return boost::none;
280302
}
281303
data.tile_key = tile_key;
@@ -289,7 +311,15 @@ boost::optional<QuadTreeIndex::IndexData> QuadTreeIndex::Find(
289311
if (entry == end || entry->key != key) {
290312
return aggregated ? FindNearestParent(tile_key) : boost::none;
291313
}
292-
if (!ReadIndexData(data, entry->tag_offset)) {
314+
const auto offset = entry->tag_offset;
315+
316+
// The limit is the offset for the next entry, or the end of the index data.
317+
const auto limit = [&]() {
318+
auto next = entry + 1;
319+
return (next == end) ? raw_data_->size() : next->tag_offset;
320+
}();
321+
322+
if (!ReadIndexData(data, offset, limit)) {
293323
return boost::none;
294324
}
295325
data.tile_key = tile_key;
@@ -302,29 +332,38 @@ boost::optional<QuadTreeIndex::IndexData> QuadTreeIndex::FindNearestParent(
302332
olp::geo::TileKey::FromQuadKey64(data_->root_tilekey);
303333

304334
if (tile_key.Level() >= root_tile_key.Level()) {
335+
auto parents_begin = ParentEntryBegin();
336+
uint32_t limit = parents_begin == ParentEntryEnd()
337+
? static_cast<uint32_t>(raw_data_->size())
338+
: parents_begin->tag_offset;
339+
305340
for (auto it = SubEntryEnd(); it-- != SubEntryBegin();) {
306341
auto key = root_tile_key.AddedSubkey64(it->sub_quadkey);
307342
if (tile_key.IsChildOf(key)) {
308343
IndexData data;
309344
data.tile_key = key;
310-
if (!ReadIndexData(data, it->tag_offset)) {
345+
if (!ReadIndexData(data, it->tag_offset, limit)) {
311346
return boost::none;
312347
}
313348
return data;
314349
}
350+
limit = it->tag_offset;
315351
}
316352
}
317353

354+
uint32_t limit = static_cast<uint32_t>(raw_data_->size());
355+
318356
for (auto it = ParentEntryEnd(); it-- != ParentEntryBegin();) {
319357
auto key = geo::TileKey::FromQuadKey64(it->key);
320358
if (tile_key.IsChildOf(key)) {
321359
IndexData data;
322360
data.tile_key = key;
323-
if (!ReadIndexData(data, it->tag_offset)) {
361+
if (!ReadIndexData(data, it->tag_offset, limit)) {
324362
return boost::none;
325363
}
326364
return data;
327365
}
366+
limit = it->tag_offset;
328367
}
329368
return boost::none;
330369
}
@@ -335,22 +374,28 @@ std::vector<QuadTreeIndex::IndexData> QuadTreeIndex::GetIndexData() const {
335374
return result;
336375
}
337376
result.reserve(data_->parent_count + data_->subkey_count);
377+
378+
uint32_t limit = static_cast<uint32_t>(raw_data_->size());
379+
338380
for (auto it = ParentEntryEnd(); it-- != ParentEntryBegin();) {
339381
QuadTreeIndex::IndexData data;
340382
data.tile_key = geo::TileKey::FromQuadKey64(it->key);
341-
if (ReadIndexData(data, it->tag_offset)) {
383+
if (ReadIndexData(data, it->tag_offset, limit)) {
342384
result.emplace_back(std::move(data));
343385
}
386+
limit = it->tag_offset;
344387
}
388+
345389
for (auto it = SubEntryEnd(); it-- != SubEntryBegin();) {
346390
QuadTreeIndex::IndexData data;
347391
const olp::geo::TileKey& root_tile_key =
348392
olp::geo::TileKey::FromQuadKey64(data_->root_tilekey);
349393
auto subtile = root_tile_key.AddedSubkey64(std::uint64_t(it->sub_quadkey));
350394
data.tile_key = subtile;
351-
if (ReadIndexData(data, it->tag_offset)) {
395+
if (ReadIndexData(data, it->tag_offset, limit)) {
352396
result.emplace_back(std::move(data));
353397
}
398+
limit = it->tag_offset;
354399
}
355400
return result;
356401
}

‎olp-cpp-sdk-dataservice-read/src/repositories/QuadTreeIndex.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ class QuadTreeIndex {
128128
return reinterpret_cast<const uint8_t*>(data_) + size_;
129129
}
130130

131-
bool ReadIndexData(IndexData& data, uint32_t offset) const;
131+
bool ReadIndexData(IndexData& data, uint32_t offset, uint32_t limit) const;
132132

133133
DataHeader* data_ = nullptr;
134134
cache::KeyValueCache::ValueTypePtr raw_data_ = nullptr;

‎olp-cpp-sdk-dataservice-read/tests/QuadTreeIndexTest.cpp‎

Lines changed: 126 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2020 HERE Europe B.V.
2+
* Copyright (C) 2020-2022 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -36,8 +36,6 @@ constexpr auto HTTP_RESPONSE_WRONG_FORMAT =
3636
R"jsonString({"parentQuads": 0,"subQuads": 0})jsonString";
3737

3838
TEST(QuadTreeIndexTest, ParseBlob) {
39-
using testing::Return;
40-
4139
auto tile_key = olp::geo::TileKey::FromHereTile("381");
4240
auto stream = std::stringstream(HTTP_RESPONSE_QUADKEYS);
4341
read::QuadTreeIndex index(tile_key, 1, stream);
@@ -142,4 +140,129 @@ TEST(QuadTreeIndexTest, ParseBlob) {
142140
}
143141
}
144142

143+
// The binary dump for HTTP_RESPONSE_QUADKEYS generated index, before the CRC
144+
// field was introduced.
145+
const char* quad_tree_index_dump =
146+
"\x7d\x01\x00\x00\x00\x00\x00\x00\x00\x00\x01\x03\x05\x00\x00\x00"
147+
"\x01\x00\xcc\xcc\x68\x00\x00\x00\x04\x00\xcc\xcc\xa6\x00\x00\x00"
148+
"\x05\x00\xcc\xcc\xe5\x00\x00\x00\x06\x00\xcc\xcc\x24\x01\x00\x00"
149+
"\x07\x00\xcc\xcc\x63\x01\x00\x00\x05\x00\x00\x00\x00\x00\x00\x00"
150+
"\xa2\x01\x00\x00\xcc\xcc\xcc\xcc\x17\x00\x00\x00\x00\x00\x00\x00"
151+
"\xe1\x01\x00\x00\xcc\xcc\xcc\xcc\x5f\x00\x00\x00\x00\x00\x00\x00"
152+
"\x20\x02\x00\x00\xcc\xcc\xcc\xcc\x30\x00\x00\x00\x00\x00\x00\x00"
153+
"\x59\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff"
154+
"\x42\x44\x35\x33\x41\x36\x44\x36\x30\x41\x33\x34\x43\x32\x30\x44"
155+
"\x43\x34\x32\x41\x43\x41\x42\x32\x36\x35\x30\x46\x45\x33\x36\x31"
156+
"\x2e\x34\x38\x00\x00\x00\x1a\x01\x00\x00\x00\x00\x00\x00\x15\x01"
157+
"\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\x37\x36"
158+
"\x33\x36\x33\x34\x38\x45\x35\x30\x32\x31\x35\x39\x37\x39\x41\x33"
159+
"\x39\x42\x35\x46\x33\x41\x34\x32\x39\x45\x44\x44\x42\x34\x2e\x32"
160+
"\x38\x32\x00\x00\x00\x1a\x01\x00\x00\x00\x00\x00\x00\x0f\x01\x00"
161+
"\x00\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\x38\x43\x39"
162+
"\x42\x33\x45\x30\x38\x45\x32\x39\x34\x41\x44\x42\x32\x43\x44\x30"
163+
"\x37\x45\x42\x43\x38\x34\x31\x32\x30\x36\x32\x46\x45\x2e\x32\x38"
164+
"\x32\x00\x00\x00\x1a\x01\x00\x00\x00\x00\x00\x00\x21\x01\x00\x00"
165+
"\x00\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\x39\x37\x37\x32"
166+
"\x46\x35\x45\x31\x38\x32\x32\x44\x46\x46\x32\x35\x46\x34\x38\x46"
167+
"\x31\x35\x30\x32\x39\x34\x42\x31\x45\x43\x46\x35\x2e\x32\x38\x32"
168+
"\x00\x00\x00\x1a\x01\x00\x00\x00\x00\x00\x00\x1b\x01\x00\x00\x00"
169+
"\x00\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\x42\x46\x38\x34\x44"
170+
"\x38\x45\x43\x38\x31\x32\x34\x42\x39\x36\x44\x42\x45\x35\x43\x34"
171+
"\x44\x42\x36\x38\x42\x30\x35\x39\x31\x38\x46\x2e\x32\x38\x32\x00"
172+
"\x00\x00\x1a\x01\x00\x00\x00\x00\x00\x00\x4f\x83\x01\x00\x00\x00"
173+
"\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\x31\x33\x45\x32\x43\x36"
174+
"\x32\x34\x45\x30\x31\x33\x36\x43\x33\x33\x35\x37\x44\x30\x39\x32"
175+
"\x45\x45\x37\x46\x32\x33\x31\x45\x38\x37\x2e\x32\x38\x32\x00\x00"
176+
"\x00\x1a\x01\x00\x00\x00\x00\x00\x00\xd6\xcc\x00\x00\x00\x00\x00"
177+
"\x00\xff\xff\xff\xff\xff\xff\xff\xff\x46\x38\x46\x34\x43\x33\x43"
178+
"\x42\x30\x39\x46\x42\x41\x36\x31\x42\x39\x32\x37\x32\x35\x36\x43"
179+
"\x42\x43\x42\x38\x34\x34\x31\x44\x31\x2e\x32\x38\x32\x00\x00\x00"
180+
"\xfd\x00\x00\x00\x00\x00\x00\x00\x6d\x1a\x00\x00\x00\x00\x00\x00"
181+
"\xff\xff\xff\xff\xff\xff\xff\xff\x42\x36\x46\x37\x36\x31\x34\x33"
182+
"\x31\x36\x42\x42\x38\x42\x38\x31\x34\x37\x38\x45\x44\x37\x41\x45"
183+
"\x33\x37\x30\x42\x32\x32\x41\x36\x2e\x32\x35\x33\x00\x00\x00";
184+
185+
const auto quad_tree_index_dump_len = 607;
186+
187+
TEST(QuadTreeIndexTest, BackwardsCompatibility) {
188+
auto tile_key = olp::geo::TileKey::FromHereTile("381");
189+
190+
auto dump_data = std::make_shared<std::vector<unsigned char>>();
191+
dump_data->resize(quad_tree_index_dump_len);
192+
std::copy(quad_tree_index_dump,
193+
quad_tree_index_dump + quad_tree_index_dump_len, dump_data->data());
194+
195+
read::QuadTreeIndex index(dump_data);
196+
197+
{
198+
SCOPED_TRACE("Parse json and store to blob");
199+
200+
auto data1 = index.Find(tile_key, false);
201+
ASSERT_FALSE(data1 == boost::none);
202+
EXPECT_EQ(data1->data_handle, "BD53A6D60A34C20DC42ACAB2650FE361.48");
203+
EXPECT_EQ(data1->tile_key, olp::geo::TileKey::FromHereTile("381"));
204+
EXPECT_EQ(data1->version, 48);
205+
206+
auto data2 = index.Find(olp::geo::TileKey::FromHereTile("95"), false);
207+
ASSERT_FALSE(data2 == boost::none);
208+
EXPECT_EQ(data2->data_handle, "B6F7614316BB8B81478ED7AE370B22A6.253");
209+
EXPECT_EQ(data2->tile_key, olp::geo::TileKey::FromHereTile("95"));
210+
EXPECT_EQ(data2->version, 253);
211+
212+
auto data3 = index.Find(tile_key.AddedSubHereTile("2"), false);
213+
ASSERT_FALSE(data3 == boost::none);
214+
EXPECT_EQ(data3->data_handle, "9772F5E1822DFF25F48F150294B1ECF5.282");
215+
EXPECT_EQ(data3->tile_key, olp::geo::TileKey::FromHereTile("1526"));
216+
EXPECT_EQ(data3->version, 282);
217+
218+
auto data4 = index.Find(tile_key.AddedSubHereTile("4"), false);
219+
ASSERT_FALSE(data4 == boost::none);
220+
EXPECT_EQ(data4->data_handle, "7636348E50215979A39B5F3A429EDDB4.282");
221+
EXPECT_EQ(data4->tile_key, olp::geo::TileKey::FromHereTile("1524"));
222+
EXPECT_EQ(data4->version, 282);
223+
224+
auto data5 = index.Find(olp::geo::TileKey::FromHereTile("1561298"), false);
225+
EXPECT_TRUE(data5 == boost::none);
226+
227+
auto data6 = index.Find(olp::geo::TileKey::FromHereTile("3"), false);
228+
EXPECT_TRUE(data6 == boost::none);
229+
}
230+
231+
{
232+
SCOPED_TRACE("Find Data Aggregated");
233+
234+
// Find in parents
235+
auto data1 = index.Find(olp::geo::TileKey::FromHereTile("5842"), true);
236+
ASSERT_FALSE(data1 == boost::none);
237+
EXPECT_EQ(data1->data_handle, "13E2C624E0136C3357D092EE7F231E87.282");
238+
EXPECT_EQ(data1->tile_key, olp::geo::TileKey::FromHereTile("5"));
239+
EXPECT_EQ(data1->version, 282);
240+
241+
// Find in sub quads
242+
auto data2 = index.Find(olp::geo::TileKey::FromHereTile("1561298"), true);
243+
ASSERT_FALSE(data2 == boost::none);
244+
EXPECT_EQ(data2->data_handle, "7636348E50215979A39B5F3A429EDDB4.282");
245+
EXPECT_EQ(data2->tile_key, olp::geo::TileKey::FromHereTile("1524"));
246+
EXPECT_EQ(data2->version, 282);
247+
248+
// Use bottom tile so the algorithm will try to look in childs and parents
249+
// and will fail due to no nearest parent present in data
250+
auto data3 = index.Find(olp::geo::TileKey::FromHereTile("4818"), true);
251+
EXPECT_TRUE(data3 == boost::none);
252+
}
253+
254+
{
255+
SCOPED_TRACE("Quad tree GetIndexData test");
256+
257+
for (const auto& data : index.GetIndexData()) {
258+
if (data.tile_key == olp::geo::TileKey::FromHereTile("23")) {
259+
EXPECT_EQ(data.data_handle, "F8F4C3CB09FBA61B927256CBCB8441D1.282");
260+
}
261+
if (data.tile_key == olp::geo::TileKey::FromHereTile("1524")) {
262+
EXPECT_EQ(data.data_handle, "7636348E50215979A39B5F3A429EDDB4.282");
263+
}
264+
}
265+
}
266+
}
267+
145268
} // namespace

0 commit comments

Comments
 (0)