docs(adr): ADR-006 synchronous domain events inside the publisher's transaction
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 3m16s
CI / OCR Service Tests (pull_request) Successful in 1m34s
CI / Backend Unit Tests (pull_request) Failing after 4m14s
CI / Unit & Component Tests (push) Failing after 3m29s
CI / OCR Service Tests (push) Successful in 50s
CI / Backend Unit Tests (push) Failing after 3m43s

Markus #4 (PR #366 review). PersonDisplayNameChangedEvent is the first
custom application event in this codebase — the prior @EventListener
(OcrTrainingService.recoverOrphanedRuns) consumed Spring's built-in
ApplicationReadyEvent. The pattern is load-bearing for future cross-domain
decoupling and warrants a documented decision rather than a comment buried
in the listener.

Captures: synchronous-by-default rationale, package layout (event in
publisher's model/, listener in consumer's service/), saveAllAndFlush vs
saveAll for exception surfacing, the migration path to @TransactionalEvent
Listener + @Async if archive growth forces it, and the rejected
alternatives (direct call, DB trigger, Hibernate entity listener).

Refs #362 #366

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-04-28 21:42:03 +02:00
parent 2d48821f95
commit 7906373053

View File

@@ -0,0 +1,55 @@
# ADR-006: Synchronous domain events inside the publisher's transaction
## Status
Accepted
## Context
Issue #362 introduced the first cross-domain side-effect in this codebase: when a Person's display name changes, every transcription block that mentions the person must be rewritten — both `block.text` (the literal `@OldName` substring) and the `mentionedPersons` sidecar (the `displayName` field on the matching `PersonMention`). The rewrite is bidirectionally referential — Person depends on Transcription to make the rename atomic, and Transcription depends on Person to know what the new display name is.
A direct method call from `PersonService` into `TranscriptionBlockService` would invert the existing dependency arrow (Document → Person, not Person → Transcription) and introduce a runtime-circular reference at the package level. Avoiding the cycle while keeping the rename atomic is the constraint this ADR addresses.
Two prior pieces of infrastructure constrain the solution:
- `transcription_blocks.version` (JPA `@Version`) — concurrent autosave on a referenced block must roll back the rename instead of silently overwriting the autosave.
- `OcrTrainingService.recoverOrphanedRuns` is the only existing `@EventListener` and it consumes Spring's built-in `ApplicationReadyEvent` — no precedent for a custom domain event in this codebase before now.
## Decision
`PersonService.updatePerson` publishes `PersonDisplayNameChangedEvent(personId, oldDisplayName, newDisplayName)` via `ApplicationEventPublisher` whenever `Person.getDisplayName()` flips between the pre-save snapshot and the post-save value. `PersonMentionPropagationListener` (in the transcription package's `service/` layer) handles the event with `@EventListener @Transactional`, finds blocks via `findByMentionedPersons_PersonId`, rewrites text + sidecar, and calls `saveAllAndFlush`.
**Synchronous on purpose.** Spring's default event dispatcher invokes listeners on the publishing thread, inside the publisher's transaction. The propagation runs as part of the same `@Transactional` boundary as the rename — `OptimisticLockingFailureException` from a referenced block bubbles back up, the surrounding transaction rolls back, and `PersonService.updatePerson` translates it to `DomainException(PERSON_RENAME_CONFLICT, 409)`.
**Pattern for future cross-domain decoupling:**
1. Event record in `model/` of the publishing domain (e.g. `PersonDisplayNameChangedEvent`).
2. Listener in `service/` of the consuming domain (e.g. `PersonMentionPropagationListener`).
3. `@EventListener @Transactional` on the listener method — no `@TransactionalEventListener` unless the work genuinely doesn't need to commit with the publisher.
4. `saveAllAndFlush` (not `saveAll`) on any write where exceptions must surface inside the listener call so the publisher can catch and translate them — `saveAll` defers exceptions to commit time, after the publisher's `try` block has exited.
5. Audit log line at `INFO` level on the listener method — historical-text mutation needs an audit trail.
## Alternatives Considered
| Alternative | Why rejected |
|---|---|
| `PersonService` calls `TranscriptionBlockService.propagateDisplayNameChange(...)` directly | Inverts the dependency arrow. Person becomes runtime-coupled to Transcription; future domains that also care about renames (Comments, Notifications) compound the coupling. Events keep Person agnostic of who consumes them. |
| `@TransactionalEventListener(AFTER_COMMIT) + @Async` | The propagation would run after the rename commits, on a separate transaction. A failed propagation could leave block text out of sync with the renamed person until manual repair. Atomic transactional coupling is the safer default for historical-text mutation; switch to async only when the block count makes sync latency unacceptable (rough threshold: tens of thousands of blocks per renamed person). |
| Database trigger on `persons.last_name` | PL/pgSQL trigger would have to reach into `transcription_block_mentioned_persons` and `transcription_blocks.text`, smearing domain logic across SQL and Java. JPA's `@Version` would also be invisible to the trigger, so concurrent block autosaves would race silently. |
| Hibernate entity listener (`@PostUpdate` on Person) | Couples to Hibernate internals; harder to test in isolation; mixes lifecycle hooks with cross-domain side effects. Spring's `ApplicationEventPublisher` keeps the integration declarative and unit-testable. |
## Consequences
**Easier:**
- Person domain stays free of any compile-time dependency on Transcription. Future consumers (Comments, Notifications) subscribe to the same event without `PersonService` knowing they exist.
- Rename + propagation share one transaction → no half-applied state visible to readers, no orphaned rewrites if the rename fails after propagation, no "eventually-consistent" window for an archive that prizes historical fidelity.
- Concurrent autosaves on referenced blocks raise a structured 409 the frontend can render meaningfully (`error_person_rename_conflict`) instead of a generic 500.
- The pattern itself (record event in `model/`, listener in consumer's `service/`, sync `@EventListener @Transactional`, `saveAllAndFlush`) is reusable for the next cross-domain side effect.
**Harder:**
- Listener latency adds to the rename request's response time. The 200-block latency floor (< 2 s) is a merge-blocking regression test; if archive growth pushes it up, the migration path is one-annotation: switch to `@TransactionalEventListener(AFTER_COMMIT) + @Async` and add a manual-repair tool for propagation failures.
- Tests for the listener path require routing the publisher mock through a real listener (see `PersonServiceTest#updatePerson_throwsConflict_whenBlockSaveAllAndFlushHitsOptimisticLock`). Slightly more setup than a pure-Mockito test, but exercises the production call chain.
- `saveAllAndFlush` is mandatory in any synchronous listener that must surface JPA exceptions to the publisher's `try`-block. `saveAll` alone defers the flush to transaction commit, which happens after the publisher returns.
## Future Direction
If a single rename starts touching tens of thousands of blocks, switch the listener to `@TransactionalEventListener(phase = AFTER_COMMIT)` paired with `@Async` and add (a) an idempotency key to the event so a retry doesn't double-rewrite, (b) an admin tool that scans for sidecar entries whose `displayName` doesn't match the current `Person.getDisplayName()` and repairs them. At that point the orphan-guard path (existsById check before the rewrite) re-enters the listener as a deliberate piece of the async machinery rather than dead code.