From 912dc82ad0797f252a12fe5e354947ad37bd1afc Mon Sep 17 00:00:00 2001 From: Sergey Yershov Date: Wed, 27 Apr 2016 17:38:07 +0300 Subject: [PATCH] [downloader] Refactor logic for disable downloading on cellular network and retry download on lost connection --- .../MWMDownloadTransitMapAlert.mm | 3 +- .../Classes/Widgets/MWMMapDownloadDialog.mm | 4 ++- map/framework.hpp | 3 ++ storage/downloading_policy.cpp | 27 ++++++++++++++ storage/downloading_policy.hpp | 36 +++++++++++++++++++ storage/storage.cpp | 36 +++++++------------ storage/storage.hpp | 12 +++---- storage/storage.pro | 2 ++ .../storage/storage.xcodeproj/project.pbxproj | 8 +++++ 9 files changed, 100 insertions(+), 31 deletions(-) create mode 100644 storage/downloading_policy.cpp create mode 100644 storage/downloading_policy.hpp diff --git a/iphone/Maps/Classes/CustomAlert/DownloadTransitMapsAlert/MWMDownloadTransitMapAlert.mm b/iphone/Maps/Classes/CustomAlert/DownloadTransitMapsAlert/MWMDownloadTransitMapAlert.mm index e01129d248..78c421ef10 100644 --- a/iphone/Maps/Classes/CustomAlert/DownloadTransitMapsAlert/MWMDownloadTransitMapAlert.mm +++ b/iphone/Maps/Classes/CustomAlert/DownloadTransitMapsAlert/MWMDownloadTransitMapAlert.mm @@ -135,9 +135,10 @@ CGFloat const kAnimationDuration = .05; - (void)processCountryEvent:(TCountryId const &)countryId { auto const & s = GetFramework().Storage(); + auto const & p = GetFramework().DownloadingPolicy(); if (s.CheckFailedCountries(m_countries)) { - if (s.IsAutoRetryDownloadFailed()) + if (p.IsAutoRetryDownloadFailed()) [self close]; return; } diff --git a/iphone/Maps/Classes/Widgets/MWMMapDownloadDialog.mm b/iphone/Maps/Classes/Widgets/MWMMapDownloadDialog.mm index 684865dea6..f7ee13dd90 100644 --- a/iphone/Maps/Classes/Widgets/MWMMapDownloadDialog.mm +++ b/iphone/Maps/Classes/Widgets/MWMMapDownloadDialog.mm @@ -95,6 +95,8 @@ using namespace storage; - (void)configDialog { auto & s = GetFramework().Storage(); + auto & p = GetFramework().DownloadingPolicy(); + NodeAttrs nodeAttrs; s.GetNodeAttrs(m_countryId, nodeAttrs); @@ -151,7 +153,7 @@ using namespace storage; break; case NodeStatus::Undefined: case NodeStatus::Error: - if (s.IsAutoRetryDownloadFailed()) + if (p.IsAutoRetryDownloadFailed()) [self showError:nodeAttrs.m_error]; break; case NodeStatus::OnDisk: diff --git a/map/framework.hpp b/map/framework.hpp index 37db171473..eef8dc7b89 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -22,6 +22,7 @@ #include "search/search_engine.hpp" #include "storage/downloader_search_params.hpp" +#include "storage/downloading_policy.hpp" #include "storage/storage.hpp" #include "platform/country_defines.hpp" @@ -128,6 +129,7 @@ protected: double m_startForegroundTime; double m_startBackgroundTime; + StorageDownloadingPolicy m_storageDownloadingPolicy; storage::Storage m_storage; location::TMyPositionModeChanged m_myPositionListener; @@ -219,6 +221,7 @@ public: storage::Storage & Storage() { return m_storage; } storage::Storage const & Storage() const { return m_storage; } storage::CountryInfoGetter & CountryInfoGetter() { return *m_infoGetter; } + StorageDownloadingPolicy & DownloadingPolicy() { return m_storageDownloadingPolicy; } /// @name Bookmarks, Tracks and other UserMarks //@{ diff --git a/storage/downloading_policy.cpp b/storage/downloading_policy.cpp new file mode 100644 index 0000000000..1a0bd68ae1 --- /dev/null +++ b/storage/downloading_policy.cpp @@ -0,0 +1,27 @@ +#include "storage/downloading_policy.hpp" + +#include "platform/platform.hpp" + +bool StorageDownloadingPolicy::IsDownloadingAllowed() const +{ + return !(GetPlatform().ConnectionStatus() == Platform::EConnectionType::CONNECTION_WWAN && + !IsCellularDownloadEnabled()); +} + +void StorageDownloadingPolicy::ProcessFailedCountries( + storage::TCountriesSet const & failedCountries, TProcessFunc const & func) +{ + if (IsDownloadingAllowed() && !failedCountries.empty() && m_autoRetryCounter > 0) + { + auto action = [this, func, failedCountries] + { + --m_autoRetryCounter; + func(failedCountries); + }; + m_autoRetryWorker.RestartWith([action]{ Platform().RunOnGuiThread(action); }); + } + else + { + m_autoRetryCounter = kAutoRetryCounterMax; + } +} diff --git a/storage/downloading_policy.hpp b/storage/downloading_policy.hpp new file mode 100644 index 0000000000..1322ecb345 --- /dev/null +++ b/storage/downloading_policy.hpp @@ -0,0 +1,36 @@ +#pragma once + +#include "storage/index.hpp" + +#include "base/deferred_task.hpp" + +#include "std/chrono.hpp" +#include "std/function.hpp" + +class DownloadingPolicy +{ +public: + using TProcessFunc = function; + + virtual bool IsDownloadingAllowed() const { return true; } + virtual void ScheduleRetry(storage::TCountriesSet const &, TProcessFunc const &) {} +}; + +class StorageDownloadingPolicy : public DownloadingPolicy +{ + bool m_cellularDownloadEnabled = false; + + static size_t constexpr kAutoRetryCounterMax = 3; + size_t m_autoRetryCounter = kAutoRetryCounterMax; + my::DeferredTask m_autoRetryWorker; + +public: + StorageDownloadingPolicy() : m_autoRetryWorker(seconds(20)) {} + + inline void EnableCellularDownload(bool value) { m_cellularDownloadEnabled = value; } + inline bool IsCellularDownloadEnabled() const { return m_cellularDownloadEnabled; } + inline bool IsAutoRetryDownloadFailed() const { return m_autoRetryCounter == 0; } + + bool IsDownloadingAllowed() const override; + void ScheduleRetry(storage::TCountriesSet const & failedCountries, TProcessFunc const & func) override; +}; diff --git a/storage/storage.cpp b/storage/storage.cpp index 070961742a..5a8f4b1ac1 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -111,7 +111,6 @@ MapFilesDownloader::TProgress Storage::GetOverallProgress(TCountriesVec const & Storage::Storage(string const & pathToCountriesFile /* = COUNTRIES_FILE */, string const & dataDir /* = string() */) : m_downloader(new HttpMapFilesDownloader()), m_currentSlotId(0), m_dataDir(dataDir) , m_downloadMapOnTheMap(nullptr) - , m_autoRetryWorker(seconds(20)) { SetLocale(languages::GetCurrentTwine()); LoadCountriesFile(pathToCountriesFile, m_dataDir); @@ -121,7 +120,6 @@ Storage::Storage(string const & referenceCountriesTxtJsonForTesting, unique_ptr mapDownloaderForTesting) : m_downloader(move(mapDownloaderForTesting)), m_currentSlotId(0) , m_downloadMapOnTheMap(nullptr) - , m_autoRetryWorker(seconds(20)) { m_currentVersion = LoadCountries(referenceCountriesTxtJsonForTesting, m_countries, m_affiliations); @@ -548,28 +546,20 @@ void Storage::DownloadNextCountryFromQueue() { ASSERT_THREAD_CHECKER(m_threadChecker, ()); + bool const stopDownload = !m_downloadingPolicy->IsDownloadingAllowed(); + if (m_queue.empty()) { - if (!m_failedCountries.empty() && m_autoRetryCounter > 0) - { - auto needReload = m_failedCountries; - auto action = [this, needReload] - { - --m_autoRetryCounter; - for (auto const & country : needReload) - { - NodeStatuses status; - GetNodeStatuses(country, status); - if (status.m_error == NodeErrorCode::NoInetConnection) - RetryDownloadNode(country); - } - }; - m_autoRetryWorker.RestartWith([action]{Platform().RunOnGuiThread(action);}); - } - else - { - m_autoRetryCounter = kAutoRetryCounterMax; - } + m_downloadingPolicy->ScheduleRetry(m_failedCountries, [this](TCountriesSet const & needReload) + { + for (auto const & country : needReload) + { + NodeStatuses status; + GetNodeStatuses(country, status); + if (status.m_error == NodeErrorCode::NoInetConnection) + RetryDownloadNode(country); + } + }); return; } @@ -579,7 +569,7 @@ void Storage::DownloadNextCountryFromQueue() // It's not even possible to prepare directory for files before // downloading. Mark this country as failed and switch to next // country. - if (!PreparePlaceForCountryFiles(GetCurrentDataVersion(), m_dataDir, GetCountryFile(countryId))) + if (stopDownload || !PreparePlaceForCountryFiles(GetCurrentDataVersion(), m_dataDir, GetCountryFile(countryId))) { OnMapDownloadFinished(countryId, false /* success */, queuedCountry.GetInitOptions()); NotifyStatusChangedForHierarchy(countryId); diff --git a/storage/storage.hpp b/storage/storage.hpp index 1b445b2e1d..f22381cefc 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -3,6 +3,7 @@ #include "storage/country.hpp" #include "storage/country_name_getter.hpp" #include "storage/country_tree.hpp" +#include "storage/downloading_policy.hpp" #include "storage/storage_defines.hpp" #include "storage/index.hpp" #include "storage/map_files_downloader.hpp" @@ -184,6 +185,9 @@ private: /// MapFilesDownloader::TProgress m_countryProgress; + DownloadingPolicy m_defaultDownloadingPolicy; + DownloadingPolicy * m_downloadingPolicy = &m_defaultDownloadingPolicy; + /// @name Communicate with GUI //@{ @@ -238,10 +242,6 @@ private: ThreadChecker m_threadChecker; - static size_t constexpr kAutoRetryCounterMax = 3; - size_t m_autoRetryCounter = kAutoRetryCounterMax; - my::DeferredTask m_autoRetryWorker; - void DownloadNextCountryFromQueue(); void LoadCountriesFile(string const & pathToCountriesFile, string const & dataDir, @@ -290,6 +290,8 @@ public: void Init(TUpdateCallback const & didDownload, TDeleteCallback const & willDelete); + inline void SetDownloadingPolicy(DownloadingPolicy * policy) { m_downloadingPolicy = policy; } + /// @name Interface with clients (Android/iOS). /// \brief It represents the interface which can be used by clients (Android/iOS). /// The term node means an mwm or a group of mwm like a big country. @@ -319,8 +321,6 @@ public: TOnStatusChangedCallback m_onStatusChanged; }; - inline bool IsAutoRetryDownloadFailed() const { return m_autoRetryCounter == 0; } - bool CheckFailedCountries(TCountriesVec const & countries) const; /// \brief Returns root country id of the country tree. diff --git a/storage/storage.pro b/storage/storage.pro index 1663684310..e0ac68c67f 100644 --- a/storage/storage.pro +++ b/storage/storage.pro @@ -18,6 +18,7 @@ HEADERS += \ country_polygon.hpp \ country_tree.hpp \ downloader_search_params.hpp \ + downloading_policy.hpp \ http_map_files_downloader.hpp \ index.hpp \ map_files_downloader.hpp \ @@ -31,6 +32,7 @@ SOURCES += \ country_decl.cpp \ country_info_getter.cpp \ country_name_getter.cpp \ + downloading_policy.cpp \ http_map_files_downloader.cpp \ index.cpp \ queued_country.cpp \ diff --git a/xcode/storage/storage.xcodeproj/project.pbxproj b/xcode/storage/storage.xcodeproj/project.pbxproj index 499474f640..dfe144fae1 100644 --- a/xcode/storage/storage.xcodeproj/project.pbxproj +++ b/xcode/storage/storage.xcodeproj/project.pbxproj @@ -62,6 +62,8 @@ 678CB3801CA5884800E9118D /* editor.config in Resources */ = {isa = PBXBuildFile; fileRef = 678CB37F1CA5881B00E9118D /* editor.config */; }; 67AF4A001BC579DD0048B1ED /* country_info_getter.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 67AF49FE1BC579DD0048B1ED /* country_info_getter.cpp */; }; 67AF4A011BC579DD0048B1ED /* country_info_getter.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 67AF49FF1BC579DD0048B1ED /* country_info_getter.hpp */; }; + 67BE1DC51CD2180D00572709 /* downloading_policy.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 67BE1DC31CD2180D00572709 /* downloading_policy.cpp */; }; + 67BE1DC61CD2180D00572709 /* downloading_policy.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 67BE1DC41CD2180D00572709 /* downloading_policy.hpp */; }; 67CCFA911CBD1628007FC750 /* Images.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 67CCFA901CBD1628007FC750 /* Images.xcassets */; }; 67F90B6F1C6A277900CD458E /* testingmain.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 67247FD71C60BA9900EDE56A /* testingmain.cpp */; }; 67F90B701C6A277900CD458E /* country_info_getter_test.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 67247FC31C60BA8A00EDE56A /* country_info_getter_test.cpp */; }; @@ -254,6 +256,8 @@ 678CB37F1CA5881B00E9118D /* editor.config */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = editor.config; sourceTree = ""; }; 67AF49FE1BC579DD0048B1ED /* country_info_getter.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = country_info_getter.cpp; sourceTree = ""; }; 67AF49FF1BC579DD0048B1ED /* country_info_getter.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = country_info_getter.hpp; sourceTree = ""; }; + 67BE1DC31CD2180D00572709 /* downloading_policy.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = downloading_policy.cpp; sourceTree = ""; }; + 67BE1DC41CD2180D00572709 /* downloading_policy.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = downloading_policy.hpp; sourceTree = ""; }; 67CCFA901CBD1628007FC750 /* Images.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; name = Images.xcassets; path = ../../iphone/Maps/Images.xcassets; sourceTree = SOURCE_ROOT; }; 67F90B581C6A275B00CD458E /* storage_tests.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = storage_tests.app; sourceTree = BUILT_PRODUCTS_DIR; }; 67F90BB61C6A29F700CD458E /* storage_integration_tests.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = storage_integration_tests.app; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -452,6 +456,8 @@ 675342E21A3F59EF00A0A8C3 /* storage */ = { isa = PBXGroup; children = ( + 67BE1DC31CD2180D00572709 /* downloading_policy.cpp */, + 67BE1DC41CD2180D00572709 /* downloading_policy.hpp */, 34C9BCFA1C6CCF85000DC38D /* country_name_getter.cpp */, 34C9BCFB1C6CCF85000DC38D /* country_name_getter.hpp */, 34B0931D1C61F9BA0066F4C3 /* storage_helpers.cpp */, @@ -518,6 +524,7 @@ 6753430D1A3F5A2600A0A8C3 /* country_polygon.hpp in Headers */, 34C9BCFD1C6CCF85000DC38D /* country_name_getter.hpp in Headers */, 6741251F1B4C05FA00A3E828 /* http_map_files_downloader.hpp in Headers */, + 67BE1DC61CD2180D00572709 /* downloading_policy.hpp in Headers */, 675343191A3F5A2600A0A8C3 /* storage_defines.hpp in Headers */, 67247FD41C60BA8A00EDE56A /* task_runner.hpp in Headers */, 6753430F1A3F5A2600A0A8C3 /* country.hpp in Headers */, @@ -696,6 +703,7 @@ 675343161A3F5A2600A0A8C3 /* index.cpp in Sources */, 678338D51C723B1A00FD6263 /* helpers.cpp in Sources */, 674125231B4C05FA00A3E828 /* storage_defines.cpp in Sources */, + 67BE1DC51CD2180D00572709 /* downloading_policy.cpp in Sources */, 678338D41C723B1A00FD6263 /* country_name_getter_test.cpp in Sources */, 34C9BCFC1C6CCF85000DC38D /* country_name_getter.cpp in Sources */, 6741251E1B4C05FA00A3E828 /* http_map_files_downloader.cpp in Sources */,