From 55c41fbc469e7f9b5904a63efd608d9ce26d7288 Mon Sep 17 00:00:00 2001 From: Arsentiy Milchakov Date: Tue, 26 Feb 2019 12:24:52 +0300 Subject: [ugc] save ugc file index immediately + tests. --- map/framework.cpp | 2 -- ugc/api.cpp | 10 ------ ugc/api.hpp | 2 -- ugc/storage.cpp | 78 +++++++++++++++++++++++------------------ ugc/storage.hpp | 4 +-- ugc/ugc_tests/storage_tests.cpp | 29 +++++---------- 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 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(UGCSizeAtIndex(distance(m_indexes.begin(), index))); + auto const pos = static_cast(distance(m_indexes.begin(), index)); + auto const size = static_cast(UGCSizeAtPos(pos)); vector 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(UGCSizeAtIndex(i)); + auto const size = static_cast(UGCSizeAtPos(i)); vector 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(UGCSizeAtIndex(i)); + auto const bufSize = static_cast(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 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 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(i.m_version), static_cast(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(railwayIndex.m_version), static_cast(IndexVersion::Latest), ()); TEST(!railwayIndex.m_synchronized, ()); TEST_EQUAL(railwayIndex.m_dataVersion, kMinVersionForMigration, ()); - s.SaveIndex(); } { -- cgit v1.2.3