diff --git a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java index 7b63ec27..b666c8d8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java @@ -52,6 +52,11 @@ public enum AuditKind { /** Payload: {@code {"ip": "1.2.3.4", "email": "addr"}} — password NEVER included */ LOGIN_RATE_LIMITED, + // --- Documents --- + + /** Payload: none — the deleted document's id is carried in the documentId column */ + DOCUMENT_DELETED, + // --- Reading Journeys (Lesereisen) --- /** Payload: {@code {"geschichteId": "uuid", "itemId": "uuid"}} — documentId is null (journey-scoped, not document-scoped) */ diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java index beb98256..5aa58ca7 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java @@ -168,8 +168,8 @@ public class DocumentController { @DeleteMapping("/{id}") @RequirePermission(Permission.WRITE_ALL) - public ResponseEntity deleteDocument(@PathVariable UUID id) { - documentService.deleteDocument(id); + public ResponseEntity deleteDocument(@PathVariable UUID id, Authentication authentication) { + documentService.deleteDocument(id, requireUserId(authentication)); return ResponseEntity.noContent().build(); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentDeletingEvent.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentDeletingEvent.java new file mode 100644 index 00000000..ed30aa91 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentDeletingEvent.java @@ -0,0 +1,11 @@ +package org.raddatz.familienarchiv.document; + +import java.util.UUID; + +/** + * Published by DocumentService.deleteDocument inside its @Transactional boundary, + * before documentRepository.deleteById fires. Listeners run synchronously in the + * publisher's thread and transaction via plain @EventListener — this is load-bearing: + * see ADR-038. + */ +public record DocumentDeletingEvent(UUID documentId) {} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java index 011ab3b1..61b578de 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -28,6 +28,7 @@ import org.raddatz.familienarchiv.ocr.TrainingLabel; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.tag.Tag; import org.raddatz.familienarchiv.document.DocumentRepository; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; @@ -80,6 +81,7 @@ public class DocumentService { private final TranscriptionBlockQueryService transcriptionBlockQueryService; private final AuditLogQueryService auditLogQueryService; private final ThumbnailAsyncRunner thumbnailAsyncRunner; + private final ApplicationEventPublisher eventPublisher; public record StoreResult(Document document, boolean isNew) {} @@ -1097,11 +1099,13 @@ public class DocumentService { } @Transactional - public void deleteDocument(UUID id) { + public void deleteDocument(UUID id, UUID actorId) { if (!documentRepository.existsById(id)) { throw DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id); } + eventPublisher.publishEvent(new DocumentDeletingEvent(id)); documentRepository.deleteById(id); + auditService.logAfterCommit(AuditKind.DOCUMENT_DELETED, actorId, id, null); } @Transactional diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteListener.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteListener.java new file mode 100644 index 00000000..01a7cd4e --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteListener.java @@ -0,0 +1,30 @@ +package org.raddatz.familienarchiv.geschichte.journeyitem; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.raddatz.familienarchiv.document.DocumentDeletingEvent; +import org.springframework.context.event.EventListener; +import org.springframework.stereotype.Component; + +@Component +@RequiredArgsConstructor +@Slf4j +class JourneyItemDocumentDeleteListener { + + private final JourneyItemRepository journeyItemRepository; + + /** + * Plain @EventListener — runs synchronously in the publisher's thread and transaction. + * Load-bearing choice: AFTER_COMMIT would fire after the FK ON DELETE SET NULL has + * already 500'd; @Async would run outside the delete transaction (breaks AC-5 rollback). + * See ADR-038. DocumentService cannot call JourneyItemService directly because + * Spring Framework 7 prohibits the resulting constructor-injection cycle. + */ + @EventListener + void onDocumentDeleting(DocumentDeletingEvent event) { + int deleted = journeyItemRepository.deleteNoteLessByDocumentId(event.documentId()); + if (deleted > 0) { + log.warn("Cascade-deleted {} note-less journey item(s) for document {}", deleted, event.documentId()); + } + } +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java index 66b9ab27..7f7d38ab 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java @@ -1,6 +1,7 @@ package org.raddatz.familienarchiv.geschichte.journeyitem; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; @@ -43,6 +44,20 @@ public interface JourneyItemRepository extends JpaRepository boolean existsByGeschichteIdAndDocumentId( @Param("geschichteId") UUID geschichteId, @Param("documentId") UUID documentId); + /** + * Deletes note-less items (note IS NULL or note = '') linked to the given document. + * Used by JourneyItemDocumentDeleteListener before the document row is removed, so + * the FK ON DELETE SET NULL never fires on rows that would violate chk_journey_item_not_empty. + * Explicit JPQL — same trap as existsByGeschichteIdAndDocumentId: the transient + * getDocumentId() getter makes Spring Data unable to resolve a derived query path. + * clearAutomatically = true invalidates the L1 cache so AC-2's "note-carrying survives" + * assertion never reads a stale entity. flushAutomatically = true makes the + * flush-before-delete contract explicit rather than relying on Hibernate AUTO flush mode. + */ + @Modifying(clearAutomatically = true, flushAutomatically = true) + @Query("DELETE FROM JourneyItem i WHERE i.document.id = :documentId AND (i.note IS NULL OR i.note = '')") + int deleteNoteLessByDocumentId(@Param("documentId") UUID documentId); + /** * Loads journey items with their linked Document in a single JOIN FETCH query, * eliminating the N+1 SELECT that would occur when accessing item.getDocument() diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java index b655e0bf..15934986 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java @@ -402,6 +402,7 @@ class DocumentControllerTest { @WithMockUser(authorities = "WRITE_ALL") void deleteDocument_returns204_whenHasWritePermission() throws Exception { UUID id = UUID.randomUUID(); + when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders .delete("/api/documents/" + id).with(csrf())) .andExpect(status().isNoContent()); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java index 65fbb21f..eadad6ac 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java @@ -30,6 +30,7 @@ import org.raddatz.familienarchiv.document.DocumentRepository; import org.raddatz.familienarchiv.filestorage.FileService; import org.raddatz.familienarchiv.tag.TagService; import org.raddatz.familienarchiv.person.PersonService; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; @@ -75,6 +76,7 @@ class DocumentServiceTest { @Mock AuditLogQueryService auditLogQueryService; @Mock TranscriptionBlockQueryService transcriptionBlockQueryService; @Mock ThumbnailAsyncRunner thumbnailAsyncRunner; + @Mock ApplicationEventPublisher eventPublisher; // Real factory (pure, dependency-free) so save-time title-regeneration tests exercise the // shared composition rather than a stub — the #726 single source of truth. @Spy DocumentTitleFactory documentTitleFactory = new DocumentTitleFactory(); @@ -87,7 +89,7 @@ class DocumentServiceTest { UUID id = UUID.randomUUID(); when(documentRepository.existsById(id)).thenReturn(true); - documentService.deleteDocument(id); + documentService.deleteDocument(id, UUID.randomUUID()); verify(documentRepository).deleteById(id); } @@ -97,7 +99,7 @@ class DocumentServiceTest { UUID id = UUID.randomUUID(); when(documentRepository.existsById(id)).thenReturn(false); - assertThatThrownBy(() -> documentService.deleteDocument(id)) + assertThatThrownBy(() -> documentService.deleteDocument(id, UUID.randomUUID())) .isInstanceOf(DomainException.class) .hasMessageContaining(id.toString()); verify(documentRepository, never()).deleteById(any()); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteTest.java new file mode 100644 index 00000000..f1319cb2 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteTest.java @@ -0,0 +1,261 @@ +package org.raddatz.familienarchiv.geschichte.journeyitem; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.PersistenceContext; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; +import org.raddatz.familienarchiv.document.Document; +import org.raddatz.familienarchiv.document.DocumentRepository; +import org.raddatz.familienarchiv.document.DocumentService; +import org.raddatz.familienarchiv.document.DocumentStatus; +import org.raddatz.familienarchiv.geschichte.Geschichte; +import org.raddatz.familienarchiv.geschichte.GeschichteRepository; +import org.raddatz.familienarchiv.geschichte.GeschichteStatus; +import org.raddatz.familienarchiv.geschichte.GeschichteType; +import org.raddatz.familienarchiv.user.AppUser; +import org.raddatz.familienarchiv.user.AppUserRepository; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.Import; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import org.springframework.test.context.bean.override.mockito.MockitoSpyBean; +import software.amazon.awssdk.services.s3.S3Client; + +import java.util.List; +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.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; + +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) +@ActiveProfiles("test") +@Import(PostgresContainerConfig.class) +class JourneyItemDocumentDeleteTest { + + @MockitoBean + S3Client s3Client; + + @MockitoBean + AuditService auditService; + + @MockitoSpyBean + DocumentRepository documentRepository; + + @PersistenceContext + EntityManager em; + + @Autowired DocumentService documentService; + @Autowired JourneyItemRepository journeyItemRepository; + @Autowired GeschichteRepository geschichteRepository; + @Autowired DocumentRepository docRepo; + @Autowired AppUserRepository appUserRepository; + @Autowired JdbcTemplate jdbcTemplate; + + Geschichte journey; + Document doc; + AppUser writer; + + @BeforeEach + void seed() { + writer = appUserRepository.save(AppUser.builder() + .email("delete-test-writer@test") + .password("hash") + .build()); + doc = docRepo.save(Document.builder() + .title("Testbrief") + .originalFilename("testbrief.pdf") + .status(DocumentStatus.UPLOADED) + .build()); + journey = geschichteRepository.save(Geschichte.builder() + .title("Eine Lesereise") + .status(GeschichteStatus.DRAFT) + .type(GeschichteType.JOURNEY) + .build()); + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken(writer.getEmail(), null, + List.of(new SimpleGrantedAuthority("BLOG_WRITE")))); + } + + @AfterEach + void cleanup() { + SecurityContextHolder.clearContext(); + reset(documentRepository); + // Deletion order is FK-load-bearing: journey_items reference both documents + // and geschichten, so children must be removed before their parents. + journeyItemRepository.deleteAll(); + docRepo.deleteAll(); + geschichteRepository.deleteAll(); + appUserRepository.deleteAll(); + } + + // ─── AC-1: headline ─────────────────────────────────────────────────────── + + @Test + void deleting_document_linked_via_note_less_item_deletes_item_not_500() { + JourneyItem item = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).document(doc).build()); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + + assertThat(journeyItemRepository.findById(item.getId())).isEmpty(); + assertThat(docRepo.findById(doc.getId())).isEmpty(); + } + + // ─── AC-2: note-carrying item survives as placeholder ───────────────────── + + @Test + void deleting_document_preserves_note_carrying_item_as_placeholder() { + JourneyItem item = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).document(doc).note("curator context").build()); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + em.clear(); + + JourneyItem surviving = journeyItemRepository.findById(item.getId()).orElseThrow(); + assertThat(surviving.getDocumentId()).isNull(); + assertThat(surviving.getNote()).isEqualTo("curator context"); + } + + // ─── AC-3: note-only item untouched ─────────────────────────────────────── + + @Test + void deleting_document_does_not_affect_note_only_item() { + JourneyItem noteOnly = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).note("Einleitung").build()); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + em.clear(); + + JourneyItem reloaded = journeyItemRepository.findById(noteOnly.getId()).orElseThrow(); + assertThat(reloaded.getDocumentId()).isNull(); + assertThat(reloaded.getNote()).isEqualTo("Einleitung"); + } + + // ─── AC-4: asymmetric multi-journey ─────────────────────────────────────── + + @Test + void deleting_document_applies_independently_per_referencing_item() { + Geschichte journey2 = geschichteRepository.save(Geschichte.builder() + .title("Zweite Reise") + .status(GeschichteStatus.DRAFT) + .type(GeschichteType.JOURNEY) + .build()); + + JourneyItem noteLess = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).document(doc).build()); + JourneyItem noteCarrying = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey2).position(10).document(doc).note("Begleittext").build()); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + em.clear(); + + assertThat(journeyItemRepository.findById(noteLess.getId())).isEmpty(); + JourneyItem surviving = journeyItemRepository.findById(noteCarrying.getId()).orElseThrow(); + assertThat(surviving.getDocumentId()).isNull(); + assertThat(surviving.getNote()).isEqualTo("Begleittext"); + } + + // ─── AC-5: rollback guard ───────────────────────────────────────────────── + + @Test + void listener_deletes_roll_back_when_document_delete_fails() { + JourneyItem item = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).document(doc).build()); + em.clear(); + + doThrow(new RuntimeException("simulated failure")) + .when(documentRepository).deleteById(any()); + + assertThatThrownBy(() -> documentService.deleteDocument(doc.getId(), writer.getId())) + .isInstanceOf(RuntimeException.class); + + em.clear(); + assertThat(journeyItemRepository.findById(item.getId())).isPresent(); + } + + // ─── AC-6: empty-string note boundary ──────────────────────────────────── + + @Test + void empty_string_note_item_is_cascaded_whitespace_only_note_is_preserved() { + // uq_journey_items_geschichte_document prevents two items with the same + // (geschichte_id, document_id) in one journey — use two separate journeys. + Geschichte journey2 = geschichteRepository.save(Geschichte.builder() + .title("Zweite Reise für AC-6") + .status(GeschichteStatus.DRAFT) + .type(GeschichteType.JOURNEY) + .build()); + + UUID emptyNoteItemId = UUID.randomUUID(); + UUID whitespaceNoteItemId = UUID.randomUUID(); + + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, document_id, note) VALUES (?,?,?,?,?)", + emptyNoteItemId, journey.getId(), 10, doc.getId(), ""); + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, document_id, note) VALUES (?,?,?,?,?)", + whitespaceNoteItemId, journey2.getId(), 20, doc.getId(), " "); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + em.clear(); + + assertThat(journeyItemRepository.findById(emptyNoteItemId)).isEmpty(); + JourneyItem whitespaceItem = journeyItemRepository.findById(whitespaceNoteItemId).orElseThrow(); + assertThat(whitespaceItem.getDocumentId()).isNull(); + assertThat(whitespaceItem.getNote()).isEqualTo(" "); + } + + // ─── Idempotency / no-collateral ────────────────────────────────────────── + + @Test + void deleting_document_in_zero_journeys_returns_no_collateral() { + Document unlinked = docRepo.save(Document.builder() + .title("Unverknüpfter Brief") + .originalFilename("other.pdf") + .status(DocumentStatus.UPLOADED) + .build()); + JourneyItem unrelated = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).note("unrelated note").build()); + em.clear(); + + documentService.deleteDocument(unlinked.getId(), writer.getId()); + em.clear(); + + assertThat(docRepo.findById(unlinked.getId())).isEmpty(); + assertThat(journeyItemRepository.findById(unrelated.getId())).isPresent(); + assertThat(journeyItemRepository.count()).isEqualTo(1); + } + + // ─── AC-7: audit — DOCUMENT_DELETED emitted, JOURNEY_ITEM_REMOVED absent ─ + + @Test + void deleting_document_emits_document_audit_but_no_journey_item_audit() { + journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).document(doc).build()); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + + verify(auditService).logAfterCommit(eq(AuditKind.DOCUMENT_DELETED), eq(writer.getId()), eq(doc.getId()), any()); + verify(auditService, never()).logAfterCommit(eq(AuditKind.JOURNEY_ITEM_REMOVED), any(), any(), any()); + } +} diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java index 819cf3e0..59a2bf83 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java @@ -6,8 +6,10 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.document.Document; import org.raddatz.familienarchiv.document.DocumentRepository; +import org.raddatz.familienarchiv.document.DocumentService; import org.raddatz.familienarchiv.document.DocumentStatus; import org.raddatz.familienarchiv.geschichte.Geschichte; import org.raddatz.familienarchiv.geschichte.GeschichteRepository; @@ -42,12 +44,16 @@ class JourneyItemIntegrationTest { @MockitoBean S3Client s3Client; + @MockitoBean + AuditService auditService; + @PersistenceContext EntityManager em; @Autowired GeschichteRepository geschichteRepository; @Autowired JourneyItemRepository journeyItemRepository; @Autowired JourneyItemService journeyItemService; + @Autowired DocumentService documentService; @Autowired DocumentRepository documentRepository; @Autowired AppUserRepository appUserRepository; @@ -212,8 +218,9 @@ class JourneyItemIntegrationTest { UUID itemId = saved.getItems().get(0).getId(); // extract before clear em.clear(); - // Delete document — ON DELETE SET NULL fires at DB level - documentRepository.deleteById(doc.getId()); + // Route through service so the DocumentDeletingEvent fires and the listener + // removes note-less items before ON DELETE SET NULL acts on note-carrying rows. + documentService.deleteDocument(doc.getId(), writer.getId()); em.flush(); em.clear(); @@ -351,8 +358,8 @@ class JourneyItemIntegrationTest { em.flush(); em.clear(); - // ON DELETE SET NULL fires at DB level (V72) - documentRepository.deleteById(doc.getId()); + // Route through service so the DocumentDeletingEvent fires (V72 cascade fix). + documentService.deleteDocument(doc.getId(), writer.getId()); em.flush(); em.clear(); diff --git a/docs/adr/038-domain-event-driven-journey-item-cleanup.md b/docs/adr/038-domain-event-driven-journey-item-cleanup.md new file mode 100644 index 00000000..83096f6e --- /dev/null +++ b/docs/adr/038-domain-event-driven-journey-item-cleanup.md @@ -0,0 +1,118 @@ +# ADR-038 — Domain event drives note-less journey-item cleanup on document delete + +**Status:** Accepted +**Date:** 2026-06-11 +**Issue:** #805 (P1 — deleting a document linked via a note-less journey_item 500s at DB constraint) + +## Context + +Two constraints in V72 encode contradictory rules for a journey item that has a +`document_id` but no `note`: + +- **`fk_journey_items_document` → `ON DELETE SET NULL`** — when a document is deleted, + Postgres nulls out `document_id`. +- **`chk_journey_item_not_empty`** — requires at least one of `document_id` or `note` + to be non-null. + +A note-less item (`document_id` set, `note IS NULL`) satisfies the CHECK while the +document exists. Deleting the document causes Postgres to attempt `SET NULL`, which +would leave both columns null — a direct CHECK violation. Postgres aborts the +transaction with a 500 that bypasses `GlobalExceptionHandler`. + +The natural fix — delete note-less items inside `DocumentService.deleteDocument` before +`deleteById` runs — cannot call `JourneyItemService` directly: `JourneyItemService` +already injects `DocumentService`, and Spring Framework 7 (used by Spring Boot 4) +**fully prohibits constructor-injection cycles**. The application will not start if such +a cycle is introduced. + +## Decision + +`DocumentService.deleteDocument` publishes a **`DocumentDeletingEvent`** (plain record, +payload: `documentId` UUID only) via `ApplicationEventPublisher` **before** +`documentRepository.deleteById`. A dedicated `@Component` +`JourneyItemDocumentDeleteListener` in the `geschichte.journeyitem` package consumes +this event and calls `journeyItemRepository.deleteNoteLessByDocumentId(documentId)` +directly — bypassing `JourneyItemService` to avoid re-introducing the cycle and to +suppress the per-item `JOURNEY_ITEM_REMOVED` audit emission (see audit decision below). + +### Load-bearing listener-phase choice: plain `@EventListener` + +The listener is annotated with `@EventListener` (not +`@TransactionalEventListener(AFTER_COMMIT)`, not `@Async`). **This choice is +load-bearing:** + +- **`AFTER_COMMIT` would break the fix entirely.** `AFTER_COMMIT` fires *after* the + surrounding transaction has committed. By that point, `documentRepository.deleteById` + has already executed and Postgres has already tried `ON DELETE SET NULL` — the + constraint violation fires before the listener ever runs. +- **`@Async` would break rollback atomicity (AC-5).** An async listener runs on a + separate thread in its own transaction. If `deleteDocument` subsequently rolls back + (e.g. due to an unrelated failure), the listener's deletes are in a committed async + transaction and cannot be undone. +- **Plain `@EventListener` runs synchronously in the publisher's thread and + transaction.** The listener's JPQL delete and the `deleteById` are a single atomic + unit: if either fails, both roll back together. + +### Repository method + +```java +@Modifying(clearAutomatically = true) +@Query("DELETE FROM JourneyItem i WHERE i.document.id = :documentId AND (i.note IS NULL OR i.note = '')") +int deleteNoteLessByDocumentId(@Param("documentId") UUID documentId); +``` + +`i.document.id` (the real association path) is used instead of `i.documentId`: the +transient `getDocumentId()` getter on `JourneyItem` makes Spring Data unable to resolve +a derived query path (same trap documented at `JourneyItemRepository:33-44`). + +`clearAutomatically = true` invalidates the L1 cache so subsequent reads in the same +session do not return stale entities. + +The predicate `(note IS NULL OR note = '')` covers the `note = ''` edge case that the +service layer can never produce (normalizeNote converts blank strings to null), but that +may exist via raw SQL inserts or legacy data. Whitespace-only notes (`note = ' '`) +do not match and are preserved as note-carrying placeholders. + +### Audit decision + +The listener calls the repository directly rather than routing through +`JourneyItemService.delete`. This deliberately bypasses the `JOURNEY_ITEM_REMOVED` +audit emission: a document used in multiple journeys would otherwise produce N audit +rows for a single user action. The `DOCUMENT_DELETED` entry written by `deleteDocument` +is the sole audit record for the operation. + +### Boundary: documents must not depend on journey + +The event direction is `document → journey`, never the reverse. `DocumentService` +publishes events it knows nothing about the consumers of; `JourneyItemService`'s +dependency on `DocumentService` is unchanged and remains the only cross-domain +reference. This direction is the prerequisite for the cycle constraint to hold. + +## Alternatives rejected + +- **DB trigger on `journey_items`** — trigger logic is opaque to Java developers, + invisible to code review, and not covered by the JPA test harness. +- **RESTRICT instead of SET NULL** — breaks the existing note-carrying placeholder + UX: deleting a document with a note-carrying journey item would 409 instead of + preserving the item as a placeholder. +- **Relax `chk_journey_item_not_empty`** — the constraint enforces a real invariant + (every item must have at least document or note). Removing it would allow empty rows. +- **`@Lazy` on the `JourneyItemService → DocumentService` injection** — Spring Boot 4 / + Spring Framework 7 prohibits constructor-injection cycles regardless of `@Lazy`. +- **Make `DocumentService` call `JourneyItemService`** — introduces the prohibited + cycle. Rejected at design time. + +## Consequences + +- **No schema change** — no new Flyway migration, no `db-orm.puml` / + `db-relationships.puml` update. +- This is the **first custom domain event** in the codebase. No prior + `ApplicationEventPublisher` usage existed in `main/`. New cross-domain cleanup that + cannot use direct service calls should follow this pattern. +- All tests that delete documents and then assert journey-item state **must route + through `DocumentService.deleteDocument`**, not `documentRepository.deleteById`. + The existing `JourneyItemIntegrationTest` tests that covered the note-carrying + placeholder UX have been updated accordingly. +- The `DOCUMENT_DELETED` `AuditKind` was added as part of this fix to give AC-7's + audit assertion a positive check (absence-only assertions pass vacuously if all + auditing regresses). diff --git a/docs/architecture/c4/l3-backend-3g-supporting.puml b/docs/architecture/c4/l3-backend-3g-supporting.puml index ed6a370b..8f550f89 100644 --- a/docs/architecture/c4/l3-backend-3g-supporting.puml +++ b/docs/architecture/c4/l3-backend-3g-supporting.puml @@ -20,6 +20,7 @@ System_Boundary(backend, "API Backend (Spring Boot)") { Component(geschSvc, "GeschichteService", "Spring Service", "Manages story lifecycle (DRAFT → PUBLISHED with timestamp). Supports two subtypes: STORY (prose) and JOURNEY (ordered JourneyItem sequence). Sanitizes HTML body with an allowlist policy.") Component(geschQuerySvc, "GeschichteQueryService", "Spring Service", "Read-only facade over GeschichteRepository. Exposes existsById() and findById() to prevent JourneyItemService from crossing domain boundaries.") Component(journeyItemSvc, "JourneyItemService", "Spring Service", "Manages journey item lifecycle: append (100-item cap), updateNote (three-way PATCH), delete, and reorder (DEFERRABLE position swap). Serves both STORY and JOURNEY subtypes.") + Component(journeyListener, "JourneyItemDocumentDeleteListener", "Spring @EventListener", "Consumes DocumentDeletingEvent synchronously inside the delete transaction and removes note-less journey items before ON DELETE SET NULL fires, preventing a chk_journey_item_not_empty violation. See ADR-038.") Component(exHandler, "GlobalExceptionHandler", "Spring @RestControllerAdvice", "Converts DomainException, validation errors, and generic exceptions to ErrorResponse JSON with machine-readable ErrorCode and HTTP status.") } @@ -44,6 +45,8 @@ Rel(geschCtrl, journeyItemSvc, "Delegates journey item CRUD") Rel(journeyItemSvc, geschQuerySvc, "Checks Geschichte existence") Rel(geschQuerySvc, db, "Reads geschichten", "JDBC") Rel(journeyItemSvc, db, "Reads / writes journey_items", "JDBC") +Rel(documentSvc, journeyListener, "DocumentDeletingEvent", "in-process event") +Rel(journeyListener, db, "Deletes note-less journey_items", "JDBC") Rel(auditSvc, db, "Writes audit_log", "JDBC") Rel(auditQuery, db, "Reads audit_log", "JDBC") Rel(notifSvc, db, "Reads / writes notifications", "JDBC")