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

2009-07-28 Thread Chenthill
On Fri, 2009-07-24 at 20:20 +0200, Patrick Ohly wrote:
 Hello!
 
 Let's pick up this discussion again. When we agree on the API changes,
 then I'll try to follow up with an implementation for the file backend.
 
 On Thu, 2009-01-08 at 13:48 +0100, Patrick Ohly wrote:
  Hello Suman!
  
  I forgot to ask: do you agree in general with the plan to do atomic
  updates via e_book_commit_contact() and e_cal_modify_object() by
  defining the semantic as suggested?
 
 I've come to the conclusion that overloading the old calls is not worth
 the trouble. Therefore I now suggest to add new variants of the call
 because...
 
  I also need to extend the proposal: removing an item has similar race
  conditions (sync starts, user updates item, sync removes item). The
  current APIs for removing items make it harder to pass in the expected
  REV resp. LAST-MODIFIED. The only API call that could do it without
  changing its prototype is e_book_async_remove_contact(EContact
  *contact). e_book_remove_contact(), e_cal_remove_object(),
  e_cal_remove_object_with_mod() all just take ID strings.
 
 ... we need these new variants for the delete operations anyway.
 
 I have not looked at the calendar API yet. Let me know what you think
 about the attached patch for a new ebook API and I will continue with
 calendar next, before implementing anything.
Then changes made are reasonable. Nice api documentation! This can be
done in calendar also. For calendars we use sequence numbers for items
shared with people and last-modified in general.

 
 In this current incarnation the patch tries a bit to provide simple
 implementations of the new calls, but this isn't meant to be complete.
Would be better to get these api's committed after the branching is done
for 2.27.x. We will be merging the eds-addressbook dbus port this week
(for 2.27.x) and the calendar part for evolution 3.0. Will keep you
informed on the same.

- Chenthill.

___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


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

2009-07-24 Thread Patrick Ohly
Hello!

Let's pick up this discussion again. When we agree on the API changes,
then I'll try to follow up with an implementation for the file backend.

On Thu, 2009-01-08 at 13:48 +0100, Patrick Ohly wrote:
 Hello Suman!
 
 I forgot to ask: do you agree in general with the plan to do atomic
 updates via e_book_commit_contact() and e_cal_modify_object() by
 defining the semantic as suggested?

I've come to the conclusion that overloading the old calls is not worth
the trouble. Therefore I now suggest to add new variants of the call
because...

 I also need to extend the proposal: removing an item has similar race
 conditions (sync starts, user updates item, sync removes item). The
 current APIs for removing items make it harder to pass in the expected
 REV resp. LAST-MODIFIED. The only API call that could do it without
 changing its prototype is e_book_async_remove_contact(EContact
 *contact). e_book_remove_contact(), e_cal_remove_object(),
 e_cal_remove_object_with_mod() all just take ID strings.

... we need these new variants for the delete operations anyway.

I have not looked at the calendar API yet. Let me know what you think
about the attached patch for a new ebook API and I will continue with
calendar next, before implementing anything.

In this current incarnation the patch tries a bit to provide simple
implementations of the new calls, but this isn't meant to be complete.

-- 
Bye, Patrick Ohly
--  
patrick.o...@gmx.de
http://www.estamos.de/

commit 86120f2c129bac4e24ea3d96c8d458ab68301414
Author: Patrick Ohly patrick.o...@gmx.de
Date:   Fri Jul 24 20:08:01 2009 +0200

atomic updates and deletes: check and return revision

The current API suffers from multiple race conditions: when
overwriting a contact, another client might have already made
an update that gets overwritten by older data. Removing a
contact is also affected = added an API extension that
allows backends a) to detect that clients want the more
careful data modifications and b) provides enough information
to do the checking based on the revision string in the backend
(EBOOK_STATIC_CAPABILITY_ATOMIC_MODIFICATIONS).

Software that does change tracking based on the revision string
(like SyncEvolution) needs to know which revision string was
assigned to an updated or added contact. Currently it must do
the operation, then ask for the whole contact. There is a small
window for a race condition here, but more important, this
requires another round-trip and transmit too much information.
With EBOOK_STATIC_CAPABILITY_RETURN_REV the client can be sure
to get the necessary information right away.

diff --git a/addressbook/libebook/e-book-types.h b/addressbook/libebook/e-book-types.h
index 00a814e..2dadf27 100644
--- a/addressbook/libebook/e-book-types.h
+++ b/addressbook/libebook/e-book-types.h
@@ -64,11 +64,23 @@ typedef enum {
 	E_BOOK_CHANGE_CARD_MODIFIED
 } EBookChangeType;
 
+typedef enum {
+	E_BOOK_UNDEFINED_REVISION_HANDLING = 0,
+	E_BOOK_IGNORE_REVISION = 10,
+	E_BOOK_CHECK_REVISION = 11,
+	E_BOOK_SET_REVISION = 12
+} EBookRevisionHandlingFlags;
+
 typedef struct {
 	EBookChangeType  change_type;
 	EContact*contact;
 } EBookChange;
 
+typedef struct {
+	const char *id;
+	const char *rev;
+} EBookContactInstance;
+
 G_END_DECLS
 
 #endif /* ! __E_BOOK_TYPES_H__ */
