MassImportService injected DocumentRepository for the find-or-create pattern
during ODS/Excel import. Move the two repository touchpoints (findByOriginalFilename,
save) onto DocumentService as 1-line delegations and update the consumer.
Refs #417 (C6.2 violation #1).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
TranscriptionQueueService injected DocumentRepository to fetch the four queue
projections. Move the four read methods (findSegmentationQueue,
findTranscriptionQueue, findReadyToReadQueue, findWeeklyStats) onto
DocumentService as 1-line delegations and update the consumer.
Refs #417 (C6.2 violation #5).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The Thumbnail trio (ThumbnailService, ThumbnailBackfillService,
ThumbnailAsyncRunner) all injected DocumentRepository directly. They now go
through three new DocumentService delegations:
- findById(UUID): Optional<Document> — no-throw variant for the runner's
log-and-skip behaviour on missing documents.
- findForThumbnailBackfill() — wraps the existing
findByFilePathIsNotNullAndThumbnailKeyIsNull query.
- updateThumbnailMetadata(Document) — wraps save() for the post-thumbnail
entity update.
DocumentService also gains @Lazy on its existing ThumbnailAsyncRunner field
to break the new DocumentService ↔ ThumbnailAsyncRunner cycle. lombok.config
adds @Lazy to copyableAnnotations so the field annotation reaches the
generated constructor parameter.
Refs #417 (C6.2 violations #2, #3, #4).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- PasswordResetService injects UserService instead of AppUserRepository.
- New UserService.findByEmailOptional preserves the silent-fail behaviour of
the old findByEmail-returning-Optional path; the existing throwing
findByEmail is unchanged.
- New PasswordResetService.findLatestActiveTokenForEmail exposes the latest
active reset token without leaking the repository upward.
- New @Profile("e2e") PasswordResetTestHelper wraps that read so the
AuthE2EController no longer touches PasswordResetTokenRepository directly.
Profile guard moves from the controller-only annotation to also cover the
helper bean, so the production graph never instantiates either.
Refs #417 (C6.1 violation #2 + C6.2 violation #12).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
StatsController previously injected PersonRepository and DocumentRepository
directly, violating the controller→service→repository layering rule. Move the
two count() calls into a thin StatsService that delegates to PersonService.count
and DocumentService.count. While here, add the missing @RequirePermission(READ_ALL)
flagged by AUDIT-2 §7 — anonymous callers were able to read aggregate document/
person counts.
Refs #417 (C6.1 violation #1).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Aligns the auth-account table name with the AppUser entity. The historical
mismatch (table 'users' alongside table 'persons') misled schema-first readers
into assuming the two were related; renaming to 'app_users' makes the
deliberate split between auth accounts and historical persons explicit at the
schema layer.
Scope: the table itself, the users_groups join table, and the three FK columns
whose name was literally 'user_id'. Semantic FK columns (audit_log.actor_id,
notifications.recipient_id, document_versions.editor_id, etc.) keep their
names — the role they describe is the documentation, not the type.
Closes#418. Unblocks #407 (REFACTOR-1).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Extract repeated `new java.util.HashSet<>(Set.of(TrainingLabel.KURRENT_RECOGNITION))`
into a `kurrentLabels()` helper in TrainingBlockQueryTest and add `import java.util.HashSet`.
Add clarifying comments on the two person-scoped queries in TranscriptionBlockRepository
explaining that they use `MEMBER OF d.trainingLabels` — aligned with the pre-existing
`findEligibleKurrentBlocks()` pattern.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
scriptType is only set after OCR runs, which can't happen before we have
a trained model. Both sender-based queries now filter on the training label
instead, consistent with findEligibleKurrentBlocks.
Also adds missing test coverage for findManualKurrentBlocksByPerson and
countManualKurrentBlocksByPerson (4 cases + count parity check).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GET /api/geschichten now accepts repeated personId query params and
returns only stories that mention every person supplied. Refactors the
list path to a JPA Specification chain (one EXISTS subquery per id,
mirroring DocumentSpecifications.hasTags) and embeds the
COALESCE(publishedAt, updatedAt) DESC ordering inside the spec so a
single repository.findAll covers all filter combinations.
Without this, the Geschichten feature ships dark on prod day-one — no group
holds BLOG_WRITE, so the editor controls never render even for admins. The
mapping "anyone who can write documents can also author family stories" is
the safest default and admins can revoke afterwards via the new checkbox UI.
Closes Tobias's review S5 on PR #382.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The end-to-end test creates a DRAFT, verifies it is hidden from a READ_ALL
reader (list and getById), publishes it, verifies the reader sees it, then
deletes it and confirms the join rows go with it but the linked Person
remains. Also corrects the V58 author FK to reference the actual users
table (not app_users).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
GET endpoints are open to authenticated users (the service layer enforces
DRAFT visibility). POST/PATCH/DELETE require @RequirePermission(BLOG_WRITE).
WebMvcTest slice covers 401/403/200/201/204 paths.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
DRAFT stories are 404 to readers without BLOG_WRITE (NOT_FOUND, not FORBIDDEN,
to avoid leaking existence). list() forces status=PUBLISHED for non-writers
even when they pass status=null. Body HTML is sanitised via OWASP allow-list
(p, br, strong, em, h2, h3, ul, ol, li) on every save. publishedAt is set on
every transition into PUBLISHED and cleared on retract.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
GeschichteRepository.search filters by status / personId / documentId in a
single JPQL query so the controller can serve the index page, the person
discovery card, and the document drawer column from one method. The DTO is
shared between create and update like DocumentUpdateDTO.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Geschichte holds family memory stories (issue #381). Body is unbounded TEXT
(Tiptap HTML, no length limit). Two join tables link a story to historical
Persons and Documents. A partial index speeds the public index query
(status='PUBLISHED' ORDER BY published_at DESC) and reverse-lookup indexes
support the ?personId and ?documentId filters.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Foundation for the Geschichten (story) domain (issue #381). BLOG_WRITE gates
authoring of family memory stories; GESCHICHTE_NOT_FOUND is also returned for
DRAFTs requested by users without BLOG_WRITE so existence is not leaked.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds OWASP Java HTML Sanitizer on the backend and DOMPurify on the frontend.
Together with Tiptap on the writer side they form a defense-in-depth chain
against XSS in the new Geschichte body field (issue #381).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
TranscriptionService.updateBlock was not writing mentionedPersons from the DTO
back to the entity, so @mentions were lost on every save. Clear-then-addAll
pattern avoids Hibernate orphan issues with @ElementCollection.
Switch @ElementCollection fetch to EAGER so callers can read mentionedPersons
outside an active transaction without a LazyInitializationException.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PersonMentionPropagationListener rewrites @DisplayName tokens on person rename.
Under the new design, displayName is archival (what the transcriber typed), so
the listener would corrupt transcriptions rather than correct them.
Deletes PersonMentionPropagationListener, PersonDisplayNameChangedEvent, and the
optimistic-lock catch path in PersonService.updatePerson. Removes PERSON_RENAME_CONFLICT
from ErrorCode and all tests that exercised the now-deleted code path.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extracts the Pattern+Matcher+replaceAll block into a private helper so the
loop body reads as three lines: rewrite text, update sidecar entries, nothing
else. Moves the boundary-condition rationale comment to the helper.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
createBlock has both validation guards (displayName length + personId null).
updateBlock had only the displayName test. Add the symmetric null-personId case
so a future @Valid drop from updateBlock's @RequestBody would be caught.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Method said inUnderTwoSeconds; assertion checks isLessThan(5000L) with message
"5s". Three sources of truth, three different values. Rename aligns method name
with the assertion that was intentionally raised from 2s to 5s in a prior commit.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The listener exclusively calls findByPersonIdWithMentionsFetched (JOIN FETCH).
Zero callers exist in production or test code. Leaving it is a maintenance
trap: a future caller would silently trigger N+1 loads on the lazy collection.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PersonServiceTest wired the mock on findByMentionedPersons_PersonId; the listener
now calls findByPersonIdWithMentionsFetched so the mock returned an empty list,
suppressing the saveAllAndFlush call and breaking the exception-propagation test.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2s was generous for correctness but tight for a shared VPS-hosted CI runner
(cold JVM, Testcontainers startup, competing processes). 5s still catches
O(n²) regressions and N+1 queries while eliminating flaky failures.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
updatePerson_doesNotPublishEvent_whenOnlyAliasChanges implied that alias is
processed by updatePerson — it isn't. The invariant is that the event is
suppressed when title/firstName/lastName are all unchanged regardless of
which non-displayName field changed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add updatePerson_returns409_whenRenameConflict to PersonControllerTest: exercises
the full controller→exception-handler path, not just the service layer. Verifies
HTTP 409 + $.code = PERSON_RENAME_CONFLICT when updatePerson throws a conflict.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switch from findByMentionedPersons_PersonId (derived query, returns blocks with
LAZY mentionedPersons) to findByPersonIdWithMentionsFetched (JOIN FETCH, loads
full collections in one round-trip). 200-block propagation: from 201 queries to 2.
Add @Transactional comment documenting join-transaction semantics.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add findByPersonIdWithMentionsFetched to TranscriptionBlockRepository: subquery
finds blocks referencing the renamed person, outer JOIN FETCH loads their full
mentionedPersons collection. Avoids N+1 lazy selects in the propagation listener.
Filtered JOIN FETCH (WHERE m.personId=:personId) was rejected — it loads only one
mention entry per block, risking data loss on saveAllAndFlush.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Felix self-review / Sara (PR #366 review). The trailing-`List.of()` pattern
introduced when mentionedPersons was added to the DTOs is brittle: every
future field forces another grep-and-edit pass across this file. Switch
the 8 call sites (1 Create, 7 Update) to .builder() so the test only
specifies the fields it cares about — future DTO growth is invisible to
tests that don't touch the new field.
Refs #362#366
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sara #4 (PR #366 review). The 400-on-201-chars regression guard previously
only covered POST /api/documents/{id}/transcription-blocks. The same @Valid
cascade applies to PUT /api/documents/{id}/transcription-blocks/{blockId}
via UpdateTranscriptionBlockDTO, but no test asserted it — meaning a
silent removal of @Valid on the PUT @RequestBody parameter would slip past
CI. Mirror the test for symmetry.
Refs #362#366
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Markus #6 (PR #366 review). The class lives in service/ and is service-tier
business logic — wire-by-stereotype consistency calls for @Service. Both
annotations participate in @ComponentScan equivalently, so the bean
registration is unchanged.
Refs #362#366
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sara #3 / Felix #5 (PR #366 review). The previous version stubbed
eventPublisher.publishEvent to throw, which proved the catch-and-translate
syntax but skipped the listener entirely. The test could not have detected
a regression where the listener swallowed the exception or re-wrapped it
with a non-OptimisticLocking type.
Replace with a real PersonMentionPropagationListener instance backed by a
mocked TranscriptionBlockRepository whose saveAllAndFlush throws
ObjectOptimisticLockingFailureException (the actual Spring exception
Hibernate raises). The publisher mock routes the event to the real
listener via doAnswer so the call chain is the production one:
PersonService.updatePerson → publishEvent → listener.onPersonDisplayNameChanged
→ blockRepository.saveAllAndFlush throws → exception bubbles through the
synchronous event dispatcher → PersonService catches → DomainException.
Refs #362#366
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Felix #2 / Markus #1 (PR #366 review). In the synchronous-transactional
path the existsById check could never return false — the rename and the
propagation share one transaction, so the renamed Person is guaranteed to
still exist when the listener runs. The check was forward-protection for
an eventual @Async refactor but its presence today is misleading: it
suggests a runtime branch that no test could reach against the real flow.
Delete the call, drop the PersonService dependency from the listener, drop
the now-unused PersonService.existsById, and remove the orphan-guard test
(it asserted a behaviour that the synchronous path cannot produce). When
async is added later the guard re-enters the codebase deliberately as part
of that refactor.
Refs #362#366
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Felix #1 / Markus #5 / Sara #1 (PR #366 review). The naive
text.replace("@" + old, "@" + new) silently corrupted any composite mention
that began with the renamed single-name person — e.g. renaming the
single-name "Hans" turned "@Hans Müller" into "@Henry Müller", obliterating
the historical reference to Hans Müller without warning.
Replace with a regex matching "@OldName" only at a token boundary: not
followed by a letter/digit/hyphen (catches @Hans-Peter) and not followed by
"<space><uppercase>" (catches @Hans Müller). False negatives — e.g.
sentence-initial "@Hans Bekam" — are accepted as the conservative
trade-off; corruption is irrecoverable, missed renames are not.
The new failing test reproduced the reviewer scenario exactly: two persons
("Hans Müller" + single-name "Hans"), one block referencing both, rename
Hans → Henry. Pre-fix output corrupted "@Hans Müller" to "@Henry Müller";
post-fix preserves the composite mention and only updates the standalone.
The existing partial-name guard test (Hans-Peter Müller / Hans Müller) and
multiple-occurrences test still pass — the regex is a strict superset of
the boundary constraints already covered.
Refs #362#366
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Defense in depth: until now both list and single-person reads only required
authentication, while the write endpoints (POST/PUT/DELETE) were already
gated with @RequirePermission. The hover-card and typeahead introduced in
issue #362 expose person details (life dates, notes, family relationships)
to anyone who can authenticate — adding READ_ALL aligns the GETs with the
write endpoints and matches the access tier already enforced for documents
and transcription blocks.
Two new controller-slice tests assert 403 when an authenticated user lacks
READ_ALL; existing 200-path tests now stipulate `authorities = "READ_ALL"`
explicitly.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Latency floor (Sara): a merge-blocking regression check, not a benchmark.
Seeds 200 blocks each with one mention of the same person, fires the rename,
and asserts the listener completes the entire find/mutate/saveAllAndFlush
cycle in less than two seconds against the Testcontainers Postgres.
Confirms the partial reload (one Auguste → Augusta) actually persisted so
the timing isn't measuring an empty path.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
When the propagation listener saves blocks with a stale @Version (because
another transcriber's autosave incremented version mid-rename), Hibernate
raises ObjectOptimisticLockingFailureException — Spring's translation of
the underlying JPA exception. PersonService.updatePerson now wraps the
publishEvent call in a catch for OptimisticLockingFailureException and
re-throws as DomainException(PERSON_RENAME_CONFLICT, 409). The whole
@Transactional boundary still rolls back, but the client gets a structured
409 with the localised "please retry" message instead of a generic 500.
The listener was switched from saveAll to saveAllAndFlush so the conflict
fires inside the listener call (where the catch can see it), not at
transaction commit (which is too late for in-method handling).
Test stubs the eventPublisher to throw OptimisticLockingFailureException
and asserts the translated DomainException carries PERSON_RENAME_CONFLICT
and HTTP 409. End-to-end DB-level reproduction of the JPA optimistic-lock
race requires multi-threading or two physical connections, which is
impractical inside @DataJpaTest; the underlying JPA mechanism is well
covered by Hibernate's own test suite.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds the structured error code returned when a rename rolls back because a
referenced transcription block was edited concurrently (OptimisticLockException
on transcription_blocks.version). Mirrors the contract in
frontend src/lib/errors.ts and adds the localised message keys
error_person_rename_conflict in de/en/es so the UI surfaces a retry hint
instead of a generic 500.
The actual translation of OptimisticLockException → DomainException
(PERSON_RENAME_CONFLICT) lands in the next commit alongside the integration
test that proves the rollback semantics.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A block with a sidecar entry pointing at a personId no longer in the
persons table receives a rename event for that ghost id. The listener
detects via PersonService.existsById that the entity is gone and exits
without touching block.text or the sidecar. Defends against any future
async refactor where an event could outlive the entity, or against
malformed events injected by tests / migrations.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
When the same person is mentioned twice in one block, both substrings flip
to the new display name. String.replace(String, String) is documented to
replace every occurrence, but a future regex-based refactor or a typo could
silently regress to first-match-only — this test guards against that.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Block contains both @Hans-Peter Müller and @Hans Müller; the listener fires
a rename for Hans Müller → Hans Schmidt. The simple replace("@" + old,
"@" + new) hinges on the leading @-and-space anchor: "@Hans Müller" does
not appear inside "@Hans-Peter Müller" (hyphen interrupts), so only the
standalone mention rewrites. Sidecar mirrors the same — Hans Müller's
entry flips to Hans Schmidt while Hans-Peter Müller's entry is preserved.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Save a block with no sidecar entries, fire a rename event for an unrelated
person, and assert the block reloads with its original text and empty
sidecar. Confirms findByMentionedPersons_PersonId returns an empty list and
the saveAll path does not accidentally touch unrelated rows.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Synchronous @EventListener consumer of PersonDisplayNameChangedEvent.
Finds every block whose sidecar references the renamed person via the
derived query, replaces "@OldName" with "@NewName" inside block.text, and
updates the matching PersonMention.displayName in the sidecar list. saveAll
in one batch; SLF4J info log records the audit line.
Synchronous on purpose: the rename and the propagation must commit as one
transaction so a half-applied rewrite never reaches the archive. If the
archive grows past tens of thousands of blocks, switch to
@TransactionalEventListener(AFTER_COMMIT) + @Async.
Adds PersonService.existsById to give the listener a layered way to verify
the personId still corresponds to a real Person — defensive guard for any
future async refactor where an event could outlive the entity. The check
goes through PersonService rather than PersonRepository to honour the
"services never reach into another domain's repository" rule.
Happy-path @DataJpaTest + Testcontainers asserts a single-block, single-
mention rewrite mutates both the text and the sidecar entry. blockRepository
.flush() is called explicitly so saveAll is committed before em.clear() —
in production the surrounding @Transactional flushes on commit; in test we
substitute by flushing manually.
Implements PR-A tasks 13 and 15 as one red→green cycle.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Spring Data resolves the method name to a join over
transcription_block_mentioned_persons, returning every block whose sidecar
contains the given personId. The B-tree index on person_id (V56) keeps the
lookup O(log n) — required for the rename propagation that fans out to
every block referencing the renamed person, and for the future
"show all blocks mentioning person X" query on the person detail page.
The underscore between MentionedPersons and PersonId is the explicit
property-boundary form, immune to ambiguous longest-match parsing if the
embeddable later gains another nested object.
Refs #362
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
