[ugc] save ugc file index immediately + tests.

6 files changed, 53 insertions, 72 deletions

diff --git a/map/framework.cpp b/map/framework.cpp
index b74c9ef8bf..b424a022f2 100644
--- a/map/framework.cpp
+++ b/map/framework.cpp
@@ -1394,8 +1394,6 @@ void Framework::EnterBackground()
 
   SaveViewport();
 
-  m_ugcApi->SaveUGCOnDisk();
-
   m_trafficManager.OnEnterBackground();
   m_routingManager.SetAllowSendingPoints(false);
 
diff --git a/ugc/api.cpp b/ugc/api.cpp
index 4df90ed104..172bda4bc2 100644
--- a/ugc/api.cpp
+++ b/ugc/api.cpp
@@ -54,11 +54,6 @@ void Api::SendingCompleted()
   m_thread.Push([this] { SendingCompletedImpl(); });
 }
 
-void Api::SaveUGCOnDisk()
-{
-  m_thread.Push([this] { SaveUGCOnDiskImpl(); });
-}
-
 Loader & Api::GetLoader()
 {
   return m_loader;
@@ -101,9 +96,4 @@ void Api::SendingCompletedImpl()
 {
   m_storage.MarkAllAsSynchronized();
 }
-
-void Api::SaveUGCOnDiskImpl()
-{
-  m_storage.SaveIndex();
-}
 }  // namespace ugc
diff --git a/ugc/api.hpp b/ugc/api.hpp
index 17bab94a10..eebaa7cea4 100644
--- a/ugc/api.hpp
+++ b/ugc/api.hpp
@@ -41,7 +41,6 @@ public:
   void HasUGCForPlace(uint32_t bestType, m2::PointD const & point,
                       HasUGCForPlaceCallback const & callback);
   void SendingCompleted();
-  void SaveUGCOnDisk();
 
   Loader & GetLoader();
 
@@ -52,7 +51,6 @@ private:
   void HasUGCForPlaceImpl(uint32_t bestType, m2::PointD const & point,
                           HasUGCForPlaceCallback const & callback) const;
   void SendingCompletedImpl();
-  void SaveUGCOnDiskImpl();
 
   base::thread_pool::delayed::ThreadPool m_thread;
   Storage m_storage;
diff --git a/ugc/storage.cpp b/ugc/storage.cpp
index 0113f5fb4f..d60fa379bd 100644
--- a/ugc/storage.cpp
+++ b/ugc/storage.cpp
@@ -131,6 +131,28 @@ string SerializeIndexes(ugc::UpdateIndexes const & indexes)
   return string(buffer.get());
 }
 
+bool SaveIndexes(ugc::UpdateIndexes const & indexes, std::string const & pathToTargetFile = "")
+{
+  if (indexes.empty())
+    return false;
+
+  auto const indexFilePath = pathToTargetFile.empty() ? GetIndexFilePath() : pathToTargetFile;
+  auto const jsonData = SerializeIndexes(indexes);
+  try
+  {
+    FileWriter w(indexFilePath);
+    w.Write(jsonData.c_str(), jsonData.length());
+  }
+  catch (FileWriter::Exception const & exception)
+  {
+    LOG(LERROR, ("Exception while writing file:", indexFilePath, "reason:", exception.what()));
+    base::DeleteFileX(indexFilePath);
+    return false;
+  }
+
+  return true;
+}
+
 template <typename UGCUpdate>
 ugc::Storage::SettingResult SetGenericUGCUpdate(UGCUpdate const & ugc,
                                                 FeatureType & featureType,
@@ -186,7 +208,9 @@ ugc::Storage::SettingResult SetGenericUGCUpdate(UGCUpdate const & ugc,
   }
 
   indexes.emplace_back(move(index));
-  return ugc::Storage::SettingResult::Success;
+
+  return SaveIndexes(indexes) ? ugc::Storage::SettingResult::Success
+                              : ugc::Storage::SettingResult::WritingError;
 }
 }  // namespace
 
@@ -203,7 +227,8 @@ UGCUpdate Storage::GetUGCUpdate(FeatureID const & id) const
     return {};
 
   auto const offset = index->m_offset;
-  auto const size = static_cast<size_t>(UGCSizeAtIndex(distance(m_indexes.begin(), index)));
+  auto const pos = static_cast<size_t>(distance(m_indexes.begin(), index));
+  auto const size = static_cast<size_t>(UGCSizeAtPos(pos));
   vector<uint8_t> buf;
   buf.resize(size);
   auto const ugcFilePath = GetUGCFilePath();
