[storage, platform] Fixed abrupted downloading.

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 \