From 6a7334b882a47ca193209f2012843e42aa3ed4e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Thu, 15 Oct 2015 21:30:21 +0200 Subject: [core] add support for stale responses We're now returning stale responses from cache. Those responses will have the `stale` flag set to true. Currently, all requesters in the core code discard stale responses, and cancel the request immediately after they got a non-stale response. --- include/mbgl/storage/response.hpp | 4 + src/mbgl/map/map_context.cpp | 4 + src/mbgl/map/raster_tile_data.cpp | 4 + src/mbgl/map/source.cpp | 4 + src/mbgl/map/sprite.cpp | 8 ++ src/mbgl/map/vector_tile_data.cpp | 4 + src/mbgl/storage/default_file_source.cpp | 78 +++++++++++---- src/mbgl/storage/default_file_source_impl.hpp | 2 + src/mbgl/storage/request.cpp | 1 - src/mbgl/storage/response.cpp | 12 +++ src/mbgl/text/glyph_pbf.cpp | 4 + test/storage/cache_response.cpp | 30 ++++-- test/storage/cache_revalidate.cpp | 139 +++++++++++++++++++++++--- test/storage/directory_reading.cpp | 1 + test/storage/file_reading.cpp | 3 + test/storage/http_cancel.cpp | 1 + test/storage/http_coalescing.cpp | 105 +++++++++++++++++++ test/storage/http_error.cpp | 2 + test/storage/http_header_parsing.cpp | 2 + test/storage/http_issue_1369.cpp | 1 + test/storage/http_load.cpp | 1 + test/storage/http_other_loop.cpp | 1 + test/storage/http_reading.cpp | 3 + 23 files changed, 367 insertions(+), 47 deletions(-) create mode 100644 src/mbgl/storage/response.cpp diff --git a/include/mbgl/storage/response.hpp b/include/mbgl/storage/response.hpp index e665f177fc..b232cd06f4 100644 --- a/include/mbgl/storage/response.hpp +++ b/include/mbgl/storage/response.hpp @@ -6,10 +6,14 @@ namespace mbgl { class Response { +public: + bool isExpired() const; + public: enum Status { Error, Successful, NotFound }; Status status = Error; + bool stale = false; std::string message; int64_t modified = 0; int64_t expires = 0; diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp index 0888ef2846..b197ea12f3 100644 --- a/src/mbgl/map/map_context.cpp +++ b/src/mbgl/map/map_context.cpp @@ -106,6 +106,10 @@ void MapContext::setStyleURL(const std::string& url) { FileSource* fs = util::ThreadContext::getFileSource(); styleRequest = fs->request({ Resource::Kind::Style, styleURL }, util::RunLoop::getLoop(), [this, base](const Response &res) { + if (res.stale) { + // Only handle fresh responses. + return; + } styleRequest = nullptr; if (res.status == Response::Successful) { diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp index 4ae9189a0b..cc7b6b548f 100644 --- a/src/mbgl/map/raster_tile_data.cpp +++ b/src/mbgl/map/raster_tile_data.cpp @@ -31,6 +31,10 @@ void RasterTileData::request(float pixelRatio, FileSource* fs = util::ThreadContext::getFileSource(); req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::getLoop(), [url, callback, this](const Response &res) { + if (res.stale) { + // Only handle fresh responses. + return; + } req = nullptr; if (res.status == Response::NotFound) { diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index d723e308b1..7e30682895 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -149,6 +149,10 @@ void Source::load() { FileSource* fs = util::ThreadContext::getFileSource(); req = fs->request({ Resource::Kind::Source, info.url }, util::RunLoop::getLoop(), [this](const Response &res) { + if (res.stale) { + // Only handle fresh responses. + return; + } req = nullptr; if (res.status != Response::Successful) { diff --git a/src/mbgl/map/sprite.cpp b/src/mbgl/map/sprite.cpp index 9c099b4aa2..a54d96f005 100644 --- a/src/mbgl/map/sprite.cpp +++ b/src/mbgl/map/sprite.cpp @@ -44,6 +44,10 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_) FileSource* fs = util::ThreadContext::getFileSource(); loader->jsonRequest = fs->request({ Resource::Kind::SpriteJSON, jsonURL }, util::RunLoop::getLoop(), [this, jsonURL](const Response& res) { + if (res.stale) { + // Only handle fresh responses. + return; + } loader->jsonRequest = nullptr; if (res.status == Response::Successful) { loader->data->json = res.data; @@ -60,6 +64,10 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_) loader->spriteRequest = fs->request({ Resource::Kind::SpriteImage, spriteURL }, util::RunLoop::getLoop(), [this, spriteURL](const Response& res) { + if (res.stale) { + // Only handle fresh responses. + return; + } loader->spriteRequest = nullptr; if (res.status == Response::Successful) { loader->data->image = res.data; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 6aa643b16f..2e683daaff 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -43,6 +43,10 @@ void VectorTileData::request(float pixelRatio, const std::function& call FileSource* fs = util::ThreadContext::getFileSource(); req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::getLoop(), [url, callback, this](const Response &res) { + if (res.stale) { + // Only handle fresh responses. + return; + } req = nullptr; if (res.status == Response::NotFound) { diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp index 7b3cff8253..71c42f6220 100644 --- a/src/mbgl/storage/default_file_source.cpp +++ b/src/mbgl/storage/default_file_source.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include #include @@ -102,43 +101,79 @@ void DefaultFileSource::Impl::add(Request* req) { const Resource& resource = req->resource; DefaultFileRequest* request = find(resource); - if (request) { - request->observers.insert(req); - return; + if (!request) { + request = &pending.emplace(resource, resource).first->second; } - request = &pending.emplace(resource, resource).first->second; + // Add this request as an observer so that it'll get notified when something about this + // request changes. request->observers.insert(req); - if (cache) { - startCacheRequest(request); + update(request); + + if (request->response) { + // We've got a response, so send the (potentially stale) response to the requester. + req->notify(request->response); + } +} + +void DefaultFileSource::Impl::update(DefaultFileRequest* request) { + if (request->response) { + // We've at least obtained a cache value, potentially we also got a final response. + // The observers have been notified already; send what we have to the new one as well. + + // Before returning the existing response, make sure that it is still fresh. + if (!request->response->stale && request->response->isExpired()) { + // Create a new Response object with `stale = true`, but the same data, and + // replace the current request object we have. + // TODO: Make content shared_ptrs so we won't make copies of the content. + auto response = std::make_shared(*request->response); + response->stale = true; + request->response = response; + } + + if (request->response->stale && !request->realRequest) { + // We've returned a stale response; now make sure the requester also gets a fresh + // response eventually. It's possible that there's already a request in progress. + // Note that this will also trigger updates to all other existing listeners. + // Since we already have data, we're going to verify + startRealRequest(request, request->response); + } + } else if (!request->cacheRequest && !request->realRequest) { + // There is no request in progress, and we don't have a response yet. This means we'll have + // to start the request ourselves. + if (cache) { + startCacheRequest(request); + } else { + startRealRequest(request); + } } else { - startRealRequest(request); + // There is a request in progress. We just have to wait. } } void DefaultFileSource::Impl::startCacheRequest(DefaultFileRequest* request) { // Check the cache for existing data so that we can potentially // revalidate the information without having to redownload everything. - request->cacheRequest = cache->get(request->resource, [this, request](std::unique_ptr response) { - auto expired = [&response] { - const int64_t now = std::chrono::duration_cast( - SystemClock::now().time_since_epoch()).count(); - return response->expires <= now; - }; - - if (!response || expired()) { + request->cacheRequest = cache->get(request->resource, [this, request](std::shared_ptr response) { + request->cacheRequest = nullptr; + if (response) { + response->stale = response->isExpired(); + + // Notify in all cases; requestors can decide whether they want to use stale responses. + notify(request, response, FileCache::Hint::No); + } + + if (!response || response->stale) { // No response or stale cache. Run the real request. - startRealRequest(request, std::move(response)); - } else { - // The response is fresh. We're good to notify the caller. - notify(request, std::move(response), FileCache::Hint::No); + startRealRequest(request, response); } }); } void DefaultFileSource::Impl::startRealRequest(DefaultFileRequest* request, std::shared_ptr response) { auto callback = [request, this] (std::shared_ptr res, FileCache::Hint hint) { + request->realRequest = nullptr; notify(request, res, hint); }; @@ -181,6 +216,7 @@ void DefaultFileSource::Impl::notify(DefaultFileRequest* request, std::shared_pt assert(response); // Notify all observers. + request->response = response; for (auto req : request->observers) { req->notify(response); } @@ -189,8 +225,6 @@ void DefaultFileSource::Impl::notify(DefaultFileRequest* request, std::shared_pt // Store response in database cache->put(request->resource, response, hint); } - - pending.erase(request->resource); } } diff --git a/src/mbgl/storage/default_file_source_impl.hpp b/src/mbgl/storage/default_file_source_impl.hpp index bb28933798..980eedc118 100644 --- a/src/mbgl/storage/default_file_source_impl.hpp +++ b/src/mbgl/storage/default_file_source_impl.hpp @@ -15,6 +15,7 @@ class RequestBase; struct DefaultFileRequest { const Resource resource; std::set observers; + std::shared_ptr response; std::unique_ptr cacheRequest; RequestBase* realRequest = nullptr; @@ -39,6 +40,7 @@ public: private: DefaultFileRequest* find(const Resource&); + void update(DefaultFileRequest*); void startCacheRequest(DefaultFileRequest*); void startRealRequest(DefaultFileRequest*, std::shared_ptr = nullptr); void notify(DefaultFileRequest*, std::shared_ptr, FileCache::Hint); diff --git a/src/mbgl/storage/request.cpp b/src/mbgl/storage/request.cpp index f8ad555379..55913846cf 100644 --- a/src/mbgl/storage/request.cpp +++ b/src/mbgl/storage/request.cpp @@ -49,7 +49,6 @@ Request::~Request() = default; // Called in the FileSource thread. void Request::notify(const std::shared_ptr &response_) { - assert(!std::atomic_load(&response)); assert(response_); std::atomic_store(&response, response_); async->send(); diff --git a/src/mbgl/storage/response.cpp b/src/mbgl/storage/response.cpp new file mode 100644 index 0000000000..628a2a3b99 --- /dev/null +++ b/src/mbgl/storage/response.cpp @@ -0,0 +1,12 @@ +#include +#include + +namespace mbgl { + +bool Response::isExpired() const { + const int64_t now = std::chrono::duration_cast( + SystemClock::now().time_since_epoch()).count(); + return expires <= now; +} + +} // namespace mbgl diff --git a/src/mbgl/text/glyph_pbf.cpp b/src/mbgl/text/glyph_pbf.cpp index e37e656d91..f351e66c2a 100644 --- a/src/mbgl/text/glyph_pbf.cpp +++ b/src/mbgl/text/glyph_pbf.cpp @@ -75,6 +75,10 @@ GlyphPBF::GlyphPBF(GlyphStore* store, }); auto requestCallback = [this, store, fontStack, url](const Response &res) { + if (res.stale) { + // Only handle fresh responses. + return; + } req = nullptr; if (res.status != Response::Successful) { diff --git a/test/storage/cache_response.cpp b/test/storage/cache_response.cpp index 0fba2ba5e7..1294513858 100644 --- a/test/storage/cache_response.cpp +++ b/test/storage/cache_response.cpp @@ -14,27 +14,35 @@ TEST_F(Storage, CacheResponse) { DefaultFileSource fs(&cache); const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/cache" }; + Response response; Request* req = fs.request(resource, uv_default_loop(), [&](const Response &res) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Response 1", res.data); EXPECT_LT(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("", res.etag); EXPECT_EQ("", res.message); + response = res; + }); + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); - req = fs.request(resource, uv_default_loop(), [&, res](const Response &res2) { - fs.cancel(req); - EXPECT_EQ(res.status, res2.status); - EXPECT_EQ(res.data, res2.data); - EXPECT_EQ(res.expires, res2.expires); - EXPECT_EQ(res.modified, res2.modified); - EXPECT_EQ(res.etag, res2.etag); - EXPECT_EQ(res.message, res2.message); - - CacheResponse.finish(); - }); + // Now test that we get the same values as in the previous request. If we'd go to the server + // again, we'd get different values. + req = fs.request(resource, uv_default_loop(), [&](const Response &res) { + fs.cancel(req); + EXPECT_EQ(response.status, res.status); + EXPECT_EQ(response.stale, res.stale); + EXPECT_EQ(response.data, res.data); + EXPECT_EQ(response.expires, res.expires); + EXPECT_EQ(response.modified, res.modified); + EXPECT_EQ(response.etag, res.etag); + EXPECT_EQ(response.message, res.message); + + CacheResponse.finish(); }); uv_run(uv_default_loop(), UV_RUN_DEFAULT); diff --git a/test/storage/cache_revalidate.cpp b/test/storage/cache_revalidate.cpp index 8ebedef01b..90633b83ac 100644 --- a/test/storage/cache_revalidate.cpp +++ b/test/storage/cache_revalidate.cpp @@ -5,29 +5,58 @@ #include #include -TEST_F(Storage, CacheRevalidate) { +TEST_F(Storage, CacheRevalidateSame) { SCOPED_TEST(CacheRevalidateSame) - SCOPED_TEST(CacheRevalidateModified) - SCOPED_TEST(CacheRevalidateEtag) using namespace mbgl; SQLiteCache cache(":memory:"); DefaultFileSource fs(&cache); + const Response *reference = nullptr; + const Resource revalidateSame { Resource::Unknown, "http://127.0.0.1:3000/revalidate-same" }; - Request* req = fs.request(revalidateSame, uv_default_loop(), [&](const Response &res) { - fs.cancel(req); + Request* req1 = nullptr; + Request* req2 = nullptr; + req1 = fs.request(revalidateSame, uv_default_loop(), [&](const Response &res) { + // This callback can get triggered multiple times. We only care about the first invocation. + // It will get triggered again when refreshing the req2 (see below). + static bool first = true; + if (!first) { + return; + } + first = false; + + EXPECT_EQ(nullptr, reference); + reference = &res; + EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Response", res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("snowfall", res.etag); EXPECT_EQ("", res.message); - req = fs.request(revalidateSame, uv_default_loop(), [&, res](const Response &res2) { - fs.cancel(req); + req2 = fs.request(revalidateSame, uv_default_loop(), [&, res](const Response &res2) { + // Make sure we get a different object than before, since this request should've been revalidated. + EXPECT_TRUE(reference != &res2); + + if (res2.stale) { + // Discard stale responses, if any. + return; + } + + ASSERT_TRUE(req1); + fs.cancel(req1); + req1 = nullptr; + + ASSERT_TRUE(req2); + fs.cancel(req2); + req2 = nullptr; + EXPECT_EQ(Response::Successful, res2.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Response", res2.data); // We use this to indicate that a 304 reply came back. EXPECT_LT(0, res2.expires); @@ -40,11 +69,37 @@ TEST_F(Storage, CacheRevalidate) { }); }); + uv_run(uv_default_loop(), UV_RUN_DEFAULT); +} + +TEST_F(Storage, CacheRevalidateModified) { + SCOPED_TEST(CacheRevalidateModified) + + using namespace mbgl; + + SQLiteCache cache(":memory:"); + DefaultFileSource fs(&cache); + + const Response *reference = nullptr; + const Resource revalidateModified{ Resource::Unknown, "http://127.0.0.1:3000/revalidate-modified" }; - Request* req2 = fs.request(revalidateModified, uv_default_loop(), [&](const Response &res) { - fs.cancel(req2); + Request* req1 = nullptr; + Request* req2 = nullptr; + req1 = fs.request(revalidateModified, uv_default_loop(), [&](const Response& res) { + // This callback can get triggered multiple times. We only care about the first invocation. + // It will get triggered again when refreshing the req2 (see below). + static bool first = true; + if (!first) { + return; + } + first = false; + + EXPECT_EQ(nullptr, reference); + reference = &res; + EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Response", res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(1420070400, res.modified); @@ -52,8 +107,24 @@ TEST_F(Storage, CacheRevalidate) { EXPECT_EQ("", res.message); req2 = fs.request(revalidateModified, uv_default_loop(), [&, res](const Response &res2) { + // Make sure we get a different object than before, since this request should've been revalidated. + EXPECT_TRUE(reference != &res2); + + if (res2.stale) { + // Discard stale responses, if any. + return; + } + + ASSERT_TRUE(req1); + fs.cancel(req1); + req1 = nullptr; + + ASSERT_TRUE(req2); fs.cancel(req2); + req2 = nullptr; + EXPECT_EQ(Response::Successful, res2.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Response", res2.data); // We use this to indicate that a 304 reply came back. EXPECT_LT(0, res2.expires); @@ -65,19 +136,61 @@ TEST_F(Storage, CacheRevalidate) { }); }); + uv_run(uv_default_loop(), UV_RUN_DEFAULT); +} + +TEST_F(Storage, CacheRevalidateEtag) { + SCOPED_TEST(CacheRevalidateEtag) + + using namespace mbgl; + + SQLiteCache cache(":memory:"); + DefaultFileSource fs(&cache); + + const Response *reference = nullptr; + const Resource revalidateEtag { Resource::Unknown, "http://127.0.0.1:3000/revalidate-etag" }; - Request* req3 = fs.request(revalidateEtag, uv_default_loop(), [&](const Response &res) { - fs.cancel(req3); + Request* req1 = nullptr; + Request* req2 = nullptr; + req1 = fs.request(revalidateEtag, uv_default_loop(), [&](const Response &res) { + // This callback can get triggered multiple times. We only care about the first invocation. + // It will get triggered again when refreshing the req2 (see below). + static bool first = true; + if (!first) { + return; + } + first = false; + + EXPECT_EQ(nullptr, reference); + reference = &res; + EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Response 1", res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); EXPECT_EQ("response-1", res.etag); EXPECT_EQ("", res.message); - req3 = fs.request(revalidateEtag, uv_default_loop(), [&, res](const Response &res2) { - fs.cancel(req3); + req2 = fs.request(revalidateEtag, uv_default_loop(), [&, res](const Response &res2) { + // Make sure we get a different object than before, since this request should've been revalidated. + EXPECT_TRUE(reference != &res2); + + if (res2.stale) { + // Discard stale responses, if any. + return; + } + + ASSERT_TRUE(req1); + fs.cancel(req1); + req1 = nullptr; + + ASSERT_TRUE(req2); + fs.cancel(req2); + req2 = nullptr; + EXPECT_EQ(Response::Successful, res2.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Response 2", res2.data); EXPECT_EQ(0, res2.expires); EXPECT_EQ(0, res2.modified); diff --git a/test/storage/directory_reading.cpp b/test/storage/directory_reading.cpp index 11298dff8b..f0bc7ea6d2 100644 --- a/test/storage/directory_reading.cpp +++ b/test/storage/directory_reading.cpp @@ -19,6 +19,7 @@ TEST_F(Storage, AssetReadDirectory) { [&](const Response &res) { fs.cancel(req); EXPECT_EQ(Response::Error, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ(0ul, res.data.size()); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); diff --git a/test/storage/file_reading.cpp b/test/storage/file_reading.cpp index dc0620b78b..434af703f2 100644 --- a/test/storage/file_reading.cpp +++ b/test/storage/file_reading.cpp @@ -20,6 +20,7 @@ TEST_F(Storage, AssetEmptyFile) { [&](const Response &res) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ(0ul, res.data.size()); EXPECT_EQ(0, res.expires); EXPECT_LT(1420000000, res.modified); @@ -46,6 +47,7 @@ TEST_F(Storage, AssetNonEmptyFile) { uv_default_loop(), [&](const Response &res) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ(16ul, res.data.size()); EXPECT_EQ(0, res.expires); EXPECT_LT(1420000000, res.modified); @@ -73,6 +75,7 @@ TEST_F(Storage, AssetNonExistentFile) { uv_default_loop(), [&](const Response &res) { fs.cancel(req); EXPECT_EQ(Response::Error, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ(0ul, res.data.size()); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); diff --git a/test/storage/http_cancel.cpp b/test/storage/http_cancel.cpp index 2eb599eeaa..5743ccb8f7 100644 --- a/test/storage/http_cancel.cpp +++ b/test/storage/http_cancel.cpp @@ -39,6 +39,7 @@ TEST_F(Storage, HTTPCancelMultiple) { Request* req = fs.request(resource, uv_default_loop(), [&](const Response &res) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Hello World!", res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); diff --git a/test/storage/http_coalescing.cpp b/test/storage/http_coalescing.cpp index 9dfa855501..8558362034 100644 --- a/test/storage/http_coalescing.cpp +++ b/test/storage/http_coalescing.cpp @@ -50,3 +50,108 @@ TEST_F(Storage, HTTPCoalescing) { uv_run(uv_default_loop(), UV_RUN_DEFAULT); } + +TEST_F(Storage, HTTPMultiple) { + SCOPED_TEST(HTTPMultiple) + + using namespace mbgl; + + DefaultFileSource fs(nullptr); + + const Response *reference = nullptr; + + const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test?expires=2147483647" }; + Request* req1 = nullptr; + Request* req2 = nullptr; + req1 = fs.request(resource, uv_default_loop(), [&] (const Response &res) { + EXPECT_EQ(nullptr, reference); + reference = &res; + + // Do not cancel the request right away. + EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ("Hello World!", res.data); + EXPECT_EQ(2147483647, res.expires); + EXPECT_EQ(0, res.modified); + EXPECT_EQ("", res.etag); + EXPECT_EQ("", res.message); + + // Start a second request for the same resource after the first one has been completed. + req2 = fs.request(resource, uv_default_loop(), [&] (const Response &res2) { + // Make sure we get the same object ID as before. + EXPECT_EQ(reference, &res2); + + // Now cancel both requests after both have been notified. + fs.cancel(req1); + fs.cancel(req2); + + EXPECT_EQ(Response::Successful, res2.status); + EXPECT_EQ("Hello World!", res2.data); + EXPECT_EQ(2147483647, res2.expires); + EXPECT_EQ(0, res2.modified); + EXPECT_EQ("", res2.etag); + EXPECT_EQ("", res2.message); + + HTTPMultiple.finish(); + }); + }); + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); +} + +// Tests that we get stale responses from previous requests when requesting the same thing again. +TEST_F(Storage, HTTPStale) { + SCOPED_TEST(HTTPStale) + + using namespace mbgl; + + DefaultFileSource fs(nullptr); + + int updates = 0; + int stale = 0; + + const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" }; + Request* req1 = nullptr; + Request* req2 = nullptr; + req1 = fs.request(resource, uv_default_loop(), [&] (const Response &res) { + // Do not cancel the request right away. + EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ("Hello World!", res.data); + EXPECT_EQ(false, res.stale); + EXPECT_EQ(0, res.expires); + EXPECT_EQ(0, res.modified); + EXPECT_EQ("", res.etag); + EXPECT_EQ("", res.message); + + // Don't start the request twice in case this callback gets fired multiple times. + if (req2) { + return; + } + + updates++; + + // Start a second request for the same resource after the first one has been completed. + req2 = fs.request(resource, uv_default_loop(), [&] (const Response &res2) { + EXPECT_EQ(Response::Successful, res2.status); + EXPECT_EQ("Hello World!", res2.data); + EXPECT_EQ(0, res2.expires); + EXPECT_EQ(0, res2.modified); + EXPECT_EQ("", res2.etag); + EXPECT_EQ("", res2.message); + + if (res2.stale) { + EXPECT_EQ(0, stale); + stale++; + } else { + // Now cancel both requests after both have been notified. + fs.cancel(req1); + fs.cancel(req2); + HTTPStale.finish(); + } + }); + }); + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); + + EXPECT_EQ(1, stale); + EXPECT_EQ(1, updates); +} diff --git a/test/storage/http_error.cpp b/test/storage/http_error.cpp index aebc75405d..50b46a41b8 100644 --- a/test/storage/http_error.cpp +++ b/test/storage/http_error.cpp @@ -37,6 +37,7 @@ TEST_F(Storage, HTTPError) { EXPECT_LT(1, duration) << "Backoff timer didn't wait 1 second"; EXPECT_GT(1.2, duration) << "Backoff timer fired too late"; EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Hello World!", res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); @@ -54,6 +55,7 @@ TEST_F(Storage, HTTPError) { EXPECT_LT(1.5, duration) << "Resource wasn't retried the correct number of times"; EXPECT_GT(1.7, duration) << "Resource wasn't retried the correct number of times"; EXPECT_EQ(Response::Error, res.status); + EXPECT_EQ(false, res.stale); #ifdef MBGL_HTTP_NSURL EXPECT_TRUE(res.message == "The operation couldn’t be completed. (NSURLErrorDomain error -1004.)" || diff --git a/test/storage/http_header_parsing.cpp b/test/storage/http_header_parsing.cpp index 13eab99125..93fdcb6231 100644 --- a/test/storage/http_header_parsing.cpp +++ b/test/storage/http_header_parsing.cpp @@ -20,6 +20,7 @@ TEST_F(Storage, HTTPHeaderParsing) { uv_default_loop(), [&](const Response &res) { fs.cancel(req1); EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Hello World!", res.data); EXPECT_EQ(1420797926, res.expires); EXPECT_EQ(1420794326, res.modified); @@ -35,6 +36,7 @@ TEST_F(Storage, HTTPHeaderParsing) { uv_default_loop(), [&](const Response &res) { fs.cancel(req2); EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Hello World!", res.data); EXPECT_GT(2, std::abs(res.expires - now - 120)) << "Expiration date isn't about 120 seconds in the future"; EXPECT_EQ(0, res.modified); diff --git a/test/storage/http_issue_1369.cpp b/test/storage/http_issue_1369.cpp index 954cb3b3f5..ba4e3d3885 100644 --- a/test/storage/http_issue_1369.cpp +++ b/test/storage/http_issue_1369.cpp @@ -33,6 +33,7 @@ TEST_F(Storage, HTTPIssue1369) { req = fs.request(resource, uv_default_loop(), [&](const Response &res) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Hello World!", res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); diff --git a/test/storage/http_load.cpp b/test/storage/http_load.cpp index fa0468a848..092bf2db5a 100644 --- a/test/storage/http_load.cpp +++ b/test/storage/http_load.cpp @@ -25,6 +25,7 @@ TEST_F(Storage, HTTPLoad) { fs.cancel(reqs[i]); reqs[i] = nullptr; EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ(std::string("Request ") + std::to_string(current), res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); diff --git a/test/storage/http_other_loop.cpp b/test/storage/http_other_loop.cpp index 43a655fba4..cd9fad95be 100644 --- a/test/storage/http_other_loop.cpp +++ b/test/storage/http_other_loop.cpp @@ -16,6 +16,7 @@ TEST_F(Storage, HTTPOtherLoop) { [&](const Response &res) { fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Hello World!", res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); diff --git a/test/storage/http_reading.cpp b/test/storage/http_reading.cpp index 84af943025..c5e89b88e8 100644 --- a/test/storage/http_reading.cpp +++ b/test/storage/http_reading.cpp @@ -23,6 +23,7 @@ TEST_F(Storage, HTTPReading) { fs.cancel(req1); EXPECT_EQ(uv_thread_self(), mainThread); EXPECT_EQ(Response::Successful, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("Hello World!", res.data); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); @@ -36,6 +37,7 @@ TEST_F(Storage, HTTPReading) { fs.cancel(req2); EXPECT_EQ(uv_thread_self(), mainThread); EXPECT_EQ(Response::NotFound, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("", res.message); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); @@ -48,6 +50,7 @@ TEST_F(Storage, HTTPReading) { fs.cancel(req3); EXPECT_EQ(uv_thread_self(), mainThread); EXPECT_EQ(Response::Error, res.status); + EXPECT_EQ(false, res.stale); EXPECT_EQ("HTTP status code 500", res.message); EXPECT_EQ(0, res.expires); EXPECT_EQ(0, res.modified); -- cgit v1.2.1