GNOME.org
 

[evolution-data-server] EDataBook: Queue operations while opening.

commit e3cd6cde0182991c590200da00a02a97d98ea954
Author: Matthew Barnes <mbarnes redhat com>
Date:   Wed Jan 23 10:01:50 2013 -0500

    EDataBook: Queue operations while opening.
    
    When an open operation is in progress, queue any subsequent operations
    and dispatch them after the current open operation is complete.
    
    The old behavior returned a BUSY error and forced the client to retry.
    Clients are not involved in authentication anymore, so they should not
    have to retry operations nor worry about whether the backend is opened.
    
    This should really be handled directly by EBookBackend, but because of
    the weird async semantics between EBookBackend and EDataBook, the queue
    needs to live in EDataBook for now.

 addressbook/libedata-book/e-book-backend.c |   57 ++++++++++++----------
 addressbook/libedata-book/e-data-book.c    |   71 ++++++++++++++++++++++++---
 2 files changed, 94 insertions(+), 34 deletions(-)
---
diff --git a/addressbook/libedata-book/e-book-backend.c b/addressbook/libedata-book/e-book-backend.c
index f84b496..8272bb5 100644
--- a/addressbook/libedata-book/e-book-backend.c
+++ b/addressbook/libedata-book/e-book-backend.c
@@ -14,7 +14,6 @@
 #include "e-data-book.h"
 #include "e-book-backend.h"
 
-#define EDB_OPENING_ERROR	e_data_book_create_error (E_DATA_BOOK_STATUS_BUSY, _("Cannot process, book backend is opening"))
 #define EDB_NOT_OPENED_ERROR	e_data_book_create_error (E_DATA_BOOK_STATUS_NOT_OPENED, NULL)
 
 #define E_BOOK_BACKEND_GET_PRIVATE(obj) \
