More concurrent IMAP modification handling from Brendan Cully.

4 files changed, 143 insertions, 73 deletions

diff --git a/imap/command.c b/imap/command.c
index 2f90dc48..55ab7d06 100644
--- a/imap/command.c
+++ b/imap/command.c
@@ -32,12 +32,12 @@
 #define IMAP_CMD_BUFSIZE 512
 
 /* forward declarations */
-static void cmd_finish (IMAP_DATA* idata);
 static void cmd_handle_fatal (IMAP_DATA* idata);
 static int cmd_handle_untagged (IMAP_DATA* idata);
 static void cmd_make_sequence (IMAP_DATA* idata);
 static void cmd_parse_capabilities (IMAP_DATA* idata, char* s);
-static void cmd_parse_expunge (IMAP_DATA* idata, char* s);
+static void cmd_parse_expunge (IMAP_DATA* idata, const char* s);
+static void cmd_parse_fetch (IMAP_DATA* idata, char* s);
 static void cmd_parse_myrights (IMAP_DATA* idata, char* s);
 
 static char *Capabilities[] = {
@@ -148,7 +148,7 @@ int imap_cmd_step (IMAP_DATA* idata)
   /* tagged completion code */
   if (!mutt_strncmp (cmd->buf, cmd->seq, SEQLEN))
   {
-    cmd_finish (idata);
+    imap_cmd_finish (idata);
     return imap_code (cmd->buf) ? IMAP_CMD_OK : IMAP_CMD_NO;
   }
 
@@ -229,16 +229,16 @@ int imap_cmd_running (IMAP_DATA* idata)
   return 0;
 }
 
-/* cmd_finish: When the caller has finished reading command responses,
- *   it must call this routine to perform cleanup (eg fetch new mail if
- *   detected, do expunge). Called automatically by imap_cmd_step */
-static void cmd_finish (IMAP_DATA* idata)
+/* imap_cmd_finish: Attempts to perform cleanup (eg fetch new mail if
+ *   detected, do expunge). Called automatically by imap_cmd_step, but
+ *   may be called at any time. Called by imap_check_mailbox just before
+ *   the index is refreshed, for instance. */
+void imap_cmd_finish (IMAP_DATA* idata)
 {
   if (!(idata->state == IMAP_SELECTED) || idata->ctx->closing)
     return;
   
-  if ((idata->reopen & IMAP_REOPEN_ALLOW) &&
-      (idata->reopen & (IMAP_EXPUNGE_PENDING|IMAP_NEWMAIL_PENDING)))
+  if (idata->reopen & IMAP_REOPEN_ALLOW)
   {
     int count = idata->newMailCount;
 
@@ -247,16 +247,16 @@ static void cmd_finish (IMAP_DATA* idata)
 	&& count > idata->ctx->msgcount)
     {
       /* read new mail messages */
-      dprint (2, (debugfile, "cmd_finish: Fetching new mail\n"));
+      dprint (2, (debugfile, "imap_cmd_finish: Fetching new mail\n"));
       /* check_status: curs_main uses imap_check_mailbox to detect
        *   whether the index needs updating */
       idata->check_status = IMAP_NEWMAIL_PENDING;
       idata->reopen &= ~IMAP_NEWMAIL_PENDING;
       count = imap_read_headers (idata, idata->ctx->msgcount, count-1)+1;
     }
-    else
+    else if (idata->reopen & IMAP_EXPUNGE_PENDING)
     {
-      dprint (2, (debugfile, "cmd_finish: Expunging mailbox\n"));
+      dprint (2, (debugfile, "imap_cmd_finish: Expunging mailbox\n"));
       imap_expunge_mailbox (idata);
       /* Detect whether we've gotten unexpected EXPUNGE messages */
       if (idata->reopen & IMAP_EXPUNGE_PENDING &&
@@ -334,9 +334,11 @@ static int cmd_handle_untagged (IMAP_DATA* idata)
 	idata->newMailCount = count;
       }
     }
+    /* pn vs. s: need initial seqno */
     else if (ascii_strncasecmp ("EXPUNGE", s, 7) == 0)
-      /* pn vs. s: need initial seqno */
       cmd_parse_expunge (idata, pn);
+    else if (ascii_strncasecmp ("FETCH", s, 5) == 0)
+      cmd_parse_fetch (idata, pn);
   }
   else if (ascii_strncasecmp ("CAPABILITY", s, 10) == 0)
     cmd_parse_capabilities (idata, s);
