Review fixes

4 files changed, 38 insertions, 43 deletions

diff --git a/storage/diff_scheme/diff_manager.cpp b/storage/diff_scheme/diff_manager.cpp
index c401cc7a72..7bc9347364 100644
--- a/storage/diff_scheme/diff_manager.cpp
+++ b/storage/diff_scheme/diff_manager.cpp
@@ -74,18 +74,18 @@ void Manager::ApplyDiff(ApplyDiffParams && p, std::function<void(bool const resu
       diffFile->DeleteFromDisk(MapOptions::Diff);
     }
 
-    if (!result)
+    if (result)
     {
       std::lock_guard<std::mutex> lock(m_mutex);
-      m_status = Status::NotAvailable;
-      // TODO: Log the diff applying error (Downloader_DiffScheme_error (Aloha)).
+      m_diffs.erase(countryId);
+      if (m_diffs.empty())
+        m_status = Status::NotAvailable;
     }
     else
     {
       std::lock_guard<std::mutex> lock(m_mutex);
-      m_diffs.erase(countryId);
-      if (m_diffs.empty())
-        m_status = Status::NotAvailable;
+      m_status = Status::NotAvailable;
+      // TODO: Log the diff applying error (Downloader_DiffScheme_error (Aloha)).
     }
 
     task(result);
@@ -119,6 +119,8 @@ bool Manager::HasDiffFor(storage::TCountryId const & countryId) const
 
 bool Manager::HasDiffForUnsafe(storage::TCountryId const & countryId) const
 {
+  if (m_status != diffs::Status::Available)
+    return false;
   return m_diffs.find(countryId) != m_diffs.end();
 }
 
diff --git a/storage/storage.cpp b/storage/storage.cpp
index 782125f8bf..01d7177127 100644
--- a/storage/storage.cpp
+++ b/storage/storage.cpp
@@ -1687,8 +1687,7 @@ bool Storage::GetUpdateInfo(TCountryId const & countryId, UpdateInfo & updateInf
         GetNodeStatus(descendantNode).status != NodeStatus::OnDiskOutOfDate)
       return;
     updateInfo.m_numberOfMwmFilesToUpdate += 1;  // It's not a group mwm.
-    if (m_diffManager.GetStatus() == diffs::Status::Available &&
-        m_diffManager.HasDiffFor(descendantNode.Value().Name()))
+    if (m_diffManager.HasDiffFor(descendantNode.Value().Name()))
     {
       updateInfo.m_totalUpdateSizeInBytes +=
         m_diffManager.InfoFor(descendantNode.Value().Name()).m_size;
@@ -1797,7 +1796,6 @@ void Storage::GetTopmostNodesFor(TCountryId const & countryId, TCountriesVec & n
   }
 }
 
-
 TCountryId const Storage::GetParentIdFor(TCountryId const & countryId) const
 {
   vector<TCountryTreeNode const *> nodes;
@@ -1817,19 +1815,14 @@ TCountryId const Storage::GetParentIdFor(TCountryId const & countryId) const
   return nodes[0]->Value().GetParent();
 }
 
-TMwmSize Storage::GetRemoteSize(CountryFile const & file, MapOptions opt,
-                                int64_t version) const
+TMwmSize Storage::GetRemoteSize(CountryFile const & file, MapOptions opt, int64_t version) const
 {
   if (version::IsSingleMwm(version))
   {
     if (opt == MapOptions::Nothing)
       return 0;
-
-    if (m_diffManager.GetStatus() == diffs::Status::Available &&
-        m_diffManager.HasDiffFor(file.GetName()))
-    {
+    if (m_diffManager.HasDiffFor(file.GetName()))
       return m_diffManager.InfoFor(file.GetName()).m_size;
-    }
     return file.GetRemoteSize(MapOptions::Map);
   }
 
diff --git a/storage/storage.hpp b/storage/storage.hpp
index a0dd850747..7be88e14f9 100644
--- a/storage/storage.hpp
+++ b/storage/storage.hpp
@@ -508,6 +508,8 @@ public:
   bool IsInnerNode(TCountryId const & countryId) const;
 
   TLocalAndRemoteSize CountrySizeInBytes(TCountryId const & countryId, MapOptions opt) const;
+  TMwmSize GetRemoteSize(platform::CountryFile const & file, MapOptions opt,
+                         int64_t version) const;
   platform::CountryFile const & GetCountryFile(TCountryId const & countryId) const;
   TLocalFilePtr GetLatestLocalFile(platform::CountryFile const & countryFile) const;
   TLocalFilePtr GetLatestLocalFile(TCountryId const & countryId) const;
@@ -660,9 +662,6 @@ private:
   void LoadDiffScheme();
 
   void OnDiffStatusReceived() override;
-
-  TMwmSize GetRemoteSize(platform::CountryFile const & file, MapOptions opt,
-                         int64_t version) const;
 };
 
 void GetQueuedCountries(Storage::TQueue const & queue, TCountriesSet & resultCountries);
diff --git a/storage/storage_tests/queued_country_tests.cpp b/storage/storage_tests/queued_country_tests.cpp
index 21ae11bd4f..c757180de8 100644
--- a/storage/storage_tests/queued_country_tests.cpp
+++ b/storage/storage_tests/queued_country_tests.cpp
@@ -13,19 +13,19 @@ UNIT_TEST(QueuedCountry_AddOptions)
 
   TEST_EQUAL(countryId, country.GetCountryId(), ());
   TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ());
