From d000170f52555ff43f7fb4d6457a65a3ee4dbe8c Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 6 Jun 2026 10:49:02 +0200 Subject: [PATCH] fix(tag): resolve case-colliding tag names without throwing (#730) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findOrCreate used tagRepository.findByNameIgnoreCase, which returns Optional and threw NonUniqueResultException whenever two tags collided case-insensitively (a canonical parent and its same-named lowercase child). Every document carrying such a tag became un-editable: any save re-resolves the whole tag set by name and blew up with a 500. Replace the throwing lookup with exact-case-first resolution: findByName (exact) → findAllByNameIgnoreCase (lowest-id, deterministic, never throws) → create. Delete findByNameIgnoreCase so the throwing call can't be reintroduced. Case collisions are valid tree nodes — no migration, no unique(lower(name)) constraint. Co-Authored-By: Claude Opus 4.8 --- .../familienarchiv/tag/TagRepository.java | 9 ++- .../familienarchiv/tag/TagService.java | 16 ++++- .../familienarchiv/tag/TagServiceTest.java | 60 +++++++++++++++---- 3 files changed, 70 insertions(+), 15 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagRepository.java index 108b4bc8..69b1eb03 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagRepository.java @@ -20,7 +20,14 @@ public interface TagRepository extends JpaRepository { } - Optional findByNameIgnoreCase(String name); + // Tag-name resolution (see TagService.findOrCreate). Names that collide case-insensitively across + // the canonical tree are VALID — a parent and its same-named lowercase child (e.g. "Geburt" / + // "Geburt/geburt") are distinct nodes with their own source_ref and document attachments. So + // resolution must be exact-case first, then a non-throwing list for the case-insensitive fallback. + // Do NOT add a unique(lower(name)) constraint — it would reject these legitimate rows. See #730. + Optional findByName(String name); + + List findAllByNameIgnoreCase(String name); // Lookup by the canonical tag_path, used for idempotent canonical re-import (Phase 3). Optional findBySourceRef(String sourceRef); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagService.java b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagService.java index 1dac8610..8ac6a622 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/tag/TagService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/tag/TagService.java @@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.tag; import java.util.ArrayList; import java.util.Collection; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -55,10 +56,21 @@ public class TagService { return tagRepository.findBySourceRef(sourceRef); } + /** + * Resolves a tag name to a single tag, creating one when absent. Never throws on case-insensitive + * collisions: names that differ only by case are valid distinct nodes in the canonical tree (a + * parent and its same-named lowercase child), so resolution prefers an exact-case match, then + * falls back to the lowest-id case-insensitive match, then creates. See #730. + */ public Tag findOrCreate(String name) { String cleanName = name.trim(); - return tagRepository.findByNameIgnoreCase(cleanName) - .orElseGet(() -> tagRepository.save(Tag.builder().name(cleanName).build())); + Optional exact = tagRepository.findByName(cleanName); + if (exact.isPresent()) return exact.get(); // exact-case wins (edit round-trip replays the stored name) + List caseInsensitive = tagRepository.findAllByNameIgnoreCase(cleanName); + if (!caseInsensitive.isEmpty()) { + return caseInsensitive.stream().min(Comparator.comparing(Tag::getId)).get(); // deterministic tie-break by id — never throw + } + return tagRepository.save(Tag.builder().name(cleanName).build()); // create-when-absent (orphan tag: null sourceRef/parentId) } /** diff --git a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java index 3cb2762f..aa9436f4 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/tag/TagServiceTest.java @@ -53,20 +53,54 @@ class TagServiceTest { // ─── findOrCreate ───────────────────────────────────────────────────────── @Test - void findOrCreate_returnsExisting_whenNameFound() { - Tag existing = Tag.builder().id(UUID.randomUUID()).name("Familie").build(); - when(tagRepository.findByNameIgnoreCase("Familie")).thenReturn(Optional.of(existing)); + void findOrCreate_exactCaseWins_overCaseInsensitiveSibling() { + // "Geburt" (parent) and "geburt" (child) both exist; the edit round-trip replays the stored + // name "geburt", which must bind to the exact-case row, not the parent. + Tag exact = Tag.builder().id(UUID.randomUUID()).name("geburt").build(); + when(tagRepository.findByName("geburt")).thenReturn(Optional.of(exact)); - Tag result = tagService.findOrCreate("Familie"); + Tag result = tagService.findOrCreate("geburt"); - assertThat(result).isEqualTo(existing); + assertThat(result).isEqualTo(exact); verify(tagRepository, never()).save(any()); } @Test - void findOrCreate_createsNew_whenNameNotFound() { + void findOrCreate_usesSingleCaseInsensitiveMatch_whenNoExactCase() { + // Stored name is "Weihnachten"; a save replays "weihnachten" (no exact-case row) → bind to the + // single case-insensitive match rather than creating a duplicate. + Tag stored = Tag.builder().id(UUID.randomUUID()).name("Weihnachten").build(); + when(tagRepository.findByName("weihnachten")).thenReturn(Optional.empty()); + when(tagRepository.findAllByNameIgnoreCase("weihnachten")).thenReturn(List.of(stored)); + + Tag result = tagService.findOrCreate("weihnachten"); + + assertThat(result).isEqualTo(stored); + verify(tagRepository, never()).save(any()); + } + + @Test + void findOrCreate_returnsLowestIdDeterministically_whenMultipleCaseInsensitiveMatches() { + // Two rows collide case-insensitively and neither equals the query exactly. Resolution must be + // deterministic (lowest id) and never throw — proven by calling twice and getting the same id. + Tag lowerId = Tag.builder().id(UUID.fromString("00000000-0000-0000-0000-000000000001")).name("Reisepläne").build(); + Tag higherId = Tag.builder().id(UUID.fromString("00000000-0000-0000-0000-000000000002")).name("reisepläne").build(); + when(tagRepository.findByName("REISEPLÄNE")).thenReturn(Optional.empty()); + when(tagRepository.findAllByNameIgnoreCase("REISEPLÄNE")).thenReturn(List.of(higherId, lowerId)); + + Tag first = tagService.findOrCreate("REISEPLÄNE"); + Tag second = tagService.findOrCreate("REISEPLÄNE"); + + assertThat(first.getId()).isEqualTo(lowerId.getId()); + assertThat(second.getId()).isEqualTo(first.getId()); + verify(tagRepository, never()).save(any()); + } + + @Test + void findOrCreate_createsOrphanTag_whenNameAbsent() { Tag saved = Tag.builder().id(UUID.randomUUID()).name("Krieg").build(); - when(tagRepository.findByNameIgnoreCase("Krieg")).thenReturn(Optional.empty()); + when(tagRepository.findByName("Krieg")).thenReturn(Optional.empty()); + when(tagRepository.findAllByNameIgnoreCase("Krieg")).thenReturn(List.of()); when(tagRepository.save(any())).thenReturn(saved); Tag result = tagService.findOrCreate("Krieg"); @@ -76,13 +110,15 @@ class TagServiceTest { } @Test - void findOrCreate_trimsWhitespaceBeforeLookup() { - Tag existing = Tag.builder().id(UUID.randomUUID()).name("Urlaub").build(); - when(tagRepository.findByNameIgnoreCase("Urlaub")).thenReturn(Optional.of(existing)); + void findOrCreate_trimsWhitespace_thenLandsOnCaseInsensitiveChild() { + Tag child = Tag.builder().id(UUID.randomUUID()).name("weihnachten").build(); + when(tagRepository.findByName("weihnachten")).thenReturn(Optional.empty()); + when(tagRepository.findAllByNameIgnoreCase("weihnachten")).thenReturn(List.of(child)); - tagService.findOrCreate(" Urlaub "); + Tag result = tagService.findOrCreate(" weihnachten "); - verify(tagRepository).findByNameIgnoreCase("Urlaub"); + assertThat(result).isEqualTo(child); + verify(tagRepository).findByName("weihnachten"); } // ─── update ───────────────────────────────────────────────────────────────