[bookmarks] Review fixes.

This commit is contained in:
Daria Volvenkova 2019-06-18 16:03:24 +03:00 committed by mpimenov
parent 5c0c8b0dda
commit 663e328aa1
8 changed files with 121 additions and 106 deletions

View file

@ -11,6 +11,30 @@
#include <memory>
#include <string>
struct BookmarkInfo
{
BookmarkInfo() = default;
BookmarkInfo(kml::MarkId id, kml::BookmarkData const & data)
: m_bookmarkId(id)
, m_bookmarkData(data)
{}
kml::MarkId m_bookmarkId;
kml::BookmarkData m_bookmarkData;
};
struct BookmarkGroupInfo
{
BookmarkGroupInfo() = default;
BookmarkGroupInfo(kml::MarkGroupId id, kml::MarkIdCollection && marks)
: m_groupId(id)
, m_bookmarkIds(std::move(marks))
{}
kml::MarkGroupId m_groupId;
kml::MarkIdCollection m_bookmarkIds;
};
extern std::string const kKmzExtension;
extern std::string const kKmlExtension;
extern std::string const kKmbExtension;

View file

@ -635,6 +635,7 @@ void BookmarkManager::DeleteBookmark(kml::MarkId bmId)
CHECK_THREAD_CHECKER(m_threadChecker, ());
ASSERT(IsBookmark(bmId), ());
auto const it = m_bookmarks.find(bmId);
CHECK(it != m_bookmarks.end(), ());
auto const groupId = it->second->GetGroupId();
if (groupId != kml::kInvalidMarkGroupId)
{
@ -788,11 +789,16 @@ void BookmarkManager::ClearGroup(kml::MarkGroupId groupId)
auto * group = GetGroup(groupId);
for (auto markId : group->GetUserMarks())
{
m_changesTracker.OnDeleteMark(markId);
if (IsBookmarkCategory(groupId))
{
m_changesTracker.OnDetachBookmark(markId, groupId);
m_bookmarks.erase(markId);
}
else
{
m_userMarks.erase(markId);
}
m_changesTracker.OnDeleteMark(markId);
}
for (auto trackId : group->GetUserLines())
{
@ -1359,34 +1365,45 @@ BookmarkCategory * BookmarkManager::GetBmCategory(kml::MarkGroupId categoryId)
return (it != m_categories.end() ? it->second.get() : nullptr);
}
void BookmarkManager::GetBookmarksInfo(kml::MarkIdSet const & marks, std::vector<BookmarkInfo> & bookmarksInfo)
{
bookmarksInfo.clear();
bookmarksInfo.reserve(marks.size());
for (auto markId : marks)
{
if (IsBookmark(markId))
bookmarksInfo.emplace_back(markId, GetBookmark(markId)->GetData());
}
}
void BookmarkManager::GetBookmarkGroupsInfo(MarksChangesTracker::GroupMarkIdSet const & groups,
std::vector<BookmarkGroupInfo> & groupsInfo)
{
groupsInfo.clear();
groupsInfo.reserve(groups.size());
for (auto const & groupMarks : groups)
{
auto const & markIds = groupMarks.second;
groupsInfo.emplace_back(groupMarks.first, kml::MarkIdCollection(markIds.begin(), markIds.end()));
}
}
void BookmarkManager::SendBookmarksChanges()
{
std::vector<BookmarkInfo> bookmarksInfo;
if (m_callbacks.m_createdBookmarksCallback != nullptr)
{
std::vector<std::pair<kml::MarkId, kml::BookmarkData>> bookmarksData;
auto const & createdIds = m_changesTracker.GetCreatedMarkIds();
bookmarksData.reserve(createdIds.size());
for (auto markId : createdIds)
{
if (IsBookmark(markId))
bookmarksData.emplace_back(markId, GetBookmark(markId)->GetData());
}
if (!bookmarksData.empty())
m_callbacks.m_createdBookmarksCallback(bookmarksData);
GetBookmarksInfo(m_changesTracker.GetCreatedMarkIds(), bookmarksInfo);
if (!bookmarksInfo.empty())
m_callbacks.m_createdBookmarksCallback(bookmarksInfo);
}
if (m_callbacks.m_updatedBookmarksCallback != nullptr)
{
std::vector<std::pair<kml::MarkId, kml::BookmarkData>> bookmarksData;
auto const & updatedIds = m_changesTracker.GetUpdatedMarkIds();
bookmarksData.reserve(updatedIds.size());
for (auto markId : updatedIds)
{
if (IsBookmark(markId))
bookmarksData.emplace_back(markId, GetBookmark(markId)->GetData());
}
if (!bookmarksData.empty())
m_callbacks.m_updatedBookmarksCallback(bookmarksData);
GetBookmarksInfo(m_changesTracker.GetUpdatedMarkIds(), bookmarksInfo);
if (!bookmarksInfo.empty())
m_callbacks.m_updatedBookmarksCallback(bookmarksInfo);
}
if (m_callbacks.m_deletedBookmarksCallback != nullptr)
@ -1403,32 +1420,20 @@ void BookmarkManager::SendBookmarksChanges()
m_callbacks.m_deletedBookmarksCallback(bookmarkIds);
}
std::vector<BookmarkGroupInfo> groupsInfo;
if (m_callbacks.m_attachedBookmarksCallback != nullptr)
{
std::vector<std::pair<kml::MarkGroupId, kml::MarkIdCollection>> groupMarksCollection;
auto const & attachedBookmarks = m_changesTracker.GetAttachedBookmarks();
groupMarksCollection.reserve(attachedBookmarks.size());
for (auto const & groupMarks : attachedBookmarks)
{
groupMarksCollection.emplace_back(groupMarks.first, kml::MarkIdCollection(groupMarks.second.begin(),
groupMarks.second.end()));
}
if (!groupMarksCollection.empty())
m_callbacks.m_attachedBookmarksCallback(groupMarksCollection);
GetBookmarkGroupsInfo(m_changesTracker.GetAttachedBookmarks(), groupsInfo);
if (!groupsInfo.empty())
m_callbacks.m_attachedBookmarksCallback(groupsInfo);
}
if (m_callbacks.m_detachedBookmarksCallback != nullptr)
{
std::vector<std::pair<kml::MarkGroupId, kml::MarkIdCollection>> groupMarksCollection;
auto const & detachedBookmarks = m_changesTracker.GetDetachedBookmarks();
groupMarksCollection.reserve(detachedBookmarks.size());
for (auto const & groupMarks : detachedBookmarks)
{
groupMarksCollection.emplace_back(groupMarks.first, kml::MarkIdCollection(groupMarks.second.begin(),
groupMarks.second.end()));
}
if (!groupMarksCollection.empty())
m_callbacks.m_detachedBookmarksCallback(groupMarksCollection);
GetBookmarkGroupsInfo(m_changesTracker.GetDetachedBookmarks(), groupsInfo);
if (!groupsInfo.empty())
m_callbacks.m_detachedBookmarksCallback(groupsInfo);
}
}
@ -2547,38 +2552,32 @@ void BookmarkManager::MarksChangesTracker::OnUpdateMark(kml::MarkId markId)
m_updatedMarks.insert(markId);
}
void BookmarkManager::MarksChangesTracker::OnAttachBookmark(kml::MarkId markId, kml::MarkGroupId catId)
void BookmarkManager::MarksChangesTracker::InsertBookmark(kml::MarkId markId, kml::MarkGroupId catId,
GroupMarkIdSet & setToInsert, GroupMarkIdSet & setToErase)
{
auto const itCat = m_detachedBookmarks.find(catId);
if (itCat != m_detachedBookmarks.end())
auto const itCat = setToErase.find(catId);
if (itCat != setToErase.end())
{
auto const it = itCat->second.find(markId);
if (it != itCat->second.end())
{
itCat->second.erase(it);
if (itCat->second.empty())
m_detachedBookmarks.erase(itCat);
setToErase.erase(itCat);
return;
}
}
m_attachedBookmarks[catId].insert(markId);
setToInsert[catId].insert(markId);
}
void BookmarkManager::MarksChangesTracker::OnAttachBookmark(kml::MarkId markId, kml::MarkGroupId catId)
{
InsertBookmark(markId, catId, m_attachedBookmarks, m_detachedBookmarks);
}
void BookmarkManager::MarksChangesTracker::OnDetachBookmark(kml::MarkId markId, kml::MarkGroupId catId)
{
auto const itCat = m_attachedBookmarks.find(catId);
if (itCat != m_attachedBookmarks.end())
{
auto const it = itCat->second.find(markId);
if (it != itCat->second.end())
{
itCat->second.erase(it);
if (itCat->second.empty())
m_attachedBookmarks.erase(itCat);
return;
}
}
m_detachedBookmarks[catId].insert(markId);
InsertBookmark(markId, catId, m_detachedBookmarks, m_attachedBookmarks);
}
void BookmarkManager::MarksChangesTracker::OnAddLine(kml::TrackId lineId)

