diff --git a/3party/bsdiff-courgette/bsdiff/bsdiff.h b/3party/bsdiff-courgette/bsdiff/bsdiff.h index 8718604989..a4c76f7163 100644 --- a/3party/bsdiff-courgette/bsdiff/bsdiff.h +++ b/3party/bsdiff-courgette/bsdiff/bsdiff.h @@ -439,7 +439,7 @@ BSDiffStatus ApplyBinaryPatch(OldReader & old_reader, NewSink & new_sink, // We will check whether the application process has been cancelled // upon copying every |kCheckCancelledPeriod| bytes from the old file. - const size_t kCheckCancelledPeriod = 100 * 1024; + constexpr size_t kCheckCancelledPeriod = 100 * 1024; while (control_stream_copy_counts.Size() > 0) { if (cancellable.IsCancelled()) diff --git a/generator/mwm_diff/diff.cpp b/generator/mwm_diff/diff.cpp index 829cf3e0b7..a2f7f0244f 100644 --- a/generator/mwm_diff/diff.cpp +++ b/generator/mwm_diff/diff.cpp @@ -78,20 +78,19 @@ generator::mwm_diff::DiffApplicationResult ApplyDiffVersion0( // its checksum is compared to the one stored in the diff file. MemReaderWithExceptions diffMemReader(diffBuf.data(), diffBuf.size()); - auto status = bsdiff::ApplyBinaryPatch(oldReader, newWriter, diffMemReader, cancellable); + auto const status = bsdiff::ApplyBinaryPatch(oldReader, newWriter, diffMemReader, cancellable); if (status == bsdiff::BSDiffStatus::CANCELLED) { LOG(LDEBUG, ("Diff application has been cancelled")); return DiffApplicationResult::Cancelled; } - if (status != bsdiff::BSDiffStatus::OK) - { - LOG(LERROR, ("Could not apply patch with bsdiff:", status)); - return DiffApplicationResult::Failed; - } - return DiffApplicationResult::Ok; + if (status == bsdiff::BSDiffStatus::OK) + return DiffApplicationResult::Ok; + + LOG(LERROR, ("Could not apply patch with bsdiff:", status)); + return DiffApplicationResult::Failed; } } // namespace diff --git a/generator/mwm_diff/pymwm_diff/bindings.cpp b/generator/mwm_diff/pymwm_diff/bindings.cpp index 0579c628e6..3bdb8ded79 100644 --- a/generator/mwm_diff/pymwm_diff/bindings.cpp +++ b/generator/mwm_diff/pymwm_diff/bindings.cpp @@ -32,7 +32,7 @@ namespace bool ApplyDiff(string const & oldMwmPath, string const & newMwmPath, string const & diffPath) { base::Cancellable cancellable; - auto result = generator::mwm_diff::ApplyDiff(oldMwmPath, newMwmPath, diffPath, cancellable); + auto const result = generator::mwm_diff::ApplyDiff(oldMwmPath, newMwmPath, diffPath, cancellable); switch (result) { @@ -44,6 +44,7 @@ bool ApplyDiff(string const & oldMwmPath, string const & newMwmPath, string cons UNREACHABLE(); break; } + UNREACHABLE(); } } // namespace 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 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 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 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 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; - // 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 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; 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 const & fn) { + m_diffsCancellable.Reset(); + diffs::Manager::ApplyDiffParams params; params.m_diffFile = PreparePlaceForCountryFiles(GetCurrentDataVersion(), m_dataDir, GetCountryFile(countryId));