[core] Introduce OfflineDatabase::runPackDatabaseAutomatically() API

- added a unit test
- Updated inline comments in default_file_source.hpp

4 files changed, 54 insertions, 43 deletions

diff --git a/platform/android/src/offline/offline_region.cpp b/platform/android/src/offline/offline_region.cpp
index ac9f491ab6..4276ce8f45 100644
--- a/platform/android/src/offline/offline_region.cpp
+++ b/platform/android/src/offline/offline_region.cpp
@@ -104,37 +104,40 @@ void OfflineRegion::getOfflineRegionStatus(jni::JNIEnv& env_, const jni::Object<
 void OfflineRegion::deleteOfflineRegion(jni::JNIEnv& env_, const jni::Object<OfflineRegionDeleteCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->deleteOfflineRegion(std::move(*region), [
-        //Ensure the object is not gc'd in the meanwhile
-        callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
-    ](std::exception_ptr error) mutable {
-        // Reattach, the callback comes from a different thread
-        android::UniqueEnv env = android::AttachEnv();
-
-        if (error) {
-            OfflineRegionDeleteCallback::onError(*env, *callback, error);
-        } else {
-            OfflineRegionDeleteCallback::onDelete(*env, *callback);
-        }
-    });
+    fileSource->deleteOfflineRegion(std::move(*region),
+                                    [
+                                        // Ensure the object is not gc'd in the meanwhile
+                                        callback = std::make_shared<decltype(globalCallback)>(
+                                            std::move(globalCallback))](std::exception_ptr error) mutable {
+                                        // Reattach, the callback comes from a different thread
+                                        android::UniqueEnv env = android::AttachEnv();
+
+                                        if (error) {
+                                            OfflineRegionDeleteCallback::onError(*env, *callback, error);
+                                        } else {
+                                            OfflineRegionDeleteCallback::onDelete(*env, *callback);
+                                        }
+                                    });
 }
 
-void OfflineRegion::invalidateOfflineRegion(jni::JNIEnv& env_, const jni::Object<OfflineRegionInvalidateCallback>& callback_) {
+void OfflineRegion::invalidateOfflineRegion(jni::JNIEnv& env_,
+                                            const jni::Object<OfflineRegionInvalidateCallback>& callback_) {
     auto globalCallback = jni::NewGlobal<jni::EnvAttachingDeleter>(env_, callback_);
 
-    fileSource->invalidateOfflineRegion(*region, [
-        //Ensure the object is not gc'd in the meanwhile
-        callback = std::make_shared<decltype(globalCallback)>(std::move(globalCallback))
-    ](std::exception_ptr error) mutable {
-        // Reattach, the callback comes from a different thread
-        android::UniqueEnv env = android::AttachEnv();
-
-        if (error) {
-            OfflineRegionInvalidateCallback::onError(*env, *callback, error);
-        } else {
-            OfflineRegionInvalidateCallback::onInvalidate(*env, *callback);
-        }
-    });
+    fileSource->invalidateOfflineRegion(*region,
+                                        [
+                                            // Ensure the object is not gc'd in the meanwhile
+                                            callback = std::make_shared<decltype(globalCallback)>(
+                                                std::move(globalCallback))](std::exception_ptr error) mutable {
+                                            // Reattach, the callback comes from a different thread
+                                            android::UniqueEnv env = android::AttachEnv();
+
+                                            if (error) {
+                                                OfflineRegionInvalidateCallback::onError(*env, *callback, error);
+                                            } else {
+                                                OfflineRegionInvalidateCallback::onInvalidate(*env, *callback);
+                                            }
+                                        });
 }
 
 void OfflineRegion::updateOfflineRegionMetadata(jni::JNIEnv& env_, const jni::Array<jni::jbyte>& jMetadata, const jni::Object<OfflineRegionUpdateMetadataCallback>& callback_) {
@@ -206,7 +209,10 @@ void OfflineRegion::registerNative(jni::JNIEnv& env) {
 
     #define METHOD(MethodPtr, name) jni::MakeNativePeerMethod<decltype(MethodPtr), (MethodPtr)>(name)
 
-    jni::RegisterNativePeer<OfflineRegion>( env, javaClass, "nativePtr",
+    jni::RegisterNativePeer<OfflineRegion>(
+        env,
+        javaClass,
+        "nativePtr",
         jni::MakePeer<OfflineRegion, jni::jlong, const jni::Object<FileSource>&>,
         "initialize",
         "finalize",
@@ -215,8 +221,7 @@ void OfflineRegion::registerNative(jni::JNIEnv& env) {
         METHOD(&OfflineRegion::getOfflineRegionStatus, "getOfflineRegionStatus"),
         METHOD(&OfflineRegion::deleteOfflineRegion, "deleteOfflineRegion"),
         METHOD(&OfflineRegion::invalidateOfflineRegion, "invalidateOfflineRegion"),
-        METHOD(&OfflineRegion::updateOfflineRegionMetadata, "updateOfflineRegionMetadata")
-    );
+        METHOD(&OfflineRegion::updateOfflineRegionMetadata, "updateOfflineRegionMetadata"));
 }
 
 // OfflineRegionObserver //
@@ -227,7 +232,7 @@ void OfflineRegion::OfflineRegionStatusCallback::onError(jni::JNIEnv& env,
                                                           const jni::Object<OfflineRegion::OfflineRegionStatusCallback>& callback,
                                                           std::exception_ptr error) {
     static auto& javaClass = jni::Class<OfflineRegion::OfflineRegionStatusCallback>::Singleton(env);
-    static auto method = javaClass.GetMethod<void (jni::String)>(env, "onError");
+    static auto method = javaClass.GetMethod<void(jni::String)>(env, "onError");
 
     callback.Call(env, method, jni::Make<jni::String>(env, mbgl::util::toString(error)));
 }
diff --git a/platform/default/include/mbgl/storage/offline_database.hpp b/platform/default/include/mbgl/storage/offline_database.hpp
index ac997bf6ad..67a19fcf26 100644
--- a/platform/default/include/mbgl/storage/offline_database.hpp
+++ b/platform/default/include/mbgl/storage/offline_database.hpp
@@ -74,7 +74,7 @@ public:
     expected<OfflineRegionMetadata, std::exception_ptr>
     updateMetadata(const int64_t regionID, const OfflineRegionMetadata&);
 
-    std::exception_ptr deleteRegion(OfflineRegion&&, bool pack = true);
+    std::exception_ptr deleteRegion(OfflineRegion&&);
     std::exception_ptr invalidateRegion(int64_t regionID);
 
     // Return value is (response, stored size)
@@ -94,6 +94,7 @@ public:
     bool exceedsOfflineMapboxTileCountLimit(const Resource&);
     void markUsedResources(int64_t regionID, const std::list<Resource>&);
     std::exception_ptr pack();
+    void runPackDatabaseAutomatically(bool autopack_) { autopack = autopack_; }
 
 private:
     void initialize();
@@ -149,6 +150,7 @@ private:
     optional<uint64_t> offlineMapboxTileCount;
 
     bool evict(uint64_t neededFreeSize);
+    bool autopack = true;
 };
 
 } // namespace mbgl
diff --git a/platform/default/src/mbgl/storage/default_file_source.cpp b/platform/default/src/mbgl/storage/default_file_source.cpp
index 36acb748de..e6cdb411bb 100644
--- a/platform/default/src/mbgl/storage/default_file_source.cpp
+++ b/platform/default/src/mbgl/storage/default_file_source.cpp
@@ -82,9 +82,9 @@ public:
         }
     }
 
