[core] Add an explicit "not modified" indicator to Response

This commit is contained in:
John Firebaugh
2016-01-13 16:47:36 -08:00
parent 2ee1fe4178
commit 2aafdbc1b8
7 changed files with 43 additions and 40 deletions

View File

@@ -24,6 +24,9 @@ public:
// Stale responses are fetched from cache and are expired.
bool stale = false;
// This is set to true for 304 Not Modified responses.
bool notModified = false;
// The actual data of the response. This is guaranteed to never be empty.
std::shared_ptr<const std::string> data;

View File

@@ -206,17 +206,20 @@ void HTTPAndroidRequest::onResponse(int code, std::string message, std::string e
if (code == 200) {
// Nothing to do; this is what we want
} else if (code == 304) {
response->notModified = true;
if (existingResponse) {
if (existingResponse->error) {
response->error = std::make_unique<Error>(*existingResponse->error);
if (response->expires == Seconds::zero()) {
response->expires = existingResponse->expires;
}
if (response->modified == Seconds::zero()) {
response->modified = existingResponse->modified;
}
if (response->etag.empty()) {
response->etag = existingResponse->etag;
}
response->data = existingResponse->data;
response->modified = existingResponse->modified;
// We're not updating `expired`, it was probably set during the request.
response->etag = existingResponse->etag;
} else {
// This is an unsolicited 304 response and should only happen on malfunctioning
// HTTP servers. It likely doesn't include any data, but we don't have much options.
}
} else if (code == 404) {
response->error = std::make_unique<Error>(Error::Reason::NotFound, "HTTP status code 404");

View File

@@ -226,18 +226,20 @@ void HTTPNSURLRequest::handleResult(NSData *data, NSURLResponse *res, NSError *e
if (responseCode == 200) {
// Nothing to do; this is what we want.
} else if (responseCode == 304) {
response->notModified = true;
if (existingResponse) {
// We're going to copy over the existing response's data.
if (existingResponse->error) {
response->error = std::make_unique<Error>(*existingResponse->error);
if (response->expires == Seconds::zero()) {
response->expires = existingResponse->expires;
}
if (response->modified == Seconds::zero()) {
response->modified = existingResponse->modified;
}
if (response->etag.empty()) {
response->etag = existingResponse->etag;
}
response->data = existingResponse->data;
response->modified = existingResponse->modified;
// We're not updating `expired`, it was probably set during the request.
response->etag = existingResponse->etag;
} else {
// This is an unsolicited 304 response and should only happen on malfunctioning
// HTTP servers. It likely doesn't include any data, but we don't have much options.
}
} else if (responseCode == 404) {
response->error =

View File

@@ -529,18 +529,20 @@ void HTTPCURLRequest::handleResult(CURLcode code) {
if (responseCode == 200) {
// Nothing to do; this is what we want.
} else if (responseCode == 304) {
response->notModified = true;
if (existingResponse) {
// We're going to copy over the existing response's data.
if (existingResponse->error) {
response->error = std::make_unique<Error>(*existingResponse->error);
if (response->expires == Seconds::zero()) {
response->expires = existingResponse->expires;
}
if (response->modified == Seconds::zero()) {
response->modified = existingResponse->modified;
}
if (response->etag.empty()) {
response->etag = existingResponse->etag;
}
response->data = existingResponse->data;
response->modified = existingResponse->modified;
// We're not updating `expired`, it was probably set during the request.
response->etag = existingResponse->etag;
} else {
// This is an unsolicited 304 response and should only happen on malfunctioning
// HTTP servers. It likely doesn't include any data, but we don't have much options.
}
} else if (responseCode == 404) {
response->error =

View File

@@ -252,11 +252,7 @@ void OnlineFileRequestImpl::scheduleRealRequest(OnlineFileSource::Impl& impl, bo
(!response_->error || (response_->error->reason == Response::Error::Reason::NotFound))) {
// Store response in database. Make sure we only refresh the expires column if the data
// didn't change.
SQLiteCache::Hint hint = SQLiteCache::Hint::Full;
if (response && response_->data == response->data) {
hint = SQLiteCache::Hint::Refresh;
}
impl.cache->put(resource, *response_, hint);
impl.cache->put(resource, *response_, response_->notModified ? SQLiteCache::Hint::Refresh : SQLiteCache::Hint::Full);
}
response = response_;

View File

@@ -10,6 +10,7 @@ Response::Response(const Response& res) {
Response& Response::operator=(const Response& res) {
error = res.error ? std::make_unique<Error>(*res.error) : nullptr;
stale = res.stale;
notModified = res.notModified;
data = res.data;
modified = res.modified;
expires = res.expires;

View File

@@ -48,9 +48,7 @@ TEST_F(Storage, CacheRevalidateSame) {
EXPECT_EQ(nullptr, res2.error);
EXPECT_EQ(false, res2.stale);
ASSERT_TRUE(res2.data.get());
EXPECT_EQ("Response", *res2.data);
// We use this to indicate that a 304 reply came back.
EXPECT_TRUE(res2.notModified);
EXPECT_LT(Seconds::zero(), res2.expires);
EXPECT_EQ(Seconds::zero(), res2.modified);
// We're not sending the ETag in the 304 reply, but it should still be there.
@@ -108,9 +106,7 @@ TEST_F(Storage, CacheRevalidateModified) {
EXPECT_EQ(nullptr, res2.error);
EXPECT_EQ(false, res2.stale);
ASSERT_TRUE(res2.data.get());
EXPECT_EQ("Response", *res2.data);
// We use this to indicate that a 304 reply came back.
EXPECT_TRUE(res2.notModified);
EXPECT_LT(Seconds::zero(), res2.expires);
EXPECT_EQ(1420070400, res2.modified.count());
EXPECT_EQ("", res2.etag);