fix(tag): resolve case-colliding tag names without throwing (#730)
findOrCreate used tagRepository.findByNameIgnoreCase, which returns Optional<Tag> 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 <noreply@anthropic.com>
This commit is contained in:
@@ -20,7 +20,14 @@ public interface TagRepository extends JpaRepository<Tag, UUID> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
Optional<Tag> 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<Tag> findByName(String name);
|
||||||
|
|
||||||
|
List<Tag> findAllByNameIgnoreCase(String name);
|
||||||
|
|
||||||
// Lookup by the canonical tag_path, used for idempotent canonical re-import (Phase 3).
|
// Lookup by the canonical tag_path, used for idempotent canonical re-import (Phase 3).
|
||||||
Optional<Tag> findBySourceRef(String sourceRef);
|
Optional<Tag> findBySourceRef(String sourceRef);
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.tag;
|
|||||||
|
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.Collection;
|
import java.util.Collection;
|
||||||
|
import java.util.Comparator;
|
||||||
import java.util.HashMap;
|
import java.util.HashMap;
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
import java.util.LinkedHashMap;
|
import java.util.LinkedHashMap;
|
||||||
@@ -55,10 +56,21 @@ public class TagService {
|
|||||||
return tagRepository.findBySourceRef(sourceRef);
|
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) {
|
public Tag findOrCreate(String name) {
|
||||||
String cleanName = name.trim();
|
String cleanName = name.trim();
|
||||||
return tagRepository.findByNameIgnoreCase(cleanName)
|
Optional<Tag> exact = tagRepository.findByName(cleanName);
|
||||||
.orElseGet(() -> tagRepository.save(Tag.builder().name(cleanName).build()));
|
if (exact.isPresent()) return exact.get(); // exact-case wins (edit round-trip replays the stored name)
|
||||||
|
List<Tag> 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)
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -53,20 +53,54 @@ class TagServiceTest {
|
|||||||
// ─── findOrCreate ─────────────────────────────────────────────────────────
|
// ─── findOrCreate ─────────────────────────────────────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void findOrCreate_returnsExisting_whenNameFound() {
|
void findOrCreate_exactCaseWins_overCaseInsensitiveSibling() {
|
||||||
Tag existing = Tag.builder().id(UUID.randomUUID()).name("Familie").build();
|
// "Geburt" (parent) and "geburt" (child) both exist; the edit round-trip replays the stored
|
||||||
when(tagRepository.findByNameIgnoreCase("Familie")).thenReturn(Optional.of(existing));
|
// 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());
|
verify(tagRepository, never()).save(any());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@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();
|
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);
|
when(tagRepository.save(any())).thenReturn(saved);
|
||||||
|
|
||||||
Tag result = tagService.findOrCreate("Krieg");
|
Tag result = tagService.findOrCreate("Krieg");
|
||||||
@@ -76,13 +110,15 @@ class TagServiceTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void findOrCreate_trimsWhitespaceBeforeLookup() {
|
void findOrCreate_trimsWhitespace_thenLandsOnCaseInsensitiveChild() {
|
||||||
Tag existing = Tag.builder().id(UUID.randomUUID()).name("Urlaub").build();
|
Tag child = Tag.builder().id(UUID.randomUUID()).name("weihnachten").build();
|
||||||
when(tagRepository.findByNameIgnoreCase("Urlaub")).thenReturn(Optional.of(existing));
|
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 ───────────────────────────────────────────────────────────────
|
// ─── update ───────────────────────────────────────────────────────────────
|
||||||
|
|||||||
Reference in New Issue
Block a user