lib-lda: Fix delivery logging when Sieve performs multiple actions

Previous code assumed that it would work like:
 - save/copy
 - transaction commit
 - mail_deliver_ctx_get_log_var_expand_table()
 - repeat for transaction 2

While it really works:
 - transaction 1: save/copy
 - transaction 2: save/copy
 - transaction 1: commit
 - mail_deliver_ctx_get_log_var_expand_table()
 - transaction 2: commit
 - mail_deliver_ctx_get_log_var_expand_table()

So the cache needs to be stored per transaction. This code still wouldn't
work correctly if Sieve saved mails multiple times within the same
transaction, but that doesn't happen (at least currently).

diff --git a/src/lib-lda/mail-deliver.c b/src/lib-lda/mail-deliver.c
index 29785b5579dc89781ec416f87534ada383578d9f..a50a553959e2990cf55946c824436342ccfb1cbb 100644 (file)
--- a/src/lib-lda/mail-deliver.c
+++ b/src/lib-lda/mail-deliver.c
@@ -43,6 +43,13 @@ struct mail_deliver_cache {
 
 struct mail_deliver_mailbox {
        union mailbox_module_context module_ctx;
+       bool delivery_box;
+};
+
+struct mail_deliver_transaction {
+       union mailbox_transaction_module_context module_ctx;
+
+       struct mail_deliver_cache cache;
 };
 
 static const char *lda_log_wanted_headers[] = {
@@ -226,6 +233,11 @@ int mail_deliver_save_open(struct mail_deliver_save_open_context *ctx,
        }
 
        *box_r = box = mailbox_alloc(ns->list, name, flags);
+       /* flag that this mailbox is used for delivering the mail. */
+       struct mail_deliver_mailbox *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box);
+       i_assert(mbox != NULL);
+       mbox->delivery_box = TRUE;
+
        if (mailbox_open(box) == 0)
                return 0;
 
@@ -517,13 +529,15 @@ static int mail_deliver_save_finish(struct mail_save_context *ctx)
        struct mail_deliver_mailbox *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box);
        struct mail_deliver_user *muser =
                MAIL_DELIVER_USER_CONTEXT(box->storage->user);
+       struct mail_deliver_transaction *dt =
+               MAIL_DELIVER_STORAGE_CONTEXT(ctx->transaction);
 
        if (mbox->module_ctx.super.save_finish(ctx) < 0)
                return -1;
 
        /* initialize most of the fields from dest_mail */
-       mail_deliver_log_update_cache(muser->deliver_ctx->cache,
-                                     muser->deliver_ctx->pool, ctx->dest_mail);
+       mail_deliver_log_update_cache(&dt->cache, muser->deliver_ctx->pool,
+                                     ctx->dest_mail);
        return 0;
 }
 
@@ -533,13 +547,15 @@ static int mail_deliver_copy(struct mail_save_context *ctx, struct mail *mail)
        struct mail_deliver_mailbox *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box);
        struct mail_deliver_user *muser =
                MAIL_DELIVER_USER_CONTEXT(box->storage->user);
+       struct mail_deliver_transaction *dt =
+               MAIL_DELIVER_STORAGE_CONTEXT(ctx->transaction);
 
        if (mbox->module_ctx.super.copy(ctx, mail) < 0)
                return -1;
 
        /* initialize most of the fields from dest_mail */
-       mail_deliver_log_update_cache(muser->deliver_ctx->cache,
-                                     muser->deliver_ctx->pool, ctx->dest_mail);
+       mail_deliver_log_update_cache(&dt->cache, muser->deliver_ctx->pool,
+                                     ctx->dest_mail);
        return 0;
 }
 
@@ -576,14 +592,51 @@ mail_deliver_cache_update_post_commit(struct mailbox *orig_box, uint32_t uid)
        mailbox_free(&box);
 }
 
+static struct mailbox_transaction_context *
+mail_deliver_transaction_begin(struct mailbox *box,
+                              enum mailbox_transaction_flags flags)
+{
+       struct mail_deliver_mailbox *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box);
+       struct mail_deliver_user *muser =
+               MAIL_DELIVER_USER_CONTEXT(box->storage->user);
+       struct mailbox_transaction_context *t;
+       struct mail_deliver_transaction *dt;
+
+       i_assert(muser != NULL);
+       i_assert(muser->deliver_ctx != NULL);
+
+       t = mbox->module_ctx.super.transaction_begin(box, flags);
+       dt = p_new(muser->deliver_ctx->pool, struct mail_deliver_transaction, 1);
+
+       MODULE_CONTEXT_SET(t, mail_deliver_storage_module, dt);
+       return t;
+}
+
 static int
 mail_deliver_transaction_commit(struct mailbox_transaction_context *ctx,
                                struct mail_transaction_commit_changes *changes_r)
 {
        struct mailbox *box = ctx->box;
-       union mailbox_module_context *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box);
+       struct mail_deliver_mailbox *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box);
+       struct mail_deliver_transaction *dt = MAIL_DELIVER_STORAGE_CONTEXT(ctx);
+       struct mail_deliver_user *muser =
+               MAIL_DELIVER_USER_CONTEXT(box->storage->user);
+
+       i_assert(dt != NULL);
+       i_assert(muser != NULL);
+       i_assert(muser->deliver_ctx != NULL);
+
+       /* sieve creates multiple transactions, saves the mails and
+          then commits all of them at the end. we'll need to keep
+          switching the deliver_ctx->cache for each commit.
+
+          we also want to do this only for commits generated by sieve.
+          other plugins or storage backends may be creating transactions as
+          well, which we need to ignore. */
+       if (mbox->delivery_box)
+               muser->deliver_ctx->cache = &dt->cache;
 
-       if (mbox->super.transaction_commit(ctx, changes_r) < 0)
+       if (mbox->module_ctx.super.transaction_commit(ctx, changes_r) < 0)
                return -1;
 
        if (array_count(&changes_r->saved_uids) > 0) {
@@ -620,6 +673,7 @@ static void mail_deliver_mailbox_allocated(struct mailbox *box)
        box->vlast = &mbox->module_ctx.super;
        v->save_finish = mail_deliver_save_finish;
        v->copy = mail_deliver_copy;
+       v->transaction_begin = mail_deliver_transaction_begin;
        v->transaction_commit = mail_deliver_transaction_commit;
 
        MODULE_CONTEXT_SET(box, mail_deliver_storage_module, mbox);