diff --git a/android/jni/com/mapswithme/maps/DownloadResourcesLegacyActivity.cpp b/android/jni/com/mapswithme/maps/DownloadResourcesLegacyActivity.cpp index 82461fa8b2..24f07a254f 100644 --- a/android/jni/com/mapswithme/maps/DownloadResourcesLegacyActivity.cpp +++ b/android/jni/com/mapswithme/maps/DownloadResourcesLegacyActivity.cpp @@ -2,17 +2,21 @@ #include "defines.hpp" -#include "coding/internal/file_data.hpp" -#include "coding/reader_streambuf.hpp" -#include "coding/url_encode.hpp" +#include "storage/map_files_downloader.hpp" +#include "storage/storage.hpp" #include "platform/platform.hpp" #include "platform/http_request.hpp" #include "platform/servers_list.hpp" +#include "coding/internal/file_data.hpp" +#include "coding/reader_streambuf.hpp" +#include "coding/url_encode.hpp" + #include "base/file_name_utils.hpp" #include "base/logging.hpp" #include "base/string_utils.hpp" +#include "base/url_helpers.hpp" #include "com/mapswithme/core/jni_helper.hpp" @@ -23,6 +27,8 @@ #include using namespace downloader; +using namespace storage; + using namespace std::placeholders; /// Special error codes to notify GUI about free space @@ -183,12 +189,14 @@ extern "C" LOG(LINFO, ("Finished URL list download for", curFile.m_fileName)); - GetServerListFromRequest(req, curFile.m_urls); + GetServerList(req, curFile.m_urls); - storage::Storage const & storage = g_framework->GetStorage(); + Storage const & storage = g_framework->GetStorage(); for (size_t i = 0; i < curFile.m_urls.size(); ++i) { - curFile.m_urls[i] = storage.GetFileDownloadUrl(curFile.m_urls[i], curFile.m_fileName); + curFile.m_urls[i] = MapFilesDownloader::MakeFullUrlLegacy(curFile.m_urls[i], + curFile.m_fileName, + storage.GetCurrentDataVersion()); LOG(LDEBUG, (curFile.m_urls[i])); } diff --git a/platform/servers_list.cpp b/platform/servers_list.cpp index 762ed17382..454c9fbf27 100644 --- a/platform/servers_list.cpp +++ b/platform/servers_list.cpp @@ -10,7 +10,6 @@ namespace downloader { - // Returns false if can't parse urls. Note that it also clears outUrls. bool ParseServerList(string const & jsonStr, vector & outUrls) { @@ -32,13 +31,18 @@ bool ParseServerList(string const & jsonStr, vector & outUrls) return !outUrls.empty(); } -void GetServerListFromRequest(HttpRequest const & request, vector & urls) +void GetServerList(string const & src, vector & urls) { - if (request.GetStatus() == HttpRequest::Status::Completed && ParseServerList(request.GetData(), urls)) + if (!src.empty() && ParseServerList(src, urls)) return; VERIFY(ParseServerList(GetPlatform().DefaultUrlsJSON(), urls), ()); LOG(LWARNING, ("Can't get servers list from request, using default servers:", urls)); } +void GetServerList(HttpRequest const & request, vector & urls) +{ + auto const src = request.GetStatus() == HttpRequest::Status::Completed ? request.GetData() : ""; + GetServerList(src, urls); +} } // namespace downloader diff --git a/platform/servers_list.hpp b/platform/servers_list.hpp index 65c16d3226..ae5b7f3df0 100644 --- a/platform/servers_list.hpp +++ b/platform/servers_list.hpp @@ -3,10 +3,10 @@ #include "std/vector.hpp" #include "std/string.hpp" - namespace downloader { - class HttpRequest; +class HttpRequest; - void GetServerListFromRequest(HttpRequest const & request, vector & urls); -} +void GetServerList(string const & src, vector & urls); +void GetServerList(HttpRequest const & request, vector & urls); +} // namespace downloader diff --git a/search/search_integration_tests/downloader_search_test.cpp b/search/search_integration_tests/downloader_search_test.cpp index 4cfe30f3eb..214a3bb11f 100644 --- a/search/search_integration_tests/downloader_search_test.cpp +++ b/search/search_integration_tests/downloader_search_test.cpp @@ -83,19 +83,20 @@ class TestMapFilesDownloader : public storage::MapFilesDownloader { public: // MapFilesDownloader overrides: - void GetServersList(ServersListCallback const & /* callback */) override {} - - void DownloadMapFile(vector const & /* urls */, string const & /* path */, - int64_t /* size */, FileDownloadedCallback const & /* onDownloaded */, - DownloadingProgressCallback const & /* onProgress */) override - { - } - Progress GetDownloadingProgress() override { return Progress{}; } bool IsIdle() override { return false; } void Reset() override {} + +private: + void GetServersList(ServersListCallback const & /* callback */) override {} + + void Download(vector const & /* urls */, string const & /* path */, int64_t /* size */, + FileDownloadedCallback const & /* onDownloaded */, + DownloadingProgressCallback const & /* onProgress */) override + { + } }; class TestDelegate : public DownloaderSearchCallback::Delegate diff --git a/storage/CMakeLists.txt b/storage/CMakeLists.txt index d5f90980c3..4c72c04945 100644 --- a/storage/CMakeLists.txt +++ b/storage/CMakeLists.txt @@ -27,7 +27,10 @@ set( downloading_policy.hpp http_map_files_downloader.cpp http_map_files_downloader.hpp + map_files_downloader.cpp map_files_downloader.hpp + map_files_downloader_with_ping.cpp + map_files_downloader_with_ping.hpp queued_country.cpp queued_country.hpp pinger.cpp diff --git a/storage/http_map_files_downloader.cpp b/storage/http_map_files_downloader.cpp index a7cf8417d9..ff7c9256b0 100644 --- a/storage/http_map_files_downloader.cpp +++ b/storage/http_map_files_downloader.cpp @@ -1,12 +1,11 @@ #include "storage/http_map_files_downloader.hpp" -#include "platform/platform.hpp" #include "platform/servers_list.hpp" #include "base/assert.hpp" +#include "base/string_utils.hpp" #include "std/bind.hpp" -#include "base/string_utils.hpp" namespace { @@ -31,18 +30,10 @@ HttpMapFilesDownloader::~HttpMapFilesDownloader() CHECK_THREAD_CHECKER(m_checker, ()); } -void HttpMapFilesDownloader::GetServersList(ServersListCallback const & callback) -{ - CHECK_THREAD_CHECKER(m_checker, ()); - m_request.reset(downloader::HttpRequest::Get( - GetPlatform().MetaServerUrl(), - bind(&HttpMapFilesDownloader::OnServersListDownloaded, this, callback, _1))); -} - -void HttpMapFilesDownloader::DownloadMapFile(vector const & urls, string const & path, - int64_t size, - FileDownloadedCallback const & onDownloaded, - DownloadingProgressCallback const & onProgress) +void HttpMapFilesDownloader::Download(vector const & urls, string const & path, + int64_t size, + FileDownloadedCallback const & onDownloaded, + DownloadingProgressCallback const & onProgress) { CHECK_THREAD_CHECKER(m_checker, ()); m_request.reset(downloader::HttpRequest::GetFile( @@ -76,15 +67,6 @@ void HttpMapFilesDownloader::Reset() m_request.reset(); } -void HttpMapFilesDownloader::OnServersListDownloaded(ServersListCallback const & callback, - downloader::HttpRequest & request) -{ - CHECK_THREAD_CHECKER(m_checker, ()); - vector urls; - GetServerListFromRequest(request, urls); - callback(urls); -} - void HttpMapFilesDownloader::OnMapFileDownloaded(FileDownloadedCallback const & onDownloaded, downloader::HttpRequest & request) { diff --git a/storage/http_map_files_downloader.hpp b/storage/http_map_files_downloader.hpp index a2c3d1b3a5..1ae5fe4248 100644 --- a/storage/http_map_files_downloader.hpp +++ b/storage/http_map_files_downloader.hpp @@ -1,6 +1,6 @@ #pragma once -#include "storage/map_files_downloader.hpp" +#include "storage/map_files_downloader_with_ping.hpp" #include "platform/http_request.hpp" #include "base/thread_checker.hpp" #include "std/unique_ptr.hpp" @@ -11,24 +11,22 @@ namespace storage /// and file downloading. // // *NOTE*, this class is not thread-safe. -class HttpMapFilesDownloader : public MapFilesDownloader +class HttpMapFilesDownloader : public MapFilesDownloaderWithPing { public: virtual ~HttpMapFilesDownloader(); // MapFilesDownloader overrides: - void GetServersList(ServersListCallback const & callback) override; - - void DownloadMapFile(vector const & urls, string const & path, int64_t size, - FileDownloadedCallback const & onDownloaded, - DownloadingProgressCallback const & onProgress) override; Progress GetDownloadingProgress() override; bool IsIdle() override; void Reset() override; private: - void OnServersListDownloaded(ServersListCallback const & callback, - downloader::HttpRequest & request); + // MapFilesDownloaderWithServerList overrides: + void Download(vector const & urls, string const & path, int64_t size, + FileDownloadedCallback const & onDownloaded, + DownloadingProgressCallback const & onProgress) override; + void OnMapFileDownloaded(FileDownloadedCallback const & onDownloaded, downloader::HttpRequest & request); void OnMapFileDownloadingProgress(DownloadingProgressCallback const & onProgress, diff --git a/storage/map_files_downloader.cpp b/storage/map_files_downloader.cpp new file mode 100644 index 0000000000..6c73c1efdc --- /dev/null +++ b/storage/map_files_downloader.cpp @@ -0,0 +1,99 @@ +#include "storage/map_files_downloader.hpp" + +#include "storage/queued_country.hpp" + +#include "platform/http_client.hpp" +#include "platform/platform.hpp" +#include "platform/servers_list.hpp" + +#include "coding/url_encode.hpp" + +#include "base/assert.hpp" +#include "base/string_utils.hpp" +#include "base/url_helpers.hpp" + +#include + +namespace storage +{ +namespace +{ +std::vector MakeUrlList(MapFilesDownloader::ServersList const & servers, + std::string const & relativeUrl) +{ + std::vector urls; + urls.reserve(servers.size()); + for (auto const & server : servers) + urls.emplace_back(base::url::Join(server, relativeUrl)); + + return urls; +} +} // namespace + +void MapFilesDownloader::DownloadMapFile(std::string const & relativeUrl, + std::string const & path, int64_t size, + FileDownloadedCallback const & onDownloaded, + DownloadingProgressCallback const & onProgress) +{ + if (m_serverList.empty()) + { + GetServersList([=](ServersList const & serverList) + { + m_serverList = serverList; + auto const urls = MakeUrlList(m_serverList, relativeUrl); + Download(urls, path, size, onDownloaded, onProgress); + }); + } + else + { + auto const urls = MakeUrlList(m_serverList, relativeUrl); + Download(urls, path, size, onDownloaded, onProgress); + } +} + +// static +std::string MapFilesDownloader::MakeRelativeUrl(std::string const & fileName, int64_t dataVersion, + uint64_t diffVersion) +{ + std::ostringstream url; + if (diffVersion != 0) + url << "diffs/" << dataVersion << "/" << diffVersion; + else + url << OMIM_OS_NAME "/" << dataVersion; + + return base::url::Join(url.str(), UrlEncode(fileName)); +} + +// static +std::string MapFilesDownloader::MakeFullUrlLegacy(string const & baseUrl, string const & fileName, + int64_t dataVersion) +{ + return base::url::Join(baseUrl, OMIM_OS_NAME, strings::to_string(dataVersion), + UrlEncode(fileName)); +} + +void MapFilesDownloader::SetServersList(ServersList const & serversList) +{ + m_serverList = serversList; +} + +// static +MapFilesDownloader::ServersList MapFilesDownloader::LoadServersList() +{ + platform::HttpClient request(GetPlatform().MetaServerUrl()); + std::string httpResult; + request.RunHttpRequest(httpResult); + std::vector urls; + downloader::GetServerList(httpResult, urls); + CHECK(!urls.empty(), ()); + return urls; +} + +void MapFilesDownloader::GetServersList(ServersListCallback const & callback) +{ + GetPlatform().RunTask(Platform::Thread::Network, [callback]() + { + callback(LoadServersList()); + }); +} +} // namespace storage diff --git a/storage/map_files_downloader.hpp b/storage/map_files_downloader.hpp index 96bab0ed5d..b1e461f86d 100644 --- a/storage/map_files_downloader.hpp +++ b/storage/map_files_downloader.hpp @@ -1,12 +1,14 @@ #pragma once #include "platform/http_request.hpp" +#include "platform/safe_callback.hpp" -#include "std/function.hpp" -#include "std/string.hpp" - -#include "std/utility.hpp" -#include "std/vector.hpp" +#include +#include +#include +#include +#include +#include namespace storage { @@ -16,25 +18,22 @@ class MapFilesDownloader { public: // Denotes bytes downloaded and total number of bytes. - using Progress = pair; + using Progress = std::pair; + using ServersList = std::vector; using FileDownloadedCallback = - function; - using DownloadingProgressCallback = function; - using ServersListCallback = function & urls)>; + std::function; + using DownloadingProgressCallback = std::function; + using ServersListCallback = platform::SafeCallback; virtual ~MapFilesDownloader() = default; - /// Asynchronously receives a list of all servers that can be asked - /// for a map file and invokes callback on the original thread. - virtual void GetServersList(ServersListCallback const & callback) = 0; - /// Asynchronously downloads a map file, periodically invokes /// onProgress callback and finally invokes onDownloaded - /// callback. Both callbacks will be invoked on the original thread. - virtual void DownloadMapFile(vector const & urls, string const & path, int64_t size, - FileDownloadedCallback const & onDownloaded, - DownloadingProgressCallback const & onProgress) = 0; + /// callback. Both callbacks will be invoked on the main thread. + void DownloadMapFile(std::string const & relativeUrl, std::string const & path, int64_t size, + FileDownloadedCallback const & onDownloaded, + DownloadingProgressCallback const & onProgress); /// Returns current downloading progress. virtual Progress GetDownloadingProgress() = 0; @@ -44,5 +43,28 @@ public: /// Resets downloader to the idle state. virtual void Reset() = 0; + + static std::string MakeRelativeUrl(std::string const & fileName, int64_t dataVersion, + uint64_t diffVersion); + static std::string MakeFullUrlLegacy(string const & baseUrl, string const & fileName, + int64_t dataVersion); + + void SetServersList(ServersList const & serversList); + +protected: + // Synchronously loads list of servers by http client. + static ServersList LoadServersList(); + +private: + /// Default implementation asynchronously receives a list of all servers that can be asked + /// for a map file and invokes callback on the main thread. + virtual void GetServersList(ServersListCallback const & callback); + /// This method must be implemented to asynchronous downloading file + /// from provided |urls| and saving result into |path|. + virtual void Download(std::vector const & urls, std::string const & path, + int64_t size, FileDownloadedCallback const & onDownloaded, + DownloadingProgressCallback const & onProgress) = 0; + + ServersList m_serverList; }; } // namespace storage diff --git a/storage/map_files_downloader_with_ping.cpp b/storage/map_files_downloader_with_ping.cpp new file mode 100644 index 0000000000..326ee61969 --- /dev/null +++ b/storage/map_files_downloader_with_ping.cpp @@ -0,0 +1,23 @@ +#include "storage/map_files_downloader_with_ping.hpp" + +#include "storage/pinger.hpp" + +#include "platform/platform.hpp" + +#include "base/assert.hpp" + +namespace storage +{ +void MapFilesDownloaderWithPing::GetServersList(ServersListCallback const & callback) +{ + GetPlatform().RunTask(Platform::Thread::Network, [callback]() + { + auto urls = LoadServersList(); + + CHECK(!urls.empty(), ()); + + auto const availableEndpoints = Pinger::ExcludeUnavailableEndpoints(urls); + callback(availableEndpoints.empty() ? urls : availableEndpoints); + }); +} +} // namespace storage diff --git a/storage/map_files_downloader_with_ping.hpp b/storage/map_files_downloader_with_ping.hpp new file mode 100644 index 0000000000..f5c0b2c4d3 --- /dev/null +++ b/storage/map_files_downloader_with_ping.hpp @@ -0,0 +1,12 @@ +#pragma once + +#include "storage/map_files_downloader.hpp" + +namespace storage +{ +class MapFilesDownloaderWithPing : public MapFilesDownloader +{ +private: + void GetServersList(ServersListCallback const & callback) override; +}; +} // namespace storage diff --git a/storage/pinger.cpp b/storage/pinger.cpp index 7089f1f7eb..0bc9f5971f 100644 --- a/storage/pinger.cpp +++ b/storage/pinger.cpp @@ -54,7 +54,7 @@ void SendStatistics(size_t serversLeft) namespace storage { // static -void Pinger::Ping(vector const & urls, Pinger::Pong const & pong) +Pinger::Endpoints Pinger::ExcludeUnavailableEndpoints(Endpoints const & urls) { auto const size = urls.size(); CHECK_GREATER(size, 0, ()); @@ -70,6 +70,6 @@ void Pinger::Ping(vector const & urls, Pinger::Pong const & pong) base::EraseIf(readyUrls, [](auto const & url) { return url.empty(); }); SendStatistics(readyUrls.size()); - pong(move(readyUrls)); + return readyUrls; } } // namespace storage diff --git a/storage/pinger.hpp b/storage/pinger.hpp index 6fd185455c..45e2e243ed 100644 --- a/storage/pinger.hpp +++ b/storage/pinger.hpp @@ -10,7 +10,8 @@ namespace storage class Pinger { public: - using Pong = platform::SafeCallback readyUrls)>; - static void Ping(std::vector const & urls, Pong const & pong); + using Endpoints = std::vector; + // Returns list of available endpoints. Works synchronously. + static Endpoints ExcludeUnavailableEndpoints(Endpoints const & urls); }; } // namespace storage diff --git a/storage/storage.cpp b/storage/storage.cpp index 4685f284db..979c930eba 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -1,4 +1,5 @@ #include "storage/storage.hpp" + #include "storage/http_map_files_downloader.hpp" #include "defines.hpp" @@ -8,11 +9,9 @@ #include "platform/mwm_version.hpp" #include "platform/platform.hpp" #include "platform/preferred_languages.hpp" -#include "platform/servers_list.hpp" #include "platform/settings.hpp" #include "coding/internal/file_data.hpp" -#include "coding/url_encode.hpp" #include "base/exception.hpp" #include "base/file_name_utils.hpp" @@ -26,8 +25,6 @@ #include #include -#include "std/target_os.hpp" - #include #include "3party/Alohalytics/src/alohalytics.h" @@ -197,8 +194,6 @@ void Storage::PrefetchMigrateData() m_prefetchStorage->EnableKeepDownloadingQueue(false); m_prefetchStorage->Init([](CountryId const &, LocalFilePtr const) {}, [](CountryId const &, LocalFilePtr const) { return false; }); - if (!m_downloadingUrlsForTesting.empty()) - m_prefetchStorage->SetDownloadingUrlsForTesting(m_downloadingUrlsForTesting); } void Storage::Migrate(CountriesVec const & existedCountries) @@ -723,15 +718,7 @@ void Storage::DownloadNextFile(QueuedCountry const & country) return; } - if (m_sessionServerList || !m_downloadingUrlsForTesting.empty()) - { - DoDownload(); - } - else - { - LoadServerListForSession(); - SetDeferDownloading(); - } + DoDownload(); } void Storage::DeleteFromDownloader(CountryId const & countryId) @@ -870,7 +857,6 @@ void Storage::ReportProgressForHierarchy(CountryId const & countryId, void Storage::DoDownload() { CHECK_THREAD_CHECKER(m_threadChecker, ()); - CHECK(m_sessionServerList || !m_downloadingUrlsForTesting.empty(), ()); // Queue can be empty because countries were deleted from queue. if (m_queue.empty()) @@ -894,17 +880,13 @@ void Storage::DoDownload() } } - vector const & downloadingUrls = - m_downloadingUrlsForTesting.empty() ? *m_sessionServerList : m_downloadingUrlsForTesting; - vector fileUrls; - fileUrls.reserve(downloadingUrls.size()); - for (string const & url : downloadingUrls) - fileUrls.push_back(GetFileDownloadUrl(url, queuedCountry)); + auto const & id = queuedCountry.GetCountryId(); + auto const options = queuedCountry.GetCurrentFileOptions(); + auto const relativeUrl = GetDownloadRelativeUrl(id, options); + auto const filePath = GetFileDownloadPath(id, options); - string const filePath = - GetFileDownloadPath(queuedCountry.GetCountryId(), queuedCountry.GetCurrentFileOptions()); using namespace std::placeholders; - m_downloader->DownloadMapFile(fileUrls, filePath, GetDownloadSize(queuedCountry), + m_downloader->DownloadMapFile(relativeUrl, filePath, GetDownloadSize(queuedCountry), bind(&Storage::OnMapFileDownloadFinished, this, _1, _2), bind(&Storage::OnMapFileDownloadProgress, this, _1)); } @@ -920,7 +902,7 @@ void Storage::DoDeferredDownloadIfNeeded() { CHECK_THREAD_CHECKER(m_threadChecker, ()); - if (!m_needToStartDeferredDownloading || !m_sessionServerList) + if (!m_needToStartDeferredDownloading) return; m_needToStartDeferredDownloading = false; @@ -1054,37 +1036,17 @@ void Storage::OnMapDownloadFinished(CountryId const & countryId, HttpRequest::St RegisterDownloadedFiles(countryId, options); } -string Storage::GetFileDownloadUrl(string const & baseUrl, - QueuedCountry const & queuedCountry) const +string Storage::GetDownloadRelativeUrl(CountryId const & countryId, MapOptions options) const { - auto const & countryId = queuedCountry.GetCountryId(); - auto const options = queuedCountry.GetCurrentFileOptions(); - CountryFile const & countryFile = GetCountryFile(countryId); + auto const & countryFile = GetCountryFile(countryId); + auto const dataVersion = GetCurrentDataVersion(); + auto const fileName = GetFileName(countryFile.GetName(), options, dataVersion); - string const fileName = GetFileName(countryFile.GetName(), options, GetCurrentDataVersion()); - - ostringstream url; - url << baseUrl; - string const currentVersion = strings::to_string(GetCurrentDataVersion()); + uint64_t diffVersion = 0; if (options == MapOptions::Diff) - { - uint64_t version; - CHECK(m_diffManager.VersionFor(countryId, version), ()); - url << "diffs/" << currentVersion << "/" << strings::to_string(version); - } - else - { - url << OMIM_OS_NAME "/" << currentVersion; - } + CHECK(m_diffManager.VersionFor(countryId, diffVersion), ()); - url << "/" << UrlEncode(fileName); - return url.str(); -} - -string Storage::GetFileDownloadUrl(string const & baseUrl, string const & fileName) const -{ - return baseUrl + OMIM_OS_NAME "/" + strings::to_string(GetCurrentDataVersion()) + "/" + - UrlEncode(fileName); + return MapFilesDownloader::MakeRelativeUrl(fileName, dataVersion, diffVersion); } CountryId Storage::FindCountryIdByFile(string const & name) const @@ -1193,10 +1155,9 @@ bool Storage::IsDiffApplyingInProgressToCountry(CountryId const & countryId) con void Storage::SetLocale(string const & locale) { m_countryNameGetter.SetLocale(locale); } string Storage::GetLocale() const { return m_countryNameGetter.GetLocale(); } -void Storage::SetDownloaderForTesting(unique_ptr && downloader) +void Storage::SetDownloaderForTesting(unique_ptr downloader) { m_downloader = move(downloader); - LoadServerListForTesting(); } void Storage::SetEnabledIntegrityValidationForTesting(bool enabled) @@ -1209,9 +1170,9 @@ void Storage::SetCurrentDataVersionForTesting(int64_t currentVersion) m_currentVersion = currentVersion; } -void Storage::SetDownloadingUrlsForTesting(vector const & downloadingUrls) +void Storage::SetDownloadingServersForTesting(vector const & downloadingUrls) { - m_downloadingUrlsForTesting = downloadingUrls; + m_downloader->SetServersList(downloadingUrls); } void Storage::SetLocaleForTesting(string const & jsonBuffer, string const & locale) @@ -1648,41 +1609,6 @@ void Storage::ApplyDiff(CountryId const & countryId, functionGetServersList([this](vector const & urls) { PingServerList(urls); }); -} - -void Storage::LoadServerListForTesting() -{ - CHECK_THREAD_CHECKER(m_threadChecker, ()); - - m_downloader->GetServersList([this](auto const & urls) { m_sessionServerList = urls; }); -} - -void Storage::PingServerList(vector const & urls) -{ - CHECK_THREAD_CHECKER(m_threadChecker, ()); - - if (urls.empty()) - return; - - GetPlatform().RunTask(Platform::Thread::Network, [urls, this] { - Pinger::Ping(urls, [this, urls](vector readyUrls) { - CHECK_THREAD_CHECKER(m_threadChecker, ()); - - if (readyUrls.empty()) - m_sessionServerList = urls; - else - m_sessionServerList = move(readyUrls); - - DoDeferredDownloadIfNeeded(); - }); - }); -} - bool Storage::IsPossibleToAutoupdate() const { CHECK_THREAD_CHECKER(m_threadChecker, ()); diff --git a/storage/storage.hpp b/storage/storage.hpp index 853e0d9b4e..ea55624124 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -6,7 +6,6 @@ #include "storage/diff_scheme/diff_manager.hpp" #include "storage/downloading_policy.hpp" #include "storage/map_files_downloader.hpp" -#include "storage/pinger.hpp" #include "storage/queued_country.hpp" #include "storage/storage_defines.hpp" @@ -241,10 +240,6 @@ private: // downloading maps to a special place but not for continue working with them from this place. std::string m_dataDir; - // A list of urls for downloading maps. It's necessary for storage integration tests. - // For example {"http://new-search.mapswithme.com/"}. - std::vector m_downloadingUrlsForTesting; - bool m_integrityValidationEnabled = true; // |m_downloadMapOnTheMap| is called when an end user clicks on download map or retry button @@ -274,7 +269,6 @@ private: std::vector m_notAppliedDiffs; bool m_needToStartDeferredDownloading = false; - boost::optional> m_sessionServerList; StartDownloadingCallback m_startDownloadingCallback; @@ -579,10 +573,8 @@ public: CountryId GetCurrentDownloadingCountryId() const; void EnableKeepDownloadingQueue(bool enable) {m_keepDownloadingQueue = enable;} - /// get download url by base url & queued country - std::string GetFileDownloadUrl(std::string const & baseUrl, - QueuedCountry const & queuedCountry) const; - std::string GetFileDownloadUrl(std::string const & baseUrl, std::string const & fileName) const; + + std::string GetDownloadRelativeUrl(CountryId const & countryId, MapOptions options) const; /// @param[out] res Populated with oudated countries. void GetOutdatedCountries(vector & countries) const; @@ -595,9 +587,9 @@ public: // for testing: void SetEnabledIntegrityValidationForTesting(bool enabled); - void SetDownloaderForTesting(std::unique_ptr && downloader); + void SetDownloaderForTesting(std::unique_ptr downloader); void SetCurrentDataVersionForTesting(int64_t currentVersion); - void SetDownloadingUrlsForTesting(std::vector const & downloadingUrls); + void SetDownloadingServersForTesting(std::vector const & downloadingUrls); void SetLocaleForTesting(std::string const & jsonBuffer, std::string const & locale); /// Returns true if the diff scheme is available and all local outdated maps can be updated via @@ -722,10 +714,6 @@ private: // Should be called once on startup, downloading process should be suspended until this method // was not called. Do not call this method manually. void OnDiffStatusReceived(diffs::Status const status) override; - - void LoadServerListForSession(); - void LoadServerListForTesting(); - void PingServerList(std::vector const & urls); }; void GetQueuedCountries(Storage::Queue const & queue, CountriesSet & resultCountries); diff --git a/storage/storage_integration_tests/migrate_tests.cpp b/storage/storage_integration_tests/migrate_tests.cpp index 602cc386db..203bd27c35 100644 --- a/storage/storage_integration_tests/migrate_tests.cpp +++ b/storage/storage_integration_tests/migrate_tests.cpp @@ -88,7 +88,7 @@ UNIT_TEST(StorageFastMigrationTests) // // Somewhere in Moscow, Russia // ms::LatLon curPos(55.7, 37.7); // -// s.SetDownloadingUrlsForTesting({"http://direct.mapswithme.com/"}); +// s.SetDownloadingServersForTesting({"http://direct.mapswithme.com/"}); // s.Subscribe(stateChanged, progressChanged); // for (auto const & countryId : kOldCountries) // s.DownloadNode(countryId); diff --git a/storage/storage_integration_tests/storage_3levels_tests.cpp b/storage/storage_integration_tests/storage_3levels_tests.cpp index 7312cc1698..962a435401 100644 --- a/storage/storage_integration_tests/storage_3levels_tests.cpp +++ b/storage/storage_integration_tests/storage_3levels_tests.cpp @@ -56,7 +56,7 @@ UNIT_TEST(SmallMwms_3levels_Test) }; storage.Subscribe(onChangeCountryFn, onProgressFn); - storage.SetDownloadingUrlsForTesting({kTestWebServer}); + storage.SetDownloadingServersForTesting({kTestWebServer}); NodeAttrs attrs; storage.GetNodeAttrs(kCountryId, attrs); diff --git a/storage/storage_integration_tests/storage_downloading_tests.cpp b/storage/storage_integration_tests/storage_downloading_tests.cpp index 3793ca6036..40bed26ebf 100644 --- a/storage/storage_integration_tests/storage_downloading_tests.cpp +++ b/storage/storage_integration_tests/storage_downloading_tests.cpp @@ -60,7 +60,7 @@ void InitStorage(Storage & storage, Storage::ProgressFunction const & onProgress storage.Init(Update, [](CountryId const &, storage::LocalFilePtr const) { return false; }); storage.RegisterAllLocalMaps(false /* enableDiffs */); storage.Subscribe(bind(&ChangeCountry, ref(storage), _1), onProgressFn); - storage.SetDownloadingUrlsForTesting({kTestWebServer}); + storage.SetDownloadingServersForTesting({kTestWebServer}); } } // namespace diff --git a/storage/storage_integration_tests/storage_group_download_tests.cpp b/storage/storage_integration_tests/storage_group_download_tests.cpp index 68964f102a..3a91432f39 100644 --- a/storage/storage_integration_tests/storage_group_download_tests.cpp +++ b/storage/storage_integration_tests/storage_group_download_tests.cpp @@ -264,7 +264,7 @@ void TestDownloadDelete(bool downloadOneByOne, bool deleteOneByOne) storage.Init(onUpdatedFn, [](CountryId const &, storage::LocalFilePtr const) { return false; }); storage.RegisterAllLocalMaps(false /* enableDiffs */); - storage.SetDownloadingUrlsForTesting({kTestWebServer}); + storage.SetDownloadingServersForTesting({kTestWebServer}); tests_support::ScopedDir cleanupVersionDir(version); diff --git a/storage/storage_integration_tests/storage_http_tests.cpp b/storage/storage_integration_tests/storage_http_tests.cpp index 4b0ce0afbc..911c4f8547 100644 --- a/storage/storage_integration_tests/storage_http_tests.cpp +++ b/storage/storage_integration_tests/storage_http_tests.cpp @@ -70,7 +70,7 @@ void InitStorage(Storage & storage, Storage::UpdateCallback const & didDownload, storage.Init(didDownload, [](CountryId const &, LocalFilePtr const) { return false; }); storage.RegisterAllLocalMaps(false /* enableDiffs */); storage.Subscribe(changeCountryFunction, progress); - storage.SetDownloadingUrlsForTesting({kTestWebServer}); + storage.SetDownloadingServersForTesting({kTestWebServer}); } class StorageHttpTest diff --git a/storage/storage_integration_tests/storage_update_tests.cpp b/storage/storage_integration_tests/storage_update_tests.cpp index e320a89e63..7a10d47a74 100644 --- a/storage/storage_integration_tests/storage_update_tests.cpp +++ b/storage/storage_integration_tests/storage_update_tests.cpp @@ -99,7 +99,7 @@ UNIT_TEST(SmallMwms_Update_Test) testing::StopEventLoop(); }; storage.Subscribe(onChangeCountryFn, onProgressFn); - storage.SetDownloadingUrlsForTesting({kTestWebServer}); + storage.SetDownloadingServersForTesting({kTestWebServer}); CountriesVec children; storage.GetChildren(kGroupCountryId, children); @@ -136,7 +136,7 @@ UNIT_TEST(SmallMwms_Update_Test) testing::StopEventLoop(); }; storage.Subscribe(onChangeCountryFn, onProgressFn); - storage.SetDownloadingUrlsForTesting({kTestWebServer}); + storage.SetDownloadingServersForTesting({kTestWebServer}); CountriesVec children; storage.GetChildren(kGroupCountryId, children); diff --git a/storage/storage_tests/fake_map_files_downloader.cpp b/storage/storage_tests/fake_map_files_downloader.cpp index 00f213e968..666648afdf 100644 --- a/storage/storage_tests/fake_map_files_downloader.cpp +++ b/storage/storage_tests/fake_map_files_downloader.cpp @@ -5,33 +5,26 @@ #include "base/assert.hpp" #include "base/scope_guard.hpp" -#include "std/algorithm.hpp" -#include "std/bind.hpp" +#include +#include +#include namespace storage { int64_t const FakeMapFilesDownloader::kBlockSize; FakeMapFilesDownloader::FakeMapFilesDownloader(TaskRunner & taskRunner) - : m_progress(make_pair(0, 0)), m_idle(true), m_timestamp(0), m_taskRunner(taskRunner) + : m_progress(std::make_pair(0, 0)), m_idle(true), m_timestamp(0), m_taskRunner(taskRunner) { - m_servers.push_back("http://test-url/"); + SetServersList({"http://test-url/"}); } FakeMapFilesDownloader::~FakeMapFilesDownloader() { CHECK(m_checker.CalledOnOriginalThread(), ()); } -void FakeMapFilesDownloader::GetServersList(ServersListCallback const & callback) -{ - CHECK(m_checker.CalledOnOriginalThread(), ()); - m_idle = false; - SCOPE_GUARD(resetIdle, bind(&FakeMapFilesDownloader::Reset, this)); - callback(m_servers); -} - -void FakeMapFilesDownloader::DownloadMapFile(vector const & urls, string const & path, - int64_t size, - FileDownloadedCallback const & onDownloaded, - DownloadingProgressCallback const & onProgress) +void FakeMapFilesDownloader::Download(std::vector const & urls, + std::string const & path, int64_t size, + FileDownloadedCallback const & onDownloaded, + DownloadingProgressCallback const & onProgress) { CHECK(m_checker.CalledOnOriginalThread(), ()); @@ -44,7 +37,7 @@ void FakeMapFilesDownloader::DownloadMapFile(vector const & urls, string m_onProgress = onProgress; ++m_timestamp; - m_taskRunner.PostTask(bind(&FakeMapFilesDownloader::DownloadNextChunk, this, m_timestamp)); + m_taskRunner.PostTask(std::bind(&FakeMapFilesDownloader::DownloadNextChunk, this, m_timestamp)); } MapFilesDownloader::Progress FakeMapFilesDownloader::GetDownloadingProgress() @@ -71,7 +64,7 @@ void FakeMapFilesDownloader::DownloadNextChunk(uint64_t timestamp) { CHECK(m_checker.CalledOnOriginalThread(), ()); - static string kZeroes(kBlockSize, '\0'); + static std::string kZeroes(kBlockSize, '\0'); if (timestamp != m_timestamp) return; @@ -86,13 +79,13 @@ void FakeMapFilesDownloader::DownloadNextChunk(uint64_t timestamp) return; } - int64_t const bs = min(m_progress.second - m_progress.first, kBlockSize); + int64_t const bs = std::min(m_progress.second - m_progress.first, kBlockSize); m_progress.first += bs; m_writer->Write(kZeroes.data(), bs); m_writer->Flush(); m_taskRunner.PostTask(bind(m_onProgress, m_progress)); - m_taskRunner.PostTask(bind(&FakeMapFilesDownloader::DownloadNextChunk, this, timestamp)); + m_taskRunner.PostTask(std::bind(&FakeMapFilesDownloader::DownloadNextChunk, this, timestamp)); } } // namespace storage diff --git a/storage/storage_tests/fake_map_files_downloader.hpp b/storage/storage_tests/fake_map_files_downloader.hpp index f3ee837b67..cfb4ae0ccf 100644 --- a/storage/storage_tests/fake_map_files_downloader.hpp +++ b/storage/storage_tests/fake_map_files_downloader.hpp @@ -1,9 +1,14 @@ #pragma once #include "storage/map_files_downloader.hpp" + #include "coding/file_writer.hpp" + #include "base/thread_checker.hpp" -#include "std/unique_ptr.hpp" + +#include +#include +#include namespace storage { @@ -23,26 +28,24 @@ public: FakeMapFilesDownloader(TaskRunner & taskRunner); - virtual ~FakeMapFilesDownloader(); + ~FakeMapFilesDownloader(); - // MapFilesDownloader overrides: - void GetServersList(ServersListCallback const & callback) override; - - void DownloadMapFile(vector const & urls, string const & path, int64_t size, - FileDownloadedCallback const & onDownloaded, - DownloadingProgressCallback const & onProgress) override; Progress GetDownloadingProgress() override; bool IsIdle() override; void Reset() override; private: + // MapFilesDownloader overrides: + void Download(std::vector const & urls, std::string const & path, int64_t size, + FileDownloadedCallback const & onDownloaded, + DownloadingProgressCallback const & onProgress) override; + void DownloadNextChunk(uint64_t requestId); - vector m_servers; Progress m_progress; bool m_idle; - unique_ptr m_writer; + std::unique_ptr m_writer; FileDownloadedCallback m_onDownloaded; DownloadingProgressCallback m_onProgress; diff --git a/storage/storage_tests/storage_tests.cpp b/storage/storage_tests/storage_tests.cpp index 2576898ada..fb41227597 100644 --- a/storage/storage_tests/storage_tests.cpp +++ b/storage/storage_tests/storage_tests.cpp @@ -608,6 +608,7 @@ UNIT_TEST(StorageTest_Smoke) UNIT_CLASS_TEST(StorageTest, CountryDownloading) { + Platform::ThreadRunner m_runner; CountryId const azerbaijanCountryId = storage.FindCountryIdByFile("Azerbaijan"); TEST(IsCountryIdValid(azerbaijanCountryId), ()); @@ -969,6 +970,7 @@ UNIT_CLASS_TEST(TwoComponentStorageTest, DeleteCountry) UNIT_TEST(StorageTest_FailedDownloading) { + Platform::ThreadRunner m_runner; Storage storage; storage.Init(&OnCountryDownloaded, [](CountryId const &, LocalFilePtr const) { return false; }); storage.SetDownloaderForTesting(make_unique()); @@ -1000,6 +1002,7 @@ UNIT_TEST(StorageTest_FailedDownloading) UNIT_TEST(StorageTest_ObsoleteMapsRemoval) { + Platform::ThreadRunner m_runner; Storage storage; CountryFile country("Azerbaijan"); diff --git a/storage/storage_tests/test_map_files_downloader.cpp b/storage/storage_tests/test_map_files_downloader.cpp index ebb3801741..50a1dee5d0 100644 --- a/storage/storage_tests/test_map_files_downloader.cpp +++ b/storage/storage_tests/test_map_files_downloader.cpp @@ -7,7 +7,6 @@ namespace storage { void TestMapFilesDownloader::GetServersList(ServersListCallback const & callback) { - vector urls = {"http://localhost:34568/unit_tests/"}; - callback(urls); + callback({"http://localhost:34568/unit_tests/"}); } } // namespace storage diff --git a/storage/storage_tests/test_map_files_downloader.hpp b/storage/storage_tests/test_map_files_downloader.hpp index 1b2ac332a6..1322e7987c 100644 --- a/storage/storage_tests/test_map_files_downloader.hpp +++ b/storage/storage_tests/test_map_files_downloader.hpp @@ -6,7 +6,7 @@ namespace storage { class TestMapFilesDownloader : public HttpMapFilesDownloader { -public: +private: // MapFilesDownloader overrides: void GetServersList(ServersListCallback const & callback) override; };