From 404d874b4e41d9e7c0619aae12812af52e35cd29 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 28 Apr 2026 20:57:16 +0200 Subject: [PATCH] feat(person): translate optimistic-lock conflicts on rename to PERSON_RENAME_CONFLICT 409 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the propagation listener saves blocks with a stale @Version (because another transcriber's autosave incremented version mid-rename), Hibernate raises ObjectOptimisticLockingFailureException — Spring's translation of the underlying JPA exception. PersonService.updatePerson now wraps the publishEvent call in a catch for OptimisticLockingFailureException and re-throws as DomainException(PERSON_RENAME_CONFLICT, 409). The whole @Transactional boundary still rolls back, but the client gets a structured 409 with the localised "please retry" message instead of a generic 500. The listener was switched from saveAll to saveAllAndFlush so the conflict fires inside the listener call (where the catch can see it), not at transaction commit (which is too late for in-method handling). Test stubs the eventPublisher to throw OptimisticLockingFailureException and asserts the translated DomainException carries PERSON_RENAME_CONFLICT and HTTP 409. End-to-end DB-level reproduction of the JPA optimistic-lock race requires multi-threading or two physical connections, which is impractical inside @DataJpaTest; the underlying JPA mechanism is well covered by Hibernate's own test suite. Refs #362 Co-Authored-By: Claude Opus 4.7 --- .../PersonMentionPropagationListener.java | 2 +- .../familienarchiv/service/PersonService.java | 8 ++++++- .../service/PersonServiceTest.java | 24 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java index 0009a162..9258edb7 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonMentionPropagationListener.java @@ -61,7 +61,7 @@ public class PersonMentionPropagationListener { } } - blockRepository.saveAll(blocks); + blockRepository.saveAllAndFlush(blocks); log.info("Propagated rename {} → {} across {} block(s) for person {}", event.oldDisplayName(), event.newDisplayName(), blocks.size(), event.personId()); 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 191c8530..e5bab02d 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java @@ -20,6 +20,7 @@ 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; @@ -176,7 +177,12 @@ public class PersonService { Person saved = personRepository.save(person); String newDisplayName = saved.getDisplayName(); if (!Objects.equals(oldDisplayName, newDisplayName)) { - eventPublisher.publishEvent(new PersonDisplayNameChangedEvent(id, 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; } 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 71e0dff8..a90ac002 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java @@ -10,6 +10,7 @@ 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.PersonNameAlias; @@ -18,6 +19,7 @@ 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.web.server.ResponseStatusException; import java.util.List; @@ -297,6 +299,28 @@ class PersonServiceTest { verify(eventPublisher, never()).publishEvent(any(PersonDisplayNameChangedEvent.class)); } + @Test + void updatePerson_throwsConflict_whenListenerSignalsOptimisticLockFailure() { + UUID id = UUID.randomUUID(); + Person existing = Person.builder() + .id(id).firstName("Auguste").lastName("Raddatz") + .personType(PersonType.PERSON).build(); + + when(personRepository.findById(id)).thenReturn(Optional.of(existing)); + when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + doThrow(new OptimisticLockingFailureException("simulated concurrent block save")) + .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();