diff options
author | Maxim Pimenov <m@maps.me> | 2019-02-04 13:23:34 +0300 |
---|---|---|
committer | Vlad Mihaylenko <vxmihaylenko@gmail.com> | 2019-02-13 16:10:22 +0300 |
commit | 971f63eb5e1203de50a72d20614e2f12143f5496 (patch) | |
tree | 5786658bac9e56294db3b498dd60c43ed63b1440 /storage | |
parent | a94ac8da5bf643bd28a263d5fa41e35e9b769f53 (diff) |
Review fixes.
Diffstat (limited to 'storage')
-rw-r--r-- | storage/diff_scheme/diff_manager.cpp | 30 | ||||
-rw-r--r-- | storage/diff_scheme/diff_manager.hpp | 18 | ||||
-rw-r--r-- | storage/diff_scheme/diff_types.hpp | 13 | ||||
-rw-r--r-- | storage/storage.cpp | 6 |
4 files changed, 40 insertions, 27 deletions
diff --git a/storage/diff_scheme/diff_manager.cpp b/storage/diff_scheme/diff_manager.cpp index 4609c839c0..9f4e78cf46 100644 --- a/storage/diff_scheme/diff_manager.cpp +++ b/storage/diff_scheme/diff_manager.cpp @@ -45,9 +45,8 @@ void Manager::Load(LocalMapsInfo && info) }); } -void Manager::ApplyDiff( - ApplyDiffParams && p, base::Cancellable const & cancellable, - std::function<void(generator::mwm_diff::DiffApplicationResult result)> const & task) +void Manager::ApplyDiff(ApplyDiffParams && p, base::Cancellable const & cancellable, + Manager::OnDiffApplicationFinished const & task) { using namespace generator::mwm_diff; @@ -58,7 +57,7 @@ void Manager::ApplyDiff( auto & diffReadyPath = p.m_diffReadyPath; auto & diffFile = p.m_diffFile; auto const diffPath = diffFile->GetPath(MapOptions::Diff); - DiffApplicationResult result = DiffApplicationResult::Failed; + auto result = DiffApplicationResult::Failed; diffFile->SyncWithDisk(); @@ -75,7 +74,7 @@ void Manager::ApplyDiff( std::lock_guard<std::mutex> lock(m_mutex); auto it = m_diffs.find(diffFile->GetCountryName()); CHECK(it != m_diffs.end(), ()); - it->second.m_downloaded = true; + it->second.m_status = SingleDiffStatus::Downloaded; } string const oldMwmPath = p.m_oldMwmFile->GetPath(MapOptions::Map); @@ -84,8 +83,11 @@ void Manager::ApplyDiff( result = generator::mwm_diff::ApplyDiff(oldMwmPath, diffApplyingInProgressPath, diffPath, cancellable); - if (!base::RenameFileX(diffApplyingInProgressPath, newMwmPath)) + if (result == DiffApplicationResult::Ok && + !base::RenameFileX(diffApplyingInProgressPath, newMwmPath)) + { result = DiffApplicationResult::Failed; + } base::DeleteFileX(diffApplyingInProgressPath); @@ -101,10 +103,9 @@ void Manager::ApplyDiff( std::lock_guard<std::mutex> lock(m_mutex); auto it = m_diffs.find(diffFile->GetCountryName()); CHECK(it != m_diffs.end(), ()); - it->second.m_applied = true; - it->second.m_downloaded = false; + it->second.m_status = SingleDiffStatus::Applied; + break; } - break; case DiffApplicationResult::Cancelled: break; case DiffApplicationResult::Failed: { @@ -120,9 +121,9 @@ void Manager::ApplyDiff( auto it = m_diffs.find(diffFile->GetCountryName()); CHECK(it != m_diffs.end(), ()); - it->second.m_downloaded = false; + it->second.m_status = SingleDiffStatus::NotDownloaded; + break; } - break; } task(result); @@ -142,8 +143,9 @@ bool Manager::SizeFor(storage::CountryId const & countryId, uint64_t & size) con bool Manager::SizeToDownloadFor(storage::CountryId const & countryId, uint64_t & size) const { - return WithDiff(countryId, - [&size](DiffInfo const & info) { size = (info.m_downloaded ? 0 : info.m_size); }); + return WithDiff(countryId, [&size](DiffInfo const & info) { + size = (info.m_status == SingleDiffStatus::Downloaded ? 0 : info.m_size); + }); } bool Manager::VersionFor(storage::CountryId const & countryId, uint64_t & version) const @@ -161,7 +163,7 @@ void Manager::RemoveAppliedDiffs() std::lock_guard<std::mutex> lock(m_mutex); for (auto it = m_diffs.begin(); it != m_diffs.end();) { - if (it->second.m_applied) + if (it->second.m_status == SingleDiffStatus::Applied) it = m_diffs.erase(it); else ++it; diff --git a/storage/diff_scheme/diff_manager.hpp b/storage/diff_scheme/diff_manager.hpp index 4a4e1950a6..a993498c6a 100644 --- a/storage/diff_scheme/diff_manager.hpp +++ b/storage/diff_scheme/diff_manager.hpp @@ -40,14 +40,19 @@ public: virtual void OnDiffStatusReceived(Status const status) = 0; }; - // Returns the size of the diff file for |countryId| or 0 if there is no diff. - bool SizeFor(storage::CountryId const & countryId, uint64_t & size) const; + using OnDiffApplicationFinished = + std::function<void(generator::mwm_diff::DiffApplicationResult result)>; - // Returns how many bytes are left for the diff to be downloaded for |countryId| + // If the diff is available, sets |size| to its size and returns true. + // Otherwise, returns false. + bool SizeFor(storage::TCountryId const & countryId, uint64_t & size) const; + + // Sets |size| to how many bytes are left for the diff to be downloaded for |countryId| // or 0 if there is no diff available or it has already been downloaded. // This method may overestimate because it does not account for the possibility // of resuming an old download, i.e. the return value is either 0 or the diff size. - bool SizeToDownloadFor(storage::CountryId const & countryId, uint64_t & size) const; + // Returns true iff the diff is available. + bool SizeToDownloadFor(storage::TCountryId const & countryId, uint64_t & size) const; bool VersionFor(storage::CountryId const & countryId, uint64_t & version) const; bool IsPossibleToAutoupdate() const; @@ -62,9 +67,8 @@ public: Status GetStatus() const; void Load(LocalMapsInfo && info); - void ApplyDiff( - ApplyDiffParams && p, base::Cancellable const & cancellable, - std::function<void(generator::mwm_diff::DiffApplicationResult result)> const & task); + void ApplyDiff(ApplyDiffParams && p, base::Cancellable const & cancellable, + OnDiffApplicationFinished const & task); bool AddObserver(Observer & observer) { return m_observers.Add(observer); } bool RemoveObserver(Observer const & observer) { return m_observers.Remove(observer); } diff --git a/storage/diff_scheme/diff_types.hpp b/storage/diff_scheme/diff_types.hpp index 2f44e80b57..823895d7bc 100644 --- a/storage/diff_scheme/diff_types.hpp +++ b/storage/diff_scheme/diff_types.hpp @@ -10,6 +10,7 @@ namespace storage { namespace diffs { +// Status of the diff manager as a whole. enum class Status { Undefined, @@ -17,13 +18,21 @@ enum class Status Available }; +// Status of a single diff. +enum class SingleDiffStatus +{ + NotDownloaded, + Downloaded, + Applied +}; + struct DiffInfo final { DiffInfo(uint64_t size, uint64_t version) : m_size(size), m_version(version) {} + uint64_t m_size; uint64_t m_version; - bool m_applied = false; - bool m_downloaded = false; + SingleDiffStatus m_status = SingleDiffStatus::NotDownloaded; }; using NameDiffInfoMap = std::unordered_map<storage::CountryId, DiffInfo>; diff --git a/storage/storage.cpp b/storage/storage.cpp index d99c75fd68..d3dc57c1c4 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -951,12 +951,8 @@ void Storage::RegisterDownloadedFiles(CountryId const & countryId, MapOptions op if (options == MapOptions::Diff) { - /// At this point a diff applying process is going to start - /// and we can't stop the process. - /// TODO: Make the applying process cancellable. m_queue.begin()->SetFrozen(true); NotifyStatusChangedForHierarchy(countryId); - m_diffsCancellable.Reset(); ApplyDiff(countryId, fn); return; } @@ -1561,6 +1557,8 @@ void Storage::LoadDiffScheme() void Storage::ApplyDiff(CountryId const & countryId, function<void(bool isSuccess)> const & fn) { + m_diffsCancellable.Reset(); + diffs::Manager::ApplyDiffParams params; params.m_diffFile = PreparePlaceForCountryFiles(GetCurrentDataVersion(), m_dataDir, GetCountryFile(countryId)); |