evolution-data-server r9911 - in trunk/camel: . providers/imap

[msuman svn gnome org]

Author: msuman
Date: Mon Jan 12 03:46:45 2009
New Revision: 9911
URL: http://svn.gnome.org/viewvc/evolution-data-server?rev=9911&view=rev

Log:
Patch from Robert Collins  <robertc robertcollins net>: Fix for bug #564339 (Reduce the IO done during sync by not reading into memory locally cached files).

Modified:
   trunk/camel/ChangeLog
   trunk/camel/camel-folder.c
   trunk/camel/camel-folder.h
   trunk/camel/camel-offline-folder.c
   trunk/camel/providers/imap/ChangeLog
   trunk/camel/providers/imap/camel-imap-folder.c
   trunk/camel/providers/imap/camel-imap-message-cache.c
   trunk/camel/providers/imap/camel-imap-message-cache.h

Modified: trunk/camel/camel-folder.c
==============================================================================
--- trunk/camel/camel-folder.c	(original)
+++ trunk/camel/camel-folder.c	Mon Jan 12 03:46:45 2009
@@ -88,6 +88,7 @@
 			    CamelException *ex);
 
 static GPtrArray        *get_uids            (CamelFolder *folder);
+static GPtrArray 	*get_uncached_uids   (CamelFolder *, GPtrArray * uids, CamelException *);
 static void              free_uids           (CamelFolder *folder,
 					      GPtrArray *array);
 static void              sort_uids           (CamelFolder *folder,
@@ -152,6 +153,7 @@
 	camel_folder_class->set_message_user_tag = set_message_user_tag;
 	camel_folder_class->get_message = get_message;
 	camel_folder_class->get_uids = get_uids;
+	camel_folder_class->get_uncached_uids = get_uncached_uids;
 	camel_folder_class->free_uids = free_uids;
 	camel_folder_class->sort_uids = sort_uids;
 	camel_folder_class->get_summary = get_summary;
@@ -167,6 +169,7 @@
 	camel_folder_class->delete = delete;
 	camel_folder_class->rename = folder_rename;
 	camel_folder_class->freeze = freeze;
+	camel_folder_class->sync_message = NULL;
 	camel_folder_class->thaw = thaw;
 	camel_folder_class->is_frozen = is_frozen;
 	camel_folder_class->get_quota_info = get_quota_info;
@@ -1155,6 +1158,35 @@
 	return ret;
 }
 
+/**
+ * camel_folder_sync_message:
+ * @folder: a #CamelFolder object
+ * @uid: the UID
+ * @ex: a #CamelException
+ *
+ * Ensure that a message identified by UID has been synced in the folder (so
+ * that camel_folder_get_message on it later will work in offline mode).
+ *
+ * Returns: void.
+ **/
+void
+camel_folder_sync_message (CamelFolder *folder, const char *uid, CamelException *ex)
+{
+	g_return_if_fail (CAMEL_IS_FOLDER (folder));
+	CAMEL_FOLDER_REC_LOCK(folder, lock);
+
+	/* Use the sync_message method if the class implements it. */
+	if (CF_CLASS (folder)->sync_message)
+		CF_CLASS (folder)->sync_message (folder, uid, ex);
+	else {
+		CamelMimeMessage *message;
+		message = CF_CLASS (folder)->get_message (folder, uid, ex);
+		if (message)
+			  camel_object_unref(message);
+	}
+	CAMEL_FOLDER_REC_UNLOCK(folder, lock);
+}
+
 static GPtrArray *
 get_uids(CamelFolder *folder)
 {
@@ -1215,6 +1247,41 @@
 }
 
 