@@ -305,7 +330,7 @@ void Storage::Migrate(string const & indexFilePath)
     DefragmentationImpl(true /* force */);
     // fallthrough
   case migration::Result::Success:
-    if (!SaveIndex(indexFilePath))
+    if (!SaveIndexes(m_indexes, indexFilePath))
     {
       base::DeleteFileX(indexFilePath);
       base::DeleteFileX(ugcFilePath);
@@ -343,28 +368,6 @@ UpdateIndexes::const_iterator Storage::FindIndex(uint32_t bestType, m2::PointD c
     });
 }
 
-bool Storage::SaveIndex(std::string const & pathToTargetFile /* = "" */) const
-{
-  if (m_indexes.empty())
-    return false;
-
-  auto const indexFilePath = pathToTargetFile.empty() ? GetIndexFilePath() : pathToTargetFile;
-  auto const jsonData = SerializeIndexes(m_indexes);
-  try
-  {
-    FileWriter w(indexFilePath);
-    w.Write(jsonData.c_str(), jsonData.length());
-  }
-  catch (FileWriter::Exception const & exception)
-  {
-    LOG(LERROR, ("Exception while writing file:", indexFilePath, "reason:", exception.what()));
-    base::DeleteFileX(indexFilePath);
-    return false;
-  }
-
-  return true;
-}
-
 void Storage::Defragmentation()
 {
   DefragmentationImpl(false /* force */);
@@ -391,7 +394,7 @@ void Storage::DefragmentationImpl(bool force)
         continue;
 
       auto const offset = index.m_offset;
-      auto const size = static_cast<size_t>(UGCSizeAtIndex(i));
+      auto const size = static_cast<size_t>(UGCSizeAtPos(i));
       vector<uint8_t> buf;
       buf.resize(size);
       r.Read(offset, buf.data(), size);
@@ -436,7 +439,7 @@ string Storage::GetUGCToSend() const
       continue;
 
     auto const offset = index.m_offset;
-    auto const bufSize = static_cast<size_t>(UGCSizeAtIndex(i));
+    auto const bufSize = static_cast<size_t>(UGCSizeAtPos(i));
     buf.resize(bufSize);
     try
     {
@@ -520,23 +523,23 @@ void Storage::MarkAllAsSynchronized()
 
   auto const indexPath = GetIndexFilePath();
   base::DeleteFileX(indexPath);
-  SaveIndex();
+  SaveIndexes(m_indexes);
 }
 
-uint64_t Storage::UGCSizeAtIndex(size_t const indexPosition) const
+uint64_t Storage::UGCSizeAtPos(size_t const pos) const
 {
   CHECK(!m_indexes.empty(), ());
   auto const indexesSize = m_indexes.size();
-  CHECK_LESS(indexPosition, indexesSize, ());
-  auto const indexOffset = m_indexes[indexPosition].m_offset;
+  CHECK_LESS(pos, indexesSize, ());
+  auto const offset = m_indexes[pos].m_offset;
   uint64_t nextOffset;
-  if (indexPosition == indexesSize - 1)
+  if (pos == indexesSize - 1)
     CHECK(GetUGCFileSize(nextOffset), ());
   else
-    nextOffset = m_indexes[indexPosition + 1].m_offset;
+    nextOffset = m_indexes[pos + 1].m_offset;
 
-  CHECK_GREATER(nextOffset, indexOffset, ());
-  return nextOffset - indexOffset;
+  CHECK_GREATER(nextOffset, offset, ());
+  return nextOffset - offset;
 }
 
 unique_ptr<FeatureType> Storage::GetFeature(FeatureID const & id) const
@@ -579,6 +582,11 @@ void Storage::LoadForTesting(std::string const & testIndexFilePath)
   if (m_indexes.front().m_version != IndexVersion::Latest)
     Migrate(testIndexFilePath);
 }
+
+bool Storage::SaveIndexForTesting(std::string const & testIndexFilePath) const
+{
+  return SaveIndexes(m_indexes, testIndexFilePath);
+}
 }  // namespace ugc
 
 namespace lightweight
diff --git a/ugc/storage.hpp b/ugc/storage.hpp
index 113d82a53c..b9c367288f 100644
--- a/ugc/storage.hpp
+++ b/ugc/storage.hpp
@@ -35,7 +35,6 @@ public:
   };
 
   SettingResult SetUGCUpdate(FeatureID const & id, UGCUpdate const & ugc);
