Enforce Offline tile limit when merging sideloaded databasesupstream/merge-offlinedb

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;