@@ -407,7 +409,7 @@ static void cmd_parse_capabilities (IMAP_DATA* idata, char* s)
 
 /* cmd_parse_expunge: mark headers with new sequence ID and mark idata to
  *   be reopened at our earliest convenience */
-static void cmd_parse_expunge (IMAP_DATA* idata, char* s)
+static void cmd_parse_expunge (IMAP_DATA* idata, const char* s)
 {
   int expno, cur;
   HEADER* h;
@@ -433,6 +435,65 @@ static void cmd_parse_expunge (IMAP_DATA* idata, char* s)
   idata->reopen |= IMAP_EXPUNGE_PENDING;
 }
 
+/* cmd_parse_fetch: Load fetch response into IMAP_DATA. Currently only
+ *   handles unanticipated FETCH responses, and only FLAGS data. We get
+ *   these if another client has changed flags for a mailbox we've selected.
+ *   Of course, a lot of code here duplicates code in message.c. */
+static void cmd_parse_fetch (IMAP_DATA* idata, char* s)
+{
+  int msgno, cur;
+  HEADER* h = NULL;
+
+  dprint (2, (debugfile, "Handling FETCH\n"));
+
+  msgno = atoi (s);
+  
+  /* see cmd_parse_expunge */
+  for (cur = 0; cur < idata->ctx->msgcount; cur++)
+  {
+    h = idata->ctx->hdrs[cur];
+    
+    if (h->active && h->index+1 == msgno)
+    {
+      dprint (2, (debugfile, "Message UID %d updated\n", HEADER_DATA(h)->uid));
+      break;
+    }
+
+    h = NULL;
+  }
+  
+  if (!h)
+  {
+    dprint (1, (debugfile, "FETCH response ignored for this message\n"));
+    return;
+  }
+  
+  /* skip FETCH */
+  s = imap_next_word (s);
+  s = imap_next_word (s);
+
+  if (*s != '(')
+  {
+    dprint (1, (debugfile, "Malformed FETCH response"));
+    return;
+  }
+  s++;
+
+  if (ascii_strncasecmp ("FLAGS", s, 5) != 0)
+  {
+    dprint (2, (debugfile, "Only handle FLAGS updates\n"));
+    return;
+  }
+
+  /* If server flags could conflict with mutt's flags, reopen the mailbox. */
+  if (h->changed)
+    idata->reopen |= IMAP_EXPUNGE_PENDING;
+  else {
+    imap_set_flags (idata, h, s);
+    idata->check_status = IMAP_FLAGS_PENDING;
+  }
+}
+
 /* cmd_parse_myrights: set rights bits according to MYRIGHTS response */
 static void cmd_parse_myrights (IMAP_DATA* idata, char* s)
 {
diff --git a/imap/imap.c b/imap/imap.c
index a5a616e2..3a8247e8 100644
--- a/imap/imap.c
+++ b/imap/imap.c
@@ -652,7 +652,7 @@ int imap_open_mailbox (CONTEXT* ctx)
   ctx->msgcount = 0;
   count = imap_read_headers (idata, 0, count - 1) + 1;
 
-  dprint (1, (debugfile, "imap_open_mailbox(): msgcount is %d\n", ctx->msgcount));
+  dprint (2, (debugfile, "imap_open_mailbox: msgcount is %d\n", ctx->msgcount));
   FREE (&mx.mbox);
   return 0;
 
@@ -1076,6 +1076,7 @@ int imap_check_mailbox (CONTEXT *ctx, int *index_hint)
 
   IMAP_DATA* idata;
   time_t now;
+  int result = 0;
 
   idata = (IMAP_DATA*) ctx->data;
 
@@ -1087,19 +1088,21 @@ int imap_check_mailbox (CONTEXT *ctx, int *index_hint)
     if (imap_exec (idata, "NOOP", 0) != 0)
       return -1;
   }
-    
+
+  /* We call this even when we haven't run NOOP in case we have pending
+   * changes to process, since we can reopen here. */
+  imap_cmd_finish (idata);
+
   if (idata->check_status & IMAP_NEWMAIL_PENDING)
