diff options
| -rw-r--r-- | include/mbgl/storage/default_file_source.hpp | 12 | ||||
| -rw-r--r-- | platform/default/mbgl/storage/offline_database.cpp | 34 | ||||
| -rw-r--r-- | test/storage/offline_database.test.cpp | 31 |
3 files changed, 68 insertions, 9 deletions
diff --git a/include/mbgl/storage/default_file_source.hpp b/include/mbgl/storage/default_file_source.hpp index 942749fc00..929fb3d7c3 100644 --- a/include/mbgl/storage/default_file_source.hpp +++ b/include/mbgl/storage/default_file_source.hpp @@ -106,12 +106,18 @@ public: * executed on the database thread; it is the responsibility of the SDK bindings * to re-execute a user-provided callback on the main thread. * + * The secondary database may need to be upgraded to the latest schema. This is done + * in-place and requires write-access to `sideDatabasePath`; it is the + * responsibility of the SDK bindings to ensure that this path is writeable. + * * Only resources and tiles that belong to a region will be copied over. Identical * regions will be flattened into a single new region in the main database. * - * Note that the resulting new regions may not be in a completed status if the - * secondary database does not contain all the tiles or resources required by the - * region definition. + * Invokes the callback with a `MapboxOfflineTileCountExceededException` error if + * the merge operation would result in the offline tile count limit being exceeded. + * + * Merged regions may not be in a completed status if the secondary database + * does not contain all the tiles or resources required by the region definition. */ void mergeOfflineRegions(const std::string& sideDatabasePath, std::function<void (expected<OfflineRegions, std::exception_ptr>)>); diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp index a24c0cf912..37490d2f6f 100644 --- a/platform/default/mbgl/storage/offline_database.cpp +++ b/platform/default/mbgl/storage/offline_database.cpp @@ -661,23 +661,45 @@ OfflineDatabase::mergeDatabase(const std::string& sideDatabasePath) { throw std::runtime_error("Merge database has incorrect user_version"); } + auto currentTileCount = getOfflineMapboxTileCount(); + // clang-format off + mapbox::sqlite::Query queryTiles{ getStatement( + "SELECT COUNT(DISTINCT st.id) " + "FROM side.tiles st " + //only consider region tiles, and not ambient tiles. + "JOIN side.region_tiles srt ON srt.tile_id = st.id " + "LEFT JOIN tiles t ON st.url_template = t.url_template AND " + "st.pixel_ratio = t.pixel_ratio AND " + "st.z = t.z AND " + "st.x = t.x AND " + "st.y = t.y " + "WHERE t.id IS NULL " + "AND st.url_template LIKE 'mapbox://%' ") }; + // clang-format on + queryTiles.run(); + auto countOfTilesToMerge = queryTiles.get<int64_t>(0); + if ((countOfTilesToMerge + currentTileCount) > offlineMapboxTileCountLimit) { + throw MapboxTileLimitExceededException(); + } + queryTiles.reset(); + mapbox::sqlite::Transaction transaction(*db); db->exec(mergeSideloadedDatabaseSQL); transaction.commit(); // clang-format off - mapbox::sqlite::Query query{ getStatement( + mapbox::sqlite::Query queryRegions{ getStatement( "SELECT r.id, r.definition, r.description " "FROM side.regions sr " "JOIN regions r ON sr.definition = r.definition") }; // clang-format on OfflineRegions result; - while (query.run()) { + while (queryRegions.run()) { // Construct, then move because this constructor is private. - OfflineRegion region(query.get<int64_t>(0), - decodeOfflineRegionDefinition(query.get<std::string>(1)), - query.get<std::vector<uint8_t>>(2)); + OfflineRegion region(queryRegions.get<int64_t>(0), + decodeOfflineRegionDefinition(queryRegions.get<std::string>(1)), + queryRegions.get<std::vector<uint8_t>>(2)); result.emplace_back(std::move(region)); } db->exec("DETACH DATABASE side"); @@ -797,7 +819,7 @@ void OfflineDatabase::putRegionResources(int64_t regionID, completedTileSize += resourceSize; } } catch (const MapboxTileLimitExceededException&) { - // Commit the rest of the batch and retrow + // Commit the rest of the batch and rethrow transaction.commit(); throw; } diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index 7511924d80..41eae6f103 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -1266,6 +1266,37 @@ TEST(OfflineDatabase, MergeDatabaseWithMultipleRegionsWithOverlap) { } } +TEST(OfflineDatabase, MergeDatabaseWithSingleRegionTooManyNewTiles) { + FixtureLog log; + util::deleteFile(filename_sideload); + util::copyFile(filename_sideload, "test/fixtures/offline_database/sideload_sat_multiple.db"); + + OfflineDatabase db(":memory:"); + db.setOfflineMapboxTileCountLimit(1); + + auto result = db.mergeDatabase(filename_sideload); + EXPECT_FALSE(result); + EXPECT_EQ(1u, log.count({ EventSeverity::Error, Event::Database, -1, "Mapbox tile limit exceeded" })); + EXPECT_EQ(0u, log.uncheckedCount()); +} + +TEST(OfflineDatabase, MergeDatabaseWithSingleRegionTooManyExistingTiles) { + FixtureLog log; + deleteDatabaseFiles(); + util::deleteFile(filename_sideload); + util::copyFile(filename, "test/fixtures/offline_database/sideload_sat_multiple.db"); + util::copyFile(filename_sideload, "test/fixtures/offline_database/satellite_test.db"); + + OfflineDatabase db(filename); + db.setOfflineMapboxTileCountLimit(2); + + auto result = db.mergeDatabase(filename_sideload); + EXPECT_THROW(std::rethrow_exception(result.error()), MapboxTileLimitExceededException); + + EXPECT_EQ(1u, log.count({ EventSeverity::Error, Event::Database, -1, "Mapbox tile limit exceeded" })); + EXPECT_EQ(0u, log.uncheckedCount()); +} + TEST(OfflineDatabase, MergeDatabaseWithInvalidPath) { FixtureLog log; |