+/**
+ * Default: return the uids we are given.
+ */
+static GPtrArray *
+get_uncached_uids (CamelFolder *folder, GPtrArray * uids, CamelException *ex)
+{
+	GPtrArray *result;
+	int i;
+
+	result = g_ptr_array_new();
+
+	g_ptr_array_set_size(result, uids->len);
+	for (i = 0; i < uids->len; i++)
+	    result->pdata[i] = (char *)camel_pstring_strdup(uids->pdata[i]);
+	return result;
+}
+
+/**
+ * camel_folder_get_uncached_uids:
+ * @folder: a #CamelFolder object
+ * @uids: the array of uids to filter down to uncached ones.
+ *
+ * Returns the known-uncached uids from a list of uids. It may return uids
+ * which are locally cached but should never filter out a uid which is not
+ * locally cached. Free the result by called #camel_folder_free_uids.
+ * Frees the array of UIDs returned by #camel_folder_get_uids.
+ **/
+GPtrArray *
+camel_folder_get_uncached_uids (CamelFolder *folder, GPtrArray * uids, CamelException *ex)
+{
+	g_return_val_if_fail (CAMEL_IS_FOLDER (folder), NULL);
+	return CF_CLASS (folder)->get_uncached_uids(folder, uids, ex);
+}
+
+
 static int
 uidcmp (const void *v0, const void *v1)
 {

Modified: trunk/camel/camel-folder.h
==============================================================================
--- trunk/camel/camel-folder.h	(original)
+++ trunk/camel/camel-folder.h	Mon Jan 12 03:46:45 2009
@@ -211,6 +211,10 @@
 	
 	CamelFolderQuotaInfo * (*get_quota_info) (CamelFolder *folder);
 	guint32	(*count_by_expression) (CamelFolder *, const char *, CamelException *);
+	void (*sync_message)  (CamelFolder *folder,
+                               const char *uid, 
+                               CamelException *ex);
+        GPtrArray * (*get_uncached_uids)(CamelFolder *, GPtrArray * uids, CamelException *);
 	char * (*get_filename) (CamelFolder *, const char *uid, CamelException *);
 } CamelFolderClass;
 
@@ -304,12 +308,18 @@
 CamelMimeMessage * camel_folder_get_message           (CamelFolder *folder, 
 						       const char *uid, 
 						       CamelException *ex);
+void               camel_folder_sync_message          (CamelFolder *folder, 
+						       const char *uid, 
+						       CamelException *ex);
 #define camel_folder_delete_message(folder, uid) \
 	camel_folder_set_message_flags (folder, uid, CAMEL_MESSAGE_DELETED|CAMEL_MESSAGE_SEEN, CAMEL_MESSAGE_DELETED|CAMEL_MESSAGE_SEEN)
 
 GPtrArray *        camel_folder_get_uids              (CamelFolder *folder);
 void               camel_folder_free_uids             (CamelFolder *folder,
 						       GPtrArray *array);
+GPtrArray *        camel_folder_get_uncached_uids     (CamelFolder *,
+                                                       GPtrArray * uids,
+                                                       CamelException *);
 void               camel_folder_sort_uids             (CamelFolder *folder,
 						       GPtrArray *uids);
 

