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

["Suman Manjunath" <manjunath suman gmail com>]

On Wed, Jan 7, 2009 at 17:15, Patrick Ohly <patrick ohly gmx de> wrote:
> 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?

a) Looking at [1], I can't find what a REV property is. Did you mean
to use [2] ?

> 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?

Not so for calendars. When an event is open in the Evolution UI and
the backend modifies it during a refresh/update from server, the user
_is_ warned of an update to the item. If not, the backend is probably
not doing a e_cal_backend_notify_object_modified() which it ought to.
FWIW, see the open bug at [3].

> 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
>
> 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

The GroupWise server updates the 'modified' property of the item when
it actually gets modified on the server. For newly created items, it
also adds the 'created' property at the same time.

This behavior invalidates all the 'handle-at-backend' approaches to
fix the apparent bug, like:
* Check if the item is newly-created by looking for a 'modified'
property in the cached-object. This approach does not handle modifying
an older (and already cached from server) item.
* Pushing the 'modified' property from the client to the server upon
creation/modification. This does not help, since the server would
modify the property anyway.
* A series of 'if-else' trying to handle various scenarios. It was
observed that the conditions would fail for at-least one use-case,
mainly since we would not have a persistent 'last-modified' time
unless it is obtained from the server.

These drawbacks probably apply to the Exchange backend too.

> 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.

A backend may not have a LAST-MODIFIED property for a particular event
in this use case:
a) create a new appointment in the GW calendar (while online)
b) open the same appointment (before the refresh timeout)

Pushing an update in this case is not correct as the event can easily
be modified by another client during that refresh interval.

We should ensure that the cached object in EDS will always have the
same last-modified property as that on the server before opening the
event in the UI [This probably implies an (expensive?) 'live' re-cache
of the object being opened]. We already listen for changes to the
event and so the user automatically knows if anything changed since
opening the event editor.

We don't continuously listen for updates from the server and hence the
problem I mentioned above would still exist, but that is something I
can live with :)

- Suman

[1] http://tools.ietf.org/html/rfc2445
[2] http://tools.ietf.org/html/rfc2445#section-4.8.7.4
[3] https://bugzilla.novell.com/show_bug.cgi?id=184554 (not sure if it
is open for all to see but it reads: "Leaving groupwise appointments
open causes change notices" - Leaving groupwise appointments open
causes change notices when the refresh timeout to the server occurs.
Shouldn't this notice nothing has changed?)