From 835d5d4f0d0550614fa0e42024e6516e02a3f027 Mon Sep 17 00:00:00 2001 From: VladiMihaylenko Date: Fri, 22 Dec 2017 13:50:18 +0300 Subject: [PATCH] [storage] Fixed diff applying on startup. --- storage/diff_scheme/diff_manager.cpp | 21 ++------ storage/diff_scheme/diff_manager.hpp | 19 ++++++-- storage/storage.cpp | 73 +++++++++++++++++----------- 3 files changed, 66 insertions(+), 47 deletions(-) diff --git a/storage/diff_scheme/diff_manager.cpp b/storage/diff_scheme/diff_manager.cpp index 159acd1e10..8d5254973f 100644 --- a/storage/diff_scheme/diff_manager.cpp +++ b/storage/diff_scheme/diff_manager.cpp @@ -107,30 +107,19 @@ Status Manager::GetStatus() const return m_status; } -void Manager::SetStatus(Status status) +bool Manager::SizeFor(storage::TCountryId const & countryId, uint64_t & size) const { - std::lock_guard lock(m_mutex); - m_status = status; + return WithDiff(countryId, [&size](FileInfo const & info) { size = info.m_size; }); } -FileInfo const & Manager::InfoFor(storage::TCountryId const & countryId) const +bool Manager::VersionFor(storage::TCountryId const & countryId, uint64_t & version) const { - std::lock_guard lock(m_mutex); - ASSERT(HasDiffForUnsafe(countryId), ()); - return m_diffs.at(countryId); + return WithDiff(countryId, [&version](FileInfo const & info) { version = info.m_version; }); } bool Manager::HasDiffFor(storage::TCountryId const & countryId) const { - std::lock_guard lock(m_mutex); - return HasDiffForUnsafe(countryId); -} - -bool Manager::HasDiffForUnsafe(storage::TCountryId const & countryId) const -{ - if (m_status != diffs::Status::Available) - return false; - return m_diffs.find(countryId) != m_diffs.end(); + return WithDiff(countryId, [](FileInfo const &){}); } bool Manager::IsPossibleToAutoupdate() const diff --git a/storage/diff_scheme/diff_manager.hpp b/storage/diff_scheme/diff_manager.hpp index 0aa25f95ee..130b6c1d6c 100644 --- a/storage/diff_scheme/diff_manager.hpp +++ b/storage/diff_scheme/diff_manager.hpp @@ -33,12 +33,12 @@ public: virtual void OnDiffStatusReceived(Status const status) = 0; }; - FileInfo const & InfoFor(storage::TCountryId const & countryId) const; + bool SizeFor(storage::TCountryId const & countryId, uint64_t & size) const; + bool VersionFor(storage::TCountryId const & countryId, uint64_t & version) const; bool IsPossibleToAutoupdate() const; bool HasDiffFor(storage::TCountryId const & countryId) const; Status GetStatus() const; - void SetStatus(Status status); void Load(LocalMapsInfo && info); void ApplyDiff(ApplyDiffParams && p, std::function const & task); @@ -47,7 +47,20 @@ public: bool RemoveObserver(Observer const & observer) { return m_observers.Remove(observer); } private: - bool HasDiffForUnsafe(storage::TCountryId const & countryId) const; + template + bool WithDiff(storage::TCountryId const & countryId, Fn && fn) const + { + std::lock_guard lock(m_mutex); + if (m_status != Status::Available) + return false; + + auto const it = m_diffs.find(countryId); + if (it == m_diffs.cend()) + return false; + + fn(it->second); + return true; + } mutable std::mutex m_mutex; Status m_status = Status::Undefined; diff --git a/storage/storage.cpp b/storage/storage.cpp index 54c2300229..b0e4750dce 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -614,14 +614,28 @@ void Storage::DownloadNextFile(QueuedCountry const & country) { TCountryId const & countryId = country.GetCountryId(); CountryFile const & countryFile = GetCountryFile(countryId); + auto const opt = country.GetCurrentFileOptions(); - string const filePath = GetFileDownloadPath(countryId, country.GetCurrentFileOptions()); + string const readyFilePath = GetFileDownloadPath(countryId, opt); uint64_t size; + auto & p = GetPlatform(); + + // 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. + // If there is a such file we have to cause the success download scenario. + bool isDownloadedDiff = false; + if (opt == MapOptions::Diff) + { + string filePath = readyFilePath; + my::GetNameWithoutExt(filePath); + isDownloadedDiff = p.GetFileSizeByFullPath(filePath, size); + } // It may happen that the file already was downloaded, so there're // no need to request servers list and download file. Let's // switch to next file. - if (GetPlatform().GetFileSizeByFullPath(filePath, size)) + if (isDownloadedDiff || p.GetFileSizeByFullPath(readyFilePath, size)) { OnMapFileDownloadFinished(true /* success */, MapFilesDownloader::TProgress(size, size)); return; @@ -939,9 +953,15 @@ string Storage::GetFileDownloadUrl(string const & baseUrl, url << baseUrl; string const currentVersion = strings::to_string(GetCurrentDataVersion()); if (options == MapOptions::Diff) - url << "diffs/" << currentVersion << "/" << strings::to_string(m_diffManager.InfoFor(countryId).m_version); + { + uint64_t version; + CHECK(m_diffManager.VersionFor(countryId, version), ()); + url << "diffs/" << currentVersion << "/" << strings::to_string(version); + } else + { url << OMIM_OS_NAME "/" << currentVersion; + } url << "/" << UrlEncode(fileName); return url.str(); @@ -1194,8 +1214,12 @@ bool Storage::DeleteCountryFilesFromDownloader(TCountryId const & countryId) uint64_t Storage::GetDownloadSize(QueuedCountry const & queuedCountry) const { TCountryId const & countryId = queuedCountry.GetCountryId(); + uint64_t size; if (queuedCountry.GetInitOptions() == MapOptions::Diff) - return m_diffManager.InfoFor(countryId).m_size; + { + CHECK(m_diffManager.SizeFor(countryId, size), ()); + return size; + } CountryFile const & file = GetCountryFile(countryId); return GetRemoteSize(file, queuedCountry.GetCurrentFileOptions(), GetCurrentDataVersion()); @@ -1422,7 +1446,16 @@ void Storage::ApplyDiff(TCountryId const & countryId, function