diff --git a/map/bookmark_manager.cpp b/map/bookmark_manager.cpp index dc2d370439..4c3ed85924 100644 --- a/map/bookmark_manager.cpp +++ b/map/bookmark_manager.cpp @@ -472,10 +472,10 @@ void BookmarkManager::OnEditSessionClosed() { ASSERT_GREATER(m_openedEditSessionsCount, 0, ()); if (--m_openedEditSessionsCount == 0) - NotifyChanges(); + NotifyChanges(true /* saveChangesOnDisk */); } -void BookmarkManager::NotifyChanges() +void BookmarkManager::NotifyChanges(bool saveChangesOnDisk) { CHECK_THREAD_CHECKER(m_threadChecker, ()); @@ -510,7 +510,11 @@ void BookmarkManager::NotifyChanges() categoriesToSave.push_back(groupId); } - SaveBookmarks(categoriesToSave); + // During the category reloading/updating the file saving should be skipped + // because of the file is already up to date. + if (saveChangesOnDisk) + SaveBookmarks(categoriesToSave); + SendBookmarksChanges(m_bookmarksChangesTracker); } m_bookmarksChangesTracker.ResetChanges(); @@ -1549,6 +1553,17 @@ std::string BookmarkManager::GetCategoryFileName(kml::MarkGroupId categoryId) co return GetBmCategory(categoryId)->GetFileName(); } +kml::MarkGroupId BookmarkManager::GetCategoryByFileName(std::string const & fileName) const +{ + CHECK_THREAD_CHECKER(m_threadChecker, ()); + for (auto const & c : m_categories) + { + if (c.second->GetFileName() == fileName) + return c.second->GetID(); + } + return kml::kInvalidMarkGroupId; +} + m2::RectD BookmarkManager::GetCategoryRect(kml::MarkGroupId categoryId, bool addIconsSize) const { CHECK_THREAD_CHECKER(m_threadChecker, ()); @@ -2326,6 +2341,17 @@ kml::MarkGroupId BookmarkManager::CreateBookmarkCategory(std::string const & nam return groupId; } +void BookmarkManager::UpdateBookmarkCategory(kml::MarkGroupId & groupId, kml::CategoryData && data, bool autoSave /* = true */) +{ + CHECK_THREAD_CHECKER(m_threadChecker, ()); + CHECK_NOT_EQUAL(m_categories.count(groupId), 0, ()); + // The current implementation reloads the provided group. + /// @todo implement more accurate merging instead of full reloading + ClearGroup(groupId); + m_categories.emplace(groupId, std::make_unique(std::move(data), autoSave)); + m_changesTracker.OnAddGroup(groupId); +} + BookmarkCategory * BookmarkManager::CreateBookmarkCompilation(kml::CategoryData && data) { CHECK_THREAD_CHECKER(m_threadChecker, ()); @@ -2478,6 +2504,7 @@ void BookmarkManager::CreateCategories(KMLDataCollection && dataCollection, bool CHECK_THREAD_CHECKER(m_threadChecker, ()); kml::GroupIdSet loadedGroups; + bool isUpdating = false; for (auto const & [fileName, fileDataPtr] : dataCollection) { auto & fileData = *fileDataPtr; @@ -2517,9 +2544,16 @@ void BookmarkManager::CreateCategories(KMLDataCollection && dataCollection, bool UserMarkIdStorage::Instance().EnableSaving(false); + auto groupId = GetCategoryByFileName(fileName); // Set autoSave = false now to avoid useless saving in NotifyChanges(). // autoSave flag will be assigned in the end of this function. - auto const groupId = CreateBookmarkCategory(std::move(categoryData), false /* autoSave */); + if (groupId != kml::kInvalidMarkGroupId) + { + isUpdating = true; + UpdateBookmarkCategory(groupId, std::move(categoryData), false /* autoSave */); + } + else + groupId = CreateBookmarkCategory(std::move(categoryData), false /* autoSave */); loadedGroups.insert(groupId); auto * group = GetBmCategory(groupId); group->SetFileName(fileName); @@ -2578,7 +2612,9 @@ void BookmarkManager::CreateCategories(KMLDataCollection && dataCollection, bool } m_restoringCache.clear(); - NotifyChanges(); + // During the updating process the file shouldn't be re-saved on disk because it should be already up to date. + // In other case race condition may occur when multiple devices are used. + NotifyChanges(!isUpdating /* saveChangesOnDisk */); for (auto const & groupId : loadedGroups) GetBmCategory(groupId)->EnableAutoSave(autoSave); @@ -2908,7 +2944,7 @@ void BookmarkManager::SetNotificationsEnabled(bool enabled) m_notificationsEnabled = enabled; if (m_notificationsEnabled && m_openedEditSessionsCount == 0) - NotifyChanges(); + NotifyChanges(true /* saveChangesOnDisk */); } bool BookmarkManager::AreNotificationsEnabled() const @@ -3444,5 +3480,5 @@ bool BookmarkManager::EditSession::DeleteBmCategory(kml::MarkGroupId groupId) void BookmarkManager::EditSession::NotifyChanges() { - m_bmManager.NotifyChanges(); + m_bmManager.NotifyChanges(true /* saveChangesOnDisk */); } diff --git a/map/bookmark_manager.hpp b/map/bookmark_manager.hpp index 491cea6326..786dcc39c4 100644 --- a/map/bookmark_manager.hpp +++ b/map/bookmark_manager.hpp @@ -267,6 +267,7 @@ public: kml::MarkGroupId CreateBookmarkCategory(kml::CategoryData && data, bool autoSave = true); kml::MarkGroupId CreateBookmarkCategory(std::string const & name, bool autoSave = true); + void UpdateBookmarkCategory(kml::MarkGroupId & groupId, kml::CategoryData && data, bool autoSave); BookmarkCategory * CreateBookmarkCompilation(kml::CategoryData && data); @@ -593,7 +594,7 @@ private: void OnEditSessionOpened(); void OnEditSessionClosed(); - void NotifyChanges(); + void NotifyChanges(bool saveChangesOnDisk); void SaveState() const; void LoadState(); @@ -626,6 +627,8 @@ private: kml::MarkGroupId CheckAndCreateDefaultCategory(); void CheckAndResetLastIds(); + kml::MarkGroupId GetCategoryByFileName(std::string const & fileName) const; + std::unique_ptr CollectBmGroupKMLData(BookmarkCategory const * group) const; KMLDataCollectionPtr PrepareToSaveBookmarks(kml::GroupIdCollection const & groupIdCollection); diff --git a/map/map_tests/bookmarks_test.cpp b/map/map_tests/bookmarks_test.cpp index 2aa05f48a2..0ba604fda7 100644 --- a/map/map_tests/bookmarks_test.cpp +++ b/map/map_tests/bookmarks_test.cpp @@ -1070,8 +1070,9 @@ UNIT_CLASS_TEST(Runner, Bookmarks_SpecialXMLNames) BookmarkManager bmManager(BM_CALLBACKS); bmManager.EnableTestMode(true); + auto const file1Name = "file1"; BookmarkManager::KMLDataCollection kmlDataCollection1; - kmlDataCollection1.emplace_back("" /* filePath */, + kmlDataCollection1.emplace_back(file1Name /* filePath */, LoadKmlData(MemReader(kmlString3, strlen(kmlString3)), KmlFileType::Text)); bmManager.CreateCategories(std::move(kmlDataCollection1)); @@ -1096,23 +1097,25 @@ UNIT_CLASS_TEST(Runner, Bookmarks_SpecialXMLNames) bmManager.GetEditSession().DeleteBmCategory(catId); + auto const file2Name = "file2"; BookmarkManager::KMLDataCollection kmlDataCollection2; - kmlDataCollection2.emplace_back("" /* filePath */, LoadKmlFile(fileNameTmp, GetActiveKmlFileType())); + kmlDataCollection2.emplace_back(file1Name /* filePath */, LoadKmlFile(fileNameTmp, GetActiveKmlFileType())); bmManager.CreateCategories(std::move(kmlDataCollection2)); BookmarkManager::KMLDataCollection kmlDataCollection3; - kmlDataCollection3.emplace_back("" /* filePath */, + kmlDataCollection3.emplace_back(file2Name /* filePath */, LoadKmlData(MemReader(kmlString3, strlen(kmlString3)), KmlFileType::Text)); bmManager.CreateCategories(std::move(kmlDataCollection3)); TEST_EQUAL(bmManager.GetBmGroupsCount(), 2, ()); - auto const catId2 = bmManager.GetSortedBmGroupIdList().back(); - auto const catId3 = bmManager.GetSortedBmGroupIdList().front(); + auto const catId2 = bmManager.GetSortedBmGroupIdList().front(); + auto const catId3 = bmManager.GetSortedBmGroupIdList().back(); TEST_EQUAL(bmManager.GetUserMarkIds(catId2).size(), 1, ()); TEST_EQUAL(bmManager.GetCategoryName(catId2), expectedName, ()); - TEST(bmManager.GetCategoryFileName(catId2).empty(), ()); + TEST_EQUAL(bmManager.GetCategoryFileName(catId2), file1Name, ()); + TEST_EQUAL(bmManager.GetCategoryFileName(catId3), file2Name, ()); auto const bmId1 = *bmManager.GetUserMarkIds(catId2).begin(); auto const * bm1 = bmManager.GetBookmark(bmId1);