diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp index 2ed9a5831c..5d21b2261c 100644 --- a/android/jni/com/mapswithme/maps/Framework.cpp +++ b/android/jni/com/mapswithme/maps/Framework.cpp @@ -690,7 +690,7 @@ namespace android return m_bmBaloon.get(); } - BookmarkAndCategory Framework::AddBookmark(Bookmark const & bm) + BookmarkAndCategory Framework::AddBookmark(Bookmark & bm) { BookmarkAndCategory const bac = m_work.AddBookmarkEx(m_bmCategory, bm); BookmarkCategory * cat = m_work.GetBmCategory(bac.first); @@ -699,7 +699,7 @@ namespace android return bac; } - void Framework::AddBookmark(string const & category, Bookmark const & bm) + void Framework::AddBookmark(string const & category, Bookmark & bm) { m_bmCategory = category; m_bmType = bm.GetType(); diff --git a/android/jni/com/mapswithme/maps/Framework.hpp b/android/jni/com/mapswithme/maps/Framework.hpp index 2168846781..2d7336f527 100644 --- a/android/jni/com/mapswithme/maps/Framework.hpp +++ b/android/jni/com/mapswithme/maps/Framework.hpp @@ -77,7 +77,7 @@ namespace android void OnBalloonClick(gui::Element * e); void CreateBookmarkBalloon(); BookmarkBalloon * GetBookmarkBalloon(); - BookmarkAndCategory AddBookmark(Bookmark const & bm); + BookmarkAndCategory AddBookmark(Bookmark & bm); public: Framework(); @@ -139,7 +139,7 @@ namespace android void RemoveBalloonClickListener(); void DeactivatePopup(); - void AddBookmark(string const & category, Bookmark const & bm); + void AddBookmark(string const & category, Bookmark & bm); ::Framework * NativeFramework(); }; diff --git a/iphone/Maps/Bookmarks/BalloonView.mm b/iphone/Maps/Bookmarks/BalloonView.mm index 57e07c7b89..915ab452e6 100644 --- a/iphone/Maps/Bookmarks/BalloonView.mm +++ b/iphone/Maps/Bookmarks/BalloonView.mm @@ -226,12 +226,13 @@ // If coordinates are be the same, bookmark is automatically replaced Bookmark bm(m2::PointD(self.globalPosition.x, self.globalPosition.y), [self.title UTF8String], [self.color UTF8String]); - bm.SetTimeStamp(time(0)); + BookmarkCategory * cat = GetFramework().AddBookmark([self.setName UTF8String], bm); // Enable category visibility if it was turned off, so user can see newly added or edited bookmark if (!cat->IsVisible()) cat->SetVisible(true); + // Save all changes cat->SaveToKMLFile(); } diff --git a/map/bookmark.cpp b/map/bookmark.cpp index 2d22202692..9b7e820d51 100644 --- a/map/bookmark.cpp +++ b/map/bookmark.cpp @@ -16,36 +16,27 @@ #include "../std/auto_ptr.hpp" -void BookmarkCategory::AddBookmarkImpl(Bookmark const & bm, double scale) +void BookmarkCategory::AddBookmark(Bookmark const & bm) { Bookmark * p = new Bookmark(bm); - p->SetScale(scale); - m_bookmarks.push_back(p); } -void BookmarkCategory::AddBookmark(Bookmark const & bm, double scale) -{ - AddBookmarkImpl(bm, scale); -} - -void BookmarkCategory::ReplaceBookmark(size_t index, Bookmark const & bm, double scale) +void BookmarkCategory::ReplaceBookmark(size_t index, Bookmark const & bm) { ASSERT_LESS ( index, m_bookmarks.size(), () ); if (index < m_bookmarks.size()) { - Bookmark * p = new Bookmark(bm); - p->SetScale(scale); - - Bookmark const * old = m_bookmarks[index]; - // Save creation time stamp - p->SetTimeStamp(old->GetTimeStamp()); - m_bookmarks[index] = p; - - delete old; + delete m_bookmarks[index]; + m_bookmarks[index] = new Bookmark(bm); } - else - LOG(LWARNING, ("Trying to replace non-existing bookmark")); +} + +void BookmarkCategory::AssignTimeStamp(size_t index, Bookmark & bm) const +{ + ASSERT_LESS ( index, m_bookmarks.size(), () ); + if (index < m_bookmarks.size()) + bm.SetTimeStamp(m_bookmarks[index]->GetTimeStamp()); } BookmarkCategory::~BookmarkCategory() @@ -207,7 +198,8 @@ namespace bookmark_impl Bookmark bm(m_org, m_name, m_type); bm.SetTimeStamp(m_timeStamp); bm.SetDescription(m_description); - m_category.AddBookmarkImpl(bm, m_scale); + bm.SetScale(m_scale); + m_category.AddBookmark(bm); Reset(); } m_tags.pop_back(); diff --git a/map/bookmark.hpp b/map/bookmark.hpp index 3cfa470ae9..20f0ca9724 100644 --- a/map/bookmark.hpp +++ b/map/bookmark.hpp @@ -58,9 +58,6 @@ class BookmarkCategory : private noncopyable /// Stores file name from which category was loaded string m_file; - friend class bookmark_impl::KMLParser; - void AddBookmarkImpl(Bookmark const & bm, double scale); - public: BookmarkCategory(string const & name) : m_name(name), m_visible(true) {} ~BookmarkCategory(); @@ -69,8 +66,12 @@ public: /// @name Theese functions are called from Framework only. //@{ - void AddBookmark(Bookmark const & bm, double scale); - void ReplaceBookmark(size_t index, Bookmark const & bm, double scale); + void AddBookmark(Bookmark const & bm); + void ReplaceBookmark(size_t index, Bookmark const & bm); + + /// This function is called when bookmark is editing or replacing. + /// We need to assign times to the newly created bookmark from the old one. + void AssignTimeStamp(size_t index, Bookmark & bm) const; //@} void SetVisible(bool isVisible) { m_visible = isVisible; } diff --git a/map/framework.cpp b/map/framework.cpp index 8b31bfb7ca..16d5bff5f5 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -392,11 +392,12 @@ void Framework::LoadBookmark(string const & filePath) } } -BookmarkAndCategory Framework::AddBookmarkEx(string const & category, Bookmark const & bm) +BookmarkAndCategory Framework::AddBookmarkEx(string const & category, Bookmark & bm) { // Get global non-rotated viewport rect and calculate viewport scale level. - double const scale = scales::GetScaleLevelD( - m_navigator.Screen().GlobalRect().GetLocalRect()); + bm.SetScale(scales::GetScaleLevelD(m_navigator.Screen().GlobalRect().GetLocalRect())); + + bm.SetTimeStamp(time(0)); // @TODO not optimal for 1st release // Existing bookmark can be moved from one category to another, @@ -410,27 +411,32 @@ BookmarkAndCategory Framework::AddBookmarkEx(string const & category, Bookmark c int const index = cat->GetBookmark(org, squareDistance); if (index >= 0) { - // found bookmark to replace + size_t const ind = static_cast(index); + + // copy needed params from the old bookmark + cat->AssignTimeStamp(ind, bm); + if (category == cat->GetName()) { - cat->ReplaceBookmark(static_cast(index), bm, scale); + // found bookmark to replace + cat->ReplaceBookmark(ind, bm); return BookmarkAndCategory(i, index); } else { - // Bookmark was moved from one category to another - cat->DeleteBookmark(static_cast(index)); + // bookmark was moved from one category to another + cat->DeleteBookmark(ind); } } } size_t const ind = GetBmCategoryEx(category); BookmarkCategory * cat = m_bookmarks[ind]; - cat->AddBookmark(bm, scale); + cat->AddBookmark(bm); return BookmarkAndCategory(ind, cat->GetBookmarksCount()-1); } -BookmarkCategory * Framework::AddBookmark(string const & category, Bookmark const & bm) +BookmarkCategory * Framework::AddBookmark(string const & category, Bookmark & bm) { return m_bookmarks[AddBookmarkEx(category, bm).first]; } diff --git a/map/framework.hpp b/map/framework.hpp index ad75aac95f..674afbefac 100644 --- a/map/framework.hpp +++ b/map/framework.hpp @@ -188,8 +188,8 @@ public: /// @name Always returns existing or newly created bookmark category. //@{ - BookmarkCategory * AddBookmark(string const & category, Bookmark const & bm); - BookmarkAndCategory AddBookmarkEx(string const & category, Bookmark const & bm); + BookmarkCategory * AddBookmark(string const & category, Bookmark & bm); + BookmarkAndCategory AddBookmarkEx(string const & category, Bookmark & bm); //@} inline size_t GetBmCategoriesCount() const { return m_bookmarks.size(); } diff --git a/map/map_tests/bookmarks_test.cpp b/map/map_tests/bookmarks_test.cpp index 17c11c4c1d..7b5e8280f8 100644 --- a/map/map_tests/bookmarks_test.cpp +++ b/map/map_tests/bookmarks_test.cpp @@ -206,35 +206,44 @@ namespace for (size_t i = 0; i < N; ++i) FileWriter::DeleteFileX(path + arrFiles[i] + ".kml"); } + + Bookmark const * GetBookmark(Framework const & fm, m2::PointD const & pt) + { + BookmarkAndCategory const res = fm.GetBookmark(fm.GtoP(pt), 1.0); + return fm.GetBmCategory(res.first)->GetBookmark(res.second); + } } UNIT_TEST(Bookmarks_Timestamp) { - time_t const timeStamp = time(0); - m2::PointD const orgPoint(10, 10); - Bookmark b1(orgPoint, "name", "type"); - b1.SetTimeStamp(timeStamp); - TEST_NOT_EQUAL(b1.GetTimeStamp(), my::INVALID_TIME_STAMP, ()); - Framework fm; + m2::PointD const orgPoint(10, 10); + + Bookmark b1(orgPoint, "name", "type"); fm.AddBookmark("cat", b1); - BookmarkAndCategory res = fm.GetBookmark(fm.GtoP(orgPoint), 1.0); - Bookmark const * b2 = fm.GetBmCategory(res.first)->GetBookmark(res.second); - TEST_NOT_EQUAL(b2->GetTimeStamp(), my::INVALID_TIME_STAMP, ()); - TEST_EQUAL(b2->GetTimeStamp(), timeStamp, ()); + Bookmark const * pBm = GetBookmark(fm, orgPoint); + time_t const timeStamp = pBm->GetTimeStamp(); + TEST_NOT_EQUAL(timeStamp, my::INVALID_TIME_STAMP, ()); // Replace/update bookmark Bookmark b3(orgPoint, "newName", "newType"); b3.SetTimeStamp(12345); - TEST_NOT_EQUAL(b3.GetTimeStamp(), b2->GetTimeStamp(), ()); + TEST_NOT_EQUAL(b3.GetTimeStamp(), timeStamp, ()); fm.AddBookmark("cat", b3); + pBm = GetBookmark(fm, orgPoint); + TEST_EQUAL(pBm->GetTimeStamp(), timeStamp, ()); - res = fm.GetBookmark(fm.GtoP(orgPoint), 1.0); - Bookmark const * b4 = fm.GetBmCategory(res.first)->GetBookmark(res.second); - TEST_EQUAL(b4->GetTimeStamp(), timeStamp, ()); + b3.SetTimeStamp(12345); + TEST_NOT_EQUAL(b3.GetTimeStamp(), timeStamp, ()); + + // Move bookmark to the new category + fm.AddBookmark("cat1", b3); + + pBm = GetBookmark(fm, orgPoint); + TEST_EQUAL(pBm->GetTimeStamp(), timeStamp, ()); } UNIT_TEST(Bookmarks_Getting) @@ -248,9 +257,12 @@ UNIT_TEST(Bookmarks_Getting) char const * arrCat[] = { "cat1", "cat2", "cat3" }; - BookmarkCategory const * c1 = fm.AddBookmark(arrCat[0], Bookmark(m2::PointD(38, 20), "1", "placemark-red")); - BookmarkCategory const * c2 = fm.AddBookmark(arrCat[1], Bookmark(m2::PointD(41, 20), "2", "placemark-red")); - BookmarkCategory const * c3 = fm.AddBookmark(arrCat[2], Bookmark(m2::PointD(41, 40), "3", "placemark-red")); + Bookmark bm(m2::PointD(38, 20), "1", "placemark-red"); + BookmarkCategory const * c1 = fm.AddBookmark(arrCat[0], bm); + bm = Bookmark(m2::PointD(41, 20), "2", "placemark-red"); + BookmarkCategory const * c2 = fm.AddBookmark(arrCat[1], bm); + bm = Bookmark(m2::PointD(41, 40), "3", "placemark-red"); + BookmarkCategory const * c3 = fm.AddBookmark(arrCat[2], bm); TEST_NOT_EQUAL(c1, c2, ()); TEST_NOT_EQUAL(c2, c3, ()); @@ -274,12 +286,13 @@ UNIT_TEST(Bookmarks_Getting) TEST(IsValid(res), ()); TEST_EQUAL(res.first, 2, ()); TEST_EQUAL(fm.GetBmCategory(res.first)->GetName(), arrCat[2], ()); - Bookmark const * bm = fm.GetBmCategory(res.first)->GetBookmark(res.second); - TEST_EQUAL(bm->GetName(), "3", ()); - TEST_EQUAL(bm->GetType(), "placemark-red", ()); + Bookmark const * pBm = fm.GetBmCategory(res.first)->GetBookmark(res.second); + TEST_EQUAL(pBm->GetName(), "3", ()); + TEST_EQUAL(pBm->GetType(), "placemark-red", ()); // This one should replace previous bookmark - BookmarkCategory const * c33 = fm.AddBookmark(arrCat[2], Bookmark(m2::PointD(41, 40), "4", "placemark-blue")); + bm = Bookmark(m2::PointD(41, 40), "4", "placemark-blue"); + BookmarkCategory const * c33 = fm.AddBookmark(arrCat[2], bm); TEST_EQUAL(c33, c3, ()); @@ -287,16 +300,16 @@ UNIT_TEST(Bookmarks_Getting) TEST(IsValid(res), ()); BookmarkCategory * cat = fm.GetBmCategory(res.first); TEST(cat, ()); - bm = cat->GetBookmark(res.second); - TEST_EQUAL(bm->GetName(), "4", ()); - TEST_EQUAL(bm->GetType(), "placemark-blue", ()); + pBm = cat->GetBookmark(res.second); + TEST_EQUAL(pBm->GetName(), "4", ()); + TEST_EQUAL(pBm->GetType(), "placemark-blue", ()); TEST_EQUAL(cat->GetBookmarksCount(), 1, ()); cat->DeleteBookmark(0); TEST_EQUAL(cat->GetBookmarksCount(), 0, ()); - DeleteCategoryFiles(arrCat); + //DeleteCategoryFiles(arrCat); } UNIT_TEST(Bookmarks_AddressInfo) @@ -379,7 +392,8 @@ UNIT_TEST(Bookmarks_AddingMoving) m2::PointD const globalPoint = m2::PointD(40, 20); m2::PointD const pixelPoint = fm.GtoP(globalPoint); - BookmarkCategory const * c1 = fm.AddBookmark(arrCat[0], Bookmark(globalPoint, "name", "placemark-red")); + Bookmark bm(globalPoint, "name", "placemark-red"); + BookmarkCategory const * c1 = fm.AddBookmark(arrCat[0], bm); BookmarkAndCategory res = fm.GetBookmark(pixelPoint, 1.0); TEST(IsValid(res), ()); TEST_EQUAL(res.second, 0, ()); @@ -387,7 +401,8 @@ UNIT_TEST(Bookmarks_AddingMoving) TEST_EQUAL(fm.GetBmCategory(res.first)->GetName(), arrCat[0], ()); // Edit the name and type of bookmark - BookmarkCategory const * c11 = fm.AddBookmark(arrCat[0], Bookmark(globalPoint, "name2", "placemark-blue")); + bm = Bookmark(globalPoint, "name2", "placemark-blue"); + BookmarkCategory const * c11 = fm.AddBookmark(arrCat[0], bm); TEST_EQUAL(c1, c11, ()); res = fm.GetBookmark(pixelPoint, 1.0); TEST(IsValid(res), ()); @@ -399,7 +414,8 @@ UNIT_TEST(Bookmarks_AddingMoving) TEST_EQUAL(pBm->GetType(), "placemark-blue", ()); // Edit name, type and category of bookmark - BookmarkCategory const * c2 = fm.AddBookmark(arrCat[1], Bookmark(globalPoint, "name3", "placemark-green")); + bm = Bookmark(globalPoint, "name3", "placemark-green"); + BookmarkCategory const * c2 = fm.AddBookmark(arrCat[1], bm); TEST_NOT_EQUAL(c1, c2, ()); TEST_EQUAL(fm.GetBmCategoriesCount(), 2, ()); res = fm.GetBookmark(pixelPoint, 1.0); @@ -413,7 +429,7 @@ UNIT_TEST(Bookmarks_AddingMoving) TEST_EQUAL(pBm->GetName(), "name3", ()); TEST_EQUAL(pBm->GetType(), "placemark-green", ()); - DeleteCategoryFiles(arrCat); + //DeleteCategoryFiles(arrCat); } namespace @@ -456,7 +472,7 @@ UNIT_TEST(Bookmarks_InnerFolder) UNIT_TEST(BookmarkCategory_EmptyName) { BookmarkCategory * pCat = new BookmarkCategory(""); - pCat->AddBookmark(Bookmark(m2::PointD(0, 0), "", "placemark-red"), 17); + pCat->AddBookmark(Bookmark(m2::PointD(0, 0), "", "placemark-red")); pCat->SaveToKMLFile(); pCat->SetName("xxx"); diff --git a/qt/search_panel.cpp b/qt/search_panel.cpp index f0419d97be..5e07b2e357 100644 --- a/qt/search_panel.cpp +++ b/qt/search_panel.cpp @@ -148,8 +148,8 @@ void SearchPanel::OnSearchResult(ResultsT * res) if (e.GetResultType() == ResultT::RESULT_FEATURE) { // For debug purposes: add bookmarks for search results - frm.AddBookmark(SEARCH_CATEGORY, - Bookmark(e.GetFeatureCenter(), e.GetString(), "placemark-red")); + Bookmark bm(e.GetFeatureCenter(), e.GetString(), "placemark-red"); + frm.AddBookmark(SEARCH_CATEGORY, bm); m_pTable->setItem(rowCount, 0, create_item(QString::fromUtf8(e.GetFeatureType())));