[core] finish must-revalidate support

8 files changed, 138 insertions, 87 deletions

diff --git a/platform/default/default_file_source.cpp b/platform/default/default_file_source.cpp
index bf8d7b6348..9c8a38a308 100644
--- a/platform/default/default_file_source.cpp
+++ b/platform/default/default_file_source.cpp
@@ -140,7 +140,17 @@ public:
                     revalidation.priorModified = offlineResponse->modified;
                     revalidation.priorExpires = offlineResponse->expires;
                     revalidation.priorEtag = offlineResponse->etag;
-                    callback(*offlineResponse);
+
+                    // Don't return resources the server requested not to show when they're stale.
+                    // Even if we can't directly use the response, we may still use it to send a
+                    // conditional HTTP request.
+                    if (offlineResponse->isUsable()) {
+                        callback(*offlineResponse);
+                    } else {
+                        // Since we can't return the data immediately, we'll have to hold on so that
+                        // we can return it later in case we get a 304 Not Modified response.
+                        revalidation.priorData = offlineResponse->data;
+                    }
                 }
             }
 
diff --git a/platform/default/http_file_source.cpp b/platform/default/http_file_source.cpp
index 867d85fa4d..a9c442c2de 100644
--- a/platform/default/http_file_source.cpp
+++ b/platform/default/http_file_source.cpp
@@ -325,7 +325,9 @@ size_t HTTPRequest::headerCallback(char *const buffer, const size_t size, const
         baton->response->etag = std::string(buffer + begin, length - begin - 2); // remove \r\n
     } else if ((begin = headerMatches("cache-control: ", buffer, length)) != std::string::npos) {
         const std::string value { buffer + begin, length - begin - 2 }; // remove \r\n
-        baton->response->expires = http::CacheControl::parse(value.c_str()).toTimePoint();
+        const auto cc = http::CacheControl::parse(value.c_str());
+        baton->response->expires = cc.toTimePoint();
+        baton->response->mustRevalidate = cc.mustRevalidate;
     } else if ((begin = headerMatches("expires: ", buffer, length)) != std::string::npos) {
         const std::string value { buffer + begin, length - begin - 2 }; // remove \r\n
         baton->response->expires = Timestamp{ Seconds(curl_getdate(value.c_str(), nullptr)) };
diff --git a/platform/default/mbgl/storage/offline_database.cpp b/platform/default/mbgl/storage/offline_database.cpp
index bad4ccdbf1..d38f0c9108 100644
--- a/platform/default/mbgl/storage/offline_database.cpp
+++ b/platform/default/mbgl/storage/offline_database.cpp
@@ -49,7 +49,8 @@ void OfflineDatabase::ensureSchema() {
             case 2: migrateToVersion3(); // fall through
             case 3: // no-op and fall through
             case 4: migrateToVersion5(); // fall through
-            case 5: return;
+            case 5: migrateToVersion6(); // fall through
+            case 6: return;
             default: throw std::runtime_error("unknown schema version");
             }
 
@@ -83,7 +84,7 @@ void OfflineDatabase::ensureSchema() {
         db->exec("PRAGMA journal_mode = DELETE");
         db->exec("PRAGMA synchronous = FULL");
         db->exec(schema);
-        db->exec("PRAGMA user_version = 5");
+        db->exec("PRAGMA user_version = 6");
     } catch (...) {
         Log::Error(Event::Database, "Unexpected error creating database schema: %s", util::toString(std::current_exception()).c_str());
         throw;
@@ -126,6 +127,14 @@ void OfflineDatabase::migrateToVersion5() {
     db->exec("PRAGMA user_version = 5");
 }
 
+void OfflineDatabase::migrateToVersion6() {
+    mapbox::sqlite::Transaction transaction(*db);
+    db->exec("ALTER TABLE resources ADD COLUMN must_revalidate INTEGER NOT NULL DEFAULT 0");
+    db->exec("ALTER TABLE tiles ADD COLUMN must_revalidate INTEGER NOT NULL DEFAULT 0");
+    db->exec("PRAGMA user_version = 6");
+    transaction.commit();
+}
+
 OfflineDatabase::Statement OfflineDatabase::getStatement(const char * sql) {
     auto it = statements.find(sql);
 
@@ -211,8 +220,8 @@ optional<std::pair<Response, uint64_t>> OfflineDatabase::getResource(const Resou
 
     // clang-format off
     Statement stmt = getStatement(
-        //        0      1        2       3        4
-        "SELECT etag, expires, modified, data, compressed "
+        //        0      1            2            3       4      5
+        "SELECT etag, expires, must_revalidate, modified, data, compressed "
         "FROM resources "
         "WHERE url = ?");
     // clang-format on
@@ -226,14 +235,15 @@ optional<std::pair<Response, uint64_t>> OfflineDatabase::getResource(const Resou
     Response response;
     uint64_t size = 0;
 
-    response.etag     = stmt->get<optional<std::string>>(0);
-    response.expires  = stmt->get<optional<Timestamp>>(1);
-    response.modified = stmt->get<optional<Timestamp>>(2);
+    response.etag           = stmt->get<optional<std::string>>(0);
+    response.expires        = stmt->get<optional<Timestamp>>(1);
+    response.mustRevalidate = stmt->get<bool>(2);
+    response.modified       = stmt->get<optional<Timestamp>>(3);
 
-    optional<std::string> data = stmt->get<optional<std::string>>(3);
+    optional<std::string> data = stmt->get<optional<std::string>>(4);
     if (!data) {
         response.noContent = true;
-    } else if (stmt->get<int>(4)) {
+    } else if (stmt->get<bool>(5)) {
         response.data = std::make_shared<std::string>(util::decompress(*data));
         size = data->length();
     } else {
@@ -265,14 +275,16 @@ bool OfflineDatabase::putResource(const Resource& resource,
         // clang-format off
         Statement update = getStatement(
             "UPDATE resources "
-            "SET accessed = ?1, "
-            "    expires  = ?2 "
-            "WHERE url    = ?3 ");
+            "SET accessed         = ?1, "
+            "    expires          = ?2, "
+            "    must_revalidate  = ?3 "
+            "WHERE url    = ?4 ");
         // clang-format on
 
         update->bind(1, util::now());
         update->bind(2, response.expires);
-        update->bind(3, resource.url);
+        update->bind(3, response.mustRevalidate);
+        update->bind(4, resource.url);
         update->run();
         return false;
     }
@@ -286,29 +298,31 @@ bool OfflineDatabase::putResource(const Resource& resource,
     // clang-format off
     Statement update = getStatement(
         "UPDATE resources "
-        "SET kind       = ?1, "
-        "    etag       = ?2, "
-        "    expires    = ?3, "
-        "    modified   = ?4, "
-        "    accessed   = ?5, "
-        "    data       = ?6, "
-        "    compressed = ?7 "
-        "WHERE url      = ?8 ");
+        "SET kind            = ?1, "
+        "    etag            = ?2, "
+        "    expires         = ?3, "
+        "    must_revalidate = ?4, "
+        "    modified        = ?5, "
+        "    accessed        = ?6, "
+        "    data            = ?7, "
+        "    compressed      = ?8 "
+        "WHERE url           = ?9 ");
     // clang-format on
 
     update->bind(1, int(resource.kind));
     update->bind(2, response.etag);
     update->bind(3, response.expires);
-    update->bind(4, response.modified);
-    update->bind(5, util::now());
-    update->bind(8, resource.url);
+    update->bind(4, response.mustRevalidate);
+    update->bind(5, response.modified);
+    update->bind(6, util::now());
+    update->bind(9, resource.url);
 
     if (response.noContent) {
-        update->bind(6, nullptr);
-        update->bind(7, false);
+        update->bind(7, nullptr);
+        update->bind(8, false);
     } else {
-        update->bindBlob(6, data.data(), data.size(), false);
-        update->bind(7, compressed);
+        update->bindBlob(7, data.data(), data.size(), false);
+        update->bind(8, compressed);
     }
 
     update->run();
@@ -319,23 +333,24 @@ bool OfflineDatabase::putResource(const Resource& resource,
 
     // clang-format off
     Statement insert = getStatement(
-        "INSERT INTO resources (url, kind, etag, expires, modified, accessed, data, compressed) "
-        "VALUES                (?1,  ?2,   ?3,   ?4,      ?5,       ?6,       ?7,   ?8) ");
+        "INSERT INTO resources (url, kind, etag, expires, must_revalidate, modified, accessed, data, compressed) "
+        "VALUES                (?1,  ?2,   ?3,   ?4,      ?5,              ?6,       ?7,       ?8,   ?9) ");
     // clang-format on
 
     insert->bind(1, resource.url);
     insert->bind(2, int(resource.kind));
     insert->bind(3, response.etag);
     insert->bind(4, response.expires);
-    insert->bind(5, response.modified);
-    insert->bind(6, util::now());
+    insert->bind(5, response.mustRevalidate);
+    insert->bind(6, response.modified);
+    insert->bind(7, util::now());
 
     if (response.noContent) {
-        insert->bind(7, nullptr);
-        insert->bind(8, false);
+        insert->bind(8, nullptr);
+        insert->bind(9, false);
     } else {
-        insert->bindBlob(7, data.data(), data.size(), false);
-        insert->bind(8, compressed);
+        insert->bindBlob(8, data.data(), data.size(), false);
+        insert->bind(9, compressed);
     }
 
     insert->run();
@@ -366,8 +381,8 @@ optional<std::pair<Response, uint64_t>> OfflineDatabase::getTile(const Resource:
 
     // clang-format off
     Statement stmt = getStatement(
-        //        0      1        2       3        4
-        "SELECT etag, expires, modified, data, compressed "
+        //        0      1           2,            3,      4,      5
+        "SELECT etag, expires, must_revalidate, modified, data, compressed "
         "FROM tiles "
         "WHERE url_template = ?1 "
         "  AND pixel_ratio  = ?2 "
@@ -389,14 +404,15 @@ optional<std::pair<Response, uint64_t>> OfflineDatabase::getTile(const Resource:
     Response response;
     uint64_t size = 0;
 
-    response.etag     = stmt->get<optional<std::string>>(0);
-    response.expires  = stmt->get<optional<Timestamp>>(1);
-    response.modified = stmt->get<optional<Timestamp>>(2);
+    response.etag            = stmt->get<optional<std::string>>(0);
+    response.expires         = stmt->get<optional<Timestamp>>(1);
+    response.mustRevalidate  = stmt->get<bool>(2);
+    response.modified        = stmt->get<optional<Timestamp>>(3);
 
-    optional<std::string> data = stmt->get<optional<std::string>>(3);
+    optional<std::string> data = stmt->get<optional<std::string>>(4);
     if (!data) {
         response.noContent = true;
-    } else if (stmt->get<int>(4)) {
+    } else if (stmt->get<bool>(5)) {
         response.data = std::make_shared<std::string>(util::decompress(*data));
         size = data->length();
     } else {
@@ -440,22 +456,24 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile,
         // clang-format off
         Statement update = getStatement(
             "UPDATE tiles "
-            "SET accessed       = ?1, "
-            "    expires        = ?2 "
-            "WHERE url_template = ?3 "
-            "  AND pixel_ratio  = ?4 "
-            "  AND x            = ?5 "
-            "  AND y            = ?6 "
-            "  AND z            = ?7 ");
+            "SET accessed        = ?1, "
+            "    expires         = ?2, "
+            "    must_revalidate = ?3 "
+            "WHERE url_template  = ?4 "
+            "  AND pixel_ratio   = ?5 "
+            "  AND x             = ?6 "
+            "  AND y             = ?7 "
+            "  AND z             = ?8 ");
         // clang-format on
 
         update->bind(1, util::now());
         update->bind(2, response.expires);
-        update->bind(3, tile.urlTemplate);
-        update->bind(4, tile.pixelRatio);
-        update->bind(5, tile.x);
-        update->bind(6, tile.y);
-        update->bind(7, tile.z);
+        update->bind(3, response.mustRevalidate);
+        update->bind(4, tile.urlTemplate);
+        update->bind(5, tile.pixelRatio);
+        update->bind(6, tile.x);
+        update->bind(7, tile.y);
+        update->bind(8, tile.z);
         update->run();
         return false;
     }
@@ -469,35 +487,37 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile,
     // clang-format off
     Statement update = getStatement(
         "UPDATE tiles "
-        "SET modified       = ?1, "
-        "    etag           = ?2, "
-        "    expires        = ?3, "
-        "    accessed       = ?4, "
-        "    data           = ?5, "
-        "    compressed     = ?6 "
-        "WHERE url_template = ?7 "
-        "  AND pixel_ratio  = ?8 "
-        "  AND x            = ?9 "
-        "  AND y            = ?10 "
-        "  AND z            = ?11 ");
+        "SET modified        = ?1, "
+        "    etag            = ?2, "
+        "    expires         = ?3, "
+        "    must_revalidate = ?4, "
+        "    accessed        = ?5, "
+        "    data            = ?6, "
+        "    compressed      = ?7 "
+        "WHERE url_template  = ?8 "
+        "  AND pixel_ratio   = ?9 "
+        "  AND x             = ?10 "
+        "  AND y             = ?11 "
+        "  AND z             = ?12 ");
     // clang-format on
 
     update->bind(1, response.modified);
     update->bind(2, response.etag);
     update->bind(3, response.expires);
-    update->bind(4, util::now());
-    update->bind(7, tile.urlTemplate);
-    update->bind(8, tile.pixelRatio);
-    update->bind(9, tile.x);
-    update->bind(10, tile.y);
-    update->bind(11, tile.z);
+    update->bind(4, response.mustRevalidate);
+    update->bind(5, util::now());
+    update->bind(8, tile.urlTemplate);
+    update->bind(9, tile.pixelRatio);
+    update->bind(10, tile.x);
+    update->bind(11, tile.y);
+    update->bind(12, tile.z);
 
     if (response.noContent) {
-        update->bind(5, nullptr);
-        update->bind(6, false);
+        update->bind(6, nullptr);
+        update->bind(7, false);
     } else {
-        update->bindBlob(5, data.data(), data.size(), false);
-        update->bind(6, compressed);
+        update->bindBlob(6, data.data(), data.size(), false);
+        update->bind(7, compressed);
     }
 
     update->run();
@@ -508,8 +528,8 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile,
 
     // clang-format off
     Statement insert = getStatement(
-        "INSERT INTO tiles (url_template, pixel_ratio, x,  y,  z,  modified,  etag,  expires,  accessed,  data, compressed) "
-        "VALUES            (?1,           ?2,          ?3, ?4, ?5, ?6,        ?7,    ?8,       ?9,        ?10,  ?11) ");
+        "INSERT INTO tiles (url_template, pixel_ratio, x,  y,  z,  modified, must_revalidate, etag, expires, accessed,  data, compressed) "
+        "VALUES            (?1,           ?2,          ?3, ?4, ?5, ?6,       ?7,              ?8,   ?9,      ?10,       ?11,  ?12)");
     // clang-format on
 
     insert->bind(1, tile.urlTemplate);
@@ -518,16 +538,17 @@ bool OfflineDatabase::putTile(const Resource::TileData& tile,
     insert->bind(4, tile.y);
     insert->bind(5, tile.z);
     insert->bind(6, response.modified);
-    insert->bind(7, response.etag);
-    insert->bind(8, response.expires);
-    insert->bind(9, util::now());
+    insert->bind(7, response.mustRevalidate);
+    insert->bind(8, response.etag);
+    insert->bind(9, response.expires);
+    insert->bind(10, util::now());
 
     if (response.noContent) {
-        insert->bind(10, nullptr);
-        insert->bind(11, false);
+        insert->bind(11, nullptr);
+        insert->bind(12, false);
     } else {
-        insert->bindBlob(10, data.data(), data.size(), false);
-        insert->bind(11, compressed);
+        insert->bindBlob(11, data.data(), data.size(), false);
+        insert->bind(12, compressed);
     }
 
     insert->run();
diff --git a/platform/default/mbgl/storage/offline_database.hpp b/platform/default/mbgl/storage/offline_database.hpp
index 57ffcee4eb..91b544a9e0 100644
--- a/platform/default/mbgl/storage/offline_database.hpp
+++ b/platform/default/mbgl/storage/offline_database.hpp
@@ -64,6 +64,7 @@ private:
     void removeExisting();
     void migrateToVersion3();
     void migrateToVersion5();
+    void migrateToVersion6();
 
     class Statement {
     public:
diff --git a/platform/default/mbgl/storage/offline_schema.cpp.include b/platform/default/mbgl/storage/offline_schema.cpp.include
index a80c7677e6..41af81e55b 100644
--- a/platform/default/mbgl/storage/offline_schema.cpp.include
+++ b/platform/default/mbgl/storage/offline_schema.cpp.include
@@ -10,6 +10,7 @@ static const char * schema =
 "  data BLOB,\n"
 "  compressed INTEGER NOT NULL DEFAULT 0,\n"
 "  accessed INTEGER NOT NULL,\n"
+"  must_revalidate INTEGER NOT NULL DEFAULT 0,\n"
 "  UNIQUE (url)\n"
 ");\n"
 "CREATE TABLE tiles (\n"
@@ -25,6 +26,7 @@ static const char * schema =
 "  data BLOB,\n"
 "  compressed INTEGER NOT NULL DEFAULT 0,\n"
 "  accessed INTEGER NOT NULL,\n"
+"  must_revalidate INTEGER NOT NULL DEFAULT 0,\n"
 "  UNIQUE (url_template, pixel_ratio, z, x, y)\n"
 ");\n"
 "CREATE TABLE regions (\n"
diff --git a/platform/default/mbgl/storage/offline_schema.sql b/platform/default/mbgl/storage/offline_schema.sql
index 9df8fa6a89..722b0e0451 100644
--- a/platform/default/mbgl/storage/offline_schema.sql
+++ b/platform/default/mbgl/storage/offline_schema.sql
@@ -8,6 +8,7 @@ CREATE TABLE resources (                   -- Generic table for style, source, s
   data BLOB,
   compressed INTEGER NOT NULL DEFAULT 0,
   accessed INTEGER NOT NULL,
+  must_revalidate INTEGER NOT NULL DEFAULT 0,
   UNIQUE (url)
 );
 
@@ -24,6 +25,7 @@ CREATE TABLE tiles (
   data BLOB,
   compressed INTEGER NOT NULL DEFAULT 0,
   accessed INTEGER NOT NULL,
+  must_revalidate INTEGER NOT NULL DEFAULT 0,
   UNIQUE (url_template, pixel_ratio, z, x, y)
 );
 
diff --git a/platform/default/online_file_source.cpp b/platform/default/online_file_source.cpp
index f10e0f8ffb..08011f5ac1 100644
--- a/platform/default/online_file_source.cpp
+++ b/platform/default/online_file_source.cpp
@@ -320,6 +320,14 @@ void OnlineFileRequest::completed(Response response) {
         resource.priorModified = response.modified;
     }
 
+    if (response.notModified && resource.priorData) {
+        // When the priorData field is set, it indicates that we had to revalidate the request and
+        // that the requestor hasn't gotten data yet. If we get a 304 response, this means that we
+        // have send the cached data to give the requestor a chance to actually obtain the data.
+        response.data = std::move(resource.priorData);
+        response.notModified = false;
+    }
+
     bool isExpired = false;
 
     if (response.expires) {
diff --git a/platform/default/sqlite3.cpp b/platform/default/sqlite3.cpp
index 269c97716f..2e08354fdf 100644
--- a/platform/default/sqlite3.cpp
+++ b/platform/default/sqlite3.cpp
@@ -287,6 +287,11 @@ bool Statement::run() {
     }
 }
 
+template <> bool Statement::get(int offset) {
+    assert(impl);
+    return sqlite3_column_int(impl->stmt, offset);
+}
+
 template <> int Statement::get(int offset) {
     assert(impl);
     return sqlite3_column_int(impl->stmt, offset);