Modified: trunk/camel/camel-offline-folder.c
==============================================================================
--- trunk/camel/camel-offline-folder.c	(original)
+++ trunk/camel/camel-offline-folder.c	Mon Jan 12 03:46:45 2009
@@ -247,8 +247,7 @@
 offline_folder_downsync (CamelOfflineFolder *offline, const char *expression, CamelException *ex)
 {
 	CamelFolder *folder = (CamelFolder *) offline;
-	CamelMimeMessage *message;
-	GPtrArray *uids;
+	GPtrArray *uids, *uncached_uids = NULL;
 	int i;
 	
 	camel_operation_start (NULL, _("Syncing messages in folder '%s' to disk"), folder->full_name);
@@ -257,27 +256,29 @@
 		uids = camel_folder_search_by_expression (folder, expression, ex);
 	else
 		uids = camel_folder_get_uids (folder);
-	
-	if (!uids) {
-		camel_operation_end (NULL);
-		return;
+
+	if (!uids)
+	  	goto done;
+	uncached_uids = camel_folder_get_uncached_uids(folder, uids, ex);
+	if (uids) {
+		if (expression)
+			camel_folder_search_free (folder, uids);
+		else
+			camel_folder_free_uids (folder, uids);
 	}
 	
-	for (i = 0; i < uids->len; i++) {
-		int pc = i * 100 / uids->len;
-		
-		message = camel_folder_get_message (folder, uids->pdata[i], ex);
+	if (!uncached_uids)
+		goto done;
+	
+	for (i = 0; i < uncached_uids->len; i++) {
+		int pc = i * 100 / uncached_uids->len;
+		camel_folder_sync_message (folder, uncached_uids->pdata[i], ex);
 		camel_operation_progress (NULL, pc);
-		if (message == NULL)
-			break;
-		
-		camel_object_unref (message);
 	}
-	
-	if (expression)
-		camel_folder_search_free (folder, uids);
-	else
-		camel_folder_free_uids (folder, uids);
+
+done:
+	if (uncached_uids)
+		camel_folder_free_uids(folder, uncached_uids);
 	
 	camel_operation_end (NULL);
 }

Modified: trunk/camel/providers/imap/camel-imap-folder.c
==============================================================================
--- trunk/camel/providers/imap/camel-imap-folder.c	(original)
+++ trunk/camel/providers/imap/camel-imap-folder.c	Mon Jan 12 03:46:45 2009
@@ -102,11 +102,14 @@
 static void imap_expunge (CamelFolder *folder, CamelException *ex);
 //static void imap_cache_message (CamelDiscoFolder *disco_folder, const char *uid, CamelException *ex);
 static void imap_rename (CamelFolder *folder, const char *new);
+static GPtrArray * imap_get_uncached_uids (CamelFolder *folder, GPtrArray * uids, CamelException *ex);
 static char* imap_get_filename (CamelFolder *folder, const char *uid, CamelException *ex);
 
 /* message manipulation */
 static CamelMimeMessage *imap_get_message (CamelFolder *folder, const gchar *uid,
 					   CamelException *ex);
+static void imap_sync_message (CamelFolder *folder, const gchar *uid,
+			       CamelException *ex);
 static void imap_append_online (CamelFolder *folder, CamelMimeMessage *message,
 				const CamelMessageInfo *info, char **appended_uid,
 				CamelException *ex);
@@ -136,6 +139,12 @@
 
 static GData *parse_fetch_response (CamelImapFolder *imap_folder, char *msg_att);
 
+/* internal helpers */
+static CamelImapMessageInfo * imap_folder_summary_uid_or_error(
+	CamelFolderSummary *summary,
+	const char * uid,
+	CamelException *ex);
+
 #ifdef G_OS_WIN32
 /* The strtok() in Microsoft's C library is MT-safe (but still uses
  * only one buffer pointer per thread, but for the use of strtok_r()
@@ -167,7 +176,9 @@
 	camel_folder_class->expunge = imap_expunge;
 	camel_folder_class->sync= imap_sync;
 	camel_folder_class->append_message = imap_append_online;
+	camel_folder_class->sync_message = imap_sync_message;
 	camel_folder_class->transfer_messages_to = imap_transfer_online;
+	camel_folder_class->get_uncached_uids = imap_get_uncached_uids;
 	camel_folder_class->get_filename = imap_get_filename;
 }
 
@@ -2826,6 +2837,20 @@
 	return FALSE;
 }
 
+static CamelImapMessageInfo *
+imap_folder_summary_uid_or_error(CamelFolderSummary *summary, const char * uid, CamelException *ex)
+{
+  	CamelImapMessageInfo *mi;
+	mi = (CamelImapMessageInfo *)camel_folder_summary_uid (summary, uid);
+	if (mi == NULL) {
+		camel_exception_setv (
+                	ex, CAMEL_EXCEPTION_FOLDER_INVALID_UID,
+			_("Cannot get message with message ID %s: %s"),
+			uid, _("No such message available."));
+	}
+	return mi;
+}
+
 static CamelMimeMessage *
 imap_get_message (CamelFolder *folder, const char *uid, CamelException *ex)
 {
@@ -2836,14 +2861,9 @@
 	CamelStream *stream = NULL;
 	int retry;
 
-	mi = (CamelImapMessageInfo *)camel_folder_summary_uid (folder->summary, uid);
-	if (mi == NULL) {
-		camel_exception_setv (
-                	ex, CAMEL_EXCEPTION_FOLDER_INVALID_UID,
-			_("Cannot get message with message ID %s: %s"),
-			uid, _("No such message available."));
-		return NULL;
-	}
+	mi = imap_folder_summary_uid_or_error(folder->summary, uid, ex);
+	if (!mi)
+	  return NULL;
 
 	/* If its cached in full, just get it as is, this is only a shortcut,
 	   since we get stuff from the cache anyway.  It affects a busted connection though. */
@@ -2964,6 +2984,44 @@
 	return msg;
 }
 
+/**
+ * imap_sync_message
+ *
+ * Ensure that a message is cached locally, but don't retrieve the content if
+ * it is already local.
+ */
+static void
+imap_sync_message (CamelFolder *folder, const char *uid, CamelException *ex)
+{
+	CamelImapFolder *imap_folder = CAMEL_IMAP_FOLDER (folder);
+	CamelImapMessageInfo *mi;
+	CamelMimeMessage *msg = NULL;
+	CamelStream *stream = NULL;
+
+	mi = imap_folder_summary_uid_or_error(folder->summary, uid, ex);
+	if (!mi)
+	  /* No such UID - is this duplicate work? The sync process selects
+	   * UIDs to start with.
+	   */
+	  return;
+	camel_message_info_free(&mi->info);
+
+	/* If we can get a stream, assume its fully cached. This may be false
+	 * if partial streams are saved elsewhere in the code - but that seems
+	 * best solved by knowning more about whether a given message is fully
+	 * available locally or not,
+	 */
+	/* If its cached in full, just get it as is, this is only a shortcut,
+	   since we get stuff from the cache anyway.  It affects a busted connection though. */
+	if ((stream = camel_imap_folder_fetch_data(imap_folder, uid, "", TRUE, NULL))) {
+		camel_object_unref (stream);
+		return;
+	}
+	msg = imap_get_message(folder, uid, ex);
+	if (msg)
+		camel_object_unref(msg);
+}
+
 /* FIXME Remove it after confirming
 static void
 imap_cache_message (CamelDiscoFolder *disco_folder, const char *uid,
@@ -3938,3 +3996,19 @@
 	CAMEL_SERVICE_REC_UNLOCK (imap_store, connect_lock);
 	return res;
 }
+
+/**
+ * Scan for messages that are local and return the rest.
+ */
+static GPtrArray *
+imap_get_uncached_uids (CamelFolder *folder, GPtrArray * uids, CamelException *ex)
+{
+	GPtrArray *result;
+	CamelImapFolder *imap_folder = CAMEL_IMAP_FOLDER (folder);
+
+	CAMEL_IMAP_FOLDER_REC_LOCK (imap_folder, cache_lock);
+	result = camel_imap_message_cache_filter_cached (imap_folder->cache, uids, ex);
+	CAMEL_IMAP_FOLDER_REC_UNLOCK (imap_folder, cache_lock);
+	return result;
+}
+

Modified: trunk/camel/providers/imap/camel-imap-message-cache.c
==============================================================================
--- trunk/camel/providers/imap/camel-imap-message-cache.c	(original)
+++ trunk/camel/providers/imap/camel-imap-message-cache.c	Mon Jan 12 03:46:45 2009
@@ -35,6 +35,7 @@
 #include "camel-data-wrapper.h"
 #include "camel-exception.h"
 #include "camel-stream-fs.h"
+#include "camel-string-utils.h"
 
 #include "camel-imap-message-cache.h"
 
@@ -42,9 +43,27 @@
 #define O_BINARY 0
 #endif
 
+/* Common define to start reducing duplication of base-part handling on win32.
+ */
+#ifdef G_OS_WIN32
+/* See comment in insert_setup() */
+#define BASE_PART_SUFFIX ".~"
+#else
+#define BASE_PART_SUFFIX "."
+#endif
+
 static void finalize (CamelImapMessageCache *cache);
 static void stream_finalize (CamelObject *stream, gpointer event_data, gpointer user_data);
 
+struct _part_find {
+	/* UID name on disk - e.g. "0." or "0.HEADERS". On windows "0." is
+	 * stored as "0.~"
+	 */
+	char *disk_part_name;
+	/* Was the part found? */
+	int found;
+};
+
 
 CamelType
 camel_imap_message_cache_get_type (void)
@@ -142,6 +161,8 @@
  * Return value: a new CamelImapMessageCache object using @path for
  * storage. If cache files already exist in @path, then any that do not
  * correspond to messages in @summary will be deleted.
+ * @path is scanned for its contents, which means creating a cache object can be
+ * expensive, but the parts hash is immediately usable.
  **/
 CamelImapMessageCache *
 camel_imap_message_cache_new (const char *path, CamelFolderSummary *summary,
@@ -620,3 +641,67 @@
 		}
 	}
 }