-  bool SaveIndex(std::string const & pathToTargetFile = "") const;
   std::string GetUGCToSend() const;
   void MarkAllAsSynchronized();
   void Defragmentation();
@@ -48,10 +47,11 @@ public:
   size_t GetNumberOfDeletedForTesting() const { return m_numberOfDeleted; }
   SettingResult SetUGCUpdateForTesting(FeatureID const & id, v0::UGCUpdate const & ugc);
   void LoadForTesting(std::string const & testIndexFilePath);
+  bool SaveIndexForTesting(std::string const & testIndexFilePath = "") const;
 
 private:
   void DefragmentationImpl(bool force);
-  uint64_t UGCSizeAtIndex(size_t const indexPosition) const;
+  uint64_t UGCSizeAtPos(size_t const pos) const;
   std::unique_ptr<FeatureType> GetFeature(FeatureID const & id) const;
   void Migrate(std::string const & indexFilePath);
   UpdateIndexes::const_iterator FindIndex(FeatureID const & id) const;
diff --git a/ugc/ugc_tests/storage_tests.cpp b/ugc/ugc_tests/storage_tests.cpp
index e2a8caae6e..15b94f3a62 100644
--- a/ugc/ugc_tests/storage_tests.cpp
+++ b/ugc/ugc_tests/storage_tests.cpp
@@ -47,7 +47,7 @@ string const kTestMwmName = "ugc storage test";
 bool DeleteIndexFile(ugc::IndexVersion v = ugc::IndexVersion::Latest)
 {
   if (v == ugc::IndexVersion::Latest)
-    return base::DeleteFileX(base::JoinPath(GetPlatform().WritableDir(), "index.json"));
+    return base::DeleteFileX(base::JoinPath(GetPlatform().SettingsDir(), "index.json"));
 
   string version;
   switch (v)
@@ -60,14 +60,13 @@ bool DeleteIndexFile(ugc::IndexVersion v = ugc::IndexVersion::Latest)
     break;
   }
 
-  return base::DeleteFileX(base::JoinPath(GetPlatform().WritableDir(), "index.json." + version));
+  return base::DeleteFileX(base::JoinPath(GetPlatform().SettingsDir(), "index.json." + version));
 }
 
 bool DeleteUGCFile(ugc::IndexVersion v = ugc::IndexVersion::Latest)
 {
   if (v == ugc::IndexVersion::Latest)
-    return base::DeleteFileX(base::JoinPath(GetPlatform().WritableDir(), "ugc.update.bin"));
-
+    return base::DeleteFileX(base::JoinPath(GetPlatform().SettingsDir(), "ugc.update.bin"));
 
   string version;
   switch (v)
@@ -80,7 +79,8 @@ bool DeleteUGCFile(ugc::IndexVersion v = ugc::IndexVersion::Latest)
       break;
   }
 
-  return base::DeleteFileX(base::JoinPath(GetPlatform().WritableDir(), "ugc.update.bin." + version));
+  return base::DeleteFileX(
+      base::JoinPath(GetPlatform().SettingsDir(), "ugc.update.bin." + version));
 }
 }  // namespace
 
@@ -203,6 +203,7 @@ public:
   ~StorageTest()
   {
     TEST(DeleteUGCFile(), ());
+    TEST(DeleteIndexFile(), ());
   }
 };
 }  // namespace ugc_tests
@@ -224,7 +225,6 @@ UNIT_CLASS_TEST(StorageTest, Smoke)
   TEST(!storage.GetUGCToSend().empty(), ());
   storage.MarkAllAsSynchronized();
   TEST(storage.GetUGCToSend().empty(), ());
-  TEST(DeleteIndexFile(), ());
 }
 
 UNIT_CLASS_TEST(StorageTest, DuplicatesAndDefragmentationSmoke)
@@ -288,7 +288,6 @@ UNIT_CLASS_TEST(StorageTest, LoadIndex)
     storage.Load();
     TEST_EQUAL(storage.SetUGCUpdate(cafeId, cafeUGC), Storage::SettingResult::Success, ());
     TEST_EQUAL(storage.SetUGCUpdate(railwayId, railwayUGC), Storage::SettingResult::Success, ());
-    storage.SaveIndex();
   }
 
   Storage storage(builder.GetDataSource());