View file

@ -58,13 +58,11 @@ public:
struct Callbacks
{
using GetStringsBundleFn = std::function<StringsBundle const &()>;
using CreatedBookmarksCallback = std::function<void(std::vector<std::pair<kml::MarkId, kml::BookmarkData>> const &)>;
using UpdatedBookmarksCallback = std::function<void(std::vector<std::pair<kml::MarkId, kml::BookmarkData>> const &)>;
using CreatedBookmarksCallback = std::function<void(std::vector<BookmarkInfo> const &)>;
using UpdatedBookmarksCallback = std::function<void(std::vector<BookmarkInfo> const &)>;
using DeletedBookmarksCallback = std::function<void(std::vector<kml::MarkId> const &)>;
using AttachedBookmarksCallback = std::function<void(std::vector<std::pair<kml::MarkGroupId,
kml::MarkIdCollection>> const &)>;
using DetachedBookmarksCallback = std::function<void(std::vector<std::pair<kml::MarkGroupId,
kml::MarkIdCollection>> const &)>;
using AttachedBookmarksCallback = std::function<void(std::vector<BookmarkGroupInfo> const &)>;
using DetachedBookmarksCallback = std::function<void(std::vector<BookmarkGroupInfo> const &)>;
template <typename StringsBundleGetter, typename CreateListener, typename UpdateListener, typename DeleteListener,
typename AttachListener, typename DetachListener>
@ -385,6 +383,9 @@ private:
df::UserLineMark const * GetUserLineMark(kml::TrackId lineId) const override;
private:
void InsertBookmark(kml::MarkId markId, kml::MarkGroupId catId,
GroupMarkIdSet & setToInsert, GroupMarkIdSet & setToErase);
BookmarkManager & m_bmManager;
kml::MarkIdSet m_createdMarks;
@ -507,6 +508,10 @@ private:
void UpdateBmGroupIdList();
void SendBookmarksChanges();
void GetBookmarksInfo(kml::MarkIdSet const & marks, std::vector<BookmarkInfo> & bookmarks);
void GetBookmarkGroupsInfo(MarksChangesTracker::GroupMarkIdSet const & groups,
std::vector<BookmarkGroupInfo> & groupsInfo);
kml::MarkGroupId CheckAndCreateDefaultCategory();
void CheckAndResetLastIds();

View file

@ -488,19 +488,11 @@ Framework::Framework(FrameworkParams const & params)
m_bmManager = make_unique<BookmarkManager>(m_user, BookmarkManager::Callbacks(
[this]() -> StringsBundle const & { return m_stringsBundle; },
[this](vector<pair<kml::MarkId, kml::BookmarkData>> const & marks) {
GetSearchAPI().OnBookmarksCreated(marks);
},
[this](vector<pair<kml::MarkId, kml::BookmarkData>> const & marks) {
GetSearchAPI().OnBookmarksUpdated(marks);
},
[this](vector<BookmarkInfo> const & marks) { GetSearchAPI().OnBookmarksCreated(marks); },
[this](vector<BookmarkInfo> const & marks) { GetSearchAPI().OnBookmarksUpdated(marks); },
[this](vector<kml::MarkId> const & marks) { GetSearchAPI().OnBookmarksDeleted(marks); },
[this](vector<pair<kml::MarkGroupId, kml::MarkIdCollection>> const & marks) {
GetSearchAPI().OnBookmarksAttached(marks);
},
[this](vector<pair<kml::MarkGroupId, kml::MarkIdCollection>> const & marks) {
GetSearchAPI().OnBookmarksDetached(marks);
}));
[this](vector<BookmarkGroupInfo> const & marks) { GetSearchAPI().OnBookmarksAttached(marks); },
[this](vector<BookmarkGroupInfo> const & marks) { GetSearchAPI().OnBookmarksDetached(marks); }));
m_ParsedMapApi.SetBookmarkManager(m_bmManager.get());
m_routingManager.SetBookmarkManager(m_bmManager.get());

