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

4 files changed, 37 insertions, 34 deletions

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_;