imapc: Fix sending initial FETCH after reconnection SELECTs mailbox

Move sending the FETCH when the SELECT returns tagged OK reply instead of
delaying it until mailbox is next synced. Most importantly this allows
sending the FETCH before any retried commands that are also sent after
SELECT receives tagged reply.

diff --git a/src/lib-storage/index/imapc/imapc-mailbox.c b/src/lib-storage/index/imapc/imapc-mailbox.c
index 254a4eddcc42d99492cadb14493351d873c45ed8..048b037a22ce04cb8a7ff13ad41693a6ddc40c60 100644 (file)
--- a/src/lib-storage/index/imapc/imapc-mailbox.c
+++ b/src/lib-storage/index/imapc/imapc-mailbox.c
@@ -157,35 +157,79 @@ static void imapc_mailbox_idle_notify(struct imapc_mailbox *mbox)
        }
 }
 
-void imapc_mailbox_fetch_state_finish(struct imapc_mailbox *mbox,
-                                     struct mail_index_view *sync_view,
-                                     struct mail_index_transaction *trans)
+static void
+imapc_mailbox_fetch_state_finish(struct imapc_mailbox *mbox)
 {
        uint32_t lseq, uid, msg_count;
 
-       if (mbox->sync_next_lseq == 0)
+       if (mbox->sync_next_lseq == 0) {
+               /* FETCH n:*, not 1:* */
+               i_assert(mbox->state_fetched_success ||
+                        (mbox->box.flags & MAILBOX_FLAG_SAVEONLY) != 0);
                return;
+       }
 
        /* if we haven't seen FETCH reply for some messages at the end of
           mailbox they've been externally expunged. */
-       msg_count = mail_index_view_get_messages_count(sync_view);
+       msg_count = mail_index_view_get_messages_count(mbox->delayed_sync_view);
        for (lseq = mbox->sync_next_lseq; lseq <= msg_count; lseq++) {
-               mail_index_lookup_uid(sync_view, lseq, &uid);
+               mail_index_lookup_uid(mbox->delayed_sync_view, lseq, &uid);
                if (uid >= mbox->sync_uid_next) {
                        /* another process already added new messages to index
                           that our IMAP connection hasn't seen yet */
                        break;
                }
-               mail_index_expunge(trans, lseq);
+               mail_index_expunge(mbox->delayed_sync_trans, lseq);
        }
 
        mbox->sync_next_lseq = 0;
        mbox->sync_next_rseq = 0;
+       mbox->state_fetched_success = TRUE;
+}
+
+static void
+imapc_mailbox_fetch_state_callback(const struct imapc_command_reply *reply,
+                                  void *context)
+{
+       struct imapc_mailbox *mbox = context;
+
+       mbox->state_fetching_uid1 = FALSE;
+       imapc_client_stop(mbox->storage->client->client);
+
+       switch (reply->state) {
+       case IMAPC_COMMAND_STATE_OK:
+               imapc_mailbox_fetch_state_finish(mbox);
+               break;
+       case IMAPC_COMMAND_STATE_NO:
+               imapc_copy_error_from_reply(mbox->storage, MAIL_ERROR_PARAMS, reply);
+               break;
+       case IMAPC_COMMAND_STATE_DISCONNECTED:
+               mail_storage_set_internal_error(mbox->box.storage);
+
+               break;
+       default:
+               mail_storage_set_critical(mbox->box.storage,
+                       "imapc: state FETCH failed: %s", reply->text_full);
+               break;
+       }
 }
 
