[tests] Simplify MockFileSource

5 files changed, 59 insertions, 157 deletions

diff --git a/test/fixtures/mock_file_source.cpp b/test/fixtures/mock_file_source.cpp
index b0d7018873..22c906c173 100644
--- a/test/fixtures/mock_file_source.cpp
+++ b/test/fixtures/mock_file_source.cpp
@@ -1,161 +1,69 @@
-#include "../fixtures/util.hpp"
 #include "mock_file_source.hpp"
-
 #include <mbgl/util/io.hpp>
-#include <mbgl/util/thread.hpp>
-#include <mbgl/util/timer.hpp>
-
-#include <algorithm>
-#include <unordered_map>
 
 namespace mbgl {
 
 class MockFileRequest : public FileRequest {
 public:
-    MockFileRequest(const Resource& resource_,
-                    MockFileSource& fileSource_)
-        : resource(resource_),
-          fileSource(fileSource_) {
+    MockFileRequest(MockFileSource& fileSource_)
+        : fileSource(fileSource_) {
     }
 
     ~MockFileRequest() {
-        fileSource.cancel(this);
+        fileSource.pending.erase(this);
     }
 
-    Resource resource;
     MockFileSource& fileSource;
-
-    std::unique_ptr<WorkRequest> workRequest;
 };
 
-class MockFileSource::Impl {
-public:
-    Impl(Type type, const std::string& match)
-        : type_(type), match_(match) {
-        timer_.start(std::chrono::milliseconds(1000), std::chrono::milliseconds(1000), [this] { dispatchPendingRequests(); });
-        timer_.unref();
-    }
-
-    ~Impl() {
-        timer_.stop();
-    }
-
-    void setOnRequestDelayedCallback(std::function<void(void)> callback) {
-        requestEnqueuedCallback_ = callback;
-    }
-
-    void handleRequest(FileRequest*, Resource, Callback);
-    void cancelRequest(FileRequest*);
-
-private:
-    void replyWithSuccess(Resource, Callback) const;
-    void replyWithSuccessWithDelay(FileRequest*, Resource, Callback);
-    void replyWithFailure(Resource, Callback) const;
-    void replyWithCorruptedData(Resource, Callback) const;
-
-    void dispatchPendingRequests();
-
-    Type type_;
-    std::string match_;
-    std::unordered_map<FileRequest*, std::pair<Resource, Callback>> pendingRequests_;
-    util::Timer timer_;
-
-    std::function<void(void)> requestEnqueuedCallback_;
-};
-
-void MockFileSource::Impl::replyWithSuccess(Resource resource, Callback callback) const {
-    Response res;
-
-    try {
-        res.data = std::make_shared<const std::string>(util::read_file(resource.url));
-    } catch (const std::exception& err) {
-        res.error = std::make_unique<Response::Error>(Response::Error::Reason::Other, err.what());
-    }
-
-    callback(res);
+MockFileSource::MockFileSource(Type type_, const std::string& match_)
+    : type(type_), match(match_) {
+    timer.unref();
+    timer.start(std::chrono::milliseconds(10), std::chrono::milliseconds(10), [this] {
+        // Explicit move to avoid iterator invalidation if ~MockFileRequest gets called within the loop.
+        auto pending_ = std::move(pending);
+        for (auto& pair : pending_) {
+            respond(pair.second.first, pair.second.second);
+        }
+    });
 }
 
-void MockFileSource::Impl::replyWithSuccessWithDelay(FileRequest* req, Resource resource, Callback callback) {
-    if (resource.url.find(match_) == std::string::npos) {
-        replyWithSuccess(resource, callback);
-        return;
-    }
-
-    pendingRequests_.emplace(req, std::make_pair(resource, callback));
-    requestEnqueuedCallback_();
+MockFileSource::~MockFileSource() {
+    timer.stop();
 }
 
-void MockFileSource::Impl::replyWithFailure(Resource resource, Callback callback) const {
-    if (resource.url.find(match_) == std::string::npos) {
-        replyWithSuccess(resource, callback);
-        return;
+void MockFileSource::respond(Resource resource, Callback callback) const {
+    if (type == Type::Success || resource.url.find(match) == std::string::npos) {
+        Response res;
+        try {
+            res.data = std::make_shared<const std::string>(util::read_file(resource.url));
+        } catch (const std::exception& err) {
+            res.error = std::make_unique<Response::Error>(Response::Error::Reason::Other, err.what());
+        }
+        callback(res);
+    } else if (type == Type::RequestFail) {
+        Response res;
+        res.error = std::make_unique<Response::Error>(Response::Error::Reason::Other, "Failed by the test case");
+        callback(res);
+    } else if (type == Type::RequestWithCorruptedData) {
+        Response res;
+        auto data = std::make_shared<std::string>(util::read_file(resource.url));
+        data->insert(0, "CORRUPTED");
+        res.data = std::move(data);
+        callback(res);
     }
-
-    Response res;
-    res.error = std::make_unique<Response::Error>(Response::Error::Reason::Other, "Failed by the test case");
-    callback(res);
 }
 
-void MockFileSource::Impl::replyWithCorruptedData(Resource resource, Callback callback) const {
-    if (resource.url.find(match_) == std::string::npos) {
-        replyWithSuccess(resource, callback);
-        return;
-    }
-
-    Response res;
-    auto data = std::make_shared<std::string>(util::read_file(resource.url));
-    data->insert(0, "CORRUPTED");
-    res.data = std::move(data);
-    callback(res);
-}
-
-void MockFileSource::Impl::handleRequest(FileRequest* req, Resource resource, Callback callback) {
-    switch (type_) {
-    case Type::Success:
-        replyWithSuccess(resource, callback);
-        break;
-    case Type::SuccessWithDelay:
-        replyWithSuccessWithDelay(req, resource, callback);
-        break;
-    case Type::RequestFail:
-        replyWithFailure(resource, callback);
-        break;
-    case Type::RequestWithCorruptedData:
-        replyWithCorruptedData(resource, callback);
-        break;
-    default:
-        EXPECT_TRUE(false) << "Should never be reached.";
-    }
-}
+std::unique_ptr<FileRequest> MockFileSource::request(const Resource& resource, Callback callback) {
+    auto req = std::make_unique<MockFileRequest>(*this);
 
-void MockFileSource::Impl::cancelRequest(FileRequest* req) {
-    pendingRequests_.erase(req);
-}
+    pending.emplace(req.get(), std::make_pair(resource, callback));
 
-void MockFileSource::Impl::dispatchPendingRequests() {
-    for (auto& pair : pendingRequests_) {
-        replyWithSuccess(pair.second.first, pair.second.second);
+    if (requestEnqueuedCallback && resource.url.find(match) != std::string::npos) {
+        requestEnqueuedCallback();
     }
 
-    pendingRequests_.clear();
-}
-
-MockFileSource::MockFileSource(Type type, const std::string& match)
-    : thread_(std::make_unique<util::Thread<Impl>>(util::ThreadContext{"FileSource", util::ThreadType::Unknown, util::ThreadPriority::Low}, type, match)) {
-}
-
-void MockFileSource::setOnRequestDelayedCallback(std::function<void(void)> callback) {
-    thread_->invokeSync(&Impl::setOnRequestDelayedCallback, callback);
-}
-
-std::unique_ptr<FileRequest> MockFileSource::request(const Resource& res, Callback callback) {
-    auto req = std::make_unique<MockFileRequest>(res, *this);
-    req->workRequest = thread_->invokeWithCallback(&Impl::handleRequest, callback, req.get(), res);
     return std::move(req);
 }
 
-void MockFileSource::cancel(FileRequest* req) {
-    thread_->invoke(&Impl::cancelRequest, req);
-}
-
 } // namespace mbgl
diff --git a/test/fixtures/mock_file_source.hpp b/test/fixtures/mock_file_source.hpp
index f41f808e36..245e0da0eb 100644
--- a/test/fixtures/mock_file_source.hpp
+++ b/test/fixtures/mock_file_source.hpp
@@ -2,10 +2,10 @@
 #define TEST_RESOURCES_MOCK_FILE_SOURCE
 
 #include <mbgl/storage/file_source.hpp>
-#include <mbgl/util/thread.hpp>
+#include <mbgl/util/timer.hpp>
 
 #include <string>
-#include <memory>
+#include <unordered_map>
 
 namespace mbgl {
 
@@ -16,12 +16,6 @@ public:
     // Success:
     //     Will reply to every request correctly with valid data.
     //
-    // SuccessWithDelay:
-    //     Will reply to every request correctly with valid data,
-    //     but the ones that contains the "match" string on the
-    //     URL will be answered after a delay. This can be useful
-    //     for testing request cancellation.
-    //
     // RequestFail:
     //     Will reply with an error to requests that contains
     //     the "match" string on the URL.
@@ -32,28 +26,28 @@ public:
     //     string on the URL.
     enum Type {
         Success,
-        SuccessWithDelay,
         RequestFail,
         RequestWithCorruptedData
     };
 
-    class Impl;
-
-    MockFileSource(Type type, const std::string& match);
-    ~MockFileSource() override = default;
+    MockFileSource(Type, const std::string& match);
+    ~MockFileSource() override;
 
-    // Function that gets called when a delayed resource is enqueued. The
-    // callback must be safe to call from any thread.
-    void setOnRequestDelayedCallback(std::function<void(void)> callback);
+    // Function that gets called when a matching resource is enqueued.
+    std::function<void (void)> requestEnqueuedCallback;
 
     // FileSource implementation.
     std::unique_ptr<FileRequest> request(const Resource&, Callback) override;
 
 private:
+    void respond(Resource, Callback) const;
+
     friend class MockFileRequest;
-    void cancel(FileRequest*);
 
-    const std::unique_ptr<util::Thread<Impl>> thread_;
+    Type type;
+    std::string match;
+    std::unordered_map<FileRequest*, std::pair<Resource, Callback>> pending;
+    util::Timer timer;
 };
 
 }
diff --git a/test/sprite/sprite_store.cpp b/test/sprite/sprite_store.cpp
index c1b79c7cc6..8f9aebbe67 100644
--- a/test/sprite/sprite_store.cpp
+++ b/test/sprite/sprite_store.cpp
@@ -220,15 +220,15 @@ TEST(SpriteStore, LoadingCorrupted) {
 }
 
 TEST(SpriteStore, LoadingCancel) {
-    SpriteStoreTest test(MockFileSource::SuccessWithDelay, "sprite.json");
+    SpriteStoreTest test(MockFileSource::Success, "sprite.json");
 
     test.observer.spriteLoaded = [&] () {
         FAIL() << "Should never be called";
     };
 
-    test.fileSource.setOnRequestDelayedCallback([&]{
+    test.fileSource.requestEnqueuedCallback = [&]{
         test.end();
-    });
+    };
 
     test.run("test/fixtures/resources/sprite");
 }
diff --git a/test/style/glyph_store.cpp b/test/style/glyph_store.cpp
index f177036d87..a53b20c6d2 100644
--- a/test/style/glyph_store.cpp
+++ b/test/style/glyph_store.cpp
@@ -109,15 +109,15 @@ TEST(GlyphStore, LoadingCorrupted) {
 }
 
 TEST(GlyphStore, LoadingCancel) {
-    GlyphStoreTest test(MockFileSource::SuccessWithDelay, "glyphs.pbf");
+    GlyphStoreTest test(MockFileSource::Success, "glyphs.pbf");
 
     test.observer.glyphsLoaded = [&] (const std::string&, const GlyphRange&) {
         FAIL() << "Should never be called";
     };
 
-    test.fileSource.setOnRequestDelayedCallback([&]{
+    test.fileSource.requestEnqueuedCallback = [&]{
         test.end();
-    });
+    };
 
     test.run(
         "test/fixtures/resources/glyphs.pbf",
diff --git a/test/style/pending_resources.cpp b/test/style/pending_resources.cpp
index e87de1a31b..62aee032da 100644
--- a/test/style/pending_resources.cpp
+++ b/test/style/pending_resources.cpp
@@ -24,7 +24,7 @@ TEST_P(PendingResources, DeleteMapObjectWithPendingRequest) {
 
     auto display = std::make_shared<mbgl::HeadlessDisplay>();
     HeadlessView view(display, 1, 1000, 1000);
-    MockFileSource fileSource(MockFileSource::SuccessWithDelay, GetParam());
+    MockFileSource fileSource(MockFileSource::Success, GetParam());
 
     std::unique_ptr<Map> map = std::make_unique<Map>(view, fileSource, MapMode::Still);
 
@@ -34,7 +34,7 @@ TEST_P(PendingResources, DeleteMapObjectWithPendingRequest) {
     });
 
     endTest.unref();
-    fileSource.setOnRequestDelayedCallback([&endTest] { endTest.send(); });
+    fileSource.requestEnqueuedCallback = [&endTest] { endTest.send(); };
 
     const std::string style = util::read_file("test/fixtures/resources/style.json");
     map->setStyleJSON(style, ".");