marcel/familienarchiv
1
0
Fork 0
Code Issues 119 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix(person): resolve case-colliding aliases without throwing (#731)

Browse Source 
findOrCreateByAlias resolved via Optional<Person> 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 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-06-06 12:50:21 +02:00
parent 43601a3770
commit 20cfe41f21
5 changed files with 151 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java
View File

@@ -29,8 +29,16 @@ public interface PersonRepository extends JpaRepository<Person, UUID> {
// Stammbaum-Knoten: alle Personen mit family_member = true. // Stammbaum-Knoten: alle Personen mit family_member = true.
List<Person> findByFamilyMemberTrueOrderByLastNameAscFirstNameAsc(); List<Person> findByFamilyMemberTrueOrderByLastNameAscFirstNameAsc();
// Lookup by full alias string, used during ODS mass import // Exact-case alias lookup — the first resolution step in findOrCreateByAlias.
Optional<Person> findByAliasIgnoreCase(String alias); // 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<Person> 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<Person> findAllByAliasIgnoreCase(String alias);
// Lookup by the normalizer person_id, used for idempotent canonical re-import (Phase 3). // Lookup by the normalizer person_id, used for idempotent canonical re-import (Phase 3).
Optional<Person> findBySourceRef(String sourceRef); Optional<Person> findBySourceRef(String sourceRef);

57
backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java
View File

@@ -1,5 +1,6 @@
package org.raddatz.familienarchiv.person; package org.raddatz.familienarchiv.person;
import java.util.Comparator;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Optional;
import java.util.UUID; import java.util.UUID;
@@ -125,32 +126,42 @@ public class PersonService {
PersonType type = PersonTypeClassifier.classify(alias); PersonType type = PersonTypeClassifier.classify(alias);
if (type == PersonType.SKIP) return null; if (type == PersonType.SKIP) return null;
return personRepository.findByAliasIgnoreCase(alias).orElseGet(() -> { // Aliases differing only by case (müller / Müller) are valid distinct persons, not
if (type == PersonType.INSTITUTION || type == PersonType.GROUP) { // duplicates, so resolution must never throw: exact-case first, then the lowest-id
return personRepository.save(Person.builder() // case-insensitive sibling, then create. Mirrors the tag path — see ADR-032.
.alias(alias) Optional<Person> exact = personRepository.findByAlias(alias);
.lastName(alias) if (exact.isPresent()) return exact.get(); // exact-case wins
.personType(type) List<Person> caseInsensitive = personRepository.findAllByAliasIgnoreCase(alias);
.build()); 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); // Create-when-absent: institution/group keep the full label in lastName; a person name
Person person = personRepository.save(Person.builder() // 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) .alias(alias)
.firstName(split.firstName()) .lastName(alias)
.lastName(split.lastName()) .personType(type)
.build()); .build());
if (split.maidenName() != null) { }
int nextSortOrder = aliasRepository.findMaxSortOrder(person.getId()) + 1;
aliasRepository.save(PersonNameAlias.builder() PersonNameParser.SplitName split = PersonNameParser.split(alias);
.person(person) Person person = personRepository.save(Person.builder()
.lastName(split.maidenName()) .alias(alias)
.type(PersonNameAliasType.MAIDEN_NAME) .firstName(split.firstName())
.sortOrder(nextSortOrder) .lastName(split.lastName())
.build()); .build());
} if (split.maidenName() != null) {
return person; 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;
} }
/** /**

28
backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java
View File

@@ -121,26 +121,34 @@ class PersonRepositoryTest {
.containsExactly("Anna", "Clara"); .containsExactly("Anna", "Clara");
} }
// ─── findByAliasIgnoreCase ──────────────────────────────────────────────── // ─── findByAlias (exact) / findAllByAliasIgnoreCase (case-folding siblings) ───
@Test @Test
void findByAliasIgnoreCase_returnsMatchingPerson() { void findByAlias_returnsExactCaseMatchOnly() {
personRepository.save(Person.builder() personRepository.save(Person.builder()
.firstName("Karl").lastName("Brandt").alias("Opa Karl").build()); .firstName("Karl").lastName("Brandt").alias("Opa Karl").build());
Optional<Person> found = personRepository.findByAliasIgnoreCase("opa karl"); assertThat(personRepository.findByAlias("Opa Karl")).isPresent();
assertThat(personRepository.findByAlias("opa karl")).isEmpty(); // exact-case: a folded form does NOT match
assertThat(found).isPresent();
assertThat(found.get().getFirstName()).isEqualTo("Karl");
} }
@Test @Test
void findByAliasIgnoreCase_returnsEmpty_whenAliasDoesNotMatch() { void findAllByAliasIgnoreCase_returnsEmpty_whenAliasDoesNotMatch() {
Optional<Person> found = personRepository.findByAliasIgnoreCase("nobody"); assertThat(personRepository.findAllByAliasIgnoreCase("nobody")).isEmpty();
assertThat(found).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 ─────────────────────── // ─── findByFirstNameIgnoreCaseAndLastNameIgnoreCase ───────────────────────
@Test @Test

29
backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java
View File

@@ -20,6 +20,7 @@ import jakarta.persistence.EntityManager;
import jakarta.persistence.PersistenceContext; import jakarta.persistence.PersistenceContext;
import java.util.Set; import java.util.Set;
import java.util.UUID;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
@@ -75,6 +76,34 @@ class PersonServiceIntegrationTest {
assertThat(result.getLastName()).isEqualTo("Cram"); 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 ────────────────── // ─── #667: confirm round-trip + reader-default semantics ──────────────────
@Test @Test

72
backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java
View File

@@ -375,14 +375,57 @@ class PersonServiceTest {
// ─── findOrCreateByAlias ───────────────────────────────────────────────── // ─── findOrCreateByAlias ─────────────────────────────────────────────────
@Test @Test
void findOrCreateByAlias_returnsExisting_whenAliasFound() { void findOrCreateByAlias_returnsExactCaseMatch_overCaseInsensitiveSibling() {
String alias = "Walter de Gruyter"; String alias = "müller";
Person existing = Person.builder().id(UUID.randomUUID()).alias(alias).build(); Person exact = Person.builder().id(UUID.randomUUID()).alias("müller").build();
when(personRepository.findByAliasIgnoreCase(alias)).thenReturn(Optional.of(existing)); when(personRepository.findByAlias(alias)).thenReturn(Optional.of(exact));
Person result = personService.findOrCreateByAlias(alias); 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()); verify(personRepository, never()).save(any());
} }
@@ -390,7 +433,8 @@ class PersonServiceTest {
void findOrCreateByAlias_createsNew_whenAliasNotFound() { void findOrCreateByAlias_createsNew_whenAliasNotFound() {
String alias = "Clara Cram"; String alias = "Clara Cram";
Person saved = Person.builder().id(UUID.randomUUID()).alias(alias).firstName("Clara").lastName("Cram").build(); 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(personRepository.save(any())).thenReturn(saved);
Person result = personService.findOrCreateByAlias(alias); Person result = personService.findOrCreateByAlias(alias);
@@ -403,7 +447,8 @@ class PersonServiceTest {
void findOrCreateByAlias_createsMaidenNameAlias_whenGebPresent() { void findOrCreateByAlias_createsMaidenNameAlias_whenGebPresent() {
String alias = "Clara Cram geb. de Gruyter"; String alias = "Clara Cram geb. de Gruyter";
Person saved = Person.builder().id(UUID.randomUUID()).alias(alias).firstName("Clara").lastName("Cram").build(); 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(personRepository.save(any())).thenReturn(saved);
when(aliasRepository.findMaxSortOrder(saved.getId())).thenReturn(0); when(aliasRepository.findMaxSortOrder(saved.getId())).thenReturn(0);
when(aliasRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); when(aliasRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
@@ -425,7 +470,8 @@ class PersonServiceTest {
@Test @Test
void findOrCreateByAlias_setsInstitutionType_withFullNameInLastName() { void findOrCreateByAlias_setsInstitutionType_withFullNameInLastName() {
String alias = "Arthur Collignon GmbH"; 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 -> { when(personRepository.save(any())).thenAnswer(inv -> {
Person p = inv.getArgument(0); Person p = inv.getArgument(0);
p.setId(UUID.randomUUID()); p.setId(UUID.randomUUID());
@@ -442,7 +488,8 @@ class PersonServiceTest {
@Test @Test
void findOrCreateByAlias_setsGroupType_withFullNameInLastName() { void findOrCreateByAlias_setsGroupType_withFullNameInLastName() {
String alias = "Geschwister de Gruyter"; 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 -> { when(personRepository.save(any())).thenAnswer(inv -> {
Person p = inv.getArgument(0); Person p = inv.getArgument(0);
p.setId(UUID.randomUUID()); p.setId(UUID.randomUUID());
@@ -460,7 +507,8 @@ class PersonServiceTest {
void findOrCreateByAlias_noAlias_whenNoGeb() { void findOrCreateByAlias_noAlias_whenNoGeb() {
String alias = "Clara Cram"; String alias = "Clara Cram";
Person saved = Person.builder().id(UUID.randomUUID()).alias(alias).firstName("Clara").lastName("Cram").build(); 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(personRepository.save(any())).thenReturn(saved);
personService.findOrCreateByAlias(alias); personService.findOrCreateByAlias(alias);
@@ -472,11 +520,11 @@ class PersonServiceTest {
void findOrCreateByAlias_trimsInput() { void findOrCreateByAlias_trimsInput() {
String alias = " Clara Cram "; String alias = " Clara Cram ";
Person saved = Person.builder().id(UUID.randomUUID()).alias("Clara Cram").build(); 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); personService.findOrCreateByAlias(alias);
verify(personRepository).findByAliasIgnoreCase("Clara Cram"); verify(personRepository).findByAlias("Clara Cram");
} }
// ─── updatePerson (notes) ──────────────────────────────────────────────── // ─── updatePerson (notes) ────────────────────────────────────────────────