Revamp imapx_job_refresh_info_start() to make use of modseq and uidnext

Also add the uidnext and modseq fields to CamelIMAPXSummary (from commit
86513477) although they're not going to be _stored_ yet (or potentially
ever in the 2.30 branch). But it's easier to keep the code the same.

This fixes the fact that for every STATUS request, we are first issuing
SELECT and then NOOP on the folder in question, which is _really_ slow on
some crappy servers like Microsoft Exchange. It was also ensuring that
_every_ STATUS request we were sending was invalid because it was for the
currently-selected folder (which is forbidden by RFC3501 §6.3.10.

(cherry-picked from commit b0214462be6e21c2a312d8fe3e31ffdd2825d153)

2 files changed, 104 insertions, 37 deletions

diff --git a/camel/providers/imapx/camel-imapx-server.c b/camel/providers/imapx/camel-imapx-server.c
index 01449fc94..e6ef4253e 100644
--- a/camel/providers/imapx/camel-imapx-server.c
+++ b/camel/providers/imapx/camel-imapx-server.c
@@ -3324,12 +3324,18 @@ imapx_job_scan_changes_done(CamelIMAPXServer *is, CamelIMAPXCommand *ic)
 		CamelMessageInfo *s_minfo = NULL;
 		CamelIMAPXMessageInfo *info;
 		CamelFolderSummary *s = job->folder->summary;
+		CamelIMAPXFolder *ifolder = (CamelIMAPXFolder *)job->folder;
 		GSList *removed = NULL, *l;
 		gboolean fetch_new = FALSE;
 		gint i;
 		guint j = 0;
 		GPtrArray *uids;
 
+		/* Actually we wanted to do this after the SELECT but before the
+		   FETCH command was issued. But this should suffice. */
+		((CamelIMAPXSummary *)s)->uidnext = ifolder->uidnext_on_server;
+		((CamelIMAPXSummary *)s)->modseq = ifolder->modseq_on_server;
+
 		/* Here we do the typical sort/iterate/merge loop.
 		   If the server flags dont match what we had, we modify our
 		   flags to pick up what the server now has - but we merge
@@ -3550,61 +3556,120 @@ imapx_job_refresh_info_start (CamelIMAPXServer *is, CamelIMAPXJob *job)
 {
 	guint32 total;
 	CamelIMAPXFolder *ifolder = (CamelIMAPXFolder *) job->folder;
+	CamelIMAPXSummary *isum = (CamelIMAPXSummary *)job->folder->summary;
 	CamelFolder *folder = job->folder;
 	CamelException *ex = job->ex;
+	gboolean need_rescan = FALSE;
+	gboolean is_selected = FALSE;
 
-	total = camel_folder_summary_count (folder->summary);
-	/* Check if there are any new messages. The old imap doc says one needs to reselect in case of inbox to fetch
-	   new messages. Need to check if its still true. Just use noop now */
-	if (ifolder->exists_on_server == total) {
-		camel_imapx_server_noop (is, folder, ex);
-
-		if (camel_exception_is_set (ex))
-			goto done;
-	}
-
-	/* Fetch the new messages */
-	if (ifolder->exists_on_server > total)
-	{
-		imapx_server_fetch_new_messages (is, folder, FALSE, job->ex);
-		if (camel_exception_is_set (job->ex))
-			goto done;
-	}
-
-	/* Sync changes before fetching status, else unread count will not match. need to think about better ways for this */
+	/* Sync changes first, else unread count will not
+	   match. Need to think about better ways for this */
 	imapx_server_sync_changes (is, folder, job->pri, ex);
 	if (camel_exception_is_set (job->ex))
 		goto done;
 
-	/* Check if a rescan is needed */
+#if 0 /* There are issues with this still; continue with the buggy behaviour
+	 where we issue STATUS on the current folder, for now...*/
+	if (is->select && !strcmp(folder->full_name, is->select))
+		is_selected = TRUE;
+#endif
 	total = camel_folder_summary_count (folder->summary);
-	if (ifolder->exists_on_server == total) {
-		guint32 unread;
+
+	/* We don't have valid unread count or modseq for currently-selected server
+	   (unless we want to re-SELECT it). We fake unread count when fetching
+	   message flags, but don't depend on modseq for the selected folder */
+	if (total != ifolder->exists_on_server ||
+	    isum->uidnext != ifolder->uidnext_on_server ||
+	    folder->summary->unread_count != ifolder->unread_on_server ||
+	    (!is_selected && isum->modseq != ifolder->modseq_on_server))
+		need_rescan = TRUE;
+
+	/* This is probably the first check of this folder after startup;
+	   use STATUS to check whether the cached summary is valid, rather
+	   than blindly updating. Only for servers which support CONDSTORE
+	   though. */
+	if ((isum->modseq && !ifolder->modseq_on_server))
+		need_rescan = FALSE;
+
+	/* If we don't think there's anything to do, poke it to check */
+	if (!need_rescan) {
 		CamelIMAPXCommand *ic;
+		
+		if (is_selected) {
+			/* We may not issue STATUS on the current folder. Use SELECT or NOOP instead. */
+			if (0 /* server needs SELECT not just NOOP*/) {
+				if (imapx_idle_supported(is) && imapx_in_idle(is))
+					imapx_stop_idle(is, job->ex);
+				if (camel_exception_is_set(job->ex))
+					goto done;
+				/* This doesn't work -- this is an immediate command, not queued */
+				imapx_select(is, folder, TRUE, job->ex);
+				if (camel_exception_is_set(job->ex))
+					goto done;
+			} else {
+				/* Or maybe just NOOP, unless we're in IDLE in which case do nothing */
+				if (!imapx_idle_supported(is) || !imapx_in_idle(is)) {
+					camel_imapx_server_noop(is, folder, job->ex);
+					if (camel_exception_is_set(job->ex))
+						goto done;
+				}
+			}
+		} else {
+			if (is->cinfo->capa & IMAPX_CAPABILITY_CONDSTORE)
+				ic = camel_imapx_command_new (is, "STATUS", NULL, "STATUS %f (MESSAGES UNSEEN UIDNEXT HIGHESTMODSEQ)", folder);
+			else
+				ic = camel_imapx_command_new (is, "STATUS", NULL, "STATUS %f (MESSAGES UNSEEN UIDNEXT)", folder);
+		
+			ic->job = job;
+			ic->pri = job->pri;
 
-		ic = camel_imapx_command_new (is, "STATUS", folder->full_name, "STATUS %f (MESSAGES UNSEEN)", folder);
-		ic->job = job;
-		ic->pri = job->pri;
-		imapx_command_run_sync (is, ic);
+			imapx_command_run_sync (is, ic);
 
-		if (camel_exception_is_set (ic->ex) || ic->status->result != IMAPX_OK) {
-			if (!camel_exception_is_set (ic->ex))
-				camel_exception_setv(job->ex, 1, "Error refreshing folder: %s", ic->status->text);
-			else
-				camel_exception_xfer (job->ex, ic->ex);
+			if (camel_exception_is_set (ic->ex) || ic->status->result != IMAPX_OK) {
+				if (!camel_exception_is_set (ic->ex))
+					camel_exception_setv(job->ex, 1, "Error refreshing folder: %s", ic->status->text);
+				else
+					camel_exception_xfer (job->ex, ic->ex);
 
+				camel_imapx_command_free (ic);
+				goto done;
+			}
 			camel_imapx_command_free (ic);
-			goto done;
 		}
-		camel_imapx_command_free (ic);
 
-		camel_object_get (folder, NULL, CAMEL_FOLDER_UNREAD, &unread, NULL);
-		if (ifolder->exists_on_server == total && unread == ifolder->unread_on_server)
+		/* Recalulate need_rescan */
+		if (total != ifolder->exists_on_server ||
+		    isum->uidnext != ifolder->uidnext_on_server ||
+		    folder->summary->unread_count != ifolder->unread_on_server ||
+		    (!is_selected && isum->modseq != ifolder->modseq_on_server))
+			need_rescan = TRUE;
+
+	}
+
+	e(printf("folder %s is %sselected, total %u / %u, unread %u / %u, modseq %llu / %llu, uidnext %u / %u: will %srescan\n",
+		 folder->full_name, is_selected?"": "not ", total, ifolder->exists_on_server,
+		 folder->summary->unread_count, ifolder->unread_on_server,
+		 (unsigned long long)isum->modseq, (unsigned long long)ifolder->modseq_on_server,
+		 isum->uidnext, ifolder->uidnext_on_server,
+		 need_rescan?"":"not "));
+
+	/* Fetch new messages first, so that they appear to the user ASAP */
+	if (ifolder->exists_on_server > total ||
+	    ifolder->uidnext_on_server > isum->uidnext)
+	{
+		if (!total)
+			need_rescan = FALSE;
+
+		imapx_server_fetch_new_messages (is, folder, FALSE, job->ex);
+		if (camel_exception_is_set (job->ex))
 			goto done;
 	}
 
-	imapx_job_scan_changes_start (is, job);
-	return;
+	/* Refetch flags for the entire folder */
+	if (need_rescan) {
+		imapx_job_scan_changes_start (is, job);
+		return;
+	}
 
 done:
 	imapx_job_done (is, job);
diff --git a/camel/providers/imapx/camel-imapx-summary.h b/camel/providers/imapx/camel-imapx-summary.h
index f6078b6cf..0dee06faf 100644
--- a/camel/providers/imapx/camel-imapx-summary.h
+++ b/camel/providers/imapx/camel-imapx-summary.h
@@ -59,6 +59,8 @@ struct _CamelIMAPXSummary {
 
 	guint32 version;
 	guint32 validity;
+	guint32 uidnext;
+	guint64 modseq;
 };
 
 struct _CamelIMAPXSummaryClass {