-void imapc_mailbox_fetch_state(struct imapc_mailbox *mbox, string_t *str,
-                              uint32_t first_uid)
+static void
+imapc_mailbox_fetch_state(struct imapc_mailbox *mbox, uint32_t first_uid)
 {
+       struct imapc_command *cmd;
+
+       if (mbox->exists_count == 0) {
+               /* empty mailbox - no point in fetching anything */
+               mbox->state_fetched_success = TRUE;
+               return;
+       }
+       if (mbox->state_fetching_uid1) {
+               /* retrying after reconnection - don't send duplicate */
+               return;
+       }
+
+       string_t *str = t_str_new(64);
        str_printfa(str, "UID FETCH %u:* (FLAGS", first_uid);
        if (imapc_mailbox_has_modseqs(mbox)) {
                str_append(str, " MODSEQ");
@@ -207,6 +251,19 @@ void imapc_mailbox_fetch_state(struct imapc_mailbox *mbox, string_t *str,
 
        }
        str_append_c(str, ')');
+
+       cmd = imapc_client_mailbox_cmd(mbox->client_box,
+               imapc_mailbox_fetch_state_callback, mbox);
+       if (first_uid == 1) {
+               mbox->sync_next_lseq = 1;
+               mbox->sync_next_rseq = 1;
+               mbox->state_fetched_success = FALSE;
+               /* only the FETCH 1:* is retriable - others will be retried
+                  by the 1:* after the reconnection */
+               imapc_command_set_flags(cmd, IMAPC_COMMAND_FLAG_RETRIABLE);
+       }
+       mbox->state_fetching_uid1 = first_uid == 1;
+       imapc_command_send(cmd, str_c(str));
 }
 
 static void
@@ -226,19 +283,21 @@ imapc_untagged_exists(const struct imapc_untagged_reply *reply,
                return;
        }
 
+       mbox->exists_count = exists_count;
+       mbox->exists_received = TRUE;
+
        view = mbox->delayed_sync_view;
        if (view == NULL)
                view = imapc_mailbox_get_sync_view(mbox);
 
        if (mbox->selecting) {
                /* We don't know the latest flags, refresh them. */
-               mbox->sync_fetch_first_uid = 1;
+               imapc_mailbox_fetch_state(mbox, 1);
        } else if (mbox->sync_fetch_first_uid != 1) {
                hdr = mail_index_get_header(view);
                mbox->sync_fetch_first_uid = hdr->next_uid;
+               imapc_mailbox_fetch_state(mbox, hdr->next_uid);
        }
-       mbox->exists_count = exists_count;
-       mbox->exists_received = TRUE;
        imapc_mailbox_idle_notify(mbox);
 }
 
@@ -344,7 +403,7 @@ imapc_mailbox_msgmap_update(struct imapc_mailbox *mbox,
                imapc_msgmap_append(msgmap, rseq, uid);
                if (uid < mbox->min_append_uid) {
                        /* message is already added to index */
-               } else if (mbox->syncing) {
+               } else {
                        mail_index_append(mbox->delayed_sync_trans,
                                          uid, lseq_r);
                        mbox->min_append_uid = uid + 1;
@@ -464,8 +523,6 @@ static void imapc_untagged_fetch(const struct imapc_untagged_reply *reply,
        if (rseq == mbox->sync_next_rseq) {
                /* we're doing the initial full sync of mails. expunge any
                   mails that no longer exist. */
-               i_assert(mbox->syncing);
-
                while (mbox->sync_next_lseq < lseq) {
                        mail_index_expunge(mbox->delayed_sync_trans,
                                           mbox->sync_next_lseq);

diff --git a/src/lib-storage/index/imapc/imapc-storage.c b/src/lib-storage/index/imapc/imapc-storage.c
index c16e6aa0bd33bcdbb2d787fd53f465a9a3b48a6a..02883d04ca9c3bcedf5c711e5af4f6dd4eb8eace 100644 (file)
--- a/src/lib-storage/index/imapc/imapc-storage.c
+++ b/src/lib-storage/index/imapc/imapc-storage.c
@@ -137,7 +137,8 @@ void imapc_mailbox_run_nofetch(struct imapc_mailbox *mbox)
 {
        do {
                imapc_client_run(mbox->storage->client->client);
-       } while (mbox->storage->reopen_count > 0);
+       } while (mbox->storage->reopen_count > 0 ||
+                mbox->state_fetching_uid1);
 }
 
 void imapc_simple_callback(const struct imapc_command_reply *reply,
@@ -627,9 +628,6 @@ static void imapc_mailbox_reopen(void *context)
                                    imapc_mailbox_get_remote_name(mbox));
        }
        mbox->storage->reopen_count++;
-
-       if (mbox->syncing)
-               imapc_sync_mailbox_reopened(mbox);
 }
 
 static void
@@ -728,8 +726,10 @@ int imapc_mailbox_select(struct imapc_mailbox *mbox)
                        imapc_mailbox_get_remote_name(mbox));
        }
 