-  {
-    idata->check_status &= ~IMAP_NEWMAIL_PENDING;
-    return M_NEW_MAIL;
-  }
-  if (idata->check_status & IMAP_EXPUNGE_PENDING)
-  {
-    idata->check_status &= ~IMAP_EXPUNGE_PENDING;
-    return M_REOPENED;
-  }
+    result = M_NEW_MAIL;
+  else if (idata->check_status & IMAP_EXPUNGE_PENDING)
+    result = M_REOPENED;
+  else if (idata->check_status & IMAP_FLAGS_PENDING)
+    result = M_FLAGS;
 
-  return 0;
+  idata->check_status = 0;
+
+  return result;
 }
 
 /* returns count of recent messages if new = 1, else count of total messages.
diff --git a/imap/imap_private.h b/imap/imap_private.h
index 3dbcdf6f..7dbcafac 100644
--- a/imap/imap_private.h
+++ b/imap/imap_private.h
@@ -50,9 +50,10 @@
 #define SEQLEN 5
 
 #define IMAP_REOPEN_ALLOW     (1<<0)
-#define IMAP_EXPUNGE_PENDING  (1<<1)
-#define IMAP_NEWMAIL_PENDING  (1<<2)
-#define IMAP_EXPUNGE_EXPECTED (1<<3)
+#define IMAP_EXPUNGE_EXPECTED (1<<1)
+#define IMAP_EXPUNGE_PENDING  (1<<2)
+#define IMAP_NEWMAIL_PENDING  (1<<3)
+#define IMAP_FLAGS_PENDING    (1<<4)
 
 /* imap_exec flags (see imap_exec) */
 #define IMAP_CMD_FAIL_OK (1<<0)
@@ -205,6 +206,7 @@ int imap_authenticate (IMAP_DATA* idata);
 /* command.c */
 int imap_cmd_start (IMAP_DATA* idata, const char* cmd);
 int imap_cmd_step (IMAP_DATA* idata);
+void imap_cmd_finish (IMAP_DATA* idata);
 int imap_code (const char* s);
 int imap_exec (IMAP_DATA* idata, const char* cmd, int flags);
 
@@ -212,6 +214,7 @@ int imap_exec (IMAP_DATA* idata, const char* cmd, int flags);
 void imap_add_keywords (char* s, HEADER* keywords, LIST* mailbox_flags, size_t slen);
 void imap_free_header_data (void** data);
 int imap_read_headers (IMAP_DATA* idata, int msgbegin, int msgend);
+char* imap_set_flags (IMAP_DATA* idata, HEADER* h, char* s);
 
 /* util.c */
 int imap_continue (const char* msg, const char* resp);
diff --git a/imap/message.c b/imap/message.c
index 1110bdb7..f6804637 100644
--- a/imap/message.c
+++ b/imap/message.c
@@ -58,7 +58,6 @@ int imap_read_headers (IMAP_DATA* idata, int msgbegin, int msgend)
 
   ctx = idata->ctx;
 