View file

@ -835,29 +835,29 @@ UNIT_CLASS_TEST(Runner, Bookmarks_Listeners)
resultChanges = Changes();
};
auto const onCreate = [&resultChanges](vector<pair<kml::MarkId, kml::BookmarkData>> const & marks)
auto const onCreate = [&resultChanges](vector<BookmarkInfo> const & marks)
{
for (auto const & markPair : marks)
resultChanges.m_createdMarks.insert(markPair.first);
for (auto const & mark : marks)
resultChanges.m_createdMarks.insert(mark.m_bookmarkId);
};
auto const onUpdate = [&resultChanges](vector<pair<kml::MarkId, kml::BookmarkData>> const & marks)
auto const onUpdate = [&resultChanges](vector<BookmarkInfo> const & marks)
{
for (auto const & markPair : marks)
resultChanges.m_updatedMarks.insert(markPair.first);
for (auto const & mark : marks)
resultChanges.m_updatedMarks.insert(mark.m_bookmarkId);
};
auto const onDelete = [&resultChanges](vector<kml::MarkId> const & marks)
{
resultChanges.m_deletedMarks.insert(marks.begin(), marks.end());
};
auto const onAttach = [&resultChanges](vector<pair<kml::MarkGroupId, kml::MarkIdCollection>> const & groupMarksCollection)
auto const onAttach = [&resultChanges](vector<BookmarkGroupInfo> const & groupMarksCollection)
{
for (auto const & groupMarks : groupMarksCollection)
resultChanges.m_attachedMarks[groupMarks.first].insert(groupMarks.second.begin(), groupMarks.second.end());
for (auto const & group : groupMarksCollection)
resultChanges.m_attachedMarks[group.m_groupId].insert(group.m_bookmarkIds.begin(), group.m_bookmarkIds.end());
};
auto const onDetach = [&resultChanges](vector<pair<kml::MarkGroupId, kml::MarkIdCollection>> const & groupMarksCollection)
auto const onDetach = [&resultChanges](vector<BookmarkGroupInfo> const & groupMarksCollection)
{
for (auto const & groupMarks : groupMarksCollection)
resultChanges.m_detachedMarks[groupMarks.first].insert(groupMarks.second.begin(), groupMarks.second.end());
for (auto const & group : groupMarksCollection)
resultChanges.m_detachedMarks[group.m_groupId].insert(group.m_bookmarkIds.begin(), group.m_bookmarkIds.end());
};
BookmarkManager::Callbacks callbacks(

View file

@ -132,7 +132,7 @@ UNIT_CLASS_TEST(SearchAPITest, MultipleViewportsRequests)
UNIT_CLASS_TEST(SearchAPITest, BookmarksSearch)
{
vector<pair<kml::MarkId, kml::BookmarkData>> marks;
vector<BookmarkInfo> marks;
kml::BookmarkData data;
kml::SetDefaultStr(data.m_name, "R&R dinner");

View file

@ -66,17 +66,13 @@ bookmarks::Id MarkIDToBookmarkId(kml::MarkId id)
kml::MarkId BookmarkIdToMarkID(bookmarks::Id id) { return static_cast<kml::MarkId>(id); }
void AppendBookmarkIdDocs(vector<pair<kml::MarkId, kml::BookmarkData>> const & marks,
void AppendBookmarkIdDocs(vector<BookmarkInfo> const & marks,
vector<BookmarkIdDoc> & result)
{
result.reserve(result.size() + marks.size());
for (auto const & mark : marks)
{
auto const & id = mark.first;
auto const & data = mark.second;
result.emplace_back(MarkIDToBookmarkId(id), bookmarks::Doc(data));
}
result.emplace_back(MarkIDToBookmarkId(mark.m_bookmarkId), bookmarks::Doc(mark.m_bookmarkData));
}
void AppendBookmarkIds(vector<kml::MarkId> const & marks, vector<bookmarks::Id> & result)
@ -373,14 +369,14 @@ void SearchAPI::FilterAllHotelsInViewport(m2::RectD const & viewport,
m_delegate.FilterHotels(filterTasks, move(featureIds));
}
void SearchAPI::OnBookmarksCreated(vector<pair<kml::MarkId, kml::BookmarkData>> const & marks)
void SearchAPI::OnBookmarksCreated(vector<BookmarkInfo> const & marks)
{
vector<BookmarkIdDoc> data;
AppendBookmarkIdDocs(marks, data);
m_engine.OnBookmarksCreated(data);
}
void SearchAPI::OnBookmarksUpdated(vector<pair<kml::MarkId, kml::BookmarkData>> const & marks)
void SearchAPI::OnBookmarksUpdated(vector<BookmarkInfo> const & marks)
{
vector<BookmarkIdDoc> data;
AppendBookmarkIdDocs(marks, data);
@ -394,11 +390,11 @@ void SearchAPI::OnBookmarksDeleted(vector<kml::MarkId> const & marks)
m_engine.OnBookmarksDeleted(data);
}
void SearchAPI::OnBookmarksAttached(std::vector<std::pair<kml::MarkGroupId, kml::MarkIdCollection>> const & marks)
void SearchAPI::OnBookmarksAttached(std::vector<BookmarkGroupInfo> const & marks)
{
}
void SearchAPI::OnBookmarksDetached(std::vector<std::pair<kml::MarkGroupId, kml::MarkIdCollection>> const & marks)
void SearchAPI::OnBookmarksDetached(std::vector<BookmarkGroupInfo> const & marks)
{
}

View file

@ -2,6 +2,7 @@
#include "map/booking_filter_params.hpp"
#include "map/bookmark.hpp"
#include "map/bookmark_helpers.hpp"
#include "map/everywhere_search_callback.hpp"
#include "map/search_product_info.hpp"
#include "map/viewport_search_callback.hpp"
@ -13,8 +14,6 @@
#include "search/result.hpp"
#include "search/search_params.hpp"
#include "kml/type_utils.hpp"
#include "geometry/point2d.hpp"
#include "geometry/rect2d.hpp"
@ -145,11 +144,11 @@ public:
void FilterAllHotelsInViewport(m2::RectD const & viewport,
booking::filter::Tasks const & filterTasks) override;
void OnBookmarksCreated(std::vector<std::pair<kml::MarkId, kml::BookmarkData>> const & marks);
void OnBookmarksUpdated(std::vector<std::pair<kml::MarkId, kml::BookmarkData>> const & marks);
void OnBookmarksCreated(std::vector<BookmarkInfo> const & marks);
void OnBookmarksUpdated(std::vector<BookmarkInfo> const & marks);
void OnBookmarksDeleted(std::vector<kml::MarkId> const & marks);
void OnBookmarksAttached(std::vector<std::pair<kml::MarkGroupId, kml::MarkIdCollection>> const & marks);
void OnBookmarksDetached(std::vector<std::pair<kml::MarkGroupId, kml::MarkIdCollection>> const & marks);
void OnBookmarksAttached(std::vector<BookmarkGroupInfo> const & marks);
void OnBookmarksDetached(std::vector<BookmarkGroupInfo> const & marks);
private:
struct SearchIntent