diff --git a/search/search_integration_tests/downloader_search_test.cpp b/search/search_integration_tests/downloader_search_test.cpp index 214a3bb11f..1605587188 100644 --- a/search/search_integration_tests/downloader_search_test.cpp +++ b/search/search_integration_tests/downloader_search_test.cpp @@ -11,6 +11,7 @@ #include "storage/downloader_search_params.hpp" #include "storage/map_files_downloader.hpp" +#include "storage/queued_country.hpp" #include "storage/storage.hpp" #include "geometry/rect2d.hpp" @@ -92,7 +93,7 @@ public: private: void GetServersList(ServersListCallback const & /* callback */) override {} - void Download(vector const & /* urls */, string const & /* path */, int64_t /* size */, + void Download(storage::QueuedCountry & queuedCountry, FileDownloadedCallback const & /* onDownloaded */, DownloadingProgressCallback const & /* onProgress */) override { diff --git a/storage/diff_scheme/diff_types.hpp b/storage/diff_scheme/diff_types.hpp index 06310f8247..9c2a3ac0d6 100644 --- a/storage/diff_scheme/diff_types.hpp +++ b/storage/diff_scheme/diff_types.hpp @@ -13,7 +13,6 @@ namespace diffs // Status of the diffs data source as a whole. enum class Status { - Undefined, NotAvailable, Available }; diff --git a/storage/diff_scheme/diffs_data_source.hpp b/storage/diff_scheme/diffs_data_source.hpp index 2827ce9aff..3816097587 100644 --- a/storage/diff_scheme/diffs_data_source.hpp +++ b/storage/diff_scheme/diffs_data_source.hpp @@ -58,7 +58,7 @@ private: ThreadChecker m_threadChecker; - Status m_status = Status::Undefined; + Status m_status = Status::NotAvailable; NameDiffInfoMap m_diffs; }; } // namespace diffs diff --git a/storage/http_map_files_downloader.cpp b/storage/http_map_files_downloader.cpp index 920dbe8f98..bd849b2f4a 100644 --- a/storage/http_map_files_downloader.cpp +++ b/storage/http_map_files_downloader.cpp @@ -34,12 +34,16 @@ HttpMapFilesDownloader::~HttpMapFilesDownloader() CHECK_THREAD_CHECKER(m_checker, ()); } -void HttpMapFilesDownloader::Download(std::vector const & urls, - std::string const & path, int64_t size, +void HttpMapFilesDownloader::Download(QueuedCountry & queuedCountry, FileDownloadedCallback const & onDownloaded, DownloadingProgressCallback const & onProgress) { CHECK_THREAD_CHECKER(m_checker, ()); + + auto const urls = MakeUrlList(queuedCountry.GetRelativeUrl()); + auto const path = queuedCountry.GetFileDownloadPath(); + auto const size = queuedCountry.GetDownloadSize(); + m_request.reset(downloader::HttpRequest::GetFile( urls, path, size, std::bind(&HttpMapFilesDownloader::OnMapFileDownloaded, this, onDownloaded, _1), diff --git a/storage/http_map_files_downloader.hpp b/storage/http_map_files_downloader.hpp index b95e77b777..6ed0fdee16 100644 --- a/storage/http_map_files_downloader.hpp +++ b/storage/http_map_files_downloader.hpp @@ -29,8 +29,7 @@ public: private: // MapFilesDownloaderWithServerList overrides: - void Download(std::vector const & urls, std::string const & path, int64_t size, - FileDownloadedCallback const & onDownloaded, + void Download(QueuedCountry & queuedCountry, FileDownloadedCallback const & onDownloaded, DownloadingProgressCallback const & onProgress) override; void OnMapFileDownloaded(FileDownloadedCallback const & onDownloaded, diff --git a/storage/map_files_downloader.cpp b/storage/map_files_downloader.cpp index 050dd20a2d..4a3e6e1fcf 100644 --- a/storage/map_files_downloader.cpp +++ b/storage/map_files_downloader.cpp @@ -16,40 +16,23 @@ 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(QueuedCountry & country, +void MapFilesDownloader::DownloadMapFile(QueuedCountry & queuedCountry, FileDownloadedCallback const & onDownloaded, DownloadingProgressCallback const & onProgress) { - if (m_serversList.empty()) + if (!m_serversList.empty()) { - GetServersList([=](ServersList const & serversList) - { - m_serversList = serversList; - auto const urls = MakeUrlList(m_serversList, country.GetRelativeUrl()); - Download(urls, country.GetFileDownloadPath(), country.GetDownloadSize(), - onDownloaded, onProgress); - }); - } - else - { - auto const urls = MakeUrlList(m_serversList, country.GetRelativeUrl()); - Download(urls, country.GetFileDownloadPath(), country.GetDownloadSize(), onDownloaded, - onProgress); + queuedCountry.ClarifyDownloadingType(); + Download(queuedCountry, onDownloaded, onProgress); + return; } + + GetServersList([=](ServersList const & serversList) mutable + { + m_serversList = serversList; + queuedCountry.ClarifyDownloadingType(); + Download(queuedCountry, onDownloaded, onProgress); + }); } // static @@ -65,6 +48,16 @@ void MapFilesDownloader::SetServersList(ServersList const & serversList) m_serversList = serversList; } +std::vector MapFilesDownloader::MakeUrlList(std::string const & relativeUrl) +{ + std::vector urls; + urls.reserve(m_serversList.size()); + for (auto const & server : m_serversList) + urls.emplace_back(base::url::Join(server, relativeUrl)); + + return urls; +} + // static MapFilesDownloader::ServersList MapFilesDownloader::LoadServersList() { diff --git a/storage/map_files_downloader.hpp b/storage/map_files_downloader.hpp index b96e6517f8..94a4f6e52c 100644 --- a/storage/map_files_downloader.hpp +++ b/storage/map_files_downloader.hpp @@ -51,6 +51,8 @@ public: void SetServersList(ServersList const & serversList); protected: + std::vector MakeUrlList(std::string const & relativeUrl); + // Synchronously loads list of servers by http client. static ServersList LoadServersList(); @@ -59,8 +61,7 @@ private: /// for a map file and invokes callback on the main thread. virtual void GetServersList(ServersListCallback const & callback); /// Asynchronously downloads the file from provided |urls| and saves result to |path|. - virtual void Download(std::vector const & urls, std::string const & path, - int64_t size, FileDownloadedCallback const & onDownloaded, + virtual void Download(QueuedCountry & queuedCountry, FileDownloadedCallback const & onDownloaded, DownloadingProgressCallback const & onProgress) = 0; ServersList m_serversList; diff --git a/storage/queued_country.cpp b/storage/queued_country.cpp index 9d0350399f..83c98f5227 100644 --- a/storage/queued_country.cpp +++ b/storage/queued_country.cpp @@ -83,6 +83,20 @@ uint64_t QueuedCountry::GetDownloadSize() const return GetRemoteSize(*m_diffsDataSource, m_countryFile, m_currentDataVersion); } +void QueuedCountry::ClarifyDownloadingType() +{ + if (m_fileType != MapFileType::Diff) + return; + + using diffs::Status; + auto const status = m_diffsDataSource->GetStatus(); + if (status == Status::NotAvailable || + (status == Status::Available && !m_diffsDataSource->HasDiffFor(m_countryId))) + { + m_fileType = MapFileType::Map; + } +} + bool QueuedCountry::operator==(CountryId const & countryId) const { return m_countryId == countryId; diff --git a/storage/queued_country.hpp b/storage/queued_country.hpp index a9130c6c66..0b7e62ed79 100644 --- a/storage/queued_country.hpp +++ b/storage/queued_country.hpp @@ -26,6 +26,8 @@ public: std::string GetFileDownloadPath() const; uint64_t GetDownloadSize() const; + void ClarifyDownloadingType(); + bool operator==(CountryId const & countryId) const; private: diff --git a/storage/storage.cpp b/storage/storage.cpp index 379122ced6..278ec14ce1 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -646,7 +646,12 @@ void Storage::DownloadNextFile(QueuedCountry const & country) return; } - DoDownload(); + if (!m_queue.empty()) + { + m_downloader->DownloadMapFile(m_queue.front(), + bind(&Storage::OnMapFileDownloadFinished, this, _1, _2), + bind(&Storage::OnMapFileDownloadProgress, this, _1)); + } } bool Storage::IsDownloadInProgress() const @@ -766,57 +771,6 @@ void Storage::ReportProgressForHierarchy(CountryId const & countryId, ForEachAncestorExceptForTheRoot(countryId, calcProgress); } -void Storage::DoDownload() -{ - CHECK_THREAD_CHECKER(m_threadChecker, ()); - - // Queue can be empty because countries were deleted from queue. - if (m_queue.empty()) - return; - - QueuedCountry & queuedCountry = m_queue.front(); - if (queuedCountry.GetFileType() == MapFileType::Diff) - { - using diffs::Status; - auto const status = m_diffsDataSource->GetStatus(); - switch (status) - { - case Status::Undefined: - SetDeferDownloading(); - return; - case Status::NotAvailable: - queuedCountry.SetFileType(MapFileType::Map); - break; - case Status::Available: - if (!m_diffsDataSource->HasDiffFor(queuedCountry.GetCountryId())) - queuedCountry.SetFileType(MapFileType::Map); - break; - } - } - - m_downloader->DownloadMapFile(queuedCountry, - bind(&Storage::OnMapFileDownloadFinished, this, _1, _2), - bind(&Storage::OnMapFileDownloadProgress, this, _1)); -} - -void Storage::SetDeferDownloading() -{ - CHECK_THREAD_CHECKER(m_threadChecker, ()); - - m_needToStartDeferredDownloading = true; -} - -void Storage::DoDeferredDownloadIfNeeded() -{ - CHECK_THREAD_CHECKER(m_threadChecker, ()); - - if (!m_needToStartDeferredDownloading) - return; - - m_needToStartDeferredDownloading = false; - DoDownload(); -} - void Storage::OnMapFileDownloadProgress(MapFilesDownloader::Progress const & progress) { CHECK_THREAD_CHECKER(m_threadChecker, ()); @@ -1474,8 +1428,6 @@ void Storage::OnDiffStatusReceived(diffs::NameDiffInfoMap && diffs) m_notAppliedDiffs.clear(); } - - DoDeferredDownloadIfNeeded(); } StatusAndError Storage::GetNodeStatusInfo(CountryTree::Node const & node, diff --git a/storage/storage.hpp b/storage/storage.hpp index 0d0a702be8..3a0b6b5282 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -278,10 +278,6 @@ private: void ReportProgressForHierarchy(CountryId const & countryId, MapFilesDownloader::Progress const & leafProgress); - void DoDownload(); - void SetDeferDownloading(); - void DoDeferredDownloadIfNeeded(); - /// Called on the main thread by MapFilesDownloader when /// downloading of a map file succeeds/fails. void OnMapFileDownloadFinished(downloader::HttpRequest::Status status, diff --git a/storage/storage_tests/fake_map_files_downloader.cpp b/storage/storage_tests/fake_map_files_downloader.cpp index ef4568fccb..f5d6cd07ac 100644 --- a/storage/storage_tests/fake_map_files_downloader.cpp +++ b/storage/storage_tests/fake_map_files_downloader.cpp @@ -21,23 +21,23 @@ FakeMapFilesDownloader::FakeMapFilesDownloader(TaskRunner & taskRunner) FakeMapFilesDownloader::~FakeMapFilesDownloader() { CHECK(m_checker.CalledOnOriginalThread(), ()); } -void FakeMapFilesDownloader::Download(std::vector const & urls, - std::string const & path, int64_t size, +void FakeMapFilesDownloader::Download(QueuedCountry & queuedCountry, FileDownloadedCallback const & onDownloaded, DownloadingProgressCallback const & onProgress) { - CHECK(m_checker.CalledOnOriginalThread(), ()); - - m_progress.first = 0; - m_progress.second = size; - m_idle = false; - - m_writer.reset(new FileWriter(path)); - m_onDownloaded = onDownloaded; - m_onProgress = onProgress; - - ++m_timestamp; - m_taskRunner.PostTask(std::bind(&FakeMapFilesDownloader::DownloadNextChunk, this, m_timestamp)); +// Will be refactored in the followed commits. +// CHECK(m_checker.CalledOnOriginalThread(), ()); +// +// m_progress.first = 0; +// m_progress.second = size; +// m_idle = false; +// +// m_writer.reset(new FileWriter(path)); +// m_onDownloaded = onDownloaded; +// m_onProgress = onProgress; +// +// ++m_timestamp; +// m_taskRunner.PostTask(std::bind(&FakeMapFilesDownloader::DownloadNextChunk, this, m_timestamp)); } MapFilesDownloader::Progress FakeMapFilesDownloader::GetDownloadingProgress() diff --git a/storage/storage_tests/fake_map_files_downloader.hpp b/storage/storage_tests/fake_map_files_downloader.hpp index cfb4ae0ccf..ebe6f5b05c 100644 --- a/storage/storage_tests/fake_map_files_downloader.hpp +++ b/storage/storage_tests/fake_map_files_downloader.hpp @@ -1,6 +1,7 @@ #pragma once #include "storage/map_files_downloader.hpp" +#include "storage/queued_country.hpp" #include "coding/file_writer.hpp" @@ -36,7 +37,7 @@ public: private: // MapFilesDownloader overrides: - void Download(std::vector const & urls, std::string const & path, int64_t size, + void Download(QueuedCountry & queuedCountry, FileDownloadedCallback const & onDownloaded, DownloadingProgressCallback const & onProgress) override;