diff --git a/map/bookmark_manager.cpp b/map/bookmark_manager.cpp index c3e378c399..b3ab85a8c8 100644 --- a/map/bookmark_manager.cpp +++ b/map/bookmark_manager.cpp @@ -1325,12 +1325,12 @@ void BookmarkManager::CreateCategories(KMLDataCollection && dataCollection, bool for (auto const & data : dataCollection) { auto const & fileName = data.first; - auto & fileData = *data.second.get(); + auto & fileData = *data.second; auto & categoryData = fileData.m_categoryData; - if ((categoryData.m_id != kml::kInvalidMarkGroupId) && - (UserMarkIdStorage::Instance().IsJustCreated() || fileData.m_deviceId != GetPlatform().UniqueClientId())) + if (!UserMarkIdStorage::Instance().CheckIds(fileData) || HasDuplicatedIds(fileData)) { + //TODO: notify subscribers(like search subsystem). This KML could have been indexed. ResetIds(fileData); } @@ -1385,6 +1385,29 @@ void BookmarkManager::CreateCategories(KMLDataCollection && dataCollection, bool } } +bool BookmarkManager::HasDuplicatedIds(kml::FileData const & fileData) const +{ + CHECK_THREAD_CHECKER(m_threadChecker, ()); + if (fileData.m_categoryData.m_id == kml::kInvalidMarkGroupId) + return false; + + if (m_categories.find(fileData.m_categoryData.m_id) != m_categories.cend()) + return true; + + for (auto const & b : fileData.m_bookmarksData) + { + if (m_bookmarks.count(b.m_id) > 0) + return true; + } + + for (auto const & t : fileData.m_tracksData) + { + if (m_tracks.count(t.m_id) > 0) + return true; + } + return false; +} + std::unique_ptr BookmarkManager::CollectBmGroupKMLData(BookmarkCategory const * group) const { auto kmlData = std::make_unique(); diff --git a/map/bookmark_manager.hpp b/map/bookmark_manager.hpp index 8a69608d1e..f85c9f1af1 100644 --- a/map/bookmark_manager.hpp +++ b/map/bookmark_manager.hpp @@ -443,6 +443,8 @@ private: bool CanConvert() const; void FinishConversion(ConversionHandler const & handler, bool result); + bool HasDuplicatedIds(kml::FileData const & fileData) const; + ThreadChecker m_threadChecker; Callbacks m_callbacks; diff --git a/map/user_mark_id_storage.cpp b/map/user_mark_id_storage.cpp index 9a344c7b66..5c0495b70e 100644 --- a/map/user_mark_id_storage.cpp +++ b/map/user_mark_id_storage.cpp @@ -13,10 +13,10 @@ std::string const kLastBookmarkCategoryId = "LastBookmarkCategoryId"; } // namespace UserMarkIdStorage::UserMarkIdStorage() - : m_isJustCreated(false) - , m_lastUserMarkId(0) + : m_lastUserMarkId(0) { - m_isJustCreated = !(HasKey(kLastBookmarkCategoryId) && HasKey(kLastBookmarkId) && HasKey(kLastTrackId)); + m_isJustCreated = !(HasKey(kLastBookmarkCategoryId) && HasKey(kLastBookmarkId) && + HasKey(kLastTrackId)); if (m_isJustCreated) { ResetCategoryId(); @@ -29,6 +29,10 @@ UserMarkIdStorage::UserMarkIdStorage() LoadLastTrackId(); LoadLastCategoryId(); } + + m_initialLastBookmarkId = m_lastBookmarkId; + m_initialLastTrackId = m_lastTrackId; + m_initialLastCategoryId = m_lastCategoryId; } // static @@ -44,11 +48,6 @@ UserMark::Type UserMarkIdStorage::GetMarkType(kml::MarkId id) return static_cast(id >> (sizeof(id) * 8 - kMarkIdTypeBitsCount)); } -bool UserMarkIdStorage::IsJustCreated() const -{ - return m_isJustCreated; -} - void UserMarkIdStorage::ResetBookmarkId() { m_lastBookmarkId = 0; @@ -67,6 +66,41 @@ void UserMarkIdStorage::ResetCategoryId() SaveLastCategoryId(); } +bool UserMarkIdStorage::CheckIds(kml::FileData const & fileData) const +{ + // File has no ids. Check passed. + if (fileData.m_categoryData.m_id == kml::kInvalidMarkGroupId) + return true; + + // Storage is just created. Check failed. + if (m_isJustCreated) + return false; + + // File was created on another device. Check failed. + if (fileData.m_deviceId != GetPlatform().UniqueClientId()) + return false; + + // There are ids of categories, bookmarks or tracks with values + // more than last stored maximums. Check failed. + if (fileData.m_categoryData.m_id > m_initialLastCategoryId) + return false; + + for (auto const & b : fileData.m_bookmarksData) + { + if (b.m_id > m_initialLastBookmarkId) + return false; + } + + for (auto const & t : fileData.m_tracksData) + { + if (t.m_id > m_initialLastTrackId) + return false; + } + + // No one corner case. Check passed. + return true; +} + kml::MarkId UserMarkIdStorage::GetNextUserMarkId(UserMark::Type type) { static_assert(UserMark::Type::USER_MARK_TYPES_COUNT <= (1 << kMarkIdTypeBitsCount), diff --git a/map/user_mark_id_storage.hpp b/map/user_mark_id_storage.hpp index c23457b99f..9d0a00feab 100644 --- a/map/user_mark_id_storage.hpp +++ b/map/user_mark_id_storage.hpp @@ -2,6 +2,8 @@ #include "map/user_mark.hpp" +#include "kml/types.hpp" + #include class UserMarkIdStorage @@ -11,12 +13,12 @@ public: static UserMark::Type GetMarkType(kml::MarkId id); - bool IsJustCreated() const; - kml::MarkId GetNextUserMarkId(UserMark::Type type); kml::TrackId GetNextTrackId(); kml::MarkGroupId GetNextCategoryId(); + bool CheckIds(kml::FileData const & fileData) const; + void ResetBookmarkId(); void ResetTrackId(); void ResetCategoryId(); @@ -37,12 +39,17 @@ private: bool HasKey(std::string const & name); - std::atomic m_isJustCreated; + bool m_isJustCreated; + //TODO: do we really need atomics here? std::atomic m_lastBookmarkId; std::atomic m_lastTrackId; std::atomic m_lastUserMarkId; std::atomic m_lastCategoryId; + uint64_t m_initialLastBookmarkId; + uint64_t m_initialLastTrackId; + uint64_t m_initialLastCategoryId; + bool m_testModeEnabled = false; };