Re: [Evolution-hackers] EBookSqlite support for EWS

2014-08-23 Thread Tristan Van Berkom
On Fri, 2014-08-22 at 17:46 -0500, David Woodhouse wrote:
> On Fri, 2014-08-22 at 17:04 -0500, David Woodhouse wrote:
> > 
> > ¹ Seriously, it *was* an accident. I thought I needed to port to
> >   EBookSqlite to make cursors work, which I need for Yuuma's PKCS#11
> >   module. But EBookBackendSqliteDB can do cursors anyway, so I didn't
> >   need to do the conversion. Not that it's not worthwhile anyway...
> 
> Hm, actually I think maybe it *is* necessary. Sure, we have 
> e_book_backend_sqlitedb_cursor_new() but we can't make an
> EDataBookCursor out of that. Commit a8e5f5c0 ported
> EDataBookCursorSqlite over to be based on EBookSqlite and now it doesn't
> work for EBookBackendSqliteDB any more.
> 
> But that's OK; I've done the port to EBookSqlite now anyway. And as an
> added bonus, as it migrates to the new database format I can add new
> summary fields. With the new PKCS#11 module we're *really* going to want
> e_book_query_field_exists(E_CONTACT_X509_CERT) to be fast.
> 
> Unfortunately, we only support boolean and string fields in the summary;
> we can't just add E_CONTACT_X509_CERT to the list of summary fields.
> 
> Although actually, all I *wanted* was a boolean. I didn't really want
> the whole of the cert data to be duplicated in the summary; I only
> wanted a boolean indication that the cert was *present*...
> 
> Tristan, any ideas on how best to handle this?

Hi David,
Unfortunately I did not take into account the possibility of
speeding up e_book_query_field_exists() queries without speeding up
other queries on the same EContactField when designing the
ESourceBackendSummarySetup extension API.

That said, there are two clear paths to achieving what you want.
One way is to add a synthetic boolean EContactField (described in more
detail below) and the other would be to extend EBookSqlite and
ESourceBackendSummarySetup to allow for speeding up
e_book_query_field_exists() tests without adding the entire string to
the summary.

While the latter would benefit other possible use cases, I doubt that
there are enough use cases to justify the work it would take, so I would
suggest the former 'easy way out'.


Adding a synthetic boolean EContactField

Some contact fields are synthetic fields which are basically resolved on
demand whenever an EContact is asked for the value of the given field.

Some relevant examples of these are the E_CONTACT_NAME_OR_ORG and
E_CONTACT_CATEGORIES fields as they are easy enough to follow in
e-contact.c

If you were to define a new synthetic boolean field named
E_CONTACT_HAS_X509_CERT, then you would just need to add a case to
e_contact_get() which returns TRUE or FALSE depending on the presence of
the certificate in the vcard, and you would have a usable EContactField
to use for your addressbook summary index.

This would of course not speed up:
 e_book_query_field_exists (E_CONTACT_X509_CERT)

but would allow you to add E_CONTACT_HAS_X509_CERT to your summary as an
indexed boolean field.


Also, while I'm here, your patch which modifies the
EBSQL_LOCK_OR_RETURN() macro looks fine and correct to me, you also
have my blessing on your patch which adds the E_BOOK_SQL_SYNC_DATA_KEY,
The only reason I did not add that one is because I did not see it being
used in the backends I looked at.


I hope this mail was helpful to you and I'm happy to see EBookSqlite
get some more mileage :)

Best,
-Tristan


___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
https://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] EBookSqlite support for EWS

2014-08-22 Thread David Woodhouse
On Fri, 2014-08-22 at 17:04 -0500, David Woodhouse wrote:
> 
> ¹ Seriously, it *was* an accident. I thought I needed to port to
>   EBookSqlite to make cursors work, which I need for Yuuma's PKCS#11
>   module. But EBookBackendSqliteDB can do cursors anyway, so I didn't
>   need to do the conversion. Not that it's not worthwhile anyway...

Hm, actually I think maybe it *is* necessary. Sure, we have 
e_book_backend_sqlitedb_cursor_new() but we can't make an
EDataBookCursor out of that. Commit a8e5f5c0 ported
EDataBookCursorSqlite over to be based on EBookSqlite and now it doesn't
work for EBookBackendSqliteDB any more.

But that's OK; I've done the port to EBookSqlite now anyway. And as an
added bonus, as it migrates to the new database format I can add new
summary fields. With the new PKCS#11 module we're *really* going to want
e_book_query_field_exists(E_CONTACT_X509_CERT) to be fast.

Unfortunately, we only support boolean and string fields in the summary;
we can't just add E_CONTACT_X509_CERT to the list of summary fields.

Although actually, all I *wanted* was a boolean. I didn't really want
the whole of the cert data to be duplicated in the summary; I only
wanted a boolean indication that the cert was *present*...

Tristan, any ideas on how best to handle this?

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature
___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
https://mail.gnome.org/mailman/listinfo/evolution-hackers


[Evolution-hackers] EBookSqlite support for EWS

2014-08-22 Thread David Woodhouse
I accidentally¹ ported the EWS addressbook to EBookSqlite. Patch
(against 3.12.5) attached.

It ends up hitting this check and not actually doing any database
writes:

(evolution-addressbook-factory:19473): libedata-book-WARNING **: The
GCancellable passed to `e_book_sqlite_add_contacts' is not the same as
the cancel object passed to e_book_sqlite_lock()

I think the check is wrong, because we're not using e_book_sqlite_lock()
at all. Does it make sense to fix it thus: 

diff --git a/addressbook/libedata-book/e-book-sqlite.c 
b/addressbook/libedata-book/e-book-sqlite.c
index 0ae89cd..9e5cb78 100644
--- a/addressbook/libedata-book/e-book-sqlite.c
+++ b/addressbook/libedata-book/e-book-sqlite.c
@@ -209,7 +209,7 @@ ebsql_init_debug (void)
 #define EBSQL_LOCK_OR_RETURN(ebsql, cancellable, val) \
G_STMT_START { \
EBSQL_LOCK_MUTEX (&(ebsql)->priv->lock); \
-   if (cancellable != NULL && \
+   if (cancellable != NULL && (ebsql)->priv->cancel && \
(ebsql)->priv->cancel != cancellable) { \
g_warning ("The GCancellable passed to `%s' " \
   "is not the same as the cancel object " \



-- 
dwmw2

¹ Seriously, it *was* an accident. I thought I needed to port to
  EBookSqlite to make cursors work, which I need for Yuuma's PKCS#11
  module. But EBookBackendSqliteDB can do cursors anyway, so I didn't
  need to do the conversion. Not that it's not worthwhile anyway...

diff --git a/src/addressbook/e-book-backend-ews.c b/src/addressbook/e-book-backend-ews.c
index 3315475..ec7bf38 100644
--- a/src/addressbook/e-book-backend-ews.c
+++ b/src/addressbook/e-book-backend-ews.c
@@ -73,7 +73,7 @@ struct _EBookBackendEwsPrivate {
 	gchar *oab_url;
 	gchar *folder_name;
 
-	EBookBackendSqliteDB *summary;
+	EBookSqlite *summary;
 
 	gboolean only_if_exists;
 	gboolean is_writable;
@@ -1371,7 +1371,7 @@ ews_create_contact_cb (GObject *object,
 
 		e_contact_set (create_contact->contact, E_CONTACT_UID, item_id->id);
 		e_contact_set (create_contact->contact, E_CONTACT_REV, item_id->change_key);
-		e_book_backend_sqlitedb_new_contact (ebews->priv->summary, ebews->priv->folder_id, create_contact->contact, TRUE, &error);
+		e_book_sqlite_add_contact (ebews->priv->summary, create_contact->contact, NULL, TRUE, create_contact->cancellable, &error);
 
 		if (error == NULL) {
 			GSList *contacts;
@@ -1510,6 +1510,7 @@ typedef struct {
 	EDataBook *book;
 	guint32 opid;
 	GSList *sl_ids;
+	GCancellable *cancellable;
 } EwsRemoveContact;
 
 static void
@@ -1529,7 +1530,7 @@ ews_book_remove_contact_cb (GObject *object,
 	g_return_if_fail (priv->summary != NULL);
 
 	if (!g_simple_async_result_propagate_error (simple, &error))
-		deleted = e_book_backend_sqlitedb_remove_contacts (priv->summary, priv->folder_id, remove_contact->sl_ids, &error);
+		deleted = e_book_sqlite_remove_contacts (priv->summary, remove_contact->sl_ids, remove_contact->cancellable, &error);
 
 	if (deleted)
 		e_data_book_respond_remove_contacts (remove_contact->book, remove_contact->opid, EDB_ERROR (SUCCESS),  remove_contact->sl_ids);
@@ -1542,6 +1543,7 @@ ews_book_remove_contact_cb (GObject *object,
 	g_slist_free_full (remove_contact->sl_ids, g_free);
 	g_object_unref (remove_contact->ebews);
 	g_object_unref (remove_contact->book);
+	g_object_unref (remove_contact->cancellable);
 	g_free (remove_contact);
 	g_clear_error (&error);
 }
@@ -1593,6 +1595,7 @@ e_book_backend_ews_remove_contacts (EBookBackend *backend,
 	remove_contact->book = g_object_ref (book);
 	remove_contact->opid = opid;
 	remove_contact->sl_ids = copy;
+	remove_contact->cancellable = g_object_ref(cancellable);
 
 	e_ews_connection_delete_items (
 		priv->cnc, EWS_PRIORITY_MEDIUM, (GSList *) id_list,
@@ -1647,12 +1650,11 @@ ews_modify_contact_cb (GObject *object,
 
 		id = e_contact_get (modify_contact->old_contact, E_CONTACT_UID);
 
-		e_book_backend_sqlitedb_remove_contact (priv->summary, priv->folder_id, id, &error);
-		e_book_backend_sqlitedb_new_contact (
+		e_book_sqlite_remove_contact (ebews->priv->summary, id, modify_contact->cancellable, &error);
+		e_book_sqlite_add_contact (
 ebews->priv->summary,
-ebews->priv->folder_id,
-modify_contact->new_contact,
-TRUE,
+modify_contact->new_contact, NULL,
+TRUE, modify_contact->cancellable,
 &error);
 
 		if (error == NULL) {
@@ -1830,10 +1832,7 @@ e_book_backend_ews_modify_contacts (EBookBackend *backend,
 	id->id = e_contact_get (contact, E_CONTACT_UID);
 	id->change_key = e_contact_get (contact, E_CONTACT_REV);
 
-	old_contact = e_book_backend_sqlitedb_get_contact (
-		priv->summary, priv->folder_id,
-		id->id, NULL, NULL, &error);
-	if (!old_contact) {
+	if (!e_book_sqlite_get_contact (priv->summary, id->id, TRUE, &old_contact, &error)) {
 		g_object_unref (contact);
 		e_data_book_respond_modify_contacts (book, opid, EDB_ERROR (NOT_SUPPORTED), NULL);
 		return;
@@ -1900,20 +1899,25 @@ e_book_