-       while (ctx.ret == -2)
+       while (ctx.ret == -2 || mbox->state_fetching_uid1)
                imapc_mailbox_run(mbox);
+       if (!mbox->state_fetched_success)
+               ctx.ret = -1;
        return ctx.ret;
 }
 

diff --git a/src/lib-storage/index/imapc/imapc-storage.h b/src/lib-storage/index/imapc/imapc-storage.h
index 6e6ac315d197cd7aee8ae6391a0534fa72459b57..85c4ee4a6d3af1c6f8d29eb45aa8a07130a2f199 100644 (file)
--- a/src/lib-storage/index/imapc/imapc-storage.h
+++ b/src/lib-storage/index/imapc/imapc-storage.h
@@ -143,6 +143,8 @@ struct imapc_mailbox {
        bool initial_sync_done:1;
        bool selected:1;
        bool exists_received:1;
+       bool state_fetching_uid1:1;
+       bool state_fetched_success:1;
 };
 
 struct imapc_simple_context {
@@ -196,12 +198,6 @@ void imapc_mailbox_set_corrupted(struct imapc_mailbox *mbox,
                                 const char *reason, ...) ATTR_FORMAT(2, 3);
 const char *imapc_mailbox_get_remote_name(struct imapc_mailbox *mbox);
 
-void imapc_mailbox_fetch_state(struct imapc_mailbox *mbox, string_t *cmd,
-                              uint32_t first_uid);
-void imapc_mailbox_fetch_state_finish(struct imapc_mailbox *mbox,
-                                     struct mail_index_view *sync_view,
-                                     struct mail_index_transaction *trans);
-
 void imapc_storage_client_register_untagged(struct imapc_storage_client *client,
                                            const char *name,
                                            imapc_storage_callback_t *callback);

diff --git a/src/lib-storage/index/imapc/imapc-sync.c b/src/lib-storage/index/imapc/imapc-sync.c
index 9e06be54a29a960482c1b69d367874f8431bef72..ef3533185b0fbcac4f16df7757a1840019925bca 100644 (file)
--- a/src/lib-storage/index/imapc/imapc-sync.c
+++ b/src/lib-storage/index/imapc/imapc-sync.c
@@ -352,16 +352,12 @@ imapc_initial_sync_check(struct imapc_sync_context *ctx, bool nooped)
 }
 
 static void
