From f0a7c45064c3ce3f509b1c2035fcaa07ccc35a99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Tue, 1 Aug 2017 18:00:48 +0200 Subject: [core] finish must-revalidate support --- .gitignore | 4 +- include/mbgl/storage/resource.hpp | 1 + include/mbgl/storage/response.hpp | 10 ++ platform/android/src/http_file_source.cpp | 4 +- platform/darwin/src/http_file_source.mm | 4 +- platform/default/default_file_source.cpp | 12 +- platform/default/http_file_source.cpp | 4 +- platform/default/mbgl/storage/offline_database.cpp | 191 ++++++++++++--------- platform/default/mbgl/storage/offline_database.hpp | 1 + .../mbgl/storage/offline_schema.cpp.include | 2 + platform/default/mbgl/storage/offline_schema.sql | 2 + platform/default/online_file_source.cpp | 8 + platform/default/sqlite3.cpp | 5 + platform/qt/src/http_request.cpp | 4 +- platform/qt/src/sqlite3.cpp | 1 + src/mbgl/storage/response.cpp | 1 + test/fixtures/offline_database/v5.db | Bin 0 -> 19456 bytes test/storage/default_file_source.test.cpp | 110 ++++++++---- test/storage/http_file_source.test.cpp | 9 + test/storage/offline_database.test.cpp | 69 ++++++-- test/storage/online_file_source.test.cpp | 10 ++ test/storage/server.js | 6 +- 22 files changed, 316 insertions(+), 142 deletions(-) create mode 100644 test/fixtures/offline_database/v5.db diff --git a/.gitignore b/.gitignore index b6a8498460..ce65e4c9b6 100644 --- a/.gitignore +++ b/.gitignore @@ -19,8 +19,8 @@ xcuserdata /test/fixtures/offline_database/offline.db-* /test/fixtures/offline_database/invalid.db /test/fixtures/offline_database/invalid.db-* -/test/fixtures/offline_database/v*.db -/test/fixtures/offline_database/v*.db-* +/test/fixtures/offline_database/migrated.db +/test/fixtures/offline_database/migrated.db-* /test/fixtures/**/actual.png /test/fixtures/**/diff.png /test/output diff --git a/include/mbgl/storage/resource.hpp b/include/mbgl/storage/resource.hpp index 7e9ced8049..5d44f4869f 100644 --- a/include/mbgl/storage/resource.hpp +++ b/include/mbgl/storage/resource.hpp @@ -68,6 +68,7 @@ public: optional priorModified = {}; optional priorExpires = {}; optional priorEtag = {}; + std::shared_ptr priorData; }; } // namespace mbgl diff --git a/include/mbgl/storage/response.hpp b/include/mbgl/storage/response.hpp index 32fe4e0c8a..711f008e83 100644 --- a/include/mbgl/storage/response.hpp +++ b/include/mbgl/storage/response.hpp @@ -27,6 +27,10 @@ public: // This is set to true for 304 Not Modified responses. bool notModified = false; + // This is set to true when the server requested that no expired resources be used by + // specifying "Cache-Control: must-revalidate". + bool mustRevalidate = false; + // The actual data of the response. Present only for non-error, non-notModified responses. std::shared_ptr data; @@ -37,6 +41,12 @@ public: bool isFresh() const { return expires ? *expires > util::now() : !error; } + + // Indicates whether we are allowed to use this response according to HTTP caching rules. + // It may or may not be stale. + bool isUsable() const { + return !mustRevalidate || (expires && *expires > util::now()); + } }; class Response::Error { diff --git a/platform/android/src/http_file_source.cpp b/platform/android/src/http_file_source.cpp index ee1429bd74..8eb9416e9d 100644 --- a/platform/android/src/http_file_source.cpp +++ b/platform/android/src/http_file_source.cpp @@ -117,7 +117,9 @@ void HTTPRequest::onResponse(jni::JNIEnv& env, int code, } if (cacheControl) { - response.expires = http::CacheControl::parse(jni::Make(env, cacheControl).c_str()).toTimePoint(); + const auto cc = http::CacheControl::parse(jni::Make(env, cacheControl).c_str()); + response.expires = cc.toTimePoint(); + response.mustRevalidate = cc.mustRevalidate; } if (expires) { diff --git a/platform/darwin/src/http_file_source.mm b/platform/darwin/src/http_file_source.mm index 649cebb47f..4a16ad82fb 100644 --- a/platform/darwin/src/http_file_source.mm +++ b/platform/darwin/src/http_file_source.mm @@ -266,7 +266,9 @@ std::unique_ptr HTTPFileSource::request(const Resource& resource, NSDictionary *headers = [(NSHTTPURLResponse *)res allHeaderFields]; NSString *cache_control = [headers objectForKey:@"Cache-Control"]; if (cache_control) { - response.expires = http::CacheControl::parse([cache_control UTF8String]).toTimePoint(); + const auto cc = http::CacheControl::parse([cache_control UTF8String]); + response.expires = cc.toTimePoint(); + response.mustRevalidate = cc.mustRevalidate; } NSString *expires = [headers objectForKey:@"Expires"]; 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> 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> OfflineDatabase::getResource(const Resou Response response; uint64_t size = 0; - response.etag = stmt->get>(0); - response.expires = stmt->get>(1); - response.modified = stmt->get>(2); + response.etag = stmt->get>(0); + response.expires = stmt->get>(1); + response.mustRevalidate = stmt->get(2); + response.modified = stmt->get>(3); - optional data = stmt->get>(3); + optional data = stmt->get>(4); if (!data) { response.noContent = true; - } else if (stmt->get(4)) { + } else if (stmt->get(5)) { response.data = std::make_shared(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> 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> OfflineDatabase::getTile(const Resource: Response response; uint64_t size = 0; - response.etag = stmt->get>(0); - response.expires = stmt->get>(1); - response.modified = stmt->get>(2); + response.etag = stmt->get>(0); + response.expires = stmt->get>(1); + response.mustRevalidate = stmt->get(2); + response.modified = stmt->get>(3); - optional data = stmt->get>(3); + optional data = stmt->get>(4); if (!data) { response.noContent = true; - } else if (stmt->get(4)) { + } else if (stmt->get(5)) { response.data = std::make_shared(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); diff --git a/platform/qt/src/http_request.cpp b/platform/qt/src/http_request.cpp index ef753c3e0e..386a2d9ef4 100644 --- a/platform/qt/src/http_request.cpp +++ b/platform/qt/src/http_request.cpp @@ -82,7 +82,9 @@ void HTTPRequest::handleNetworkReply(QNetworkReply *reply) } else if (header == "etag") { response.etag = std::string(line.second.constData(), line.second.size()); } else if (header == "cache-control") { - response.expires = http::CacheControl::parse(line.second.constData()).toTimePoint(); + const auto cc = http::CacheControl::parse(line.second.constData()); + response.expires = cc.toTimePoint(); + response.mustRevalidate = cc.mustRevalidate; } else if (header == "expires") { response.expires = util::parseTimestamp(line.second.constData()); } else if (header == "retry-after") { diff --git a/platform/qt/src/sqlite3.cpp b/platform/qt/src/sqlite3.cpp index 0cd78d85ce..7d47ae552b 100644 --- a/platform/qt/src/sqlite3.cpp +++ b/platform/qt/src/sqlite3.cpp @@ -312,6 +312,7 @@ bool Statement::run() { return impl->query.next(); } +template bool Statement::get(int); template int Statement::get(int); template int64_t Statement::get(int); template double Statement::get(int); diff --git a/src/mbgl/storage/response.cpp b/src/mbgl/storage/response.cpp index a174339074..222f55db84 100644 --- a/src/mbgl/storage/response.cpp +++ b/src/mbgl/storage/response.cpp @@ -14,6 +14,7 @@ Response& Response::operator=(const Response& res) { error = res.error ? std::make_unique(*res.error) : nullptr; noContent = res.noContent; notModified = res.notModified; + mustRevalidate = res.mustRevalidate; data = res.data; modified = res.modified; expires = res.expires; diff --git a/test/fixtures/offline_database/v5.db b/test/fixtures/offline_database/v5.db new file mode 100644 index 0000000000..9bba351208 Binary files /dev/null and b/test/fixtures/offline_database/v5.db differ diff --git a/test/storage/default_file_source.test.cpp b/test/storage/default_file_source.test.cpp index 41e305a692..b8aebae7e7 100644 --- a/test/storage/default_file_source.test.cpp +++ b/test/storage/default_file_source.test.cpp @@ -22,6 +22,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(CacheResponse)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response 1", *res.data); EXPECT_TRUE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); response = res; @@ -34,6 +35,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(CacheResponse)) { ASSERT_TRUE(res2.data.get()); EXPECT_EQ(*response.data, *res2.data); EXPECT_EQ(response.expires, res2.expires); + EXPECT_EQ(response.mustRevalidate, res2.mustRevalidate); EXPECT_EQ(response.modified, res2.modified); EXPECT_EQ(response.etag, res2.etag); @@ -51,35 +53,53 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(CacheRevalidateSame)) { const Resource revalidateSame { Resource::Unknown, "http://127.0.0.1:3000/revalidate-same" }; std::unique_ptr req1; std::unique_ptr req2; - uint16_t counter = 0; + bool gotResponse = false; // First request causes the response to get cached. req1 = fs.request(revalidateSame, [&](Response res) { req1.reset(); EXPECT_EQ(nullptr, res.error); + EXPECT_FALSE(res.notModified); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_TRUE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_EQ("snowfall", *res.etag); - // Second request returns the cached response, then immediately revalidates. - req2 = fs.request(revalidateSame, [&, res](Response res2) { - if (counter == 0) { - ++counter; + // The first response is stored in the cache, but it has 'must-revalidate' set. This means + // it can't return the cached response right away and we must wait for the revalidation + // request to complete. We can distinguish the cached response from the revalided response + // because the latter has an expiration date, while the cached response doesn't. + req2 = fs.request(revalidateSame, [&](Response res2) { + if (!gotResponse) { + // Even though we could find the response in the database, we send a revalidation + // request and get a 304 response. Since we haven't sent a reply yet, we're forcing + // notModified to be false so that implementations can continue to use the + // notModified flag to skip parsing new data. + gotResponse = true; + EXPECT_EQ(nullptr, res2.error); EXPECT_FALSE(res2.notModified); + ASSERT_TRUE(res2.data.get()); + EXPECT_EQ("Response", *res2.data); + EXPECT_TRUE(bool(res2.expires)); + EXPECT_TRUE(res2.mustRevalidate); + EXPECT_FALSE(bool(res2.modified)); + EXPECT_EQ("snowfall", *res2.etag); } else { + // The test server sends a Cache-Control header with a max-age of 1 second. This + // means that our OnlineFileSource implementation will request the tile again after + // 1 second. This time, our request already had a prior response, so we don't need + // to send the data again, and instead can actually forward the notModified flag. req2.reset(); - EXPECT_EQ(nullptr, res2.error); EXPECT_TRUE(res2.notModified); - ASSERT_FALSE(res2.data.get()); + EXPECT_FALSE(res2.data.get()); EXPECT_TRUE(bool(res2.expires)); + EXPECT_TRUE(res2.mustRevalidate); EXPECT_FALSE(bool(res2.modified)); - // We're not sending the ETag in the 304 reply, but it should still be there. EXPECT_EQ("snowfall", *res2.etag); - loop.stop(); } }); @@ -96,34 +116,53 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(CacheRevalidateModified)) { "http://127.0.0.1:3000/revalidate-modified" }; std::unique_ptr req1; std::unique_ptr req2; - uint16_t counter = 0; + bool gotResponse = false; // First request causes the response to get cached. req1 = fs.request(revalidateModified, [&](Response res) { req1.reset(); EXPECT_EQ(nullptr, res.error); + EXPECT_FALSE(res.notModified); ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_TRUE(res.mustRevalidate); EXPECT_EQ(Timestamp{ Seconds(1420070400) }, *res.modified); EXPECT_FALSE(res.etag); - // Second request returns the cached response, then immediately revalidates. + // The first response is stored in the cache, but it has 'must-revalidate' set. This means + // it can't return the cached response right away and we must wait for the revalidation + // request to complete. We can distinguish the cached response from the revalided response + // because the latter has an expiration date, while the cached response doesn't. req2 = fs.request(revalidateModified, [&, res](Response res2) { - if (counter == 0) { - ++counter; + if (!gotResponse) { + // Even though we could find the response in the database, we send a revalidation + // request and get a 304 response. Since we haven't sent a reply yet, we're forcing + // notModified to be false so that implementations can continue to use the + // notModified flag to skip parsing new data. + gotResponse = true; + EXPECT_EQ(nullptr, res2.error); EXPECT_FALSE(res2.notModified); + ASSERT_TRUE(res2.data.get()); + EXPECT_EQ("Response", *res2.data); + EXPECT_TRUE(bool(res2.expires)); + EXPECT_TRUE(res2.mustRevalidate); + EXPECT_EQ(Timestamp{ Seconds(1420070400) }, *res2.modified); + EXPECT_FALSE(res2.etag); } else { + // The test server sends a Cache-Control header with a max-age of 1 second. This + // means that our OnlineFileSource implementation will request the tile again after + // 1 second. This time, our request already had a prior response, so we don't need + // to send the data again, and instead can actually forward the notModified flag. req2.reset(); - EXPECT_EQ(nullptr, res2.error); EXPECT_TRUE(res2.notModified); - ASSERT_FALSE(res2.data.get()); + EXPECT_FALSE(res2.data.get()); EXPECT_TRUE(bool(res2.expires)); + EXPECT_TRUE(res2.mustRevalidate); EXPECT_EQ(Timestamp{ Seconds(1420070400) }, *res2.modified); EXPECT_FALSE(res2.etag); - loop.stop(); } }); @@ -139,7 +178,6 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(CacheRevalidateEtag)) { const Resource revalidateEtag { Resource::Unknown, "http://127.0.0.1:3000/revalidate-etag" }; std::unique_ptr req1; std::unique_ptr req2; - uint16_t counter = 0; // First request causes the response to get cached. req1 = fs.request(revalidateEtag, [&](Response res) { @@ -149,27 +187,24 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(CacheRevalidateEtag)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response 1", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_TRUE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_EQ("response-1", *res.etag); - // Second request returns the cached response, then immediately revalidates. + // Second request does not return the cached response, since it had Cache-Control: must-revalidate set. req2 = fs.request(revalidateEtag, [&, res](Response res2) { - if (counter == 0) { - ++counter; - EXPECT_FALSE(res2.notModified); - } else { - req2.reset(); + req2.reset(); - EXPECT_EQ(nullptr, res2.error); - ASSERT_TRUE(res2.data.get()); - EXPECT_NE(res.data, res2.data); - EXPECT_EQ("Response 2", *res2.data); - EXPECT_FALSE(bool(res2.expires)); - EXPECT_FALSE(bool(res2.modified)); - EXPECT_EQ("response-2", *res2.etag); + EXPECT_EQ(nullptr, res2.error); + ASSERT_TRUE(res2.data.get()); + EXPECT_NE(res.data, res2.data); + EXPECT_EQ("Response 2", *res2.data); + EXPECT_FALSE(bool(res2.expires)); + EXPECT_TRUE(res2.mustRevalidate); + EXPECT_FALSE(bool(res2.modified)); + EXPECT_EQ("response-2", *res2.etag); - loop.stop(); - } + loop.stop(); }); }); @@ -203,6 +238,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(HTTPIssue1369)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -232,6 +268,7 @@ TEST(DefaultFileSource, OptionalNonExpired) { EXPECT_EQ("Cached value", *res.data); ASSERT_TRUE(bool(res.expires)); EXPECT_EQ(*response.expires, *res.expires); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -261,6 +298,7 @@ TEST(DefaultFileSource, OptionalExpired) { EXPECT_EQ("Cached value", *res.data); ASSERT_TRUE(bool(res.expires)); EXPECT_EQ(*response.expires, *res.expires); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -301,6 +339,7 @@ TEST(DefaultFileSource, OptionalNotFound) { EXPECT_EQ("Not found in offline database", res.error->message); EXPECT_FALSE(res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -334,6 +373,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(NoCacheRefreshEtagNotModified)) { EXPECT_FALSE(res.data.get()); ASSERT_TRUE(bool(res.expires)); EXPECT_LT(util::now(), *res.expires); + EXPECT_TRUE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); ASSERT_TRUE(bool(res.etag)); EXPECT_EQ("snowfall", *res.etag); @@ -368,6 +408,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(NoCacheRefreshEtagModified)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_TRUE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); ASSERT_TRUE(bool(res.etag)); EXPECT_EQ("snowfall", *res.etag); @@ -403,6 +444,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(NoCacheFull)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_TRUE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); ASSERT_TRUE(bool(res.etag)); EXPECT_EQ("snowfall", *res.etag); @@ -437,6 +479,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(NoCacheRefreshModifiedNotModified)) EXPECT_FALSE(res.data.get()); ASSERT_TRUE(bool(res.expires)); EXPECT_LT(util::now(), *res.expires); + EXPECT_TRUE(res.mustRevalidate); ASSERT_TRUE(bool(res.modified)); EXPECT_EQ(Timestamp{ Seconds(1420070400) }, *res.modified); EXPECT_FALSE(bool(res.etag)); @@ -471,6 +514,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(NoCacheRefreshModifiedModified)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_TRUE(res.mustRevalidate); EXPECT_EQ(Timestamp{ Seconds(1420070400) }, *res.modified); EXPECT_FALSE(res.etag); loop.stop(); @@ -502,6 +546,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(SetResourceTransform)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -518,6 +563,7 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(SetResourceTransform)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); diff --git a/test/storage/http_file_source.test.cpp b/test/storage/http_file_source.test.cpp index 5b081d7d57..006b7a0fb3 100644 --- a/test/storage/http_file_source.test.cpp +++ b/test/storage/http_file_source.test.cpp @@ -26,6 +26,7 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(HTTP200)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -44,6 +45,7 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(HTTP404)) { EXPECT_EQ("HTTP status code 404", res.error->message); EXPECT_FALSE(bool(res.data)); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -61,6 +63,7 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(HTTPTile404)) { EXPECT_FALSE(bool(res.error)); EXPECT_FALSE(bool(res.data)); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -78,6 +81,7 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(HTTP200EmptyData)) { EXPECT_FALSE(bool(res.error)); EXPECT_EQ(*res.data, std::string()); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -95,6 +99,7 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(HTTP204)) { EXPECT_FALSE(bool(res.error)); EXPECT_FALSE(bool(res.data)); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -113,6 +118,7 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(HTTP500)) { EXPECT_EQ("HTTP status code 500", res.error->message); EXPECT_FALSE(bool(res.data)); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -131,6 +137,7 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(ExpiresParsing)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_EQ(Timestamp{ Seconds(1420797926) }, res.expires); + EXPECT_FALSE(res.mustRevalidate); EXPECT_EQ(Timestamp{ Seconds(1420794326) }, res.modified); EXPECT_EQ("foo", *res.etag); loop.stop(); @@ -148,6 +155,7 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(CacheControlParsing)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_GT(Seconds(2), util::abs(*res.expires - util::now() - Seconds(120))) << "Expiration date isn't about 120 seconds in the future"; + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -176,6 +184,7 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(Load)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ(std::string("Request ") + std::to_string(current), *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp index 872310e46f..c196107639 100644 --- a/test/storage/offline_database.test.cpp +++ b/test/storage/offline_database.test.cpp @@ -634,24 +634,35 @@ static int databaseSyncMode(const std::string& path) { return stmt.get(0); } +static std::vector databaseTableColumns(const std::string& path, const std::string& name) { + mapbox::sqlite::Database db(path, mapbox::sqlite::ReadOnly); + const auto sql = std::string("pragma table_info(") + name + ")"; + mapbox::sqlite::Statement stmt = db.prepare(sql.c_str()); + std::vector columns; + while (stmt.run()) { + columns.push_back(stmt.get(1)); + } + return columns; +} + TEST(OfflineDatabase, MigrateFromV2Schema) { using namespace mbgl; // v2.db is a v2 database containing a single offline region with a small number of resources. - deleteFile("test/fixtures/offline_database/v5.db"); - writeFile("test/fixtures/offline_database/v5.db", util::read_file("test/fixtures/offline_database/v2.db")); + deleteFile("test/fixtures/offline_database/migrated.db"); + writeFile("test/fixtures/offline_database/migrated.db", util::read_file("test/fixtures/offline_database/v2.db")); { - OfflineDatabase db("test/fixtures/offline_database/v5.db", 0); + OfflineDatabase db("test/fixtures/offline_database/migrated.db", 0); auto regions = db.listRegions(); for (auto& region : regions) { db.deleteRegion(std::move(region)); } } - EXPECT_EQ(5, databaseUserVersion("test/fixtures/offline_database/v5.db")); - EXPECT_LT(databasePageCount("test/fixtures/offline_database/v5.db"), + EXPECT_EQ(6, databaseUserVersion("test/fixtures/offline_database/migrated.db")); + EXPECT_LT(databasePageCount("test/fixtures/offline_database/migrated.db"), databasePageCount("test/fixtures/offline_database/v2.db")); } @@ -660,18 +671,18 @@ TEST(OfflineDatabase, MigrateFromV3Schema) { // v3.db is a v3 database, migrated from v2. - deleteFile("test/fixtures/offline_database/v5.db"); - writeFile("test/fixtures/offline_database/v5.db", util::read_file("test/fixtures/offline_database/v3.db")); + deleteFile("test/fixtures/offline_database/migrated.db"); + writeFile("test/fixtures/offline_database/migrated.db", util::read_file("test/fixtures/offline_database/v3.db")); { - OfflineDatabase db("test/fixtures/offline_database/v5.db", 0); + OfflineDatabase db("test/fixtures/offline_database/migrated.db", 0); auto regions = db.listRegions(); for (auto& region : regions) { db.deleteRegion(std::move(region)); } } - EXPECT_EQ(5, databaseUserVersion("test/fixtures/offline_database/v5.db")); + EXPECT_EQ(6, databaseUserVersion("test/fixtures/offline_database/migrated.db")); } TEST(OfflineDatabase, MigrateFromV4Schema) { @@ -679,22 +690,50 @@ TEST(OfflineDatabase, MigrateFromV4Schema) { // v4.db is a v4 database, migrated from v2 & v3. This database used `journal_mode = WAL` and `synchronous = NORMAL`. - deleteFile("test/fixtures/offline_database/v5.db"); - writeFile("test/fixtures/offline_database/v5.db", util::read_file("test/fixtures/offline_database/v4.db")); + deleteFile("test/fixtures/offline_database/migrated.db"); + writeFile("test/fixtures/offline_database/migrated.db", util::read_file("test/fixtures/offline_database/v4.db")); { - OfflineDatabase db("test/fixtures/offline_database/v5.db", 0); + OfflineDatabase db("test/fixtures/offline_database/migrated.db", 0); auto regions = db.listRegions(); for (auto& region : regions) { db.deleteRegion(std::move(region)); } } - EXPECT_EQ(5, databaseUserVersion("test/fixtures/offline_database/v5.db")); + EXPECT_EQ(6, databaseUserVersion("test/fixtures/offline_database/migrated.db")); // Journal mode should be DELETE after migration to v5. - EXPECT_EQ("delete", databaseJournalMode("test/fixtures/offline_database/v5.db")); + EXPECT_EQ("delete", databaseJournalMode("test/fixtures/offline_database/migrated.db")); // Synchronous setting should be FULL (2) after migration to v5. - EXPECT_EQ(2, databaseSyncMode("test/fixtures/offline_database/v5.db")); + EXPECT_EQ(2, databaseSyncMode("test/fixtures/offline_database/migrated.db")); +} + + +TEST(OfflineDatabase, MigrateFromV5Schema) { + using namespace mbgl; + + // v5.db is a v5 database, migrated from v2, v3 & v4. + + deleteFile("test/fixtures/offline_database/migrated.db"); + writeFile("test/fixtures/offline_database/migrated.db", util::read_file("test/fixtures/offline_database/v5.db")); + + { + OfflineDatabase db("test/fixtures/offline_database/migrated.db", 0); + auto regions = db.listRegions(); + for (auto& region : regions) { + db.deleteRegion(std::move(region)); + } + } + + EXPECT_EQ(6, databaseUserVersion("test/fixtures/offline_database/migrated.db")); + + EXPECT_EQ((std::vector{ "id", "url_template", "pixel_ratio", "z", "x", "y", + "expires", "modified", "etag", "data", "compressed", + "accessed", "must_revalidate" }), + databaseTableColumns("test/fixtures/offline_database/migrated.db", "tiles")); + EXPECT_EQ((std::vector{ "id", "url", "kind", "expires", "modified", "etag", "data", + "compressed", "accessed", "must_revalidate" }), + databaseTableColumns("test/fixtures/offline_database/migrated.db", "resources")); } diff --git a/test/storage/online_file_source.test.cpp b/test/storage/online_file_source.test.cpp index 1a1d2d42f8..70bfe3ac95 100644 --- a/test/storage/online_file_source.test.cpp +++ b/test/storage/online_file_source.test.cpp @@ -36,6 +36,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(CancelMultiple)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -62,6 +63,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(TemporaryError)) { EXPECT_EQ("HTTP status code 500", res.error->message); ASSERT_FALSE(bool(res.data)); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); } break; @@ -73,6 +75,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(TemporaryError)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -99,6 +102,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(ConnectionError)) { EXPECT_EQ(Response::Error::Reason::Connection, res.error->reason); ASSERT_FALSE(res.data.get()); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); @@ -126,6 +130,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(Timeout)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Hello World!", *res.data); EXPECT_TRUE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); if (counter == 4) { @@ -150,6 +155,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RetryDelayOnExpiredTile)) { counter++; EXPECT_EQ(nullptr, res.error); EXPECT_GT(util::now(), *res.expires); + EXPECT_FALSE(res.mustRevalidate); }); util::Timer timer; @@ -170,6 +176,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RetryOnClockSkew)) { const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/clockskew" }; std::unique_ptr req1 = fs.request(resource, [&](Response res) { + EXPECT_FALSE(res.mustRevalidate); switch (counter++) { case 0: { EXPECT_EQ(nullptr, res.error); @@ -240,6 +247,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(Load)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ(std::string("Request ") + std::to_string(current), *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); @@ -277,6 +285,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(NetworkStatusChange)) { ASSERT_TRUE(res.data.get()); EXPECT_EQ("Response", *res.data); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); loop.stop(); @@ -315,6 +324,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(NetworkStatusChangePreempt)) { EXPECT_EQ(Response::Error::Reason::Connection, res.error->reason); ASSERT_FALSE(res.data.get()); EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); EXPECT_FALSE(bool(res.modified)); EXPECT_FALSE(bool(res.etag)); diff --git a/test/storage/server.js b/test/storage/server.js index b54ff835ec..d6429e4635 100755 --- a/test/storage/server.js +++ b/test/storage/server.js @@ -44,8 +44,8 @@ app.get('/cache', function(req, res) { app.get('/revalidate-same', function(req, res) { if (req.headers['if-none-match'] == 'snowfall') { - // Second request can be cached for 30 seconds. - res.setHeader('Cache-Control', 'max-age=30'); + // Second request can be cached for 1 second. + res.setHeader('Cache-Control', 'max-age=1, must-revalidate'); res.status(304).end(); } else { // First request must always be revalidated. @@ -67,7 +67,7 @@ app.get('/revalidate-modified', function(req, res) { if (req.headers['if-modified-since']) { var modified_since = new Date(req.headers['if-modified-since']); if (modified_since >= jan1) { - res.setHeader('Cache-Control', 'max-age=30'); + res.setHeader('Cache-Control', 'max-age=1, must-revalidate'); res.status(304).end(); return; } -- cgit v1.2.1