diff --git a/kml/serdes.cpp b/kml/serdes.cpp index 0f74787e53..3441649b4a 100644 --- a/kml/serdes.cpp +++ b/kml/serdes.cpp @@ -562,7 +562,7 @@ void KmlParser::ParseLineCoordinates(std::string const & s, char const * blockSe m2::PointD pt; if (ParsePoint(*tupleIter, coordSeparator, pt)) { - if (m_points.empty() || !(pt - m_points.back()).IsAlmostZero()) + if (m_points.empty() || !pt.EqualDxDy(m_points.back(), 1e-5 /* eps */)) m_points.push_back(std::move(pt)); } ++tupleIter; diff --git a/map/CMakeLists.txt b/map/CMakeLists.txt index 739d48be2e..c421ed8833 100644 --- a/map/CMakeLists.txt +++ b/map/CMakeLists.txt @@ -90,6 +90,8 @@ set( transit/transit_reader.hpp user.cpp user.hpp + user_mark_id_storage.cpp + user_mark_id_storage.hpp user_mark_layer.cpp user_mark_layer.hpp user_mark.cpp diff --git a/map/bookmark_helpers.cpp b/map/bookmark_helpers.cpp index 7fbf7196c0..b709580a2e 100644 --- a/map/bookmark_helpers.cpp +++ b/map/bookmark_helpers.cpp @@ -8,8 +8,16 @@ std::unique_ptr LoadKmlFile(std::string const & file, bool useBinary) { - auto kmlData = LoadKmlData(FileReader(file), useBinary); - + std::unique_ptr kmlData; + try + { + kmlData = LoadKmlData(FileReader(file), useBinary); + } + catch (std::exception const & e) + { + LOG(LWARNING, ("KML", useBinary ? "binary" : "text", "loading failure:", e.what())); + kmlData.reset(); + } if (kmlData == nullptr) LOG(LWARNING, ("Loading bookmarks failed, file", file)); return kmlData; @@ -36,9 +44,19 @@ std::unique_ptr LoadKmlData(Reader const & reader, bool useBinary LOG(LWARNING, ("KML", useBinary ? "binary" : "text", "reading failure:", e.what())); return nullptr; } + catch (kml::binary::DeserializerKml::DeserializeException const & e) + { + LOG(LWARNING, ("KML binary deserialization failure:", e.what())); + return nullptr; + } + catch (kml::DeserializerKml::DeserializeException const & e) + { + LOG(LWARNING, ("KML text deserialization failure:", e.what())); + return nullptr; + } catch (std::exception const & e) { - LOG(LWARNING, ("KML", useBinary ? "binary" : "text", "deserialization failure:", e.what())); + LOG(LWARNING, ("KML", useBinary ? "binary" : "text", "loading failure:", e.what())); return nullptr; } return data; diff --git a/map/bookmark_manager.cpp b/map/bookmark_manager.cpp index 36a0c6bf1c..ffdd565c8f 100644 --- a/map/bookmark_manager.cpp +++ b/map/bookmark_manager.cpp @@ -5,6 +5,7 @@ #include "map/routing_mark.hpp" #include "map/search_mark.hpp" #include "map/user_mark.hpp" +#include "map/user_mark_id_storage.hpp" #include "drape_frontend/drape_engine.hpp" #include "drape_frontend/visual_params.hpp" @@ -41,7 +42,6 @@ using namespace std::placeholders; namespace { -std::string const kLastBookmarkCategoryId = "LastBookmarkCategoryId"; std::string const kLastEditedBookmarkCategory = "LastBookmarkCategory"; // TODO(darina): Delete old setting. std::string const kLastBookmarkType = "LastBookmarkType"; @@ -50,27 +50,6 @@ std::string const kKmzExtension = ".kmz"; std::string const kKmlExtension = ".kml"; std::string const kKmbExtension = ".kmb"; -uint64_t LoadLastBmCategoryId() -{ - uint64_t lastId; - std::string val; - if (GetPlatform().GetSecureStorage().Load(kLastBookmarkCategoryId, val) && strings::to_uint64(val, lastId)) - return std::max(static_cast(UserMark::USER_MARK_TYPES_COUNT_MAX), lastId); - return static_cast(UserMark::USER_MARK_TYPES_COUNT_MAX); -} - -void SaveLastBmCategoryId(uint64_t lastId) -{ - GetPlatform().GetSecureStorage().Save(kLastBookmarkCategoryId, strings::to_string(lastId)); -} - -uint64_t ResetLastBmCategoryId() -{ - auto const lastId = static_cast(UserMark::USER_MARK_TYPES_COUNT_MAX); - SaveLastBmCategoryId(lastId); - return lastId; -} - // Returns extension with a dot in a lower case. std::string GetFileExt(std::string const & filePath) { @@ -388,14 +367,13 @@ BookmarkManager::BookmarkManager(Callbacks && callbacks) : m_callbacks(std::move(callbacks)) , m_changesTracker(*this) , m_needTeardown(false) - , m_lastGroupID(LoadLastBmCategoryId()) , m_bookmarkCloud(Cloud::CloudParams("bmc.json", "bookmarks", "BookmarkCloudParam", GetBookmarksDirectory(), std::string(kKmbExtension), std::bind(&ConvertBeforeUploading, _1, _2), std::bind(&ConvertAfterDownloading, _1, _2))) { ASSERT(m_callbacks.m_getStringsBundle != nullptr, ()); - ASSERT_GREATER_OR_EQUAL(m_lastGroupID, UserMark::USER_MARK_TYPES_COUNT_MAX, ()); + m_userMarkLayers.reserve(UserMark::USER_MARK_TYPES_COUNT - 1); for (uint32_t i = 1; i < UserMark::USER_MARK_TYPES_COUNT; ++i) m_userMarkLayers.emplace_back(std::make_unique(static_cast(i))); @@ -1168,8 +1146,7 @@ kml::MarkGroupId BookmarkManager::CreateBookmarkCategory(kml::CategoryData && da if (data.m_id == kml::kInvalidMarkGroupId) { - data.m_id = ++m_lastGroupID; - SaveLastBmCategoryId(m_lastGroupID); + data.m_id = PersistentIdStorage::Instance().GetNextCategoryId(); } auto groupId = data.m_id; ASSERT_EQUAL(m_categories.count(groupId), 0, ()); @@ -1182,8 +1159,7 @@ kml::MarkGroupId BookmarkManager::CreateBookmarkCategory(kml::CategoryData && da kml::MarkGroupId BookmarkManager::CreateBookmarkCategory(std::string const & name, bool autoSave) { ASSERT_THREAD_CHECKER(m_threadChecker, ()); - auto const groupId = ++m_lastGroupID; - SaveLastBmCategoryId(m_lastGroupID); + auto const groupId = PersistentIdStorage::Instance().GetNextCategoryId(); m_categories[groupId] = my::make_unique(name, groupId, autoSave); m_bmGroupsIdList.push_back(groupId); m_changesTracker.OnAddGroup(groupId); @@ -1199,12 +1175,13 @@ void BookmarkManager::CheckAndCreateDefaultCategory() void BookmarkManager::CheckAndResetLastIds() { + auto & idStorage = PersistentIdStorage::Instance(); if (m_categories.empty()) - m_lastGroupID = ResetLastBmCategoryId(); + idStorage.ResetCategoryId(); if (m_bookmarks.empty()) - UserMark::ResetLastId(UserMark::BOOKMARK); + idStorage.ResetBookmarkId(); if (m_tracks.empty()) - Track::ResetLastId(); + idStorage.ResetTrackId(); } bool BookmarkManager::DeleteBmCategory(kml::MarkGroupId groupId) @@ -1329,7 +1306,7 @@ void BookmarkManager::CreateCategories(KMLDataCollection && dataCollection, bool } else { - bool const saveAfterCreation = categoryData.m_id == kml::kInvalidMarkGroupId; + bool const saveAfterCreation = autoSave && (categoryData.m_id == kml::kInvalidMarkGroupId); groupId = CreateBookmarkCategory(std::move(categoryData), saveAfterCreation); loadedGroups.insert(groupId); group = GetBmCategory(groupId); @@ -1461,7 +1438,16 @@ void BookmarkManager::SaveBookmarks(kml::GroupIdCollection const & groupIdCollec return; bool const migrated = migration::IsMigrationCompleted(); - GetPlatform().RunTask(Platform::Thread::File, [this, migrated, kmlDataCollection]() + + if (m_testModeEnabled) + { + // Save bookmarks synchronously. + for (auto const & kmlItem : *kmlDataCollection) + SaveKmlFileSafe(*kmlItem.second, kmlItem.first, migrated); + return; + } + + GetPlatform().RunTask(Platform::Thread::File, [this, migrated, kmlDataCollection = std::move(kmlDataCollection)]() { for (auto const & kmlItem : *kmlDataCollection) SaveKmlFileSafe(*kmlItem.second, kmlItem.first, migrated); @@ -1712,6 +1698,12 @@ void BookmarkManager::CancelCloudRestoring() m_bookmarkCloud.CancelRestoring(); } +void BookmarkManager::EnableTestMode(bool enable) +{ + PersistentIdStorage::Instance().EnableTestMode(enable); + m_testModeEnabled = enable; +} + kml::GroupIdSet BookmarkManager::MarksChangesTracker::GetAllGroupIds() const { auto const & groupIds = m_bmManager.GetBmGroupsIdList(); @@ -1880,6 +1872,12 @@ bool BookmarkManager::IsMigrated() return migration::IsMigrationCompleted(); } +// static +std::string BookmarkManager::GetActualBookmarksDirectory() +{ + return IsMigrated() ? GetBookmarksDirectory() : GetPlatform().SettingsDir(); +} + BookmarkManager::EditSession::EditSession(BookmarkManager & manager) : m_bmManager(manager) { diff --git a/map/bookmark_manager.hpp b/map/bookmark_manager.hpp index c3b1a8fb94..7bf6f22959 100644 --- a/map/bookmark_manager.hpp +++ b/map/bookmark_manager.hpp @@ -141,7 +141,6 @@ public: void UpdateViewport(ScreenBase const & screen); void Teardown(); - static bool IsBookmarkCategory(kml::MarkGroupId groupId) { return groupId >= UserMark::USER_MARK_TYPES_COUNT_MAX; } static bool IsBookmark(kml::MarkId markId) { return UserMark::GetMarkType(markId) == UserMark::BOOKMARK; } static UserMark::Type GetGroupType(kml::MarkGroupId groupId) @@ -265,6 +264,7 @@ public: void CancelCloudRestoring(); /// These functions are public for unit tests only. You shouldn't call them from client code. + void EnableTestMode(bool enable); bool SaveBookmarkCategory(kml::MarkGroupId groupId); bool SaveBookmarkCategory(kml::MarkGroupId groupId, Writer & writer, bool useBinary) const; void CreateCategories(KMLDataCollection && dataCollection, bool autoSave = true); @@ -272,6 +272,7 @@ public: static std::string GenerateUniqueFileName(std::string const & path, std::string name, std::string const & fileExt); static std::string GenerateValidAndUniqueFilePathForKML(std::string const & fileName); static std::string GenerateValidAndUniqueFilePathForKMB(std::string const & fileName); + static std::string GetActualBookmarksDirectory(); static bool IsMigrated(); private: @@ -479,5 +480,7 @@ private: Cloud::RestoreRequestedHandler m_onRestoreRequested; Cloud::RestoredFilesPreparedHandler m_onRestoredFilesPrepared; + bool m_testModeEnabled = false; + DISALLOW_COPY_AND_MOVE(BookmarkManager); }; diff --git a/map/map_tests/bookmarks_test.cpp b/map/map_tests/bookmarks_test.cpp index a0b6e4250b..e40a6de2c1 100644 --- a/map/map_tests/bookmarks_test.cpp +++ b/map/map_tests/bookmarks_test.cpp @@ -12,6 +12,7 @@ #include "platform/platform.hpp" #include "platform/preferred_languages.hpp" +#include "coding/file_name_utils.hpp" #include "coding/internal/file_data.hpp" #include @@ -158,8 +159,10 @@ void CheckBookmarks(BookmarkManager const & bmManager, kml::MarkGroupId groupId) bm = bmManager.GetBookmark(*it++); m2::PointD org = bm->GetPivot(); - TEST_ALMOST_EQUAL_ULPS(MercatorBounds::XToLon(org.x), 27.566765, ()); - TEST_ALMOST_EQUAL_ULPS(MercatorBounds::YToLat(org.y), 53.900047, ()); + + double const kEps = 1e-6; + TEST(my::AlmostEqualAbs(MercatorBounds::XToLon(org.x), 27.566765, kEps), ()); + TEST(my::AlmostEqualAbs(MercatorBounds::YToLat(org.y), 53.900047, kEps), ()); TEST_EQUAL(bm->GetName(), "From: Минск, Минская область, Беларусь", ()); TEST_EQUAL(bm->GetColor(), kml::PredefinedColor::Blue, ()); TEST_EQUAL(bm->GetDescription(), "", ()); @@ -167,8 +170,8 @@ void CheckBookmarks(BookmarkManager const & bmManager, kml::MarkGroupId groupId) bm = bmManager.GetBookmark(*it++); org = bm->GetPivot(); - TEST_ALMOST_EQUAL_ULPS(MercatorBounds::XToLon(org.x), 27.551532, ()); - TEST_ALMOST_EQUAL_ULPS(MercatorBounds::YToLat(org.y), 53.89306, ()); + TEST(my::AlmostEqualAbs(MercatorBounds::XToLon(org.x), 27.551532, kEps), ()); + TEST(my::AlmostEqualAbs(MercatorBounds::YToLat(org.y), 53.89306, kEps), ()); TEST_EQUAL(bm->GetName(), "", ()); TEST_EQUAL(bm->GetDescription(), "Amps & ", ()); TEST_EQUAL(kml::ToSecondsSinceEpoch(bm->GetTimeStamp()), 0, ()); @@ -178,6 +181,8 @@ void CheckBookmarks(BookmarkManager const & bmManager, kml::MarkGroupId groupId) UNIT_CLASS_TEST(Runner, Bookmarks_ImportKML) { BookmarkManager bmManager((BookmarkManager::Callbacks(bmCallbacks))); + bmManager.EnableTestMode(true); + BookmarkManager::KMLDataCollection kmlDataCollection; kmlDataCollection.emplace_back(""/* filePath */, @@ -196,14 +201,17 @@ UNIT_CLASS_TEST(Runner, Bookmarks_ImportKML) UNIT_CLASS_TEST(Runner, Bookmarks_ExportKML) { - std::string const fileName = GetPlatform().SettingsDir() + "UnitTestBookmarks.kml"; + string const dir = BookmarkManager::GetActualBookmarksDirectory(); + string const ext = BookmarkManager::IsMigrated() ? ".kmb" : BOOKMARKS_FILE_EXTENSION; + string const fileName = my::JoinPath(dir, "UnitTestBookmarks" + ext); BookmarkManager bmManager((BookmarkManager::Callbacks(bmCallbacks))); - BookmarkManager::KMLDataCollection kmlDataCollection; + bmManager.EnableTestMode(true); - kmlDataCollection.emplace_back(""/* filePath */, - LoadKmlData(MemReader(kmlString, strlen(kmlString)), false /* useBinary */)); - bmManager.CreateCategories(std::move(kmlDataCollection), false /* autoSave */); + BookmarkManager::KMLDataCollection kmlDataCollection1; + kmlDataCollection1.emplace_back("", + LoadKmlData(MemReader(kmlString, strlen(kmlString)), false /* useBinary */)); + bmManager.CreateCategories(std::move(kmlDataCollection1), false /* autoSave */); TEST_EQUAL(bmManager.GetBmGroupsIdList().size(), 1, ()); auto const groupId1 = bmManager.GetBmGroupsIdList().front(); @@ -217,7 +225,7 @@ UNIT_CLASS_TEST(Runner, Bookmarks_ExportKML) { FileWriter writer(fileName); - bmManager.SaveBookmarkCategory(groupId1, writer, false /* useBinary */); + bmManager.SaveBookmarkCategory(groupId1, writer, BookmarkManager::IsMigrated()); } bmManager.GetEditSession().ClearGroup(groupId1); @@ -227,11 +235,11 @@ UNIT_CLASS_TEST(Runner, Bookmarks_ExportKML) TEST_EQUAL(bmManager.HasBmCategory(groupId1), false, ()); TEST_EQUAL(bmManager.GetBmGroupsIdList().size(), 0, ()); - kmlDataCollection.clear(); - kmlDataCollection.emplace_back("", LoadKmlData(FileReader(fileName), false /* useBinary */)); - TEST(kmlDataCollection.back().second, ()); + BookmarkManager::KMLDataCollection kmlDataCollection2; + kmlDataCollection2.emplace_back("", LoadKmlData(FileReader(fileName), BookmarkManager::IsMigrated())); + TEST(kmlDataCollection2.back().second, ()); - bmManager.CreateCategories(std::move(kmlDataCollection), false /* autoSave */); + bmManager.CreateCategories(std::move(kmlDataCollection2), false /* autoSave */); TEST_EQUAL(bmManager.GetBmGroupsIdList().size(), 1, ()); auto const groupId2 = bmManager.GetBmGroupsIdList().front(); @@ -241,11 +249,11 @@ UNIT_CLASS_TEST(Runner, Bookmarks_ExportKML) bmManager.GetEditSession().DeleteBmCategory(groupId2); TEST_EQUAL(bmManager.HasBmCategory(groupId2), false, ()); - kmlDataCollection.clear(); - kmlDataCollection.emplace_back("", LoadKmlFile(fileName, false /* useBinary */)); - TEST(kmlDataCollection.back().second, ()); + BookmarkManager::KMLDataCollection kmlDataCollection3; + kmlDataCollection3.emplace_back(fileName, LoadKmlFile(fileName, BookmarkManager::IsMigrated())); + TEST(kmlDataCollection3.back().second, ()); - bmManager.CreateCategories(std::move(kmlDataCollection), false /* autoSave */); + bmManager.CreateCategories(std::move(kmlDataCollection3), BookmarkManager::IsMigrated()); TEST_EQUAL(bmManager.GetBmGroupsIdList().size(), 1, ()); auto const groupId3 = bmManager.GetBmGroupsIdList().front(); @@ -263,9 +271,10 @@ namespace { void DeleteCategoryFiles(vector const & arrFiles) { - string const path = GetPlatform().SettingsDir(); + string const path = BookmarkManager::GetActualBookmarksDirectory(); + string const extension = BookmarkManager::IsMigrated() ? ".kmb" : BOOKMARKS_FILE_EXTENSION; for (auto const & fileName : arrFiles) - FileWriter::DeleteFileX(path + fileName + BOOKMARKS_FILE_EXTENSION); + FileWriter::DeleteFileX(my::JoinPath(path, fileName + extension)); } UserMark const * GetMark(Framework & fm, m2::PointD const & pt) @@ -301,11 +310,11 @@ UNIT_TEST(Bookmarks_Timestamp) Framework fm(kFrameworkParams); df::VisualParams::Init(1.0, 1024); - m2::PointD const orgPoint(10, 10); - - vector const arrCat = {"cat", "cat1"}; - BookmarkManager & bmManager = fm.GetBookmarkManager(); + bmManager.EnableTestMode(true); + + m2::PointD const orgPoint(10, 10); + vector const arrCat = {"cat", "cat1"}; kml::BookmarkData b1; kml::SetDefaultStr(b1.m_name, "name"); @@ -357,6 +366,7 @@ UNIT_TEST(Bookmarks_Getting) //TEST(m2::AlmostEqualULPs(m2::PointD(400, 200), pixC), (pixC)); BookmarkManager & bmManager = fm.GetBookmarkManager(); + bmManager.EnableTestMode(true); vector const arrCat = {"cat1", "cat2", "cat3"}; @@ -516,6 +526,7 @@ UNIT_TEST(Bookmarks_AddingMoving) m2::PointD const pixelPoint = fm.GtoP(globalPoint); BookmarkManager & bmManager = fm.GetBookmarkManager(); + bmManager.EnableTestMode(true); vector const arrCat = {"cat1", "cat2"}; auto const cat1 = bmManager.CreateBookmarkCategory(arrCat[0], false /* autoSave */); @@ -530,6 +541,7 @@ UNIT_TEST(Bookmarks_AddingMoving) kml::BookmarkData bm2; kml::SetDefaultStr(bm2.m_name, "name2"); + bm2.m_point = globalPoint; bm2.m_color.m_predefinedColor = kml::PredefinedColor::Blue; auto const * pBm11 = bmManager.GetEditSession().CreateBookmark(std::move(bm2), cat1); TEST_EQUAL(pBm1->GetGroupId(), pBm11->GetGroupId(), ()); @@ -541,6 +553,7 @@ UNIT_TEST(Bookmarks_AddingMoving) // Edit name, type and category of bookmark kml::BookmarkData bm3; kml::SetDefaultStr(bm3.m_name, "name3"); + bm3.m_point = globalPoint; bm3.m_color.m_predefinedColor = kml::PredefinedColor::Green; auto const * pBm2 = bmManager.GetEditSession().CreateBookmark(std::move(bm3), cat2); TEST_NOT_EQUAL(pBm1->GetGroupId(), pBm2->GetGroupId(), ()); @@ -587,6 +600,8 @@ char const * kmlString2 = UNIT_CLASS_TEST(Runner, Bookmarks_InnerFolder) { BookmarkManager bmManager((BookmarkManager::Callbacks(bmCallbacks))); + bmManager.EnableTestMode(true); + BookmarkManager::KMLDataCollection kmlDataCollection; kmlDataCollection.emplace_back("" /* filePath */, @@ -600,6 +615,8 @@ UNIT_CLASS_TEST(Runner, Bookmarks_InnerFolder) UNIT_CLASS_TEST(Runner, BookmarkCategory_EmptyName) { BookmarkManager bmManager((BookmarkManager::Callbacks(bmCallbacks))); + bmManager.EnableTestMode(true); + auto const catId = bmManager.CreateBookmarkCategory("", false /* autoSave */); kml::BookmarkData bm; bm.m_point = m2::PointD(0, 0); @@ -643,7 +660,7 @@ char const * kmlString3 = return false; if (b1.GetScale() != b2.GetScale()) return false; - if (!m2::AlmostEqualULPs(b1.GetPivot(), b2.GetPivot())) + if (!my::AlmostEqualAbs(b1.GetPivot(), b2.GetPivot(), 1e-6 /* eps*/)) return false; // do not check timestamp @@ -654,48 +671,68 @@ char const * kmlString3 = UNIT_CLASS_TEST(Runner, Bookmarks_SpecialXMLNames) { BookmarkManager bmManager((BookmarkManager::Callbacks(bmCallbacks))); - BookmarkManager::KMLDataCollection kmlDataCollection; - kmlDataCollection.emplace_back("" /* filePath */, + bmManager.EnableTestMode(true); + + BookmarkManager::KMLDataCollection kmlDataCollection1; + kmlDataCollection1.emplace_back("" /* filePath */, LoadKmlData(MemReader(kmlString3, strlen(kmlString3)), false /* useBinary */)); - bmManager.CreateCategories(std::move(kmlDataCollection), false /* autoSave */); + bmManager.CreateCategories(std::move(kmlDataCollection1), false /* autoSave */); auto const & groupIds = bmManager.GetBmGroupsIdList(); TEST_EQUAL(groupIds.size(), 1, ()); + auto const catId = groupIds.front(); - auto const expectedName = "3663 and M & J Seafood Branches"; TEST_EQUAL(bmManager.GetUserMarkIds(catId).size(), 1, ()); + + string expectedName = "3663 and M & J Seafood Branches"; + TEST_EQUAL(bmManager.GetCategoryName(catId), expectedName, ()); + + // change category name to avoid merging it with the second one + expectedName = "test"; + bmManager.GetEditSession().SetCategoryName(catId, expectedName); + TEST(bmManager.SaveBookmarkCategory(catId), ()); - TEST_EQUAL(bmManager.GetCategoryName(catId), expectedName, ()); - // change category name to avoid merging it with the second one auto const fileName = bmManager.GetCategoryFileName(catId); - auto editSession = bmManager.GetEditSession(); - editSession.SetCategoryName(catId, "test"); + auto const fileNameTmp = fileName + ".backup"; + TEST(my::CopyFileX(fileName, fileNameTmp), ()); + + bmManager.GetEditSession().DeleteBmCategory(catId); + + BookmarkManager::KMLDataCollection kmlDataCollection2; + kmlDataCollection2.emplace_back("" /* filePath */, + LoadKmlFile(fileNameTmp, BookmarkManager::IsMigrated())); + bmManager.CreateCategories(std::move(kmlDataCollection2), false /* autoSave */); + + BookmarkManager::KMLDataCollection kmlDataCollection3; + kmlDataCollection3.emplace_back("" /* filePath */, + LoadKmlData(MemReader(kmlString3, strlen(kmlString3)), false /* useBinary */)); + bmManager.CreateCategories(std::move(kmlDataCollection3), false /* autoSave */); - kmlDataCollection.clear(); - kmlDataCollection.emplace_back(fileName, LoadKmlFile(fileName, BookmarkManager::IsMigrated())); - bmManager.CreateCategories(std::move(kmlDataCollection), false /* autoSave */); TEST_EQUAL(bmManager.GetBmGroupsIdList().size(), 2, ()); - auto const catId2 = bmManager.GetBmGroupsIdList().back(); + auto const catId2 = bmManager.GetBmGroupsIdList().front(); + auto const catId3 = bmManager.GetBmGroupsIdList().back(); TEST_EQUAL(bmManager.GetUserMarkIds(catId2).size(), 1, ()); TEST_EQUAL(bmManager.GetCategoryName(catId2), expectedName, ()); - TEST_EQUAL(bmManager.GetCategoryFileName(catId2), fileName, ()); + TEST_EQUAL(bmManager.GetCategoryFileName(catId2), "", ()); - auto const bmId1 = *bmManager.GetUserMarkIds(catId).begin(); + auto const bmId1 = *bmManager.GetUserMarkIds(catId2).begin(); auto const * bm1 = bmManager.GetBookmark(bmId1); - auto const bmId2 = *bmManager.GetUserMarkIds(catId2).begin(); + auto const bmId2 = *bmManager.GetUserMarkIds(catId3).begin(); auto const * bm2 = bmManager.GetBookmark(bmId2); TEST(EqualBookmarks(*bm1, *bm2), ()); TEST_EQUAL(bm1->GetName(), "![X1]{X2}(X3)", ()); - TEST(my::DeleteFileX(fileName), ()); + TEST(my::DeleteFileX(fileNameTmp), ()); } UNIT_CLASS_TEST(Runner, TrackParsingTest_1) { string const kmlFile = GetPlatform().TestsDataPathForFile("kml-with-track-kml.test"); BookmarkManager bmManager((BookmarkManager::Callbacks(bmCallbacks))); + bmManager.EnableTestMode(true); + BookmarkManager::KMLDataCollection kmlDataCollection; kmlDataCollection.emplace_back(kmlFile, LoadKmlFile(kmlFile, false /* useBinary */)); TEST(kmlDataCollection.back().second, ("KML can't be loaded")); @@ -729,6 +766,8 @@ UNIT_CLASS_TEST(Runner, TrackParsingTest_2) { string const kmlFile = GetPlatform().TestsDataPathForFile("kml-with-track-from-google-earth.test"); BookmarkManager bmManager((BookmarkManager::Callbacks(bmCallbacks))); + bmManager.EnableTestMode(true); + BookmarkManager::KMLDataCollection kmlDataCollection; kmlDataCollection.emplace_back(kmlFile, LoadKmlFile(kmlFile, false /* useBinary */)); @@ -794,6 +833,7 @@ UNIT_CLASS_TEST(Runner, Bookmarks_Listeners) onDelete); BookmarkManager bmManager(std::move(callbacks)); + bmManager.EnableTestMode(true); auto const catId = bmManager.CreateBookmarkCategory("Default", false /* autoSave */); @@ -841,11 +881,13 @@ UNIT_CLASS_TEST(Runner, Bookmarks_Listeners) UNIT_CLASS_TEST(Runner, Bookmarks_AutoSave) { BookmarkManager bmManager((BookmarkManager::Callbacks(bmCallbacks))); - kml::BookmarkData data0; + bmManager.EnableTestMode(true); + kml::MarkId bmId0; auto const catId = bmManager.CreateBookmarkCategory("test"); - data0.m_point = m2::PointD(0.0, 0.0); { + kml::BookmarkData data0; + data0.m_point = m2::PointD(0.0, 0.0); kml::SetDefaultStr(data0.m_name, "name 0"); auto editSession = bmManager.GetEditSession(); bmId0 = editSession.CreateBookmark(std::move(data0))->GetId(); diff --git a/map/map_tests/kmz_unarchive_test.cpp b/map/map_tests/kmz_unarchive_test.cpp index 2148254f0b..60b3d5842c 100644 --- a/map/map_tests/kmz_unarchive_test.cpp +++ b/map/map_tests/kmz_unarchive_test.cpp @@ -2,6 +2,7 @@ #include "map/bookmark_helpers.hpp" #include "map/framework.hpp" +#include "map/user_mark_id_storage.hpp" #include "platform/platform.hpp" @@ -16,6 +17,8 @@ UNIT_TEST(KMZ_UnzipTest) { + PersistentIdStorage::Instance().EnableTestMode(true); + string const kmzFile = GetPlatform().TestsDataPathForFile("test.kmz"); ZipFileReader::FileListT files; ZipFileReader::FilesList(kmzFile, files); diff --git a/map/track.cpp b/map/track.cpp index 90158e219c..9f0ad61bf5 100644 --- a/map/track.cpp +++ b/map/track.cpp @@ -1,50 +1,11 @@ #include "map/track.hpp" +#include "map/user_mark_id_storage.hpp" #include "geometry/distance_on_sphere.hpp" #include "geometry/mercator.hpp" -#include "platform/platform.hpp" - -#include "base/string_utils.hpp" - -namespace -{ -static const std::string kLastLineId = "LastLineId"; - -uint64_t LoadLastLineId() -{ - uint64_t lastId; - std::string val; - if (GetPlatform().GetSecureStorage().Load(kLastLineId, val) && strings::to_uint64(val, lastId)) - return lastId; - return 0; -} - -void SaveLastLineId(uint64_t lastId) -{ - GetPlatform().GetSecureStorage().Save(kLastLineId, strings::to_string(lastId)); -} - -kml::TrackId GetNextUserLineId(bool reset = false) -{ - static std::atomic lastLineId(LoadLastLineId()); - - if (reset) - { - SaveLastLineId(0); - lastLineId = 0; - return kml::kInvalidTrackId; - } - - auto const id = static_cast(++lastLineId); - SaveLastLineId(lastLineId); - return id; -} - -} // namespace - Track::Track(kml::TrackData && data) - : Base(data.m_id == kml::kInvalidTrackId ? GetNextUserLineId() : data.m_id) + : Base(data.m_id == kml::kInvalidTrackId ? PersistentIdStorage::Instance().GetNextTrackId() : data.m_id) , m_data(std::move(data)) , m_groupID(0) { @@ -52,12 +13,6 @@ Track::Track(kml::TrackData && data) ASSERT_GREATER(m_data.m_points.size(), 1, ()); } -// static -void Track::ResetLastId() -{ - UNUSED_VALUE(GetNextUserLineId(true /* reset */)); -} - string Track::GetName() const { return kml::GetDefaultStr(m_data.m_name); diff --git a/map/track.hpp b/map/track.hpp index 38b767509e..f4fcfd986d 100644 --- a/map/track.hpp +++ b/map/track.hpp @@ -10,8 +10,6 @@ class Track : public df::UserLineMark public: explicit Track(kml::TrackData && data); - static void ResetLastId(); - bool IsDirty() const override { return m_isDirty; } void ResetChanges() const override { m_isDirty = false; } diff --git a/map/user_mark.cpp b/map/user_mark.cpp index ab6f02e7ea..85c2105aa0 100644 --- a/map/user_mark.cpp +++ b/map/user_mark.cpp @@ -1,88 +1,24 @@ #include "map/user_mark.hpp" -#include "map/user_mark_layer.hpp" - -#include "indexer/classificator.hpp" +#include "map/user_mark_id_storage.hpp" #include "geometry/mercator.hpp" -#include "platform/platform.hpp" - -#include "base/string_utils.hpp" - -#include - -namespace -{ -static const uint32_t kMarkIdTypeBitsCount = 4; -static const std::string kLastBookmarkId = "LastBookmarkId"; - -uint64_t LoadLastBookmarkId() -{ - uint64_t lastId; - std::string val; - if (GetPlatform().GetSecureStorage().Load(kLastBookmarkId, val) && strings::to_uint64(val, lastId)) - return lastId; - return 0; -} - -void SaveLastBookmarkId(uint64_t lastId) -{ - GetPlatform().GetSecureStorage().Save(kLastBookmarkId, strings::to_string(lastId)); -} - -kml::MarkId GetNextUserMarkId(UserMark::Type type, bool reset = false) -{ - static std::atomic lastBookmarkId(LoadLastBookmarkId()); - static std::atomic lastUserMarkId(0); - - if (reset) - { - if (type == UserMark::Type::BOOKMARK) - { - SaveLastBookmarkId(0); - lastBookmarkId = 0; - } - return kml::kInvalidMarkId; - } - - static_assert(UserMark::Type::USER_MARK_TYPES_COUNT <= (1 << kMarkIdTypeBitsCount), "Not enough bits for user mark type."); - - auto const typeBits = static_cast(type) << (sizeof(kml::MarkId) * 8 - kMarkIdTypeBitsCount); - if (type == UserMark::Type::BOOKMARK) - { - auto const id = static_cast((++lastBookmarkId) | typeBits); - SaveLastBookmarkId(lastBookmarkId); - return id; - } - else - { - return static_cast((++lastUserMarkId) | typeBits); - } -} -} // namespace - UserMark::UserMark(kml::MarkId id, m2::PointD const & ptOrg, UserMark::Type type) - : df::UserPointMark(id == kml::kInvalidMarkId ? GetNextUserMarkId(type) : id) + : df::UserPointMark(id == kml::kInvalidMarkId ? PersistentIdStorage::Instance().GetNextUserMarkId(type) : id) , m_ptOrg(ptOrg) { ASSERT_EQUAL(GetMarkType(), type, ()); } UserMark::UserMark(m2::PointD const & ptOrg, UserMark::Type type) - : df::UserPointMark(GetNextUserMarkId(type)) + : df::UserPointMark(PersistentIdStorage::Instance().GetNextUserMarkId(type)) , m_ptOrg(ptOrg) {} // static UserMark::Type UserMark::GetMarkType(kml::MarkId id) { - return static_cast(id >> (sizeof(id) * 8 - kMarkIdTypeBitsCount)); -} - -// static -void UserMark::ResetLastId(UserMark::Type type) -{ - UNUSED_VALUE(GetNextUserMarkId(type, true /* reset */)); + return PersistentIdStorage::GetMarkType(id); } m2::PointD const & UserMark::GetPivot() const diff --git a/map/user_mark.hpp b/map/user_mark.hpp index fd20ca58f9..e60823cea3 100644 --- a/map/user_mark.hpp +++ b/map/user_mark.hpp @@ -48,7 +48,6 @@ public: UserMark(m2::PointD const & ptOrg, UserMark::Type type); static Type GetMarkType(kml::MarkId id); - static void ResetLastId(UserMark::Type type); Type GetMarkType() const { return GetMarkType(GetId()); } kml::MarkGroupId GetGroupId() const override { return GetMarkType(); } diff --git a/map/user_mark_id_storage.cpp b/map/user_mark_id_storage.cpp new file mode 100644 index 0000000000..5660f664b2 --- /dev/null +++ b/map/user_mark_id_storage.cpp @@ -0,0 +1,140 @@ +#include "map/user_mark_id_storage.hpp" + +#include "platform/platform.hpp" + +#include "base/string_utils.hpp" + +namespace +{ +uint32_t const kMarkIdTypeBitsCount = 4; +std::string const kLastBookmarkId = "LastBookmarkId"; +std::string const kLastTrackId = "LastTrackId"; +std::string const kLastBookmarkCategoryId = "LastBookmarkCategoryId"; +} // namespace + +PersistentIdStorage::PersistentIdStorage() + : m_lastUserMarkId(0) +{ + LoadLastBookmarkId(); + LoadLastTrackId(); + LoadLastCategoryId(); +} + +// static +PersistentIdStorage & PersistentIdStorage::Instance() +{ + static PersistentIdStorage instance; + return instance; +} + +// static +UserMark::Type PersistentIdStorage::GetMarkType(kml::MarkId id) +{ + return static_cast(id >> (sizeof(id) * 8 - kMarkIdTypeBitsCount)); +} + +void PersistentIdStorage::ResetBookmarkId() +{ + m_lastBookmarkId = 0; + SaveLastBookmarkId(); +} + +void PersistentIdStorage::ResetTrackId() +{ + m_lastTrackId = 0; + SaveLastTrackId(); +} + +void PersistentIdStorage::ResetCategoryId() +{ + m_lastCategoryId = static_cast(UserMark::Type::USER_MARK_TYPES_COUNT_MAX); + SaveLastCategoryId(); +} + +kml::MarkId PersistentIdStorage::GetNextUserMarkId(UserMark::Type type) +{ + static_assert(UserMark::Type::USER_MARK_TYPES_COUNT <= (1 << kMarkIdTypeBitsCount), + "Not enough bits for user mark type."); + + auto const typeBits = static_cast(type) << (sizeof(kml::MarkId) * 8 - kMarkIdTypeBitsCount); + if (type == UserMark::Type::BOOKMARK) + { + auto const id = static_cast((++m_lastBookmarkId) | typeBits); + SaveLastBookmarkId(); + return id; + } + else + { + return static_cast((++m_lastUserMarkId) | typeBits); + } +} + +kml::TrackId PersistentIdStorage::GetNextTrackId() +{ + auto const id = static_cast(++m_lastTrackId); + SaveLastTrackId(); + return id; +} + +kml::MarkGroupId PersistentIdStorage::GetNextCategoryId() +{ + auto const id = static_cast(++m_lastCategoryId); + SaveLastCategoryId(); + return id; +} + +void PersistentIdStorage::LoadLastBookmarkId() +{ + uint64_t lastId; + std::string val; + if (GetPlatform().GetSecureStorage().Load(kLastBookmarkId, val) && strings::to_uint64(val, lastId)) + m_lastBookmarkId = lastId; + else + m_lastBookmarkId = 0; +} + +void PersistentIdStorage::LoadLastTrackId() +{ + uint64_t lastId; + std::string val; + if (GetPlatform().GetSecureStorage().Load(kLastTrackId, val) && strings::to_uint64(val, lastId)) + m_lastTrackId = lastId; + else + m_lastTrackId = 0; +} + +void PersistentIdStorage::LoadLastCategoryId() +{ + uint64_t lastId; + std::string val; + if (GetPlatform().GetSecureStorage().Load(kLastBookmarkCategoryId, val) && strings::to_uint64(val, lastId)) + m_lastCategoryId = std::max(static_cast(UserMark::USER_MARK_TYPES_COUNT_MAX), lastId); + else + m_lastCategoryId = static_cast(UserMark::USER_MARK_TYPES_COUNT_MAX); +} + +void PersistentIdStorage::SaveLastBookmarkId() +{ + if (m_testModeEnabled) + return; + GetPlatform().GetSecureStorage().Save(kLastBookmarkId, strings::to_string(m_lastBookmarkId.load())); +} + +void PersistentIdStorage::SaveLastTrackId() +{ + if (m_testModeEnabled) + return; + GetPlatform().GetSecureStorage().Save(kLastTrackId, strings::to_string(m_lastTrackId.load())); +} + +void PersistentIdStorage::SaveLastCategoryId() +{ + if (m_testModeEnabled) + return; + GetPlatform().GetSecureStorage().Save(kLastBookmarkCategoryId, strings::to_string(m_lastCategoryId.load())); +} + +void PersistentIdStorage::EnableTestMode(bool enable) +{ + m_testModeEnabled = enable; +} diff --git a/map/user_mark_id_storage.hpp b/map/user_mark_id_storage.hpp new file mode 100644 index 0000000000..f52ed55e55 --- /dev/null +++ b/map/user_mark_id_storage.hpp @@ -0,0 +1,42 @@ +#pragma once + +#include "map/user_mark.hpp" + +#include + +class PersistentIdStorage +{ +public: + static PersistentIdStorage & Instance(); + + static UserMark::Type GetMarkType(kml::MarkId id); + + kml::MarkId GetNextUserMarkId(UserMark::Type type); + kml::TrackId GetNextTrackId(); + kml::MarkGroupId GetNextCategoryId(); + + void ResetBookmarkId(); + void ResetTrackId(); + void ResetCategoryId(); + + // Disable saving. For test purpose only! + void EnableTestMode(bool enable); + +private: + PersistentIdStorage(); + + void LoadLastBookmarkId(); + void LoadLastTrackId(); + void LoadLastCategoryId(); + + void SaveLastBookmarkId(); + void SaveLastTrackId(); + void SaveLastCategoryId(); + + std::atomic m_lastBookmarkId; + std::atomic m_lastTrackId; + std::atomic m_lastUserMarkId; + std::atomic m_lastCategoryId; + + bool m_testModeEnabled = false; +}; diff --git a/xcode/map/map.xcodeproj/project.pbxproj b/xcode/map/map.xcodeproj/project.pbxproj index 7c0ad7ce1e..22f72b7191 100644 --- a/xcode/map/map.xcodeproj/project.pbxproj +++ b/xcode/map/map.xcodeproj/project.pbxproj @@ -129,6 +129,8 @@ BB4E5F281FCC664A00A77250 /* transit_reader.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BB4E5F241FCC664A00A77250 /* transit_reader.cpp */; }; BBA014AD2073C784007402E4 /* bookmark_helpers.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BBA014AB2073C783007402E4 /* bookmark_helpers.cpp */; }; BBA014AE2073C784007402E4 /* bookmark_helpers.hpp in Headers */ = {isa = PBXBuildFile; fileRef = BBA014AC2073C784007402E4 /* bookmark_helpers.hpp */; }; + BBA014B120754997007402E4 /* user_mark_id_storage.hpp in Headers */ = {isa = PBXBuildFile; fileRef = BBA014AF20754996007402E4 /* user_mark_id_storage.hpp */; }; + BBA014B220754997007402E4 /* user_mark_id_storage.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BBA014B020754996007402E4 /* user_mark_id_storage.cpp */; }; BBD9E2C61EE9D01900DF189A /* routing_mark.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BBD9E2C41EE9D01900DF189A /* routing_mark.cpp */; }; BBD9E2C71EE9D01900DF189A /* routing_mark.hpp in Headers */ = {isa = PBXBuildFile; fileRef = BBD9E2C51EE9D01900DF189A /* routing_mark.hpp */; }; BBFC7E3A202D29C000531BE7 /* user_mark_layer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = BBFC7E38202D29BF00531BE7 /* user_mark_layer.cpp */; }; @@ -300,6 +302,8 @@ BB4E5F241FCC664A00A77250 /* transit_reader.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = transit_reader.cpp; path = transit/transit_reader.cpp; sourceTree = ""; }; BBA014AB2073C783007402E4 /* bookmark_helpers.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = bookmark_helpers.cpp; sourceTree = ""; }; BBA014AC2073C784007402E4 /* bookmark_helpers.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = bookmark_helpers.hpp; sourceTree = ""; }; + BBA014AF20754996007402E4 /* user_mark_id_storage.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = user_mark_id_storage.hpp; sourceTree = ""; }; + BBA014B020754996007402E4 /* user_mark_id_storage.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = user_mark_id_storage.cpp; sourceTree = ""; }; BBD9E2C41EE9D01900DF189A /* routing_mark.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = routing_mark.cpp; sourceTree = ""; }; BBD9E2C51EE9D01900DF189A /* routing_mark.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = routing_mark.hpp; sourceTree = ""; }; BBFC7E38202D29BF00531BE7 /* user_mark_layer.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = user_mark_layer.cpp; sourceTree = ""; }; @@ -592,6 +596,8 @@ BB4E5F201FCC663700A77250 /* transit */, 674C385F1BFF3095000D603B /* user_mark.cpp */, 675346331A4054E800A0A8C3 /* user_mark.hpp */, + BBA014B020754996007402E4 /* user_mark_id_storage.cpp */, + BBA014AF20754996007402E4 /* user_mark_id_storage.hpp */, 45A2D9D31F7556EB003310A0 /* user.cpp */, 45A2D9D41F7556EB003310A0 /* user.hpp */, 3D4E99811FB462B60025B48C /* viewport_search_params.hpp */, @@ -668,6 +674,7 @@ F6FC3CB71FC323430001D929 /* discovery_manager.hpp in Headers */, 6753469C1A4054E800A0A8C3 /* track.hpp in Headers */, 675346651A4054E800A0A8C3 /* framework.hpp in Headers */, + BBA014B120754997007402E4 /* user_mark_id_storage.hpp in Headers */, 674A2A381B2715FB001A525C /* osm_opening_hours.hpp in Headers */, F6D2CE7F1EDEB7F500636DFD /* routing_manager.hpp in Headers */, 670E39411C46C5C700E9C0A6 /* gps_tracker.hpp in Headers */, @@ -815,6 +822,7 @@ F6B283071C1B03320081957A /* gps_track_storage.cpp in Sources */, 6753463A1A4054E800A0A8C3 /* address_finder.cpp in Sources */, 670E39401C46C5C700E9C0A6 /* gps_tracker.cpp in Sources */, + BBA014B220754997007402E4 /* user_mark_id_storage.cpp in Sources */, 6753464A1A4054E800A0A8C3 /* bookmark.cpp in Sources */, 45580ABE1E2CBD5E00CD535D /* benchmark_tools.cpp in Sources */, BBD9E2C61EE9D01900DF189A /* routing_mark.cpp in Sources */,