docs(person): clarify case-collision scope at the exact-case lookups (#731)
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m15s
CI / OCR Service Tests (pull_request) Successful in 22s
CI / Backend Unit Tests (pull_request) Successful in 3m42s
CI / fail2ban Regex (pull_request) Successful in 46s
CI / Semgrep Security Scan (pull_request) Successful in 21s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m5s
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m15s
CI / OCR Service Tests (pull_request) Successful in 22s
CI / Backend Unit Tests (pull_request) Successful in 3m42s
CI / fail2ban Regex (pull_request) Successful in 46s
CI / Semgrep Security Scan (pull_request) Successful in 21s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m5s
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 <noreply@anthropic.com>
This commit is contained in:
@@ -111,6 +111,9 @@ public class PersonService {
|
|||||||
}
|
}
|
||||||
|
|
||||||
public Optional<Person> findByName(String firstName, String lastName) {
|
public Optional<Person> 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<Person> exact = personRepository.findByFirstNameAndLastName(firstName, lastName);
|
Optional<Person> exact = personRepository.findByFirstNameAndLastName(firstName, lastName);
|
||||||
if (exact.isPresent()) return exact;
|
if (exact.isPresent()) return exact;
|
||||||
List<Person> caseInsensitive =
|
List<Person> caseInsensitive =
|
||||||
@@ -136,8 +139,11 @@ public class PersonService {
|
|||||||
if (type == PersonType.SKIP) return null;
|
if (type == PersonType.SKIP) return null;
|
||||||
|
|
||||||
// Aliases differing only by case (müller / Müller) are valid distinct persons, not
|
// 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.
|
// 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<Person> exact = personRepository.findByAlias(alias);
|
Optional<Person> exact = personRepository.findByAlias(alias);
|
||||||
if (exact.isPresent()) return exact.get(); // exact-case wins
|
if (exact.isPresent()) return exact.get(); // exact-case wins
|
||||||
List<Person> caseInsensitive = personRepository.findAllByAliasIgnoreCase(alias);
|
List<Person> caseInsensitive = personRepository.findAllByAliasIgnoreCase(alias);
|
||||||
|
|||||||
@@ -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
|
which never matches, so a null first name resolves to **no sender**. This is pinned by a
|
||||||
real-Postgres repository test.
|
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
|
- **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
|
(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
|
merge/dedupe, code-only and reversible, and no shared `resolveExactThenCi(...)` helper — the
|
||||||
|
|||||||
Reference in New Issue
Block a user