@@ -303,7 +302,6 @@ UNIT_CLASS_TEST(StorageTest, LoadIndex)
 
   TEST_EQUAL(storage.SetUGCUpdate(cafeId, cafeUGC), Storage::SettingResult::Success, ());
   TEST_EQUAL(indexArray.size(), 3, ());
-  TEST(DeleteIndexFile(), ());
 }
 
 UNIT_CLASS_TEST(StorageTest, ContentTest)
@@ -357,7 +355,6 @@ UNIT_CLASS_TEST(StorageTest, ContentTest)
   storage.MarkAllAsSynchronized();
   TEST(firstIndex.m_synchronized, ());
   TEST(lastIndex.m_synchronized, ());
-  TEST(DeleteIndexFile(), ());
 }
 
 UNIT_CLASS_TEST(StorageTest, InvalidUGC)
@@ -420,7 +417,6 @@ UNIT_CLASS_TEST(StorageTest, NumberOfUnsynchronized)
     Storage storage(builder.GetDataSource());
     storage.Load();
     TEST_EQUAL(storage.SetUGCUpdate(cafeId, cafeUGC), Storage::SettingResult::Success, ());
-    storage.SaveIndex();
   }
 
   Storage storage(builder.GetDataSource());
@@ -433,8 +429,6 @@ UNIT_CLASS_TEST(StorageTest, NumberOfUnsynchronized)
 
   storage.MarkAllAsSynchronized();
   TEST_EQUAL(storage.GetNumberOfUnsynchronized(), 0, ());
-
-  TEST(DeleteIndexFile(), ());
 }
 
 UNIT_CLASS_TEST(StorageTest, GetNumberOfUnsentSeparately)
@@ -450,7 +444,6 @@ UNIT_CLASS_TEST(StorageTest, GetNumberOfUnsentSeparately)
     Storage storage(builder.GetDataSource());
     storage.Load();
     TEST_EQUAL(storage.SetUGCUpdate(cafeId, cafeUGC), Storage::SettingResult::Success, ());
-    storage.SaveIndex();
     TEST_EQUAL(storage.GetNumberOfUnsynchronized(), 1, ());
   }
 
@@ -461,11 +454,9 @@ UNIT_CLASS_TEST(StorageTest, GetNumberOfUnsentSeparately)
     storage.Load();
     storage.MarkAllAsSynchronized();
     TEST_EQUAL(storage.GetNumberOfUnsynchronized(), 0, ());
-    storage.SaveIndex();
   }
 
   TEST_EQUAL(lightweight::impl::GetNumberOfUnsentUGC(), 0, ());
-  TEST(DeleteIndexFile(), ());
 }
 
 UNIT_TEST(UGC_IndexMigrationFromV0ToV1Smoke)
@@ -509,8 +500,6 @@ UNIT_TEST(UGC_IndexMigrationFromV0ToV1Smoke)
       TEST_EQUAL(static_cast<uint8_t>(i.m_version), static_cast<uint8_t>(IndexVersion::Latest), ());
       TEST(!i.m_synchronized, ());
     }
-
-    TEST(s.SaveIndex(indexFilePath), ());
   }
 
   {
@@ -534,7 +523,7 @@ UNIT_TEST(UGC_NoReviews)
   auto & builder = MwmBuilder::Builder();
   Storage s(builder.GetDataSource());
   s.Load();
-  s.SaveIndex();
+  s.SaveIndexForTesting();
   // When we didn't write any reviews there should be no index file and no ugc file.
   TEST(!DeleteIndexFile(), ());
   TEST(!DeleteUGCFile(), ());
@@ -568,8 +557,7 @@ UNIT_TEST(UGC_TooOldDataVersionsForMigration)
     railwayIndex.m_dataVersion = kMinVersionForMigration;
     railwayIndex.m_version = IndexVersion::V0;
     railwayIndex.m_synchronized = true;
-
-    s.SaveIndex();
+    s.SaveIndexForTesting();
   }
 
   {
@@ -584,7 +572,6 @@ UNIT_TEST(UGC_TooOldDataVersionsForMigration)
     TEST_EQUAL(static_cast<uint8_t>(railwayIndex.m_version), static_cast<uint8_t>(IndexVersion::Latest), ());
     TEST(!railwayIndex.m_synchronized, ());
     TEST_EQUAL(railwayIndex.m_dataVersion, kMinVersionForMigration, ());
-    s.SaveIndex();
   }
 
   {