GNOME.org
 

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

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 Ohly gmx de
http://www.estamos.de/


commit 86120f2c129bac4e24ea3d96c8d458ab68301414
Author: Patrick Ohly <patrick ohly 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 = 1<<0,
+	E_BOOK_CHECK_REVISION = 1<<1,
+	E_BOOK_SET_REVISION = 1<<2
+} 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 (gboolean        sync,
+		   int             flags,
 		   EBook          *book,
 		   EContact       *contact,
 		   GError        **error,
@@ -487,6 +494,12 @@ do_commit_contact (gboolean        sync,
 	CORBA_Environment ev;
 	char *vcard_str;
 
+	/* callers must decide what to do with the revision */
+	e_return_error_if_fail (flags, E_BOOK_ERROR_INVALID_ARG, FALSE);
+
+	/* TODO: backends don't implement anything else right now */
+	e_return_error_if_fail (flags != E_BOOK_IGNORE_REVISION, E_BOOK_ERROR_INVALID_ARG, FALSE);
+
 	g_mutex_lock (book->priv->mutex);
 
 	if (book->priv->load_state != E_BOOK_SOURCE_LOADED) {
@@ -577,7 +590,13 @@ do_commit_contact (gboolean        sync,
  * @error: a #GError to set on failure
  *
  * Applies the changes made to @contact to the stored version in
- * @book.
+ * @book. The revision set for @contact is ignored. The backend
+ * assigns a new, unique value automatically.
+ *
+ * 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
  **/
@@ -589,7 +608,47 @@ e_book_commit_contact (EBook           *book,
 	e_return_error_if_fail (book && E_IS_BOOK (book), E_BOOK_ERROR_INVALID_ARG, FALSE);
 	e_return_error_if_fail (contact && E_IS_CONTACT (contact), E_BOOK_ERROR_INVALID_ARG, FALSE);
 
-	return do_commit_contact (TRUE,
+	return do_commit_contact (TRUE, E_BOOK_IGNORE_REVISION,
+				  book, contact, error,
+				  NULL, NULL);
+
+}
+
+/**
+ * e_book_commit_contact_instance:
+ * @book: an #EBook
+ * @contact: an #EContact
+ * @flags: #E_BOOK_IGNORE_REVISION, #E_BOOK_CHECK_REVISION, or #E_BOOK_SET_REVISION
+ * @error: a #GError to set on failure
+ *
+ * Applies the changes made to @contact to the stored version in
+ * @book. The handling of the revision set for @contact depends on
+ * @flags. With E_BOOK_IGNORE_REVISION, the value is ignored. With
+ * E_BOOK_CHECK_REVISION, the string must match the current one at the
+ * time when the backend processes the request.  When it does not
+ * match, the commit fails. Using this is recommended to avoid
+ * overwriting changes that others might have made in the
+ * meantime. With E_BOOK_SET_REVISION, the contact is stored with the
+ * given revision, if possible. If this is not possible, then a new
+ * revision is assigned. Use this when restoring data from a backup.
+ *
+ * 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
+ **/
+gboolean
+e_book_commit_contact_instance (EBook           *book,
+				EContact        *contact,
+				int              flags,
+				GError         **error)
+{
+	e_return_error_if_fail (book && E_IS_BOOK (book), E_BOOK_ERROR_INVALID_ARG, FALSE);
+	e_return_error_if_fail (contact && E_IS_CONTACT (contact), E_BOOK_ERROR_INVALID_ARG, FALSE);
+
+	return do_commit_contact (TRUE, flags,
 				  book, contact, error,
 				  NULL, NULL);
 
@@ -602,8 +661,14 @@ e_book_commit_contact (EBook           *book,
  * @cb: function to call when the operation finishes
  * @closure: data to pass to callback function
  *
- * Applies the changes made to @contact to the stored version in
- * @book without blocking.
+ * Applies the changes made to @contact to the stored version in @book
+ * without blocking. The revision set for @contact is ignored. The
+ * backend assigns a new, unique value automatically.
+ *
+ * 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 the operation was started, %FALSE otherwise.
  **/
@@ -613,11 +678,50 @@ e_book_async_commit_contact (EBook                 *book,
 			     EBookCallback          cb,
 			     gpointer               closure)
 {
-	return !do_commit_contact (FALSE,
+	return !do_commit_contact (FALSE, E_BOOK_IGNORE_REVISION,
+				   book, contact, NULL,
+				   cb, closure);
+}
+
+/**
+ * e_book_async_commit_contact_instance:
+ * @book: an #EBook
+ * @contact: an #EContact
+ * @flags: #E_BOOK_IGNORE_REVISION, #E_BOOK_CHECK_REVISION, or #E_BOOK_SET_REVISION
+ * @cb: function to call when the operation finishes
+ * @closure: data to pass to callback function
+ *
+ * Applies the changes made to @contact to the stored version in @book
+ * without blocking. The handling of the revision set for @contact
+ * depends on @flags. With E_BOOK_IGNORE_REVISION, the value is
+ * ignored. With E_BOOK_CHECK_REVISION, the string must match the
+ * current one at the time when the backend processes the request.
+ * When it does not match, the commit fails. Using this is recommended
+ * to avoid overwriting changes that others might have made in the
+ * meantime. With E_BOOK_SET_REVISION, the contact is stored with the
+ * given revision, if possible. If this is not possible, then a new
+ * revision is assigned. Use this when restoring data from a backup.
+ *
+ * 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 the operation was started, %FALSE otherwise.
+ **/
+guint
+e_book_async_commit_contact_instance (EBook                 *book,
+				      EContact              *contact,
+				      int                    flags,
+				      EBookCallback          cb,
+				      gpointer               closure)
+{
+	return !do_commit_contact (FALSE, flags,
 				   book, contact, NULL,
 				   cb, closure);
 }
 
+
 static gboolean
 do_get_required_fields (gboolean             sync,
 			 EBook               *book,
@@ -1682,6 +1786,36 @@ e_book_remove_contact (EBook       *book,
 }
 
 /**
+ * e_book_remove_contact_instance:
+ * @book: an #EBook
+ * @id: a string
+ * @rev: the REV string of the contact which is to be removed, NULL allowed
+ * @error: a #GError to set on failure
+ *
+ * Removes the contact with id @id and revision @rev from @book. If
+ * @rev is NULL, then it does not matter what revision is currently in
+ * the @book. If @rev is not NULL, then the current revision has to
+ * match that string, otherwise removal is rejected with
+ * E_BOOK_ERROR_PERMISSION_DENIED. If @rev is not NULL and the backend
+ * does not support revision checking, then
+ * E_BOOK_ERROR_PROTOCOL_NOT_SUPPORTED is returned.
+ *
+ * Return value: %TRUE if successful, %FALSE otherwise
+ **/
+gboolean
+e_book_remove_contact_instance (EBook       *book,
+				const char  *id,
+				const char  *rev,
+				GError     **error)
+{
+	/* TODO: no backend supports revision checking right now */
+	e_return_error_if_fail (!rev, E_BOOK_ERROR_PROTOCOL_NOT_SUPPORTED, FALSE);
+
+	return e_book_remove_contact(book, id, error);
+}
+
+
+/**
  * e_book_remove_contacts:
  * @book: an #EBook
  * @ids: an #GList of const char *id's
@@ -1708,6 +1842,51 @@ e_book_remove_contacts (EBook    *book,
 }
 
 /**
+ * e_book_remove_contact_instances:
+ * @book: an #EBook
+ * @instances: an #GList of const EBookContactInstance *
+ * @error: a #GError to set on failure
+ *
+ * Removes the contacts with ids and revisions from the list @ids from
+ * @book. This is always more efficient than calling
+ * e_book_remove_contact_revision() if you have more than one id to remove, as
+ * some backends can implement it as a batch request.
+ *
+ * Revision checking for each contact is done as in
+ * e_book_remove_contact_instance().  Errors are returned if removal
+ * of any contact failed. In such a case, other contacts may have been
+ * removed successfully. If you need to know which contacts were
+ * removed, better use e_book_remove_contact_instance().
+ *
+ * Return value: %TRUE if successful, %FALSE otherwise
+ **/
+gboolean
+e_book_remove_contact_instances (EBook    *book,
+				 GList    *instances,
+				 GError  **error)
+{
+	GList *ids = NULL;
+	GList *next;
+	gboolean result = FALSE;
+
+	for (next = instances; next; next = next->next) {
+		EBookContactInstance *instance = next->data;
+
+		if (instance->rev) {
+			g_list_free (ids);
+			e_return_error_if_fail (!instance->rev, E_BOOK_ERROR_PROTOCOL_NOT_SUPPORTED, FALSE);
+		}
+		ids = g_list_prepend(ids, (void *)instance->id);
+	}
+
+	result = e_book_remove_contacts (book, ids, error);
+
+	g_list_free (ids);
+	return result;
+}
+
+
+/**
  * e_book_async_remove_contact:
  * @book: an #EBook
  * @contact: an #EContact
@@ -1734,6 +1913,30 @@ e_book_async_remove_contact (EBook                 *book,
 	return e_book_async_remove_contact_by_id (book, id, cb, closure);
 }
 
+
+
+/**
+ * e_book_async_remove_contact_instance:
+ * @book: an #EBook
+ * @contact: an #EContact
+ * @cb: a function to call when the operation finishes
+ * @closure: data to pass to callback function
+ *
+ * Removes @contact from @book. Does revision checking like
+ * e_book_remove_contact_instance(), if @contact has a revision set.
+ *
+ * Return value: %TRUE if successful, %FALSE otherwise
+ **/
+guint
+e_book_async_remove_contact_instance (EBook                 *book,
+				      EContact              *contact,
+				      EBookCallback          cb,
+				      gpointer               closure)
+{
+	// TODO: not implemented
+	return FALSE;
+}
+
 /**
  * e_book_async_remove_contact_by_id:
  * @book: an #EBook
@@ -1762,6 +1965,32 @@ e_book_async_remove_contact_by_id (EBook                 *book,
 }
 
 /**
+ * e_book_async_remove_contact_instance_by_id:
+ * @book: an #EBook
+ * @id: a unique ID string specifying the contact
+ * @rev: the REV string of the contact which is to be removed, NULL allowed
+ * @cb: a function to call when the operation finishes
+ * @closure: data to pass to callback function
+ *
+ * Removes the contact with id @id from @book. Does revision checking
+ * like e_book_remove_contact_instance() if @rev is not NULL.
+ *
+ * Return value: %TRUE if successful, %FALSE otherwise
+ **/
+guint
+e_book_async_remove_contact_instance_by_id (EBook                 *book,
+					    const char            *id,
+					    const char            *rev,
+					    EBookCallback          cb,
+					    gpointer               closure)
+{
+	/* TODO: no backend supports revision checking right now */
+	g_return_val_if_fail (!rev, FALSE);
+
+	return e_book_async_remove_contact_by_id (book, id, cb, closure);
+}
+
+/**
  * e_book_async_remove_contacts:
  * @book: an #EBook
  * @ids: a #GList of const char *id's
@@ -1769,7 +1998,7 @@ e_book_async_remove_contact_by_id (EBook                 *book,
  * @closure: data to pass to callback function
  *
  * Removes the contacts with ids from the list @ids from @book.  This is
- * always more efficient than calling e_book_remove_contact() if you
+ * always more efficient than calling e_book_async_remove_contact_by_id() if you
  * have more than one id to remove, as some backends can implement it
  * as a batch request.
  *
@@ -1789,6 +2018,29 @@ e_book_async_remove_contacts (EBook                 *book,
 				    cb, closure);
 }
 
+/**
+ * e_book_async_remove_contact_instances:
+ * @book: an #EBook
+ * @instances: a #GList of const EBookInstance *
+ * @cb: a function to call when the operation finishes
+ * @closure: data to pass to callback function
+ *
+ * Removes the contacts with ids and revision from the list @instances
+ * from @book. Revision checking and caveats as for
+ * e_book_remove_contact_instances().
+ *
+ * Return value: %TRUE if successful, %FALSE otherwise
+ **/
+guint
+e_book_async_remove_contact_instances (EBook                 *book,
+				       GList                 *instances,
+				       EBookCallback          cb,
+				       gpointer               closure)
+{
+	/* TODO: not implemented */
+	return FALSE;
+}
+
 
 static gboolean
 do_get_book_view (gboolean sync,
diff --git a/addressbook/libebook/e-book.h b/addressbook/libebook/e-book.h
index 8234572..07c8c91 100644
--- a/addressbook/libebook/e-book.h
+++ b/addressbook/libebook/e-book.h
@@ -228,6 +228,12 @@ void     e_book_free_change_list           (GList       *change_list);
 const char *e_book_get_uri                 (EBook       *book);
 ESource    *e_book_get_source              (EBook       *book);
 
+/** operations where the backend updates a contact's revision return the new revision */
+#define EBOOK_STATIC_CAPABILITY_RETURN_REV               "return-ref"
+
+/** backend supports atomic update/delete operations (the _instance variant of the calls) */
+#define EBOOK_STATIC_CAPABILITY_ATOMIC_MODIFICATIONS     "atomic-modifications"
+
 const char *e_book_get_static_capabilities (EBook    *book,
 					    GError  **error);
 gboolean    e_book_check_static_capability (EBook       *book,
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]