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

refactor(backend): delete rename-propagation listener and its infrastructure

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-04-29 14:58:18 +02:00
parent bc58d77f2c
commit 2d19ca7244
8 changed files with 6 additions and 490 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java
View File

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

227
backend/src/test/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListenerTest.java
View File

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

130
backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java
View File

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