From 2d19ca724400e7fd0f48e3ee06cabd778d4ea45f Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 29 Apr 2026 14:58:18 +0200 Subject: [PATCH] refactor(backend): delete rename-propagation listener and its infrastructure PersonMentionPropagationListener rewrites @DisplayName tokens on person rename. Under the new design, displayName is archival (what the transcriber typed), so the listener would corrupt transcriptions rather than correct them. Deletes PersonMentionPropagationListener, PersonDisplayNameChangedEvent, and the optimistic-lock catch path in PersonService.updatePerson. Removes PERSON_RENAME_CONFLICT from ErrorCode and all tests that exercised the now-deleted code path. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/exception/ErrorCode.java | 5 - .../model/PersonDisplayNameChangedEvent.java | 23 -- .../familienarchiv/model/PersonMention.java | 1 + .../PersonMentionPropagationListener.java | 77 ------ .../familienarchiv/service/PersonService.java | 18 +- .../controller/PersonControllerTest.java | 15 -- .../PersonMentionPropagationListenerTest.java | 227 ------------------ .../service/PersonServiceTest.java | 130 +--------- 8 files changed, 6 insertions(+), 490 deletions(-) delete mode 100644 backend/src/main/java/org/raddatz/familienarchiv/model/PersonDisplayNameChangedEvent.java delete mode 100644 backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java delete mode 100644 backend/src/test/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListenerTest.java 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 d9a3a30f..517eb1da 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -15,11 +15,6 @@ 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 */ DOCUMENT_NOT_FOUND, diff --git a/backend/src/main/java/org/raddatz/familienarchiv/model/PersonDisplayNameChangedEvent.java b/backend/src/main/java/org/raddatz/familienarchiv/model/PersonDisplayNameChangedEvent.java deleted file mode 100644 index 1b748c1a..00000000 --- a/backend/src/main/java/org/raddatz/familienarchiv/model/PersonDisplayNameChangedEvent.java +++ /dev/null @@ -1,23 +0,0 @@ -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 index 79b232d6..2ca2033e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/model/PersonMention.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/model/PersonMention.java @@ -26,5 +26,6 @@ public class PersonMention { @Size(max = 200) @Column(name = "display_name", nullable = false, length = 200) @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + // Archival: the text the transcriber typed after @. Never updated on person rename. private String displayName; } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java deleted file mode 100644 index a3c03206..00000000 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java +++ /dev/null @@ -1,77 +0,0 @@ -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 33873459..e1adbdba 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java @@ -1,7 +1,6 @@ package org.raddatz.familienarchiv.service; import java.util.List; -import java.util.Objects; import java.util.Optional; import java.util.UUID; @@ -13,14 +12,11 @@ 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; @@ -34,7 +30,6 @@ public class PersonService { private final PersonRepository personRepository; private final PersonNameAliasRepository aliasRepository; - private final ApplicationEventPublisher eventPublisher; public List findAll(String q) { if (q == null) { @@ -161,7 +156,6 @@ 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()); @@ -170,17 +164,7 @@ public class PersonService { person.setNotes(dto.getNotes() == null || dto.getNotes().isBlank() ? null : dto.getNotes().trim()); person.setBirthYear(dto.getBirthYear()); person.setDeathYear(dto.getDeathYear()); - 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; + return personRepository.save(person); } @Transactional 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 41a4a0ac..9de8a3a1 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java @@ -333,21 +333,6 @@ 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/service/PersonMentionPropagationListenerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListenerTest.java deleted file mode 100644 index c6b89716..00000000 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListenerTest.java +++ /dev/null @@ -1,227 +0,0 @@ -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 b502ac7c..24827177 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java @@ -2,7 +2,6 @@ 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; @@ -10,22 +9,14 @@ 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; @@ -33,14 +24,16 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.*; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class PersonServiceTest { @Mock PersonRepository personRepository; @Mock PersonNameAliasRepository aliasRepository; - @Mock ApplicationEventPublisher eventPublisher; @InjectMocks PersonService personService; // ─── getById ───────────────────────────────────────────────────────────── @@ -252,121 +245,6 @@ 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