From 8ee222c45e31413b03d1c83006cf4eadc0e6d3a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Fri, 13 May 2016 18:24:37 +0200 Subject: [PATCH] [core] allow creating optional requests Introduces "optional" requests. These should be fulfilled by the FileSource only there's a low-cost/easy way to obtain the data (e.g. from a local cache). If the data for an optional request cannot be found, it *must* return a Response object with a NotFound error. Traditional "required" requests still work the same way, with one change: If you set any prior* field in the Resource (i.e. priorModified, priorEtag, or priorExpires), the DefaultFileSource assumes that you already have the cache value and will not consult the cache before performing the request. If a prior cache lookup didn't turn up any data, and you therefore don't have an Etag or Modified value, you can still skip the cache by setting priorExpires. This will of course always result in a non-conditional HTTP request. --- platform/default/default_file_source.cpp | 36 +++- test/storage/default_file_source.cpp | 252 +++++++++++++++++++++++ 2 files changed, 277 insertions(+), 11 deletions(-) diff --git a/platform/default/default_file_source.cpp b/platform/default/default_file_source.cpp index afcb63e94..9d369210f 100644 --- a/platform/default/default_file_source.cpp +++ b/platform/default/default_file_source.cpp @@ -82,21 +82,35 @@ public: } void request(AsyncRequest* req, Resource resource, Callback callback) { - auto offlineResponse = offlineDatabase.get(resource); - Resource revalidation = resource; - if (offlineResponse) { - revalidation.priorModified = offlineResponse->modified; - revalidation.priorExpires = offlineResponse->expires; - revalidation.priorEtag = offlineResponse->etag; - callback(*offlineResponse); + const bool hasPrior = resource.priorEtag || resource.priorModified || resource.priorExpires; + if (!hasPrior || resource.necessity == Resource::Optional) { + auto offlineResponse = offlineDatabase.get(resource); + + if (resource.necessity == Resource::Optional && !offlineResponse) { + // Ensure there's always a response that we can send, so the caller knows that + // there's no optional data available in the cache. + offlineResponse.emplace(); + offlineResponse->noContent = true; + offlineResponse->error = std::make_unique( + Response::Error::Reason::NotFound, "Not found in offline database"); + } + + if (offlineResponse) { + revalidation.priorModified = offlineResponse->modified; + revalidation.priorExpires = offlineResponse->expires; + revalidation.priorEtag = offlineResponse->etag; + callback(*offlineResponse); + } } - tasks[req] = onlineFileSource.request(revalidation, [=] (Response onlineResponse) { - this->offlineDatabase.put(revalidation, onlineResponse); - callback(onlineResponse); - }); + if (resource.necessity == Resource::Required) { + tasks[req] = onlineFileSource.request(revalidation, [=] (Response onlineResponse) { + this->offlineDatabase.put(revalidation, onlineResponse); + callback(onlineResponse); + }); + } } void cancel(AsyncRequest* req) { diff --git a/test/storage/default_file_source.cpp b/test/storage/default_file_source.cpp index 09b10007e..f4c23c4c7 100644 --- a/test/storage/default_file_source.cpp +++ b/test/storage/default_file_source.cpp @@ -208,3 +208,255 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(HTTPIssue1369)) { loop.run(); } + +TEST(DefaultFileSource, OptionalNonExpired) { + util::RunLoop loop; + DefaultFileSource fs(":memory:", "."); + + const Resource optionalResource { Resource::Unknown, "http://127.0.0.1:3000/test", {}, Resource::Optional }; + + using namespace std::chrono_literals; + + Response response; + response.data = std::make_shared("Cached value"); + response.expires = util::now() + 1h; + fs.put(optionalResource, response); + + std::unique_ptr req; + req = fs.request(optionalResource, [&](Response res) { + req.reset(); + EXPECT_EQ(nullptr, res.error); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Cached value", *res.data); + ASSERT_TRUE(bool(res.expires)); + EXPECT_EQ(*response.expires, *res.expires); + EXPECT_FALSE(bool(res.modified)); + EXPECT_FALSE(bool(res.etag)); + loop.stop(); + }); + + loop.run(); +} + +TEST(DefaultFileSource, OptionalExpired) { + util::RunLoop loop; + DefaultFileSource fs(":memory:", "."); + + const Resource optionalResource { Resource::Unknown, "http://127.0.0.1:3000/test", {}, Resource::Optional }; + + using namespace std::chrono_literals; + + Response response; + response.data = std::make_shared("Cached value"); + response.expires = util::now() - 1h; + fs.put(optionalResource, response); + + std::unique_ptr req; + req = fs.request(optionalResource, [&](Response res) { + req.reset(); + EXPECT_EQ(nullptr, res.error); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Cached value", *res.data); + ASSERT_TRUE(bool(res.expires)); + EXPECT_EQ(*response.expires, *res.expires); + EXPECT_FALSE(bool(res.modified)); + EXPECT_FALSE(bool(res.etag)); + loop.stop(); + }); + + loop.run(); +} + +TEST(DefaultFileSource, OptionalNotFound) { + util::RunLoop loop; + DefaultFileSource fs(":memory:", "."); + + const Resource optionalResource { Resource::Unknown, "http://127.0.0.1:3000/test", {}, Resource::Optional }; + + using namespace std::chrono_literals; + + std::unique_ptr req; + req = fs.request(optionalResource, [&](Response res) { + req.reset(); + ASSERT_TRUE(res.error.get()); + EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason); + EXPECT_EQ("Not found in offline database", res.error->message); + EXPECT_FALSE(res.data); + EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(bool(res.modified)); + EXPECT_FALSE(bool(res.etag)); + loop.stop(); + }); + + loop.run(); +} + +// Test that we can make a request with etag data that doesn't first try to load +// from cache like a regular request +TEST(DefaultFileSource, TEST_REQUIRES_SERVER(NoCacheRefreshEtagNotModified)) { + util::RunLoop loop; + DefaultFileSource fs(":memory:", "."); + + Resource resource { Resource::Unknown, "http://127.0.0.1:3000/revalidate-same" }; + resource.priorEtag.emplace("snowfall"); + + using namespace std::chrono_literals; + + // Put a fake value into the cache to make sure we're not retrieving anything from the cache. + Response response; + response.data = std::make_shared("Cached value"); + response.expires = util::now() + 1h; + fs.put(resource, response); + + std::unique_ptr req; + req = fs.request(resource, [&](Response res) { + req.reset(); + EXPECT_EQ(nullptr, res.error); + EXPECT_TRUE(res.notModified); + EXPECT_FALSE(res.data.get()); + ASSERT_TRUE(bool(res.expires)); + EXPECT_LT(util::now(), *res.expires); + EXPECT_FALSE(bool(res.modified)); + ASSERT_TRUE(bool(res.etag)); + EXPECT_EQ("snowfall", *res.etag); + loop.stop(); + }); + + loop.run(); +} + +// Test that we can make a request with etag data that doesn't first try to load +// from cache like a regular request +TEST(DefaultFileSource, TEST_REQUIRES_SERVER(NoCacheRefreshEtagModified)) { + util::RunLoop loop; + DefaultFileSource fs(":memory:", "."); + + Resource resource { Resource::Unknown, "http://127.0.0.1:3000/revalidate-same" }; + resource.priorEtag.emplace("sunshine"); + + using namespace std::chrono_literals; + + // Put a fake value into the cache to make sure we're not retrieving anything from the cache. + Response response; + response.data = std::make_shared("Cached value"); + response.expires = util::now() + 1h; + fs.put(resource, response); + + std::unique_ptr req; + req = fs.request(resource, [&](Response res) { + req.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_FALSE(bool(res.modified)); + ASSERT_TRUE(bool(res.etag)); + EXPECT_EQ("snowfall", *res.etag); + loop.stop(); + }); + + loop.run(); +} + +// Test that we can make a request that doesn't first try to load +// from cache like a regular request. +TEST(DefaultFileSource, TEST_REQUIRES_SERVER(NoCacheFull)) { + util::RunLoop loop; + DefaultFileSource fs(":memory:", "."); + + Resource resource { Resource::Unknown, "http://127.0.0.1:3000/revalidate-same" }; + // Setting any prior field results in skipping the cache. + resource.priorExpires.emplace(Seconds(0)); + + using namespace std::chrono_literals; + + // Put a fake value into the cache to make sure we're not retrieving anything from the cache. + Response response; + response.data = std::make_shared("Cached value"); + response.expires = util::now() + 1h; + fs.put(resource, response); + + std::unique_ptr req; + req = fs.request(resource, [&](Response res) { + req.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_FALSE(bool(res.modified)); + ASSERT_TRUE(bool(res.etag)); + EXPECT_EQ("snowfall", *res.etag); + loop.stop(); + }); + + loop.run(); +} + +// Test that we can make a request with a Modified field that doesn't first try to load +// from cache like a regular request +TEST(DefaultFileSource, TEST_REQUIRES_SERVER(NoCacheRefreshModifiedNotModified)) { + util::RunLoop loop; + DefaultFileSource fs(":memory:", "."); + + Resource resource { Resource::Unknown, "http://127.0.0.1:3000/revalidate-modified" }; + resource.priorModified.emplace(Seconds(1420070400)); // January 1, 2015 + + using namespace std::chrono_literals; + + // Put a fake value into the cache to make sure we're not retrieving anything from the cache. + Response response; + response.data = std::make_shared("Cached value"); + response.expires = util::now() + 1h; + fs.put(resource, response); + + std::unique_ptr req; + req = fs.request(resource, [&](Response res) { + req.reset(); + EXPECT_EQ(nullptr, res.error); + EXPECT_TRUE(res.notModified); + EXPECT_FALSE(res.data.get()); + ASSERT_TRUE(bool(res.expires)); + EXPECT_LT(util::now(), *res.expires); + ASSERT_TRUE(bool(res.modified)); + EXPECT_EQ(Timestamp{ Seconds(1420070400) }, *res.modified); + EXPECT_FALSE(bool(res.etag)); + loop.stop(); + }); + + loop.run(); +} + +// Test that we can make a request with a Modified field that doesn't first try to load +// from cache like a regular request +TEST(DefaultFileSource, TEST_REQUIRES_SERVER(NoCacheRefreshModifiedModified)) { + util::RunLoop loop; + DefaultFileSource fs(":memory:", "."); + + Resource resource { Resource::Unknown, "http://127.0.0.1:3000/revalidate-modified" }; + resource.priorModified.emplace(Seconds(1417392000)); // December 1, 2014 + + using namespace std::chrono_literals; + + // Put a fake value into the cache to make sure we're not retrieving anything from the cache. + Response response; + response.data = std::make_shared("Cached value"); + response.expires = util::now() + 1h; + fs.put(resource, response); + + std::unique_ptr req; + req = fs.request(resource, [&](Response res) { + req.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_EQ(Timestamp{ Seconds(1420070400) }, *res.modified); + EXPECT_FALSE(res.etag); + loop.stop(); + }); + + loop.run(); +}