diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index 5fe40e7e..c06fb72c 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -18,6 +18,7 @@ import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.user.UserService; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -87,7 +88,16 @@ public class JourneyItemService { .document(doc) .note(note) .build(); - JourneyItem saved = journeyItemRepository.save(item); + // saveAndFlush so the partial unique index on (geschichte_id, document_id) + // fires here, not at commit — two concurrent appends can both pass the + // exists() pre-check above, and the index is the atomic backstop (V74). + JourneyItem saved; + try { + saved = journeyItemRepository.saveAndFlush(item); + } catch (DataIntegrityViolationException e) { + throw DomainException.conflict(ErrorCode.JOURNEY_DOCUMENT_ALREADY_ADDED, + "Document already in journey: " + dto.getDocumentId()); + } UUID actorId = currentUser().getId(); auditService.logAfterCommit(AuditKind.JOURNEY_ITEM_ADDED, actorId, null, diff --git a/backend/src/main/resources/db/migration/V74__journey_items_document_dedup_and_note_length.sql b/backend/src/main/resources/db/migration/V74__journey_items_document_dedup_and_note_length.sql new file mode 100644 index 00000000..5939d2c6 --- /dev/null +++ b/backend/src/main/resources/db/migration/V74__journey_items_document_dedup_and_note_length.sql @@ -0,0 +1,21 @@ +-- Two constraints the service-level checks need as atomic backstops: +-- +-- 1. Partial unique index on (geschichte_id, document_id): the append dedup +-- guard is a check-then-insert (existsByGeschichteIdAndDocumentId), so two +-- concurrent appends of the same document can both pass the pre-check. +-- The index rejects the second INSERT; JourneyItemService.append translates +-- the DataIntegrityViolationException into the same 409 +-- JOURNEY_DOCUMENT_ALREADY_ADDED as the friendly pre-check. +-- Partial (WHERE document_id IS NOT NULL) — note-only interludes must not collide. +-- +-- 2. CHECK on note length: mirrors chk_text_length on transcription_blocks. +-- 2000 is the spec'd limit — JourneyItemService.MAX_NOTE_LENGTH, the frontend +-- maxlength, and the i18n error message all agree (#793). + +CREATE UNIQUE INDEX uq_journey_items_geschichte_document + ON journey_items (geschichte_id, document_id) + WHERE document_id IS NOT NULL; + +ALTER TABLE journey_items + ADD CONSTRAINT chk_journey_item_note_length + CHECK (note IS NULL OR length(note) <= 2000); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemConstraintsTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemConstraintsTest.java index a2615003..02284161 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemConstraintsTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemConstraintsTest.java @@ -74,6 +74,60 @@ class JourneyItemConstraintsTest { assertThat(condeferred).as("constraint must be initially deferred").isTrue(); } + @Test + void unique_index_rejects_duplicate_document_per_geschichte() { + // Atomic backstop for the service-level dedup pre-check (check-then-insert race). + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, document_id) VALUES (?, ?, ?, ?)", + UUID.randomUUID(), geschichteId, 10, documentId); + assertThatThrownBy(() -> + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, document_id) VALUES (?, ?, ?, ?)", + UUID.randomUUID(), geschichteId, 20, documentId)) + .isInstanceOf(DataIntegrityViolationException.class); + } + + @Test + void unique_index_allows_same_document_in_different_journeys() { + Geschichte other = geschichteRepository.save(Geschichte.builder() + .title("Andere Lesereise") + .status(GeschichteStatus.DRAFT) + .type(GeschichteType.JOURNEY) + .build()); + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, document_id) VALUES (?, ?, ?, ?)", + UUID.randomUUID(), geschichteId, 10, documentId); + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, document_id) VALUES (?, ?, ?, ?)", + UUID.randomUUID(), other.getId(), 10, documentId); + Integer count = jdbcTemplate.queryForObject( + "SELECT COUNT(*) FROM journey_items WHERE document_id = ?", Integer.class, documentId); + assertThat(count).isEqualTo(2); + } + + @Test + void unique_index_allows_multiple_note_only_items() { + // document_id IS NULL rows must not collide — the index is partial. + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, note) VALUES (?, ?, ?, ?)", + UUID.randomUUID(), geschichteId, 10, "erste Notiz"); + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, note) VALUES (?, ?, ?, ?)", + UUID.randomUUID(), geschichteId, 20, "zweite Notiz"); + Integer count = jdbcTemplate.queryForObject( + "SELECT COUNT(*) FROM journey_items WHERE geschichte_id = ?", Integer.class, geschichteId); + assertThat(count).isEqualTo(2); + } + + @Test + void note_length_check_rejects_2001_chars() { + assertThatThrownBy(() -> + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, note) VALUES (?, ?, ?, ?)", + UUID.randomUUID(), geschichteId, 10, "x".repeat(2001))) + .isInstanceOf(DataIntegrityViolationException.class); + } + @Test void position_check_rejects_nonpositive() { UUID itemId = UUID.randomUUID(); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java index b839e113..1be93320 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java @@ -94,9 +94,11 @@ class JourneyItemIntegrationTest { void items_are_returned_in_position_order_regardless_of_insertion_order() { Geschichte managed = geschichteRepository.findById(journey.getId()).orElseThrow(); + // Distinct content per item — V74's partial unique index forbids the same + // document twice in one journey, and ordering doesn't depend on it. JourneyItem third = JourneyItem.builder().geschichte(managed).position(3000).document(doc).build(); - JourneyItem first = JourneyItem.builder().geschichte(managed).position(1000).document(doc).build(); - JourneyItem second = JourneyItem.builder().geschichte(managed).position(2000).document(doc).build(); + JourneyItem first = JourneyItem.builder().geschichte(managed).position(1000).note("erstes").build(); + JourneyItem second = JourneyItem.builder().geschichte(managed).position(2000).note("zweites").build(); managed.getItems().addAll(List.of(third, first, second)); geschichteRepository.save(managed); em.flush(); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index 63b7be32..c309dcf5 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -149,7 +149,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.empty()); JourneyItem saved = savedItem(itemId, journey, 10, null, "Note"); - when(journeyItemRepository.save(any())).thenReturn(saved); + when(journeyItemRepository.saveAndFlush(any())).thenReturn(saved); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); dto.setNote("Note"); @@ -166,7 +166,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(2L); when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.of(40)); JourneyItem saved = savedItem(itemId, journey, 50, null, "Note"); - when(journeyItemRepository.save(any())).thenReturn(saved); + when(journeyItemRepository.saveAndFlush(any())).thenReturn(saved); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); dto.setNote("Note"); @@ -225,7 +225,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.empty()); JourneyItem saved = savedItem(itemId, journey, 10, null, "x".repeat(2000)); - when(journeyItemRepository.save(any())).thenReturn(saved); + when(journeyItemRepository.saveAndFlush(any())).thenReturn(saved); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); dto.setNote("x".repeat(2000)); @@ -328,7 +328,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(99L); when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.of(2000)); JourneyItem saved = savedItem(itemId, journey, 2010, null, "Note"); - when(journeyItemRepository.save(any())).thenReturn(saved); + when(journeyItemRepository.saveAndFlush(any())).thenReturn(saved); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); dto.setNote("Note"); @@ -336,6 +336,30 @@ class JourneyItemServiceTest { assertThat(journeyItemService.append(geschichteId, dto).position()).isEqualTo(2010); } + @Test + void append_maps_unique_index_violation_to_409_JOURNEY_DOCUMENT_ALREADY_ADDED() { + // Two concurrent appends can both pass the exists() pre-check; the partial + // unique index then rejects the second INSERT at flush. The service must + // translate that into the same friendly 409 as the pre-check. + Geschichte journey = journey(geschichteId); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(1L); + when(journeyItemRepository.existsByGeschichteIdAndDocumentId(eq(geschichteId), any())).thenReturn(false); + when(documentService.findSummaryByIdInternal(any())).thenReturn(makeDoc(UUID.randomUUID(), null, List.of(), null, null)); + when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.of(10)); + when(journeyItemRepository.saveAndFlush(any())) + .thenThrow(new org.springframework.dao.DataIntegrityViolationException( + "duplicate key value violates unique constraint \"uq_journey_items_geschichte_document\"")); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setDocumentId(UUID.randomUUID()); + + assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.JOURNEY_DOCUMENT_ALREADY_ADDED)); + } + @Test void append_audits_JOURNEY_ITEM_ADDED() { Geschichte journey = journey(geschichteId); @@ -343,7 +367,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.empty()); JourneyItem saved = savedItem(itemId, journey, 10, null, "Note"); - when(journeyItemRepository.save(any())).thenReturn(saved); + when(journeyItemRepository.saveAndFlush(any())).thenReturn(saved); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); dto.setNote("Note"); diff --git a/docs/architecture/db/db-orm.puml b/docs/architecture/db/db-orm.puml index a1ebd7d2..27cb0d38 100644 --- a/docs/architecture/db/db-orm.puml +++ b/docs/architecture/db/db-orm.puml @@ -377,9 +377,10 @@ package "Supporting" { geschichte_id : UUID <> document_id : UUID <> position : INTEGER NOT NULL CHECK (position > 0) - note : TEXT + note : TEXT CHECK (length <= 2000) == UNIQUE (geschichte_id, position) DEFERRABLE INITIALLY DEFERRED + UNIQUE (geschichte_id, document_id) WHERE document_id IS NOT NULL } } diff --git a/docs/architecture/db/db-relationships.puml b/docs/architecture/db/db-relationships.puml index 571bc61f..c610921e 100644 --- a/docs/architecture/db/db-relationships.puml +++ b/docs/architecture/db/db-relationships.puml @@ -131,5 +131,6 @@ geschichten_persons }o--|| geschichten : geschichte_id geschichten_persons }o--|| persons : person_id journey_items }o--|| geschichten : geschichte_id (ON DELETE CASCADE) journey_items }o--o| documents : document_id (ON DELETE SET NULL) +note right of journey_items : partial UNIQUE (geschichte_id, document_id)\nWHERE document_id IS NOT NULL (V74) @enduml