feat(transcription): person @mention sidecar + rename propagation (PR-A backend, #362) #366
@@ -34,11 +34,13 @@ public class PersonController {
|
||||
private final DocumentService documentService;
|
||||
|
||||
@GetMapping
|
||||
@RequirePermission(Permission.READ_ALL)
|
||||
public ResponseEntity<List<PersonSummaryDTO>> 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);
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<PersonMention> mentionedPersons = new ArrayList<>();
|
||||
}
|
||||
|
||||
@@ -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<PersonMention> mentionedPersons = new ArrayList<>();
|
||||
}
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
* <p>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
|
||||
) {
|
||||
}
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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<PersonMention> mentionedPersons = new ArrayList<>();
|
||||
|
||||
@Column(length = 200)
|
||||
private String label;
|
||||
|
||||
|
||||
@@ -29,6 +29,15 @@ public interface TranscriptionBlockRepository extends JpaRepository<Transcriptio
|
||||
|
||||
Optional<TranscriptionBlock> 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<TranscriptionBlock> findByPersonIdWithMentionsFetched(@Param("personId") UUID personId);
|
||||
|
||||
void deleteByAnnotationId(UUID annotationId);
|
||||
|
||||
int countByDocumentId(UUID documentId);
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
* <p>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<TranscriptionBlock> 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 " <Uppercase>"
|
||||
// (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));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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<PersonSummaryDTO> 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
|
||||
|
||||
@@ -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);
|
||||
@@ -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);
|
||||
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<TranscriptionBlock> 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"));
|
||||
}
|
||||
}
|
||||
@@ -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<PersonMention> 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<UUID> 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();
|
||||
}
|
||||
}
|
||||
@@ -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<PersonDisplayNameChangedEvent> 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
|
||||
|
||||
@@ -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<Map<String, Object>> 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());
|
||||
}
|
||||
|
||||
55
docs/adr/006-synchronous-domain-events-in-transaction.md
Normal file
55
docs/adr/006-synchronous-domain-events-in-transaction.md
Normal file
@@ -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.
|
||||
@@ -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.",
|
||||
|
||||
@@ -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.",
|
||||
|
||||
@@ -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.",
|
||||
|
||||
@@ -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':
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user