[core] Let SQLiteCache decide when store or refresh responses

5 files changed, 18 insertions, 23 deletions

diff --git a/platform/default/online_file_source.cpp b/platform/default/online_file_source.cpp
index 657871b907..a470a83462 100644
--- a/platform/default/online_file_source.cpp
+++ b/platform/default/online_file_source.cpp
@@ -244,15 +244,8 @@ void OnlineFileRequestImpl::scheduleRealRequest(OnlineFileSource::Impl& impl, bo
         realRequest = impl.httpContext->createRequest(resource.url, [this, &impl](std::shared_ptr<const Response> response_) {
             realRequest = nullptr;
 
-            // Only update the cache for successful or 404 responses.
-            // In particular, we don't want to write a Canceled request, or one that failed due to
-            // connection errors to the cache. Server errors are hopefully also temporary, so we're not
-            // caching them either.
-            if (impl.cache &&
-                (!response_->error || (response_->error->reason == Response::Error::Reason::NotFound))) {
-                // Store response in database. Make sure we only refresh the expires column if the data
-                // didn't change.
-                impl.cache->put(resource, *response_, response_->notModified ? SQLiteCache::Hint::Refresh : SQLiteCache::Hint::Full);
+            if (impl.cache) {
+                impl.cache->put(resource, *response_);
             }
 
             response = response_;
diff --git a/platform/default/sqlite_cache.cpp b/platform/default/sqlite_cache.cpp
index 53f2c10c23..444637b9af 100644
--- a/platform/default/sqlite_cache.cpp
+++ b/platform/default/sqlite_cache.cpp
@@ -367,14 +367,16 @@ void SQLiteCache::Impl::get(const Resource &resource, Callback callback) {
     }
 }
 
-void SQLiteCache::put(const Resource &resource, const Response& response, Hint hint) {
-    // Can be called from any thread, but most likely from the file source thread. We are either
-    // storing a new response or updating the currently stored response, potentially setting a new
-    // expiry date.
-    if (hint == Hint::Full) {
-        thread->invoke(&Impl::put, resource, response);
-    } else if (hint == Hint::Refresh) {
+void SQLiteCache::put(const Resource& resource, const Response& response) {
+    // Except for 404s, don't store errors in the cache.
+    if (response.error && response.error->reason != Response::Error::Reason::NotFound) {
+        return;
+    }
+
+    if (response.notModified) {
         thread->invoke(&Impl::refresh, resource, response.expires);
+    } else {
+        thread->invoke(&Impl::put, resource, response);
     }
 }
 
diff --git a/src/mbgl/storage/sqlite_cache.hpp b/src/mbgl/storage/sqlite_cache.hpp
index 76e7aed440..94eeee3d46 100644
--- a/src/mbgl/storage/sqlite_cache.hpp
+++ b/src/mbgl/storage/sqlite_cache.hpp
@@ -26,11 +26,10 @@ public:
     void setMaximumCacheSize(uint64_t size);
     void setMaximumCacheEntrySize(uint64_t size);
 
-    enum class Hint : bool { Full, Refresh };
     using Callback = std::function<void(std::unique_ptr<Response>)>;
 
     std::unique_ptr<WorkRequest> get(const Resource&, Callback);
-    void put(const Resource&, const Response&, Hint hint);
+    void put(const Resource&, const Response&);
 
     class Impl;
 
diff --git a/test/storage/cache_response.cpp b/test/storage/cache_response.cpp
index df895ab7b4..26f439ea8f 100644
--- a/test/storage/cache_response.cpp
+++ b/test/storage/cache_response.cpp
@@ -66,7 +66,7 @@ TEST_F(Storage, CacheNotFound) {
     // Insert existing data into the cache that will be marked as stale.
     Response response;
     response.data = std::make_shared<const std::string>("existing data");
-    cache.put(resource, response, SQLiteCache::Hint::Full);
+    cache.put(resource, response);
 
     std::unique_ptr<FileRequest> req1;
     std::unique_ptr<WorkRequest> req2;
@@ -124,7 +124,7 @@ TEST_F(Storage, DontCacheConnectionErrors) {
     // Insert existing data into the cache that will be marked as stale.
     Response response;
     response.data = std::make_shared<const std::string>("existing data");
-    cache.put(resource, response, SQLiteCache::Hint::Full);
+    cache.put(resource, response);
 
     std::unique_ptr<FileRequest> req1;
     std::unique_ptr<WorkRequest> req2;
@@ -180,7 +180,7 @@ TEST_F(Storage, DontCacheServerErrors) {
     // Insert existing data into the cache that will be marked as stale.
     Response response;
     response.data = std::make_shared<const std::string>("existing data");
-    cache.put(resource, response, SQLiteCache::Hint::Full);
+    cache.put(resource, response);
 
     std::unique_ptr<FileRequest> req1;
     std::unique_ptr<WorkRequest> req2;
diff --git a/test/storage/cache_size.cpp b/test/storage/cache_size.cpp
index 7cea572139..4ef4371e98 100644
--- a/test/storage/cache_size.cpp
+++ b/test/storage/cache_size.cpp
@@ -53,7 +53,7 @@ void insertTile(mbgl::SQLiteCache* cache, unsigned id, uint64_t size) {
     response.data = data;
 
     Resource resource{ Resource::Kind::Tile, url };
-    cache->put(resource, response, SQLiteCache::Hint::Full);
+    cache->put(resource, response);
 }
 
 void refreshTile(mbgl::SQLiteCache* cache, unsigned id) {
@@ -64,9 +64,10 @@ void refreshTile(mbgl::SQLiteCache* cache, unsigned id) {
     Response response;
     response.modified = toSeconds(SystemClock::now());
     response.expires = toSeconds(SystemClock::now()) + Seconds(5000);
+    response.notModified = true;
 
     Resource resource{ Resource::Kind::Tile, url };
-    cache->put(resource, response, SQLiteCache::Hint::Refresh);
+    cache->put(resource, response);
 }
 
 uint64_t cacheSize(mbgl::SQLiteCache* cache, unsigned entryCount, uint64_t entrySize) {