Refactor campaign delivery decision paths

2026-07-29 17:06:15 +02:00
parent 5240749ae1
commit dd9592a192
9 changed files with 1103 additions and 593 deletions
@@ -222,6 +222,33 @@ class _MailChannelOutcome:
        )


+@dataclass(frozen=True, slots=True)
+class _DeliveryOutcomeSummary:
+    mail_accepted: bool
+    postbox_accepted: int
+    postbox_rejected: int
+    outcome_unknown: bool
+    temporary_rejection: bool
+
+    @property
+    def accepted_count(self) -> int:
+        return int(self.mail_accepted) + self.postbox_accepted
+
+
+@dataclass(frozen=True, slots=True)
+class _ImapAppendContext:
+    snapshot: ExecutionSnapshot
+    message_bytes: bytes
+    folder: str
+
+
+@dataclass(frozen=True, slots=True)
+class _ClaimedImapAppend:
+    job: CampaignJob
+    attempt: ImapAppendAttempt
+    claim_token: str
+
+
 QUEUEABLE_VALIDATION_STATUSES = {
    JobValidationStatus.READY.value,
    JobValidationStatus.WARNING.value,
@@ -2338,47 +2365,50 @@ def _final_multichannel_status(
    mail: _MailChannelOutcome | None,
    postbox: PostboxChannelOutcome | None,
 ) -> str:
-    mail_accepted = bool(mail and mail.accepted)
-    postbox_accepted = int(postbox.accepted_count if postbox else 0)
-    postbox_rejected = int(postbox.rejected_count if postbox else 0)
-    unknown = bool(
-        (mail and mail.outcome_unknown)
-        or (postbox and postbox.outcome_unknown)
-    )
-    if unknown:
+    outcome = _delivery_outcome_summary(mail=mail, postbox=postbox)
+    if outcome.outcome_unknown:
        return JobSendStatus.OUTCOME_UNKNOWN.value
+    if not outcome.accepted_count:
+        return JobSendStatus.FAILED_TEMPORARY.value if outcome.temporary_rejection else JobSendStatus.FAILED_PERMANENT.value
+    classifier = _ACCEPTED_DELIVERY_CLASSIFIERS.get(channel_policy, _classify_fallback_delivery)
+    return classifier(outcome)

-    accepted_count = int(mail_accepted) + postbox_accepted
-    if accepted_count:
-        if channel_policy == DeliveryChannelPolicy.POSTBOX:
-            return (
-                JobSendStatus.PARTIALLY_ACCEPTED.value
-                if postbox_rejected
-                else JobSendStatus.POSTBOX_ACCEPTED.value
-            )
-        if channel_policy == DeliveryChannelPolicy.MAIL_AND_POSTBOX:
-            if mail_accepted and postbox_accepted and not postbox_rejected:
-                return JobSendStatus.DELIVERED.value
-            return JobSendStatus.PARTIALLY_ACCEPTED.value
-        if postbox_rejected:
-            return JobSendStatus.PARTIALLY_ACCEPTED.value
-        return (
-            JobSendStatus.SMTP_ACCEPTED.value
-            if mail_accepted
-            else JobSendStatus.POSTBOX_ACCEPTED.value
-        )

-    temporary = bool(
-        (mail and mail.rejected_temporary)
-        or (postbox and postbox.rejected_temporary)
-    )
-    return (
-        JobSendStatus.FAILED_TEMPORARY.value
-        if temporary
-        else JobSendStatus.FAILED_PERMANENT.value
+def _delivery_outcome_summary(
+    *,
+    mail: _MailChannelOutcome | None,
+    postbox: PostboxChannelOutcome | None,
+) -> _DeliveryOutcomeSummary:
+    return _DeliveryOutcomeSummary(
+        mail_accepted=bool(mail and mail.accepted),
+        postbox_accepted=int(postbox.accepted_count if postbox else 0),
+        postbox_rejected=int(postbox.rejected_count if postbox else 0),
+        outcome_unknown=bool((mail and mail.outcome_unknown) or (postbox and postbox.outcome_unknown)),
+        temporary_rejection=bool((mail and mail.rejected_temporary) or (postbox and postbox.rejected_temporary)),
    )


+def _classify_postbox_delivery(outcome: _DeliveryOutcomeSummary) -> str:
+    return JobSendStatus.PARTIALLY_ACCEPTED.value if outcome.postbox_rejected else JobSendStatus.POSTBOX_ACCEPTED.value
+
+
+def _classify_dual_delivery(outcome: _DeliveryOutcomeSummary) -> str:
+    fully_delivered = outcome.mail_accepted and outcome.postbox_accepted > 0 and not outcome.postbox_rejected
+    return JobSendStatus.DELIVERED.value if fully_delivered else JobSendStatus.PARTIALLY_ACCEPTED.value
+
+
+def _classify_fallback_delivery(outcome: _DeliveryOutcomeSummary) -> str:
+    if outcome.postbox_rejected:
+        return JobSendStatus.PARTIALLY_ACCEPTED.value
+    return JobSendStatus.SMTP_ACCEPTED.value if outcome.mail_accepted else JobSendStatus.POSTBOX_ACCEPTED.value
+
+
+_ACCEPTED_DELIVERY_CLASSIFIERS = {
+    DeliveryChannelPolicy.POSTBOX: _classify_postbox_delivery,
+    DeliveryChannelPolicy.MAIL_AND_POSTBOX: _classify_dual_delivery,
+}
+
+
 def _multichannel_messages(
    mail: _MailChannelOutcome | None,
    postbox: PostboxChannelOutcome | None,
@@ -2931,22 +2961,29 @@ def _mark_imap_append_outcome_unknown_after_effect(
    )


-def append_sent_for_job(session: Session, *, job_id: str, dry_run: bool = False) -> AppendSentResult:
-    """Append one successfully sent job's exact EML to the configured IMAP Sent folder."""
+def _imap_append_precondition(
+    session: Session,
+    job: CampaignJob,
+    *,
+    dry_run: bool,
+) -> AppendSentResult | None:
+    if not _mail_was_accepted(session, job.id, send_status=job.send_status):
+        return AppendSentResult(
+            job_id=job.id,
+            status="not_sent",
+            attempt_number=0,
+            dry_run=dry_run,
+            message="SMTP has not accepted this job",
+        )
+    return _imap_blocked_result(session, job, dry_run=dry_run)

-    job = session.get(CampaignJob, job_id)
-    if not job:
-        raise SendJobError(f"Job not found: {job_id}")
-    if not _mail_was_accepted(
-        session,
-        job.id,
-        send_status=job.send_status,
-    ):
-        return AppendSentResult(job_id=job_id, status="not_sent", attempt_number=0, dry_run=dry_run, message="SMTP has not accepted this job")
-    blocked = _imap_blocked_result(session, job, dry_run=dry_run)
-    if blocked is not None:
-        return blocked

+def _prepare_imap_append(
+    session: Session,
+    job: CampaignJob,
+    *,
+    dry_run: bool,
+) -> _ImapAppendContext | AppendSentResult:
    version = session.get(CampaignVersion, job.campaign_version_id)
    if not version:
        raise SendJobError("Campaign version not found")
@@ -2965,20 +3002,25 @@ def append_sent_for_job(session: Session, *, job_id: str, dry_run: bool = False)
        session.add(job)
        session.commit()
        return AppendSentResult(job_id=job.id, status="skipped", attempt_number=0, dry_run=dry_run, message=job.last_error)
+    return _ImapAppendContext(
+        snapshot=snapshot,
+        message_bytes=_load_eml_bytes_for_job(job),
+        folder=snapshot.delivery.imap_append_sent.folder or "auto",
+    )

-    message_bytes = _load_eml_bytes_for_job(job)
-    folder = snapshot.delivery.imap_append_sent.folder or "auto"
-    if dry_run:
-        attempts = session.query(ImapAppendAttempt).filter(ImapAppendAttempt.job_id == job.id).count()
-        return AppendSentResult(
-            job_id=job.id,
-            status="dry_run",
-            attempt_number=attempts,
-            dry_run=True,
-            folder=folder,
-            message=f"Would append {len(message_bytes)} bytes to IMAP folder {folder!r}",
-        )

+def _imap_append_dry_run(session: Session, job: CampaignJob, context: _ImapAppendContext) -> AppendSentResult:
+    return AppendSentResult(
+        job_id=job.id,
+        status="dry_run",
+        attempt_number=_imap_attempt_count(session, job.id),
+        dry_run=True,
+        folder=context.folder,
+        message=f"Would append {len(context.message_bytes)} bytes to IMAP folder {context.folder!r}",
+    )
+
+
+def _claim_imap_append(session: Session, job: CampaignJob) -> _ClaimedImapAppend | AppendSentResult:
    claim_token = _claim_job_for_imap_append(session, job)
    if claim_token is None:
        current = session.get(CampaignJob, job.id)
@@ -2993,80 +3035,98 @@ def append_sent_for_job(session: Session, *, job_id: str, dry_run: bool = False)
            attempt_number=_imap_attempt_count(session, current.id),
            message=f"Job is no longer eligible for IMAP append: {current.imap_status}",
        )
-    job = session.get(CampaignJob, job.id)
-    if job is None:
+    claimed_job = session.get(CampaignJob, job.id)
+    if claimed_job is None:
        raise SendJobError("Claimed campaign job disappeared before IMAP append")
-    attempt = _record_imap_attempt_start(session, job, claim_token)
+    return _ClaimedImapAppend(
+        job=claimed_job,
+        attempt=_record_imap_attempt_start(session, claimed_job, claim_token),
+        claim_token=claim_token,
+    )
+
+
+def _record_imap_provider_error(
+    session: Session,
+    claimed: _ClaimedImapAppend,
+    *,
+    folder: str,
+    message: str,
+    outcome_unknown: bool,
+) -> None:
+    owned = _record_imap_append_failure(
+        session,
+        job=claimed.job,
+        attempt=claimed.attempt,
+        claim_token=claimed.claim_token,
+        folder=folder,
+        message=message,
+        outcome_unknown=outcome_unknown,
+    )
+    if not owned:
+        _imap_result_after_lost_claim(
+            session,
+            job_id=claimed.job.id,
+            attempt=claimed.attempt,
+            provider_succeeded=outcome_unknown,
+        )
+
+
+def _invoke_imap_append(session: Session, claimed: _ClaimedImapAppend, context: _ImapAppendContext):
+    snapshot = context.snapshot
+    job = claimed.job
+    return mail_integration().append_campaign_message_to_sent(
+        session,
+        tenant_id=job.tenant_id,
+        campaign_id=job.campaign_id,
+        profile_id=snapshot.mail_profile_id,
+        message_bytes=context.message_bytes,
+        folder=None if context.folder == "auto" else context.folder,
+        expected_smtp_transport_revision=snapshot.smtp_transport_revision or "",
+        expected_imap_transport_revision=snapshot.imap_transport_revision,
+        smtp_server_id=snapshot.smtp_server_id,
+        smtp_credential_id=snapshot.smtp_credential_id,
+        imap_server_id=snapshot.imap_server_id,
+        imap_credential_id=snapshot.imap_credential_id,
+    )
+
+
+def _perform_imap_append(
+    session: Session,
+    claimed: _ClaimedImapAppend,
+    context: _ImapAppendContext,
+) -> AppendSentResult:
    try:
-        result = mail_integration().append_campaign_message_to_sent(
-            session,
-            tenant_id=job.tenant_id,
-            campaign_id=job.campaign_id,
-            profile_id=snapshot.mail_profile_id,
-            message_bytes=message_bytes,
-            folder=None if folder == "auto" else folder,
-            expected_smtp_transport_revision=snapshot.smtp_transport_revision or "",
-            expected_imap_transport_revision=snapshot.imap_transport_revision,
-            smtp_server_id=snapshot.smtp_server_id,
-            smtp_credential_id=snapshot.smtp_credential_id,
-            imap_server_id=snapshot.imap_server_id,
-            imap_credential_id=snapshot.imap_credential_id,
-        )
+        result = _invoke_imap_append(session, claimed, context)
    except (MailProfileError, ImapConfigurationError, ImapAppendError) as exc:
-        outcome_unknown = bool(getattr(exc, "outcome_unknown", False))
-        owned = _record_imap_append_failure(
+        _record_imap_provider_error(
            session,
-            job=job,
-            attempt=attempt,
-            claim_token=claim_token,
-            folder=folder,
+            claimed,
+            folder=context.folder,
            message=str(exc),
-            outcome_unknown=outcome_unknown,
+            outcome_unknown=bool(getattr(exc, "outcome_unknown", False)),
        )
-        if not owned:
-            _imap_result_after_lost_claim(
-                session,
-                job_id=job.id,
-                attempt=attempt,
-                provider_succeeded=outcome_unknown,
-            )
        raise
    except Exception:
        reason = (
            "The Sent-folder append outcome is unknown after an unexpected provider failure; "
            "inspect and reconcile the mailbox before retrying."
        )
-        owned = _record_imap_append_failure(
-            session,
-            job=job,
-            attempt=attempt,
-            claim_token=claim_token,
-            folder=folder,
-            message=reason,
-            outcome_unknown=True,
-        )
-        if not owned:
-            _imap_result_after_lost_claim(
-                session,
-                job_id=job.id,
-                attempt=attempt,
-                provider_succeeded=True,
-            )
+        _record_imap_provider_error(session, claimed, folder=context.folder, message=reason, outcome_unknown=True)
        raise ImapAppendError(reason, outcome_unknown=True) from None
    try:
        return _record_imap_append_success(
            session,
-            job=job,
-            attempt=attempt,
-            claim_token=claim_token,
+            job=claimed.job,
+            attempt=claimed.attempt,
+            claim_token=claimed.claim_token,
            folder=result.folder,
        )
    except Exception:
        return _mark_imap_append_outcome_unknown_after_effect(
            session,
-            job_id=job.id,
-            attempt_id=attempt.id,
-            claim_token=claim_token,
+            job_id=claimed.job.id,
+            attempt_id=claimed.attempt.id,
+            claim_token=claimed.claim_token,
            reason=(
                "The IMAP provider accepted the append, but persisting the outcome failed. "
                "Automatic retry is stopped until an operator reconciles the mailbox."
@@ -3074,6 +3134,26 @@ def append_sent_for_job(session: Session, *, job_id: str, dry_run: bool = False)
        )


+def append_sent_for_job(session: Session, *, job_id: str, dry_run: bool = False) -> AppendSentResult:
+    """Append one successfully sent job's exact EML to the configured IMAP Sent folder."""
+
+    job = session.get(CampaignJob, job_id)
+    if not job:
+        raise SendJobError(f"Job not found: {job_id}")
+    blocked = _imap_append_precondition(session, job, dry_run=dry_run)
+    if blocked is not None:
+        return blocked
+    prepared = _prepare_imap_append(session, job, dry_run=dry_run)
+    if isinstance(prepared, AppendSentResult):
+        return prepared
+    if dry_run:
+        return _imap_append_dry_run(session, job, prepared)
+    claimed = _claim_imap_append(session, job)
+    if isinstance(claimed, AppendSentResult):
+        return claimed
+    return _perform_imap_append(session, claimed, prepared)
+
+
 def enqueue_pending_imap_appends(
    session: Session,
    *,
@@ -173,97 +173,82 @@ def _participants(job: CampaignJob) -> tuple[PostboxParticipantRef, ...]:


 def _attachments(job: CampaignJob) -> tuple[PostboxAttachmentRef, ...]:
-    values = [
-        PostboxAttachmentRef(
-            reference_type="campaign_eml",
-            reference_id=job.id,
-            name=f"{job.entry_id or job.entry_index}.eml",
-            media_type="message/rfc822",
-            size_bytes=job.eml_size_bytes,
-            digest=job.eml_sha256,
-            metadata={
-                "campaign_id": job.campaign_id,
-                "campaign_version_id": job.campaign_version_id,
-            },
-        )
-    ]
+    values = [_eml_attachment(job)]
    for rule_index, rule in enumerate(job.resolved_attachments or []):
        if not isinstance(rule, dict):
            continue
        managed_matches = rule.get("managed_matches")
        if isinstance(managed_matches, list) and managed_matches:
-            for match in managed_matches:
-                if not isinstance(match, dict):
-                    continue
-                reference_id = str(
-                    match.get("version_id")
-                    or match.get("asset_id")
-                    or match.get("blob_id")
-                    or ""
-                ).strip()
-                if not reference_id:
-                    continue
-                values.append(
-                    PostboxAttachmentRef(
-                        reference_type=(
-                            "file_version"
-                            if match.get("version_id")
-                            else "file_asset"
-                        ),
-                        reference_id=reference_id,
-                        name=str(match.get("filename") or "").strip() or None,
-                        media_type=(
-                            str(match.get("content_type"))
-                            if match.get("content_type")
-                            else None
-                        ),
-                        size_bytes=(
-                            int(match["size_bytes"])
-                            if match.get("size_bytes") is not None
-                            else None
-                        ),
-                        digest=(
-                            str(match.get("checksum_sha256"))
-                            if match.get("checksum_sha256")
-                            else None
-                        ),
-                        metadata={
-                            key: value
-                            for key, value in match.items()
-                            if key
-                            in {
-                                "asset_id",
-                                "version_id",
-                                "blob_id",
-                                "display_path",
-                                "relative_path",
-                                "owner_type",
-                                "owner_id",
-                                "source_revision",
-                            }
-                        },
-                    )
-                )
+            values.extend(_managed_attachments(managed_matches))
            continue
        matches = rule.get("matches")
-        if not isinstance(matches, list):
-            continue
-        for match_index, match in enumerate(matches):
-            values.append(
-                PostboxAttachmentRef(
-                    reference_type="campaign_attachment",
-                    reference_id=f"{job.id}:{rule_index}:{match_index}",
-                    name=str(match).rsplit("/", 1)[-1] or None,
-                    metadata={
-                        "campaign_id": job.campaign_id,
-                        "campaign_version_id": job.campaign_version_id,
-                        "job_id": job.id,
-                    },
-                )
-            )
+        if isinstance(matches, list):
+            values.extend(_campaign_attachments(job, rule_index=rule_index, matches=matches))
    return tuple(values)


+def _eml_attachment(job: CampaignJob) -> PostboxAttachmentRef:
+    return PostboxAttachmentRef(
+        reference_type="campaign_eml",
+        reference_id=job.id,
+        name=f"{job.entry_id or job.entry_index}.eml",
+        media_type="message/rfc822",
+        size_bytes=job.eml_size_bytes,
+        digest=job.eml_sha256,
+        metadata={"campaign_id": job.campaign_id, "campaign_version_id": job.campaign_version_id},
+    )
+
+
+MANAGED_ATTACHMENT_METADATA_KEYS = {
+    "asset_id",
+    "version_id",
+    "blob_id",
+    "display_path",
+    "relative_path",
+    "owner_type",
+    "owner_id",
+    "source_revision",
+}
+
+
+def _managed_attachment(match: dict[str, Any]) -> PostboxAttachmentRef | None:
+    reference_id = str(match.get("version_id") or match.get("asset_id") or match.get("blob_id") or "").strip()
+    if not reference_id:
+        return None
+    return PostboxAttachmentRef(
+        reference_type="file_version" if match.get("version_id") else "file_asset",
+        reference_id=reference_id,
+        name=str(match.get("filename") or "").strip() or None,
+        media_type=str(match.get("content_type")) if match.get("content_type") else None,
+        size_bytes=int(match["size_bytes"]) if match.get("size_bytes") is not None else None,
+        digest=str(match.get("checksum_sha256")) if match.get("checksum_sha256") else None,
+        metadata={key: value for key, value in match.items() if key in MANAGED_ATTACHMENT_METADATA_KEYS},
+    )
+
+
+def _managed_attachments(matches: list[Any]) -> list[PostboxAttachmentRef]:
+    attachments = (_managed_attachment(match) for match in matches if isinstance(match, dict))
+    return [attachment for attachment in attachments if attachment is not None]
+
+
+def _campaign_attachments(
+    job: CampaignJob,
+    *,
+    rule_index: int,
+    matches: list[Any],
+) -> list[PostboxAttachmentRef]:
+    metadata = {"campaign_id": job.campaign_id, "campaign_version_id": job.campaign_version_id, "job_id": job.id}
+    return [
+        PostboxAttachmentRef(
+            reference_type="campaign_attachment",
+            reference_id=f"{job.id}:{rule_index}:{match_index}",
+            name=str(match).rsplit("/", 1)[-1] or None,
+            metadata=metadata,
+        )
+        for match_index, match in enumerate(matches)
+    ]
+
+
 def _sender_label(job: CampaignJob) -> str | None:
    recipients = job.resolved_recipients or {}
    sender = recipients.get("from")