@@ -434,6 +433,9 @@ e_book_backend_open (EBookBackend *backend,
 	g_return_if_fail (E_IS_BOOK_BACKEND (backend));
 	g_return_if_fail (E_IS_DATA_BOOK (book));
 
+	/* This should never be called while we're opening. */
+	g_return_if_fail (!e_book_backend_is_opening (backend));
+
 	g_mutex_lock (&backend->priv->clients_mutex);
 
 	if (e_book_backend_is_opened (backend)) {
@@ -443,10 +445,6 @@ e_book_backend_open (EBookBackend *backend,
 		e_data_book_report_online (book, backend->priv->online);
 
 		e_book_backend_respond_opened (backend, book, opid, NULL);
-	} else if (e_book_backend_is_opening (backend)) {
-		g_mutex_unlock (&backend->priv->clients_mutex);
-
-		e_data_book_respond_open (book, opid, EDB_OPENING_ERROR);
 	} else {
 		backend->priv->opening = TRUE;
 		g_mutex_unlock (&backend->priv->clients_mutex);
@@ -482,9 +480,10 @@ e_book_backend_refresh (EBookBackend *backend,
 	g_return_if_fail (backend != NULL);
 	g_return_if_fail (E_IS_BOOK_BACKEND (backend));
 
-	if (e_book_backend_is_opening (backend))
-		e_data_book_respond_refresh (book, opid, EDB_OPENING_ERROR);
-	else if (!E_BOOK_BACKEND_GET_CLASS (backend)->refresh)
+	/* This should never be called while we're opening. */
+	g_return_if_fail (!e_book_backend_is_opening (backend));
+
+	if (!E_BOOK_BACKEND_GET_CLASS (backend)->refresh)
 		e_data_book_respond_refresh (book, opid, e_data_book_create_error (E_DATA_BOOK_STATUS_NOT_SUPPORTED, NULL));
 	else if (!e_book_backend_is_opened (backend))
 		e_data_book_respond_refresh (book, opid, EDB_NOT_OPENED_ERROR);
@@ -518,9 +517,10 @@ e_book_backend_create_contacts (EBookBackend *backend,
 	g_return_if_fail (vcards);
 	g_return_if_fail (E_BOOK_BACKEND_GET_CLASS (backend)->create_contacts);
 
-	if (e_book_backend_is_opening (backend))
-		e_data_book_respond_create_contacts (book, opid, EDB_OPENING_ERROR, NULL);
-	else if (!e_book_backend_is_opened (backend))
+	/* This should never be called while we're opening. */
+	g_return_if_fail (!e_book_backend_is_opening (backend));
+
+	if (!e_book_backend_is_opened (backend))
 		e_data_book_respond_create_contacts (book, opid, EDB_NOT_OPENED_ERROR, NULL);
 	else
 		(* E_BOOK_BACKEND_GET_CLASS (backend)->create_contacts) (backend, book, opid, cancellable, vcards);
@@ -550,9 +550,10 @@ e_book_backend_remove_contacts (EBookBackend *backend,
 	g_return_if_fail (id_list);
 	g_return_if_fail (E_BOOK_BACKEND_GET_CLASS (backend)->remove_contacts);
 
-	if (e_book_backend_is_opening (backend))
-		e_data_book_respond_remove_contacts (book, opid, EDB_OPENING_ERROR, NULL);
-	else if (!e_book_backend_is_opened (backend))
+	/* This should never be called while we're opening. */
+	g_return_if_fail (!e_book_backend_is_opening (backend));
+
+	if (!e_book_backend_is_opened (backend))
 		e_data_book_respond_remove_contacts (book, opid, EDB_NOT_OPENED_ERROR, NULL);
 	else
 		(* E_BOOK_BACKEND_GET_CLASS (backend)->remove_contacts) (backend, book, opid, cancellable, id_list);
@@ -584,9 +585,10 @@ e_book_backend_modify_contacts (EBookBackend *backend,
 	g_return_if_fail (vcards);
 	g_return_if_fail (E_BOOK_BACKEND_GET_CLASS (backend)->modify_contacts);
 
-	if (e_book_backend_is_opening (backend))
-		e_data_book_respond_modify_contacts (book, opid, EDB_OPENING_ERROR, NULL);
-	else if (!e_book_backend_is_opened (backend))
+	/* This should never be called while we're opening. */
+	g_return_if_fail (!e_book_backend_is_opening (backend));
+
+	if (!e_book_backend_is_opened (backend))
 		e_data_book_respond_modify_contacts (book, opid, EDB_NOT_OPENED_ERROR, NULL);
 	else
 		(* E_BOOK_BACKEND_GET_CLASS (backend)->modify_contacts) (backend, book, opid, cancellable, vcards);
@@ -616,9 +618,10 @@ e_book_backend_get_contact (EBookBackend *backend,
 	g_return_if_fail (id);
 	g_return_if_fail (E_BOOK_BACKEND_GET_CLASS (backend)->get_contact);
 
-	if (e_book_backend_is_opening (backend))
-		e_data_book_respond_get_contact (book, opid, EDB_OPENING_ERROR, NULL);
-	else if (!e_book_backend_is_opened (backend))
+	/* This should never be called while we're opening. */
+	g_return_if_fail (!e_book_backend_is_opening (backend));
+
+	if (!e_book_backend_is_opened (backend))
 		e_data_book_respond_get_contact (book, opid, EDB_NOT_OPENED_ERROR, NULL);
 	else
 		(* E_BOOK_BACKEND_GET_CLASS (backend)->get_contact) (backend, book, opid, cancellable, id);
@@ -648,9 +651,10 @@ e_book_backend_get_contact_list (EBookBackend *backend,
 	g_return_if_fail (query);
 	g_return_if_fail (E_BOOK_BACKEND_GET_CLASS (backend)->get_contact_list);
 
-	if (e_book_backend_is_opening (backend))
-		e_data_book_respond_get_contact_list (book, opid, EDB_OPENING_ERROR, NULL);
-	else if (!e_book_backend_is_opened (backend))
+	/* This should never be called while we're opening. */
+	g_return_if_fail (!e_book_backend_is_opening (backend));
+
+	if (!e_book_backend_is_opened (backend))
 		e_data_book_respond_get_contact_list (book, opid, EDB_NOT_OPENED_ERROR, NULL);
 	else
 		(* E_BOOK_BACKEND_GET_CLASS (backend)->get_contact_list) (backend, book, opid, cancellable, query);
@@ -682,9 +686,10 @@ e_book_backend_get_contact_list_uids (EBookBackend *backend,
 	g_return_if_fail (query);
 	g_return_if_fail (E_BOOK_BACKEND_GET_CLASS (backend)->get_contact_list_uids);
 
-	if (e_book_backend_is_opening (backend))
-		e_data_book_respond_get_contact_list_uids (book, opid, EDB_OPENING_ERROR, NULL);
-	else if (!e_book_backend_is_opened (backend))
+	/* This should never be called while we're opening. */
+	g_return_if_fail (!e_book_backend_is_opening (backend));
+
+	if (!e_book_backend_is_opened (backend))
 		e_data_book_respond_get_contact_list_uids (book, opid, EDB_NOT_OPENED_ERROR, NULL);
 	else
 		(* E_BOOK_BACKEND_GET_CLASS (backend)->get_contact_list_uids) (backend, book, opid, cancellable, query);
diff --git a/addressbook/libedata-book/e-data-book.c b/addressbook/libedata-book/e-data-book.c
index 6ab2353..161a9c8 100644
--- a/addressbook/libedata-book/e-data-book.c
+++ b/addressbook/libedata-book/e-data-book.c
@@ -47,6 +47,12 @@ struct _EDataBookPrivate {
 
 	GRecMutex pending_ops_lock;
 	GHashTable *pending_ops; /* opid to GCancellable for still running operations */
+
+	/* Operations are queued while an
+	 * open operation is in progress. */
+	GMutex open_lock;
+	guint32 open_opid;
+	GQueue open_queue;
 };
 
 enum {
@@ -289,6 +295,25 @@ op_new (OperationID op,
 }
 
 static void
+op_dispatch (EDataBook *book,
+             OperationData *data)
+{
+	g_mutex_lock (&book->priv->open_lock);
+
+	/* If an open operation is currently in progress, queue this
+	 * operation to be dispatched when the open operation finishes. */
+	if (book->priv->open_opid > 0) {
+		g_queue_push_tail (&book->priv->open_queue, data);
+	} else {
+		if (data->op == OP_OPEN)
+			book->priv->open_opid = data->id;
+		e_operation_pool_push (ops_pool, data);
+	}
+
+	g_mutex_unlock (&book->priv->open_lock);
+}
+
+static void
 op_complete (EDataBook *book,
              guint32 opid)
 {
@@ -507,7 +532,7 @@ impl_Book_open (EGdbusBook *interface,
 
 	e_gdbus_book_complete_open (interface, invocation, op->id);
 
-	e_operation_pool_push (ops_pool, op);
+	op_dispatch (book, op);
 
 	return TRUE;
 }
@@ -523,7 +548,7 @@ impl_Book_refresh (EGdbusBook *interface,
 
 	e_gdbus_book_complete_refresh (interface, invocation, op->id);
 
-	e_operation_pool_push (ops_pool, op);
+	op_dispatch (book, op);
 
 	return TRUE;
 }
@@ -551,7 +576,7 @@ impl_Book_get_contact (EGdbusBook *interface,
 
 	e_gdbus_book_complete_get_contact (interface, invocation, op->id);
 
-	e_operation_pool_push (ops_pool, op);
+	op_dispatch (book, op);
 
 	return TRUE;
 }
@@ -577,7 +602,7 @@ impl_Book_get_contact_list (EGdbusBook *interface,
 
 	e_gdbus_book_complete_get_contact_list (interface, invocation, op->id);
 
-	e_operation_pool_push (ops_pool, op);
+	op_dispatch (book, op);
 
 	return TRUE;
 }
@@ -603,7 +628,7 @@ impl_Book_get_contact_list_uids (EGdbusBook *interface,
 
 	e_gdbus_book_complete_get_contact_list_uids (interface, invocation, op->id);
 
-	e_operation_pool_push (ops_pool, op);
+	op_dispatch (book, op);
 
 	return TRUE;
 }
@@ -629,7 +654,7 @@ impl_Book_add_contacts (EGdbusBook *interface,
 
 	e_gdbus_book_complete_add_contacts (interface, invocation, op->id);
 
-	e_operation_pool_push (ops_pool, op);
+	op_dispatch (book, op);
 
 	return TRUE;
 }
@@ -655,7 +680,7 @@ impl_Book_modify_contacts (EGdbusBook *interface,
 
 	e_gdbus_book_complete_modify_contacts (interface, invocation, op->id);
 
-	e_operation_pool_push (ops_pool, op);
+	op_dispatch (book, op);
 
 	return TRUE;
 }
@@ -677,7 +702,7 @@ impl_Book_remove_contacts (EGdbusBook *interface,
 
 	e_gdbus_book_complete_remove_contacts (interface, invocation, op->id);
 
-	e_operation_pool_push (ops_pool, op);
+	op_dispatch (book, op);
 
 	return TRUE;
 }
@@ -695,6 +720,7 @@ impl_Book_get_backend_property (EGdbusBook *interface,
 
 	e_gdbus_book_complete_get_backend_property (interface, invocation, op->id);
 
+	/* This operation is never queued. */
 	e_operation_pool_push (ops_pool, op);
 
 	return TRUE;
@@ -713,6 +739,7 @@ impl_Book_set_backend_property (EGdbusBook *interface,
 
 	e_gdbus_book_complete_set_backend_property (interface, invocation, op->id);
 
+	/* This operation is never queued. */
 	e_operation_pool_push (ops_pool, op);
 
 	return TRUE;
@@ -742,6 +769,7 @@ impl_Book_get_view (EGdbusBook *interface,
 
 	e_gdbus_book_complete_get_view (interface, invocation, op->id);
 
+	/* This operation is never queued. */
 	e_operation_pool_push (ops_pool, op);
 
 	return TRUE;
@@ -760,6 +788,7 @@ impl_Book_cancel_operation (EGdbusBook *interface,
 
 	e_gdbus_book_complete_cancel_operation (interface, invocation, NULL);
 
+	/* This operation is never queued. */
 	e_operation_pool_push (ops_pool, op);
 
 	return TRUE;
@@ -776,6 +805,7 @@ impl_Book_cancel_all (EGdbusBook *interface,
 
 	e_gdbus_book_complete_cancel_all (interface, invocation, NULL);
 
+	/* This operation is never queued. */
 	e_operation_pool_push (ops_pool, op);
 
 	return TRUE;
@@ -794,6 +824,7 @@ impl_Book_close (EGdbusBook *interface,
 
 	e_gdbus_book_complete_close (interface, invocation, NULL);
 
+	/* This operation is never queued. */
 	e_operation_pool_push (ops_pool, op);
 
 	return TRUE;
@@ -813,6 +844,23 @@ e_data_book_respond_open (EDataBook *book,
 
 	if (error != NULL)
 		g_error_free (error);
+
+	/* Dispatch any pending operations. */
+
+	g_mutex_lock (&book->priv->open_lock);
+
+	if (opid == book->priv->open_opid) {
+		OperationData *op;
+
+		book->priv->open_opid = 0;
+
+		while (!g_queue_is_empty (&book->priv->open_queue)) {
+			op = g_queue_pop_head (&book->priv->open_queue);
+			e_operation_pool_push (ops_pool, op);
+		}
+	}
+
+	g_mutex_unlock (&book->priv->open_lock);
 }
 
 /**
@@ -1310,6 +1358,11 @@ data_book_finalize (GObject *object)
 		priv->dbus_interface = NULL;
 	}
 
+	g_mutex_clear (&priv->open_lock);
+
+	/* This should be empty now, else we leak memory. */
+	g_warn_if_fail (g_queue_is_empty (&priv->open_queue));
+
 	/* Chain up to parent's finalize() method. */
 	G_OBJECT_CLASS (e_data_book_parent_class)->finalize (object);
 }
@@ -1403,6 +1456,8 @@ e_data_book_init (EDataBook *ebook)
 		g_direct_hash, g_direct_equal, NULL, g_object_unref);
 	g_rec_mutex_init (&ebook->priv->pending_ops_lock);
 
+	g_mutex_init (&ebook->priv->open_lock);
+
 	dbus_interface = ebook->priv->dbus_interface;
 	g_signal_connect (
 		dbus_interface, "handle-open",
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]