From d410b47e875fb8f356189ebd361f4a6468b85281 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Wed, 26 Feb 2020 17:47:03 +0200 Subject: [PATCH] [core] OnlineFileSource is never accessed directly --- include/mbgl/storage/online_file_source.hpp | 2 +- platform/android/src/file_source.cpp | 3 +- platform/android/src/file_source.hpp | 2 +- platform/darwin/src/MGLOfflineStorage.mm | 4 +- .../darwin/src/MGLOfflineStorage_Private.h | 2 +- .../src/mbgl/storage/file_source_manager.cpp | 2 +- platform/qt/src/qmapboxgl.cpp | 2 +- test/src/mbgl/test/fake_file_source.hpp | 4 +- test/storage/main_resource_loader.test.cpp | 3 +- test/storage/online_file_source.test.cpp | 274 +++++++++--------- 10 files changed, 144 insertions(+), 154 deletions(-) diff --git a/include/mbgl/storage/online_file_source.hpp b/include/mbgl/storage/online_file_source.hpp index bb7adbb5d50..d830450f967 100644 --- a/include/mbgl/storage/online_file_source.hpp +++ b/include/mbgl/storage/online_file_source.hpp @@ -10,6 +10,7 @@ class OnlineFileSource : public FileSource { OnlineFileSource(); ~OnlineFileSource() override; +private: // FileSource overrides std::unique_ptr request(const Resource&, Callback) override; bool canRequest(const Resource&) const override; @@ -19,7 +20,6 @@ class OnlineFileSource : public FileSource { mapbox::base::Value getProperty(const std::string&) const override; void setResourceTransform(ResourceTransform) override; -private: class Impl; const std::unique_ptr impl; }; diff --git a/platform/android/src/file_source.cpp b/platform/android/src/file_source.cpp index 5a46bffd3e6..f95066546a7 100644 --- a/platform/android/src/file_source.cpp +++ b/platform/android/src/file_source.cpp @@ -43,8 +43,7 @@ FileSource::FileSource(jni::JNIEnv& _env, const jni::String& accessToken, const mbgl::FileSourceManager::get()->getFileSource(mbgl::FileSourceType::ResourceLoader, resourceOptions); databaseSource = std::static_pointer_cast(std::shared_ptr( mbgl::FileSourceManager::get()->getFileSource(mbgl::FileSourceType::Database, resourceOptions))); - onlineSource = std::static_pointer_cast(std::shared_ptr( - mbgl::FileSourceManager::get()->getFileSource(mbgl::FileSourceType::Network, resourceOptions))); + onlineSource = mbgl::FileSourceManager::get()->getFileSource(mbgl::FileSourceType::Network, resourceOptions); } FileSource::~FileSource() { diff --git a/platform/android/src/file_source.hpp b/platform/android/src/file_source.hpp index 591bfbc93e3..3bf0fed2eb0 100644 --- a/platform/android/src/file_source.hpp +++ b/platform/android/src/file_source.hpp @@ -75,7 +75,7 @@ class FileSource { std::unique_ptr> resourceTransform; std::function pathChangeCallback; std::shared_ptr databaseSource; - std::shared_ptr onlineSource; + std::shared_ptr onlineSource; std::shared_ptr resourceLoader; }; diff --git a/platform/darwin/src/MGLOfflineStorage.mm b/platform/darwin/src/MGLOfflineStorage.mm index 275a58f418c..4c71286b798 100644 --- a/platform/darwin/src/MGLOfflineStorage.mm +++ b/platform/darwin/src/MGLOfflineStorage.mm @@ -46,7 +46,7 @@ @interface MGLOfflineStorage () @property (nonatomic, strong, readwrite) NSMutableArray *packs; @property (nonatomic) std::shared_ptr mbglDatabaseFileSource; -@property (nonatomic) std::shared_ptr mbglOnlineFileSource; +@property (nonatomic) std::shared_ptr mbglOnlineFileSource; @property (nonatomic) std::shared_ptr mbglFileSource; @property (nonatomic) std::string mbglCachePath; @property (nonatomic, getter=isPaused) BOOL paused; @@ -233,7 +233,7 @@ - (instancetype)init { options.withCachePath(_mbglCachePath) .withAssetPath([NSBundle mainBundle].resourceURL.path.UTF8String); _mbglFileSource = mbgl::FileSourceManager::get()->getFileSource(mbgl::FileSourceType::ResourceLoader, options); - _mbglOnlineFileSource = std::static_pointer_cast(std::shared_ptr(mbgl::FileSourceManager::get()->getFileSource(mbgl::FileSourceType::Network, options))); + _mbglOnlineFileSource = mbgl::FileSourceManager::get()->getFileSource(mbgl::FileSourceType::Network, options); _mbglDatabaseFileSource = std::static_pointer_cast(std::shared_ptr(mbgl::FileSourceManager::get()->getFileSource(mbgl::FileSourceType::Database, options))); // Observe for changes to the API base URL (and find out the current one). diff --git a/platform/darwin/src/MGLOfflineStorage_Private.h b/platform/darwin/src/MGLOfflineStorage_Private.h index 13bddd79992..c01163e766b 100644 --- a/platform/darwin/src/MGLOfflineStorage_Private.h +++ b/platform/darwin/src/MGLOfflineStorage_Private.h @@ -19,7 +19,7 @@ NS_ASSUME_NONNULL_BEGIN /** The shared online file source object owned by the shared offline storage object. */ -@property (nonatomic) std::shared_ptr mbglOnlineFileSource; +@property (nonatomic) std::shared_ptr mbglOnlineFileSource; /** The shared resource loader file source object owned by the shared offline storage object. diff --git a/platform/default/src/mbgl/storage/file_source_manager.cpp b/platform/default/src/mbgl/storage/file_source_manager.cpp index 2981096dace..1a13f055681 100644 --- a/platform/default/src/mbgl/storage/file_source_manager.cpp +++ b/platform/default/src/mbgl/storage/file_source_manager.cpp @@ -27,7 +27,7 @@ class DefaultFileSourceManagerImpl final : public FileSourceManager { [](const ResourceOptions&) { return std::make_unique(); }); registerFileSourceFactory(FileSourceType::Network, [](const ResourceOptions& options) { - auto networkSource = std::make_unique(); + std::unique_ptr networkSource = std::make_unique(); networkSource->setProperty(ACCESS_TOKEN_KEY, options.accessToken()); networkSource->setProperty(API_BASE_URL_KEY, options.baseURL()); return networkSource; diff --git a/platform/qt/src/qmapboxgl.cpp b/platform/qt/src/qmapboxgl.cpp index 5b47174b773..14e9d575582 100644 --- a/platform/qt/src/qmapboxgl.cpp +++ b/platform/qt/src/qmapboxgl.cpp @@ -1753,7 +1753,7 @@ QMapboxGLPrivate::QMapboxGLPrivate(QMapboxGL *q, const QMapboxGLSettings &settin }}; std::shared_ptr fs = mbgl::FileSourceManager::get()->getFileSource(mbgl::FileSourceType::Network, resourceOptions); - std::static_pointer_cast(fs)->setResourceTransform(std::move(transform)); + fs->setResourceTransform(std::move(transform)); } // Needs to be Queued to give time to discard redundant draw calls via the `renderQueued` flag. diff --git a/test/src/mbgl/test/fake_file_source.hpp b/test/src/mbgl/test/fake_file_source.hpp index 1faf4b7a18e..7c5adbfff4f 100644 --- a/test/src/mbgl/test/fake_file_source.hpp +++ b/test/src/mbgl/test/fake_file_source.hpp @@ -75,10 +75,10 @@ class FakeOnlineFileSource : public FakeFileSource { } mapbox::base::Value getProperty(const std::string& property) const override { - return onlineFs.getProperty(property); + return onlineFs->getProperty(property); } - OnlineFileSource onlineFs; + std::unique_ptr onlineFs = std::make_unique(); }; } // namespace mbgl diff --git a/test/storage/main_resource_loader.test.cpp b/test/storage/main_resource_loader.test.cpp index 307f92463cc..2622d91183f 100644 --- a/test/storage/main_resource_loader.test.cpp +++ b/test/storage/main_resource_loader.test.cpp @@ -530,9 +530,8 @@ TEST(MainResourceLoader, TEST_REQUIRES_SERVER(NoCacheRefreshModifiedModified)) { TEST(MainResourceLoader, TEST_REQUIRES_SERVER(SetResourceTransform)) { util::RunLoop loop; MainResourceLoader resourceLoader(ResourceOptions{}); - std::shared_ptr fs = + std::shared_ptr onlinefs = FileSourceManager::get()->getFileSource(FileSourceType::Network, ResourceOptions{}); - auto onlinefs = std::static_pointer_cast(fs); // Translates the URL "localhost://test to http://127.0.0.1:3000/test Actor transform( diff --git a/test/storage/online_file_source.test.cpp b/test/storage/online_file_source.test.cpp index 9d8fbfab64e..1bed5f9618d 100644 --- a/test/storage/online_file_source.test.cpp +++ b/test/storage/online_file_source.test.cpp @@ -14,25 +14,23 @@ using namespace mbgl; TEST(OnlineFileSource, Cancel) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); - fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, [&](Response) { - ADD_FAILURE() << "Callback should not be called"; - }); + fs->request({Resource::Unknown, "http://127.0.0.1:3000/test"}, + [&](Response) { ADD_FAILURE() << "Callback should not be called"; }); loop.runOnce(); } TEST(OnlineFileSource, TEST_REQUIRES_SERVER(CancelMultiple)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" }; - std::unique_ptr req2 = fs.request(resource, [&](Response) { - ADD_FAILURE() << "Callback should not be called"; - }); - std::unique_ptr req = fs.request(resource, [&](Response res) { + std::unique_ptr req2 = + fs->request(resource, [&](Response) { ADD_FAILURE() << "Callback should not be called"; }); + std::unique_ptr req = fs->request(resource, [&](Response res) { req.reset(); EXPECT_EQ(nullptr, res.error); ASSERT_TRUE(res.data.get()); @@ -50,38 +48,38 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(CancelMultiple)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(TemporaryError)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); const auto start = Clock::now(); int counter = 0; - auto req = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/temporary-error" }, [&](Response res) { + auto req = fs->request({Resource::Unknown, "http://127.0.0.1:3000/temporary-error"}, [&](Response res) { switch (counter++) { - case 0: { - const auto duration = std::chrono::duration(Clock::now() - start).count(); - EXPECT_GT(0.2, duration) << "Initial error request took too long"; - ASSERT_NE(nullptr, res.error); - EXPECT_EQ(Response::Error::Reason::Server, res.error->reason); - EXPECT_EQ("HTTP status code 500", res.error->message); - ASSERT_FALSE(bool(res.data)); - EXPECT_FALSE(bool(res.expires)); - EXPECT_FALSE(res.mustRevalidate); - EXPECT_FALSE(bool(res.modified)); - EXPECT_FALSE(bool(res.etag)); - } break; - case 1: { - const auto duration = std::chrono::duration(Clock::now() - start).count(); - EXPECT_LT(0.99, duration) << "Backoff timer didn't wait 1 second"; - EXPECT_GT(1.2, duration) << "Backoff timer fired too late"; - EXPECT_EQ(nullptr, res.error); - ASSERT_TRUE(res.data.get()); - EXPECT_EQ("Hello World!", *res.data); - EXPECT_FALSE(bool(res.expires)); - EXPECT_FALSE(res.mustRevalidate); - EXPECT_FALSE(bool(res.modified)); - EXPECT_FALSE(bool(res.etag)); - loop.stop(); - } break; + case 0: { + const auto duration = std::chrono::duration(Clock::now() - start).count(); + EXPECT_GT(0.2, duration) << "Initial error request took too long"; + ASSERT_NE(nullptr, res.error); + EXPECT_EQ(Response::Error::Reason::Server, res.error->reason); + EXPECT_EQ("HTTP status code 500", res.error->message); + ASSERT_FALSE(bool(res.data)); + EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); + EXPECT_FALSE(bool(res.modified)); + EXPECT_FALSE(bool(res.etag)); + } break; + case 1: { + const auto duration = std::chrono::duration(Clock::now() - start).count(); + EXPECT_LT(0.99, duration) << "Backoff timer didn't wait 1 second"; + EXPECT_GT(1.2, duration) << "Backoff timer fired too late"; + EXPECT_EQ(nullptr, res.error); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Hello World!", *res.data); + EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); + EXPECT_FALSE(bool(res.modified)); + EXPECT_FALSE(bool(res.etag)); + loop.stop(); + } break; } }); @@ -90,13 +88,13 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(TemporaryError)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(ConnectionError)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); const auto start = Clock::now(); int counter = 0; int wait = 0; - std::unique_ptr req = fs.request({ Resource::Unknown, "http://127.0.0.1:3001/" }, [&](Response res) { + std::unique_ptr req = fs->request({Resource::Unknown, "http://127.0.0.1:3001/"}, [&](Response res) { const auto duration = std::chrono::duration(Clock::now() - start).count(); EXPECT_LT(wait - 0.01, duration) << "Backoff timer didn't wait 1 second"; EXPECT_GT(wait + 0.3, duration) << "Backoff timer fired too late"; @@ -121,12 +119,12 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(ConnectionError)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(Timeout)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); int counter = 0; const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test?cachecontrol=max-age=1" }; - std::unique_ptr req = fs.request(resource, [&](Response res) { + std::unique_ptr req = fs->request(resource, [&](Response res) { counter++; EXPECT_EQ(nullptr, res.error); ASSERT_TRUE(res.data.get()); @@ -148,12 +146,12 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(Timeout)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RetryDelayOnExpiredTile)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); int counter = 0; const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test?expires=10000" }; - std::unique_ptr req = fs.request(resource, [&](Response res) { + std::unique_ptr req = fs->request(resource, [&](Response res) { counter++; EXPECT_EQ(nullptr, res.error); EXPECT_GT(util::now(), *res.expires); @@ -168,27 +166,27 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RetryDelayOnExpiredTile)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RetryOnClockSkew)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); int counter = 0; const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/clockskew" }; - std::unique_ptr req1 = fs.request(resource, [&](Response res) { + std::unique_ptr req1 = fs->request(resource, [&](Response res) { EXPECT_FALSE(res.mustRevalidate); switch (counter++) { - case 0: { - EXPECT_EQ(nullptr, res.error); - EXPECT_GT(util::now(), *res.expires); - } break; - case 1: { - EXPECT_EQ(nullptr, res.error); + case 0: { + EXPECT_EQ(nullptr, res.error); + EXPECT_GT(util::now(), *res.expires); + } break; + case 1: { + EXPECT_EQ(nullptr, res.error); - auto now = util::now(); - EXPECT_LT(now + Seconds(40), *res.expires) << "Expiration not interpolated to 60s"; - EXPECT_GT(now + Seconds(80), *res.expires) << "Expiration not interpolated to 60s"; + auto now = util::now(); + EXPECT_LT(now + Seconds(40), *res.expires) << "Expiration not interpolated to 60s"; + EXPECT_GT(now + Seconds(80), *res.expires) << "Expiration not interpolated to 60s"; - loop.stop(); - } break; + loop.stop(); + } break; } }); @@ -197,37 +195,31 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RetryOnClockSkew)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RespectPriorExpires)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); // Very long expiration time, should never arrive. Resource resource1{ Resource::Unknown, "http://127.0.0.1:3000/test" }; resource1.priorExpires = util::now() + Seconds(100000); - std::unique_ptr req1 = fs.request(resource1, [&](Response) { - FAIL() << "Should never be called"; - }); + std::unique_ptr req1 = fs->request(resource1, [&](Response) { FAIL() << "Should never be called"; }); // No expiration time, should be requested immediately. Resource resource2{ Resource::Unknown, "http://127.0.0.1:3000/test" }; - std::unique_ptr req2 = fs.request(resource2, [&](Response) { - loop.stop(); - }); + std::unique_ptr req2 = fs->request(resource2, [&](Response) { loop.stop(); }); // Very long expiration time, should never arrive. Resource resource3{ Resource::Unknown, "http://127.0.0.1:3000/test" }; resource3.priorExpires = util::now() + Seconds(100000); - std::unique_ptr req3 = fs.request(resource3, [&](Response) { - FAIL() << "Should never be called"; - }); + std::unique_ptr req3 = fs->request(resource3, [&](Response) { FAIL() << "Should never be called"; }); loop.run(); } TEST(OnlineFileSource, TEST_REQUIRES_SERVER(Load)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); const int concurrency = 50; const int max = 10000; @@ -237,24 +229,23 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(Load)) { std::function req = [&](int i) { const auto current = number++; - reqs[i] = fs.request({ Resource::Unknown, - std::string("http://127.0.0.1:3000/load/") + util::toString(current) }, - [&, i, current](Response res) { - reqs[i].reset(); - EXPECT_EQ(nullptr, res.error); - ASSERT_TRUE(res.data.get()); - EXPECT_EQ(std::string("Request ") + util::toString(current), *res.data); - EXPECT_FALSE(bool(res.expires)); - EXPECT_FALSE(res.mustRevalidate); - EXPECT_FALSE(bool(res.modified)); - EXPECT_FALSE(bool(res.etag)); - - if (number <= max) { - req(i); - } else if (current == max) { - loop.stop(); - } - }); + reqs[i] = fs->request({Resource::Unknown, std::string("http://127.0.0.1:3000/load/") + util::toString(current)}, + [&, i, current](Response res) { + reqs[i].reset(); + EXPECT_EQ(nullptr, res.error); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ(std::string("Request ") + util::toString(current), *res.data); + EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); + EXPECT_FALSE(bool(res.modified)); + EXPECT_FALSE(bool(res.etag)); + + if (number <= max) { + req(i); + } else if (current == max) { + loop.stop(); + } + }); }; for (int i = 0; i < concurrency; i++) { @@ -272,21 +263,21 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(Load)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(NetworkStatusChange)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/delayed" }; // This request takes 200 milliseconds to answer. - std::unique_ptr req = fs.request(resource, [&](Response res) { - req.reset(); - EXPECT_EQ(nullptr, res.error); - ASSERT_TRUE(res.data.get()); - EXPECT_EQ("Response", *res.data); - EXPECT_FALSE(bool(res.expires)); - EXPECT_FALSE(res.mustRevalidate); - EXPECT_FALSE(bool(res.modified)); - EXPECT_FALSE(bool(res.etag)); - loop.stop(); + std::unique_ptr req = fs->request(resource, [&](Response res) { + req.reset(); + EXPECT_EQ(nullptr, res.error); + ASSERT_TRUE(res.data.get()); + EXPECT_EQ("Response", *res.data); + EXPECT_FALSE(bool(res.expires)); + EXPECT_FALSE(res.mustRevalidate); + EXPECT_FALSE(bool(res.modified)); + EXPECT_FALSE(bool(res.etag)); + loop.stop(); }); // After 50 milliseconds, we're going to trigger a NetworkStatus change. @@ -302,14 +293,14 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(NetworkStatusChange)) { // reachability issues. TEST(OnlineFileSource, TEST_REQUIRES_SERVER(NetworkStatusChangePreempt)) { util::RunLoop loop; - OnlineFileSource fs; - fs.pause(); + std::unique_ptr fs = std::make_unique(); + fs->pause(); const auto start = Clock::now(); int counter = 0; const Resource resource{ Resource::Unknown, "http://127.0.0.1:3001/test" }; - std::unique_ptr req = fs.request(resource, [&](Response res) { + std::unique_ptr req = fs->request(resource, [&](Response res) { const auto duration = std::chrono::duration(Clock::now() - start).count(); if (counter == 0) { EXPECT_GT(0.2, duration) << "Response came in too late"; @@ -339,13 +330,13 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(NetworkStatusChangePreempt)) { mbgl::NetworkStatus::Reachable(); }); - fs.resume(); + fs->resume(); loop.run(); } TEST(OnlineFileSource, TEST_REQUIRES_SERVER(NetworkStatusOnlineOffline)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" }; @@ -357,7 +348,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(NetworkStatusOnlineOffline)) { NetworkStatus::Set(NetworkStatus::Status::Online); }); - std::unique_ptr req = fs.request(resource, [&](Response res) { + std::unique_ptr req = fs->request(resource, [&](Response res) { req.reset(); EXPECT_EQ(nullptr, res.error); @@ -373,9 +364,9 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(NetworkStatusOnlineOffline)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RateLimitStandard)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); - auto req = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/rate-limit?std=true" }, [&](Response res) { + auto req = fs->request({Resource::Unknown, "http://127.0.0.1:3000/rate-limit?std=true"}, [&](Response res) { ASSERT_NE(nullptr, res.error); EXPECT_EQ(Response::Error::Reason::RateLimit, res.error->reason); ASSERT_EQ(true, bool(res.error->retryAfter)); @@ -388,9 +379,9 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RateLimitStandard)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RateLimitMBX)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); - auto req = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/rate-limit?mbx=true" }, [&](Response res) { + auto req = fs->request({Resource::Unknown, "http://127.0.0.1:3000/rate-limit?mbx=true"}, [&](Response res) { ASSERT_NE(nullptr, res.error); EXPECT_EQ(Response::Error::Reason::RateLimit, res.error->reason); ASSERT_EQ(true, bool(res.error->retryAfter)); @@ -403,9 +394,9 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RateLimitMBX)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RateLimitDefault)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); - auto req = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/rate-limit" }, [&](Response res) { + auto req = fs->request({Resource::Unknown, "http://127.0.0.1:3000/rate-limit"}, [&](Response res) { ASSERT_NE(nullptr, res.error); EXPECT_EQ(Response::Error::Reason::RateLimit, res.error->reason); ASSERT_FALSE(res.error->retryAfter); @@ -417,46 +408,46 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RateLimitDefault)) { TEST(OnlineFileSource, GetBaseURLAndAccessTokenWhilePaused) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); - fs.pause(); + fs->pause(); auto baseURL = "http://url"; auto accessToken = "access_token"; - fs.setProperty(API_BASE_URL_KEY, baseURL); - fs.setProperty(ACCESS_TOKEN_KEY, accessToken); + fs->setProperty(API_BASE_URL_KEY, baseURL); + fs->setProperty(ACCESS_TOKEN_KEY, accessToken); - EXPECT_EQ(*fs.getProperty(API_BASE_URL_KEY).getString(), baseURL); - EXPECT_EQ(*fs.getProperty(ACCESS_TOKEN_KEY).getString(), accessToken); + EXPECT_EQ(*fs->getProperty(API_BASE_URL_KEY).getString(), baseURL); + EXPECT_EQ(*fs->getProperty(ACCESS_TOKEN_KEY).getString(), accessToken); } TEST(OnlineFileSource, ChangeAPIBaseURL){ util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); - EXPECT_EQ(mbgl::util::API_BASE_URL, *fs.getProperty(API_BASE_URL_KEY).getString()); + EXPECT_EQ(mbgl::util::API_BASE_URL, *fs->getProperty(API_BASE_URL_KEY).getString()); const std::string customURL = "test.domain"; - fs.setProperty(API_BASE_URL_KEY, customURL); - EXPECT_EQ(customURL, *fs.getProperty(API_BASE_URL_KEY).getString()); + fs->setProperty(API_BASE_URL_KEY, customURL); + EXPECT_EQ(customURL, *fs->getProperty(API_BASE_URL_KEY).getString()); } TEST(OnlineFileSource, TEST_REQUIRES_SERVER(LowHighPriorityRequests)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); std::size_t response_counter = 0; const std::size_t NUM_REQUESTS = 3; NetworkStatus::Set(NetworkStatus::Status::Offline); - fs.setProperty(MAX_CONCURRENT_REQUESTS_KEY, 1u); + fs->setProperty(MAX_CONCURRENT_REQUESTS_KEY, 1u); // After DefaultFileSource was split, OnlineFileSource lives on a separate // thread. Pause OnlineFileSource, so that messages are queued for processing. - fs.pause(); + fs->pause(); // First regular request. Resource regular1{Resource::Unknown, "http://127.0.0.1:3000/load/1"}; - std::unique_ptr req_0 = fs.request(regular1, [&](Response) { + std::unique_ptr req_0 = fs->request(regular1, [&](Response) { response_counter++; req_0.reset(); }); @@ -464,7 +455,7 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(LowHighPriorityRequests)) { // Low priority request that will be queued and should be requested last. Resource low_prio{Resource::Unknown, "http://127.0.0.1:3000/load/2"}; low_prio.setPriority(Resource::Priority::Low); - std::unique_ptr req_1 = fs.request(low_prio, [&](Response) { + std::unique_ptr req_1 = fs->request(low_prio, [&](Response) { response_counter++; req_1.reset(); EXPECT_EQ(response_counter, NUM_REQUESTS); // make sure this is responded last @@ -473,12 +464,12 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(LowHighPriorityRequests)) { // Second regular priority request that should de-preoritize low priority request. Resource regular2{ Resource::Unknown, "http://127.0.0.1:3000/load/3" }; - std::unique_ptr req_2 = fs.request(regular2, [&](Response) { + std::unique_ptr req_2 = fs->request(regular2, [&](Response) { response_counter++; req_2.reset(); }); - fs.resume(); + fs->resume(); NetworkStatus::Set(NetworkStatus::Status::Online); loop.run(); } @@ -486,33 +477,34 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(LowHighPriorityRequests)) { TEST(OnlineFileSource, TEST_REQUIRES_SERVER(LowHighPriorityRequestsMany)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); int response_counter = 0; int correct_low = 0; int correct_regular = 0; NetworkStatus::Set(NetworkStatus::Status::Offline); - fs.setProperty(MAX_CONCURRENT_REQUESTS_KEY, 1u); - fs.pause(); + fs->setProperty(MAX_CONCURRENT_REQUESTS_KEY, 1u); + fs->pause(); std::vector> collector; for (int num_reqs = 0; num_reqs < 20; num_reqs++) { if (num_reqs % 2 == 0) { - std::unique_ptr req = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/load/" + std::to_string(num_reqs) }, [&](Response) { - response_counter++; + std::unique_ptr req = fs->request( + {Resource::Unknown, "http://127.0.0.1:3000/load/" + std::to_string(num_reqs)}, [&](Response) { + response_counter++; - if (response_counter <= 10) { // count the high priority requests that arrive late correctly - correct_regular++; - } - }); + if (response_counter <= 10) { // count the high priority requests that arrive late correctly + correct_regular++; + } + }); collector.push_back(std::move(req)); } else { Resource resource = { Resource::Unknown, "http://127.0.0.1:3000/load/" + std::to_string(num_reqs) }; resource.setPriority(Resource::Priority::Low); - std::unique_ptr req = fs.request(std::move(resource), [&](Response) { + std::unique_ptr req = fs->request(std::move(resource), [&](Response) { response_counter++; if (response_counter > 10) { // count the low priority requests that arrive late correctly @@ -533,30 +525,30 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(LowHighPriorityRequestsMany)) { } } - fs.resume(); + fs->resume(); NetworkStatus::Set(NetworkStatus::Status::Online); loop.run(); } TEST(OnlineFileSource, TEST_REQUIRES_SERVER(MaximumConcurrentRequests)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); - ASSERT_EQ(*fs.getProperty(MAX_CONCURRENT_REQUESTS_KEY).getUint(), 20u); + ASSERT_EQ(*fs->getProperty(MAX_CONCURRENT_REQUESTS_KEY).getUint(), 20u); - fs.setProperty(MAX_CONCURRENT_REQUESTS_KEY, 10u); - ASSERT_EQ(*fs.getProperty(MAX_CONCURRENT_REQUESTS_KEY).getUint(), 10u); + fs->setProperty(MAX_CONCURRENT_REQUESTS_KEY, 10u); + ASSERT_EQ(*fs->getProperty(MAX_CONCURRENT_REQUESTS_KEY).getUint(), 10u); } TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RequestSameUrlMultipleTimes)) { util::RunLoop loop; - OnlineFileSource fs; + std::unique_ptr fs = std::make_unique(); int count = 0; std::vector> requests; for (int i = 0; i < 100; ++i) { - requests.emplace_back(fs.request({ Resource::Unknown, "http://127.0.0.1:3000/load" }, [&](Response) { + requests.emplace_back(fs->request({Resource::Unknown, "http://127.0.0.1:3000/load"}, [&](Response) { if (++count == 100) { loop.stop(); }