Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[kml] [bookmarks] Make the categories, bookmarks, tracks identifiable across devices, sessions, platforms etc. #8164

Open
kirylkaveryn opened this issue May 13, 2024 · 1 comment
Labels
Bookmarks and Tracks Bookmarks, imported tracks, and KML, KMZ, KMB, GPX import or export Core Cross-platform C++ libraries with a core functionality Refactoring

Comments

@kirylkaveryn
Copy link
Contributor

kirylkaveryn commented May 13, 2024

Issue description

While developing #7641 and #7978 I faced a blocker problem when the categories, tracks, and bookmark IDs are ints that temporarily created during only the current session and incrementally updated during the every bm reloading.

Some examples:

  1. the user opens the bookmarks list or PlacePpage screen and the app receives an update from the cloud that the bm's color or name was changed (it can be not only the iCloud, but another related source associated with the current group/bm/track) but the bm's ID from the update may NOT be equal to the current even they have an equal content. This situation can easily happen when the connection isn't stable or the phone was in airplane mode but the user turns it on;
  2. when the user adds the bookmarks on mac and continues editing them on the phone (simultaneous editing). Reloading of the files on the device can make the opened content outdated and will lead to the crash;
  3. the user deletes the bookmark, deletes the group, and restores the group, and the deleted bm cannot be returned back because the category has a different id;
  4. the user deletes the bookmark and reloads the app, the deleted bm's ID will be assigned to the other bm, so the restoration process is tightened to the current active session

How this works step-by-step (simplified):

  1. There are collection with the bookmarks on the device:
Collection001
-- bm001
-- bm002
-- bm003
  1. User opens the Collection001 to see a list OR opens bm001 to edit the color
  2. ...the app gets an update and parser parse the updated kml.
  3. Collection reloads to:
Collection002
-- bm004 (old bm001)
-- bm005
-- bm006
-- bm007
  1. The opened collection Collection001 OR opened bookmark bm001 have missed IDs and app will crash if the user trys to change smt.

There can be other examples, but the point is that kml's content isn't identifieble even during the one session in some user flows.

You can check the code starting from the:

kml::MarkGroupId BookmarkManager::CreateBookmarkCategory(std::string const & name, bool autoSave)
{
  CHECK_THREAD_CHECKER(m_threadChecker, ());
  auto const groupId = UserMarkIdStorage::Instance().GetNextCategoryId(); //<--- here
  CHECK_EQUAL(m_categories.count(groupId), 0, ());
  m_categories[groupId] = std::make_unique<BookmarkCategory>(name, groupId, autoSave);
  UpdateBmGroupIdList();
  m_changesTracker.OnAddGroup(groupId);
  NotifyBookmarksChanged();
  NotifyCategoriesChanged();
  return groupId;
}

The possible solutions:

1. Create the UUID and add it to the kml as a new field

Pros:

  • easy
  • created only once

Cons:

  • additional field in kml
  • increase file weight
  • affects reading/writing code
  • if OM is updated on multiple devices simultaneously the uuids will be generated on both devices an it will make a lot of merge conflicts (duplication of bookmarks)

2. Use the calculated hash as an ID based on the:

  • coordinates with some precision for the bookmarks
  • some start points for the tracks (it may limit the track editing in the future)
  • category names for the categories

Pros:

  • no changes in the kml and doesn't affect reading/writing code
  • uniqueness is based on the content and independent from the device/session

Cons:

  • harder to implement
  • needs some logic to support the equality operator during the hash collisions

@organicmaps/contributors
Please share your thoughts! This is a blocker for several tasks.
I will update this description based on your ideas.

@kirylkaveryn kirylkaveryn added Enhancement New feature or request, an improvement of some existing feature Bookmarks and Tracks Bookmarks, imported tracks, and KML, KMZ, KMB, GPX import or export Core Cross-platform C++ libraries with a core functionality Refactoring labels May 13, 2024
@biodranik biodranik removed the Enhancement New feature or request, an improvement of some existing feature label May 13, 2024
@biodranik
Copy link
Member

More high-level usage scenarios:

  1. Bookmarks and tracks are synced on the same user's devices using iCloud.
  2. The same as (1) but using other clouds (actual for Android and desktop devices).
  3. Bookmarks and tracks are shared with other users in read-only mode.
  4. The same as (3) but bookmarks and tracks are editable by other users.
  5. Everything above is working from a browser (e.g. users are planning trips on their computers and these trips (bookmarks, tracks, later areas too) are immediately available on their devices and other users' devices.

Merge conflicts are inevitable, the automated way of solving it will make OM the most convenient trip planning tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bookmarks and Tracks Bookmarks, imported tracks, and KML, KMZ, KMB, GPX import or export Core Cross-platform C++ libraries with a core functionality Refactoring
Projects
None yet
Development

No branches or pull requests

2 participants