+
+
+static void
+_match_part(gpointer part_name, gpointer user_data)
+{
+	struct _part_find *part_find = (struct _part_find *) user_data;
+	if (g_str_equal(part_name, part_find->disk_part_name))
+		part_find->found = 1;
+}
+
+/**
+ * Filter uids by the uids cached in cache.
+ * The intent is that only uids fully cached are returned, but that may not be
+ * what is achieved. An additional constraint is that this check should be
+ * cheap, so that going offline is not an expensive operation. Filtering all
+ * uids is inefficient in the first place; significant processing per uid 
+ * makes synchronisation very expensive. At the suggestion of Srinivasa Ragavan
+ * (see http://bugzilla.gnome.org/show_bug.cgi?id=564339) the cache->parts hash
+ * table is consulted. If there is a parts-list in the hash table containing
+ * the part "", then we assume the message has been completely downloaded. This
+ * is incorrect (see http://bugzilla.gnome.org/show_bug.cgi?id=561211 for the
+ * symptoms). The code this replaces, a loop over all uids asking for the ""
+ * part of the message has the same flaw: it is no /less/ accurate to assess
+ * 'cached' in the manner this method does (assuming no concurrent process is
+ * removing messages from the cache).
+ *
+ * In the future, fixing bug 561211 needs a check for *all* the parts of a
+ * given uid. If the complete list of parts is available in the folder summary
+ * information then it can be done cheaply, otherwise some redesign will be
+ * needed.
+ */
+GPtrArray *
+camel_imap_message_cache_filter_cached(CamelImapMessageCache *cache, GPtrArray *uids, CamelException *ex)
+{
+	GPtrArray *result, *parts_list;
+	int i;
+	struct _part_find part_find;
+	/* Look for a part "" for each uid. */
+	result = g_ptr_array_sized_new(uids->len);
+	for (i = 0; i < uids->len; i++) {
+		if ((parts_list = g_hash_table_lookup(cache->parts, uids->pdata[i]))) {
+			/* At least one part locally present; look for "" (the
+			 * HEADERS part can be present without anything else,
+			 * and that part is not useful for users wanting to
+			 * read the message).
+			 */
+			part_find.found = 0;
+			part_find.disk_part_name = g_strdup_printf("%s" BASE_PART_SUFFIX,
+								   (char *)uids->pdata[i]);
+			g_ptr_array_foreach(parts_list, _match_part, &part_find);
+			g_free(part_find.disk_part_name);
+			if (part_find.found)
+				/* The message is cached locally, do not
+				 * include it in the result.
+				 */
+				continue;
+		}
+		/* No message parts, or message part "" not found: include the
+		 * uid in the result.
+		 */
+		g_ptr_array_add(result, (char *)camel_pstring_strdup(uids->pdata[i]));
+	}
+	return result;
+}

Modified: trunk/camel/providers/imap/camel-imap-message-cache.h
==============================================================================
--- trunk/camel/providers/imap/camel-imap-message-cache.h	(original)
+++ trunk/camel/providers/imap/camel-imap-message-cache.h	Mon Jan 12 03:46:45 2009
@@ -41,6 +41,13 @@
 	CamelObject parent_object;
 
 	char *path;
+        /* parts contains two sorts of objects.
+         * If the key contains '.' then it is a stream (also reverse-indexed in
+         * cached).
+         * Otherwise it is a g_ptr_array containing the subparts the message
+         * has. (e.g. 0., or 0.MIME.1).
+         */
+        /* cached contains streams for recently accessed messages */
 	GHashTable *parts, *cached;
 	guint32 max_uid;
 };
@@ -103,6 +110,9 @@
 					      CamelException *ex);
 gboolean     camel_imap_message_cache_delete (const char *path, 
 					      CamelException *ex);
+GPtrArray *  camel_imap_message_cache_filter_cached(CamelImapMessageCache *,
+                                              GPtrArray *uids,
+                                              CamelException *ex);
 
 /* Standard Camel function */
 CamelType camel_imap_message_cache_get_type (void);