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

feat(journeyitem): V74 — partial unique index + note-length CHECK as atomic backstops

Browse Source 
The append dedup guard was check-then-insert: two concurrent appends of the
same document both pass exists() and both insert. The partial unique index
on (geschichte_id, document_id) WHERE document_id IS NOT NULL closes the
race; append saveAndFlush-es and maps the DataIntegrityViolationException to
the same 409 JOURNEY_DOCUMENT_ALREADY_ADDED as the friendly pre-check. The
CHECK on note length pins the 2000-char contract in the schema, mirroring
chk_text_length on transcription_blocks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-06-10 07:21:41 +02:00
parent 02efe738a0
commit b3d54a12c4
7 changed files with 122 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java
View File

@@ -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,

21
backend/src/main/resources/db/migration/V74__journey_items_document_dedup_and_note_length.sql Normal file
View File

@@ -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);

54
backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemConstraintsTest.java
View File

@@ -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();

6
backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java
View File

@@ -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();

34
backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java
View File

@@ -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");

3
docs/architecture/db/db-orm.puml
View File

@@ -377,9 +377,10 @@ package "Supporting" {
geschichte_id : UUID <<FK>>
document_id : UUID <<FK>>
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
}
}

1
docs/architecture/db/db-relationships.puml
View File

@@ -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