From 81273226590e76f46cdc64869f708e6d609aa177 Mon Sep 17 00:00:00 2001 From: Arsentiy Milchakov Date: Fri, 27 Sep 2019 13:30:43 +0300 Subject: [PATCH] [storage] got rid of support for two-component mwms from queued country --- platform/country_file.cpp | 1 + storage/queued_country.cpp | 45 +-------- storage/queued_country.hpp | 20 +--- storage/storage.cpp | 48 ++++------ storage/storage_tests/CMakeLists.txt | 1 - .../storage_tests/queued_country_tests.cpp | 93 ------------------- 6 files changed, 28 insertions(+), 180 deletions(-) delete mode 100644 storage/storage_tests/queued_country_tests.cpp diff --git a/platform/country_file.cpp b/platform/country_file.cpp index 9b64730e97..be970aa0b5 100644 --- a/platform/country_file.cpp +++ b/platform/country_file.cpp @@ -20,6 +20,7 @@ string GetNameWithExt(string const & countryFile, MapOptions file) switch (file) { case MapOptions::Map: + case MapOptions::MapWithCarRouting: return countryFile + DATA_FILE_EXTENSION; case MapOptions::CarRouting: return countryFile + DATA_FILE_EXTENSION + ROUTING_FILE_EXTENSION; diff --git a/storage/queued_country.cpp b/storage/queued_country.cpp index 3d7c3aa4d3..b4719eb7fc 100644 --- a/storage/queued_country.cpp +++ b/storage/queued_country.cpp @@ -5,53 +5,18 @@ namespace storage { QueuedCountry::QueuedCountry(CountryId const & countryId, MapOptions opt) - : m_countryId(countryId), m_init(opt), m_left(opt), m_current(MapOptions::Nothing) + : m_countryId(countryId), m_downloadingType(opt) { - // @TODO(bykoianko) Probably it's nessecary to check if InIndexInCountryTree here. ASSERT(IsCountryIdValid(GetCountryId()), ("Only valid countries may be downloaded.")); - ASSERT(m_left != MapOptions::Nothing, ("Empty file set was requested for downloading.")); - SwitchToNextFile(); } -void QueuedCountry::AddOptions(MapOptions opt) +void QueuedCountry::SetDownloadingType(MapOptions type) { - for (MapOptions file : {MapOptions::Map, MapOptions::CarRouting}) - { - if (HasOptions(opt, file) && !HasOptions(m_init, file)) - { - m_init = SetOptions(m_init, file); - m_left = SetOptions(m_left, file); - } - } + m_downloadingType = type; } -void QueuedCountry::RemoveOptions(MapOptions opt) +MapOptions QueuedCountry::GetDownloadingType() const { - for (MapOptions file : {MapOptions::Map, MapOptions::CarRouting, MapOptions::Diff}) - { - if (HasOptions(opt, file) && HasOptions(m_init, file)) - { - m_init = UnsetOptions(m_init, file); - m_left = UnsetOptions(m_left, file); - } - } - if (HasOptions(opt, m_current)) - m_current = LeastSignificantOption(m_left); -} - -void QueuedCountry::ResetToDefaultOptions() -{ - m_init = MapOptions::MapWithCarRouting; - m_left = MapOptions::MapWithCarRouting; - m_current = LeastSignificantOption(m_left); -} - -bool QueuedCountry::SwitchToNextFile() -{ - ASSERT(HasOptions(m_left, m_current), - ("Current file (", m_current, ") is not specified in left files (", m_left, ").")); - m_left = UnsetOptions(m_left, m_current); - m_current = LeastSignificantOption(m_left); - return m_current != MapOptions::Nothing; + return m_downloadingType; } } // namespace storage diff --git a/storage/queued_country.hpp b/storage/queued_country.hpp index fc574106ad..4499dd472e 100644 --- a/storage/queued_country.hpp +++ b/storage/queued_country.hpp @@ -6,32 +6,20 @@ namespace storage { -/// Country queued for downloading. -/// @TODO(bykoianko) This class assumes that a map may consist of one or two mwm files. -/// But single mwm files are used now. So this class should be redisigned to support -/// only single mwm case. class QueuedCountry { public: QueuedCountry(CountryId const & m_countryId, MapOptions opt); - void AddOptions(MapOptions opt); - void RemoveOptions(MapOptions opt); - /// In case we can't update file using diff scheme. - void ResetToDefaultOptions(); - bool SwitchToNextFile(); + void SetDownloadingType(MapOptions type); + MapOptions GetDownloadingType() const; - inline CountryId const & GetCountryId() const { return m_countryId; } - inline MapOptions GetInitOptions() const { return m_init; } - inline MapOptions GetCurrentFileOptions() const { return m_current; } - inline MapOptions GetDownloadedFilesOptions() const { return UnsetOptions(m_init, m_left); } + CountryId const & GetCountryId() const { return m_countryId; } inline bool operator==(CountryId const & countryId) const { return m_countryId == countryId; } private: CountryId m_countryId; - MapOptions m_init; - MapOptions m_left; - MapOptions m_current; + MapOptions m_downloadingType; }; } // namespace storage diff --git a/storage/storage.cpp b/storage/storage.cpp index 80d15afe36..2a50bf36b9 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -405,7 +405,7 @@ LocalAndRemoteSize Storage::CountrySizeInBytes(CountryId const & countryId, MapO if (!m_downloader->IsIdle() && IsCountryFirstInQueue(countryId)) { sizes.first = m_downloader->GetDownloadingProgress().first + - GetRemoteSize(countryFile, queuedCountry->GetDownloadedFilesOptions(), + GetRemoteSize(countryFile, queuedCountry->GetDownloadingType(), GetCurrentDataVersion()); } return sizes; @@ -523,7 +523,7 @@ void Storage::SaveDownloadQueue() ostringstream update; for (auto const & item : m_queue) { - auto & ss = item.GetInitOptions() == MapOptions::Diff ? update : download; + auto & ss = item.GetDownloadingType() == MapOptions::Diff ? update : download; ss << (ss.str().empty() ? "" : ";") << item.GetCountryId(); } @@ -567,11 +567,8 @@ void Storage::DownloadCountry(CountryId const & countryId, MapOptions opt) if (opt == MapOptions::Nothing) return; - if (QueuedCountry * queuedCountry = FindCountryInQueue(countryId)) - { - queuedCountry->AddOptions(opt); + if (FindCountryInQueue(countryId) != nullptr) return; - } m_failedCountries.erase(countryId); m_queue.push_back(QueuedCountry(countryId, opt)); @@ -681,7 +678,7 @@ void Storage::DownloadNextCountryFromQueue() if (stopDownload || !PreparePlaceForCountryFiles(GetCurrentDataVersion(), m_dataDir, GetCountryFile(countryId))) { - OnMapDownloadFinished(countryId, HttpRequest::Status::Failed, queuedCountry.GetInitOptions()); + OnMapDownloadFinished(countryId, HttpRequest::Status::Failed, queuedCountry.GetDownloadingType()); return; } @@ -694,7 +691,7 @@ void Storage::DownloadNextCountryFromQueue() void Storage::DownloadNextFile(QueuedCountry const & country) { CountryId const & countryId = country.GetCountryId(); - auto const opt = country.GetCurrentFileOptions(); + auto const opt = country.GetDownloadingType(); string const readyFilePath = GetFileDownloadPath(countryId, opt); uint64_t size; @@ -805,7 +802,7 @@ void Storage::OnMapFileDownloadFinished(HttpRequest::Status status, // Send statistics to PushWoosh. We send these statistics only for the newly downloaded // mwms, not for updated ones. - if (success && queuedCountry.GetInitOptions() != MapOptions::Diff) + if (success && queuedCountry.GetDownloadingType() != MapOptions::Diff) { auto const it = m_localFiles.find(countryId); if (it == m_localFiles.end()) @@ -818,7 +815,7 @@ void Storage::OnMapFileDownloadFinished(HttpRequest::Status status, } } - OnMapDownloadFinished(countryId, status, queuedCountry.GetInitOptions()); + OnMapDownloadFinished(countryId, status, queuedCountry.GetDownloadingType()); } void Storage::ReportProgress(CountryId const & countryId, MapFilesDownloader::Progress const & p) @@ -861,7 +858,7 @@ void Storage::DoDownload() return; QueuedCountry & queuedCountry = m_queue.front(); - if (queuedCountry.GetInitOptions() == MapOptions::Diff) + if (queuedCountry.GetDownloadingType() == MapOptions::Diff) { using diffs::Status; auto const status = m_diffManager.GetStatus(); @@ -869,17 +866,17 @@ void Storage::DoDownload() { case Status::Undefined: SetDeferDownloading(); return; case Status::NotAvailable: - queuedCountry.ResetToDefaultOptions(); + queuedCountry.SetDownloadingType(MapOptions::Map); break; case Status::Available: if (!m_diffManager.HasDiffFor(queuedCountry.GetCountryId())) - queuedCountry.ResetToDefaultOptions(); + queuedCountry.SetDownloadingType(MapOptions::Map); break; } } auto const & id = queuedCountry.GetCountryId(); - auto const options = queuedCountry.GetCurrentFileOptions(); + auto const options = queuedCountry.GetDownloadingType(); auto const relativeUrl = GetDownloadRelativeUrl(id, options); auto const filePath = GetFileDownloadPath(id, options); @@ -1243,27 +1240,18 @@ bool Storage::DeleteCountryFilesFromDownloader(CountryId const & countryId) m_diffsBeingApplied.erase(countryId); } - MapOptions const opt = queuedCountry->GetInitOptions(); if (IsCountryFirstInQueue(countryId)) { - // Abrupt downloading of the current file if it should be removed. - if (HasOptions(opt, queuedCountry->GetCurrentFileOptions())) - m_downloader->Reset(); + m_downloader->Reset(); // Remove all files the downloader has created for the country. DeleteDownloaderFilesForCountry(GetCurrentDataVersion(), m_dataDir, GetCountryFile(countryId)); } - queuedCountry->RemoveOptions(opt); - - // Remove country from the queue if there's nothing to download. - if (queuedCountry->GetInitOptions() == MapOptions::Nothing) - { - auto it = find(m_queue.begin(), m_queue.end(), countryId); - ASSERT(it != m_queue.end(), ()); - PopFromQueue(it); - SaveDownloadQueue(); - } + auto it = find(m_queue.begin(), m_queue.end(), countryId); + ASSERT(it != m_queue.end(), ()); + PopFromQueue(it); + SaveDownloadQueue(); if (!m_queue.empty() && m_downloader->IsIdle()) { @@ -1280,14 +1268,14 @@ uint64_t Storage::GetDownloadSize(QueuedCountry const & queuedCountry) const { CountryId const & countryId = queuedCountry.GetCountryId(); uint64_t size; - if (queuedCountry.GetInitOptions() == MapOptions::Diff) + if (queuedCountry.GetDownloadingType() == MapOptions::Diff) { CHECK(m_diffManager.SizeToDownloadFor(countryId, size), ()); return size; } CountryFile const & file = GetCountryFile(countryId); - return GetRemoteSize(file, queuedCountry.GetCurrentFileOptions(), GetCurrentDataVersion()); + return GetRemoteSize(file, queuedCountry.GetDownloadingType(), GetCurrentDataVersion()); } string Storage::GetFileDownloadPath(CountryId const & countryId, MapOptions options) const diff --git a/storage/storage_tests/CMakeLists.txt b/storage/storage_tests/CMakeLists.txt index 80528bb434..7bc7ab069a 100644 --- a/storage/storage_tests/CMakeLists.txt +++ b/storage/storage_tests/CMakeLists.txt @@ -10,7 +10,6 @@ set( fake_map_files_downloader.hpp helpers.cpp helpers.hpp - queued_country_tests.cpp simple_tree_test.cpp storage_tests.cpp task_runner.cpp diff --git a/storage/storage_tests/queued_country_tests.cpp b/storage/storage_tests/queued_country_tests.cpp deleted file mode 100644 index bf34d586f9..0000000000 --- a/storage/storage_tests/queued_country_tests.cpp +++ /dev/null @@ -1,93 +0,0 @@ -#include "testing/testing.hpp" - -#include "storage/queued_country.hpp" -#include "storage/storage.hpp" - -namespace storage -{ -UNIT_TEST(QueuedCountry_AddOptions) -{ - Storage storage; - CountryId const countryId = storage.FindCountryIdByFile("USA_Georgia"); - QueuedCountry country(countryId, MapOptions::CarRouting); - - TEST_EQUAL(countryId, country.GetCountryId(), ()); - TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ()); - - country.AddOptions(MapOptions::Map); - TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ()); - - TEST(country.SwitchToNextFile(), ()); - TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ()); - - TEST(!country.SwitchToNextFile(), ()); - TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ()); -} - -UNIT_TEST(QueuedCountry_RemoveOptions) -{ - Storage storage; - CountryId const countryId = storage.FindCountryIdByFile("USA_Georgia"); - - { - QueuedCountry country(countryId, MapOptions::MapWithCarRouting); - TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ()); - - country.RemoveOptions(MapOptions::Map); - TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ()); - - country.RemoveOptions(MapOptions::CarRouting); - TEST_EQUAL(MapOptions::Nothing, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ()); - } - - { - QueuedCountry country(countryId, MapOptions::MapWithCarRouting); - TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ()); - - country.SwitchToNextFile(); - TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::Map, country.GetDownloadedFilesOptions(), ()); - - country.RemoveOptions(MapOptions::CarRouting); - TEST_EQUAL(MapOptions::Map, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::Map, country.GetDownloadedFilesOptions(), ()); - } - - { - QueuedCountry country(countryId, MapOptions::MapWithCarRouting); - TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ()); - - country.SwitchToNextFile(); - TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::Map, country.GetDownloadedFilesOptions(), ()); - - country.RemoveOptions(MapOptions::Map); - TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ()); - - country.SwitchToNextFile(); - TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ()); - TEST_EQUAL(MapOptions::CarRouting, country.GetDownloadedFilesOptions(), ()); - } -} -} // namespace storage