EBookBackendFile: Avoid lost data in contact modifications
authorTristan Van Berkom <tristanvb@openismus.com>
Thu, 24 Jan 2013 05:57:03 +0000 (14:57 +0900)
committerTristan Van Berkom <tristanvb@openismus.com>
Tue, 29 Jan 2013 07:15:23 +0000 (16:15 +0900)
When modifying contacts, ensure that the contact revisions are in
sync with the existing ones, otherwise report the E_DATA_BOOK_STATUS_OUT_OF_SYNC
error. This indicates that the client should refresh it's local copy
of the contact before trying to modify it again.

Conflicts:

addressbook/backends/file/e-book-backend-file.c

addressbook/backends/file/e-book-backend-file.c

index a0a35bd..f1d56a6 100644 (file)
@@ -69,7 +69,8 @@ struct _EBookBackendFilePrivate {
        gchar     *photo_dirname;
        gchar     *revision;
        gint       rev_counter;
-       GRecMutex  revision_mutex;
+       gboolean   revision_guards;
+       GRWLock    lock;
 
        EBookBackendSqliteDB *sqlitedb;
 };
@@ -617,8 +618,6 @@ e_book_backend_file_bump_revision (EBookBackendFile *bf)
 {
        GError *error = NULL;
 
-       g_rec_mutex_lock (&bf->priv->revision_mutex);
-
        g_free (bf->priv->revision);
        bf->priv->revision = e_book_backend_file_new_revision (bf);
 
@@ -635,8 +634,6 @@ e_book_backend_file_bump_revision (EBookBackendFile *bf)
        e_book_backend_notify_property_changed (E_BOOK_BACKEND (bf),
                                                BOOK_BACKEND_PROPERTY_REVISION,
                                                bf->priv->revision);
-
-       g_rec_mutex_unlock (&bf->priv->revision_mutex);
 }
 
 static void
@@ -644,8 +641,6 @@ e_book_backend_file_load_revision (EBookBackendFile *bf)
 {
        GError *error = NULL;
 
-       g_rec_mutex_lock (&bf->priv->revision_mutex);
-
        if (!e_book_backend_sqlitedb_get_revision (bf->priv->sqlitedb,
                                                   SQLITEDB_FOLDER_ID,
                                                   &bf->priv->revision,
@@ -657,23 +652,18 @@ e_book_backend_file_load_revision (EBookBackendFile *bf)
        } else if (bf->priv->revision == NULL) {
                e_book_backend_file_bump_revision (bf);
        }
-
-       g_rec_mutex_unlock (&bf->priv->revision_mutex);
 }
 
+
 static void
-set_revision (EContact *contact)
+set_revision (EBookBackendFile *bf,
+             EContact *contact)
 {
-       gchar time_string[100] = {0};
-       const struct tm *tm = NULL;
-       time_t t;
-
-       t = time (NULL);
-       tm = gmtime (&t);
-       if (tm)
-               strftime (time_string, 100, "%Y-%m-%dT%H:%M:%SZ", tm);
-       e_contact_set (contact, E_CONTACT_REV, time_string);
+       gchar *rev;
 
+       rev = e_book_backend_file_new_revision (bf);
+       e_contact_set (contact, E_CONTACT_REV, rev);
+       g_free (rev);
 }
 
 /****************************************************************
@@ -723,7 +713,7 @@ do_create (EBookBackendFile *bf,
 
                rev = e_contact_get_const (contact, E_CONTACT_REV);
                if (!(rev && *rev))
-                       set_revision (contact);
+                       set_revision (bf, contact);
 
                status = maybe_transform_vcard_for_photo (bf, NULL, contact, perror);
 
@@ -790,9 +780,13 @@ e_book_backend_file_create_contacts (EBookBackendSync *backend,
 {
        EBookBackendFile *bf = E_BOOK_BACKEND_FILE (backend);
 
+       g_rw_lock_writer_lock (&(bf->priv->lock));
+
        if (do_create (bf, vcards, added_contacts, perror)) {
                e_book_backend_file_bump_revision (bf);
        }
+
+       g_rw_lock_writer_unlock (&(bf->priv->lock));
 }
 
 static void
@@ -809,6 +803,8 @@ e_book_backend_file_remove_contacts (EBookBackendSync *backend,
        gboolean          delete_failed = FALSE;
        const GSList     *l;
 
+       g_rw_lock_writer_lock (&(bf->priv->lock));
+
        for (l = id_list; l != NULL; l = l->next) {
                const gchar *id;
                EContact *contact;
@@ -855,13 +851,16 @@ e_book_backend_file_remove_contacts (EBookBackendSync *backend,
                        g_propagate_error (perror, local_error);
                }
 
+               e_book_backend_file_bump_revision (bf);
+
                *ids = removed_ids;
        } else {
                *ids = NULL;
                e_util_free_string_slist (removed_ids);
        }
 
-       e_book_backend_file_bump_revision (bf);
+       g_rw_lock_writer_unlock (&(bf->priv->lock));
+
        e_util_free_object_slist (removed_contacts);
 }
 
@@ -885,9 +884,12 @@ e_book_backend_file_modify_contacts (EBookBackendSync *backend,
                return;
        }
 
+       g_rw_lock_writer_lock (&(bf->priv->lock));
+
        for (l = vcards; l != NULL; l = l->next) {
                gchar *id;
                EContact *contact, *old_contact;
+               const gchar *contact_rev, *old_contact_rev;
 
                contact = e_contact_new_from_vcard (l->data);
                id = e_contact_get (contact, E_CONTACT_UID);
@@ -914,7 +916,26 @@ e_book_backend_file_modify_contacts (EBookBackendSync *backend,
                        g_object_unref (contact);
                        break;
                }
-               old_contacts = g_slist_prepend (old_contacts, old_contact);
+
+               if (bf->priv->revision_guards) {
+                       contact_rev = e_contact_get_const (contact, E_CONTACT_REV);
+                       old_contact_rev = e_contact_get_const (old_contact, E_CONTACT_REV);
+
+                       if (!contact_rev || !old_contact_rev ||
+                           strcmp (contact_rev, old_contact_rev) != 0) {
+                               g_set_error (perror, E_DATA_BOOK_ERROR,
+                                            E_DATA_BOOK_STATUS_OUT_OF_SYNC,
+                                            _("Tried to modify contact '%s' with out of sync revision"),
+                                            (gchar *)e_contact_get_const (contact, E_CONTACT_UID));
+
+                               status = STATUS_ERROR;
+
+                               g_free (id);
+                               g_object_unref (contact);
+                               g_object_unref (old_contact);
+                               break;
+                       }
+               }
 
                /* Transform incomming photo blobs to uris before storing this to the DB */
                status = maybe_transform_vcard_for_photo (bf, old_contact, contact, &local_error);
@@ -929,8 +950,9 @@ e_book_backend_file_modify_contacts (EBookBackendSync *backend,
                }
 
                /* update the revision (modified time of contact) */
-               set_revision (contact);
+               set_revision (bf, contact);
 
+               old_contacts      = g_slist_prepend (old_contacts, old_contact);
                modified_contacts = g_slist_prepend (modified_contacts, contact);
                ids               = g_slist_prepend (ids, id);
        }
