diff options
author | Yuri Gorshenin <y@maps.me> | 2015-10-13 18:18:53 +0300 |
---|---|---|
committer | Yuri Gorshenin <y@maps.me> | 2015-10-15 11:40:59 +0300 |
commit | 0f9a4fea92130a76d51ea9b296714c5c2c447659 (patch) | |
tree | 061b79e1172cdad6f5721213caa369d9711268a8 /storage | |
parent | 8b7da571dc4892d2ad08f905cc8c19c9838eabe3 (diff) |
[storage, platform] Fixed abrupted downloading.
Diffstat (limited to 'storage')
-rw-r--r-- | storage/storage.cpp | 42 | ||||
-rw-r--r-- | storage/storage.hpp | 1 | ||||
-rw-r--r-- | storage/storage_tests/storage_tests.cpp | 127 | ||||
-rw-r--r-- | storage/storage_tests/storage_tests.pro | 13 |
4 files changed, 156 insertions, 27 deletions
diff --git a/storage/storage.cpp b/storage/storage.cpp index a62e1ab292..dee802f990 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -23,7 +23,6 @@ #include "3party/Alohalytics/src/alohalytics.h" - using namespace downloader; using namespace platform; @@ -392,6 +391,20 @@ void Storage::DownloadNextCountryFromQueue() return; QueuedCountry & queuedCountry = m_queue.front(); + TIndex const & index = queuedCountry.GetIndex(); + + // It's not even possible to prepare directory for files before + // downloading. Mark this country as failed and switch to next + // country. + if (!PreparePlaceForCountryFiles(GetCountryFile(index), GetCurrentDataVersion())) + { + OnMapDownloadFinished(index, false /* success */, queuedCountry.GetInitOptions()); + NotifyStatusChanged(index); + m_queue.pop_front(); + DownloadNextCountryFromQueue(); + return; + } + DownloadNextFile(queuedCountry); // New status for the country, "Downloading" @@ -400,7 +413,20 @@ void Storage::DownloadNextCountryFromQueue() void Storage::DownloadNextFile(QueuedCountry const & country) { - CountryFile const & countryFile = GetCountryFile(country.GetIndex()); + TIndex const & index = country.GetIndex(); + CountryFile const & countryFile = GetCountryFile(index); + + string const filePath = GetFileDownloadPath(index, country.GetCurrentFile()); + uint64_t 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)) + { + OnMapFileDownloadFinished(true /* success */, MapFilesDownloader::TProgress(size, size)); + return; + } // send Country name for statistics m_downloader->GetServersList(GetCurrentDataVersion(), countryFile.GetNameWithoutExt(), @@ -747,6 +773,11 @@ void Storage::SetDownloaderForTesting(unique_ptr<MapFilesDownloader> && download m_downloader = move(downloader); } +void Storage::SetCurrentDataVersionForTesting(int64_t currentVersion) +{ + m_currentVersion = currentVersion; +} + Storage::TLocalFilePtr Storage::GetLocalFile(TIndex const & index, int64_t version) const { auto const it = m_localFiles.find(index); @@ -831,6 +862,9 @@ bool Storage::DeleteCountryFilesFromDownloader(TIndex const & index, MapOptions // Abrupt downloading of the current file if it should be removed. if (HasOptions(opt, queuedCountry->GetCurrentFile())) m_downloader->Reset(); + + // Remove all files downloader had been created for a country. + DeleteDownloaderFilesForCountry(GetCountryFile(index), GetCurrentDataVersion()); } queuedCountry->RemoveOptions(opt); @@ -858,8 +892,6 @@ uint64_t Storage::GetDownloadSize(QueuedCountry const & queuedCountry) const string Storage::GetFileDownloadPath(TIndex const & index, MapOptions file) const { - Platform & platform = GetPlatform(); - CountryFile const & countryFile = GetCountryFile(index); - return platform.WritablePathForFile(countryFile.GetNameWithExt(file) + READY_FILE_EXTENSION); + return platform::GetFileDownloadPath(GetCountryFile(index), file, GetCurrentDataVersion()); } } // namespace storage diff --git a/storage/storage.hpp b/storage/storage.hpp index 2d3b69eb32..a76f709e30 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -179,6 +179,7 @@ public: inline int64_t GetCurrentDataVersion() const { return m_currentVersion; } void SetDownloaderForTesting(unique_ptr<MapFilesDownloader> && downloader); + void SetCurrentDataVersionForTesting(int64_t currentVersion); private: friend void UnitTest_StorageTest_DeleteCountry(); diff --git a/storage/storage_tests/storage_tests.cpp b/storage/storage_tests/storage_tests.cpp index 733aa7c452..6ee91c7017 100644 --- a/storage/storage_tests/storage_tests.cpp +++ b/storage/storage_tests/storage_tests.cpp @@ -4,6 +4,7 @@ #include "storage/storage_defines.hpp" #include "storage/storage_tests/fake_map_files_downloader.hpp" #include "storage/storage_tests/task_runner.hpp" +#include "storage/storage_tests/test_map_files_downloader.hpp" #include "indexer/indexer_tests/test_mwm_set.hpp" @@ -23,11 +24,14 @@ #include "base/string_utils.hpp" #include "std/bind.hpp" +#include "std/condition_variable.hpp" #include "std/map.hpp" +#include "std/mutex.hpp" #include "std/shared_ptr.hpp" #include "std/unique_ptr.hpp" #include "std/vector.hpp" +#include <QtCore/QCoreApplication> using namespace platform; @@ -198,26 +202,6 @@ unique_ptr<CountryDownloaderChecker> CancelledCountryDownloaderChecker(Storage & vector<TStatus>{TStatus::ENotDownloaded, TStatus::EDownloading, TStatus::ENotDownloaded}); } -void OnCountryDownloaded(LocalCountryFile const & localFile) -{ - LOG(LINFO, ("OnCountryDownloaded:", localFile)); -} - -TLocalFilePtr CreateDummyMapFile(CountryFile const & countryFile, int64_t version, size_t size) -{ - TLocalFilePtr localFile = PreparePlaceForCountryFiles(countryFile, version); - TEST(localFile.get(), ("Can't prepare place for", countryFile, "(version ", version, ")")); - { - string const zeroes(size, '\0'); - FileWriter writer(localFile->GetPath(MapOptions::Map)); - writer.Write(zeroes.data(), zeroes.size()); - } - localFile->SyncWithDisk(); - TEST_EQUAL(MapOptions::Map, localFile->GetFiles(), ()); - TEST_EQUAL(size, localFile->GetSize(MapOptions::Map), ()); - return localFile; -} - class CountryStatusChecker { public: @@ -259,6 +243,77 @@ private: int m_slot; }; +class FailedDownloadingWaiter +{ +public: + FailedDownloadingWaiter(Storage & storage, TIndex const & index) + : m_storage(storage), m_index(index), m_finished(false) + { + m_slot = m_storage.Subscribe(bind(&FailedDownloadingWaiter::OnStatusChanged, this, _1), + bind(&FailedDownloadingWaiter::OnProgress, this, _1, _2)); + } + + ~FailedDownloadingWaiter() + { + Wait(); + m_storage.Unsubscribe(m_slot); + } + + void Wait() + { + unique_lock<mutex> lock(m_mu); + m_cv.wait(lock, [this]() + { + return m_finished; + }); + } + + void OnStatusChanged(TIndex const & index) + { + if (index != m_index) + return; + TStatus const status = m_storage.CountryStatusEx(index); + if (status != TStatus::EDownloadFailed) + return; + lock_guard<mutex> lock(m_mu); + m_finished = true; + m_cv.notify_one(); + + QCoreApplication::exit(); + } + + void OnProgress(TIndex const & /* index */, LocalAndRemoteSizeT const & /* progress */) {} + +private: + Storage & m_storage; + TIndex const m_index; + int m_slot; + + mutex m_mu; + condition_variable m_cv; + bool m_finished; +}; + +void OnCountryDownloaded(LocalCountryFile const & localFile) +{ + LOG(LINFO, ("OnCountryDownloaded:", localFile)); +} + +TLocalFilePtr CreateDummyMapFile(CountryFile const & countryFile, int64_t version, size_t size) +{ + TLocalFilePtr localFile = PreparePlaceForCountryFiles(countryFile, version); + TEST(localFile.get(), ("Can't prepare place for", countryFile, "(version", version, ")")); + { + string const zeroes(size, '\0'); + FileWriter writer(localFile->GetPath(MapOptions::Map)); + writer.Write(zeroes.data(), zeroes.size()); + } + localFile->SyncWithDisk(); + TEST_EQUAL(MapOptions::Map, localFile->GetFiles(), ()); + TEST_EQUAL(size, localFile->GetSize(MapOptions::Map), ()); + return localFile; +} + void InitStorage(Storage & storage, TaskRunner & runner, Storage::TUpdate const & update = &OnCountryDownloaded) { @@ -619,4 +674,36 @@ UNIT_TEST(StorageTest_DeleteCountry) map.Reset(); routing.Reset(); } + +UNIT_TEST(StorageTest_FailedDownloading) +{ + Storage storage; + storage.Init(&OnCountryDownloaded); + storage.SetDownloaderForTesting(make_unique<TestMapFilesDownloader>()); + storage.SetCurrentDataVersionForTesting(1234); + + TIndex const index = storage.FindIndexByFile("Uruguay"); + CountryFile const countryFile = storage.GetCountryFile(index); + + // To prevent interference from other tests and on other tests it's + // better to remove temprorary downloader files. + DeleteDownloaderFilesForCountry(countryFile, storage.GetCurrentDataVersion()); + MY_SCOPE_GUARD(cleanup, [&]() + { + DeleteDownloaderFilesForCountry(countryFile, storage.GetCurrentDataVersion()); + }); + + { + FailedDownloadingWaiter waiter(storage, index); + storage.DownloadCountry(index, MapOptions::Map); + QCoreApplication::exec(); + } + + // File wasn't downloaded, but temprorary downloader files must exist. + string const downloadPath = + GetFileDownloadPath(countryFile, MapOptions::Map, storage.GetCurrentDataVersion()); + TEST(!Platform::IsFileExistsByFullPath(downloadPath), ()); + TEST(Platform::IsFileExistsByFullPath(downloadPath + DOWNLOADING_FILE_EXTENSION), ()); + TEST(Platform::IsFileExistsByFullPath(downloadPath + RESUME_FILE_EXTENSION), ()); +} } // namespace storage diff --git a/storage/storage_tests/storage_tests.pro b/storage/storage_tests/storage_tests.pro index 50cb71e9b6..b2fe16d81b 100644 --- a/storage/storage_tests/storage_tests.pro +++ b/storage/storage_tests/storage_tests.pro @@ -10,14 +10,22 @@ DEPENDENCIES = storage indexer platform_tests_support platform geometry coding b include($$ROOT_DIR/common.pri) -macx-*: LIBS *= "-framework IOKit" -linux*|win32-msvc*: QT *= network +DEFINES *= OMIM_UNIT_TEST_WITH_QT_EVENT_LOOP QT *= core +macx-* { + QT *= gui widgets # needed for QApplication with event loop, to test async events (downloader, etc.) + LIBS *= "-framework IOKit" "-framework QuartzCore" +} +win32*|linux* { + QT *= network +} + HEADERS += \ fake_map_files_downloader.hpp \ task_runner.hpp \ + test_map_files_downloader.hpp \ SOURCES += \ ../../testing/testingmain.cpp \ @@ -27,3 +35,4 @@ SOURCES += \ simple_tree_test.cpp \ storage_tests.cpp \ task_runner.cpp \ + test_map_files_downloader.cpp \ |