docs(adr): ADR-038 — domain event drives note-less journey-item cleanup on document delete (#805)
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m49s
CI / OCR Service Tests (pull_request) Successful in 31s
CI / Backend Unit Tests (pull_request) Failing after 6m43s
CI / fail2ban Regex (pull_request) Successful in 58s
CI / Semgrep Security Scan (pull_request) Successful in 24s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m7s
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m49s
CI / OCR Service Tests (pull_request) Successful in 31s
CI / Backend Unit Tests (pull_request) Failing after 6m43s
CI / fail2ban Regex (pull_request) Successful in 58s
CI / Semgrep Security Scan (pull_request) Successful in 24s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m7s
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
118
docs/adr/038-domain-event-driven-journey-item-cleanup.md
Normal file
118
docs/adr/038-domain-event-driven-journey-item-cleanup.md
Normal file
@@ -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).
|
||||||
Reference in New Issue
Block a user