feat(person): enforce person-delete integrity at the DB layer (V71) (#684)
Add ON DELETE behaviour to the two V1 FKs into persons (documents.sender_id -> SET NULL, document_receivers.person_id -> CASCADE) and a real FK with ON DELETE CASCADE on the transcription_block_mentioned_persons soft reference, cleaning up pre-existing orphan mention rows first. The cascade stays strictly at the join/reference layer and never reaches documents rows. Proven by new Postgres-backed PersonRepositoryTest cascade tests (AC-1/2/3/8 plus the cascade-boundary document-survival guard). Rewrites the now-stale V56 'no FK' comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -8,12 +8,13 @@
|
|||||||
-- all blocks mentioning person X" query on the person detail page joins on
|
-- all blocks mentioning person X" query on the person detail page joins on
|
||||||
-- the indexed person_id column — equally fast as JSONB GIN containment, no
|
-- the indexed person_id column — equally fast as JSONB GIN containment, no
|
||||||
-- new dependency. document_comments.comment_mentions stays as a many-to-many
|
-- new dependency. document_comments.comment_mentions stays as a many-to-many
|
||||||
-- to AppUser; the divergence is intentional: Person mentions need lazy
|
-- to AppUser; the divergence is intentional.
|
||||||
-- degradation when a person is deleted (no FK), while user mentions don't.
|
|
||||||
--
|
--
|
||||||
-- No FK on person_id: when a Person is deleted we want @Auguste Raddatz to
|
-- person_id FK: added in V71 with ON DELETE CASCADE (this column was originally
|
||||||
-- remain visible as plain unlinked text inside the transcription rather than
|
-- created without a FK — that decision was reversed). The literal "@Name" in
|
||||||
-- vanishing or cascade-deleting the block.
|
-- transcription_blocks.text keeps the name visible after a person is deleted;
|
||||||
|
-- the sidecar row carries only the link and is removed with the person, so the
|
||||||
|
-- read renderer degrades to plain unlinked text. See ADR-032.
|
||||||
|
|
||||||
CREATE TABLE transcription_block_mentioned_persons (
|
CREATE TABLE transcription_block_mentioned_persons (
|
||||||
block_id UUID NOT NULL REFERENCES transcription_blocks(id) ON DELETE CASCADE,
|
block_id UUID NOT NULL REFERENCES transcription_blocks(id) ON DELETE CASCADE,
|
||||||
|
|||||||
@@ -0,0 +1,50 @@
|
|||||||
|
-- Move person-delete referential integrity from application code into the database (#684).
|
||||||
|
--
|
||||||
|
-- Before this migration, PersonService.deletePerson nulled documents.sender_id and removed
|
||||||
|
-- document_receivers rows in Java before deleting the person, because the two V1 FKs into
|
||||||
|
-- persons had no ON DELETE behaviour. Any other delete path (a future endpoint, a manual
|
||||||
|
-- psql, a batch job) could still orphan rows or 500. This migration makes the database the
|
||||||
|
-- single source of truth so a person delete is safe from every path.
|
||||||
|
--
|
||||||
|
-- Cascade boundary: the cascade stays STRICTLY at the join/reference layer and NEVER reaches
|
||||||
|
-- documents rows — a cascade into documents would destroy historical letters. sender_id is
|
||||||
|
-- SET NULL (documents.senderText preserves the raw textual attribution); the receiver join
|
||||||
|
-- row and the @-mention sidecar row are dropped.
|
||||||
|
--
|
||||||
|
-- No NOT VALID + VALIDATE two-step: these tables are small (thousands of rows → sub-second
|
||||||
|
-- ACCESS EXCLUSIVE lock). Do NOT copy this drop-and-recreate pattern onto a large table.
|
||||||
|
--
|
||||||
|
-- Not audit-logged: a DB ON DELETE cascade runs below AuditService — a known, accepted trade.
|
||||||
|
-- The person-delete action itself is still logged at the service layer.
|
||||||
|
|
||||||
|
-- documents.sender_id → ON DELETE SET NULL (deleted sender clears the link; the document survives).
|
||||||
|
ALTER TABLE public.documents
|
||||||
|
DROP CONSTRAINT fkl5xhww7es3b4um01vmly4y18m,
|
||||||
|
ADD CONSTRAINT fkl5xhww7es3b4um01vmly4y18m
|
||||||
|
FOREIGN KEY (sender_id) REFERENCES public.persons(id) ON DELETE SET NULL;
|
||||||
|
|
||||||
|
-- document_receivers.person_id → ON DELETE CASCADE (drop the join row), the symmetric
|
||||||
|
-- completion of V14, which added the same to the document_id side of this table.
|
||||||
|
ALTER TABLE public.document_receivers
|
||||||
|
DROP CONSTRAINT fkcg7r68qvosqricx1betgrlt7s,
|
||||||
|
ADD CONSTRAINT fkcg7r68qvosqricx1betgrlt7s
|
||||||
|
FOREIGN KEY (person_id) REFERENCES public.persons(id) ON DELETE CASCADE;
|
||||||
|
|
||||||
|
-- Soft reference fix: transcription_block_mentioned_persons.person_id was a UUID with no FK
|
||||||
|
-- (V56), so deleting a person left dangling mention rows. Give it a real FK with CASCADE.
|
||||||
|
-- Clean up pre-existing orphans first — production likely holds dangling rows because the old
|
||||||
|
-- deletePerson never cleaned mention rows, and the ADD CONSTRAINT validation scan fails on them.
|
||||||
|
-- A DO block with RAISE NOTICE surfaces the purge count: Flyway runs each statement via JDBC
|
||||||
|
-- and discards a trailing SELECT's result set, so a "SELECT count(*)" would log nothing.
|
||||||
|
DO $$
|
||||||
|
DECLARE removed int;
|
||||||
|
BEGIN
|
||||||
|
DELETE FROM transcription_block_mentioned_persons m
|
||||||
|
WHERE NOT EXISTS (SELECT 1 FROM persons p WHERE p.id = m.person_id);
|
||||||
|
GET DIAGNOSTICS removed = ROW_COUNT;
|
||||||
|
RAISE NOTICE 'V71 orphaned_mention_rows_removed=%', removed;
|
||||||
|
END $$;
|
||||||
|
|
||||||
|
ALTER TABLE public.transcription_block_mentioned_persons
|
||||||
|
ADD CONSTRAINT fk_tbmp_person
|
||||||
|
FOREIGN KEY (person_id) REFERENCES public.persons(id) ON DELETE CASCADE;
|
||||||
@@ -21,6 +21,7 @@ import jakarta.persistence.PersistenceContext;
|
|||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
|
import java.util.UUID;
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
|
|
||||||
@@ -707,4 +708,133 @@ class PersonRepositoryTest {
|
|||||||
assertThat(found).isPresent();
|
assertThat(found).isPresent();
|
||||||
assertThat(found.get().getGeneration()).isNull();
|
assertThat(found.get().getGeneration()).isNull();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ─── #684: ON DELETE integrity enforced at the database layer ──────────────
|
||||||
|
// A raw deleteById (bypassing PersonService) must keep referential integrity:
|
||||||
|
// documents.sender_id → SET NULL, document_receivers.person_id → CASCADE, and the
|
||||||
|
// transcription_block_mentioned_persons soft reference → CASCADE. These run against
|
||||||
|
// real Postgres because the FK ON DELETE behaviour never fires on H2.
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void deleteById_personSenderOfAReceiverOfB_nullsSender_dropsReceiverRow_bothDocumentsSurvive() {
|
||||||
|
Person target = personRepository.save(Person.builder().firstName("Weg").lastName("Person").build());
|
||||||
|
Person bystander = personRepository.save(Person.builder().firstName("Bleibt").lastName("Hier").build());
|
||||||
|
|
||||||
|
Document sent = documentRepository.save(Document.builder()
|
||||||
|
.title("Gesendet").originalFilename("sent.pdf")
|
||||||
|
.status(DocumentStatus.UPLOADED).sender(target).build());
|
||||||
|
Document received = documentRepository.save(Document.builder()
|
||||||
|
.title("Empfangen").originalFilename("received.pdf")
|
||||||
|
.status(DocumentStatus.UPLOADED).sender(bystander)
|
||||||
|
.receivers(Set.of(target)).build());
|
||||||
|
entityManager.flush();
|
||||||
|
entityManager.clear();
|
||||||
|
|
||||||
|
personRepository.deleteById(target.getId());
|
||||||
|
entityManager.flush();
|
||||||
|
entityManager.clear();
|
||||||
|
|
||||||
|
assertThat(personRepository.findById(target.getId())).isEmpty();
|
||||||
|
|
||||||
|
Document reloadedSent = documentRepository.findById(sent.getId()).orElseThrow();
|
||||||
|
assertThat(reloadedSent.getSender()).isNull(); // AC-1: SET NULL
|
||||||
|
|
||||||
|
Document reloadedReceived = documentRepository.findById(received.getId()).orElseThrow();
|
||||||
|
assertThat(reloadedReceived.getReceivers())
|
||||||
|
.noneMatch(p -> p.getId().equals(target.getId())); // AC-2: CASCADE drops the join row
|
||||||
|
|
||||||
|
// Cascade-boundary guard (Nora, non-negotiable): the cascade stops at the join/reference
|
||||||
|
// layer — both documents themselves survive. Guards against a future migration turning
|
||||||
|
// documents.sender_id SET NULL into CASCADE and destroying historical letters.
|
||||||
|
assertThat(documentRepository.findById(sent.getId())).isPresent();
|
||||||
|
assertThat(documentRepository.findById(received.getId())).isPresent();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void deleteById_receiverWithCoReceiver_dropsOnlyDeletedPersonsJoinRow() {
|
||||||
|
Person target = personRepository.save(Person.builder().firstName("Weg").lastName("Person").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 doc = documentRepository.save(Document.builder()
|
||||||
|
.title("Brief").originalFilename("brief.pdf")
|
||||||
|
.status(DocumentStatus.UPLOADED).sender(sender)
|
||||||
|
.receivers(Set.of(target, coReceiver)).build());
|
||||||
|
entityManager.flush();
|
||||||
|
entityManager.clear();
|
||||||
|
|
||||||
|
personRepository.deleteById(target.getId());
|
||||||
|
entityManager.flush();
|
||||||
|
entityManager.clear();
|
||||||
|
|
||||||
|
Document reloaded = documentRepository.findById(doc.getId()).orElseThrow();
|
||||||
|
assertThat(reloaded.getReceivers()).extracting(Person::getId)
|
||||||
|
.containsExactly(coReceiver.getId()); // co-receiver untouched
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void deleteById_personIsSenderAndReceiverOfSameDocument_documentSurvives_senderNull_receiverDropped() {
|
||||||
|
// AC-8: the trickier same-document interaction the cross-document cases don't exercise.
|
||||||
|
Person target = personRepository.save(Person.builder().firstName("Beides").lastName("Person").build());
|
||||||
|
Person coReceiver = personRepository.save(Person.builder().firstName("Mit").lastName("Empfänger").build());
|
||||||
|
|
||||||
|
Document doc = documentRepository.save(Document.builder()
|
||||||
|
.title("Selbstbrief").originalFilename("self.pdf")
|
||||||
|
.status(DocumentStatus.UPLOADED).sender(target)
|
||||||
|
.receivers(Set.of(target, coReceiver)).build());
|
||||||
|
entityManager.flush();
|
||||||
|
entityManager.clear();
|
||||||
|
|
||||||
|
personRepository.deleteById(target.getId());
|
||||||
|
entityManager.flush();
|
||||||
|
entityManager.clear();
|
||||||
|
|
||||||
|
Document reloaded = documentRepository.findById(doc.getId()).orElseThrow();
|
||||||
|
assertThat(reloaded.getSender()).isNull();
|
||||||
|
assertThat(reloaded.getReceivers()).extracting(Person::getId)
|
||||||
|
.containsExactly(coReceiver.getId());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void deleteById_mentionedPerson_dropsMentionRow_blockTextSurvives() {
|
||||||
|
// AC-3: the @-mention sidecar is a CASCADE soft reference, but the literal "@Name" lives
|
||||||
|
// in transcription_blocks.text and must stay visible as plain text after the person goes.
|
||||||
|
Person mentioned = personRepository.save(Person.builder().firstName("Auguste").lastName("Raddatz").build());
|
||||||
|
Document doc = documentRepository.save(Document.builder()
|
||||||
|
.title("Brief").originalFilename("brief.pdf")
|
||||||
|
.status(DocumentStatus.UPLOADED).build());
|
||||||
|
entityManager.flush();
|
||||||
|
|
||||||
|
UUID annotationId = UUID.randomUUID();
|
||||||
|
UUID blockId = UUID.randomUUID();
|
||||||
|
entityManager.createNativeQuery(
|
||||||
|
"INSERT INTO document_annotations (id, document_id, page_number, x, y, width, height, color) "
|
||||||
|
+ "VALUES (?1, ?2, 1, 0.1, 0.2, 0.3, 0.1, '#fff')")
|
||||||
|
.setParameter(1, annotationId).setParameter(2, doc.getId()).executeUpdate();
|
||||||
|
entityManager.createNativeQuery(
|
||||||
|
"INSERT INTO transcription_blocks (id, annotation_id, document_id, text) VALUES (?1, ?2, ?3, ?4)")
|
||||||
|
.setParameter(1, blockId).setParameter(2, annotationId).setParameter(3, doc.getId())
|
||||||
|
.setParameter(4, "Brief an @Auguste Raddatz").executeUpdate();
|
||||||
|
entityManager.createNativeQuery(
|
||||||
|
"INSERT INTO transcription_block_mentioned_persons (block_id, person_id, display_name) "
|
||||||
|
+ "VALUES (?1, ?2, ?3)")
|
||||||
|
.setParameter(1, blockId).setParameter(2, mentioned.getId())
|
||||||
|
.setParameter(3, "Auguste Raddatz").executeUpdate();
|
||||||
|
entityManager.flush();
|
||||||
|
entityManager.clear();
|
||||||
|
|
||||||
|
personRepository.deleteById(mentioned.getId());
|
||||||
|
entityManager.flush();
|
||||||
|
entityManager.clear();
|
||||||
|
|
||||||
|
Number mentionRows = (Number) entityManager.createNativeQuery(
|
||||||
|
"SELECT count(*) FROM transcription_block_mentioned_persons WHERE person_id = ?1")
|
||||||
|
.setParameter(1, mentioned.getId()).getSingleResult();
|
||||||
|
assertThat(mentionRows.longValue()).isZero();
|
||||||
|
|
||||||
|
String text = (String) entityManager.createNativeQuery(
|
||||||
|
"SELECT text FROM transcription_blocks WHERE id = ?1")
|
||||||
|
.setParameter(1, blockId).getSingleResult();
|
||||||
|
assertThat(text).isEqualTo("Brief an @Auguste Raddatz");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user