refactor(person): simplify mergePersons to lean on V71 cascade (#684)
Drop the explicit deleteReceiverReferences call from mergePersons — the source's leftover receiver join rows now cascade-drop via V71's ON DELETE CASCADE on deleteById. Remove the now-unused deleteReceiverReferences repository method (and its repo test), and add clearAutomatically + flushAutomatically to the remaining merge native queries so the L1 cache cannot desync from the bulk updates. Rewrite the merge unit test with verifyNoMoreInteractions and add an end-to-end merge regression test (AC-7). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -189,12 +189,15 @@ public interface PersonRepository extends JpaRepository<Person, UUID> {
|
||||
List<Person> findCorrespondentsWithFilter(@Param("personId") UUID personId, @Param("q") String q);
|
||||
|
||||
// --- Merge helpers (native SQL to bypass JPA entity layer) ---
|
||||
// clearAutomatically + flushAutomatically keep the L1 cache from desyncing: these bulk
|
||||
// updates run beneath Hibernate, and mergePersons follows them with a deleteById whose
|
||||
// ON DELETE CASCADE (V71) also fires beneath the session.
|
||||
|
||||
@Modifying
|
||||
@Modifying(clearAutomatically = true, flushAutomatically = true)
|
||||
@Query(value = "UPDATE documents SET sender_id = :target WHERE sender_id = :source", nativeQuery = true)
|
||||
void reassignSender(@Param("source") UUID source, @Param("target") UUID target);
|
||||
|
||||
@Modifying
|
||||
@Modifying(clearAutomatically = true, flushAutomatically = true)
|
||||
@Query(value = """
|
||||
INSERT INTO document_receivers (document_id, person_id)
|
||||
SELECT document_id, :target FROM document_receivers
|
||||
@@ -204,8 +207,4 @@ public interface PersonRepository extends JpaRepository<Person, UUID> {
|
||||
)
|
||||
""", nativeQuery = true)
|
||||
void insertMissingReceiverReference(@Param("source") UUID source, @Param("target") UUID target);
|
||||
|
||||
@Modifying
|
||||
@Query(value = "DELETE FROM document_receivers WHERE person_id = :source", nativeQuery = true)
|
||||
void deleteReceiverReferences(@Param("source") UUID source);
|
||||
}
|
||||
@@ -293,6 +293,12 @@ public class PersonService {
|
||||
return personRepository.save(person);
|
||||
}
|
||||
|
||||
/**
|
||||
* Merges the source person into the target, then deletes the source. Sender references move
|
||||
* to the target; receiver references the target lacks are inserted. The source's leftover
|
||||
* receiver join rows are not deleted explicitly — they cascade-drop via V71's
|
||||
* {@code ON DELETE CASCADE} on {@code document_receivers.person_id} when the source is deleted.
|
||||
*/
|
||||
@Transactional
|
||||
public void mergePersons(UUID sourceId, UUID targetId) {
|
||||
if (sourceId.equals(targetId)) {
|
||||
@@ -309,9 +315,7 @@ public class PersonService {
|
||||
// Add target as receiver where source is receiver but target is not yet
|
||||
personRepository.insertMissingReceiverReference(sourceId, targetId);
|
||||
|
||||
// Remove all remaining source receiver references (duplicates already handled)
|
||||
personRepository.deleteReceiverReferences(sourceId);
|
||||
|
||||
// Source's remaining receiver rows cascade-drop via V71's ON DELETE CASCADE.
|
||||
personRepository.deleteById(sourceId);
|
||||
}
|
||||
|
||||
|
||||
@@ -367,30 +367,6 @@ class PersonRepositoryTest {
|
||||
assertThat(result).hasSize(1);
|
||||
}
|
||||
|
||||
// ─── deleteReceiverReferences ─────────────────────────────────────────────
|
||||
|
||||
@Test
|
||||
void deleteReceiverReferences_removesPersonFromAllDocumentReceivers() {
|
||||
Person toDelete = personRepository.save(Person.builder().firstName("Weg").lastName("Person").build());
|
||||
Person sender = personRepository.save(Person.builder().firstName("Send").lastName("Er").build());
|
||||
|
||||
Document doc1 = documentRepository.save(Document.builder()
|
||||
.title("Brief 1").originalFilename("b1.pdf")
|
||||
.status(DocumentStatus.UPLOADED)
|
||||
.sender(sender).receivers(Set.of(toDelete)).build());
|
||||
Document doc2 = documentRepository.save(Document.builder()
|
||||
.title("Brief 2").originalFilename("b2.pdf")
|
||||
.status(DocumentStatus.UPLOADED)
|
||||
.sender(sender).receivers(Set.of(toDelete)).build());
|
||||
|
||||
personRepository.deleteReceiverReferences(toDelete.getId());
|
||||
entityManager.flush();
|
||||
entityManager.clear();
|
||||
|
||||
assertThat(documentRepository.findById(doc1.getId()).orElseThrow().getReceivers()).isEmpty();
|
||||
assertThat(documentRepository.findById(doc2.getId()).orElseThrow().getReceivers()).isEmpty();
|
||||
}
|
||||
|
||||
// ─── searchByName with aliases ───────────────────────────────────────────
|
||||
|
||||
@Test
|
||||
|
||||
@@ -220,4 +220,38 @@ class PersonServiceIntegrationTest {
|
||||
// The other person and the documents themselves survive the delete.
|
||||
assertThat(personRepository.findById(bystander.getId())).isPresent();
|
||||
}
|
||||
|
||||
@Test
|
||||
void mergePersons_targetInheritsReferences_sourceJoinRowCascadeDrops_noFkError() {
|
||||
// AC-7: merging a source who is sender of A and receiver of B into a target leaves the
|
||||
// target as sender of A and receiver of B, drops the source's leftover receiver row via
|
||||
// V71's ON DELETE CASCADE (no explicit delete, no FK error), and co-receivers are intact.
|
||||
Person source = personRepository.save(Person.builder().firstName("Anna").lastName("Alt").build());
|
||||
Person target = personRepository.save(Person.builder().firstName("Anna").lastName("Neu").build());
|
||||
Person coReceiver = personRepository.save(Person.builder().firstName("Mit").lastName("Empfänger").build());
|
||||
Person sender = personRepository.save(Person.builder().firstName("Send").lastName("Er").build());
|
||||
|
||||
Document docA = documentRepository.save(Document.builder()
|
||||
.title("Von Anna").originalFilename("a.pdf")
|
||||
.status(DocumentStatus.UPLOADED).sender(source).build());
|
||||
Document docB = documentRepository.save(Document.builder()
|
||||
.title("An Anna").originalFilename("b.pdf")
|
||||
.status(DocumentStatus.UPLOADED).sender(sender)
|
||||
.receivers(new java.util.HashSet<>(Set.of(source, coReceiver))).build());
|
||||
entityManager.flush();
|
||||
entityManager.clear();
|
||||
|
||||
personService.mergePersons(source.getId(), target.getId());
|
||||
entityManager.flush();
|
||||
entityManager.clear();
|
||||
|
||||
assertThat(personRepository.findById(source.getId())).isEmpty();
|
||||
|
||||
Document reloadedA = documentRepository.findById(docA.getId()).orElseThrow();
|
||||
assertThat(reloadedA.getSender().getId()).isEqualTo(target.getId());
|
||||
|
||||
Document reloadedB = documentRepository.findById(docB.getId()).orElseThrow();
|
||||
assertThat(reloadedB.getReceivers()).extracting(Person::getId)
|
||||
.containsExactlyInAnyOrder(target.getId(), coReceiver.getId());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -703,10 +703,14 @@ class PersonServiceTest {
|
||||
|
||||
personService.mergePersons(sourceId, targetId);
|
||||
|
||||
verify(personRepository).findById(sourceId);
|
||||
verify(personRepository).findById(targetId);
|
||||
verify(personRepository).reassignSender(sourceId, targetId);
|
||||
verify(personRepository).insertMissingReceiverReference(sourceId, targetId);
|
||||
verify(personRepository).deleteReceiverReferences(sourceId);
|
||||
verify(personRepository).deleteById(sourceId);
|
||||
// The source's leftover receiver rows cascade-drop via V71's ON DELETE CASCADE on
|
||||
// deleteById — merge no longer deletes them explicitly.
|
||||
verifyNoMoreInteractions(personRepository);
|
||||
}
|
||||
|
||||
// ─── getAliases ─────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user