Testcontainers integration test persisting a RANGE doc with end == start
against real Postgres + Flyway, which (unlike H2) enforces the V69
chk_meta_date_end_after_start CHECK. Pins the app guard's isBefore
semantics to the actual >= constraint, guarding against app/DB drift (AC2).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add @ExceptionHandler(DataIntegrityViolationException) returning 400
VALIDATION_ERROR with a fixed constant message, so any integrity violation
that slips past the upstream guards (a future constraint, or the import
path) becomes a clean 400 instead of a 500 + Sentry alert (AC9).
Deliberately generic — it does not inspect which constraint failed. Never
echoes ex.getMessage() (constraint name + SQL, CWE-209), logs at WARN
without passing the exception (would re-leak the SQL to Loki), and does not
call Sentry.captureException.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the second validateDateRange predicate mirroring
chk_meta_date_end_only_for_range, so a direct API client that sets an end
date without RANGE precision gets a clean 400 INVALID_DATE_RANGE instead of
a 500 (AC6). Shares the code with the end-before-start branch.
Also fix updateDocument_preservesStoredPrecision_whenDtoOmitsIt: its stored
fixture (MONTH + end date) is a state the DB CHECK forbids, so the
carried-over-state guard correctly rejects it. Switched to RANGE + end —
the only DB-valid non-null-end combo — preserving the test's intent.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover AC2 (end == start), AC3 (open-ended, end null) and AC4 (null start +
end set, which must not reject or NPE), plus end-after-start. Guards the
guard against future over-rejection that would diverge from the DB CHECK.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add ErrorCode.INVALID_DATE_RANGE and a validateDateRange guard on
DocumentService.updateDocument, run right after applyDatePrecision so it
fires before any save (updateDocumentTags persists earlier in the method).
Mirrors the V69 chk_meta_date_end_after_start CHECK: end >= start with a
null start allowed, using isBefore so equal dates stay valid. Turns a user
date typo into a clean 400 instead of a 500 + Sentry alert.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The /search path already pins the Boolean-undated->primitive coercion via
search_withoutUndatedParam_forwardsFalseToService; add the symmetric pin for
getDocumentIds so an absent param provably resolves to undated=false on the
record (never NPE). Raised in the #702 review.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Clarify at loadFilteredDates why the density path constructs a SearchFilters:
the two filter records are kept separate (density has no date/undated fields),
so it adapts here to reuse buildSearchSpec. Raised in the #702 review.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the ~29 repeated `new SearchFilters(null, null, null, null, null,
null, null, null, null, false)` literals across the search test suites with
a shared SearchFiltersFixtures.noFilters() factory (and noFilters()
.withUndated(true) for the undated-only case). Tests that pin a specific
field keep their explicit `new SearchFilters(...)` so intent stays visible.
Pure test-ergonomics cleanup raised in the #702 review; no behaviour change.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the long positional filter lists on the document search chain
with the SearchFilters record. searchDocuments now takes
(SearchFilters, DocumentSort, String dir, Pageable) and findIdsForFilter
takes a single SearchFilters; the four private helpers (buildSearchSpec,
runSearch, countUndatedForFilter, isPureTextRelevance) no longer carry a
positional 10-field filter list. The controller builds the record after
its existing tagOp/undated coercions; the density path adapts its
DensityFilters into a SearchFilters at the shared buildSearchSpec call.
The forced-undated count path is preserved via filters.withUndated(true),
so countUndatedForFilter still ignores the user's toggle (#668) while
runSearch honours it. No behaviour change.
Controller binding tests swap their positional any()/eq() matchers for
ArgumentCaptor<SearchFilters>, asserting captured.undated()/.status()/
.sender() — strictly stronger than the previous any()-soup.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Filter-only value object bundling the ten search predicates so the long
positional argument lists on the document search chain can be replaced
with one named record — killing the sender/receiver and from/to swap-bug
class. Mirrors the existing DensityFilters; carries a withUndated copy
accessor for the forced-undated count path. Unused as of this commit.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Round out the "read-only users can't write anything" boundary: a READ_ALL
principal is forbidden from posting a block comment, replying, and editing a
comment (the prior tests only used a no-authority principal for create).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the hasTranscription existence query out of the shared getDocumentById
into a dedicated getDocumentDetail used solely by GET /api/documents/{id}.
The flag is only consumed by the detail page, so the extra EXISTS query no
longer runs for the many internal getDocumentById callers (e.g. the
Geschichte resolve loop and the dashboard resume path). Behaviour of the
detail endpoint is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Read-only users will soon be able to open the transcription read view, so
the write endpoints become the real authorization boundary. Explicitly
assert a READ_ALL-only principal is forbidden from create/update/reorder/
review block writes and annotation create/patch (the prior tests only used
a no-authority principal).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
getDocumentById now populates a transient hasTranscription boolean so the
document detail page can gate the transcription entry control at first
paint (no client store, no full block fetch, no layout shift).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Domain-service wrapper over existsByDocumentId so other domains can ask
"does this document have any transcription blocks?" without reaching into
the repository.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cheap EXISTS query backing a server-side "has a transcription" signal so
read-only users can be offered the read view at first paint.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address review nit: the older getTagTree tests relied on Mockito's default
empty-list return for findSubtreeDocumentCountsPerTag. Stub it explicitly so
the two-query contract is self-documenting.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Record that getTagTree returns both documentCount (direct, read by admin
surfaces) and subtreeDocumentCount (rollup, read by the reader surfaces),
matching the corrected getTagTree JavaDoc.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover AC#1-4 (leaf=direct, distinct overlap counted once, full descendant
depth), REQ-THEMEN-05 (empty subtree absent), REQ-THEMEN-06 (cycle terminates
via the 50-level guard) and AC#7 (rollup equals distinct documents found by the
real tag-search expansion — count↔destination parity). Testcontainers
postgres:16-alpine since the recursive CTE + COUNT(DISTINCT) needs real PG.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add subtreeDocumentCount to TagTreeNodeDTO, populated by a new recursive-CTE
aggregate query that builds a tag closure and counts distinct documents per
ancestor subtree. The direct documentCount is unchanged; getTagTree now maps
both counts onto each node from two aggregate queries (no N+1).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Hiding the header upload button is UI polish; the real control is endpoint
authz. Add explicit READ_ALL-only 403 boundary tests for POST /api/documents
and POST /api/documents/quick-upload, matching the reader-only convention
already used elsewhere in this suite.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sara's QA concerns:
1. PersonControllerTest.updatePerson_returns200_whenGenerationNull was
asymmetric — only checked status 200, no body assertion. Now also
asserts `$.generation` is null in the JSON response, mirroring the
in-range test's body check.
2. New full-stack PUT→DB→GET round-trip in PersonServiceIntegrationTest
(updatePerson_clearGenerationToNull_readsBackNullFromDb) seeds a
person with generation=3, calls updatePerson with generation=null,
flushes the persistence context, and asserts the column reads back
null from the DB. Without this we only had the mocked WebMvcTest
boundary; nothing proved JPA actually wrote SQL NULL.
3. Sibling test (updatePerson_setGenerationToZero_readsBackZeroFromDb)
pins the G 0 end-to-end so a primitive zero can't silently coerce
to null anywhere along controller → service → JPA.
Refs #689
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Markus flagged the 0/10 range was duplicated across five sites (DB
CHECK, both importers, DTO @Min/@Max, dropdown range). New
PersonGeneration.MIN_GENERATION / MAX_GENERATION constants are now
the canonical Java source; the DTO annotations and both importer
guards reference them. The V70 SQL CHECK comment now points at the
Java constants so future widening updates one Java class plus one
SQL literal (Flyway forbids rewriting the migration in place).
Refs #689
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Inject RelationshipService into CanonicalImportOrchestrator and walk
PARENT_OF edges in the family network after both person loaders finish
(before documents). For every edge where child.generation is set and
not strictly deeper than parent.generation, log a WARN — soft check,
never fails the batch.
Reads through getFamilyNetwork() per the layering rule (orchestrator
never touches PersonRelationshipRepository directly). Curators see the
warning in the import log; the rest of the pipeline is unaffected so
data with curatorial gaps still loads cleanly.
Refs #689
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PersonNodeDTO is a positional record. The optional Integer generation
field is inserted between deathYear and familyMember so all four
construction sites stay readable without a builder.
- RelationshipService.getFamilyNetwork → populates with
person.getGeneration() (the Stammbaum's strict-rank source on the
frontend).
- RelationshipInferenceService.findAllFor → populates the same way;
inference UI does not consume it but the field travels along for
consistency.
- RelationshipControllerTest fixtures pass null.
Refs #689
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reads the optional `generation` integer from the canonical tree JSON and
routes it into PersonUpsertCommand. Out-of-range values are skip-and-
warned with the same policy as the register importer.
Tree imports run after register (per CanonicalImportOrchestrator); a
tree-confirmed integer overwrites a register-parsed value — both sides
are "canonical" in preferHuman terms (neither is a human edit).
Refs #689
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reads the optional `generation` cell by header name (REQUIRED_HEADERS is
not extended — REQ-IMP-001 backward-compat for older artifacts), parses
it through GENERATION_PATTERN (^\s*G?\s*(-?\d+)), and routes it into
PersonUpsertCommand.generation.
Out-of-range values (G 99, G -1) are skip-and-warned, never abort the
batch; the post-parse range guard mirrors the V70 CHECK constraint so
the DB never sees a value Bean Validation wouldn't accept.
Pinned with a parametrised CsvSource covering every shape from the
acceptance criteria plus a backward-compat test (artifact without a
generation column still imports, all upserts get generation=null).
Refs #689
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- fromCanonical writes the imported generation into a new Person row.
- mergeCanonical routes existing/canonical generation through the
existing preferHuman(Integer, Integer) overload so a human-edited
value is never overwritten on re-import (ADR-025).
- updatePerson writes generation verbatim from the form DTO so a human
can clear it back to null — same shape as birthYear/deathYear.
- createPerson(PersonUpdateDTO) writes generation so /persons/new flow
doesn't silently drop a selected G value on create.
Pinned with five tests covering the four write paths plus the
documenting test that captures preferHuman's known limitation
(explicit human null is overwritten by a non-null canonical value —
same as birthYear/deathYear, deferred to a future helper rework if it
ever produces a user-visible bug).
Refs #689
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds the optional generation field to both DTOs:
- PersonUpsertCommand gains Integer generation in the canonical-import
builder chain; service wiring lands in the next commit.
- PersonUpdateDTO gains @Min(0)@Max(10) Integer generation, the form-path
surface. The constraints mirror the V70 CHECK so validation fails fast
at the controller before reaching the DB.
PersonControllerTest pins the validation behaviour: -1 → 400, 11 → 400,
null → 200, 3 → 200 for both PUT (update) and POST (create) paths. The
GlobalExceptionHandler maps MethodArgumentNotValidException to
VALIDATION_ERROR so the frontend's extractErrorCode keeps working.
Refs #689
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Flyway V70: SMALLINT generation column with CHECK(0..10) and partial
index over non-null rows. Person.generation field surfaces it through
the JPA model. Pre-import rows and persons outside the curated family
graph legitimately stay null; the canonical importer (next commits)
back-fills via preferHuman so a human-edited value is never lost.
Refs #689
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
addRelationship now auto-flips family_member=true on both endpoints for
PARENT_OF/SPOUSE_OF/SIBLING_OF (commit 07300aef). That side-effect breaks
the pre-condition assertion in setFamilyMember_true_makes_person_appear_in_network,
which expects charlie not to appear in the network before the explicit flip.
Reset charlie's flag after addRelationship so the test still exercises the
setFamilyMember(true) -> network presence path it was written for.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
DocumentImporter exposed a package-private openFileStream(File) so a
Mockito spy could force the IO-error branch of isPdfMagicBytes. The
test-only seam leaked into production: the method existed for testing,
not for any production extensibility.
Replace with a constructor-injected FileStreamOpener interface (single
abstract method, @FunctionalInterface) and a one-line
@Component DefaultFileStreamOpener delegate. Tests now inject a mock
opener instead of spying on the importer itself, which is also a more
idiomatic Mockito usage.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
resolveReceivers passed the slug as both `sourceRef` AND `lastName`, so
an unresolved receiver "smith-john" became a provisional Person with
lastName="smith-john" — a regression of the existing senderName→Person
contract.
Fix: zip the parallel `receiver_person_ids` and `receiver_names`
columns by position (the normalizer emits them 1:1 like
sender_person_id/sender_name). When the names list is shorter than the
slugs list, fall back to slug-as-name for the missing entries.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
buildDocument was a ~30-line method mixing attribution routing, date
parsing, authoritative collection management, file metadata, and
computed flags. Split into five named helpers — applyAttribution,
applyDates, applyAuthoritativeAssociations, applyFileMetadata,
applyComputedFlags — each doing one job. Pure refactor; all 43 existing
DocumentImporterTest cases still pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The canonical importer creates persons via PersonRegisterImporter first (no family_member
set) and then upserts them via PersonTreeImporter, but mergeCanonical never propagates
family_member to existing persons — so persons with imported relationships ended up
flagged family_member=false and never appeared in /api/persons family filters or the
family-network view.
RelationshipService is documented as the owner of the family_member flag, so the fix
lives there: addRelationship now sets family_member=true on both endpoints whenever the
relation type is PARENT_OF / SPOUSE_OF / SIBLING_OF (the same set getFamilyNetwork
filters by). Non-family types (FRIEND/COLLEAGUE/EMPLOYER/DOCTOR/NEIGHBOR/OTHER) leave
the flag alone — a family doctor isn't a family member. Extracted the type list as a
FAMILY_RELATION_TYPES constant and reused it in getFamilyNetwork for a single source of truth.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address PR #687 review concerns on DocumentImporterTest:
- Sara/Felix: add catalog-shape reject tests that pass every char
pre-check but must fail INDEX_PATTERN — "J 0070" (space), "WXYZA-0001"
(5 letters), "12-0001" (no letter prefix), "W-0001X" (uppercase X).
Verified red against a weakened pattern, green against the real one,
so the pattern branch (not the char guards) is now pinned.
- Felix: restore the import java.io.OutputStream line (was over-deleted
and patched with a fully-qualified name).
- Sara: document why the resolvePdfByIndex getCanonicalPath IOException
branch is intentionally left uncovered (no deterministic injection
seam; the log.warn is the substantive fix).
Adjust the two reflective resolvePdfByIndex calls for the new rowNumber
parameter.
Refs #686
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address PR #687 review concerns on DocumentImporter:
- Tobias: thread a 1-based source row number into importRow so the
"index rejected" skip log carries a breadcrumb (the row number, never
the raw hostile index) for post-import triage.
- Elicit: emit a distinct log when a valid index has no <index>.pdf on
disk (normal PLACEHOLDER) so it is not conflated with a rejected index.
- Nora: add a log.warn in resolvePdfByIndex's getCanonicalPath IOException
branch so the quiet fail-safe skip surfaces in ops, distinct from the
deliberate symlink-escape abort.
- Felix: replace inline fully-qualified java.util.regex.Pattern with an
import.
- Nora: document that \d is intentionally ASCII-only (do not add
UNICODE_CHARACTER_CLASS).
Refs #686
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The corpus is uniform — every PDF is <index>.pdf flat in the import
dir — so resolve a document's PDF with an O(1) importDir.resolve(index
+ ".pdf") lookup instead of a recursive directory walk over the file
column. The index is validated against a strict catalog pattern
(1–4 Latin letters incl. umlauts, hyphen(s), digits, optional x) plus
the ported separator/dot/dotdot/null/slash-homoglyph/absolute-path
guards, and the resolved canonical path is asserted to stay inside the
import dir as defense-in-depth. The %PDF magic-byte check still gates
upload; status UPLOADED/PLACEHOLDER and the index→originalFilename
upsert key are unchanged. The file column and findFileRecursive walk
are gone, and the security regression tests now assert a malicious or
garbage index is rejected and a valid index resolves to exactly
importDir/<index>.pdf within containment.
Closes#686Closes#676
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The global undated-count rework moved the pure-text-RELEVANCE shortcut
into runSearch, where it ran after the unconditional
findAllMatchingIdsByFts call. That routed pure-text relevance through the
in-memory id path and returned empty match data, breaking FTS rank order
and snippet/offset enrichment.
Hoist the shortcut back to the top of searchDocuments so it short-circuits
to findFtsPageRaw before findAllMatchingIdsByFts, while still computing the
global undatedCount for all non-fast-path searches.
Refs #668
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Owner decision (#668): when two documents share a meta_date, order them by
title ascending instead of createdAt ascending. title is @Column(nullable=false)
so it is always present, giving a deterministic, human-meaningful total order.
Only the DATE-sort fast path changes; the in-memory SENDER/RECEIVER/RELEVANCE
comparators are untouched.
ORDER BY meta_date <dir> NULLS LAST, title ASC
Tests assert title-asc tiebreaking for same-date rows in BOTH directions, with a
fixture whose title order is the OPPOSITE of insertion (createdAt) order so the
test fails if the tiebreaker reverts to createdAt. The integration test drives
the production resolveSort against real Postgres.
Refs #668
The undated bucket count was page-local — derived from the year-grouping
of the current page's items, so it could never exceed the page size. The
owner's decision is for it to reflect ALL undated documents matching the
active filter across every page.
Add an undatedCount field to DocumentSearchResult, computed once per search
via a COUNT over the same filter spec with undatedOnly(true) forced —
independent of the "Nur undatierte" toggle so it never collapses to the
page slice or double-counts. A from/to range excludes undated rows by the
collision rule, so the count is legitimately 0 inside a date range.
Refs #668
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the single-sender containsExactlyInAnyOrder check with a two-sender
fixture and ordered containsExactly proving an undated doc stays within its
sender group and never floats to the page head. Add a DESC-direction case for
in-memory-path symmetry and an undated=true + sort=SENDER case capturing the
Specification to prove undatedOnly is still applied on the person-sort path.
Refs #668
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
No test calls resolveSort directly — the sort tests assert through
searchDocuments + ArgumentCaptor<Pageable>, so the package-private widening
added no value. Narrow the API surface back to private.
Refs #668
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds an optional `undated` query param to GET /api/documents/search and
/api/documents/ids, threaded through searchDocuments and findIdsForFilter
into the shared buildSearchSpec via undatedOnly(boolean). undated=true also
bypasses the pure-text RELEVANCE SQL shortcut, which skips buildSearchSpec
and would otherwise drop the predicate. The read GET stays unguarded
(WebMvc authz test pins 200 for an authenticated user, 401 unauthenticated).
A locking test proves the in-memory SENDER sort keeps undated letters under
their sender.
Refs #668
undatedOnly(false) is a no-op (null predicate); undatedOnly(true) returns
documentDate IS NULL, matching the existing hasStatus null-as-no-op pattern.
Real-Postgres tests pin the load-bearing guarantees H2 cannot prove: ASC
NULLS-LAST ordering, BETWEEN excludes null-dated rows, and that undated=true
combined with a from/to range returns empty (the collision rule).
Refs #668
resolveSort produced Sort.by(direction, "documentDate") with NATIVE null
handling, so Postgres surfaced undated (null meta_date) documents FIRST on
an ASC sort. Apply nullsLast() so undated rows order last for both ASC and
DESC, with a createdAt-asc tiebreaker for a stable total order when every
row is null-dated (the upcoming "Nur undatierte" filter).
Refs #668
Add countByFilter parity coverage for the query (LIKE) path so the shared
FILTER_WHERE slice and count can't drift, and an integration test proving
deletePerson detaches a person referenced as both sender and receiver before
delete — the documents survive (sender nulled, receiver link removed) with no
FK orphan.
Refs #667
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The legacy sort=documentCount path wrapped its result with paged(top, 0,
safeSize, top.size()), so totalElements/pageSize looked like a paged slice of
a larger set when in fact the top-N query returns the complete result. Add a
dedicated PersonSearchResult.topN factory that reports reality — totalElements
= returned count, pageSize = that count, totalPages = 1 (0 when empty) — and
pin both the populated and empty semantics with controller tests.
Refs #667
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
GET /api/persons now returns PersonSearchResult with server-side filter params
(type, familyOnly, hasDocuments, provisional) and page/size bounds (@Min/@Max
-> 400). review=true drops the clean reader default. The legacy
sort=documentCount top-N path is folded into the paged contract. Add
PATCH /{id}/confirm and DELETE /{id}, both WRITE_ALL-guarded. Remove the now
unreachable PersonService.findAll(String).
BREAKING-CHANGE: GET /api/persons response shape changes from a bare list to
PersonSearchResult { items, totalElements, pageNumber, pageSize, totalPages }.
Refs #667
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
