From eefc67bd81f0bb26446098115d0221bd29fbbc75 Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 11 Jun 2026 17:55:38 +0200 Subject: [PATCH] =?UTF-8?q?docs(adr):=20ADR-038=20=E2=80=94=20domain=20eve?= =?UTF-8?q?nt=20drives=20note-less=20journey-item=20cleanup=20on=20documen?= =?UTF-8?q?t=20delete=20(#805)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- ...omain-event-driven-journey-item-cleanup.md | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 docs/adr/038-domain-event-driven-journey-item-cleanup.md diff --git a/docs/adr/038-domain-event-driven-journey-item-cleanup.md b/docs/adr/038-domain-event-driven-journey-item-cleanup.md new file mode 100644 index 00000000..83096f6e --- /dev/null +++ b/docs/adr/038-domain-event-driven-journey-item-cleanup.md @@ -0,0 +1,118 @@ +# ADR-038 — Domain event drives note-less journey-item cleanup on document delete + +**Status:** Accepted +**Date:** 2026-06-11 +**Issue:** #805 (P1 — deleting a document linked via a note-less journey_item 500s at DB constraint) + +## Context + +Two constraints in V72 encode contradictory rules for a journey item that has a +`document_id` but no `note`: + +- **`fk_journey_items_document` → `ON DELETE SET NULL`** — when a document is deleted, + Postgres nulls out `document_id`. +- **`chk_journey_item_not_empty`** — requires at least one of `document_id` or `note` + to be non-null. + +A note-less item (`document_id` set, `note IS NULL`) satisfies the CHECK while the +document exists. Deleting the document causes Postgres to attempt `SET NULL`, which +would leave both columns null — a direct CHECK violation. Postgres aborts the +transaction with a 500 that bypasses `GlobalExceptionHandler`. + +The natural fix — delete note-less items inside `DocumentService.deleteDocument` before +`deleteById` runs — cannot call `JourneyItemService` directly: `JourneyItemService` +already injects `DocumentService`, and Spring Framework 7 (used by Spring Boot 4) +**fully prohibits constructor-injection cycles**. The application will not start if such +a cycle is introduced. + +## Decision + +`DocumentService.deleteDocument` publishes a **`DocumentDeletingEvent`** (plain record, +payload: `documentId` UUID only) via `ApplicationEventPublisher` **before** +`documentRepository.deleteById`. A dedicated `@Component` +`JourneyItemDocumentDeleteListener` in the `geschichte.journeyitem` package consumes +this event and calls `journeyItemRepository.deleteNoteLessByDocumentId(documentId)` +directly — bypassing `JourneyItemService` to avoid re-introducing the cycle and to +suppress the per-item `JOURNEY_ITEM_REMOVED` audit emission (see audit decision below). + +### Load-bearing listener-phase choice: plain `@EventListener` + +The listener is annotated with `@EventListener` (not +`@TransactionalEventListener(AFTER_COMMIT)`, not `@Async`). **This choice is +load-bearing:** + +- **`AFTER_COMMIT` would break the fix entirely.** `AFTER_COMMIT` fires *after* the + surrounding transaction has committed. By that point, `documentRepository.deleteById` + has already executed and Postgres has already tried `ON DELETE SET NULL` — the + constraint violation fires before the listener ever runs. +- **`@Async` would break rollback atomicity (AC-5).** An async listener runs on a + separate thread in its own transaction. If `deleteDocument` subsequently rolls back + (e.g. due to an unrelated failure), the listener's deletes are in a committed async + transaction and cannot be undone. +- **Plain `@EventListener` runs synchronously in the publisher's thread and + transaction.** The listener's JPQL delete and the `deleteById` are a single atomic + unit: if either fails, both roll back together. + +### Repository method + +```java +@Modifying(clearAutomatically = true) +@Query("DELETE FROM JourneyItem i WHERE i.document.id = :documentId AND (i.note IS NULL OR i.note = '')") +int deleteNoteLessByDocumentId(@Param("documentId") UUID documentId); +``` + +`i.document.id` (the real association path) is used instead of `i.documentId`: the +transient `getDocumentId()` getter on `JourneyItem` makes Spring Data unable to resolve +a derived query path (same trap documented at `JourneyItemRepository:33-44`). + +`clearAutomatically = true` invalidates the L1 cache so subsequent reads in the same +session do not return stale entities. + +The predicate `(note IS NULL OR note = '')` covers the `note = ''` edge case that the +service layer can never produce (normalizeNote converts blank strings to null), but that +may exist via raw SQL inserts or legacy data. Whitespace-only notes (`note = ' '`) +do not match and are preserved as note-carrying placeholders. + +### Audit decision + +The listener calls the repository directly rather than routing through +`JourneyItemService.delete`. This deliberately bypasses the `JOURNEY_ITEM_REMOVED` +audit emission: a document used in multiple journeys would otherwise produce N audit +rows for a single user action. The `DOCUMENT_DELETED` entry written by `deleteDocument` +is the sole audit record for the operation. + +### Boundary: documents must not depend on journey + +The event direction is `document → journey`, never the reverse. `DocumentService` +publishes events it knows nothing about the consumers of; `JourneyItemService`'s +dependency on `DocumentService` is unchanged and remains the only cross-domain +reference. This direction is the prerequisite for the cycle constraint to hold. + +## Alternatives rejected + +- **DB trigger on `journey_items`** — trigger logic is opaque to Java developers, + invisible to code review, and not covered by the JPA test harness. +- **RESTRICT instead of SET NULL** — breaks the existing note-carrying placeholder + UX: deleting a document with a note-carrying journey item would 409 instead of + preserving the item as a placeholder. +- **Relax `chk_journey_item_not_empty`** — the constraint enforces a real invariant + (every item must have at least document or note). Removing it would allow empty rows. +- **`@Lazy` on the `JourneyItemService → DocumentService` injection** — Spring Boot 4 / + Spring Framework 7 prohibits constructor-injection cycles regardless of `@Lazy`. +- **Make `DocumentService` call `JourneyItemService`** — introduces the prohibited + cycle. Rejected at design time. + +## Consequences + +- **No schema change** — no new Flyway migration, no `db-orm.puml` / + `db-relationships.puml` update. +- This is the **first custom domain event** in the codebase. No prior + `ApplicationEventPublisher` usage existed in `main/`. New cross-domain cleanup that + cannot use direct service calls should follow this pattern. +- All tests that delete documents and then assert journey-item state **must route + through `DocumentService.deleteDocument`**, not `documentRepository.deleteById`. + The existing `JourneyItemIntegrationTest` tests that covered the note-carrying + placeholder UX have been updated accordingly. +- The `DOCUMENT_DELETED` `AuditKind` was added as part of this fix to give AC-7's + audit assertion a positive check (absence-only assertions pass vacuously if all + auditing regresses).