refactor(transcription): drop dead existsById orphan guard from listener
Felix #2 / Markus #1 (PR #366 review). In the synchronous-transactional path the existsById check could never return false — the rename and the propagation share one transaction, so the renamed Person is guaranteed to still exist when the listener runs. The check was forward-protection for an eventual @Async refactor but its presence today is misleading: it suggests a runtime branch that no test could reach against the real flow. Delete the call, drop the PersonService dependency from the listener, drop the now-unused PersonService.existsById, and remove the orphan-guard test (it asserted a behaviour that the synchronous path cannot produce). When async is added later the guard re-enters the codebase deliberately as part of that refactor. Refs #362 #366 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -33,16 +33,10 @@ import java.util.regex.Pattern;
|
|||||||
public class PersonMentionPropagationListener {
|
public class PersonMentionPropagationListener {
|
||||||
|
|
||||||
private final TranscriptionBlockRepository blockRepository;
|
private final TranscriptionBlockRepository blockRepository;
|
||||||
private final PersonService personService;
|
|
||||||
|
|
||||||
@EventListener
|
@EventListener
|
||||||
@Transactional
|
@Transactional
|
||||||
public void onPersonDisplayNameChanged(PersonDisplayNameChangedEvent event) {
|
public void onPersonDisplayNameChanged(PersonDisplayNameChangedEvent event) {
|
||||||
if (!personService.existsById(event.personId())) {
|
|
||||||
log.warn("Skipping mention propagation for non-existent personId {}", event.personId());
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
List<TranscriptionBlock> blocks =
|
List<TranscriptionBlock> blocks =
|
||||||
blockRepository.findByMentionedPersons_PersonId(event.personId());
|
blockRepository.findByMentionedPersons_PersonId(event.personId());
|
||||||
if (blocks.isEmpty()) {
|
if (blocks.isEmpty()) {
|
||||||
|
|||||||
@@ -51,10 +51,6 @@ public class PersonService {
|
|||||||
.orElseThrow(() -> DomainException.notFound(ErrorCode.PERSON_NOT_FOUND, "Person not found: " + id));
|
.orElseThrow(() -> DomainException.notFound(ErrorCode.PERSON_NOT_FOUND, "Person not found: " + id));
|
||||||
}
|
}
|
||||||
|
|
||||||
public boolean existsById(UUID id) {
|
|
||||||
return personRepository.existsById(id);
|
|
||||||
}
|
|
||||||
|
|
||||||
public List<Person> findCorrespondents(UUID personId, String q) {
|
public List<Person> findCorrespondents(UUID personId, String q) {
|
||||||
if (q != null && !q.isBlank()) {
|
if (q != null && !q.isBlank()) {
|
||||||
return personRepository.findCorrespondentsWithFilter(personId, q);
|
return personRepository.findCorrespondentsWithFilter(personId, q);
|
||||||
|
|||||||
@@ -26,9 +26,6 @@ import java.util.List;
|
|||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
import static org.mockito.ArgumentMatchers.any;
|
|
||||||
import static org.mockito.Mockito.mock;
|
|
||||||
import static org.mockito.Mockito.when;
|
|
||||||
|
|
||||||
@DataJpaTest
|
@DataJpaTest
|
||||||
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
|
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
|
||||||
@@ -42,16 +39,13 @@ class PersonMentionPropagationListenerTest {
|
|||||||
@Autowired EntityManager em;
|
@Autowired EntityManager em;
|
||||||
|
|
||||||
private PersonMentionPropagationListener listener;
|
private PersonMentionPropagationListener listener;
|
||||||
private PersonService personService;
|
|
||||||
|
|
||||||
private UUID documentId;
|
private UUID documentId;
|
||||||
private UUID annotationId;
|
private UUID annotationId;
|
||||||
|
|
||||||
@BeforeEach
|
@BeforeEach
|
||||||
void setUp() {
|
void setUp() {
|
||||||
personService = mock(PersonService.class);
|
listener = new PersonMentionPropagationListener(blockRepository);
|
||||||
when(personService.existsById(any())).thenReturn(true);
|
|
||||||
listener = new PersonMentionPropagationListener(blockRepository, personService);
|
|
||||||
|
|
||||||
Document doc = documentRepository.save(Document.builder()
|
Document doc = documentRepository.save(Document.builder()
|
||||||
.title("Letter").originalFilename("letter.pdf")
|
.title("Letter").originalFilename("letter.pdf")
|
||||||
@@ -170,28 +164,6 @@ class PersonMentionPropagationListenerTest {
|
|||||||
.containsExactly("Augusta Raddatz");
|
.containsExactly("Augusta Raddatz");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
|
||||||
void noOps_whenPersonIdNoLongerExists_orphanedSidecarGuard() {
|
|
||||||
UUID orphanId = UUID.randomUUID();
|
|
||||||
when(personService.existsById(orphanId)).thenReturn(false);
|
|
||||||
|
|
||||||
TranscriptionBlock saved = saveBlock(
|
|
||||||
"Stale reference to @Ghost Name should not be rewritten.",
|
|
||||||
List.of(new PersonMention(orphanId, "Ghost Name")));
|
|
||||||
em.clear();
|
|
||||||
|
|
||||||
listener.onPersonDisplayNameChanged(
|
|
||||||
new PersonDisplayNameChangedEvent(orphanId, "Ghost Name", "Resurrected Name"));
|
|
||||||
blockRepository.flush();
|
|
||||||
em.clear();
|
|
||||||
|
|
||||||
TranscriptionBlock reloaded = blockRepository.findById(saved.getId()).orElseThrow();
|
|
||||||
assertThat(reloaded.getText()).isEqualTo("Stale reference to @Ghost Name should not be rewritten.");
|
|
||||||
assertThat(reloaded.getMentionedPersons())
|
|
||||||
.extracting(PersonMention::getDisplayName)
|
|
||||||
.containsExactly("Ghost Name");
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void propagatesAcross200Blocks_inUnderTwoSeconds_latencyFloor() {
|
void propagatesAcross200Blocks_inUnderTwoSeconds_latencyFloor() {
|
||||||
UUID personId = savedPersonId("Auguste", "Raddatz");
|
UUID personId = savedPersonId("Auguste", "Raddatz");
|
||||||
|
|||||||
Reference in New Issue
Block a user