[core] Add method for setting a maximum size for the ambient cache

- Removed this parameter from the constructor, now that we
  have a method to set it.
- Add the method and enforce the limits if the ambient cache exceeds the
  size.

5 files changed, 125 insertions, 36 deletions

diff --git a/include/mbgl/storage/default_file_source.hpp b/include/mbgl/storage/default_file_source.hpp
index ddbdab8cf..f46f41771 100644
--- a/include/mbgl/storage/default_file_source.hpp
+++ b/include/mbgl/storage/default_file_source.hpp
@@ -30,12 +30,8 @@ public:
      * regions, we want the database to remain fairly small (order tens or low hundreds
      * of megabytes).
      */
-    DefaultFileSource(const std::string& cachePath,
-                      const std::string& assetPath,
-                      uint64_t maximumCacheSize = util::DEFAULT_MAX_CACHE_SIZE);
-    DefaultFileSource(const std::string& cachePath,
-                      std::unique_ptr<FileSource>&& assetFileSource,
-                      uint64_t maximumCacheSize = util::DEFAULT_MAX_CACHE_SIZE);
+    DefaultFileSource(const std::string& cachePath, const std::string& assetPath);
+    DefaultFileSource(const std::string& cachePath, std::unique_ptr<FileSource>&& assetFileSource);
     ~DefaultFileSource() override;
 
     bool supportsCacheOnlyRequests() const override {
@@ -220,6 +216,28 @@ public:
      */
     void clearAmbientCache(std::function<void (std::exception_ptr)>);
 
+    /*
+     * Sets the maximum size in bytes for the ambient cache.
+     *
+     * This call is potentially expensive because it will try
+     * to trim the data in case the database is larger than the
+     * size defined. The size of offline regions are not affected
+     * by this settings, but the ambient cache will always try
+     * to not exceed the maximum size defined, taking into account
+     * the current size for the offline regions.
+     *
+     * If the maximum size is set to 50 MB and 40 MB are already
+     * used by offline regions, the cache size will be effectively
+     * 10 MB.
+     *
+     * Setting the size to 0 will disable the cache if there is no
+     * offline region on the database.
+     *
+     * This method should always be called before using the database,
+     * otherwise the default maximum size will be used.
+     */
+    void setMaximumAmbientCacheSize(uint64_t size, std::function<void (std::exception_ptr)> callback);
+
     // For testing only.
     void setOnlineStatus(bool);
 
diff --git a/platform/default/include/mbgl/storage/offline_database.hpp b/platform/default/include/mbgl/storage/offline_database.hpp
index 59f41a723..afce87b54 100644
--- a/platform/default/include/mbgl/storage/offline_database.hpp
+++ b/platform/default/include/mbgl/storage/offline_database.hpp
@@ -40,7 +40,7 @@ class OfflineDatabase : private util::noncopyable {
 public:
     // Limits affect ambient caching (put) only; resources required by offline
     // regions are exempt.
-    OfflineDatabase(std::string path, uint64_t maximumCacheSize = util::DEFAULT_MAX_CACHE_SIZE);
+    OfflineDatabase(std::string path);
     ~OfflineDatabase();
 
     void changePath(const std::string&);
@@ -86,6 +86,7 @@ public:
     expected<OfflineRegionDefinition, std::exception_ptr> getRegionDefinition(int64_t regionID);
     expected<OfflineRegionStatus, std::exception_ptr> getRegionCompletedStatus(int64_t regionID);
 
+    std::exception_ptr setMaximumAmbientCacheSize(uint64_t);
     void setOfflineMapboxTileCountLimit(uint64_t);
     uint64_t getOfflineMapboxTileCountLimit();
     bool offlineMapboxTileCountLimitExceeded();
@@ -104,6 +105,7 @@ private:
     void migrateToVersion3();
     void migrateToVersion6();
     void cleanup();
+    bool disabled();
 
     mapbox::sqlite::Statement& getStatement(const char *);
 
@@ -136,9 +138,9 @@ private:
     template <class T>
     T getPragma(const char *);
 
-    uint64_t maximumCacheSize;
-
+    uint64_t maximumAmbientCacheSize = util::DEFAULT_MAX_CACHE_SIZE;
     uint64_t offlineMapboxTileCountLimit = util::mapbox::DEFAULT_OFFLINE_TILE_COUNT_LIMIT;
+
     optional<uint64_t> offlineMapboxTileCount;
 
     bool evict(uint64_t neededFreeSize);
diff --git a/platform/default/src/mbgl/storage/default_file_source.cpp b/platform/default/src/mbgl/storage/default_file_source.cpp
index b296e448a..19932d8d0 100644
--- a/platform/default/src/mbgl/storage/default_file_source.cpp
+++ b/platform/default/src/mbgl/storage/default_file_source.cpp
@@ -20,10 +20,10 @@ namespace mbgl {
 
 class DefaultFileSource::Impl {
 public:
-    Impl(std::shared_ptr<FileSource> assetFileSource_, std::string cachePath, uint64_t maximumCacheSize)
+    Impl(std::shared_ptr<FileSource> assetFileSource_, std::string cachePath)
             : assetFileSource(std::move(assetFileSource_))
             , localFileSource(std::make_unique<LocalFileSource>())
-            , offlineDatabase(std::make_unique<OfflineDatabase>(cachePath, maximumCacheSize)) {
+            , offlineDatabase(std::make_unique<OfflineDatabase>(std::move(cachePath))) {
     }
 
     void setAPIBaseURL(const std::string& url) {
@@ -196,6 +196,10 @@ public:
         callback(offlineDatabase->clearAmbientCache());
     }
 
+    void setMaximumAmbientCacheSize(uint64_t size, std::function<void (std::exception_ptr)> callback) {
+        callback(offlineDatabase->setMaximumAmbientCacheSize(size));
+    }
+
 private:
     expected<OfflineDownload*, std::exception_ptr> getDownload(int64_t regionID) {
         auto it = downloads.find(regionID);
@@ -220,17 +224,13 @@ private:
     std::unordered_map<int64_t, std::unique_ptr<OfflineDownload>> downloads;
 };
 
-DefaultFileSource::DefaultFileSource(const std::string& cachePath,
-                                     const std::string& assetPath,
-                                     uint64_t maximumCacheSize)
-    : DefaultFileSource(cachePath, std::make_unique<AssetFileSource>(assetPath), maximumCacheSize) {
+DefaultFileSource::DefaultFileSource(const std::string& cachePath, const std::string& assetPath)
+    : DefaultFileSource(cachePath, std::make_unique<AssetFileSource>(assetPath)) {
 }
 
-DefaultFileSource::DefaultFileSource(const std::string& cachePath,
-                                     std::unique_ptr<FileSource>&& assetFileSource_,
-                                     uint64_t maximumCacheSize)
+DefaultFileSource::DefaultFileSource(const std::string& cachePath, std::unique_ptr<FileSource>&& assetFileSource_)
         : assetFileSource(std::move(assetFileSource_))
-        , impl(std::make_unique<util::Thread<Impl>>("DefaultFileSource", assetFileSource, cachePath, maximumCacheSize)) {
+        , impl(std::make_unique<util::Thread<Impl>>("DefaultFileSource", assetFileSource, cachePath)) {
 }
 
 DefaultFileSource::~DefaultFileSource() = default;
@@ -351,6 +351,10 @@ void DefaultFileSource::clearAmbientCache(std::function<void (std::exception_ptr
     impl->actor().invoke(&Impl::clearAmbientCache, std::move(callback));
 }
 
+void DefaultFileSource::setMaximumAmbientCacheSize(uint64_t size, std::function<void (std::exception_ptr)> callback) {
+    impl->actor().invoke(&Impl::setMaximumAmbientCacheSize, size, std::move(callback));
+}
+
 // For testing only:
 
 void DefaultFileSource::setOnlineStatus(const bool status) {
diff --git a/platform/default/src/mbgl/storage/offline_database.cpp b/platform/default/src/mbgl/storage/offline_database.cpp
index 7caa013f9..24ad3e843 100644
--- a/platform/default/src/mbgl/storage/offline_database.cpp
+++ b/platform/default/src/mbgl/storage/offline_database.cpp
@@ -13,9 +13,8 @@
 
 namespace mbgl {
 
-OfflineDatabase::OfflineDatabase(std::string path_, uint64_t maximumCacheSize_)
-    : path(std::move(path_)),
-      maximumCacheSize(maximumCacheSize_) {
+OfflineDatabase::OfflineDatabase(std::string path_)
+    : path(std::move(path_)) {
     try {
         initialize();
     } catch (const util::IOException& ex) {
@@ -88,6 +87,19 @@ void OfflineDatabase::cleanup() {
     }
 }
 
+bool OfflineDatabase::disabled() {
+    if (maximumAmbientCacheSize) {
+        return false;
+    }
+
+    auto regions = listRegions();
+    if (regions && !regions.value().empty()) {
+        return false;
+    }
+
+    return true;
+}
+
 void OfflineDatabase::handleError(const mapbox::sqlite::Exception& ex, const char* action) {
     if (ex.code == mapbox::sqlite::ResultCode::NotADB ||
         ex.code == mapbox::sqlite::ResultCode::Corrupt ||
@@ -177,6 +189,10 @@ mapbox::sqlite::Statement& OfflineDatabase::getStatement(const char* sql) {
 }
 
 optional<Response> OfflineDatabase::get(const Resource& resource) try {
+    if (disabled()) {
+        return nullopt;
+    }
+
     auto result = getInternal(resource);
     return result ? optional<Response>{ result->first } : nullopt;
 } catch (const util::IOException& ex) {
@@ -209,6 +225,11 @@ std::pair<bool, uint64_t> OfflineDatabase::put(const Resource& resource, const R
     if (!db) {
         initialize();
     }
+
+    if (disabled()) {
+        return { false, 0 };
+    }
+
     mapbox::sqlite::Transaction transaction(*db, mapbox::sqlite::Transaction::Immediate);
     auto result = putInternal(resource, response, true);
     transaction.commit();
@@ -1120,7 +1141,7 @@ bool OfflineDatabase::evict(uint64_t neededFreeSize) {
 
     // The addition of pageSize is a fudge factor to account for non `data` column
     // size, and because pages can get fragmented on the database.
-    while (usedSize() + neededFreeSize + pageSize > maximumCacheSize) {
+    while (usedSize() + neededFreeSize + pageSize > maximumAmbientCacheSize) {
         // clang-format off
         mapbox::sqlite::Query accessedQuery{ getStatement(
             "SELECT max(accessed) "
@@ -1187,6 +1208,29 @@ bool OfflineDatabase::evict(uint64_t neededFreeSize) {
     return true;
 }
 
+std::exception_ptr OfflineDatabase::setMaximumAmbientCacheSize(uint64_t size) {
+    uint64_t previousMaximumAmbientCacheSize = maximumAmbientCacheSize;
+
+    try {
+        maximumAmbientCacheSize = size;
+
+        uint64_t databaseSize = getPragma<int64_t>("PRAGMA page_size")
+            * getPragma<int64_t>("PRAGMA page_count");
+
+        if (databaseSize > maximumAmbientCacheSize) {
+            evict(0);
+            db->exec("VACUUM");
+        }
+
+        return nullptr;
+    } catch (const mapbox::sqlite::Exception& ex) {
+        maximumAmbientCacheSize = previousMaximumAmbientCacheSize;
+        handleError(ex, "set maximum ambient cache size");
+
+        return std::current_exception();
+    }
+}
+
 void OfflineDatabase::setOfflineMapboxTileCountLimit(uint64_t limit) {
     offlineMapboxTileCountLimit = limit;
 }
diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp
index 5801c77fe..882a10e9e 100644
--- a/test/storage/offline_database.test.cpp
+++ b/test/storage/offline_database.test.cpp
@@ -318,7 +318,7 @@ TEST(OfflineDatabase, TEST_REQUIRES_WRITE(GetResourceFromOfflineRegion)) {
     deleteDatabaseFiles();
     util::copyFile(filename, "test/fixtures/offline_database/satellite_test.db");
 
-    OfflineDatabase db(filename, mapbox::sqlite::ReadOnly);
+    OfflineDatabase db(filename);
 
     Resource resource = Resource::style("mapbox://styles/mapbox/satellite-v9");
     ASSERT_TRUE(db.get(resource));
@@ -755,7 +755,8 @@ TEST(OfflineDatabase, PutReturnsSize) {
 
 TEST(OfflineDatabase, PutEvictsLeastRecentlyUsedResources) {
     FixtureLog log;
-    OfflineDatabase db(":memory:", 1024 * 100);
+    OfflineDatabase db(":memory:");
+    db.setMaximumAmbientCacheSize(1024 * 100);
 
     Response response;
     response.data = randomString(1024);
@@ -773,7 +774,9 @@ TEST(OfflineDatabase, PutEvictsLeastRecentlyUsedResources) {
 
 TEST(OfflineDatabase, PutRegionResourceDoesNotEvict) {
     FixtureLog log;
-    OfflineDatabase db(":memory:", 1024 * 100);
+    OfflineDatabase db(":memory:");
+    db.setMaximumAmbientCacheSize(1024 * 100);
+
     OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, true };
     auto region = db.createRegion(definition, OfflineRegionMetadata());
     ASSERT_TRUE(region);
@@ -793,7 +796,8 @@ TEST(OfflineDatabase, PutRegionResourceDoesNotEvict) {
 
 TEST(OfflineDatabase, PutFailsWhenEvictionInsuffices) {
     FixtureLog log;
-    OfflineDatabase db(":memory:", 1024 * 100);
+    OfflineDatabase db(":memory:");
+    db.setMaximumAmbientCacheSize(1024 * 100);
 
     Response big;
     big.data = randomString(1024 * 100);
@@ -849,7 +853,9 @@ TEST(OfflineDatabase, GetRegionCompletedStatus) {
 
 TEST(OfflineDatabase, HasRegionResource) {
     FixtureLog log;
-    OfflineDatabase db(":memory:", 1024 * 100);
+    OfflineDatabase db(":memory:");
+    db.setMaximumAmbientCacheSize(1024 * 100);
+
     OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, true };
     auto region = db.createRegion(definition, OfflineRegionMetadata());
     ASSERT_TRUE(region);
@@ -873,7 +879,9 @@ TEST(OfflineDatabase, HasRegionResource) {
 
 TEST(OfflineDatabase, HasRegionResourceTile) {
     FixtureLog log;
-    OfflineDatabase db(":memory:", 1024 * 100);
+    OfflineDatabase db(":memory:");
+    db.setMaximumAmbientCacheSize(1024 * 100);
+
     OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, false };
     auto region = db.createRegion(definition, OfflineRegionMetadata());
     ASSERT_TRUE(region);
@@ -968,7 +976,9 @@ TEST(OfflineDatabase, OfflineMapboxTileCount) {
 
 TEST(OfflineDatabase, BatchInsertion) {
     FixtureLog log;
-    OfflineDatabase db(":memory:", 1024 * 100);
+    OfflineDatabase db(":memory:");
+    db.setMaximumAmbientCacheSize(1024 * 100);
+
     OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, true };
     auto region = db.createRegion(definition, OfflineRegionMetadata());
     ASSERT_TRUE(region);
@@ -993,8 +1003,10 @@ TEST(OfflineDatabase, BatchInsertion) {
 
 TEST(OfflineDatabase, BatchInsertionMapboxTileCountExceeded) {
     FixtureLog log;
-    OfflineDatabase db(":memory:", 1024 * 100);
+    OfflineDatabase db(":memory:");
     db.setOfflineMapboxTileCountLimit(1);
+    db.setMaximumAmbientCacheSize(1024 * 100);
+
     OfflineTilePyramidRegionDefinition definition { "", LatLngBounds::world(), 0, INFINITY, 1.0, false };
     auto region = db.createRegion(definition, OfflineRegionMetadata());
     ASSERT_TRUE(region);
@@ -1031,7 +1043,9 @@ TEST(OfflineDatabase, MigrateFromV2Schema) {
     util::copyFile(filename, "test/fixtures/offline_database/v2.db");
 
     {
-        OfflineDatabase db(filename, 0);
+        OfflineDatabase db(filename);
+        db.setMaximumAmbientCacheSize(0);
+
         auto regions = db.listRegions();
         ASSERT_TRUE(regions);
         for (auto& region : regions.value()) {
@@ -1053,7 +1067,9 @@ TEST(OfflineDatabase, MigrateFromV3Schema) {
     util::copyFile(filename, "test/fixtures/offline_database/v3.db");
 
     {
-        OfflineDatabase db(filename, 0);
+        OfflineDatabase db(filename);
+        db.setMaximumAmbientCacheSize(0);
+
         auto regions = db.listRegions().value();
         for (auto& region : regions) {
             db.deleteRegion(std::move(region));
@@ -1072,7 +1088,9 @@ TEST(OfflineDatabase, MigrateFromV4Schema) {
     util::copyFile(filename, "test/fixtures/offline_database/v4.db");
 
     {
-        OfflineDatabase db(filename, 0);
+        OfflineDatabase db(filename);
+        db.setMaximumAmbientCacheSize(0);
+
         auto regions = db.listRegions().value();
         for (auto& region : regions) {
             db.deleteRegion(std::move(region));
@@ -1098,7 +1116,9 @@ TEST(OfflineDatabase, MigrateFromV5Schema) {
     util::copyFile(filename, "test/fixtures/offline_database/v5.db");
 
     {
-        OfflineDatabase db(filename, 0);
+        OfflineDatabase db(filename);
+        db.setMaximumAmbientCacheSize(0);
+
         auto regions = db.listRegions().value();
         for (auto& region : regions) {
             db.deleteRegion(std::move(region));
@@ -1126,7 +1146,8 @@ TEST(OfflineDatabase, DowngradeSchema) {
     util::copyFile(filename, "test/fixtures/offline_database/v999.db");
 
     {
-        OfflineDatabase db(filename, 0);
+        OfflineDatabase db(filename);
+        db.setMaximumAmbientCacheSize(0);
     }
 
     EXPECT_EQ(6, databaseUserVersion(filename));