-    void deleteRegion(OfflineRegion&& region, std::function<void(std::exception_ptr)> callback, bool pack) {
+    void deleteRegion(OfflineRegion&& region, std::function<void(std::exception_ptr)> callback) {
         downloads.erase(region.getID());
-        callback(offlineDatabase->deleteRegion(std::move(region), pack));
+        callback(offlineDatabase->deleteRegion(std::move(region)));
     }
 
     void invalidateRegion(int64_t regionID, std::function<void (std::exception_ptr)> callback) {
@@ -208,6 +208,8 @@ public:
 
     void packDatabase(std::function<void(std::exception_ptr)> callback) { callback(offlineDatabase->pack()); }
 
+    void runPackDatabaseAutomatically(bool autopack) { offlineDatabase->runPackDatabaseAutomatically(autopack); }
+
 private:
     expected<OfflineDownload*, std::exception_ptr> getDownload(int64_t regionID) {
         auto it = downloads.find(regionID);
@@ -316,10 +318,8 @@ void DefaultFileSource::updateOfflineMetadata(const int64_t regionID,
     impl->actor().invoke(&Impl::updateMetadata, regionID, metadata, callback);
 }
 
-void DefaultFileSource::deleteOfflineRegion(OfflineRegion&& region,
-                                            std::function<void(std::exception_ptr)> callback,
-                                            bool pack) {
-    impl->actor().invoke(&Impl::deleteRegion, std::move(region), callback, pack);
+void DefaultFileSource::deleteOfflineRegion(OfflineRegion&& region, std::function<void(std::exception_ptr)> callback) {
+    impl->actor().invoke(&Impl::deleteRegion, std::move(region), callback);
 }
 
 void DefaultFileSource::invalidateOfflineRegion(OfflineRegion& region,
@@ -363,6 +363,10 @@ void DefaultFileSource::packDatabase(std::function<void(std::exception_ptr)> cal
     impl->actor().invoke(&Impl::packDatabase, std::move(callback));
 }
 
+void DefaultFileSource::runPackDatabaseAutomatically(bool autopack) {
+    impl->actor().invoke(&Impl::runPackDatabaseAutomatically, autopack);
+}
+
 void DefaultFileSource::invalidateAmbientCache(std::function<void (std::exception_ptr)> callback) {
     impl->actor().invoke(&Impl::invalidateAmbientCache, std::move(callback));
 }
diff --git a/platform/default/src/mbgl/storage/offline_database.cpp b/platform/default/src/mbgl/storage/offline_database.cpp
index fef524ce57..974815191b 100644
--- a/platform/default/src/mbgl/storage/offline_database.cpp
+++ b/platform/default/src/mbgl/storage/offline_database.cpp
@@ -154,7 +154,7 @@ void OfflineDatabase::removeExisting() {
 void OfflineDatabase::removeOldCacheTable() {
     assert(db);
     db->exec("DROP TABLE IF EXISTS http_cache");
-    vacuum();
+    if (autopack) vacuum();
 }
 
 void OfflineDatabase::createSchema() {
@@ -705,7 +705,7 @@ std::exception_ptr OfflineDatabase::clearAmbientCache() try {
 
     resourceQuery.run();
 
-    vacuum();
+    if (autopack) vacuum();
 
     return nullptr;
 } catch (...) {
@@ -884,7 +884,7 @@ OfflineDatabase::updateMetadata(const int64_t regionID, const OfflineRegionMetad
     return unexpected<std::exception_ptr>(std::current_exception());
 }
 
-std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region, bool pack) try {
+std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region) try {
     {
         mapbox::sqlite::Query query{ getStatement("DELETE FROM regions WHERE id = ?") };
         query.bind(1, region.getID());
@@ -893,7 +893,7 @@ std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region, bool pa
 
     evict(0);
     assert(db);
-    if (pack) vacuum();
+    if (autopack) vacuum();
 
     // Ensure that the cached offlineTileCount value is recalculated.
     offlineMapboxTileCount = nullopt;
@@ -1235,7 +1235,7 @@ std::exception_ptr OfflineDatabase::setMaximumAmbientCacheSize(uint64_t size) {
 
         if (databaseSize > maximumAmbientCacheSize) {
             evict(0);
-            vacuum();
+            if (autopack) vacuum();
         }
 
         return nullptr;