From 20cfe41f21f17e62ff3920a3b545926d8e71b0d5 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 6 Jun 2026 12:50:21 +0200 Subject: [PATCH 1/4] fix(person): resolve case-colliding aliases without throwing (#731) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findOrCreateByAlias resolved via Optional findByAliasIgnoreCase, which throws NonUniqueResultException once two aliases collide only by case (müller / Müller) — a generic 500 on the importer path. Mirror the #730 tag fix: resolve exact-case first, then the lowest-id case-insensitive sibling, then create-when-absent (institution/group and maiden-name alias preserved). The throwing Optional<…>IgnoreCase variant is deleted so it can't be reused. Co-Authored-By: Claude Opus 4.8 --- .../person/PersonRepository.java | 12 +++- .../familienarchiv/person/PersonService.java | 57 +++++++++------ .../person/PersonRepositoryTest.java | 28 +++++--- .../person/PersonServiceIntegrationTest.java | 29 ++++++++ .../person/PersonServiceTest.java | 72 +++++++++++++++---- 5 files changed, 151 insertions(+), 47 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java index 0ca82751..841d8f38 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java @@ -29,8 +29,16 @@ public interface PersonRepository extends JpaRepository { // Stammbaum-Knoten: alle Personen mit family_member = true. List findByFamilyMemberTrueOrderByLastNameAscFirstNameAsc(); - // Lookup by full alias string, used during ODS mass import - Optional findByAliasIgnoreCase(String alias); + // Exact-case alias lookup — the first resolution step in findOrCreateByAlias. + // Case-colliding aliases across persons (müller / Müller) are valid human labels, NOT + // duplicates: source_ref is the stable identity (ADR-025/032), alias is editable. Do NOT + // add a unique(lower(alias)) constraint — see ADR-032. + Optional findByAlias(String alias); + + // Plural case-insensitive alias lookup — the fallback step. Returns ALL case-folding + // siblings so the service can pick a deterministic one (lowest id) instead of letting a + // derived Optional<…>IgnoreCase throw NonUniqueResultException. See ADR-032. + List findAllByAliasIgnoreCase(String alias); // Lookup by the normalizer person_id, used for idempotent canonical re-import (Phase 3). Optional findBySourceRef(String sourceRef); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java index 1646c236..f0a682db 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java @@ -1,5 +1,6 @@ package org.raddatz.familienarchiv.person; +import java.util.Comparator; import java.util.List; import java.util.Optional; import java.util.UUID; @@ -125,32 +126,42 @@ public class PersonService { PersonType type = PersonTypeClassifier.classify(alias); if (type == PersonType.SKIP) return null; - return personRepository.findByAliasIgnoreCase(alias).orElseGet(() -> { - if (type == PersonType.INSTITUTION || type == PersonType.GROUP) { - return personRepository.save(Person.builder() - .alias(alias) - .lastName(alias) - .personType(type) - .build()); - } + // Aliases differing only by case (müller / Müller) are valid distinct persons, not + // duplicates, so resolution must never throw: exact-case first, then the lowest-id + // case-insensitive sibling, then create. Mirrors the tag path — see ADR-032. + Optional exact = personRepository.findByAlias(alias); + if (exact.isPresent()) return exact.get(); // exact-case wins + List caseInsensitive = personRepository.findAllByAliasIgnoreCase(alias); + if (!caseInsensitive.isEmpty()) { + return caseInsensitive.stream().min(Comparator.comparing(Person::getId)).orElseThrow(); // deterministic tie-break — list is non-empty, never throws + } - PersonNameParser.SplitName split = PersonNameParser.split(alias); - Person person = personRepository.save(Person.builder() + // Create-when-absent: institution/group keep the full label in lastName; a person name + // is split and a maiden name (geb. …) becomes a MAIDEN_NAME alias. + if (type == PersonType.INSTITUTION || type == PersonType.GROUP) { + return personRepository.save(Person.builder() .alias(alias) - .firstName(split.firstName()) - .lastName(split.lastName()) + .lastName(alias) + .personType(type) .build()); - if (split.maidenName() != null) { - int nextSortOrder = aliasRepository.findMaxSortOrder(person.getId()) + 1; - aliasRepository.save(PersonNameAlias.builder() - .person(person) - .lastName(split.maidenName()) - .type(PersonNameAliasType.MAIDEN_NAME) - .sortOrder(nextSortOrder) - .build()); - } - return person; - }); + } + + PersonNameParser.SplitName split = PersonNameParser.split(alias); + Person person = personRepository.save(Person.builder() + .alias(alias) + .firstName(split.firstName()) + .lastName(split.lastName()) + .build()); + if (split.maidenName() != null) { + int nextSortOrder = aliasRepository.findMaxSortOrder(person.getId()) + 1; + aliasRepository.save(PersonNameAlias.builder() + .person(person) + .lastName(split.maidenName()) + .type(PersonNameAliasType.MAIDEN_NAME) + .sortOrder(nextSortOrder) + .build()); + } + return person; } /** diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java index 21c24847..48f518d2 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java @@ -121,26 +121,34 @@ class PersonRepositoryTest { .containsExactly("Anna", "Clara"); } - // ─── findByAliasIgnoreCase ──────────────────────────────────────────────── + // ─── findByAlias (exact) / findAllByAliasIgnoreCase (case-folding siblings) ─── @Test - void findByAliasIgnoreCase_returnsMatchingPerson() { + void findByAlias_returnsExactCaseMatchOnly() { personRepository.save(Person.builder() .firstName("Karl").lastName("Brandt").alias("Opa Karl").build()); - Optional found = personRepository.findByAliasIgnoreCase("opa karl"); - - assertThat(found).isPresent(); - assertThat(found.get().getFirstName()).isEqualTo("Karl"); + assertThat(personRepository.findByAlias("Opa Karl")).isPresent(); + assertThat(personRepository.findByAlias("opa karl")).isEmpty(); // exact-case: a folded form does NOT match } @Test - void findByAliasIgnoreCase_returnsEmpty_whenAliasDoesNotMatch() { - Optional found = personRepository.findByAliasIgnoreCase("nobody"); - - assertThat(found).isEmpty(); + void findAllByAliasIgnoreCase_returnsEmpty_whenAliasDoesNotMatch() { + assertThat(personRepository.findAllByAliasIgnoreCase("nobody")).isEmpty(); } + @Test + void findAllByAliasIgnoreCase_foldsUmlautCase_inRealPostgres() { + // Proves Postgres LOWER() folds ü the same way for both rows — a plain-ASCII probe would + // stay green even if umlaut folding regressed. Both case-colliding aliases must match. + personRepository.save(Person.builder().lastName("Müller").alias("Müller").build()); + personRepository.save(Person.builder().lastName("müller").alias("müller").build()); + + assertThat(personRepository.findAllByAliasIgnoreCase("MÜLLER")).hasSize(2); + } + + // ─── findByFirstNameIgnoreCaseAndLastNameIgnoreCase ─────────────────────── + // ─── findByFirstNameIgnoreCaseAndLastNameIgnoreCase ─────────────────────── @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java index 66e0a93b..285d927a 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java @@ -20,6 +20,7 @@ import jakarta.persistence.EntityManager; import jakarta.persistence.PersistenceContext; import java.util.Set; +import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -75,6 +76,34 @@ class PersonServiceIntegrationTest { assertThat(result.getLastName()).isEqualTo("Cram"); } + // ─── #731: case-colliding alias resolution against real Postgres ─────────── + // The umlaut pair is mandatory — only the real DB proves Postgres LOWER() folds ü; a + // plain-ASCII test would stay green while umlaut aliases regressed. + + @Test + void findOrCreateByAlias_resolvesUmlautAliasCollision_toLowestId_withoutThrow() { + Person muller = personRepository.save(Person.builder().lastName("Müller").alias("Müller").build()); + Person mullerLower = personRepository.save(Person.builder().lastName("müller").alias("müller").build()); + UUID expected = muller.getId().compareTo(mullerLower.getId()) <= 0 ? muller.getId() : mullerLower.getId(); + + // No exact-case "MÜLLER" row → falls through to the case-insensitive branch with two + // candidates and must pick the lowest id, never throwing NonUniqueResultException. + Person resolved = personService.findOrCreateByAlias("MÜLLER"); + + assertThat(resolved.getId()).isEqualTo(expected); + } + + @Test + void findOrCreateByAlias_umlautAliasCollision_isDeterministicAcrossCalls() { + personRepository.save(Person.builder().lastName("Müller").alias("Müller").build()); + personRepository.save(Person.builder().lastName("müller").alias("müller").build()); + + Person first = personService.findOrCreateByAlias("MÜLLER"); + Person second = personService.findOrCreateByAlias("MÜLLER"); + + assertThat(second.getId()).isEqualTo(first.getId()); + } + // ─── #667: confirm round-trip + reader-default semantics ────────────────── @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java index e6a9035a..300681dd 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java @@ -375,14 +375,57 @@ class PersonServiceTest { // ─── findOrCreateByAlias ───────────────────────────────────────────────── @Test - void findOrCreateByAlias_returnsExisting_whenAliasFound() { - String alias = "Walter de Gruyter"; - Person existing = Person.builder().id(UUID.randomUUID()).alias(alias).build(); - when(personRepository.findByAliasIgnoreCase(alias)).thenReturn(Optional.of(existing)); + void findOrCreateByAlias_returnsExactCaseMatch_overCaseInsensitiveSibling() { + String alias = "müller"; + Person exact = Person.builder().id(UUID.randomUUID()).alias("müller").build(); + when(personRepository.findByAlias(alias)).thenReturn(Optional.of(exact)); Person result = personService.findOrCreateByAlias(alias); - assertThat(result).isEqualTo(existing); + assertThat(result).isEqualTo(exact); + verify(personRepository, never()).findAllByAliasIgnoreCase(any()); + verify(personRepository, never()).save(any()); + } + + @Test + void findOrCreateByAlias_returnsExactCaseMatch_evenWhenMultipleSiblingsCollide() { + String alias = "Müller"; + Person exact = Person.builder().id(UUID.randomUUID()).alias("Müller").build(); + when(personRepository.findByAlias(alias)).thenReturn(Optional.of(exact)); + + Person result = personService.findOrCreateByAlias(alias); + + assertThat(result).isEqualTo(exact); + // exact-case short-circuits — the case-insensitive siblings are never consulted. + verify(personRepository, never()).findAllByAliasIgnoreCase(any()); + } + + @Test + void findOrCreateByAlias_usesSingleCaseInsensitiveMatch_whenNoExactCase() { + String alias = "müller"; + Person only = Person.builder().id(UUID.randomUUID()).alias("Müller").build(); + when(personRepository.findByAlias(alias)).thenReturn(Optional.empty()); + when(personRepository.findAllByAliasIgnoreCase(alias)).thenReturn(List.of(only)); + + Person result = personService.findOrCreateByAlias(alias); + + assertThat(result).isEqualTo(only); + verify(personRepository, never()).save(any()); + } + + @Test + void findOrCreateByAlias_returnsLowestIdDeterministically_whenMultipleCaseInsensitiveMatches() { + String alias = "müller"; + Person lower = Person.builder().id(UUID.fromString("00000000-0000-0000-0000-000000000001")).alias("Müller").build(); + Person higher = Person.builder().id(UUID.fromString("00000000-0000-0000-0000-000000000002")).alias("müller").build(); + when(personRepository.findByAlias(alias)).thenReturn(Optional.empty()); + when(personRepository.findAllByAliasIgnoreCase(alias)).thenReturn(List.of(higher, lower)); // unordered + + Person first = personService.findOrCreateByAlias(alias); + Person second = personService.findOrCreateByAlias(alias); + + assertThat(first.getId()).isEqualTo(lower.getId()); // lowest id wins + assertThat(second.getId()).isEqualTo(first.getId()); // same result every call — never throws verify(personRepository, never()).save(any()); } @@ -390,7 +433,8 @@ class PersonServiceTest { void findOrCreateByAlias_createsNew_whenAliasNotFound() { String alias = "Clara Cram"; Person saved = Person.builder().id(UUID.randomUUID()).alias(alias).firstName("Clara").lastName("Cram").build(); - when(personRepository.findByAliasIgnoreCase(alias)).thenReturn(Optional.empty()); + when(personRepository.findByAlias(alias)).thenReturn(Optional.empty()); + when(personRepository.findAllByAliasIgnoreCase(alias)).thenReturn(List.of()); when(personRepository.save(any())).thenReturn(saved); Person result = personService.findOrCreateByAlias(alias); @@ -403,7 +447,8 @@ class PersonServiceTest { void findOrCreateByAlias_createsMaidenNameAlias_whenGebPresent() { String alias = "Clara Cram geb. de Gruyter"; Person saved = Person.builder().id(UUID.randomUUID()).alias(alias).firstName("Clara").lastName("Cram").build(); - when(personRepository.findByAliasIgnoreCase(alias)).thenReturn(Optional.empty()); + when(personRepository.findByAlias(alias)).thenReturn(Optional.empty()); + when(personRepository.findAllByAliasIgnoreCase(alias)).thenReturn(List.of()); when(personRepository.save(any())).thenReturn(saved); when(aliasRepository.findMaxSortOrder(saved.getId())).thenReturn(0); when(aliasRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); @@ -425,7 +470,8 @@ class PersonServiceTest { @Test void findOrCreateByAlias_setsInstitutionType_withFullNameInLastName() { String alias = "Arthur Collignon GmbH"; - when(personRepository.findByAliasIgnoreCase(alias)).thenReturn(Optional.empty()); + when(personRepository.findByAlias(alias)).thenReturn(Optional.empty()); + when(personRepository.findAllByAliasIgnoreCase(alias)).thenReturn(List.of()); when(personRepository.save(any())).thenAnswer(inv -> { Person p = inv.getArgument(0); p.setId(UUID.randomUUID()); @@ -442,7 +488,8 @@ class PersonServiceTest { @Test void findOrCreateByAlias_setsGroupType_withFullNameInLastName() { String alias = "Geschwister de Gruyter"; - when(personRepository.findByAliasIgnoreCase(alias)).thenReturn(Optional.empty()); + when(personRepository.findByAlias(alias)).thenReturn(Optional.empty()); + when(personRepository.findAllByAliasIgnoreCase(alias)).thenReturn(List.of()); when(personRepository.save(any())).thenAnswer(inv -> { Person p = inv.getArgument(0); p.setId(UUID.randomUUID()); @@ -460,7 +507,8 @@ class PersonServiceTest { void findOrCreateByAlias_noAlias_whenNoGeb() { String alias = "Clara Cram"; Person saved = Person.builder().id(UUID.randomUUID()).alias(alias).firstName("Clara").lastName("Cram").build(); - when(personRepository.findByAliasIgnoreCase(alias)).thenReturn(Optional.empty()); + when(personRepository.findByAlias(alias)).thenReturn(Optional.empty()); + when(personRepository.findAllByAliasIgnoreCase(alias)).thenReturn(List.of()); when(personRepository.save(any())).thenReturn(saved); personService.findOrCreateByAlias(alias); @@ -472,11 +520,11 @@ class PersonServiceTest { void findOrCreateByAlias_trimsInput() { String alias = " Clara Cram "; Person saved = Person.builder().id(UUID.randomUUID()).alias("Clara Cram").build(); - when(personRepository.findByAliasIgnoreCase("Clara Cram")).thenReturn(Optional.of(saved)); + when(personRepository.findByAlias("Clara Cram")).thenReturn(Optional.of(saved)); personService.findOrCreateByAlias(alias); - verify(personRepository).findByAliasIgnoreCase("Clara Cram"); + verify(personRepository).findByAlias("Clara Cram"); } // ─── updatePerson (notes) ──────────────────────────────────────────────── -- 2.49.1 From ddf378aaace91eac0336163f46651f9724949722 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 6 Jun 2026 13:03:04 +0200 Subject: [PATCH 2/4] fix(person): resolve ambiguous sender names to null on upload (#731) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findByName resolved via Optional findByFirstNameIgnoreCaseAndLastNameIgnoreCase, which threw NonUniqueResultException once two people shared a first+last name case- insensitively (hans müller / Hans Müller) — a 500 on the routine upload path (DocumentService.storeDocument sender resolution). findByName now resolves exact-case → single case-insensitive match → else empty. The sender path deliberately diverges from the alias path: an ambiguous name leaves the sender UNSET rather than guessing the lowest id, because correct provenance beats a confidently-wrong pre-fill a reviewer won't re-check. The two new name queries use explicit HQL equality so a null first name binds as `= NULL` (no match) instead of the derived-query fold to `first_name IS NULL`, which would widen a last-name-only row in as a sender. Pins the opaque error path (IncorrectResultSizeDataAccessException stays INTERNAL_ERROR with no Hibernate/SQL/row-count leak) and extends ADR-032 with the Person section. Co-Authored-By: Claude Opus 4.8 --- .../person/PersonRepository.java | 18 +++++- .../familienarchiv/person/PersonService.java | 11 +++- .../raddatz/familienarchiv/person/README.md | 4 +- .../exception/GlobalExceptionHandlerTest.java | 25 ++++++++ .../person/PersonRepositoryTest.java | 31 ++++++--- .../person/PersonServiceIntegrationTest.java | 63 +++++++++++++++++++ .../person/PersonServiceTest.java | 43 +++++++++++++ ...me-resolution-tolerates-case-collisions.md | 45 +++++++++++-- 8 files changed, 223 insertions(+), 17 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java index 841d8f38..39b8191b 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java @@ -43,8 +43,22 @@ public interface PersonRepository extends JpaRepository { // Lookup by the normalizer person_id, used for idempotent canonical re-import (Phase 3). Optional findBySourceRef(String sourceRef); - // Exact first+last name match, used for filename-based sender lookup - Optional findByFirstNameIgnoreCaseAndLastNameIgnoreCase(String firstName, String lastName); + // Exact-case first+last name match — the first step of filename-based sender resolution. + // Explicit `=` (HQL, not a derived query) so a null firstName binds as `first_name = NULL` + // — never a match — instead of the derived-query fold to `first_name IS NULL`, which would + // pull a last-name-only row in as a sender (a provenance defect). See ADR-032. + @Query("SELECT p FROM Person p WHERE p.firstName = :firstName AND p.lastName = :lastName") + Optional findByFirstNameAndLastName(@Param("firstName") String firstName, + @Param("lastName") String lastName); + + // Plural case-insensitive first+last name match — lets findByName bail to empty on 2+ matches + // instead of letting a derived Optional<…>IgnoreCase throw NonUniqueResultException. Same + // null fail-closed guarantee as above: LOWER(:firstName) is NULL for a null arg, so a null + // first name resolves to no match (not first_name IS NULL widening). See ADR-032. + @Query("SELECT p FROM Person p WHERE LOWER(p.firstName) = LOWER(:firstName) " + + "AND LOWER(p.lastName) = LOWER(:lastName)") + List findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase(@Param("firstName") String firstName, + @Param("lastName") String lastName); // --- PersonSummaryDTO with document count --- diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java index f0a682db..f0547bfa 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java @@ -111,7 +111,16 @@ public class PersonService { } public Optional findByName(String firstName, String lastName) { - return personRepository.findByFirstNameIgnoreCaseAndLastNameIgnoreCase(firstName, lastName); + Optional exact = personRepository.findByFirstNameAndLastName(firstName, lastName); + if (exact.isPresent()) return exact; + List caseInsensitive = + personRepository.findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase(firstName, lastName); + // Deliberate divergence from findOrCreateByAlias: an ambiguous filename leaves the sender + // UNSET rather than picking the lowest id. The archive's value is correct provenance — a + // confidently-wrong pre-filled "Hans Müller" is worse than an empty field, because a + // reviewer won't re-check a pre-filled value. Do NOT "consistency-clean" this into the + // lowest-id fallback. See ADR-032. + return caseInsensitive.size() == 1 ? Optional.of(caseInsensitive.get(0)) : Optional.empty(); } /** Lookup by the normalizer person_id — used by the canonical importer for register-first matching. */ diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/README.md b/backend/src/main/java/org/raddatz/familienarchiv/person/README.md index f507f540..f3d36719 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/README.md +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/README.md @@ -20,8 +20,8 @@ Features: person CRUD, name alias management, person merge (deduplication), fami | `getById(UUID)` | document, geschichte, ocr | Fetch one person by ID | | `getAllById(List)` | document | Bulk fetch for sender/receiver resolution | | `findAll(String q)` | document, dashboard | List all persons | -| `findByName(String firstName, String lastName)` | document | Typeahead search | -| `findOrCreateByAlias(String rawName)` | importing | Idempotent create during mass import; type classification happens internally | +| `findByName(String firstName, String lastName)` | document | Filename-based **sender resolution** in `storeDocument`: exact-case match → single case-insensitive match → else **empty** (ambiguous names leave the sender unset; a null first name never matches). See ADR-032. | +| `findOrCreateByAlias(String rawName)` | importing | Idempotent create during mass import; type classification happens internally. Resolves exact-case → lowest-id case-insensitive sibling → create — never throws on case-colliding aliases. See ADR-032. | | `findAllFamilyMembers()` | dashboard | Family member list for stats | | `findCorrespondents()` | document | Correspondent list for conversation filter | | `count()` | dashboard | Total person count for stats | diff --git a/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java index c75e9fae..324c17a1 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandlerTest.java @@ -12,6 +12,7 @@ import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; import org.slf4j.LoggerFactory; import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.http.ResponseEntity; import static org.assertj.core.api.Assertions.assertThat; @@ -37,6 +38,30 @@ class GlobalExceptionHandlerTest { } } + @Test + void handleGeneric_incorrectResultSize_staysOpaque_noHibernateOrRowCountLeak() { + // #731: before the fix, a case-colliding alias/name made Hibernate throw + // NonUniqueResultException → IncorrectResultSizeDataAccessException, which has no + // dedicated handler and falls through to handleGeneric. The fix removes the throw, but + // this pins the handler: a stray one must stay opaque — no Hibernate class name, no SQL, + // no "2 results were returned" row count reaching the client (CWE-209). + IncorrectResultSizeDataAccessException ex = new IncorrectResultSizeDataAccessException( + "query did not return a unique result: 2 results were returned", 1, 2); + + try (MockedStatic sentryMock = mockStatic(Sentry.class)) { + ResponseEntity response = handler.handleGeneric(ex); + + assertThat(response.getStatusCode().value()).isEqualTo(500); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().code()).isEqualTo(ErrorCode.INTERNAL_ERROR); + assertThat(response.getBody().message()) + .isEqualTo("An unexpected error occurred") + .doesNotContain("results were returned") + .doesNotContain("NonUnique") + .doesNotContain("IncorrectResultSize"); + } + } + @Test void handleDataIntegrityViolation_returns400_withoutLeakingConstraint_orSentry() { // A DataIntegrityViolationException carries the constraint name + SQL in its message; diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java index 48f518d2..e53366b9 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java @@ -147,19 +147,34 @@ class PersonRepositoryTest { assertThat(personRepository.findAllByAliasIgnoreCase("MÜLLER")).hasSize(2); } - // ─── findByFirstNameIgnoreCaseAndLastNameIgnoreCase ─────────────────────── - - // ─── findByFirstNameIgnoreCaseAndLastNameIgnoreCase ─────────────────────── + // ─── findByFirstNameAndLastName (exact) / findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase ─── @Test - void findByFirstNameIgnoreCaseAndLastNameIgnoreCase_returnsMatch() { + void findByFirstNameAndLastName_returnsExactCaseMatchOnly() { personRepository.save(Person.builder().firstName("Maria").lastName("Raddatz").build()); - Optional found = personRepository.findByFirstNameIgnoreCaseAndLastNameIgnoreCase( - "maria", "raddatz"); + assertThat(personRepository.findByFirstNameAndLastName("Maria", "Raddatz")).isPresent(); + assertThat(personRepository.findByFirstNameAndLastName("maria", "raddatz")).isEmpty(); // exact-case only + } - assertThat(found).isPresent(); - assertThat(found.get().getFirstName()).isEqualTo("Maria"); + @Test + void findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase_foldsUmlautCase_inRealPostgres() { + personRepository.save(Person.builder().firstName("Hans").lastName("Müller").build()); + personRepository.save(Person.builder().firstName("hans").lastName("müller").build()); + + assertThat(personRepository.findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase("HANS", "MÜLLER")) + .hasSize(2); + } + + @Test + void findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase_nullFirstName_foldsToNoMatch() { + // Fail-closed: a last-name-only filename (null first name) must NOT widen to first_name IS + // NULL and pull in the institution/last-name-only row as a "sender". Proven on real + // Postgres because a mocked unit test cannot catch the IS NULL vs `= NULL` semantics. + personRepository.save(Person.builder().lastName("Müller").build()); // first_name NULL + + assertThat(personRepository.findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase(null, "Müller")) + .isEmpty(); } // ─── findCorrespondents ─────────────────────────────────────────────────── diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java index 285d927a..bcf1245b 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java @@ -4,6 +4,7 @@ import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.PostgresContainerConfig; import org.raddatz.familienarchiv.document.Document; import org.raddatz.familienarchiv.document.DocumentRepository; +import org.raddatz.familienarchiv.document.DocumentService; import org.raddatz.familienarchiv.document.DocumentStatus; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.person.PersonType; @@ -16,6 +17,8 @@ import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.transaction.annotation.Transactional; import software.amazon.awssdk.services.s3.S3Client; +import org.springframework.mock.web.MockMultipartFile; + import jakarta.persistence.EntityManager; import jakarta.persistence.PersistenceContext; @@ -34,6 +37,7 @@ class PersonServiceIntegrationTest { @Autowired PersonService personService; @Autowired PersonRepository personRepository; @Autowired DocumentRepository documentRepository; + @Autowired DocumentService documentService; @PersistenceContext EntityManager entityManager; @@ -104,6 +108,65 @@ class PersonServiceIntegrationTest { assertThat(second.getId()).isEqualTo(first.getId()); } + // ─── #731: filename-based sender resolution against real Postgres ────────── + + @Test + void storeDocument_resolvesSender_whenFilenameNameIsUnique() throws Exception { + Person hans = personRepository.save(Person.builder().firstName("Hans").lastName("Müller").build()); + + Document doc = uploadNamed("1965-03-12_Müller_Hans.pdf").document(); + + assertThat(doc.getSender()).isNotNull(); + assertThat(doc.getSender().getId()).isEqualTo(hans.getId()); + } + + @Test + void storeDocument_resolvesSender_onSingleCaseInsensitiveMatch() throws Exception { + Person hans = personRepository.save(Person.builder().firstName("Hans").lastName("Müller").build()); + + // Filename folds to "hans müller"; the only stored person is "Hans Müller". + Document doc = uploadNamed("1965-03-12_müller_hans.pdf").document(); + + assertThat(doc.getSender()).isNotNull(); + assertThat(doc.getSender().getId()).isEqualTo(hans.getId()); + } + + @Test + void storeDocument_leavesSenderUnset_whenFilenameNameIsAmbiguous() throws Exception { + // Two persons collide case-insensitively; the filename casing ("HANS"/"MÜLLER") matches + // neither exactly → no exact-case winner → bail to null (never an arbitrary guess), no 500. + personRepository.save(Person.builder().firstName("Hans").lastName("Müller").build()); + personRepository.save(Person.builder().firstName("hans").lastName("müller").build()); + + Document doc = uploadNamed("1965-03-12_MÜLLER_HANS.pdf").document(); + + assertThat(doc.getSender()).isNull(); + } + + @Test + void storeDocument_leavesSenderUnset_whenFilenameHasNoFirstName() throws Exception { + // A last-name-only filename never resolves to a sender (the parser yields no parsed name). + personRepository.save(Person.builder().lastName("Müller").build()); + + Document doc = uploadNamed("1965-03-12_Müller.pdf").document(); + + assertThat(doc.getSender()).isNull(); + } + + @Test + void findByName_nullFirstName_resolvesToEmpty_inRealPostgres() { + // Fail-closed against the real DB: a null first name must NOT widen to first_name IS NULL + // and pick up the last-name-only row. + personRepository.save(Person.builder().lastName("Müller").build()); // first_name NULL + + assertThat(personService.findByName(null, "Müller")).isEmpty(); + } + + private DocumentService.StoreResult uploadNamed(String filename) throws Exception { + MockMultipartFile file = new MockMultipartFile("file", filename, "application/pdf", new byte[]{1, 2, 3}); + return documentService.storeDocument(file, null); + } + // ─── #667: confirm round-trip + reader-default semantics ────────────────── @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java index 300681dd..18008afa 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java @@ -527,6 +527,49 @@ class PersonServiceTest { verify(personRepository).findByAlias("Clara Cram"); } + // ─── findByName (filename-based sender resolution) ──────────────────────── + + @Test + void findByName_returnsExactCaseMatch_overCaseInsensitiveSibling() { + Person exact = Person.builder().id(UUID.randomUUID()).firstName("Hans").lastName("Müller").build(); + when(personRepository.findByFirstNameAndLastName("Hans", "Müller")).thenReturn(Optional.of(exact)); + + assertThat(personService.findByName("Hans", "Müller")).contains(exact); + verify(personRepository, never()).findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase(any(), any()); + } + + @Test + void findByName_usesSingleCaseInsensitiveMatch_whenNoExactCase() { + Person only = Person.builder().id(UUID.randomUUID()).firstName("Hans").lastName("Müller").build(); + when(personRepository.findByFirstNameAndLastName("hans", "müller")).thenReturn(Optional.empty()); + when(personRepository.findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase("hans", "müller")) + .thenReturn(List.of(only)); + + assertThat(personService.findByName("hans", "müller")).contains(only); + } + + @Test + void findByName_bailsToEmpty_whenTwoOrMoreCaseInsensitiveMatches() { + Person a = Person.builder().id(UUID.randomUUID()).firstName("Hans").lastName("Müller").build(); + Person b = Person.builder().id(UUID.randomUUID()).firstName("hans").lastName("müller").build(); + when(personRepository.findByFirstNameAndLastName("hans", "müller")).thenReturn(Optional.empty()); + when(personRepository.findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase("hans", "müller")) + .thenReturn(List.of(a, b)); + + // Ambiguous sender → unset, never an arbitrary guess (provenance correctness over a + // confidently-wrong pre-fill). This is the deliberate divergence from the alias path. + assertThat(personService.findByName("hans", "müller")).isEmpty(); + } + + @Test + void findByName_returnsEmpty_whenFirstNameNullFoldsToNoMatch() { + when(personRepository.findByFirstNameAndLastName(null, "Müller")).thenReturn(Optional.empty()); + when(personRepository.findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase(null, "Müller")) + .thenReturn(List.of()); + + assertThat(personService.findByName(null, "Müller")).isEmpty(); + } + // ─── updatePerson (notes) ──────────────────────────────────────────────── @Test diff --git a/docs/adr/032-tag-name-resolution-tolerates-case-collisions.md b/docs/adr/032-tag-name-resolution-tolerates-case-collisions.md index 9a1fe61f..ae9040f0 100644 --- a/docs/adr/032-tag-name-resolution-tolerates-case-collisions.md +++ b/docs/adr/032-tag-name-resolution-tolerates-case-collisions.md @@ -82,15 +82,52 @@ added later. `IncorrectResultSizeDataAccessException`, and `GlobalExceptionHandler`'s generic handler maps any stray one to `INTERNAL_ERROR` with no Hibernate/SQL leak — so no dedicated handler was added. -- **The sibling Person path is unfixed but tracked.** `PersonService.findOrCreateByAlias` - (`findByAliasIgnoreCase`) and `findByFirstNameIgnoreCaseAndLastNameIgnoreCase` carry the same - latent `Optional`-non-unique throw on user-influenced names; deferred to #731 rather than - widened into this fix. +- **The sibling Person path is fixed the same way — see the Person extension below (#731).** - Postgres `LOWER()` folding of umlauts (`ü`/`ä`) is the actual correctness hinge of the fallback and cannot be proven by a mocked repo, so it is pinned by a Testcontainers `postgres:16-alpine` test on a `Glückwünsche`/`glückwünsche` pair; a plain-ASCII test would stay green while the bug reappeared for umlaut tags. +## Person extension (#731) + +The Person domain carried the same latent throw on **two** user-influenced lookup surfaces, and +is fixed with the same exact-case-first, non-throwing pattern — but with a deliberately +**different fallback per surface**, because the two paths have different consequences. + +- **Alias path — `PersonService.findOrCreateByAlias` — deterministic lowest-id (mirrors tag).** + `findByAliasIgnoreCase` (`Optional`) is replaced by `findByAlias` (exact) → `findAllByAliasIgnoreCase` + (plural, lowest id) → the existing create-when-absent branch (INSTITUTION/GROUP and the + maiden-name alias are preserved verbatim). There is no human in the importer loop and the path + creates-on-absent anyway, so a deterministic guess is the right behaviour — exactly like tags. + +- **Name/sender path — `PersonService.findByName` — bail to null on ambiguity (the new wrinkle).** + Used only by `DocumentService.storeDocument` to resolve the upload **sender** from the parsed + filename. `findByFirstNameIgnoreCaseAndLastNameIgnoreCase` (`Optional`) is replaced by + `findByFirstNameAndLastName` (exact) → `findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase` + (plural). Resolution returns the exact-case match, else the single case-insensitive match, else + — on **two or more** matches — **empty**. The sender is left unset rather than guessing. + + **Why this diverges from the alias (and tag) decision:** the archive's value is correct + provenance. A confidently-wrong pre-filled `Hans Müller` is worse than an empty field, because a + senior reviewer will not re-check a value that is already filled in, whereas an empty sender + routes the document into the "needs completion" state (`metadataComplete=false`) for a human to + assign. The load-bearing comment at `findByName` records this so a future "consistency cleanup" + does not reintroduce the confidently-wrong-sender bug by switching it to lowest-id. + +- **Fail-closed on a null first name.** A parsed filename can lack a first name. The two new name + methods use explicit HQL equality (`= :firstName`) rather than a derived + `…IgnoreCase` query, because Spring Data folds a null derived-query argument to `first_name IS + NULL` — which would silently widen the match and pull a last-name-only / institution row in as a + "sender" (a quiet provenance-integrity defect). With HQL equality a null binds as `= NULL`, + which never matches, so a null first name resolves to **no sender**. This is pinned by a + real-Postgres repository test. + +- **Same stance as tags otherwise:** no `unique(lower(alias))` / `unique(lower(name))` constraint + (collisions are valid human labels; `source_ref` is the stable identity per ADR-025), no + merge/dedupe, code-only and reversible, and no shared `resolveExactThenCi(...)` helper — the + two Person paths have different fallbacks, so the exact→CI→fallback logic is inlined at each + with its load-bearing comment (KISS). + ## Alternatives considered - **A `unique(lower(name))` index** — rejected: the collisions are valid canonical nodes, and -- 2.49.1 From cd741b9f57686fa98ce18bf59dc6d43a7409d415 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 6 Jun 2026 13:36:22 +0200 Subject: [PATCH 3/4] docs(person): clarify case-collision scope at the exact-case lookups (#731) Review noted the "never throws" claim was overstated: the exact-case Optional lookups still surface a NonUniqueResultException on two byte-identical same-case rows. That is a true data anomaly out of #731's scope (ambiguous = case-insensitive) and resolves to the opaque INTERNAL_ERROR, never a wrong row. Record that boundary at both resolution points and in ADR-032 so the gap is not silently assumed covered. Co-Authored-By: Claude Opus 4.8 --- .../org/raddatz/familienarchiv/person/PersonService.java | 8 +++++++- .../032-tag-name-resolution-tolerates-case-collisions.md | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java index f0547bfa..30d1395f 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java @@ -111,6 +111,9 @@ public class PersonService { } public Optional findByName(String firstName, String lastName) { + // Same scope as findOrCreateByAlias (#731): a case-collision resolves without throwing; + // two byte-identical same-case persons are an out-of-scope data anomaly the exact + // Optional below would surface as the opaque INTERNAL_ERROR, not a wrong sender. Optional exact = personRepository.findByFirstNameAndLastName(firstName, lastName); if (exact.isPresent()) return exact; List caseInsensitive = @@ -136,8 +139,11 @@ public class PersonService { if (type == PersonType.SKIP) return null; // Aliases differing only by case (müller / Müller) are valid distinct persons, not - // duplicates, so resolution must never throw: exact-case first, then the lowest-id + // duplicates, so a CASE-COLLISION must not throw: exact-case first, then the lowest-id // case-insensitive sibling, then create. Mirrors the tag path — see ADR-032. + // Scope (#731): "ambiguous" means case-insensitive. Two BYTE-IDENTICAL same-case aliases + // are a true data anomaly out of scope here; the exact Optional below would surface that + // as the opaque INTERNAL_ERROR (never a wrong row), not silently pick one. Optional exact = personRepository.findByAlias(alias); if (exact.isPresent()) return exact.get(); // exact-case wins List caseInsensitive = personRepository.findAllByAliasIgnoreCase(alias); diff --git a/docs/adr/032-tag-name-resolution-tolerates-case-collisions.md b/docs/adr/032-tag-name-resolution-tolerates-case-collisions.md index ae9040f0..090ee9fd 100644 --- a/docs/adr/032-tag-name-resolution-tolerates-case-collisions.md +++ b/docs/adr/032-tag-name-resolution-tolerates-case-collisions.md @@ -122,6 +122,12 @@ is fixed with the same exact-case-first, non-throwing pattern — but with a del which never matches, so a null first name resolves to **no sender**. This is pinned by a real-Postgres repository test. +- **Scope — "ambiguous" is case-insensitive only.** Both exact-case lookups (`findByAlias`, + `findByFirstNameAndLastName`) return `Optional`, so two **byte-identical same-case** rows would + still throw `NonUniqueResultException`. That is a true data anomaly, deliberately out of scope + (it is not a case-collision), and it surfaces as the opaque `INTERNAL_ERROR` — never a silently + wrong row — so it is no worse than any other unexpected error and needs no extra handling here. + - **Same stance as tags otherwise:** no `unique(lower(alias))` / `unique(lower(name))` constraint (collisions are valid human labels; `source_ref` is the stable identity per ADR-025), no merge/dedupe, code-only and reversible, and no shared `resolveExactThenCi(...)` helper — the -- 2.49.1 From 7629e3589728d12f3445603c847298f86776d77f Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 6 Jun 2026 13:52:25 +0200 Subject: [PATCH 4/4] =?UTF-8?q?docs(adr):=20renumber=20tag=20case-collisio?= =?UTF-8?q?n=20ADR=20032=20=E2=86=92=20033=20to=20resolve=20number=20clash?= =?UTF-8?q?=20(#731)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both #730 (tag case-collision) and #684 (person-delete DB integrity) landed an ADR-032 on main. Renumber the tag/case-collision one to 033 — it is referenced only from this PR's person-domain comments and its own file, so the move is self-contained and touches no Flyway migration. The person-delete ADR-032 and the V71 migration comment that cites it are deliberately left untouched (editing an applied migration would drift its Flyway checksum). Co-Authored-By: Claude Opus 4.8 --- .../familienarchiv/person/PersonRepository.java | 10 +++++----- .../raddatz/familienarchiv/person/PersonService.java | 4 ++-- .../java/org/raddatz/familienarchiv/person/README.md | 4 ++-- ...3-tag-name-resolution-tolerates-case-collisions.md} | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) rename docs/adr/{032-tag-name-resolution-tolerates-case-collisions.md => 033-tag-name-resolution-tolerates-case-collisions.md} (99%) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java index 39b8191b..fe619e0b 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java @@ -31,13 +31,13 @@ public interface PersonRepository extends JpaRepository { // Exact-case alias lookup — the first resolution step in findOrCreateByAlias. // Case-colliding aliases across persons (müller / Müller) are valid human labels, NOT - // duplicates: source_ref is the stable identity (ADR-025/032), alias is editable. Do NOT - // add a unique(lower(alias)) constraint — see ADR-032. + // duplicates: source_ref is the stable identity (ADR-025/033), alias is editable. Do NOT + // add a unique(lower(alias)) constraint — see ADR-033. Optional findByAlias(String alias); // Plural case-insensitive alias lookup — the fallback step. Returns ALL case-folding // siblings so the service can pick a deterministic one (lowest id) instead of letting a - // derived Optional<…>IgnoreCase throw NonUniqueResultException. See ADR-032. + // derived Optional<…>IgnoreCase throw NonUniqueResultException. See ADR-033. List findAllByAliasIgnoreCase(String alias); // Lookup by the normalizer person_id, used for idempotent canonical re-import (Phase 3). @@ -46,7 +46,7 @@ public interface PersonRepository extends JpaRepository { // Exact-case first+last name match — the first step of filename-based sender resolution. // Explicit `=` (HQL, not a derived query) so a null firstName binds as `first_name = NULL` // — never a match — instead of the derived-query fold to `first_name IS NULL`, which would - // pull a last-name-only row in as a sender (a provenance defect). See ADR-032. + // pull a last-name-only row in as a sender (a provenance defect). See ADR-033. @Query("SELECT p FROM Person p WHERE p.firstName = :firstName AND p.lastName = :lastName") Optional findByFirstNameAndLastName(@Param("firstName") String firstName, @Param("lastName") String lastName); @@ -54,7 +54,7 @@ public interface PersonRepository extends JpaRepository { // Plural case-insensitive first+last name match — lets findByName bail to empty on 2+ matches // instead of letting a derived Optional<…>IgnoreCase throw NonUniqueResultException. Same // null fail-closed guarantee as above: LOWER(:firstName) is NULL for a null arg, so a null - // first name resolves to no match (not first_name IS NULL widening). See ADR-032. + // first name resolves to no match (not first_name IS NULL widening). See ADR-033. @Query("SELECT p FROM Person p WHERE LOWER(p.firstName) = LOWER(:firstName) " + "AND LOWER(p.lastName) = LOWER(:lastName)") List findAllByFirstNameIgnoreCaseAndLastNameIgnoreCase(@Param("firstName") String firstName, diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java index 30d1395f..81fbb406 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java @@ -122,7 +122,7 @@ public class PersonService { // UNSET rather than picking the lowest id. The archive's value is correct provenance — a // confidently-wrong pre-filled "Hans Müller" is worse than an empty field, because a // reviewer won't re-check a pre-filled value. Do NOT "consistency-clean" this into the - // lowest-id fallback. See ADR-032. + // lowest-id fallback. See ADR-033. return caseInsensitive.size() == 1 ? Optional.of(caseInsensitive.get(0)) : Optional.empty(); } @@ -140,7 +140,7 @@ public class PersonService { // Aliases differing only by case (müller / Müller) are valid distinct persons, not // duplicates, so a CASE-COLLISION must not throw: exact-case first, then the lowest-id - // case-insensitive sibling, then create. Mirrors the tag path — see ADR-032. + // case-insensitive sibling, then create. Mirrors the tag path — see ADR-033. // Scope (#731): "ambiguous" means case-insensitive. Two BYTE-IDENTICAL same-case aliases // are a true data anomaly out of scope here; the exact Optional below would surface that // as the opaque INTERNAL_ERROR (never a wrong row), not silently pick one. diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/README.md b/backend/src/main/java/org/raddatz/familienarchiv/person/README.md index f3d36719..ffbe6c72 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/README.md +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/README.md @@ -20,8 +20,8 @@ Features: person CRUD, name alias management, person merge (deduplication), fami | `getById(UUID)` | document, geschichte, ocr | Fetch one person by ID | | `getAllById(List)` | document | Bulk fetch for sender/receiver resolution | | `findAll(String q)` | document, dashboard | List all persons | -| `findByName(String firstName, String lastName)` | document | Filename-based **sender resolution** in `storeDocument`: exact-case match → single case-insensitive match → else **empty** (ambiguous names leave the sender unset; a null first name never matches). See ADR-032. | -| `findOrCreateByAlias(String rawName)` | importing | Idempotent create during mass import; type classification happens internally. Resolves exact-case → lowest-id case-insensitive sibling → create — never throws on case-colliding aliases. See ADR-032. | +| `findByName(String firstName, String lastName)` | document | Filename-based **sender resolution** in `storeDocument`: exact-case match → single case-insensitive match → else **empty** (ambiguous names leave the sender unset; a null first name never matches). See ADR-033. | +| `findOrCreateByAlias(String rawName)` | importing | Idempotent create during mass import; type classification happens internally. Resolves exact-case → lowest-id case-insensitive sibling → create — never throws on case-colliding aliases. See ADR-033. | | `findAllFamilyMembers()` | dashboard | Family member list for stats | | `findCorrespondents()` | document | Correspondent list for conversation filter | | `count()` | dashboard | Total person count for stats | diff --git a/docs/adr/032-tag-name-resolution-tolerates-case-collisions.md b/docs/adr/033-tag-name-resolution-tolerates-case-collisions.md similarity index 99% rename from docs/adr/032-tag-name-resolution-tolerates-case-collisions.md rename to docs/adr/033-tag-name-resolution-tolerates-case-collisions.md index 090ee9fd..bb907b4a 100644 --- a/docs/adr/032-tag-name-resolution-tolerates-case-collisions.md +++ b/docs/adr/033-tag-name-resolution-tolerates-case-collisions.md @@ -1,4 +1,4 @@ -# ADR-032 — Tag-name resolution tolerates case-collisions: exact-case first, then a deterministic lowest-id fallback, and never a `unique(lower(name))` constraint +# ADR-033 — Tag-name resolution tolerates case-collisions: exact-case first, then a deterministic lowest-id fallback, and never a `unique(lower(name))` constraint **Date:** 2026-06-06 **Status:** Accepted -- 2.49.1