diff --git a/addressbook/libebook/e-book.c b/addressbook/libebook/e-book.c
index fefe4fa..07ab5d7 100644
--- a/addressbook/libebook/e-book.c
+++ b/addressbook/libebook/e-book.c
@@ -367,7 +367,10 @@ do_add_contact (gboolean  sync,
  * @contact: an #EContact
  * @error: a #GError to set on failure
  *
- * Adds @contact to @book.
+ * Adds @contact to @book. If the backend has the
+ * #EBOOK_STATIC_CAPABILITY_RETURN_REV, then the revision string
+ * assigned to the contact is guaranteed to be set in @contact when
+ * the function returns. The UID in @contact is set in all cases.
  *
  * Return value: %TRUE if successful, %FALSE otherwise.
  **/
@@ -393,7 +396,10 @@ e_book_add_contact (EBook   *book,
  * @cb: function to call when the operation finishes
  * @closure: data to pass to callback function
  *
- * Adds @contact to @book without blocking.
+ * Adds @contact to @book without blocking. If the backend has the
+ * #EBOOK_STATIC_CAPABILITY_RETURN_REV, then the revision string
+ * assigned to the contact is guaranteed to be set in @contact when
+ * the operation completes. The UID in @contact is set in all cases.
  *
  * Return value: %TRUE if the operation was started, %FALSE otherwise.
  **/
@@ -476,6 +482,7 @@ e_book_response_add_contact (EBook   *book,
 
 static gboolean
 do_commit_contact (gbooleansync,
+		   int flags,
 		   EBook  *book,
 		   EContact   *contact,
 		   GError**error,
@@ -487,6 +494,12 @@ do_commit_contact (gbooleansync,
 	CORBA_Environment ev;
 	char *vcard_str;
 
+	/* callers must decide what to do with 

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

2009-01-08 Thread Patrick Ohly
On Thu, 2009-01-08 at 10:10 +0530, Suman Manjunath wrote:
 On Wed, Jan 7, 2009 at 17:15, Patrick Ohly patrick.o...@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] ?

REV is part of vCard 3.0. It would be used in ebook. ecal would use
LAST-MODIFIED instead. Does the GroupWise server update REV when
contacts are modified?

  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.

I only tested with a contact. There the problem occurs as described
above.

  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,

the apparent bug =
https://bugzilla.novell.com/show_bug.cgi?id=184554 ?

Or do you mean the problem with overwriting more recent changes?

I'm not sure I understood how the GroupWise server and backend work.
What you are saying is that locally cached data might get out of sync
with the server and thus checks in the Evolution backend cannot be
implemented 100% accurately, right?

That sounds like a real problem, but I still see room for improvement.
For example, the backend could implement the workaround that I described
in my email. A client can't do that because without changes in the
backend it might read stale data, compare successfully, then push an
update which overwrites modifications on the server.

  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)

That's because the local copy of the appointment is not identical to the
one on the server, right? Perhaps the backend could force an immediate
refresh of this particular appointment to get the server's
LAST-MODIFIED?

-- 
Bye, Patrick Ohly
--  
patrick.o...@gmx.de
http://www.estamos.de/

___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


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

2009-01-08 Thread Patrick Ohly
Hello Suman!

I forgot to ask: do you agree in general with the plan to do atomic
updates via e_book_commit_contact() and e_cal_modify_object() by
defining the semantic as suggested?

I also need to extend the proposal: removing an item has similar race
conditions (sync starts, user updates item, sync removes item). The
current APIs for removing items make it harder to pass in the expected
REV resp. LAST-MODIFIED. The only API call that could do it without
changing its prototype is e_book_async_remove_contact(EContact
*contact). e_book_remove_contact(), e_cal_remove_object(),
e_cal_remove_object_with_mod() all just take ID strings.

When e_cal_remove*() refers to multiple VEVENTs it might also be
difficult to specify the expected LAST-MODIFIED of each VEVENT. I don't
have a good solution for the remove case. New API calls?

e_cal_modify_object() has the same problem with multiple VEVENTs, but
this can be solved: for CALOBJ_MOD_THIS[ANDPRIOR|FUTURE], the
LAST-MODIFIED property of the referenced VEVENT is to be compared. For
CALOBJ_MOD_ALL the main event is checked.

-- 
Bye, Patrick Ohly
--  
patrick.o...@gmx.de
http://www.estamos.de/

___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


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

2009-01-08 Thread Chenthill
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 

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

2009-01-08 Thread Suman Manjunath
On Thu, Jan 8, 2009 at 15:19, Patrick Ohly patrick.o...@gmx.de wrote:
 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,

 the apparent bug =
 https://bugzilla.novell.com/show_bug.cgi?id=184554 ?

Yep.. it is this ^^ bug. And I agree with Chen that it needs a fix at
the backend.

-Suman
___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


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

2009-01-07 Thread Stefano Canepa
Il giorno mer, 07/01/2009 alle 12.45 +0100, Patrick Ohly ha scritto:
 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 started studing a plugin to do the sync on user request. I'm going to
present my idea in another email just to have clearer threads.

Bye
sc

-- 
Stefano Canepa aka sc: s...@linux.it - http://www.stefanocanepa.it
Three great virtues of a programmer: laziness, impatience and hubris.
Le tre grandi virtù di un programmatore: pigrizia, impazienza e
arroganza. (Larry Wall)


signature.asc
Description: Questa è una parte del messaggio	firmata digitalmente
___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


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

2009-01-07 Thread Suman Manjunath
On Wed, Jan 7, 2009 at 17:15, Patrick Ohly patrick.o...@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]