-  /* define search string */
   if (mutt_bit_isset (idata->capabilities,IMAP4REV1))
   {
     snprintf (hdrreq, sizeof (hdrreq), "BODY.PEEK[HEADER.FIELDS (%s)]", 
@@ -302,48 +301,8 @@ int imap_fetch_message (MESSAGE *msg, CONTEXT *ctx, int msgno)
 	else if ((ascii_strncasecmp ("FLAGS", pc, 5) == 0) &&
 		 !ctx->hdrs[msgno]->changed)
 	{
-	  IMAP_HEADER newh;
-	  unsigned char readonly;
-
-	  h = ctx->hdrs[msgno];
-
-	  memset (&newh, 0, sizeof (newh));
-	  newh.data = safe_calloc (1, sizeof (IMAP_HEADER_DATA));
-
-	  dprint (2, (debugfile, "imap_fetch_message: parsing FLAGS\n"));
-	  if ((pc = msg_parse_flags (&newh, pc)) == NULL)
+	  if ((pc = imap_set_flags (idata, ctx->hdrs[msgno], pc)) == NULL)
 	    goto bail;
-	      
-	  /* this is less efficient than the code which used to be here,
-	   * but (1) this is only invoked when fetching messages, and (2)
-	   * this way, we can make sure that side effects of flag changes
-	   * are taken account of the proper way.
-	   */
-
-	  /* YAUH (yet another ugly hack): temporarily set context to
-	   * read-write even if it's read-only, so *server* updates of
-	   * flags can be processed by mutt_set_flag. ctx->changed must
-	   * be restored afterwards */
-	  readonly = ctx->readonly;
-	  ctx->readonly = 0;
-	    
-	  mutt_set_flag (ctx, h, M_NEW, 
-			 !(newh.read || newh.old || h->read || h->old));
-	  mutt_set_flag (ctx, h, M_OLD, newh.old);
-	  mutt_set_flag (ctx, h, M_READ, h->read || newh.read);
-	  mutt_set_flag (ctx, h, M_DELETE, h->deleted || newh.deleted);
-	  mutt_set_flag (ctx, h, M_FLAG, h->flagged || newh.flagged);
-	  mutt_set_flag (ctx, h, M_REPLIED, h->replied || newh.replied);
-
-	  /* this message is now definitively *not* changed (mutt_set_flag
-	   * marks things changed as a side-effect) */
-	  h->changed = 0;
-	  ctx->changed &= ~readonly;
-	  ctx->readonly = readonly;
-
-	  mutt_free_list (&(HEADER_DATA(h)->keywords));
-	  HEADER_DATA(h)->keywords = newh.data->keywords;
-	  FREE(&newh.data);
 	}
       }
     }
@@ -694,6 +653,51 @@ void imap_free_header_data (void** data)
   safe_free (data);
 }
 
+/* imap_set_flags: fill out the message header according to the flags from
+ *   the server. Expects a flags line of the form "FLAGS (flag flag ...)" */
+char* imap_set_flags (IMAP_DATA* idata, HEADER* h, char* s)
+{
+  CONTEXT* ctx = idata->ctx;
+  IMAP_HEADER newh;
+  unsigned char readonly;
+
+  memset (&newh, 0, sizeof (newh));
+  newh.data = safe_calloc (1, sizeof (IMAP_HEADER_DATA));
+
+  dprint (2, (debugfile, "imap_fetch_message: parsing FLAGS\n"));
+  if ((s = msg_parse_flags (&newh, s)) == NULL)
+  {
+    FREE (&newh.data);
+    return NULL;
+  }
+  
+  /* YAUH (yet another ugly hack): temporarily set context to
+   * read-write even if it's read-only, so *server* updates of
+   * flags can be processed by mutt_set_flag. ctx->changed must
+   * be restored afterwards */
+  readonly = ctx->readonly;
+  ctx->readonly = 0;
+	    
+  mutt_set_flag (ctx, h, M_NEW, !(newh.read || newh.old));
+  mutt_set_flag (ctx, h, M_OLD, newh.old);
+  mutt_set_flag (ctx, h, M_READ, newh.read);
+  mutt_set_flag (ctx, h, M_DELETE, newh.deleted);
+  mutt_set_flag (ctx, h, M_FLAG, newh.flagged);
+  mutt_set_flag (ctx, h, M_REPLIED, newh.replied);
+
+  /* this message is now definitively *not* changed (mutt_set_flag
+   * marks things changed as a side-effect) */
+  h->changed = 0;
+  ctx->changed &= ~readonly;
+  ctx->readonly = readonly;
+
+  mutt_free_list (&(HEADER_DATA(h)->keywords));
+  HEADER_DATA(h)->keywords = newh.data->keywords;
+  FREE(&newh.data);
+
+  return s;
+}
+
 /* msg_fetch_header: import IMAP FETCH response into an IMAP_HEADER.
  *   Expects string beginning with * n FETCH.
  *   Returns:
@@ -846,8 +850,7 @@ static int msg_parse_fetch (IMAP_HEADER *h, char *s)
   return 0;
 }
 
-/* msg_parse_flags: fill out the message header according to the flags from the
- *   server. Expects a flags line of the form "FLAGS (flag flag ...)" */
+/* msg_parse_flags: read a FLAGS token into an IMAP_HEADER */
 static char* msg_parse_flags (IMAP_HEADER* h, char* s)
 {
   int recent = 0;