Skip to content

Commit 1219080

Browse files
author
Liubov Didkivska
authored
Fix iifinite loop in PrefetchTiles. Add tests with wrong levels for PrefetchTiles. (#691)
Fix infinite loop in PrefetchTiles. Add tests with wrong levels for PrefetchTiles. Add tests for PrefetchTiles with different min/max levels. Fix infinite recursion by adding checks for valid TileKey value. Reduce min/max level to current level, current level+depth(depth is 4). Relates-To: OLPEDGE-1624 Signed-off-by: Liubov Didkivska <[email protected]>
1 parent a15cb84 commit 1219080

File tree

3 files changed

+141
-6
lines changed

3 files changed

+141
-6
lines changed

olp-cpp-sdk-dataservice-read/src/repositories/PrefetchTilesRepository.cpp

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,23 @@ SubQuadsRequest PrefetchTilesRepository::EffectiveTileKeys(
6969
SubQuadsRequest ret;
7070
auto current_level = tile_key.Level();
7171

72+
// min/max values was not set. Use default wide range.
73+
if (max_level == 0 && min_level == 0) {
74+
max_level = geo::TileKey().Level();
75+
}
76+
7277
auto AddParents = [&]() {
7378
auto tile{tile_key.Parent()};
74-
while (tile.Level() >= min_level) {
79+
while (tile.Level() >= min_level && tile.IsValid()) {
7580
if (tile.Level() <= max_level)
7681
ret[tile.ToHereTile()] = std::make_pair(tile, 0);
7782
tile = tile.Parent();
7883
}
7984
};
8085

81-
auto AddChildren = [&](const geo::TileKey& tile) {
82-
auto child_tiles = EffectiveTileKeys(tile, min_level, max_level, false);
86+
auto AddChildren = [&](const geo::TileKey& tile, unsigned int min,
87+
unsigned int max) {
88+
auto child_tiles = EffectiveTileKeys(tile, min, max, false);
8389
for (auto tile : child_tiles) {
8490
// check if child already exist, if so, use the greater depth
8591
auto old_child = ret.find(tile.first);
@@ -99,7 +105,7 @@ SubQuadsRequest PrefetchTilesRepository::EffectiveTileKeys(
99105
// from there
100106
auto children = GetChildAtLevel(tile_key, min_level);
101107
for (auto child : children) {
102-
AddChildren(child);
108+
AddChildren(child, min_level, max_level);
103109
}
104110
} else {
105111
// tile is within min and max
@@ -111,13 +117,14 @@ SubQuadsRequest PrefetchTilesRepository::EffectiveTileKeys(
111117
if (max_level - current_level <= kMaxQuadTreeIndexDepth) {
112118
ret[tile_key_str] = std::make_pair(tile_key, max_level - current_level);
113119
} else {
114-
// Backend only takes MAX_QUAD_TREE_INDEX_DEPTH at a time, so we have to
120+
// Backend only takes kMaxQuadTreeIndexDepth at a time, so we have to
115121
// manually calculate all the tiles that should included
116122
ret[tile_key_str] = std::make_pair(tile_key, kMaxQuadTreeIndexDepth);
117123
auto children =
118124
GetChildAtLevel(tile_key, current_level + kMaxQuadTreeIndexDepth + 1);
119125
for (auto child : children) {
120-
AddChildren(child);
126+
AddChildren(child, current_level,
127+
current_level + kMaxQuadTreeIndexDepth);
121128
}
122129
}
123130
}
@@ -127,6 +134,9 @@ SubQuadsRequest PrefetchTilesRepository::EffectiveTileKeys(
127134

128135
std::vector<geo::TileKey> PrefetchTilesRepository::GetChildAtLevel(
129136
const geo::TileKey& tile_key, unsigned int min_level) {
137+
if (!tile_key.IsValid()) {
138+
return {};
139+
}
130140
if (tile_key.Level() >= min_level) {
131141
return {tile_key};
132142
}

tests/functional/olp-cpp-sdk-dataservice-read/DataserviceReadVersionedLayerClientTest.cpp

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,98 @@ TEST_F(DataserviceReadVersionedLayerClientTest, Prefetch) {
263263
}
264264
}
265265

266+
TEST_F(DataserviceReadVersionedLayerClientTest, PrefetchWrongLevels) {
267+
const auto catalog =
268+
olp::client::HRN::FromString(CustomParameters::getArgument(
269+
"dataservice_read_test_versioned_prefetch_catalog"));
270+
const auto kLayerId = CustomParameters::getArgument(
271+
"dataservice_read_test_versioned_prefetch_layer");
272+
const auto kTileId = CustomParameters::getArgument(
273+
"dataservice_read_test_versioned_prefetch_tile");
274+
275+
auto client = std::make_unique<olp::dataservice::read::VersionedLayerClient>(
276+
catalog, kLayerId, *settings_);
277+
278+
{
279+
SCOPED_TRACE("Prefetch tiles online and store them in memory cache");
280+
std::vector<olp::geo::TileKey> tile_keys = {
281+
olp::geo::TileKey::FromHereTile(kTileId)};
282+
283+
{
284+
SCOPED_TRACE("min/max levels are 0");
285+
auto request = olp::dataservice::read::PrefetchTilesRequest()
286+
.WithTileKeys(tile_keys)
287+
.WithMinLevel(0)
288+
.WithMaxLevel(0);
289+
290+
std::promise<PrefetchTilesResponse> promise;
291+
std::future<PrefetchTilesResponse> future = promise.get_future();
292+
auto token = client->PrefetchTiles(
293+
request, [&promise](PrefetchTilesResponse response) {
294+
promise.set_value(std::move(response));
295+
});
296+
297+
ASSERT_NE(future.wait_for(kWaitTimeout), std::future_status::timeout);
298+
PrefetchTilesResponse response = future.get();
299+
EXPECT_SUCCESS(response);
300+
const auto& result = response.GetResult();
301+
302+
for (auto tile_result : result) {
303+
EXPECT_SUCCESS(*tile_result);
304+
ASSERT_TRUE(tile_result->tile_key_.IsValid());
305+
}
306+
307+
ASSERT_EQ(10u, result.size());
308+
}
309+
310+
{
311+
SCOPED_TRACE(" min level greater than max level");
312+
auto request = olp::dataservice::read::PrefetchTilesRequest()
313+
.WithTileKeys(tile_keys)
314+
.WithMinLevel(-1)
315+
.WithMaxLevel(0);
316+
317+
std::promise<PrefetchTilesResponse> promise;
318+
std::future<PrefetchTilesResponse> future = promise.get_future();
319+
auto token = client->PrefetchTiles(
320+
request, [&promise](PrefetchTilesResponse response) {
321+
promise.set_value(std::move(response));
322+
});
323+
324+
ASSERT_NE(future.wait_for(kWaitTimeout), std::future_status::timeout);
325+
PrefetchTilesResponse response = future.get();
326+
EXPECT_FALSE(response.IsSuccessful());
327+
ASSERT_TRUE(response.GetResult().empty());
328+
}
329+
{
330+
SCOPED_TRACE(" min/max levels are very wide range");
331+
auto request = olp::dataservice::read::PrefetchTilesRequest()
332+
.WithTileKeys(tile_keys)
333+
.WithMinLevel(0)
334+
.WithMaxLevel(-1);
335+
336+
std::promise<PrefetchTilesResponse> promise;
337+
std::future<PrefetchTilesResponse> future = promise.get_future();
338+
auto token = client->PrefetchTiles(
339+
request, [&promise](PrefetchTilesResponse response) {
340+
promise.set_value(std::move(response));
341+
});
342+
343+
ASSERT_NE(future.wait_for(kWaitTimeout), std::future_status::timeout);
344+
PrefetchTilesResponse response = future.get();
345+
EXPECT_SUCCESS(response);
346+
const auto& result = response.GetResult();
347+
348+
for (auto tile_result : result) {
349+
EXPECT_SUCCESS(*tile_result);
350+
ASSERT_TRUE(tile_result->tile_key_.IsValid());
351+
}
352+
353+
ASSERT_EQ(10u, result.size());
354+
}
355+
}
356+
}
357+
266358
TEST_F(DataserviceReadVersionedLayerClientTest, PrefetchWithCancellableFuture) {
267359
const auto catalog =
268360
olp::client::HRN::FromString(CustomParameters::getArgument(

tests/integration/olp-cpp-sdk-dataservice-read/VersionedLayerClientTest.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1627,6 +1627,39 @@ TEST_F(DataserviceReadVersionedLayerClientTest, PrefetchTilesWithCache) {
16271627
}
16281628
}
16291629

1630+
TEST_F(DataserviceReadVersionedLayerClientTest, PrefetchTilesWrongLevels) {
1631+
olp::client::HRN catalog(GetTestCatalog());
1632+
constexpr auto kLayerId = "hype-test-prefetch";
1633+
1634+
std::vector<olp::geo::TileKey> tile_keys = {
1635+
olp::geo::TileKey::FromHereTile("5904591")};
1636+
1637+
ON_CALL(*network_mock_, Send(_, _, _, _, _))
1638+
.WillByDefault(
1639+
ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
1640+
olp::http::HttpStatusCode::FORBIDDEN),
1641+
HTTP_RESPONSE_403));
1642+
1643+
auto request = olp::dataservice::read::PrefetchTilesRequest()
1644+
.WithTileKeys(tile_keys)
1645+
.WithMinLevel(0)
1646+
.WithMaxLevel(0);
1647+
1648+
auto client = std::make_unique<olp::dataservice::read::VersionedLayerClient>(
1649+
catalog, kLayerId, 4, *settings_);
1650+
ASSERT_TRUE(client);
1651+
1652+
auto cancel_future = client->PrefetchTiles(request);
1653+
auto raw_future = cancel_future.GetFuture();
1654+
1655+
ASSERT_NE(raw_future.wait_for(kWaitTimeout), std::future_status::timeout);
1656+
PrefetchTilesResponse response = raw_future.get();
1657+
ASSERT_FALSE(response.IsSuccessful());
1658+
ASSERT_EQ(olp::client::ErrorCode::AccessDenied,
1659+
response.GetError().GetErrorCode());
1660+
ASSERT_TRUE(response.GetResult().empty());
1661+
}
1662+
16301663
TEST_F(DataserviceReadVersionedLayerClientTest,
16311664
PrefetchTilesCancelOnClientDeletion) {
16321665
auto wait_for_cancel = std::make_shared<std::promise<void>>();

0 commit comments

Comments
 (0)