[Old map downloader] Review fixes

platform/local_country_file_utils.cpp

@@ -24,28 +24,29 @@
 
 namespace platform
 {
+
 namespace migrate
 {
-  // Set of functions to support migration between different versions of MWM
-  // with totaly incompatible formats.
-  // 160107 - Migrate to small single file MWM
-  uint32_t constexpr kRequiredVersion = 160107;
-  bool NeedMigrate()
-  {
-    uint32_t version;
-    if (!Settings::Get("LastMigration", version))
-      return true;
-
-    if (version >= kRequiredVersion)
-      return false;
-
+// Set of functions to support migration between different versions of MWM
+// with totaly incompatible formats.
+// 160107 - Migrate to small single file MWM
+uint32_t constexpr kMinRequiredVersion = 160107;
+bool NeedMigrate()
+{
+  uint32_t version;
+  if (!Settings::Get("LastMigration", version))
     return true;
-  }
 
-  void SetMigrationFlag()
-  {
-    Settings::Set("LastMigration", kRequiredVersion);
-  }
+  if (version >= kMinRequiredVersion)
+    return false;
+
+  return true;
+}
+
+void SetMigrationFlag()
+{
+  Settings::Set("LastMigration", kMinRequiredVersion);
+}
 }  // namespace migrate
 
 namespace

platform/local_country_file_utils.hpp

@@ -12,10 +12,11 @@ class ModelReader;
 
 namespace platform
 {
+
 namespace migrate
 {
-  bool NeedMigrate();
-  void SetMigrationFlag();
+bool NeedMigrate();
+void SetMigrationFlag();
 }
 
 // Removes all files downloader creates during downloading of a country.

storage/storage.cpp

@@ -125,11 +125,11 @@ void Storage::DeleteAllLocalMaps(vector<TIndex> * existedCountries /* = nullptr
 {
   for (auto const & localFiles : m_localFiles)
   {
+    if (existedCountries)
+      existedCountries->push_back(localFiles.first);
     for (auto const & localFile : localFiles.second)
     {
-      LOG_SHORT(LINFO, ("Remove:", localFiles.first, DebugPrint(*localFile)));
-      if (existedCountries)
-        existedCountries->push_back(localFiles.first);
+      LOG_SHORT(LINFO, ("Removing:", localFiles.first, DebugPrint(*localFile)));
       localFile->SyncWithDisk();
       DeleteFromDiskWithIndexes(*localFile, MapOptions::MapWithCarRouting);
     }

storage/storage_tests/migrate_tests.cpp

@@ -8,7 +8,7 @@ using namespace platform;
 
 UNIT_TEST(StorageTest_FastMigrate)
 {
-  // Set clear state
+  // Set clear state.
   {
     Settings::Clear();
     Framework f;
@@ -36,7 +36,14 @@ UNIT_TEST(StorageTests_Migrate)
   auto & s = f.Storage();
   s.DeleteAllLocalMaps();
 
-  vector<storage::TIndex> const kOldCountries = { s.FindIndexByFile("Estonia")};
+  auto cleanup = [&]()
+  {
+    s.DeleteAllLocalMaps();
+    Settings::Clear();
+  };
+  MY_SCOPE_GUARD(cleanupAtExit, cleanup);
+
+  vector<storage::TIndex> const kOldCountries = {s.FindIndexByFile("Estonia")};
 
   auto stateChanged = [&](storage::TIndex const & id)
   {
@@ -57,7 +64,7 @@ UNIT_TEST(StorageTests_Migrate)
   for (auto const & countryId : kOldCountries)
     f.GetCountryTree().GetActiveMapLayout().DownloadMap(countryId, MapOptions::MapWithCarRouting);
 
-  // Wait for downloading complete.
+  // Wait for completion of downloading.
   QCoreApplication::exec();
 
   TEST_EQUAL(s.GetDownloadedFilesCount(), kOldCountries.size(), ());
@@ -66,18 +73,16 @@ UNIT_TEST(StorageTests_Migrate)
 
   f.Migrate();
 
-  vector<storage::TIndex> const kNewCountries = {s.FindIndexByFile("Estonia_East"), s.FindIndexByFile("Estonia_West")};
+  vector<storage::TIndex> const kNewCountries = {s.FindIndexByFile("Estonia_East"),
+                                                 s.FindIndexByFile("Estonia_West")};
 
   for (auto const & countryId : kNewCountries)
     f.GetCountryTree().GetActiveMapLayout().DownloadMap(countryId, MapOptions::Map);
 
-  // Wait for downloading complete.
+  // Wait for completion of downloading.
   QCoreApplication::exec();
 
   TEST_EQUAL(s.GetDownloadedFilesCount(), kNewCountries.size(), ());
   for (auto const & countryId : kNewCountries)
     TEST_EQUAL(storage::TStatus::EOnDisk, s.CountryStatusEx(countryId), (countryId));
-
-  s.DeleteAllLocalMaps();
-  Settings::Clear();
 }

storage/storage_tests/storage_tests.pro

@@ -6,7 +6,6 @@ CONFIG -= app_bundle
 TEMPLATE = app
 
 ROOT_DIR = ../..
-#DEPENDENCIES = storage indexer platform_tests_support platform geometry coding base jansson tomcrypt stats_client succinct
 DEPENDENCIES = map drape_frontend routing search storage indexer drape platform_tests_support platform editor opening_hours geometry \
                coding base freetype expat fribidi tomcrypt jansson protobuf osrm stats_client \
                minizip succinct pugixml oauthcpp