diff --git a/3party/bsdiff-courgette/bsdiff/bsdiff.h b/3party/bsdiff-courgette/bsdiff/bsdiff.h index 9a3d44fe67..8718604989 100644 --- a/3party/bsdiff-courgette/bsdiff/bsdiff.h +++ b/3party/bsdiff-courgette/bsdiff/bsdiff.h @@ -80,6 +80,7 @@ #include "coding/write_to_sink.hpp" #include "coding/writer.hpp" +#include "base/cancellable.hpp" #include "base/checked_cast.hpp" #include "base/logging.hpp" #include "base/string_utils.hpp" @@ -380,9 +381,9 @@ BSDiffStatus CreateBinaryPatch(OldReader & old_reader, // the CRC of the original file stored in the patch file, before applying the // patch to it. template -BSDiffStatus ApplyBinaryPatch(OldReader & old_reader, - NewSink & new_sink, - PatchReader & patch_reader) { +BSDiffStatus ApplyBinaryPatch(OldReader & old_reader, NewSink & new_sink, + PatchReader & patch_reader, const base::Cancellable & cancellable) +{ ReaderSource old_source(old_reader); ReaderSource patch_source(patch_reader); @@ -436,7 +437,14 @@ BSDiffStatus ApplyBinaryPatch(OldReader & old_reader, auto pending_diff_zeros = ReadVarUint(diff_skips); + // 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; + while (control_stream_copy_counts.Size() > 0) { + if (cancellable.IsCancelled()) + return CANCELLED; + auto copy_count = ReadVarUint(control_stream_copy_counts); auto extra_count = ReadVarUint(control_stream_extra_counts); auto seek_adjustment = ReadVarInt(control_stream_seeks); @@ -452,6 +460,9 @@ BSDiffStatus ApplyBinaryPatch(OldReader & old_reader, // Add together bytes from the 'old' file and the 'diff' stream. for (size_t i = 0; i < copy_count; ++i) { + if (i > 0 && i % kCheckCancelledPeriod == 0 && cancellable.IsCancelled()) + return CANCELLED; + uint8_t diff_byte = 0; if (pending_diff_zeros) { --pending_diff_zeros; diff --git a/3party/bsdiff-courgette/bsdiff/bsdiff_common.h b/3party/bsdiff-courgette/bsdiff/bsdiff_common.h index a1aeecbf07..4cb87a5512 100644 --- a/3party/bsdiff-courgette/bsdiff/bsdiff_common.h +++ b/3party/bsdiff-courgette/bsdiff/bsdiff_common.h @@ -18,7 +18,8 @@ enum BSDiffStatus { CRC_ERROR = 2, READ_ERROR = 3, UNEXPECTED_ERROR = 4, - WRITE_ERROR = 5 + WRITE_ERROR = 5, + CANCELLED = 6, }; // The patch stream starts with a MBSPatchHeader. @@ -57,7 +58,7 @@ BSDiffStatus MBS_ReadHeader(Source & src, MBSPatchHeader* header) { return OK; } -std::string DebugPrint(BSDiffStatus status) { +inline std::string DebugPrint(BSDiffStatus status) { switch (status) { case OK: return "OK"; case MEM_ERROR: return "MEM_ERROR"; @@ -65,6 +66,7 @@ std::string DebugPrint(BSDiffStatus status) { case READ_ERROR: return "READ_ERROR"; case UNEXPECTED_ERROR: return "UNEXPECTED_ERROR"; case WRITE_ERROR: return "WRITE_ERROR"; + case CANCELLED: return "CANCELLED"; } return "Unknown status"; } diff --git a/generator/mwm_diff/diff.cpp b/generator/mwm_diff/diff.cpp index 38b60a69d9..829cf3e0b7 100644 --- a/generator/mwm_diff/diff.cpp +++ b/generator/mwm_diff/diff.cpp @@ -7,6 +7,8 @@ #include "coding/writer.hpp" #include "coding/zlib.hpp" +#include "base/assert.hpp" +#include "base/cancellable.hpp" #include "base/logging.hpp" #include @@ -52,9 +54,12 @@ bool MakeDiffVersion0(FileReader & oldReader, FileReader & newReader, FileWriter return true; } -bool ApplyDiffVersion0(FileReader & oldReader, FileWriter & newWriter, - ReaderSource & diffFileSource) +generator::mwm_diff::DiffApplicationResult ApplyDiffVersion0( + FileReader & oldReader, FileWriter & newWriter, ReaderSource & diffFileSource, + base::Cancellable const & cancellable) { + using generator::mwm_diff::DiffApplicationResult; + vector deflatedDiff(diffFileSource.Size()); diffFileSource.Read(deflatedDiff.data(), deflatedDiff.size()); @@ -73,15 +78,20 @@ bool ApplyDiffVersion0(FileReader & oldReader, FileWriter & newWriter, // 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); + auto 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 false; + return DiffApplicationResult::Failed; } - return true; + return DiffApplicationResult::Ok; } } // namespace @@ -119,7 +129,8 @@ bool MakeDiff(string const & oldMwmPath, string const & newMwmPath, string const return false; } -bool ApplyDiff(string const & oldMwmPath, string const & newMwmPath, string const & diffPath) +DiffApplicationResult ApplyDiff(string const & oldMwmPath, string const & newMwmPath, + string const & diffPath, base::Cancellable const & cancellable) { try { @@ -132,22 +143,33 @@ bool ApplyDiff(string const & oldMwmPath, string const & newMwmPath, string cons switch (version) { - case VERSION_V0: return ApplyDiffVersion0(oldReader, newWriter, diffFileSource); + case VERSION_V0: return ApplyDiffVersion0(oldReader, newWriter, diffFileSource, cancellable); default: LOG(LERROR, ("Unknown version format of mwm diff:", version)); } } catch (Reader::Exception const & e) { LOG(LERROR, ("Could not open file when applying a patch:", e.Msg())); - return false; + return DiffApplicationResult::Failed; } catch (Writer::Exception const & e) { LOG(LERROR, ("Could not open file when applying a patch:", e.Msg())); - return false; + return DiffApplicationResult::Failed; } - return false; + return DiffApplicationResult::Failed; +} + +string DebugPrint(DiffApplicationResult const & result) +{ + switch (result) + { + case DiffApplicationResult::Ok: return "Ok"; + case DiffApplicationResult::Failed: return "Failed"; + case DiffApplicationResult::Cancelled: return "Cancelled"; + } + UNREACHABLE(); } } // namespace mwm_diff } // namespace generator diff --git a/generator/mwm_diff/diff.hpp b/generator/mwm_diff/diff.hpp index 3cfaf99384..4a378d9135 100644 --- a/generator/mwm_diff/diff.hpp +++ b/generator/mwm_diff/diff.hpp @@ -2,10 +2,22 @@ #include +namespace base +{ +class Cancellable; +} + namespace generator { namespace mwm_diff { +enum class DiffApplicationResult +{ + Ok, + Failed, + Cancelled, +}; + // Makes a diff that, when applied to the mwm at |oldMwmPath|, will // result in the mwm at |newMwmPath|. The diff is stored at |diffPath|. // It is assumed that the files at |oldMwmPath| and |newMwmPath| are valid mwms. @@ -17,8 +29,12 @@ bool MakeDiff(std::string const & oldMwmPath, std::string const & newMwmPath, // mwm is stored at |newMwmPath|. // It is assumed that the file at |oldMwmPath| is a valid mwm and the file // at |diffPath| is a valid mwmdiff. -// Returns true on success and false on failure. -bool ApplyDiff(std::string const & oldMwmPath, std::string const & newMwmPath, - std::string const & diffPath); +// The application process can be stopped via |cancellable| in which case +// it is up to the caller to clean the partially written file at |diffPath|. +DiffApplicationResult ApplyDiff(std::string const & oldMwmPath, std::string const & newMwmPath, + std::string const & diffPath, + base::Cancellable const & cancellable); + +std::string DebugPrint(DiffApplicationResult const & result); } // namespace mwm_diff } // namespace generator diff --git a/generator/mwm_diff/mwm_diff_tests/diff_tests.cpp b/generator/mwm_diff/mwm_diff_tests/diff_tests.cpp index 88f0d4dac4..eadb067372 100644 --- a/generator/mwm_diff/mwm_diff_tests/diff_tests.cpp +++ b/generator/mwm_diff/mwm_diff_tests/diff_tests.cpp @@ -45,8 +45,10 @@ UNIT_TEST(IncrementalUpdates_Smoke) FileWriter writer(newMwmPath1); } + base::Cancellable cancellable; TEST(MakeDiff(oldMwmPath, newMwmPath1, diffPath), ()); - TEST(ApplyDiff(oldMwmPath, newMwmPath2, diffPath), ()); + TEST_EQUAL(ApplyDiff(oldMwmPath, newMwmPath2, diffPath, cancellable), DiffApplicationResult::Ok, + ()); { // Alter the old mwm slightly. @@ -60,10 +62,16 @@ UNIT_TEST(IncrementalUpdates_Smoke) } TEST(MakeDiff(oldMwmPath, newMwmPath1, diffPath), ()); - TEST(ApplyDiff(oldMwmPath, newMwmPath2, diffPath), ()); + TEST_EQUAL(ApplyDiff(oldMwmPath, newMwmPath2, diffPath, cancellable), DiffApplicationResult::Ok, + ()); TEST(base::IsEqualFiles(newMwmPath1, newMwmPath2), ()); + cancellable.Cancel(); + TEST_EQUAL(ApplyDiff(oldMwmPath, newMwmPath2, diffPath, cancellable), + DiffApplicationResult::Cancelled, ()); + cancellable.Reset(); + { // Corrupt the diff file contents. vector diffContents = FileReader(diffPath).ReadAsBytes(); @@ -76,14 +84,16 @@ UNIT_TEST(IncrementalUpdates_Smoke) writer.Write(diffContents.data(), diffContents.size()); } - TEST(!ApplyDiff(oldMwmPath, newMwmPath2, diffPath), ()); + TEST_EQUAL(ApplyDiff(oldMwmPath, newMwmPath2, diffPath, cancellable), + DiffApplicationResult::Failed, ()); { // Reset the diff file contents. FileWriter writer(diffPath); } - TEST(!ApplyDiff(oldMwmPath, newMwmPath2, diffPath), ()); + TEST_EQUAL(ApplyDiff(oldMwmPath, newMwmPath2, diffPath, cancellable), + DiffApplicationResult::Failed, ()); } } // namespace mwm_diff } // namespace generator diff --git a/generator/mwm_diff/pymwm_diff/bindings.cpp b/generator/mwm_diff/pymwm_diff/bindings.cpp index aa5624449d..0579c628e6 100644 --- a/generator/mwm_diff/pymwm_diff/bindings.cpp +++ b/generator/mwm_diff/pymwm_diff/bindings.cpp @@ -1,5 +1,8 @@ #include "generator/mwm_diff/diff.hpp" +#include "base/assert.hpp" +#include "base/cancellable.hpp" + #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wreorder" #pragma GCC diagnostic ignored "-Wunused-local-typedefs" @@ -19,11 +22,37 @@ using namespace std; +namespace +{ +// Applies the diff at |diffPath| to the mwm at |oldMwmPath|. The resulting +// mwm is stored at |newMwmPath|. +// It is assumed that the file at |oldMwmPath| is a valid mwm and the file +// at |diffPath| is a valid mwmdiff. +// Returns true on success and false on failure. +bool ApplyDiff(string const & oldMwmPath, string const & newMwmPath, string const & diffPath) +{ + base::Cancellable cancellable; + auto result = generator::mwm_diff::ApplyDiff(oldMwmPath, newMwmPath, diffPath, cancellable); + + switch (result) + { + case generator::mwm_diff::DiffApplicationResult::Ok: + return true; + case generator::mwm_diff::DiffApplicationResult::Failed: + return false; + case generator::mwm_diff::DiffApplicationResult::Cancelled: + UNREACHABLE(); + break; + } + UNREACHABLE(); +} +} // namespace + BOOST_PYTHON_MODULE(pymwm_diff) { using namespace boost::python; scope().attr("__version__") = PYBINDINGS_VERSION; def("make_diff", generator::mwm_diff::MakeDiff); - def("apply_diff", generator::mwm_diff::ApplyDiff); + def("apply_diff", ApplyDiff); } diff --git a/storage/diff_scheme/diff_manager.cpp b/storage/diff_scheme/diff_manager.cpp index d2ac94d56e..4609c839c0 100644 --- a/storage/diff_scheme/diff_manager.cpp +++ b/storage/diff_scheme/diff_manager.cpp @@ -1,13 +1,12 @@ #include "storage/diff_scheme/diff_manager.hpp" #include "storage/diff_scheme/diff_scheme_checker.hpp" -#include "generator/mwm_diff/diff.hpp" - #include "platform/platform.hpp" #include "coding/internal/file_data.hpp" #include "base/assert.hpp" +#include "base/cancellable.hpp" namespace storage { @@ -21,8 +20,7 @@ void Manager::Load(LocalMapsInfo && info) m_localMapsInfo = std::move(info); } - m_workerThread.Push([this, localMapsInfo] - { + m_workerThread.Push([this, localMapsInfo] { NameDiffInfoMap const diffs = Checker::Check(localMapsInfo); std::lock_guard lock(m_mutex); @@ -47,17 +45,20 @@ void Manager::Load(LocalMapsInfo && info) }); } -void Manager::ApplyDiff(ApplyDiffParams && p, std::function const & task) +void Manager::ApplyDiff( + ApplyDiffParams && p, base::Cancellable const & cancellable, + std::function const & task) { - m_workerThread.Push([this, p, task] - { + using namespace generator::mwm_diff; + + m_workerThread.Push([this, p, &cancellable, task] { CHECK(p.m_diffFile, ()); CHECK(p.m_oldMwmFile, ()); auto & diffReadyPath = p.m_diffReadyPath; auto & diffFile = p.m_diffFile; auto const diffPath = diffFile->GetPath(MapOptions::Diff); - bool result = false; + DiffApplicationResult result = DiffApplicationResult::Failed; diffFile->SyncWithDisk(); @@ -70,31 +71,58 @@ void Manager::ApplyDiff(ApplyDiffParams && p, std::functionSyncWithDisk(); + { + std::lock_guard lock(m_mutex); + auto it = m_diffs.find(diffFile->GetCountryName()); + CHECK(it != m_diffs.end(), ()); + it->second.m_downloaded = true; + } + string const oldMwmPath = p.m_oldMwmFile->GetPath(MapOptions::Map); string const newMwmPath = diffFile->GetPath(MapOptions::Map); string const diffApplyingInProgressPath = newMwmPath + DIFF_APPLYING_FILE_EXTENSION; - result = generator::mwm_diff::ApplyDiff(oldMwmPath, diffApplyingInProgressPath, diffPath) && - base::RenameFileX(diffApplyingInProgressPath, newMwmPath); + + result = generator::mwm_diff::ApplyDiff(oldMwmPath, diffApplyingInProgressPath, diffPath, + cancellable); + if (!base::RenameFileX(diffApplyingInProgressPath, newMwmPath)) + result = DiffApplicationResult::Failed; + + base::DeleteFileX(diffApplyingInProgressPath); + + if (result != DiffApplicationResult::Ok) + base::DeleteFileX(newMwmPath); } - diffFile->DeleteFromDisk(MapOptions::Diff); - - if (result) + switch (result) { + case DiffApplicationResult::Ok: + { + diffFile->DeleteFromDisk(MapOptions::Diff); 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; } - else + break; + case DiffApplicationResult::Cancelled: break; + case DiffApplicationResult::Failed: { - std::lock_guard lock(m_mutex); - m_status = Status::NotAvailable; + diffFile->DeleteFromDisk(MapOptions::Diff); GetPlatform().GetMarketingService().SendMarketingEvent( marketing::kDiffSchemeError, {{"type", "patching"}, {"error", isFilePrepared ? "Cannot apply diff" : "Cannot prepare file"}}); + + std::lock_guard lock(m_mutex); + m_status = Status::NotAvailable; + + auto it = m_diffs.find(diffFile->GetCountryName()); + CHECK(it != m_diffs.end(), ()); + it->second.m_downloaded = false; + } + break; } task(result); @@ -112,6 +140,12 @@ bool Manager::SizeFor(storage::CountryId const & countryId, uint64_t & size) con return WithDiff(countryId, [&size](DiffInfo const & info) { size = info.m_size; }); } +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); }); +} + bool Manager::VersionFor(storage::CountryId const & countryId, uint64_t & version) const { return WithDiff(countryId, [&version](DiffInfo const & info) { version = info.m_version; }); @@ -119,7 +153,7 @@ bool Manager::VersionFor(storage::CountryId const & countryId, uint64_t & versio bool Manager::HasDiffFor(storage::CountryId const & countryId) const { - return WithDiff(countryId, [](DiffInfo const &){}); + return WithDiff(countryId, [](DiffInfo const &) {}); } void Manager::RemoveAppliedDiffs() diff --git a/storage/diff_scheme/diff_manager.hpp b/storage/diff_scheme/diff_manager.hpp index f75b6c45e3..4a4e1950a6 100644 --- a/storage/diff_scheme/diff_manager.hpp +++ b/storage/diff_scheme/diff_manager.hpp @@ -3,6 +3,8 @@ #include "storage/diff_scheme/diff_types.hpp" #include "storage/storage_defines.hpp" +#include "generator/mwm_diff/diff.hpp" + #include "base/observer_list.hpp" #include "base/thread_checker.hpp" #include "base/thread_pool_delayed.hpp" @@ -11,6 +13,11 @@ #include #include +namespace base +{ +class Cancellable; +} + namespace storage { namespace diffs @@ -33,17 +40,31 @@ 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; + + // Returns 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; + bool VersionFor(storage::CountryId const & countryId, uint64_t & version) const; bool IsPossibleToAutoupdate() const; + + // Checks whether the diff for |countryId| is available for download or + // has been downloaded. bool HasDiffFor(storage::CountryId const & countryId) const; + void RemoveAppliedDiffs(); void AbortDiffScheme(); Status GetStatus() const; void Load(LocalMapsInfo && info); - void ApplyDiff(ApplyDiffParams && p, std::function const & task); + void ApplyDiff( + ApplyDiffParams && p, base::Cancellable const & cancellable, + std::function 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 7a9737c731..2f44e80b57 100644 --- a/storage/diff_scheme/diff_types.hpp +++ b/storage/diff_scheme/diff_types.hpp @@ -23,6 +23,7 @@ struct DiffInfo final uint64_t m_size; uint64_t m_version; bool m_applied = false; + bool m_downloaded = false; }; using NameDiffInfoMap = std::unordered_map; diff --git a/storage/queued_country.hpp b/storage/queued_country.hpp index f1f7396fe4..31f0f99ad3 100644 --- a/storage/queued_country.hpp +++ b/storage/queued_country.hpp @@ -21,7 +21,7 @@ public: void ResetToDefaultOptions(); bool SwitchToNextFile(); - void SetFrozen() { m_isFrozen = true; } + void SetFrozen(bool isFrozen) { m_isFrozen = isFrozen; } bool IsFrozen() const { return m_isFrozen; } inline CountryId const & GetCountryId() const { return m_countryId; } diff --git a/storage/storage.cpp b/storage/storage.cpp index 3e1772e1b0..d99c75fd68 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -33,9 +33,10 @@ #include "3party/Alohalytics/src/alohalytics.h" using namespace downloader; +using namespace generator::mwm_diff; using namespace platform; -using namespace std; using namespace std::chrono; +using namespace std; namespace storage { @@ -129,7 +130,7 @@ Storage::Storage(string const & pathToCountriesFile /* = COUNTRIES_FILE */, { SetLocale(languages::GetCurrentTwine()); LoadCountriesFile(pathToCountriesFile, m_dataDir); - CalMaxMwmSizeBytes(); + CalcMaxMwmSizeBytes(); } Storage::Storage(string const & referenceCountriesTxtJsonForTesting, @@ -142,7 +143,7 @@ Storage::Storage(string const & referenceCountriesTxtJsonForTesting, m_currentVersion = LoadCountriesFromBuffer(referenceCountriesTxtJsonForTesting, m_countries, m_affiliations); CHECK_LESS_OR_EQUAL(0, m_currentVersion, ("Can't load test countries file")); - CalMaxMwmSizeBytes(); + CalcMaxMwmSizeBytes(); } void Storage::Init(UpdateCallback const & didDownload, DeleteCallback const & willDelete) @@ -391,9 +392,9 @@ LocalAndRemoteSize Storage::CountrySizeInBytes(CountryId const & countryId, MapO LocalAndRemoteSize sizes(0, GetRemoteSize(countryFile, opt, GetCurrentDataVersion())); if (!m_downloader->IsIdle() && IsCountryFirstInQueue(countryId)) { - sizes.first = - m_downloader->GetDownloadingProgress().first + - GetRemoteSize(countryFile, queuedCountry->GetDownloadedFilesOptions(), GetCurrentDataVersion()); + sizes.first = m_downloader->GetDownloadingProgress().first + + GetRemoteSize(countryFile, queuedCountry->GetDownloadedFilesOptions(), + GetCurrentDataVersion()); } return sizes; } @@ -442,17 +443,23 @@ LocalFilePtr Storage::GetLatestLocalFile(CountryId const & countryId) const Status Storage::CountryStatus(CountryId const & countryId) const { + CHECK_THREAD_CHECKER(m_threadChecker, ()); + // Check if this country has failed while downloading. if (m_failedCountries.count(countryId) > 0) return Status::EDownloadFailed; - // Check if we already downloading this country or have it in the queue + // Check if we are already downloading this country or have it in the queue. if (IsCountryInQueue(countryId)) { if (IsCountryFirstInQueue(countryId)) - return IsDiffApplyingInProgressToCountry(countryId) ? Status::EApplying : Status::EDownloading; - else - return Status::EInQueue; + { + if (IsDiffApplyingInProgressToCountry(countryId)) + return Status::EApplying; + return Status::EDownloading; + } + + return Status::EInQueue; } return Status::EUnknown; @@ -583,8 +590,7 @@ void Storage::DeleteCustomCountryVersion(LocalCountryFile const & localFile) } } - CountryId const countryId = FindCountryIdByFile(countryFile.GetName()); - if (!(IsLeaf(countryId))) + if (!IsLeaf(countryId)) { LOG(LERROR, ("Removed files for an unknown country:", localFile)); return; @@ -664,6 +670,16 @@ void Storage::DownloadNextFile(QueuedCountry const & country) uint64_t size; auto & p = GetPlatform(); + if (opt == MapOptions::Nothing) + { + // The diff was already downloaded (so the opt is Nothing) and + // is being applied. + // Still, an update of the status of another country might + // have kicked the downloader so we're here. + ASSERT(IsDiffApplyingInProgressToCountry(countryId), ()); + return; + } + // Since a downloaded valid diff file may be either with .diff or .diff.ready extension, // we have to check these both cases in order to find // the diff file which is ready to apply. @@ -726,8 +742,8 @@ void Storage::LoadCountriesFile(string const & pathToCountriesFile, string const if (m_countries.IsEmpty()) { - m_currentVersion = LoadCountriesFromFile(pathToCountriesFile, m_countries, - m_affiliations, mapping); + m_currentVersion = + LoadCountriesFromFile(pathToCountriesFile, m_countries, m_affiliations, mapping); LOG_SHORT(LINFO, ("Loaded countries list for version:", m_currentVersion)); if (m_currentVersion < 0) LOG(LERROR, ("Can't load countries file", pathToCountriesFile)); @@ -788,7 +804,8 @@ void Storage::OnMapFileDownloadFinished(HttpRequest::Status status, auto const it = m_localFiles.find(countryId); if (it == m_localFiles.end()) { - GetPlatform().GetMarketingService().SendPushWooshTag(marketing::kMapLastDownloaded, countryId); + GetPlatform().GetMarketingService().SendPushWooshTag(marketing::kMapLastDownloaded, + countryId); char nowStr[18]{}; tm now = base::GmTime(time(nullptr)); strftime(nowStr, sizeof(nowStr), "%Y-%m-%d %H:%M", &now); @@ -909,12 +926,11 @@ void Storage::RegisterDownloadedFiles(CountryId const & countryId, MapOptions op { CHECK_THREAD_CHECKER(m_threadChecker, ()); - auto const fn = [this, countryId](bool isSuccess) - { + auto const fn = [this, countryId](bool isSuccess) { CHECK_THREAD_CHECKER(m_threadChecker, ()); if (!isSuccess) { - OnDownloadFailed(countryId); + OnMapDownloadFailed(countryId); return; } @@ -938,8 +954,9 @@ void Storage::RegisterDownloadedFiles(CountryId const & countryId, MapOptions op /// 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(); + m_queue.begin()->SetFrozen(true); NotifyStatusChangedForHierarchy(countryId); + m_diffsCancellable.Reset(); ApplyDiff(countryId, fn); return; } @@ -1000,12 +1017,12 @@ void Storage::OnMapDownloadFinished(CountryId const & countryId, HttpRequest::St ASSERT_NOT_EQUAL(MapOptions::Nothing, options, ("This method should not be called for empty files set.")); - alohalytics::LogEvent( - "$OnMapDownloadFinished", - alohalytics::TStringMap({{"name", countryId}, - {"status", status == HttpRequest::Status::Completed ? "ok" : "failed"}, - {"version", strings::to_string(GetCurrentDataVersion())}, - {"option", DebugPrint(options)}})); + alohalytics::LogEvent("$OnMapDownloadFinished", + alohalytics::TStringMap( + {{"name", countryId}, + {"status", status == HttpRequest::Status::Completed ? "ok" : "failed"}, + {"version", strings::to_string(GetCurrentDataVersion())}, + {"option", DebugPrint(options)}})); GetPlatform().GetMarketingService().SendMarketingEvent(marketing::kDownloaderMapActionFinished, {{"action", "download"}}); @@ -1017,7 +1034,7 @@ void Storage::OnMapDownloadFinished(CountryId const & countryId, HttpRequest::St NotifyStatusChanged(GetRootId()); } - OnDownloadFailed(countryId); + OnMapDownloadFailed(countryId); return; } @@ -1054,7 +1071,7 @@ string Storage::GetFileDownloadUrl(string const & baseUrl, string Storage::GetFileDownloadUrl(string const & baseUrl, string const & fileName) const { return baseUrl + OMIM_OS_NAME "/" + strings::to_string(GetCurrentDataVersion()) + "/" + - UrlEncode(fileName); + UrlEncode(fileName); } CountryId Storage::FindCountryIdByFile(string const & name) const @@ -1276,7 +1293,7 @@ bool Storage::DeleteCountryFilesFromDownloader(CountryId const & countryId) return false; if (queuedCountry->IsFrozen()) - return false; + m_diffsCancellable.Cancel(); MapOptions const opt = queuedCountry->GetInitOptions(); if (IsCountryFirstInQueue(countryId)) @@ -1285,7 +1302,7 @@ bool Storage::DeleteCountryFilesFromDownloader(CountryId const & countryId) if (HasOptions(opt, queuedCountry->GetCurrentFileOptions())) m_downloader->Reset(); - // Remove all files downloader had been created for a country. + // Remove all files the downloader has created for the country. DeleteDownloaderFilesForCountry(GetCurrentDataVersion(), m_dataDir, GetCountryFile(countryId)); } @@ -1317,7 +1334,7 @@ uint64_t Storage::GetDownloadSize(QueuedCountry const & queuedCountry) const uint64_t size; if (queuedCountry.GetInitOptions() == MapOptions::Diff) { - CHECK(m_diffManager.SizeFor(countryId, size), ()); + CHECK_NOT_EQUAL(m_diffManager.SizeFor(countryId, size), 0, ()); return size; } @@ -1393,9 +1410,8 @@ void Storage::GetChildrenInGroups(CountryId const & parent, CountriesVec & downl // All disputed territories in subtree with root == |parent|. CountriesVec allDisputedTerritories; parentNode->ForEachChild([&](CountryTreeNode const & childNode) { - vector> disputedTerritoriesAndStatus; - StatusAndError const childStatus = GetNodeStatusInfo(childNode, - disputedTerritoriesAndStatus, + vector> disputedTerritoriesAndStatus; + StatusAndError const childStatus = GetNodeStatusInfo(childNode, disputedTerritoriesAndStatus, true /* isDisputedTerritoriesCounted */); CountryId const & childValue = childNode.Value().Name(); @@ -1509,7 +1525,7 @@ bool Storage::IsDisputed(CountryTreeNode const & node) const return found.size() > 1; } -void Storage::CalMaxMwmSizeBytes() +void Storage::CalcMaxMwmSizeBytes() { m_maxMwmSizeBytes = 0; m_countries.GetRoot().ForEachInSubtree([&](CountryTree::Node const & node) { @@ -1562,27 +1578,36 @@ void Storage::ApplyDiff(CountryId const & countryId, functionGetCountryName(), kSourceKey)) - { - base::DeleteFileX(diffFile->GetPath(MapOptions::Map)); - applyResult = false; - } + m_diffManager.ApplyDiff( + move(params), m_diffsCancellable, + [this, fn, countryId, diffFile](DiffApplicationResult result) { + static string const kSourceKey = "diff"; + if (result == DiffApplicationResult::Ok && m_integrityValidationEnabled && + !ValidateIntegrity(diffFile, diffFile->GetCountryName(), kSourceKey)) + { + base::DeleteFileX(diffFile->GetPath(MapOptions::Map)); + result = DiffApplicationResult::Failed; + } - GetPlatform().RunTask(Platform::Thread::Gui, [this, fn, diffFile, applyResult] - { - if (applyResult) - { - RegisterCountryFiles(diffFile); - Platform::DisableBackupForFile(diffFile->GetPath(MapOptions::Map)); - } - fn(applyResult); - }); - }); + GetPlatform().RunTask(Platform::Thread::Gui, [this, fn, diffFile, countryId, result] { + switch (result) + { + case DiffApplicationResult::Ok: + { + RegisterCountryFiles(diffFile); + Platform::DisableBackupForFile(diffFile->GetPath(MapOptions::Map)); + fn(true); + } + break; + case DiffApplicationResult::Cancelled: + { + CHECK(!IsDiffApplyingInProgressToCountry(countryId), ()); + } + break; + case DiffApplicationResult::Failed: fn(false); break; + } + }); + }); } void Storage::LoadServerListForSession() @@ -1653,9 +1678,9 @@ void Storage::OnDiffStatusReceived(diffs::Status const status) DoDeferredDownloadIfNeeded(); } -StatusAndError Storage::GetNodeStatusInfo(CountryTreeNode const & node, - vector> & disputedTerritories, - bool isDisputedTerritoriesCounted) const +StatusAndError Storage::GetNodeStatusInfo( + CountryTreeNode const & node, vector> & disputedTerritories, + bool isDisputedTerritoriesCounted) const { // Leaf node status. if (node.ChildrenCount() == 0) @@ -1723,7 +1748,7 @@ void Storage::GetNodeAttrs(CountryId const & countryId, NodeAttrs & nodeAttrs) c m_countryNameGetter.Get(countryId + LOCALIZATION_DESCRIPTION_SUFFIX); // Progress. - if (nodeAttrs.m_status == NodeStatus::OnDisk) + if (nodeAttrs.m_status == NodeStatus::OnDisk || nodeAttrs.m_status == NodeStatus::Applying) { // Group or leaf node is on disk and up to date. MwmSize const subTreeSizeBytes = node->Value().GetSubtreeMwmSizeBytes(); @@ -1849,13 +1874,13 @@ MapFilesDownloader::Progress Storage::CalculateProgress( if (downloadingMwm == d && downloadingMwm != kInvalidCountryId) { localAndRemoteBytes.first += downloadingMwmProgress.first; - localAndRemoteBytes.second += GetRemoteSize(GetCountryFile(d), MapOptions::Map, - GetCurrentDataVersion()); + localAndRemoteBytes.second += + GetRemoteSize(GetCountryFile(d), MapOptions::Map, GetCurrentDataVersion()); } else if (mwmsInQueue.count(d) != 0) { - localAndRemoteBytes.second += GetRemoteSize(GetCountryFile(d), MapOptions::Map, - GetCurrentDataVersion()); + localAndRemoteBytes.second += + GetRemoteSize(GetCountryFile(d), MapOptions::Map, GetCurrentDataVersion()); } else if (m_justDownloaded.count(d) != 0) { @@ -1879,12 +1904,18 @@ void Storage::UpdateNode(CountryId const & countryId) void Storage::CancelDownloadNode(CountryId const & countryId) { + CHECK_THREAD_CHECKER(m_threadChecker, ()); + CountriesSet setQueue; GetQueuedCountries(m_queue, setQueue); ForEachInSubtree(countryId, [&](CountryId const & descendantId, bool /* groupNode */) { + if (IsDiffApplyingInProgressToCountry(descendantId)) + m_diffsCancellable.Cancel(); + if (setQueue.count(descendantId) != 0) DeleteFromDownloader(descendantId); + if (m_failedCountries.count(descendantId) != 0) { m_failedCountries.erase(descendantId); @@ -1906,24 +1937,24 @@ void Storage::RetryDownloadNode(CountryId const & countryId) bool Storage::GetUpdateInfo(CountryId const & countryId, UpdateInfo & updateInfo) const { - auto const updateInfoAccumulator = [&updateInfo, this](CountryTreeNode const & descendantNode) { - if (descendantNode.ChildrenCount() != 0 || - GetNodeStatus(descendantNode).status != NodeStatus::OnDiskOutOfDate) + auto const updateInfoAccumulator = [&updateInfo, this](CountryTreeNode const & node) { + if (node.ChildrenCount() != 0 || GetNodeStatus(node).status != NodeStatus::OnDiskOutOfDate) return; + updateInfo.m_numberOfMwmFilesToUpdate += 1; // It's not a group mwm. - uint64_t size; - if (m_diffManager.SizeFor(descendantNode.Value().Name(), size)) + if (m_diffManager.HasDiffFor(node.Value().Name())) { + uint64_t size; + m_diffManager.SizeToDownloadFor(node.Value().Name(), size); updateInfo.m_totalUpdateSizeInBytes += size; } else { - updateInfo.m_totalUpdateSizeInBytes += - descendantNode.Value().GetSubtreeMwmSizeBytes(); + updateInfo.m_totalUpdateSizeInBytes += node.Value().GetSubtreeMwmSizeBytes(); } LocalAndRemoteSize sizes = - CountrySizeInBytes(descendantNode.Value().Name(), MapOptions::MapWithCarRouting); + CountrySizeInBytes(node.Value().Name(), MapOptions::MapWithCarRouting); updateInfo.m_sizeDifference += static_cast(sizes.second) - static_cast(sizes.first); }; @@ -2065,7 +2096,7 @@ MwmSize Storage::GetRemoteSize(CountryFile const & file, MapOptions opt, int64_t return size; } -void Storage::OnDownloadFailed(CountryId const & countryId) +void Storage::OnMapDownloadFailed(CountryId const & countryId) { m_failedCountries.insert(countryId); auto it = find(m_queue.begin(), m_queue.end(), countryId); diff --git a/storage/storage.hpp b/storage/storage.hpp index 64fa36fb85..725745b840 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -12,6 +12,7 @@ #include "platform/local_country_file.hpp" +#include "base/cancellable.hpp" #include "base/deferred_task.hpp" #include "base/thread_checker.hpp" #include "base/thread_pool_delayed.hpp" @@ -138,7 +139,14 @@ struct NodeStatuses bool m_groupNode; }; -/// This class is used for downloading, updating and deleting maps. +// This class is used for downloading, updating and deleting maps. +// Storage manages a queue of mwms to be downloaded. +// Every operation with this queue must be executed +// on the storage thread. In the current implementation, the storage +// thread coincides with the main (UI) thread. +// Downloading of only one mwm at a time is supported, so while the +// mwm at the top of the queue is being downloaded (or updated by +// applying a diff file) all other mwms have to wait. class Storage : public diffs::Manager::Observer { public: @@ -173,7 +181,7 @@ private: /// Set of mwm files which have been downloaded recently. /// When a mwm file is downloaded it's moved from |m_queue| to |m_justDownloaded|. - /// When a new mwm file is added to |m_queue| |m_justDownloaded| is cleared. + /// When a new mwm file is added to |m_queue|, |m_justDownloaded| is cleared. /// Note. This set is necessary for implementation of downloading progress of /// mwm group. CountriesSet m_justDownloaded; @@ -191,6 +199,12 @@ private: /// MapFilesDownloader::Progress m_countryProgress; + // Used to cancel an ongoing diff application. + // |m_diffsCancellable| is reset every time when a task to apply a diff is posted. + // We use the fact that at most one diff is being applied at a time and the + // calls to the diff manager's ApplyDiff are coordinated from the storage thread. + base::Cancellable m_diffsCancellable; + DownloadingPolicy m_defaultDownloadingPolicy; DownloadingPolicy * m_downloadingPolicy = &m_defaultDownloadingPolicy; @@ -682,9 +696,9 @@ private: /// Returns true if |node.Value().Name()| is a disputed territory and false otherwise. bool IsDisputed(CountryTreeNode const & node) const; - void CalMaxMwmSizeBytes(); - - void OnDownloadFailed(CountryId const & countryId); + void CalcMaxMwmSizeBytes(); + + void OnMapDownloadFailed(CountryId const & countryId); void LoadDiffScheme(); void ApplyDiff(CountryId const & countryId, std::function const & fn);