# HG changeset patch # User Timo Sirainen # Date 1487673490 -7200 # Node ID bfa56df7a065132a60c4c0e417cf067508023cb7 # Parent b68299be012add70ace506fa8de8a13170d0cd70 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 -r b68299be012a -r bfa56df7a065 src/lib-lda/mail-deliver.c --- a/src/lib-lda/mail-deliver.c Tue Feb 21 12:36:52 2017 +0200 +++ b/src/lib-lda/mail-deliver.c Tue Feb 21 12:38:10 2017 +0200 @@ -43,6 +43,13 @@ 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[] = { @@ -223,6 +230,11 @@ } *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; @@ -514,13 +526,15 @@ 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; } @@ -530,13 +544,15 @@ 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; } @@ -573,14 +589,51 @@ 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); - if (mbox->super.transaction_commit(ctx, changes_r) < 0) + /* 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->module_ctx.super.transaction_commit(ctx, changes_r) < 0) return -1; if (array_count(&changes_r->saved_uids) > 0) { @@ -617,6 +670,7 @@ 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);