-  TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
+  TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
 
   country.AddOptions(MapOptions::Map);
   TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
-  TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
+  TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
 
   TEST(country.SwitchToNextFile(), ());
   TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
-  TEST_EQUAL(MapOptions::Map, country.GetCurrentFile(), ());
+  TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ());
 
   TEST(!country.SwitchToNextFile(), ());
   TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
-  TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFile(), ());
+  TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ());
 }
 
 UNIT_TEST(QueuedCountry_RemoveOptions)
@@ -36,57 +36,58 @@ UNIT_TEST(QueuedCountry_RemoveOptions)
   {
     QueuedCountry country(countryId, MapOptions::MapWithCarRouting);
     TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
-    TEST_EQUAL(MapOptions::Map, country.GetCurrentFile(), ());
-    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
+    TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
 
     country.RemoveOptions(MapOptions::Map);
     TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ());
-    TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
-    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
+    TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
 
     country.RemoveOptions(MapOptions::CarRouting);
     TEST_EQUAL(MapOptions::Nothing, country.GetInitOptions(), ());
-    TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFile(), ());
-    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
+    TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
   }
 
   {
     QueuedCountry country(countryId, MapOptions::MapWithCarRouting);
     TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
-    TEST_EQUAL(MapOptions::Map, country.GetCurrentFile(), ());
-    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
+    TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
 
     country.SwitchToNextFile();
     TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
-    TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
-    TEST_EQUAL(MapOptions::Map, country.GetDownloadedFiles(), ());
+    TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::Map, country.GetDownloadedFilesOptions(), ());
 
     country.RemoveOptions(MapOptions::CarRouting);
     TEST_EQUAL(MapOptions::Map, country.GetInitOptions(), ());
-    TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFile(), ());
-    TEST_EQUAL(MapOptions::Map, country.GetDownloadedFiles(), ());
+    TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::Map, country.GetDownloadedFilesOptions(), ());
   }
 
   {
     QueuedCountry country(countryId, MapOptions::MapWithCarRouting);
     TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
-    TEST_EQUAL(MapOptions::Map, country.GetCurrentFile(), ());
-    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
+    TEST_EQUAL(MapOptions::Map, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
 
     country.SwitchToNextFile();
     TEST_EQUAL(MapOptions::MapWithCarRouting, country.GetInitOptions(), ());
-    TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
-    TEST_EQUAL(MapOptions::Map, country.GetDownloadedFiles(), ());
+    TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::Map, country.GetDownloadedFilesOptions(), ());
 
     country.RemoveOptions(MapOptions::Map);
     TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ());
-    TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFile(), ());
-    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFiles(), ());
+    TEST_EQUAL(MapOptions::CarRouting, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::Nothing, country.GetDownloadedFilesOptions(), ());
 
     country.SwitchToNextFile();
     TEST_EQUAL(MapOptions::CarRouting, country.GetInitOptions(), ());
-    TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFile(), ());
-    TEST_EQUAL(MapOptions::CarRouting, country.GetDownloadedFiles(), ());
+    TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::Nothing, country.GetCurrentFileOptions(), ());
+    TEST_EQUAL(MapOptions::CarRouting, country.GetDownloadedFilesOptions(), ());
   }
 }
 }  // namespace storage