@@ -959,6 +981,11 @@ e_book_backend_file_modify_contacts (EBookBackendSync *backend,
                }
        }
 
+       if (status != STATUS_ERROR)
+               e_book_backend_file_bump_revision (bf);
+
+       g_rw_lock_writer_unlock (&(bf->priv->lock));
+
        if (status != STATUS_ERROR) {
                *contacts = g_slist_reverse (modified_contacts);
        } else {
@@ -968,8 +995,6 @@ e_book_backend_file_modify_contacts (EBookBackendSync *backend,
 
        e_util_free_string_slist (ids);
        g_slist_free_full (old_contacts, g_object_unref);
-
-       e_book_backend_file_bump_revision (bf);
 }
 
 static void
@@ -988,9 +1013,12 @@ e_book_backend_file_get_contact (EBookBackendSync *backend,
                return;
        }
 
+       g_rw_lock_reader_lock (&(bf->priv->lock));
+
        *vcard = e_book_backend_sqlitedb_get_vcard_string (bf->priv->sqlitedb,
                                                           SQLITEDB_FOLDER_ID, id,
                                                           NULL, NULL, &local_error);
+       g_rw_lock_reader_unlock (&(bf->priv->lock));
 
        if (local_error) {
 
@@ -1027,10 +1055,12 @@ e_book_backend_file_get_contact_list (EBookBackendSync *backend,
                return;
        }
 
+       g_rw_lock_reader_lock (&(bf->priv->lock));
        summary_list = e_book_backend_sqlitedb_search (
                bf->priv->sqlitedb, SQLITEDB_FOLDER_ID,
                query, NULL,
                NULL, NULL, &local_error);
+       g_rw_lock_reader_unlock (&(bf->priv->lock));
 
        if (summary_list) {
 
@@ -1072,10 +1102,12 @@ e_book_backend_file_get_contact_list_uids (EBookBackendSync *backend,
                return;
        }
 
+       g_rw_lock_reader_lock (&(bf->priv->lock));
        uids = e_book_backend_sqlitedb_search_uids (
                bf->priv->sqlitedb,
                SQLITEDB_FOLDER_ID,
                query, NULL, &local_error);
+       g_rw_lock_reader_unlock (&(bf->priv->lock));
 
        if (uids == NULL && local_error != NULL) {
                g_warning ("Failed to fetch contact ids: %s", local_error->message);
@@ -1187,11 +1219,13 @@ book_view_thread (gpointer data)
        d (printf ("signalling parent thread\n"));
        e_flag_set (closure->running);
 
+       g_rw_lock_reader_lock (&(bf->priv->lock));
        summary_list = e_book_backend_sqlitedb_search (
                bf->priv->sqlitedb,
                SQLITEDB_FOLDER_ID,
                query, fields_of_interest,
                NULL, NULL, &local_error);
+       g_rw_lock_reader_unlock (&(bf->priv->lock));
 
        if (!summary_list && local_error != NULL) {
                g_warning (G_STRLOC ": Failed to query initial contacts: %s", local_error->message);
@@ -1278,6 +1312,7 @@ e_book_backend_file_open (EBookBackendSync *backend,
        GError           *local_error = NULL;
        gboolean          populated;
        ESourceBackendSummarySetup *setup;
+       ESourceRevisionGuards *guards;
 
        source = e_backend_get_source (E_BACKEND (backend));
        registry = e_book_backend_get_registry (E_BOOK_BACKEND (backend));
@@ -1287,7 +1322,11 @@ e_book_backend_file_open (EBookBackendSync *backend,
        backup   = g_build_filename (dirname, "addressbook.db.old", NULL);
 
        g_type_ensure (E_TYPE_SOURCE_BACKEND_SUMMARY_SETUP);
+       g_type_ensure (E_TYPE_SOURCE_REVISION_GUARDS);
        setup = e_source_get_extension (source, E_SOURCE_EXTENSION_BACKEND_SUMMARY_SETUP);
+       guards = e_source_get_extension (source, E_SOURCE_EXTENSION_REVISION_GUARDS);
+
+       bf->priv->revision_guards = e_source_revision_guards_get_enabled (guards);
 
        /* The old BDB exists, lets migrate that to sqlite right away
         */
@@ -1455,14 +1494,14 @@ e_book_backend_file_open (EBookBackendSync *backend,
                return;
        bf->priv->photo_dirname = dirname;
 
-       g_rec_mutex_lock (&bf->priv->revision_mutex);
+       g_rw_lock_writer_lock (&(bf->priv->lock));
        if (!bf->priv->revision) {
                e_book_backend_file_load_revision (bf);
                e_book_backend_notify_property_changed (E_BOOK_BACKEND (backend),
                                                        BOOK_BACKEND_PROPERTY_REVISION,
                                                        bf->priv->revision);
-               g_rec_mutex_unlock (&bf->priv->revision_mutex);
        }
+       g_rw_lock_writer_unlock (&(bf->priv->lock));
 
        e_book_backend_notify_online (E_BOOK_BACKEND (backend), TRUE);
        e_book_backend_notify_readonly (E_BOOK_BACKEND (backend), FALSE);
@@ -1499,9 +1538,9 @@ e_book_backend_file_get_backend_property (EBookBackendSync *backend,
                *prop_value = e_data_book_string_slist_to_comma_string (fields);
                g_slist_free (fields);
        } else if (g_str_equal (prop_name, BOOK_BACKEND_PROPERTY_REVISION)) {
-               g_rec_mutex_lock (&bf->priv->revision_mutex);
+               g_rw_lock_reader_lock (&(bf->priv->lock));
                *prop_value = g_strdup (bf->priv->revision);
-               g_rec_mutex_unlock (&bf->priv->revision_mutex);
+               g_rw_lock_reader_unlock (&(bf->priv->lock));
        } else {
                processed = FALSE;
        }
@@ -1606,7 +1645,7 @@ e_book_backend_file_finalize (GObject *object)
 
        g_free (priv->photo_dirname);
        g_free (priv->revision);
-       g_rec_mutex_clear (&priv->revision_mutex);
+       g_rw_lock_clear (&(priv->lock));
 
        /* Chain up to parent's finalize() method. */
        G_OBJECT_CLASS (e_book_backend_file_parent_class)->finalize (object);
@@ -1648,7 +1687,7 @@ e_book_backend_file_init (EBookBackendFile *backend)
 {
        backend->priv = E_BOOK_BACKEND_FILE_GET_PRIVATE (backend);
 
-       g_rec_mutex_init (&backend->priv->revision_mutex);
+       g_rw_lock_init (&(backend->priv->lock));
 
        g_signal_connect (
                backend, "notify::online",