diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/PersonController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/PersonController.java index 329a1969..33be9f1b 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/PersonController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/PersonController.java @@ -34,11 +34,13 @@ public class PersonController { private final DocumentService documentService; @GetMapping + @RequirePermission(Permission.READ_ALL) public ResponseEntity> getPersons(@RequestParam(required = false) String q) { return ResponseEntity.ok(personService.findAll(q)); } @GetMapping("/{id}") + @RequirePermission(Permission.READ_ALL) public Person getPerson(@PathVariable UUID id) { return personService.getById(id); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java index 4d206cd7..da162ffa 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java @@ -1,5 +1,6 @@ package org.raddatz.familienarchiv.controller; +import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.raddatz.familienarchiv.dto.CreateTranscriptionBlockDTO; @@ -45,7 +46,7 @@ public class TranscriptionBlockController { @RequirePermission(Permission.WRITE_ALL) public TranscriptionBlock createBlock( @PathVariable UUID documentId, - @RequestBody CreateTranscriptionBlockDTO dto, + @Valid @RequestBody CreateTranscriptionBlockDTO dto, Authentication authentication) { UUID userId = requireUserId(authentication); return transcriptionService.createBlock(documentId, dto, userId); @@ -56,7 +57,7 @@ public class TranscriptionBlockController { public TranscriptionBlock updateBlock( @PathVariable UUID documentId, @PathVariable UUID blockId, - @RequestBody UpdateTranscriptionBlockDTO dto, + @Valid @RequestBody UpdateTranscriptionBlockDTO dto, Authentication authentication) { UUID userId = requireUserId(authentication); return transcriptionService.updateBlock(documentId, blockId, dto, userId); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/dto/CreateTranscriptionBlockDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/dto/CreateTranscriptionBlockDTO.java index 90f46359..d3d6a332 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/dto/CreateTranscriptionBlockDTO.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/dto/CreateTranscriptionBlockDTO.java @@ -1,14 +1,21 @@ package org.raddatz.familienarchiv.dto; +import jakarta.validation.Valid; import jakarta.validation.constraints.Min; import jakarta.validation.constraints.Positive; import lombok.AllArgsConstructor; +import lombok.Builder; import lombok.Data; import lombok.NoArgsConstructor; +import org.raddatz.familienarchiv.model.PersonMention; + +import java.util.ArrayList; +import java.util.List; @Data @NoArgsConstructor @AllArgsConstructor +@Builder public class CreateTranscriptionBlockDTO { @Min(0) private int pageNumber; @@ -22,4 +29,8 @@ public class CreateTranscriptionBlockDTO { private double height; private String text; private String label; + + @Valid + @Builder.Default + private List mentionedPersons = new ArrayList<>(); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/dto/UpdateTranscriptionBlockDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/dto/UpdateTranscriptionBlockDTO.java index f0577e6f..210d4c74 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/dto/UpdateTranscriptionBlockDTO.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/dto/UpdateTranscriptionBlockDTO.java @@ -1,13 +1,24 @@ package org.raddatz.familienarchiv.dto; +import jakarta.validation.Valid; import lombok.AllArgsConstructor; +import lombok.Builder; import lombok.Data; import lombok.NoArgsConstructor; +import org.raddatz.familienarchiv.model.PersonMention; + +import java.util.ArrayList; +import java.util.List; @Data @NoArgsConstructor @AllArgsConstructor +@Builder public class UpdateTranscriptionBlockDTO { private String text; private String label; + + @Valid + @Builder.Default + private List mentionedPersons = new ArrayList<>(); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java index 6d4e2533..d9a3a30f 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -15,6 +15,10 @@ public enum ErrorCode { ALIAS_NOT_FOUND, /** The submitted personType value is not allowed (e.g. SKIP is import-only). 400 */ INVALID_PERSON_TYPE, + /** A concurrent edit on a referenced transcription block prevented the rename + * from committing (optimistic-lock conflict). The whole rename rolls back; the + * client should refetch and retry. 409 */ + PERSON_RENAME_CONFLICT, // --- Documents --- /** A document with the given ID does not exist. 404 */ diff --git a/backend/src/main/java/org/raddatz/familienarchiv/model/PersonDisplayNameChangedEvent.java b/backend/src/main/java/org/raddatz/familienarchiv/model/PersonDisplayNameChangedEvent.java new file mode 100644 index 00000000..1b748c1a --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/model/PersonDisplayNameChangedEvent.java @@ -0,0 +1,23 @@ +package org.raddatz.familienarchiv.model; + +import java.util.UUID; + +/** + * Published by PersonService when a save changes Person.getDisplayName() — i.e. + * any mutation to the fields that DisplayNameFormatter consumes (title, + * firstName, lastName). Listeners on the transcription side rewrite block text + * and sidecar entries that reference the old name. + * + *

This is the first custom application event in the codebase. The previous + * only listener (OcrTrainingService.recoverOrphanedRuns) listens to Spring's + * built-in ApplicationReadyEvent. Future cross-domain decoupling should follow + * the same shape: record-typed event in model/, listener in the consuming + * domain's service/ package, synchronous @EventListener inside the publisher's + * transaction unless the workload genuinely needs to defer. + */ +public record PersonDisplayNameChangedEvent( + UUID personId, + String oldDisplayName, + String newDisplayName +) { +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/model/PersonMention.java b/backend/src/main/java/org/raddatz/familienarchiv/model/PersonMention.java new file mode 100644 index 00000000..79b232d6 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/model/PersonMention.java @@ -0,0 +1,30 @@ +package org.raddatz.familienarchiv.model; + +import io.swagger.v3.oas.annotations.media.Schema; +import jakarta.persistence.Column; +import jakarta.persistence.Embeddable; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + +import java.util.UUID; + +@Embeddable +@Data +@NoArgsConstructor +@AllArgsConstructor +public class PersonMention { + + @NotNull + @Column(name = "person_id", nullable = false) + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + private UUID personId; + + @NotNull + @Size(max = 200) + @Column(name = "display_name", nullable = false, length = 200) + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + private String displayName; +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/model/TranscriptionBlock.java b/backend/src/main/java/org/raddatz/familienarchiv/model/TranscriptionBlock.java index 8fc4f8e1..3ff9ca78 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/model/TranscriptionBlock.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/model/TranscriptionBlock.java @@ -7,6 +7,8 @@ import org.hibernate.annotations.CreationTimestamp; import org.hibernate.annotations.UpdateTimestamp; import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.List; import java.util.UUID; @Entity @@ -33,6 +35,14 @@ public class TranscriptionBlock { @Column(columnDefinition = "TEXT") private String text; + @ElementCollection(fetch = FetchType.LAZY) + @CollectionTable( + name = "transcription_block_mentioned_persons", + joinColumns = @JoinColumn(name = "block_id")) + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + @Builder.Default + private List mentionedPersons = new ArrayList<>(); + @Column(length = 200) private String label; diff --git a/backend/src/main/java/org/raddatz/familienarchiv/repository/TranscriptionBlockRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/repository/TranscriptionBlockRepository.java index 1bf2d108..e138cbe7 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/repository/TranscriptionBlockRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/repository/TranscriptionBlockRepository.java @@ -29,6 +29,15 @@ public interface TranscriptionBlockRepository extends JpaRepository findByAnnotationId(UUID annotationId); + @Query(""" + SELECT DISTINCT b FROM TranscriptionBlock b + JOIN FETCH b.mentionedPersons + WHERE b.id IN ( + SELECT bb.id FROM TranscriptionBlock bb JOIN bb.mentionedPersons m WHERE m.personId = :personId + ) + """) + List findByPersonIdWithMentionsFetched(@Param("personId") UUID personId); + void deleteByAnnotationId(UUID annotationId); int countByDocumentId(UUID documentId); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java new file mode 100644 index 00000000..a3c03206 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java @@ -0,0 +1,77 @@ +package org.raddatz.familienarchiv.service; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.raddatz.familienarchiv.model.PersonDisplayNameChangedEvent; +import org.raddatz.familienarchiv.model.PersonMention; +import org.raddatz.familienarchiv.model.TranscriptionBlock; +import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; +import org.springframework.context.event.EventListener; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Transcription-domain consumer of {@link PersonDisplayNameChangedEvent}. When + * Person.getDisplayName() flips during a rename, this listener rewrites every + * transcription block whose sidecar references the renamed person — both the + * literal "@OldName" inside block.text and the displayName carried in the + * {@link PersonMention} entries. + * + *

Synchronous on purpose: the rename and the propagation must commit as one + * transaction so a half-applied rewrite never reaches the archive. If the + * archive grows past tens of thousands of blocks, switch to + * {@code @TransactionalEventListener(AFTER_COMMIT) + @Async} — one annotation + * change. + */ +@Service +@RequiredArgsConstructor +@Slf4j +public class PersonMentionPropagationListener { + + private final TranscriptionBlockRepository blockRepository; + + @EventListener + @Transactional // Joins publisher's transaction — async switch requires @TransactionalEventListener(AFTER_COMMIT) + public void onPersonDisplayNameChanged(PersonDisplayNameChangedEvent event) { + List blocks = + blockRepository.findByPersonIdWithMentionsFetched(event.personId()); + if (blocks.isEmpty()) { + return; + } + + String oldNeedle = "@" + event.oldDisplayName(); + String newNeedle = "@" + event.newDisplayName(); + Pattern boundary = Pattern.compile( + Pattern.quote(oldNeedle) + "(?![\\p{L}0-9\\-]| (?=\\p{Lu}))"); + String replacement = Matcher.quoteReplacement(newNeedle); + + for (TranscriptionBlock block : blocks) { + rewriteBlockText(block, boundary, replacement); + for (PersonMention mention : block.getMentionedPersons()) { + if (mention.getPersonId().equals(event.personId())) { + mention.setDisplayName(event.newDisplayName()); + } + } + } + + blockRepository.saveAllAndFlush(blocks); + + log.info("Propagated rename {} → {} across {} block(s) for person {}", + event.oldDisplayName(), event.newDisplayName(), blocks.size(), event.personId()); + } + + // Match @OldName only at a token boundary: not followed by a letter/digit/hyphen + // (catches @Hans-Peter when renaming Hans) AND not followed by " " + // (catches @Hans Müller when renaming the single-name @Hans). False negatives — + // e.g. "@Hans Bekam" where Bekam is sentence-initial — are accepted as the + // conservative trade-off; the alternative (corruption) is irrecoverable. + private void rewriteBlockText(TranscriptionBlock block, Pattern boundary, String replacement) { + if (block.getText() != null) { + block.setText(boundary.matcher(block.getText()).replaceAll(replacement)); + } + } +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java index df04aa38..33873459 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java @@ -13,11 +13,14 @@ import org.raddatz.familienarchiv.dto.PersonUpdateDTO; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.Person; +import org.raddatz.familienarchiv.model.PersonDisplayNameChangedEvent; import org.raddatz.familienarchiv.model.PersonNameAlias; import org.raddatz.familienarchiv.model.PersonNameAliasType; import org.raddatz.familienarchiv.model.PersonType; import org.raddatz.familienarchiv.repository.PersonNameAliasRepository; import org.raddatz.familienarchiv.repository.PersonRepository; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -31,6 +34,7 @@ public class PersonService { private final PersonRepository personRepository; private final PersonNameAliasRepository aliasRepository; + private final ApplicationEventPublisher eventPublisher; public List findAll(String q) { if (q == null) { @@ -157,6 +161,7 @@ public class PersonService { validateYears(dto.getBirthYear(), dto.getDeathYear()); Person person = personRepository.findById(id) .orElseThrow(() -> DomainException.notFound(ErrorCode.PERSON_NOT_FOUND, "Person not found: " + id)); + String oldDisplayName = person.getDisplayName(); person.setPersonType(dto.getPersonType()); person.setTitle(dto.getTitle() == null || dto.getTitle().isBlank() ? null : dto.getTitle().trim()); person.setFirstName(dto.getFirstName()); @@ -165,7 +170,17 @@ public class PersonService { person.setNotes(dto.getNotes() == null || dto.getNotes().isBlank() ? null : dto.getNotes().trim()); person.setBirthYear(dto.getBirthYear()); person.setDeathYear(dto.getDeathYear()); - return personRepository.save(person); + Person saved = personRepository.save(person); + String newDisplayName = saved.getDisplayName(); + if (!Objects.equals(oldDisplayName, newDisplayName)) { + try { + eventPublisher.publishEvent(new PersonDisplayNameChangedEvent(id, oldDisplayName, newDisplayName)); + } catch (OptimisticLockingFailureException e) { + throw DomainException.conflict(ErrorCode.PERSON_RENAME_CONFLICT, + "A referenced transcription block was modified concurrently — rename rolled back"); + } + } + return saved; } @Transactional diff --git a/backend/src/main/resources/db/migration/V56__add_transcription_block_mentioned_persons.sql b/backend/src/main/resources/db/migration/V56__add_transcription_block_mentioned_persons.sql new file mode 100644 index 00000000..80a44ef4 --- /dev/null +++ b/backend/src/main/resources/db/migration/V56__add_transcription_block_mentioned_persons.sql @@ -0,0 +1,25 @@ +-- Sidecar table for @-mentions inside transcription_blocks.text. +-- Each row is one (block_id, person_id, display_name) tuple emitted by the +-- typeahead in the transcription editor. block.text contains the literal +-- "@DisplayName" — the UUID lives only here so historical text stays clean. +-- +-- Schema choice: child table via @ElementCollection (mirrors the established +-- UserGroup.permissions / group_permissions pattern), NOT JSONB. The "show +-- all blocks mentioning person X" query on the person detail page joins on +-- the indexed person_id column — equally fast as JSONB GIN containment, no +-- new dependency. document_comments.comment_mentions stays as a many-to-many +-- to AppUser; the divergence is intentional: Person mentions need lazy +-- degradation when a person is deleted (no FK), while user mentions don't. +-- +-- No FK on person_id: when a Person is deleted we want @Auguste Raddatz to +-- remain visible as plain unlinked text inside the transcription rather than +-- vanishing or cascade-deleting the block. + +CREATE TABLE transcription_block_mentioned_persons ( + block_id UUID NOT NULL REFERENCES transcription_blocks(id) ON DELETE CASCADE, + person_id UUID NOT NULL, + display_name VARCHAR(200) NOT NULL +); + +CREATE INDEX idx_tbmp_person_id ON transcription_block_mentioned_persons(person_id); +CREATE INDEX idx_tbmp_block_id ON transcription_block_mentioned_persons(block_id); diff --git a/backend/src/main/resources/db/migration/V57__add_tbmp_unique_constraint.sql b/backend/src/main/resources/db/migration/V57__add_tbmp_unique_constraint.sql new file mode 100644 index 00000000..b1945c73 --- /dev/null +++ b/backend/src/main/resources/db/migration/V57__add_tbmp_unique_constraint.sql @@ -0,0 +1,5 @@ +-- Prevent duplicate sidecar rows for the same (block, person) pair. +-- @ElementCollection uses DELETE+INSERT per update so normal JPA writes can't +-- create duplicates, but a raw-SQL import or concurrent bypass of JPA could. +ALTER TABLE transcription_block_mentioned_persons + ADD CONSTRAINT uq_tbmp_block_person UNIQUE (block_id, person_id); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java index e31e2ad0..41a4a0ac 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java @@ -57,6 +57,13 @@ class PersonControllerTest { @Test @WithMockUser + void getPersons_returns403_whenMissingReadAllPermission() throws Exception { + mockMvc.perform(get("/api/persons")) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = "READ_ALL") void getPersons_returns200_withEmptyList() throws Exception { when(personService.findAll(null)).thenReturn(Collections.emptyList()); mockMvc.perform(get("/api/persons")) @@ -64,7 +71,7 @@ class PersonControllerTest { } @Test - @WithMockUser + @WithMockUser(authorities = "READ_ALL") void getPersons_delegatesQueryParam_toService() throws Exception { PersonSummaryDTO dto = mockPersonSummary("Hans", "Müller"); when(personService.findAll("Hans")).thenReturn(List.of(dto)); @@ -100,6 +107,13 @@ class PersonControllerTest { @Test @WithMockUser + void getPerson_returns403_whenMissingReadAllPermission() throws Exception { + mockMvc.perform(get("/api/persons/{id}", UUID.randomUUID())) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = "READ_ALL") void getPerson_returns200_whenFound() throws Exception { UUID id = UUID.randomUUID(); Person person = Person.builder().id(id).firstName("Anna").lastName("Schmidt").build(); @@ -319,6 +333,21 @@ class PersonControllerTest { .andExpect(jsonPath("$.lastName").value("Müller")); } + @Test + @WithMockUser(authorities = "WRITE_ALL") + void updatePerson_returns409_whenRenameConflict() throws Exception { + UUID id = UUID.randomUUID(); + when(personService.updatePerson(eq(id), any())) + .thenThrow(DomainException.conflict(ErrorCode.PERSON_RENAME_CONFLICT, + "Concurrent block edit during rename")); + + mockMvc.perform(put("/api/persons/{id}", id) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"firstName\":\"Augusta\",\"lastName\":\"Raddatz\",\"personType\":\"PERSON\"}")) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.code").value("PERSON_RENAME_CONFLICT")); + } + // ─── POST /api/persons/{id}/merge ───────────────────────────────────────── @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java index 255d19ee..bed7ff8f 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java @@ -183,6 +183,36 @@ class TranscriptionBlockControllerTest { .andExpect(status().isUnauthorized()); } + @Test + @WithMockUser(authorities = "WRITE_ALL") + void createBlock_returns400_whenMentionedPersonDisplayNameExceeds200Chars() throws Exception { + when(userService.findByEmail(any())).thenReturn(mockUser()); + String longName = "A".repeat(201); + String body = "{\"pageNumber\":1,\"x\":0.1,\"y\":0.2,\"width\":0.3,\"height\":0.4,\"text\":\"x\"," + + "\"mentionedPersons\":[{\"personId\":\"" + UUID.randomUUID() + + "\",\"displayName\":\"" + longName + "\"}]}"; + + mockMvc.perform(post(URL_BASE) + .contentType(MediaType.APPLICATION_JSON) + .content(body)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("VALIDATION_ERROR")); + } + + @Test + @WithMockUser(authorities = "WRITE_ALL") + void createBlock_returns400_whenMentionedPersonPersonIdIsNull() throws Exception { + when(userService.findByEmail(any())).thenReturn(mockUser()); + String body = "{\"pageNumber\":1,\"x\":0.1,\"y\":0.2,\"width\":0.3,\"height\":0.4,\"text\":\"x\"," + + "\"mentionedPersons\":[{\"personId\":null,\"displayName\":\"Auguste Raddatz\"}]}"; + + mockMvc.perform(post(URL_BASE) + .contentType(MediaType.APPLICATION_JSON) + .content(body)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("VALIDATION_ERROR")); + } + // ─── PUT /api/documents/{id}/transcription-blocks/{blockId} ───────────── @Test @@ -221,6 +251,34 @@ class TranscriptionBlockControllerTest { .andExpect(jsonPath("$.label").value("Anrede")); } + @Test + @WithMockUser(authorities = "WRITE_ALL") + void updateBlock_returns400_whenMentionedPersonDisplayNameExceeds200Chars() throws Exception { + when(userService.findByEmail(any())).thenReturn(mockUser()); + String longName = "A".repeat(201); + String body = "{\"text\":\"x\",\"mentionedPersons\":[{\"personId\":\"" + + UUID.randomUUID() + "\",\"displayName\":\"" + longName + "\"}]}"; + + mockMvc.perform(put(URL_BLOCK) + .contentType(MediaType.APPLICATION_JSON) + .content(body)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("VALIDATION_ERROR")); + } + + @Test + @WithMockUser(authorities = "WRITE_ALL") + void updateBlock_returns400_whenMentionedPersonPersonIdIsNull() throws Exception { + when(userService.findByEmail(any())).thenReturn(mockUser()); + String body = "{\"text\":\"x\",\"mentionedPersons\":[{\"personId\":null,\"displayName\":\"Auguste Raddatz\"}]}"; + + mockMvc.perform(put(URL_BLOCK) + .contentType(MediaType.APPLICATION_JSON) + .content(body)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("VALIDATION_ERROR")); + } + @Test @WithMockUser(authorities = "WRITE_ALL") void updateBlock_returns404_whenBlockDoesNotExist() throws Exception { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/repository/TranscriptionBlockMentionsRepositoryTest.java b/backend/src/test/java/org/raddatz/familienarchiv/repository/TranscriptionBlockMentionsRepositoryTest.java new file mode 100644 index 00000000..87af6bd5 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/repository/TranscriptionBlockMentionsRepositoryTest.java @@ -0,0 +1,127 @@ +package org.raddatz.familienarchiv.repository; + +import jakarta.persistence.EntityManager; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.config.FlywayConfig; +import org.raddatz.familienarchiv.model.Document; +import org.raddatz.familienarchiv.model.DocumentAnnotation; +import org.raddatz.familienarchiv.model.DocumentStatus; +import org.raddatz.familienarchiv.model.PersonMention; +import org.raddatz.familienarchiv.model.TranscriptionBlock; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.jdbc.test.autoconfigure.AutoConfigureTestDatabase; +import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest; +import org.springframework.context.annotation.Import; + +import java.util.List; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; + +@DataJpaTest +@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) +@Import({PostgresContainerConfig.class, FlywayConfig.class}) +class TranscriptionBlockMentionsRepositoryTest { + + @Autowired TranscriptionBlockRepository blockRepository; + @Autowired DocumentRepository documentRepository; + @Autowired AnnotationRepository annotationRepository; + @Autowired EntityManager em; + + private UUID documentId; + private UUID annotationId; + + @BeforeEach + void setUp() { + Document doc = documentRepository.save(Document.builder() + .title("Letter") + .originalFilename("letter.pdf") + .status(DocumentStatus.UPLOADED) + .build()); + documentId = doc.getId(); + + DocumentAnnotation annotation = annotationRepository.save(DocumentAnnotation.builder() + .documentId(documentId) + .pageNumber(1) + .x(0.1).y(0.2).width(0.3).height(0.4) + .color("#00C7B1") + .build()); + annotationId = annotation.getId(); + } + + @Test + void mentionedPersons_roundTripsTwoEntries() { + UUID auguste = UUID.randomUUID(); + UUID hermann = UUID.randomUUID(); + + TranscriptionBlock saved = blockRepository.saveAndFlush(TranscriptionBlock.builder() + .annotationId(annotationId) + .documentId(documentId) + .text("Liebe Tante @Auguste Raddatz, Onkel @Hermann Müller schreibt …") + .sortOrder(0) + .mentionedPersons(List.of( + new PersonMention(auguste, "Auguste Raddatz"), + new PersonMention(hermann, "Hermann Müller") + )) + .build()); + + em.clear(); + + TranscriptionBlock reloaded = blockRepository.findById(saved.getId()).orElseThrow(); + + assertThat(reloaded.getMentionedPersons()) + .extracting(PersonMention::getPersonId, PersonMention::getDisplayName) + .containsExactlyInAnyOrder( + org.assertj.core.groups.Tuple.tuple(auguste, "Auguste Raddatz"), + org.assertj.core.groups.Tuple.tuple(hermann, "Hermann Müller")); + } + + @Test + void mentionedPersons_defaultsToEmptyList_whenNotSet() { + TranscriptionBlock saved = blockRepository.saveAndFlush(TranscriptionBlock.builder() + .annotationId(annotationId) + .documentId(documentId) + .text("Plain text without mentions") + .sortOrder(0) + .build()); + + em.clear(); + + TranscriptionBlock reloaded = blockRepository.findById(saved.getId()).orElseThrow(); + assertThat(reloaded.getMentionedPersons()).isEmpty(); + } + + @Test + void findByPersonIdWithMentionsFetched_returnsOnlyBlocksReferencingPerson_withMentionsLoaded() { + UUID augusteId = UUID.randomUUID(); + UUID hermannId = UUID.randomUUID(); + + blockRepository.saveAndFlush(TranscriptionBlock.builder() + .annotationId(annotationId).documentId(documentId) + .text("Brief von @Auguste Raddatz an @Hermann Müller.") + .sortOrder(0) + .mentionedPersons(List.of( + new PersonMention(augusteId, "Auguste Raddatz"), + new PersonMention(hermannId, "Hermann Müller"))) + .build()); + blockRepository.saveAndFlush(TranscriptionBlock.builder() + .annotationId(annotationId).documentId(documentId) + .text("Unrelated block without Auguste.") + .sortOrder(1) + .mentionedPersons(List.of(new PersonMention(hermannId, "Hermann Müller"))) + .build()); + em.clear(); + + List result = + blockRepository.findByPersonIdWithMentionsFetched(augusteId); + + assertThat(result).hasSize(1); + assertThat(result.get(0).getMentionedPersons()) + .extracting(PersonMention::getPersonId, PersonMention::getDisplayName) + .containsExactlyInAnyOrder( + org.assertj.core.groups.Tuple.tuple(augusteId, "Auguste Raddatz"), + org.assertj.core.groups.Tuple.tuple(hermannId, "Hermann Müller")); + } +} diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListenerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListenerTest.java new file mode 100644 index 00000000..c6b89716 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListenerTest.java @@ -0,0 +1,227 @@ +package org.raddatz.familienarchiv.service; + +import jakarta.persistence.EntityManager; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.config.FlywayConfig; +import org.raddatz.familienarchiv.model.Document; +import org.raddatz.familienarchiv.model.DocumentAnnotation; +import org.raddatz.familienarchiv.model.DocumentStatus; +import org.raddatz.familienarchiv.model.Person; +import org.raddatz.familienarchiv.model.PersonDisplayNameChangedEvent; +import org.raddatz.familienarchiv.model.PersonMention; +import org.raddatz.familienarchiv.model.TranscriptionBlock; +import org.raddatz.familienarchiv.repository.AnnotationRepository; +import org.raddatz.familienarchiv.repository.DocumentRepository; +import org.raddatz.familienarchiv.repository.PersonRepository; +import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.jdbc.test.autoconfigure.AutoConfigureTestDatabase; +import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest; +import org.springframework.context.annotation.Import; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +@DataJpaTest +@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) +@Import({PostgresContainerConfig.class, FlywayConfig.class}) +class PersonMentionPropagationListenerTest { + + @Autowired TranscriptionBlockRepository blockRepository; + @Autowired DocumentRepository documentRepository; + @Autowired AnnotationRepository annotationRepository; + @Autowired PersonRepository personRepository; + @Autowired EntityManager em; + + private PersonMentionPropagationListener listener; + + private UUID documentId; + private UUID annotationId; + + @BeforeEach + void setUp() { + listener = new PersonMentionPropagationListener(blockRepository); + + Document doc = documentRepository.save(Document.builder() + .title("Letter").originalFilename("letter.pdf") + .status(DocumentStatus.UPLOADED).build()); + documentId = doc.getId(); + DocumentAnnotation annotation = annotationRepository.save(DocumentAnnotation.builder() + .documentId(documentId).pageNumber(1) + .x(0.1).y(0.2).width(0.3).height(0.4) + .color("#00C7B1").build()); + annotationId = annotation.getId(); + } + + private TranscriptionBlock saveBlock(String text, List mentions) { + return blockRepository.saveAndFlush(TranscriptionBlock.builder() + .annotationId(annotationId).documentId(documentId) + .text(text).sortOrder(0) + .mentionedPersons(mentions).build()); + } + + private UUID savedPersonId(String firstName, String lastName) { + Person p = personRepository.save(Person.builder() + .firstName(firstName).lastName(lastName).build()); + return p.getId(); + } + + @Test + void rewritesTextAndSidecar_whenSingleBlockReferencesRenamedPerson() { + UUID personId = savedPersonId("Auguste", "Raddatz"); + TranscriptionBlock saved = saveBlock( + "Liebe Tante @Auguste Raddatz, danke für den Brief.", + List.of(new PersonMention(personId, "Auguste Raddatz"))); + em.clear(); + + listener.onPersonDisplayNameChanged( + new PersonDisplayNameChangedEvent(personId, "Auguste Raddatz", "Augusta Raddatz")); + blockRepository.flush(); + em.clear(); + + TranscriptionBlock reloaded = blockRepository.findById(saved.getId()).orElseThrow(); + assertThat(reloaded.getText()).isEqualTo("Liebe Tante @Augusta Raddatz, danke für den Brief."); + assertThat(reloaded.getMentionedPersons()) + .extracting(PersonMention::getDisplayName) + .containsExactly("Augusta Raddatz"); + } + + @Test + void doesNotMatchPartialName_whenAnotherMentionShares_a_substring_with_renamed_person() { + UUID hansPeterId = savedPersonId("Hans-Peter", "Müller"); + UUID hansId = savedPersonId("Hans", "Müller"); + TranscriptionBlock saved = saveBlock( + "Heute hat @Hans-Peter Müller wieder mit @Hans Müller gesprochen.", + List.of( + new PersonMention(hansPeterId, "Hans-Peter Müller"), + new PersonMention(hansId, "Hans Müller"))); + em.clear(); + + listener.onPersonDisplayNameChanged( + new PersonDisplayNameChangedEvent(hansId, "Hans Müller", "Hans Schmidt")); + blockRepository.flush(); + em.clear(); + + TranscriptionBlock reloaded = blockRepository.findById(saved.getId()).orElseThrow(); + assertThat(reloaded.getText()) + .isEqualTo("Heute hat @Hans-Peter Müller wieder mit @Hans Schmidt gesprochen."); + assertThat(reloaded.getMentionedPersons()) + .extracting(PersonMention::getPersonId, PersonMention::getDisplayName) + .containsExactlyInAnyOrder( + org.assertj.core.groups.Tuple.tuple(hansPeterId, "Hans-Peter Müller"), + org.assertj.core.groups.Tuple.tuple(hansId, "Hans Schmidt")); + } + + @Test + void doesNotCorruptCompositeMention_whenRenamingSingleWordPerson() { + UUID hansMüllerId = savedPersonId("Hans", "Müller"); + UUID hansId = savedPersonId(null, "Hans"); + TranscriptionBlock saved = saveBlock( + "@Hans Müller schrieb. Auch @Hans hat geschrieben.", + List.of( + new PersonMention(hansMüllerId, "Hans Müller"), + new PersonMention(hansId, "Hans"))); + em.clear(); + + listener.onPersonDisplayNameChanged( + new PersonDisplayNameChangedEvent(hansId, "Hans", "Henry")); + blockRepository.flush(); + em.clear(); + + TranscriptionBlock reloaded = blockRepository.findById(saved.getId()).orElseThrow(); + assertThat(reloaded.getText()) + .isEqualTo("@Hans Müller schrieb. Auch @Henry hat geschrieben."); + assertThat(reloaded.getMentionedPersons()) + .extracting(PersonMention::getPersonId, PersonMention::getDisplayName) + .containsExactlyInAnyOrder( + org.assertj.core.groups.Tuple.tuple(hansMüllerId, "Hans Müller"), + org.assertj.core.groups.Tuple.tuple(hansId, "Henry")); + } + + @Test + void rewritesAllOccurrences_whenSameMentionAppearsTwiceInBlock() { + UUID personId = savedPersonId("Auguste", "Raddatz"); + TranscriptionBlock saved = saveBlock( + "Heute hat @Auguste Raddatz geschrieben, dann hat @Auguste Raddatz nochmal geschrieben.", + List.of(new PersonMention(personId, "Auguste Raddatz"))); + em.clear(); + + listener.onPersonDisplayNameChanged( + new PersonDisplayNameChangedEvent(personId, "Auguste Raddatz", "Augusta Raddatz")); + blockRepository.flush(); + em.clear(); + + TranscriptionBlock reloaded = blockRepository.findById(saved.getId()).orElseThrow(); + assertThat(reloaded.getText()) + .isEqualTo("Heute hat @Augusta Raddatz geschrieben, dann hat @Augusta Raddatz nochmal geschrieben."); + assertThat(reloaded.getMentionedPersons()) + .extracting(PersonMention::getDisplayName) + .containsExactly("Augusta Raddatz"); + } + + @Test + void propagatesAcross200Blocks_inUnderFiveSeconds_latencyFloor() { + UUID personId = savedPersonId("Auguste", "Raddatz"); + List blockIds = new ArrayList<>(); + for (int i = 0; i < 200; i++) { + TranscriptionBlock saved = blockRepository.save(TranscriptionBlock.builder() + .annotationId(annotationId).documentId(documentId) + .text("Block " + i + " mentions @Auguste Raddatz here.") + .sortOrder(i) + .mentionedPersons(List.of(new PersonMention(personId, "Auguste Raddatz"))) + .build()); + blockIds.add(saved.getId()); + } + blockRepository.flush(); + em.clear(); + + long start = System.nanoTime(); + listener.onPersonDisplayNameChanged( + new PersonDisplayNameChangedEvent(personId, "Auguste Raddatz", "Augusta Raddatz")); + blockRepository.flush(); + long elapsedMs = (System.nanoTime() - start) / 1_000_000; + + assertThat(elapsedMs) + .as("Propagation across 200 blocks must stay under 5s — merge-blocking regression floor") + .isLessThan(5000L); + + em.clear(); + TranscriptionBlock first = blockRepository.findById(blockIds.get(0)).orElseThrow(); + assertThat(first.getText()).contains("@Augusta Raddatz"); + } + + @Test + void doesNotThrow_whenBlockTextIsNull() { + UUID personId = savedPersonId("Auguste", "Raddatz"); + saveBlock(null, List.of(new PersonMention(personId, "Auguste Raddatz"))); + em.clear(); + + assertThatCode(() -> listener.onPersonDisplayNameChanged( + new PersonDisplayNameChangedEvent(personId, "Auguste Raddatz", "Augusta Raddatz"))) + .doesNotThrowAnyException(); + } + + @Test + void leavesUnrelatedBlockUntouched_whenNoSidecarReferencesPerson() { + UUID personId = savedPersonId("Auguste", "Raddatz"); + TranscriptionBlock saved = saveBlock( + "Plain text without any mentions.", + List.of()); + em.clear(); + + listener.onPersonDisplayNameChanged( + new PersonDisplayNameChangedEvent(personId, "Auguste Raddatz", "Augusta Raddatz")); + blockRepository.flush(); + em.clear(); + + TranscriptionBlock reloaded = blockRepository.findById(saved.getId()).orElseThrow(); + assertThat(reloaded.getText()).isEqualTo("Plain text without any mentions."); + assertThat(reloaded.getMentionedPersons()).isEmpty(); + } +} diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java index da2fdde4..b502ac7c 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java @@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.service; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -9,14 +10,22 @@ import org.raddatz.familienarchiv.dto.PersonNameAliasDTO; import org.raddatz.familienarchiv.dto.PersonSummaryDTO; import org.raddatz.familienarchiv.dto.PersonUpdateDTO; import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.Person; +import org.raddatz.familienarchiv.model.PersonDisplayNameChangedEvent; +import org.raddatz.familienarchiv.model.PersonMention; import org.raddatz.familienarchiv.model.PersonNameAlias; import org.raddatz.familienarchiv.model.PersonNameAliasType; import org.raddatz.familienarchiv.model.PersonType; +import org.raddatz.familienarchiv.model.TranscriptionBlock; import org.raddatz.familienarchiv.repository.PersonNameAliasRepository; import org.raddatz.familienarchiv.repository.PersonRepository; +import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.orm.ObjectOptimisticLockingFailureException; import org.springframework.web.server.ResponseStatusException; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.UUID; @@ -31,6 +40,7 @@ class PersonServiceTest { @Mock PersonRepository personRepository; @Mock PersonNameAliasRepository aliasRepository; + @Mock ApplicationEventPublisher eventPublisher; @InjectMocks PersonService personService; // ─── getById ───────────────────────────────────────────────────────────── @@ -242,6 +252,121 @@ class PersonServiceTest { assertThat(result.getAlias()).isEqualTo("Anna Alt"); } + // ─── updatePerson (display-name change event) ──────────────────────────── + + @Test + void updatePerson_publishesEvent_whenTitleChanges() { + UUID id = UUID.randomUUID(); + Person existing = Person.builder() + .id(id).title("Herr").firstName("Auguste").lastName("Raddatz") + .personType(PersonType.PERSON).build(); + String oldName = existing.getDisplayName(); + + when(personRepository.findById(id)).thenReturn(Optional.of(existing)); + when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + PersonUpdateDTO dto = new PersonUpdateDTO(); + dto.setPersonType(PersonType.PERSON); + dto.setTitle("Frau"); dto.setFirstName("Auguste"); dto.setLastName("Raddatz"); + + personService.updatePerson(id, dto); + + ArgumentCaptor captor = + ArgumentCaptor.forClass(PersonDisplayNameChangedEvent.class); + verify(eventPublisher).publishEvent(captor.capture()); + + PersonDisplayNameChangedEvent event = captor.getValue(); + assertThat(event.personId()).isEqualTo(id); + assertThat(event.oldDisplayName()).isEqualTo(oldName); + assertThat(event.newDisplayName()) + .isNotEqualTo(oldName) + .contains("Frau"); + } + + @Test + void updatePerson_doesNotPublishEvent_whenDisplayNameFieldsUnchanged() { + UUID id = UUID.randomUUID(); + Person existing = Person.builder() + .id(id).firstName("Auguste").lastName("Raddatz") + .personType(PersonType.PERSON).alias("old alias").build(); + + when(personRepository.findById(id)).thenReturn(Optional.of(existing)); + when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + PersonUpdateDTO dto = new PersonUpdateDTO(); + dto.setPersonType(PersonType.PERSON); + dto.setFirstName("Auguste"); dto.setLastName("Raddatz"); + dto.setAlias("new alias"); + + personService.updatePerson(id, dto); + + verify(eventPublisher, never()).publishEvent(any(PersonDisplayNameChangedEvent.class)); + } + + @Test + void updatePerson_throwsConflict_whenBlockSaveAllAndFlushHitsOptimisticLock() { + // Wire a real PersonMentionPropagationListener with a mocked block repository + // that throws on saveAllAndFlush. The publisher mock routes events to the + // listener so the catch path traverses the same call chain as production: + // PersonService → publishEvent → listener → saveAllAndFlush throws → catch. + UUID id = UUID.randomUUID(); + Person existing = Person.builder() + .id(id).firstName("Auguste").lastName("Raddatz") + .personType(PersonType.PERSON).build(); + + TranscriptionBlock referencingBlock = TranscriptionBlock.builder() + .id(UUID.randomUUID()).documentId(UUID.randomUUID()).annotationId(UUID.randomUUID()) + .text("Brief von @Auguste Raddatz").sortOrder(0) + .mentionedPersons(new ArrayList<>(List.of(new PersonMention(id, "Auguste Raddatz")))) + .build(); + + TranscriptionBlockRepository blockRepo = mock(TranscriptionBlockRepository.class); + when(blockRepo.findByPersonIdWithMentionsFetched(id)) + .thenReturn(List.of(referencingBlock)); + when(blockRepo.saveAllAndFlush(any())) + .thenThrow(new ObjectOptimisticLockingFailureException( + TranscriptionBlock.class, referencingBlock.getId())); + + PersonMentionPropagationListener realListener = + new PersonMentionPropagationListener(blockRepo); + + when(personRepository.findById(id)).thenReturn(Optional.of(existing)); + when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + doAnswer(inv -> { + realListener.onPersonDisplayNameChanged(inv.getArgument(0)); + return null; + }).when(eventPublisher).publishEvent(any(PersonDisplayNameChangedEvent.class)); + + PersonUpdateDTO dto = new PersonUpdateDTO(); + dto.setPersonType(PersonType.PERSON); + dto.setFirstName("Augusta"); dto.setLastName("Raddatz"); + + assertThatThrownBy(() -> personService.updatePerson(id, dto)) + .isInstanceOf(DomainException.class) + .matches(e -> ((DomainException) e).getCode() == ErrorCode.PERSON_RENAME_CONFLICT) + .matches(e -> ((DomainException) e).getStatus().value() == 409); + } + + @Test + void updatePerson_doesNotPublishEvent_whenOnlyNotesChanges() { + UUID id = UUID.randomUUID(); + Person existing = Person.builder() + .id(id).firstName("Auguste").lastName("Raddatz") + .personType(PersonType.PERSON).notes("first note").build(); + + when(personRepository.findById(id)).thenReturn(Optional.of(existing)); + when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + PersonUpdateDTO dto = new PersonUpdateDTO(); + dto.setPersonType(PersonType.PERSON); + dto.setFirstName("Auguste"); dto.setLastName("Raddatz"); + dto.setNotes("revised note"); + + personService.updatePerson(id, dto); + + verify(eventPublisher, never()).publishEvent(any(PersonDisplayNameChangedEvent.class)); + } + // ─── findOrCreateByAlias ───────────────────────────────────────────────── @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java index 7abfb237..8aa6ee99 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java @@ -98,7 +98,9 @@ class TranscriptionServiceTest { return b; }); - CreateTranscriptionBlockDTO dto = new CreateTranscriptionBlockDTO(1, 0.1, 0.2, 0.3, 0.4, "hello", null); + CreateTranscriptionBlockDTO dto = CreateTranscriptionBlockDTO.builder() + .pageNumber(1).x(0.1).y(0.2).width(0.3).height(0.4) + .text("hello").build(); TranscriptionBlock result = transcriptionService.createBlock(docId, dto, userId); @@ -168,7 +170,7 @@ class TranscriptionServiceTest { when(documentService.getDocumentById(any())).thenReturn( Document.builder().scriptType(ScriptType.TYPEWRITER).build()); - UpdateTranscriptionBlockDTO dto = new UpdateTranscriptionBlockDTO("new text", null); + UpdateTranscriptionBlockDTO dto = UpdateTranscriptionBlockDTO.builder().text("new text").build(); TranscriptionBlock result = transcriptionService.updateBlock(docId, blockId, dto, userId); @@ -189,7 +191,7 @@ class TranscriptionServiceTest { when(documentService.getDocumentById(any())).thenReturn( Document.builder().scriptType(ScriptType.TYPEWRITER).build()); - UpdateTranscriptionBlockDTO dto = new UpdateTranscriptionBlockDTO("text", "Anrede"); + UpdateTranscriptionBlockDTO dto = UpdateTranscriptionBlockDTO.builder().text("text").label("Anrede").build(); TranscriptionBlock result = transcriptionService.updateBlock(docId, blockId, dto, UUID.randomUUID()); @@ -208,7 +210,7 @@ class TranscriptionServiceTest { Document.builder().scriptType(ScriptType.TYPEWRITER).build()); TranscriptionBlock result = transcriptionService.updateBlock( - docId, blockId, new UpdateTranscriptionBlockDTO("new", null), UUID.randomUUID()); + docId, blockId, UpdateTranscriptionBlockDTO.builder().text("new").build(), UUID.randomUUID()); assertThat(result.getSource()).isEqualTo(BlockSource.MANUAL); } @@ -226,7 +228,7 @@ class TranscriptionServiceTest { when(documentService.getDocumentById(any())).thenReturn( Document.builder().scriptType(ScriptType.HANDWRITING_KURRENT).sender(sender).build()); - transcriptionService.updateBlock(docId, blockId, new UpdateTranscriptionBlockDTO("text", null), UUID.randomUUID()); + transcriptionService.updateBlock(docId, blockId, UpdateTranscriptionBlockDTO.builder().text("text").build(), UUID.randomUUID()); verify(senderModelService).checkAndTriggerTraining(senderId); } @@ -242,7 +244,7 @@ class TranscriptionServiceTest { when(documentService.getDocumentById(any())).thenReturn( Document.builder().scriptType(ScriptType.HANDWRITING_KURRENT).build()); - transcriptionService.updateBlock(docId, blockId, new UpdateTranscriptionBlockDTO("text", null), UUID.randomUUID()); + transcriptionService.updateBlock(docId, blockId, UpdateTranscriptionBlockDTO.builder().text("text").build(), UUID.randomUUID()); verify(senderModelService, never()).checkAndTriggerTraining(any()); } @@ -477,7 +479,7 @@ class TranscriptionServiceTest { Document.builder().scriptType(ScriptType.TYPEWRITER).build()); when(annotationRepository.findById(annotId)).thenReturn(Optional.of(annotation)); - transcriptionService.updateBlock(docId, blockId, new UpdateTranscriptionBlockDTO("new text", null), userId); + transcriptionService.updateBlock(docId, blockId, UpdateTranscriptionBlockDTO.builder().text("new text").build(), userId); @SuppressWarnings("unchecked") ArgumentCaptor> payloadCaptor = ArgumentCaptor.forClass(Map.class); @@ -502,7 +504,7 @@ class TranscriptionServiceTest { when(documentService.getDocumentById(any())).thenReturn( Document.builder().scriptType(ScriptType.TYPEWRITER).build()); - transcriptionService.updateBlock(docId, blockId, new UpdateTranscriptionBlockDTO("same text", null), userId); + transcriptionService.updateBlock(docId, blockId, UpdateTranscriptionBlockDTO.builder().text("same text").build(), userId); verify(auditService, never()).logAfterCommit(any(), any(), any(), any()); } diff --git a/docs/adr/006-synchronous-domain-events-in-transaction.md b/docs/adr/006-synchronous-domain-events-in-transaction.md new file mode 100644 index 00000000..fbec5ac3 --- /dev/null +++ b/docs/adr/006-synchronous-domain-events-in-transaction.md @@ -0,0 +1,55 @@ +# ADR-006: Synchronous domain events inside the publisher's transaction + +## Status + +Accepted + +## Context + +Issue #362 introduced the first cross-domain side-effect in this codebase: when a Person's display name changes, every transcription block that mentions the person must be rewritten — both `block.text` (the literal `@OldName` substring) and the `mentionedPersons` sidecar (the `displayName` field on the matching `PersonMention`). The rewrite is bidirectionally referential — Person depends on Transcription to make the rename atomic, and Transcription depends on Person to know what the new display name is. + +A direct method call from `PersonService` into `TranscriptionBlockService` would invert the existing dependency arrow (Document → Person, not Person → Transcription) and introduce a runtime-circular reference at the package level. Avoiding the cycle while keeping the rename atomic is the constraint this ADR addresses. + +Two prior pieces of infrastructure constrain the solution: + +- `transcription_blocks.version` (JPA `@Version`) — concurrent autosave on a referenced block must roll back the rename instead of silently overwriting the autosave. +- `OcrTrainingService.recoverOrphanedRuns` is the only existing `@EventListener` and it consumes Spring's built-in `ApplicationReadyEvent` — no precedent for a custom domain event in this codebase before now. + +## Decision + +`PersonService.updatePerson` publishes `PersonDisplayNameChangedEvent(personId, oldDisplayName, newDisplayName)` via `ApplicationEventPublisher` whenever `Person.getDisplayName()` flips between the pre-save snapshot and the post-save value. `PersonMentionPropagationListener` (in the transcription package's `service/` layer) handles the event with `@EventListener @Transactional`, finds blocks via `findByMentionedPersons_PersonId`, rewrites text + sidecar, and calls `saveAllAndFlush`. + +**Synchronous on purpose.** Spring's default event dispatcher invokes listeners on the publishing thread, inside the publisher's transaction. The propagation runs as part of the same `@Transactional` boundary as the rename — `OptimisticLockingFailureException` from a referenced block bubbles back up, the surrounding transaction rolls back, and `PersonService.updatePerson` translates it to `DomainException(PERSON_RENAME_CONFLICT, 409)`. + +**Pattern for future cross-domain decoupling:** +1. Event record in `model/` of the publishing domain (e.g. `PersonDisplayNameChangedEvent`). +2. Listener in `service/` of the consuming domain (e.g. `PersonMentionPropagationListener`). +3. `@EventListener @Transactional` on the listener method — no `@TransactionalEventListener` unless the work genuinely doesn't need to commit with the publisher. +4. `saveAllAndFlush` (not `saveAll`) on any write where exceptions must surface inside the listener call so the publisher can catch and translate them — `saveAll` defers exceptions to commit time, after the publisher's `try` block has exited. +5. Audit log line at `INFO` level on the listener method — historical-text mutation needs an audit trail. + +## Alternatives Considered + +| Alternative | Why rejected | +|---|---| +| `PersonService` calls `TranscriptionBlockService.propagateDisplayNameChange(...)` directly | Inverts the dependency arrow. Person becomes runtime-coupled to Transcription; future domains that also care about renames (Comments, Notifications) compound the coupling. Events keep Person agnostic of who consumes them. | +| `@TransactionalEventListener(AFTER_COMMIT) + @Async` | The propagation would run after the rename commits, on a separate transaction. A failed propagation could leave block text out of sync with the renamed person until manual repair. Atomic transactional coupling is the safer default for historical-text mutation; switch to async only when the block count makes sync latency unacceptable (rough threshold: tens of thousands of blocks per renamed person). | +| Database trigger on `persons.last_name` | PL/pgSQL trigger would have to reach into `transcription_block_mentioned_persons` and `transcription_blocks.text`, smearing domain logic across SQL and Java. JPA's `@Version` would also be invisible to the trigger, so concurrent block autosaves would race silently. | +| Hibernate entity listener (`@PostUpdate` on Person) | Couples to Hibernate internals; harder to test in isolation; mixes lifecycle hooks with cross-domain side effects. Spring's `ApplicationEventPublisher` keeps the integration declarative and unit-testable. | + +## Consequences + +**Easier:** +- Person domain stays free of any compile-time dependency on Transcription. Future consumers (Comments, Notifications) subscribe to the same event without `PersonService` knowing they exist. +- Rename + propagation share one transaction → no half-applied state visible to readers, no orphaned rewrites if the rename fails after propagation, no "eventually-consistent" window for an archive that prizes historical fidelity. +- Concurrent autosaves on referenced blocks raise a structured 409 the frontend can render meaningfully (`error_person_rename_conflict`) instead of a generic 500. +- The pattern itself (record event in `model/`, listener in consumer's `service/`, sync `@EventListener @Transactional`, `saveAllAndFlush`) is reusable for the next cross-domain side effect. + +**Harder:** +- Listener latency adds to the rename request's response time. The 200-block latency floor (< 2 s) is a merge-blocking regression test; if archive growth pushes it up, the migration path is one-annotation: switch to `@TransactionalEventListener(AFTER_COMMIT) + @Async` and add a manual-repair tool for propagation failures. +- Tests for the listener path require routing the publisher mock through a real listener (see `PersonServiceTest#updatePerson_throwsConflict_whenBlockSaveAllAndFlushHitsOptimisticLock`). Slightly more setup than a pure-Mockito test, but exercises the production call chain. +- `saveAllAndFlush` is mandatory in any synchronous listener that must surface JPA exceptions to the publisher's `try`-block. `saveAll` alone defers the flush to transaction commit, which happens after the publisher returns. + +## Future Direction + +If a single rename starts touching tens of thousands of blocks, switch the listener to `@TransactionalEventListener(phase = AFTER_COMMIT)` paired with `@Async` and add (a) an idempotency key to the event so a retry doesn't double-rewrite, (b) an admin tool that scans for sidecar entries whose `displayName` doesn't match the current `Person.getDisplayName()` and repairs them. At that point the orphan-guard path (existsById check before the rewrite) re-enters the listener as a deliberate piece of the async machinery rather than dead code. diff --git a/frontend/messages/de.json b/frontend/messages/de.json index f8cf9f8c..5d17e947 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -542,6 +542,7 @@ "person_alias_btn_delete": "Entfernen", "error_alias_not_found": "Der Namensalias wurde nicht gefunden.", "error_invalid_person_type": "Der angegebene Personentyp ist ungültig.", + "error_person_rename_conflict": "Eine andere Bearbeitung hat einen verknüpften Transkriptionsblock gleichzeitig geändert. Bitte erneut versuchen.", "validation_last_name_required": "Nachname ist Pflichtfeld.", "validation_first_name_required": "Vorname ist Pflichtfeld.", "error_ocr_service_unavailable": "Der OCR-Dienst ist nicht verfügbar.", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index afa6adfa..604aa617 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -542,6 +542,7 @@ "person_alias_btn_delete": "Remove", "error_alias_not_found": "The name alias was not found.", "error_invalid_person_type": "The specified person type is not valid.", + "error_person_rename_conflict": "Another edit modified a referenced transcription block at the same time. Please try again.", "validation_last_name_required": "Last name is required.", "validation_first_name_required": "First name is required.", "error_ocr_service_unavailable": "The OCR service is not available.", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 87d26d83..9e769d44 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -542,6 +542,7 @@ "person_alias_btn_delete": "Eliminar", "error_alias_not_found": "No se encontro el alias de nombre.", "error_invalid_person_type": "El tipo de persona especificado no es válido.", + "error_person_rename_conflict": "Otra edición modificó un bloque de transcripción referenciado al mismo tiempo. Por favor, inténtalo de nuevo.", "validation_last_name_required": "El apellido es obligatorio.", "validation_first_name_required": "El nombre es obligatorio.", "error_ocr_service_unavailable": "El servicio OCR no está disponible.", diff --git a/frontend/src/lib/errors.ts b/frontend/src/lib/errors.ts index e57f212e..3c80a1da 100644 --- a/frontend/src/lib/errors.ts +++ b/frontend/src/lib/errors.ts @@ -8,6 +8,7 @@ export type ErrorCode = | 'PERSON_NOT_FOUND' | 'ALIAS_NOT_FOUND' | 'INVALID_PERSON_TYPE' + | 'PERSON_RENAME_CONFLICT' | 'DOCUMENT_NOT_FOUND' | 'DOCUMENT_NO_FILE' | 'FILE_NOT_FOUND' @@ -79,6 +80,8 @@ export function getErrorMessage(code: ErrorCode | string | undefined): string { return m.error_alias_not_found(); case 'INVALID_PERSON_TYPE': return m.error_invalid_person_type(); + case 'PERSON_RENAME_CONFLICT': + return m.error_person_rename_conflict(); case 'DOCUMENT_NOT_FOUND': return m.error_document_not_found(); case 'DOCUMENT_NO_FILE': diff --git a/frontend/src/lib/generated/api.ts b/frontend/src/lib/generated/api.ts index 30cec4fb..4d2787ad 100644 --- a/frontend/src/lib/generated/api.ts +++ b/frontend/src/lib/generated/api.ts @@ -132,6 +132,22 @@ export interface paths { patch?: never; trace?: never; }; + "/api/documents/{documentId}/transcription-blocks/review-all": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put: operations["markAllBlocksReviewed"]; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/documents/{documentId}/transcription-blocks/reorder": { parameters: { query?: never; @@ -1611,9 +1627,15 @@ export interface components { trainingLabels?: ("KURRENT_RECOGNITION" | "KURRENT_SEGMENTATION")[]; thumbnailUrl?: string; }; + PersonMention: { + /** Format: uuid */ + personId: string; + displayName: string; + }; UpdateTranscriptionBlockDTO: { text?: string; label?: string; + mentionedPersons?: components["schemas"]["PersonMention"][]; }; TranscriptionBlock: { /** Format: uuid */ @@ -1623,6 +1645,7 @@ export interface components { /** Format: uuid */ documentId: string; text?: string; + mentionedPersons: components["schemas"]["PersonMention"][]; label?: string; /** Format: int32 */ sortOrder: number; @@ -1665,7 +1688,8 @@ export interface components { CreateRelationshipRequest: { /** Format: uuid */ relatedPersonId: string; - relationType: string; + /** @enum {string} */ + relationType: "PARENT_OF" | "SPOUSE_OF" | "SIBLING_OF" | "FRIEND" | "COLLEAGUE" | "EMPLOYER" | "DOCTOR" | "NEIGHBOR" | "OTHER"; /** Format: int32 */ fromYear?: number; /** Format: int32 */ @@ -1796,6 +1820,7 @@ export interface components { height?: number; text?: string; label?: string; + mentionedPersons?: components["schemas"]["PersonMention"][]; }; CreateCommentDTO: { content?: string; @@ -2747,6 +2772,28 @@ export interface operations { }; }; }; + markAllBlocksReviewed: { + parameters: { + query?: never; + header?: never; + path: { + documentId: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["TranscriptionBlock"][]; + }; + }; + }; + }; reorderBlocks: { parameters: { query?: never;