3 files changed, 96 insertions, 9 deletions

diff --git a/platform/default/default_file_source.cpp b/platform/default/default_file_source.cpp
index 1e855dd9f7..3fdb03e6b4 100644
--- a/platform/default/default_file_source.cpp
+++ b/platform/default/default_file_source.cpp
@@ -126,9 +126,8 @@ public:
             tasks[req] = localFileSource->request(resource, callback);
         } else {
             // Try the offline database
-            Resource revalidation = resource;
-
-            const bool hasPrior = resource.priorEtag || resource.priorModified || resource.priorExpires;
+            const bool hasPrior = resource.priorEtag || resource.priorModified ||
+                                  resource.priorExpires || resource.priorData;
             if (!hasPrior || resource.necessity == Resource::Optional) {
                 auto offlineResponse = offlineDatabase->get(resource);
 
@@ -142,27 +141,34 @@ public:
                 }
 
                 if (offlineResponse) {
-                    revalidation.priorModified = offlineResponse->modified;
-                    revalidation.priorExpires = offlineResponse->expires;
-                    revalidation.priorEtag = offlineResponse->etag;
+                    resource.priorModified = offlineResponse->modified;
+                    resource.priorExpires = offlineResponse->expires;
+                    resource.priorEtag = offlineResponse->etag;
 
                     // Don't return resources the server requested not to show when they're stale.
                     // Even if we can't directly use the response, we may still use it to send a
                     // conditional HTTP request.
                     if (offlineResponse->isUsable()) {
                         callback(*offlineResponse);
+                    } else if (resource.necessity == Resource::Optional) {
+                        // Instead of the data that we got, return a not found error so that
+                        // underlying implementations know about the fact that we couldn't find
+                        // usable cache data.
+                        offlineResponse->error = std::make_unique<Response::Error>(
+                            Response::Error::Reason::NotFound, "Cached resource is unusable");
+                        callback(*offlineResponse);
                     } else {
                         // Since we can't return the data immediately, we'll have to hold on so that
                         // we can return it later in case we get a 304 Not Modified response.
-                        revalidation.priorData = offlineResponse->data;
+                        resource.priorData = offlineResponse->data;
                     }
                 }
             }
 
             // Get from the online file source
             if (resource.necessity == Resource::Required) {
-                tasks[req] = onlineFileSource.request(revalidation, [=] (Response onlineResponse) mutable {
-                    this->offlineDatabase->put(revalidation, onlineResponse);
+                tasks[req] = onlineFileSource.request(resource, [=] (Response onlineResponse) mutable {
+                    this->offlineDatabase->put(resource, onlineResponse);
                     callback(onlineResponse);
                 });
             }
diff --git a/src/mbgl/tile/tile_loader_impl.hpp b/src/mbgl/tile/tile_loader_impl.hpp
index 899cbaf9b0..598ec32c10 100644
--- a/src/mbgl/tile/tile_loader_impl.hpp
+++ b/src/mbgl/tile/tile_loader_impl.hpp
@@ -61,7 +61,10 @@ void TileLoader<T>::loadOptional() {
             // When the optional request could not be satisfied, don't treat it as an error.
             // Instead, we make sure that the next request knows that there has been an optional
             // request before by setting one of the prior* fields.
+            resource.priorModified = res.modified;
             resource.priorExpires = Timestamp{ Seconds::zero() };
+            resource.priorEtag = res.etag;
+            resource.priorData = res.data;
         } else {
             loadedData(res);
         }
diff --git a/test/storage/default_file_source.test.cpp b/test/storage/default_file_source.test.cpp
index b8aebae7e7..b5686b5ffe 100644
--- a/test/storage/default_file_source.test.cpp
+++ b/test/storage/default_file_source.test.cpp
@@ -571,3 +571,81 @@ TEST(DefaultFileSource, TEST_REQUIRES_SERVER(SetResourceTransform)) {
 
     loop.run();
 }
+
+// Test that a stale cache file that has must-revalidate set will trigger a response.
+TEST(DefaultFileSource, TEST_REQUIRES_SERVER(RespondToStaleMustRevalidate)) {
+    util::RunLoop loop;
+    DefaultFileSource fs(":memory:", ".");
+
+    Resource resource { Resource::Unknown, "http://127.0.0.1:3000/revalidate-same" };
+    resource.necessity = Resource::Necessity::Optional;
+
+    // using namespace std::chrono_literals;
+
+    // Put an existing value in the cache that has expired, and has must-revalidate set.
+    Response response;
+    response.data = std::make_shared<std::string>("Cached value");
+    response.modified = Timestamp(Seconds(1417392000)); // December 1, 2014
+    response.expires = Timestamp(Seconds(1417392000));
+    response.mustRevalidate = true;
+    response.etag.emplace("snowfall");
+    fs.put(resource, response);
+
+    std::unique_ptr<AsyncRequest> req;
+    req = fs.request(resource, [&](Response res) {
+        req.reset();
+        ASSERT_TRUE(res.error.get());
+        EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason);
+        EXPECT_EQ("Cached resource is unusable", res.error->message);
+        EXPECT_FALSE(res.notModified);
+        ASSERT_TRUE(res.data.get());
+        EXPECT_EQ("Cached value", *res.data);
+        ASSERT_TRUE(res.expires);
+        EXPECT_EQ(Timestamp{ Seconds(1417392000) }, *res.expires);
+        EXPECT_TRUE(res.mustRevalidate);
+        ASSERT_TRUE(res.modified);
+        EXPECT_EQ(Timestamp{ Seconds(1417392000) }, *res.modified);
+        ASSERT_TRUE(res.etag);
+        EXPECT_EQ("snowfall", *res.etag);
+
+        resource.priorEtag = res.etag;
+        resource.priorModified = res.modified;
+        resource.priorExpires = res.expires;
+        resource.priorData = res.data;
+
+        loop.stop();
+    });
+
+    loop.run();
+
+    // Now run this request again, with the data we gathered from the previous stale/unusable
+    // request. We're replacing the data so that we can check that the DefaultFileSource doesn't
+    // attempt another database access if we already have the value.
+    resource.necessity = Resource::Necessity::Required;
+    resource.priorData = std::make_shared<std::string>("Prior value");
+
+    req = fs.request(resource, [&](Response res) {
+        req.reset();
+        ASSERT_EQ(nullptr, res.error.get());
+        // Since the data was found in the cache, we're doing a revalidation request. Yet, since
+        // this request hasn't returned data before, we're setting notModified to false in the
+        // OnlineFileSource to ensure that requestors know that this is the first time they're
+        // seeing this data.
+        EXPECT_FALSE(res.notModified);
+        ASSERT_TRUE(res.data.get());
+        // Ensure that it's the value that we manually inserted into the cache rather than the value
+        // the server returns, since we should be executing a revalidation request which doesn't
+        // return new data, only a 304 Not Modified response.
+        EXPECT_EQ("Prior value", *res.data);
+        ASSERT_TRUE(res.expires);
+        EXPECT_LE(util::now(), *res.expires);
+        EXPECT_TRUE(res.mustRevalidate);
+        ASSERT_TRUE(res.modified);
+        EXPECT_EQ(Timestamp{ Seconds(1417392000) }, *res.modified);
+        ASSERT_TRUE(res.etag);
+        EXPECT_EQ("snowfall", *res.etag);
+        loop.stop();
+    });
+
+    loop.run();
+}