jkiviluo
commented
5 months ago Here's a modified proposal. We can do pretty much the way you're proposing and it will probably help in many cases, but I think it still has problems. Below the chart, there are a list of options to deal with the conflict situation. I still think adding a datetime would be helpful in conflict resolution because we could default to choosing the latest version of the same item. (User could of course be able to choose the behaviour, but we don't need to implement that right now - we just need something that works better).
Algorithm
flowchart TD
A[A: Fetch a new item from the DB] --> B{"Is there a mapped-item
with the same unique key?"}
B -->|Yes| D
D{"Is the id of the DB-item same
as the id of the mapped-item?"} -->|Yes| Z[Z: Keep mapped-item]
D -->|No| E{"mapped-item has been
added in this session?"}
E -->|Yes| F["F: Would benefit from
user interaction*"] --> Z
E -->|No| H["H: Does it matter that
the item has not been
added in this session?
Would benefit from
user interaction*"] --> Z
B -->|No| J{"Is there a mapped-item with the id
of the fetched DB-item"}
J --> |Yes| N["N: Change the mapped-item id
to the next free id"]
J --> |No| L["L: Add the feched item
to memory"]'* There are some options: 1) Keep it simple: keep the mapped-item, drop the DB item. This will in some cases destroy data. 2) Make a new unique key and create a new item. 3) Check if the item contents are the same. If they are, the mapped-item could be either dropped (replaced by the item from DB) or it could replace the item from DB (re-committed). If the contents are different, then the user should be informed so the user can choose. 4) Add datetime information to db and memory that can be used to make a better informed automatic choice (e.g. take the latest).
Even that leftmost yes - yes path would benefit from the datetime - it could be a further check that the items are truly the same.
manuelma
soininen
codecov-commenter
Fixes 'id conflicts' between in-memory mapping and new contents of the DB. This is not about resolving conflicts on user-data, that should be done in another PR.
The core idea is, if a DB-item has the same unique key as a mapped-item, then they also need to have the same id. Conversely, if a DB-item has the same id as a mapped-item, then they need to have the same unique keys. The id to trust and respect will always be the most recent from the DB.
Algorithm
The Free DB-item id bit consists in making sure that no mapped-item has the same id as the DB-item. If that's the case, we need to decide what to do with the clashing mapped-item, as we cannot keep it with its current id. This is the only point that requires conflict resolution*. At the moment we 'rescue' the mapped-item, which means we make it pending for addition with a new id - but we could use a different strategy or let the user choose between a number of strategies.
@soininen @jkiviluo you could start reviewing this if you will. I still need to polish things and especially improve unit-tests. One of the old tests is not passing. I have added two new tests but need to finalize one of them and add more to test all the different cases.
NOTE: In element I was proposing a different idea which consisted in using a tuple (last-commit-id, db-id) instead of the db-id, to avoid ambiguities, but that doesn't work. We need to resolve ambiguities and the above algorithm works much better to that end.
NOTE2: One important detail for the implementation is, we need to be able to find mapped items by unique key even if they have been removed from the memory-mapping. For this reason, the
_MappedTable._ids_by_unique_key_valuedictionary now also includes removed items, and we check if the items in it are removed or not depending on what we are using them for.Checklist before merging