Re: [Evolution-hackers] concurrent modifications of items in GUI and EDS database

[Chenthill <pchenthill novell com>]

On Wed, 2009-01-07 at 12:45 +0100, Patrick Ohly wrote:
> Hello!
> 
> I'm currently thinking about synchronizing data with SyncEvolution in
> the background while the user is active with the Evolution UI. Some
> users already do that via cron jobs, but it is known to be problematic
> for several reasons, among them change tracking and concurrent editing
> of items.
> 
> I already discussed this with Ross Burton and Rob Bradford. Now I would
> like to check that we haven't missed anything.
> 
> The plan for change tracking is to get rid of the dependency on
> e_book_get_changes(). I already stopped using e_cal_get_changes()
> because it was too inflexible. Instead I'll rely on the REV resp.
> LAST-MODIFIED properties: the backend must update these each time an
> item is modified. This seems to be supported by most backends. Are there
> backends which are known to not support this?
> 
> REV and LAST-MODIFIED are treated as opaque "revision strings".
> SyncEvolution doesn't interpret the content, only compares it against
> the last synchronized value.
For calendar, we just use last modified time.

> 
> The problem with concurrent modifications is two-fold.
> Stale data in UI:
>       * user opens an item in Evolution
>       * synchronization starts in the background
>       * updates the item in EDS
>       * => When the user saves his changes, will he overwrite the more
>         recent data in EDS? Will he be warned? With Evolution the user
>         is not warned and his changes overwrite the ones in EDS (tested
>         with contacts). Evolution should listen for change signals and
>         warn the user as soon as he has stale data in the edit dialog.
>         The user then can cancel and reopen the item to redo his
>         changes. This is unlikely to happen often, so more elaborate
>         solutions (merging changes, additional buttons to copy from EDS)
>         should not be necessary. Should I file a bug for this? Anyone
>         able and willing to work on it?
> 
> Stale data in sync:
>       * when the sync starts, it builds a list of new/updated/deleted
>         items
>       * user modifies data in EDS
>       * this leads to conflicts, f.i. sync modifies item that was
>         modified by user
In both cases EDS signals modification of event to UI and all the
clients listening on the ECalView.. The clients need to watch for this
signal and update them accordingly.

> 
> Both cases need to be handled by the program which wants to make changes
> to EDS data (Evolution, sync engine). To avoid race conditions, support
> by EDS would be needed which currently doesn't exist. As a workaround
> the following method would reduce the time window in which conflicts can
> occur:
>       * get revstring before starting to make modifications (when
>         opening item in UI; when starting sync)
>       * before modifying the item, check the revstring again
>       * if the same as before, do the modification
>       * if different, handle conflict 
> 
> A secure solution would have to put the revision check into EDS itself
> to make the check and update atomic. The proposal is to add this check
> to e_book_commit_contact() and e_cal_modify_object():
>       * The caller is expected to include REV resp. LAST-MODIFIED as
>         read from EDS earlier.
>       * The EDS backend compares against the current value before
>         updating the item. If there is a mismatch, the update is
>         rejected with a suitable error code.
>       * If the values are unset, the update is always executed.
> 
> Problem 1: a client cannot tell whether the backend that it is using
> implements this check. If the backend doesn't (not yet updated, not
> possible), then the client should implement the workaround described
> above.
Since the backends emit the notification, currently it assumed that the
clients would handle it. I don think the backends check for the last
modified time while updating... I think it should be handled both at the
client side as well as EDS side to avoid race conditions.

> 
> Problem 2: the backend cannot tell whether the client wants to have the
> check enabled or not. An older client might send values for
> REV/LAST-MODIFIED and might expect to always succeed. Such a client
> could be considered broken and refusing the update might be the right
> solution.
> 
> Problem 3: the Exchange Connector does not update LAST-MODIFIED if the
> VEVENT contains it. I consider this a bug and work around it in
> SyncEvolution by filtering out the property before importing into EDS.
> Adding LAST-MODIFIED unconditionally would break change tracking.
This should be a bug if exchange is not updating the last-modified time.

> 
> Exact handling of REV and LAST-MODIFIED in the two calls is basically
> undefined right now as far as I can tell. I suggest to clarify that:
>       * backends should check them as described above
>       * backends must update them independently of what the client sends
> 
> To allow clients to query whether the backend supports the check I
> suggest to add a new capability for e_cal_get_static_capability() and
> e_book_check_static_capabilities().
> 
> Any comments?
With respect to the e_cal_remove* API, it would be good to have last
modified time specified in it. It would then require a new API to be
added..

thanks, Chenthill.