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

New issue

Open

opened 2024-05-13 08:45:06 +00:00 by kirylkaveryn · 0 comments

kirylkaveryn commented

2024-05-13 08:45:06 +00:00

Member

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:

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;
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;
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;
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):

There are collection with the bookmarks on the device:

Collection001
-- bm001
-- bm002
-- bm003

User opens the Collection001 to see a list OR opens bm001 to edit the color
...the app gets an update and parser parse the updated kml.
Collection reloads to:

Collection002
-- bm004 (old bm001)
-- bm005
-- bm006
-- bm007

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.

### Issue description While developing #7641 and #7978 I faced a blocker problem when the categories, tracks, and bookmark IDs are `int`s 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; 5. 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 ``` 2. User opens the Collection001 to see a list OR opens bm001 to edit the color 3. ...the app gets an update and parser parse the updated kml. 4. Collection reloads to: ``` Collection002 -- bm004 (old bm001) -- bm005 -- bm006 -- bm007 ``` 5. 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: ```cpp 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.

👍 1

This repo is archived. You cannot comment on issues.