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..d709eb9c 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: {@code {"documentId": "uuid"}} */ + 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/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..dce06511 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -80,6 +80,7 @@ public class DocumentService { private final TranscriptionBlockQueryService transcriptionBlockQueryService; private final AuditLogQueryService auditLogQueryService; private final ThumbnailAsyncRunner thumbnailAsyncRunner; + private final org.springframework.context.ApplicationEventPublisher eventPublisher; public record StoreResult(Document document, boolean isNew) {} @@ -1101,7 +1102,9 @@ public class DocumentService { 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, null, 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..130490f7 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,19 @@ 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. + */ + @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); + /** * 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/geschichte/journeyitem/JourneyItemDocumentDeleteTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteTest.java new file mode 100644 index 00000000..4a042b03 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteTest.java @@ -0,0 +1,259 @@ +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); + 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()); + + 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()); + 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()); + 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()); + 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())) + .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()); + 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()); + 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()); + + verify(auditService).logAfterCommit(eq(AuditKind.DOCUMENT_DELETED), any(), 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..75ce305a 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()); 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()); em.flush(); em.clear();