-imapc_sync_send_commands(struct imapc_sync_context *ctx, uint32_t first_uid)
+imapc_sync_send_commands(struct imapc_sync_context *ctx)
 {
-       string_t *cmd = t_str_new(64);
-
        if (ctx->mbox->exists_count == 0) {
                /* empty mailbox - no point in fetching anything */
                return;
        }
-       imapc_mailbox_fetch_state(ctx->mbox, cmd, first_uid);
-       imapc_sync_cmd(ctx, str_c(cmd));
 
        if (IMAPC_BOX_HAS_FEATURE(ctx->mbox, IMAPC_FEATURE_GMAIL_MIGRATION) &&
            ctx->mbox->storage->set->pop3_deleted_flag[0] != '\0') {
@@ -407,20 +403,8 @@ static void imapc_sync_index(struct imapc_sync_context *ctx)
        imapc_sync_finish_store(ctx);
        pool_unref(&ctx->pool);
 
-       if (!mbox->initial_sync_done) {
-               /* with initial syncing we're fetching all messages' flags and
-                  expunge mails from local index that no longer exist on
-                  remote server */
-               i_assert(mbox->sync_fetch_first_uid == 1);
-               mbox->sync_next_lseq = 1;
-               mbox->sync_next_rseq = 1;
-       }
-       if (mbox->sync_fetch_first_uid != 0) {
-               /* we'll resync existing messages' flags and add new messages.
-                  adding new messages requires sync locking to avoid
-                  duplicates. */
-               imapc_sync_send_commands(ctx, mbox->sync_fetch_first_uid);
-       }
+       if (!mbox->initial_sync_done)
+               imapc_sync_send_commands(ctx);
 
        imapc_sync_expunge_finish(ctx);
        while (ctx->sync_command_count > 0)
@@ -431,10 +415,6 @@ static void imapc_sync_index(struct imapc_sync_context *ctx)
        imapc_sync_uid_next(ctx);
        imapc_sync_highestmodseq(ctx);
 
-       if (!ctx->failed) {
-               imapc_mailbox_fetch_state_finish(ctx->mbox, ctx->sync_view,
-                                                ctx->trans);
-       }
        if (mbox->box.v.sync_notify != NULL)
                mbox->box.v.sync_notify(&mbox->box, 0, 0);
 
@@ -448,26 +428,6 @@ static void imapc_sync_index(struct imapc_sync_context *ctx)
        }
 }
 
-void imapc_sync_mailbox_reopened(struct imapc_mailbox *mbox)
-{
-       struct imapc_sync_context *ctx = mbox->sync_ctx;
-
-       i_assert(mbox->syncing);
-
-       if (!mbox->initial_sync_done) {
-               /* the same sync commands are automatically already retried by
-                  lib-imap-client. don't duplicate them here. */
-               return;
-       }
-
-       /* we got disconnected while syncing. need to
-          re-fetch everything */
-       mbox->sync_next_lseq = 1;
-       mbox->sync_next_rseq = 1;
-
-       imapc_sync_send_commands(ctx, 1);
-}
-
 static int
 imapc_sync_begin(struct imapc_mailbox *mbox,
                 struct imapc_sync_context **ctx_r, bool force)
@@ -612,6 +572,12 @@ imapc_mailbox_sync_init(struct mailbox *box, enum mailbox_sync_flags flags)
 
        imapc_noop_if_needed(mbox, flags);
 
+       if (imapc_storage_client_handle_auth_failure(mbox->storage->client))
+               ret = -1;
+       else if (!mbox->state_fetched_success && !mbox->state_fetching_uid1) {
+               /* initial FETCH failed already */
+               ret = -1;
+       }
        if (imapc_mailbox_commit_delayed_trans(mbox, &changes) < 0)
                ret = -1;
        if ((changes || mbox->sync_fetch_first_uid != 0 ||

diff --git a/src/lib-storage/index/imapc/imapc-sync.h b/src/lib-storage/index/imapc/imapc-sync.h
index bd34d00226349d6ecaae71a97361c0b2b4e23139..ab75fd35c5fdc6856d572c0ce0b28492f0cb0882 100644 (file)
--- a/src/lib-storage/index/imapc/imapc-sync.h
+++ b/src/lib-storage/index/imapc/imapc-sync.h
@@ -35,6 +35,5 @@ struct mailbox_sync_context *
 imapc_mailbox_sync_init(struct mailbox *box, enum mailbox_sync_flags flags);
 int imapc_mailbox_sync_deinit(struct mailbox_sync_context *ctx,
                              struct mailbox_sync_status *status_r);
-void imapc_sync_mailbox_reopened(struct imapc_mailbox *mbox);
 
 #endif