From 0cf450e3e529423737c6b4aa196b271442530345 Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Wed, 10 Feb 2016 13:22:09 -0800
Subject: [core] Retry errors encountered during offline downloads

---
 test/fixtures/stub_file_source.cpp | 11 ++++++-----
 test/fixtures/stub_file_source.hpp | 18 ++++++++++--------
 test/storage/offline_download.cpp  | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 13 deletions(-)

(limited to 'test')

diff --git a/test/fixtures/stub_file_source.cpp b/test/fixtures/stub_file_source.cpp
index 9eb37b7928..b41eded084 100644
--- a/test/fixtures/stub_file_source.cpp
+++ b/test/fixtures/stub_file_source.cpp
@@ -19,11 +19,12 @@ public:
 
 StubFileSource::StubFileSource() {
     timer.start(10ms, 10ms, [this] {
-        // Explicit move to avoid iterator invalidation if ~StubFileRequest gets called within the loop.
-        auto pending_ = std::move(pending);
+        // Explicit copy to avoid iterator invalidation if ~StubFileRequest gets called within the loop.
+        auto pending_ = pending;
         for (auto& pair : pending_) {
-            if (pair.second.first) {
-                pair.second.second(*pair.second.first);
+            optional<Response> res = std::get<1>(pair.second)(std::get<0>(pair.second));
+            if (res) {
+                std::get<2>(pair.second)(*res);
             }
         }
     });
@@ -33,7 +34,7 @@ StubFileSource::~StubFileSource() = default;
 
 std::unique_ptr<FileRequest> StubFileSource::request(const Resource& resource, Callback callback) {
     auto req = std::make_unique<StubFileRequest>(*this);
-    pending.emplace(req.get(), std::make_pair(response(resource), callback));
+    pending.emplace(req.get(), std::make_tuple(resource, response, callback));
     return std::move(req);
 }
 
diff --git a/test/fixtures/stub_file_source.hpp b/test/fixtures/stub_file_source.hpp
index 344dff6073..dbb584fdcc 100644
--- a/test/fixtures/stub_file_source.hpp
+++ b/test/fixtures/stub_file_source.hpp
@@ -15,18 +15,20 @@ public:
 
     std::unique_ptr<FileRequest> request(const Resource&, Callback) override;
 
+    using ResponseFunction = std::function<optional<Response> (const Resource&)>;
+
     // You can set the response callback on a global level by assigning this callback:
-    std::function<optional<Response> (const Resource&)> response = [this] (const Resource& resource) {
+    ResponseFunction response = [this] (const Resource& resource) {
         return defaultResponse(resource);
     };
 
     // Or set per-kind responses by setting these callbacks:
-    std::function<optional<Response> (const Resource&)> styleResponse;
-    std::function<optional<Response> (const Resource&)> sourceResponse;
-    std::function<optional<Response> (const Resource&)> tileResponse;
-    std::function<optional<Response> (const Resource&)> glyphsResponse;
-    std::function<optional<Response> (const Resource&)> spriteJSONResponse;
-    std::function<optional<Response> (const Resource&)> spriteImageResponse;
+    ResponseFunction styleResponse;
+    ResponseFunction sourceResponse;
+    ResponseFunction tileResponse;
+    ResponseFunction glyphsResponse;
+    ResponseFunction spriteJSONResponse;
+    ResponseFunction spriteImageResponse;
 
 private:
     friend class StubFileRequest;
@@ -34,7 +36,7 @@ private:
     // The default behavior is to throw if no per-kind callback has been set.
     optional<Response> defaultResponse(const Resource&);
 
-    std::unordered_map<FileRequest*, std::pair<optional<Response>, Callback>> pending;
+    std::unordered_map<FileRequest*, std::tuple<Resource, ResponseFunction, Callback>> pending;
     util::Timer timer;
 };
 
diff --git a/test/storage/offline_download.cpp b/test/storage/offline_download.cpp
index d89d4a035c..6de14d37b2 100644
--- a/test/storage/offline_download.cpp
+++ b/test/storage/offline_download.cpp
@@ -300,3 +300,35 @@ TEST(OfflineDownload, RequestError) {
 
     test.loop.run();
 }
+
+TEST(OfflineDownload, RequestErrorsAreRetried) {
+    OfflineTest test;
+    OfflineDownload download(
+        1,
+        OfflineTilePyramidRegionDefinition("http://127.0.0.1:3000/offline/style.json", LatLngBounds::world(), 0.0, 0.0, 1.0),
+        test.db, test.fileSource);
+
+    test.fileSource.styleResponse = [&] (const Resource&) {
+        test.fileSource.styleResponse = [&] (const Resource&) {
+            return test.response("offline/empty.style.json");
+        };
+
+        Response response;
+        response.error = std::make_unique<Response::Error>(Response::Error::Reason::Connection, "connection error");
+        return response;
+    };
+
+    auto observer = std::make_unique<MockObserver>();
+
+    observer->statusChangedFn = [&] (OfflineRegionStatus status) {
+        if (status.complete()) {
+            EXPECT_EQ(1, status.completedResourceCount);
+            test.loop.stop();
+        }
+    };
+
+    download.setObserver(std::move(observer));
+    download.setState(OfflineRegionDownloadState::Active);
+
+    test.loop.run();
+}
-- 
cgit v1.2.1