summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--include/mbgl/storage/response.hpp3
-rw-r--r--platform/android/src/http_request_android.cpp21
-rw-r--r--platform/darwin/http_request_nsurl.mm22
-rw-r--r--platform/default/http_request_curl.cpp22
-rw-r--r--platform/default/online_file_source.cpp6
-rw-r--r--src/mbgl/storage/response.cpp1
-rw-r--r--test/storage/cache_revalidate.cpp8
7 files changed, 43 insertions, 40 deletions
diff --git a/include/mbgl/storage/response.hpp b/include/mbgl/storage/response.hpp
index 852110cb2a..532a91da97 100644
--- a/include/mbgl/storage/response.hpp
+++ b/include/mbgl/storage/response.hpp
@@ -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;
diff --git a/platform/android/src/http_request_android.cpp b/platform/android/src/http_request_android.cpp
index a6e00ff2b5..3458040083 100644
--- a/platform/android/src/http_request_android.cpp
+++ b/platform/android/src/http_request_android.cpp
@@ -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");
diff --git a/platform/darwin/http_request_nsurl.mm b/platform/darwin/http_request_nsurl.mm
index 11c8100e60..a43210cbc0 100644
--- a/platform/darwin/http_request_nsurl.mm
+++ b/platform/darwin/http_request_nsurl.mm
@@ -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 =
diff --git a/platform/default/http_request_curl.cpp b/platform/default/http_request_curl.cpp
index e1ea931fbe..7f95d187cb 100644
--- a/platform/default/http_request_curl.cpp
+++ b/platform/default/http_request_curl.cpp
@@ -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 =
diff --git a/platform/default/online_file_source.cpp b/platform/default/online_file_source.cpp
index 634a6710a7..657871b907 100644
--- a/platform/default/online_file_source.cpp
+++ b/platform/default/online_file_source.cpp
@@ -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_;
diff --git a/src/mbgl/storage/response.cpp b/src/mbgl/storage/response.cpp
index 4b1125c110..b42fb211c8 100644
--- a/src/mbgl/storage/response.cpp
+++ b/src/mbgl/storage/response.cpp
@@ -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;
diff --git a/test/storage/cache_revalidate.cpp b/test/storage/cache_revalidate.cpp
index ffc622f37c..fc65986a17 100644
--- a/test/storage/cache_revalidate.cpp
+++ b/test/storage/cache_revalidate.cpp
@@ -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);
View Lorry Controller Status