From 793b863096fd514848273922d4a99b32c4fb1d6e Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 19 Apr 2026 13:17:54 +0200 Subject: [PATCH 1/5] feat(audit): add audit_log infrastructure and instrument AnnotationService - V46 migration: audit_log table with indexes and append-only REVOKE - audit/ package: AuditKind enum (with Javadoc payloads), AuditLog entity, AuditLogRepository, AuditService (@Async on dedicated auditExecutor) - AsyncConfig: auditExecutor with CallerRunsPolicy and queueCapacity 50 - AnnotationService: ANNOTATION_CREATED on createAnnotation() only, deferred via afterCommit() when inside a transaction Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/audit/AuditKind.java | 22 ++++++ .../familienarchiv/audit/AuditLog.java | 46 ++++++++++++ .../audit/AuditLogRepository.java | 8 ++ .../familienarchiv/audit/AuditService.java | 31 ++++++++ .../familienarchiv/config/AsyncConfig.java | 11 +++ .../service/AnnotationService.java | 24 +++++- .../db/migration/V46__add_audit_log.sql | 22 ++++++ .../audit/AuditServiceTest.java | 74 +++++++++++++++++++ .../service/AnnotationServiceTest.java | 40 ++++++++++ 9 files changed, 277 insertions(+), 1 deletion(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/audit/AuditLog.java create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/audit/AuditLogRepository.java create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/audit/AuditService.java create mode 100644 backend/src/main/resources/db/migration/V46__add_audit_log.sql create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/audit/AuditServiceTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java new file mode 100644 index 00000000..cac1ef48 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java @@ -0,0 +1,22 @@ +package org.raddatz.familienarchiv.audit; + +public enum AuditKind { + + /** Payload: none */ + FILE_UPLOADED, + + /** Payload: {@code {"oldStatus": "UPLOADED", "newStatus": "TRANSCRIBED"}} */ + STATUS_CHANGED, + + /** Payload: none */ + METADATA_UPDATED, + + /** Payload: {@code {"pageNumber": 3}} */ + TEXT_SAVED, + + /** Payload: none */ + BLOCK_REVIEWED, + + /** Payload: {@code {"pageNumber": 3}} */ + ANNOTATION_CREATED, +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditLog.java b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditLog.java new file mode 100644 index 00000000..081a9839 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditLog.java @@ -0,0 +1,46 @@ +package org.raddatz.familienarchiv.audit; + +import io.swagger.v3.oas.annotations.media.Schema; +import jakarta.persistence.*; +import lombok.*; +import org.hibernate.annotations.CreationTimestamp; +import org.hibernate.annotations.JdbcTypeCode; +import org.hibernate.type.SqlTypes; + +import java.time.OffsetDateTime; +import java.util.Map; +import java.util.UUID; + +@Entity +@Table(name = "audit_log") +@Data +@NoArgsConstructor +@AllArgsConstructor +@Builder +public class AuditLog { + + @Id + @GeneratedValue(strategy = GenerationType.UUID) + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + private UUID id; + + @Column(name = "happened_at", nullable = false, updatable = false) + @CreationTimestamp + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + private OffsetDateTime happenedAt; + + @Column(name = "actor_id") + private UUID actorId; + + @Enumerated(EnumType.STRING) + @Column(name = "kind", nullable = false) + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + private AuditKind kind; + + @Column(name = "document_id") + private UUID documentId; + + @JdbcTypeCode(SqlTypes.JSON) + @Column(columnDefinition = "jsonb") + private Map payload; +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditLogRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditLogRepository.java new file mode 100644 index 00000000..2322c8c9 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditLogRepository.java @@ -0,0 +1,8 @@ +package org.raddatz.familienarchiv.audit; + +import org.springframework.data.jpa.repository.JpaRepository; + +import java.util.UUID; + +public interface AuditLogRepository extends JpaRepository { +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditService.java b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditService.java new file mode 100644 index 00000000..0bd1e727 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditService.java @@ -0,0 +1,31 @@ +package org.raddatz.familienarchiv.audit; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.scheduling.annotation.Async; +import org.springframework.stereotype.Service; + +import java.util.Map; +import java.util.UUID; + +@Service +@RequiredArgsConstructor +@Slf4j +public class AuditService { + + private final AuditLogRepository auditLogRepository; + + @Async("auditExecutor") + public void log(AuditKind kind, UUID actorId, UUID documentId, Map payload) { + try { + auditLogRepository.save(AuditLog.builder() + .kind(kind) + .actorId(actorId) + .documentId(documentId) + .payload(payload) + .build()); + } catch (Exception e) { + log.error("Audit log write failed: kind={}, document={}", kind, documentId, e); + } + } +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/config/AsyncConfig.java b/backend/src/main/java/org/raddatz/familienarchiv/config/AsyncConfig.java index acdac4c5..c2a2f55a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/config/AsyncConfig.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/config/AsyncConfig.java @@ -23,4 +23,15 @@ public class AsyncConfig { executor.setRejectedExecutionHandler(new ThreadPoolExecutor.AbortPolicy()); return executor; } + + @Bean("auditExecutor") + public Executor auditExecutor() { + ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); + executor.setCorePoolSize(1); + executor.setMaxPoolSize(2); + executor.setQueueCapacity(50); + executor.setThreadNamePrefix("Audit-"); + executor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy()); + return executor; + } } \ No newline at end of file diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java index 7616a9a0..ff3f9cb1 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java @@ -2,6 +2,8 @@ package org.raddatz.familienarchiv.service; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.dto.CreateAnnotationDTO; import org.raddatz.familienarchiv.dto.UpdateAnnotationDTO; import org.raddatz.familienarchiv.exception.DomainException; @@ -12,8 +14,11 @@ import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; import java.util.List; +import java.util.Map; import java.util.UUID; @Slf4j @@ -23,6 +28,7 @@ public class AnnotationService { private final AnnotationRepository annotationRepository; private final TranscriptionBlockRepository blockRepository; + private final AuditService auditService; public List listAnnotations(UUID documentId) { return annotationRepository.findByDocumentId(documentId); @@ -42,7 +48,10 @@ public class AnnotationService { .createdBy(userId) .build(); - return annotationRepository.save(annotation); + DocumentAnnotation saved = annotationRepository.save(annotation); + logAfterCommit(AuditKind.ANNOTATION_CREATED, userId, saved.getDocumentId(), + Map.of("pageNumber", saved.getPageNumber())); + return saved; } @Transactional @@ -108,4 +117,17 @@ public class AnnotationService { }); } + private void logAfterCommit(AuditKind kind, UUID actorId, UUID documentId, Map payload) { + if (TransactionSynchronizationManager.isActualTransactionActive()) { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + auditService.log(kind, actorId, documentId, payload); + } + }); + } else { + auditService.log(kind, actorId, documentId, payload); + } + } + } diff --git a/backend/src/main/resources/db/migration/V46__add_audit_log.sql b/backend/src/main/resources/db/migration/V46__add_audit_log.sql new file mode 100644 index 00000000..2a01126a --- /dev/null +++ b/backend/src/main/resources/db/migration/V46__add_audit_log.sql @@ -0,0 +1,22 @@ +-- Append-only audit trail for domain-level archive activity. +-- Enables dashboard queries (Family Pulse, activity feed, resume card) in #271. + +CREATE TABLE audit_log ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + happened_at TIMESTAMPTZ NOT NULL DEFAULT now(), + -- ON DELETE SET NULL is by design: GDPR right-to-erasure. Deleted users' events + -- retain their timestamp and kind but lose actor attribution. + actor_id UUID REFERENCES app_users(id) ON DELETE SET NULL, + kind VARCHAR(50) NOT NULL, + document_id UUID REFERENCES documents(id) ON DELETE CASCADE, + payload JSONB +); + +CREATE INDEX idx_audit_log_happened_at ON audit_log (happened_at DESC); +CREATE INDEX idx_audit_log_document_id ON audit_log (document_id); +CREATE INDEX idx_audit_log_actor_id ON audit_log (actor_id); +CREATE INDEX idx_audit_log_kind ON audit_log (kind); + +-- Enforce append-only at the database layer: the application role may INSERT +-- but must not UPDATE or DELETE audit rows. +REVOKE UPDATE, DELETE ON audit_log FROM app_user; diff --git a/backend/src/test/java/org/raddatz/familienarchiv/audit/AuditServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/audit/AuditServiceTest.java new file mode 100644 index 00000000..f8c85653 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/audit/AuditServiceTest.java @@ -0,0 +1,74 @@ +package org.raddatz.familienarchiv.audit; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Map; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class AuditServiceTest { + + @Mock AuditLogRepository auditLogRepository; + @InjectMocks AuditService auditService; + + @Test + void log_savesAuditRowWithCorrectFields() { + UUID actorId = UUID.randomUUID(); + UUID documentId = UUID.randomUUID(); + Map payload = Map.of("pageNumber", 3); + + when(auditLogRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + auditService.log(AuditKind.TEXT_SAVED, actorId, documentId, payload); + + ArgumentCaptor captor = ArgumentCaptor.forClass(AuditLog.class); + verify(auditLogRepository).save(captor.capture()); + AuditLog saved = captor.getValue(); + + assertThat(saved.getKind()).isEqualTo(AuditKind.TEXT_SAVED); + assertThat(saved.getActorId()).isEqualTo(actorId); + assertThat(saved.getDocumentId()).isEqualTo(documentId); + assertThat(saved.getPayload()).isEqualTo(payload); + } + + @Test + void log_doesNotPropagateException_whenRepoThrows() { + when(auditLogRepository.save(any())).thenThrow(new RuntimeException("DB down")); + + assertThatCode(() -> + auditService.log(AuditKind.METADATA_UPDATED, UUID.randomUUID(), UUID.randomUUID(), null) + ).doesNotThrowAnyException(); + } + + @Test + void log_acceptsNullPayload() { + when(auditLogRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + auditService.log(AuditKind.FILE_UPLOADED, UUID.randomUUID(), UUID.randomUUID(), null); + + ArgumentCaptor captor = ArgumentCaptor.forClass(AuditLog.class); + verify(auditLogRepository).save(captor.capture()); + assertThat(captor.getValue().getPayload()).isNull(); + } + + @Test + void log_acceptsNullActorId() { + when(auditLogRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + auditService.log(AuditKind.BLOCK_REVIEWED, null, UUID.randomUUID(), null); + + ArgumentCaptor captor = ArgumentCaptor.forClass(AuditLog.class); + verify(auditLogRepository).save(captor.capture()); + assertThat(captor.getValue().getActorId()).isNull(); + } +} diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java index bd3064a9..0ac0118d 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java @@ -5,6 +5,9 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.ArgumentCaptor; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.dto.CreateAnnotationDTO; import org.raddatz.familienarchiv.dto.UpdateAnnotationDTO; import org.raddatz.familienarchiv.exception.DomainException; @@ -13,6 +16,8 @@ import org.raddatz.familienarchiv.repository.AnnotationRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.springframework.dao.DataIntegrityViolationException; +import java.util.Map; + import java.util.List; import java.util.Optional; import java.util.UUID; @@ -32,6 +37,7 @@ class AnnotationServiceTest { @Mock AnnotationRepository annotationRepository; @Mock TranscriptionBlockRepository blockRepository; + @Mock AuditService auditService; @InjectMocks AnnotationService annotationService; // ─── createAnnotation ───────────────────────────────────────────────────── @@ -89,6 +95,40 @@ class AnnotationServiceTest { assertThat(result.getFileHash()).isNull(); } + @Test + void createAnnotation_logsAnnotationCreatedWithPageNumber() { + UUID docId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + CreateAnnotationDTO dto = new CreateAnnotationDTO(5, 0.1, 0.1, 0.3, 0.3, "#ff0000"); + DocumentAnnotation saved = DocumentAnnotation.builder() + .id(UUID.randomUUID()).documentId(docId).pageNumber(5) + .x(0.1).y(0.1).width(0.3).height(0.3).color("#ff0000").createdBy(userId).build(); + when(annotationRepository.save(any())).thenReturn(saved); + + annotationService.createAnnotation(docId, dto, userId, null); + + @SuppressWarnings("unchecked") + ArgumentCaptor> payloadCaptor = ArgumentCaptor.forClass(Map.class); + verify(auditService).log( + org.mockito.ArgumentMatchers.eq(AuditKind.ANNOTATION_CREATED), + org.mockito.ArgumentMatchers.eq(userId), + org.mockito.ArgumentMatchers.eq(docId), + payloadCaptor.capture()); + assertThat(payloadCaptor.getValue()).containsEntry("pageNumber", 5); + } + + @Test + void createOcrAnnotation_doesNotLogAuditEvent() { + UUID docId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + CreateAnnotationDTO dto = new CreateAnnotationDTO(1, 0.1, 0.1, 0.8, 0.04, "#00C7B1"); + when(annotationRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + annotationService.createOcrAnnotation(docId, dto, userId, null, null); + + verify(auditService, never()).log(any(), any(), any(), any()); + } + // ─── createOcrAnnotation ────────────────────────────────────────────────── @Test -- 2.49.1 From 988796823629f5b7d893543f952546a4d2927c1f Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 19 Apr 2026 13:24:22 +0200 Subject: [PATCH 2/5] feat(audit): instrument TranscriptionService for TEXT_SAVED and BLOCK_REVIEWED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - reviewBlock: add userId param; log BLOCK_REVIEWED only on false→true - updateBlock: log TEXT_SAVED only when text actually changes; include pageNumber in payload (resolved from annotation) - Both events deferred via afterCommit() when inside a transaction - Update TranscriptionBlockController to pass user to reviewBlock() Co-Authored-By: Claude Sonnet 4.6 --- .../TranscriptionBlockController.java | 6 +- .../service/TranscriptionService.java | 38 +++++++- .../TranscriptionBlockControllerTest.java | 3 +- .../TranscriptionServiceGuidedTest.java | 5 +- .../service/TranscriptionServiceTest.java | 89 ++++++++++++++++++- 5 files changed, 131 insertions(+), 10 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java index 55f059e0..7b36cd26 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java @@ -85,8 +85,10 @@ public class TranscriptionBlockController { @RequirePermission(Permission.WRITE_ALL) public TranscriptionBlock reviewBlock( @PathVariable UUID documentId, - @PathVariable UUID blockId) { - return transcriptionService.reviewBlock(documentId, blockId); + @PathVariable UUID blockId, + Authentication authentication) { + UUID userId = requireUserId(authentication); + return transcriptionService.reviewBlock(documentId, blockId, userId); } @GetMapping("/{blockId}/history") diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java index 081f588d..4ec7cf94 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java @@ -2,6 +2,8 @@ package org.raddatz.familienarchiv.service; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.dto.CreateAnnotationDTO; import org.raddatz.familienarchiv.dto.CreateTranscriptionBlockDTO; import org.raddatz.familienarchiv.dto.ReorderTranscriptionBlocksDTO; @@ -19,8 +21,12 @@ import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.UUID; @Service @@ -37,6 +43,7 @@ public class TranscriptionService { private final AnnotationService annotationService; private final DocumentService documentService; private final SenderModelService senderModelService; + private final AuditService auditService; public List listBlocks(UUID documentId) { return blockRepository.findByDocumentIdOrderBySortOrderAsc(documentId); @@ -122,6 +129,7 @@ public class TranscriptionService { UpdateTranscriptionBlockDTO dto, UUID userId) { TranscriptionBlock block = getBlock(documentId, blockId); + String previousText = block.getText(); String text = sanitizeText(dto.getText()); block.setText(text); block.setSource(BlockSource.MANUAL); @@ -133,6 +141,12 @@ public class TranscriptionService { TranscriptionBlock saved = blockRepository.save(block); saveVersion(saved, userId); + if (!text.equals(previousText)) { + Optional annotation = annotationRepository.findById(block.getAnnotationId()); + int pageNumber = annotation.map(DocumentAnnotation::getPageNumber).orElse(0); + logAfterCommit(AuditKind.TEXT_SAVED, userId, documentId, Map.of("pageNumber", pageNumber)); + } + Document doc = documentService.getDocumentById(documentId); if (doc.getSender() != null && doc.getScriptType() == ScriptType.HANDWRITING_KURRENT) { senderModelService.checkAndTriggerTraining(doc.getSender().getId()); @@ -181,10 +195,15 @@ public class TranscriptionService { } @Transactional - public TranscriptionBlock reviewBlock(UUID documentId, UUID blockId) { + public TranscriptionBlock reviewBlock(UUID documentId, UUID blockId, UUID userId) { TranscriptionBlock block = getBlock(documentId, blockId); - block.setReviewed(!block.isReviewed()); - return blockRepository.save(block); + boolean wasReviewed = block.isReviewed(); + block.setReviewed(!wasReviewed); + TranscriptionBlock saved = blockRepository.save(block); + if (!wasReviewed && saved.isReviewed()) { + logAfterCommit(AuditKind.BLOCK_REVIEWED, userId, documentId, null); + } + return saved; } public List getBlockHistory(UUID documentId, UUID blockId) { @@ -208,4 +227,17 @@ public class TranscriptionService { } return text; } + + private void logAfterCommit(AuditKind kind, UUID actorId, UUID documentId, Map payload) { + if (TransactionSynchronizationManager.isActualTransactionActive()) { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + auditService.log(kind, actorId, documentId, payload); + } + }); + } else { + auditService.log(kind, actorId, documentId, payload); + } + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java index 84d58d79..2231e9fb 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java @@ -362,10 +362,11 @@ class TranscriptionBlockControllerTest { @Test @WithMockUser(authorities = "WRITE_ALL") void reviewBlock_returns200_withToggledBlock() throws Exception { + when(userService.findByEmail(any())).thenReturn(mockUser()); TranscriptionBlock reviewed = TranscriptionBlock.builder() .id(BLOCK_ID).documentId(DOC_ID).annotationId(UUID.randomUUID()) .text("text").sortOrder(0).reviewed(true).build(); - when(transcriptionService.reviewBlock(DOC_ID, BLOCK_ID)).thenReturn(reviewed); + when(transcriptionService.reviewBlock(eq(DOC_ID), eq(BLOCK_ID), any())).thenReturn(reviewed); mockMvc.perform(put("/api/documents/{documentId}/transcription-blocks/{blockId}/review", DOC_ID, BLOCK_ID)) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java index 59eed034..0e786d83 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java @@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.service; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.model.BlockSource; import org.raddatz.familienarchiv.model.TranscriptionBlock; import org.raddatz.familienarchiv.repository.AnnotationRepository; @@ -23,6 +24,7 @@ class TranscriptionServiceGuidedTest { AnnotationService annotationService; DocumentService documentService; SenderModelService senderModelService; + AuditService auditService; TranscriptionService service; UUID docId = UUID.randomUUID(); @@ -37,9 +39,10 @@ class TranscriptionServiceGuidedTest { annotationService = mock(AnnotationService.class); documentService = mock(DocumentService.class); senderModelService = mock(SenderModelService.class); + auditService = mock(AuditService.class); service = new TranscriptionService(blockRepository, versionRepository, - annotationRepository, annotationService, documentService, senderModelService); + annotationRepository, annotationService, documentService, senderModelService, auditService); when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); when(versionRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java index 7fd2aee7..4e374c41 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java @@ -2,9 +2,12 @@ package org.raddatz.familienarchiv.service; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.dto.CreateAnnotationDTO; import org.raddatz.familienarchiv.dto.CreateTranscriptionBlockDTO; import org.raddatz.familienarchiv.dto.ReorderTranscriptionBlocksDTO; @@ -21,6 +24,8 @@ import org.raddatz.familienarchiv.repository.AnnotationRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository; +import java.util.Map; + import java.util.List; import java.util.Optional; import java.util.UUID; @@ -42,6 +47,7 @@ class TranscriptionServiceTest { @Mock AnnotationService annotationService; @Mock DocumentService documentService; @Mock SenderModelService senderModelService; + @Mock AuditService auditService; @InjectMocks TranscriptionService transcriptionService; // ─── getBlock ──────────────────────────────────────────────────────────────── @@ -386,13 +392,14 @@ class TranscriptionServiceTest { void reviewBlock_setsReviewedTrue() { UUID docId = UUID.randomUUID(); UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); TranscriptionBlock block = TranscriptionBlock.builder() .id(blockId).documentId(docId).annotationId(UUID.randomUUID()) .text("corrected text").sortOrder(0).reviewed(false).build(); when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - TranscriptionBlock result = transcriptionService.reviewBlock(docId, blockId); + TranscriptionBlock result = transcriptionService.reviewBlock(docId, blockId, userId); assertThat(result.isReviewed()).isTrue(); verify(blockRepository).save(block); @@ -402,13 +409,14 @@ class TranscriptionServiceTest { void reviewBlock_togglesReviewedFalse_whenAlreadyReviewed() { UUID docId = UUID.randomUUID(); UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); TranscriptionBlock block = TranscriptionBlock.builder() .id(blockId).documentId(docId).annotationId(UUID.randomUUID()) .text("corrected text").sortOrder(0).reviewed(true).build(); when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - TranscriptionBlock result = transcriptionService.reviewBlock(docId, blockId); + TranscriptionBlock result = transcriptionService.reviewBlock(docId, blockId, userId); assertThat(result.isReviewed()).isFalse(); } @@ -419,7 +427,82 @@ class TranscriptionServiceTest { UUID blockId = UUID.randomUUID(); when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.empty()); - assertThatThrownBy(() -> transcriptionService.reviewBlock(docId, blockId)) + assertThatThrownBy(() -> transcriptionService.reviewBlock(docId, blockId, UUID.randomUUID())) .isInstanceOf(DomainException.class); } + + @Test + void reviewBlock_logsBlockReviewed_whenFlippingFalseToTrue() { + UUID docId = UUID.randomUUID(); + UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + TranscriptionBlock block = TranscriptionBlock.builder() + .id(blockId).documentId(docId).reviewed(false).build(); + when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); + when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + transcriptionService.reviewBlock(docId, blockId, userId); + + verify(auditService).log(AuditKind.BLOCK_REVIEWED, userId, docId, null); + } + + @Test + void reviewBlock_doesNotLogEvent_whenFlippingTrueToFalse() { + UUID docId = UUID.randomUUID(); + UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + TranscriptionBlock block = TranscriptionBlock.builder() + .id(blockId).documentId(docId).reviewed(true).build(); + when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); + when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + transcriptionService.reviewBlock(docId, blockId, userId); + + verify(auditService, never()).log(any(), any(), any(), any()); + } + + @Test + void updateBlock_logsTextSaved_whenTextChanges() { + UUID docId = UUID.randomUUID(); + UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + UUID annotId = UUID.randomUUID(); + TranscriptionBlock block = TranscriptionBlock.builder() + .id(blockId).documentId(docId).annotationId(annotId).text("old text").build(); + DocumentAnnotation annotation = DocumentAnnotation.builder() + .id(annotId).pageNumber(3).build(); + when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); + when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.getDocumentById(any())).thenReturn( + Document.builder().scriptType(ScriptType.TYPEWRITER).build()); + when(annotationRepository.findById(annotId)).thenReturn(Optional.of(annotation)); + + transcriptionService.updateBlock(docId, blockId, new UpdateTranscriptionBlockDTO("new text", null), userId); + + @SuppressWarnings("unchecked") + ArgumentCaptor> payloadCaptor = ArgumentCaptor.forClass(Map.class); + verify(auditService).log( + org.mockito.ArgumentMatchers.eq(AuditKind.TEXT_SAVED), + org.mockito.ArgumentMatchers.eq(userId), + org.mockito.ArgumentMatchers.eq(docId), + payloadCaptor.capture()); + assertThat(payloadCaptor.getValue()).containsEntry("pageNumber", 3); + } + + @Test + void updateBlock_doesNotLogEvent_whenTextUnchanged() { + UUID docId = UUID.randomUUID(); + UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + TranscriptionBlock block = TranscriptionBlock.builder() + .id(blockId).documentId(docId).annotationId(UUID.randomUUID()).text("same text").build(); + when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); + when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.getDocumentById(any())).thenReturn( + Document.builder().scriptType(ScriptType.TYPEWRITER).build()); + + transcriptionService.updateBlock(docId, blockId, new UpdateTranscriptionBlockDTO("same text", null), userId); + + verify(auditService, never()).log(any(), any(), any(), any()); + } } -- 2.49.1 From 2deaaf167eb7f24b895d1fbd8e5d5111f8ea3b30 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 19 Apr 2026 13:40:49 +0200 Subject: [PATCH 3/5] feat(audit): instrument DocumentService for METADATA_UPDATED, STATUS_CHANGED, FILE_UPLOADED Co-Authored-By: Claude Sonnet 4.6 --- .../service/DocumentService.java | 59 ++++++++++++- .../service/DocumentServiceTest.java | 88 +++++++++++++++++++ 2 files changed, 144 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java index b2d14d87..fb52dd7a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -3,6 +3,8 @@ package org.raddatz.familienarchiv.service; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.dto.DocumentSearchResult; import org.raddatz.familienarchiv.dto.DocumentSort; import org.raddatz.familienarchiv.dto.DocumentUpdateDTO; @@ -10,6 +12,7 @@ import org.raddatz.familienarchiv.dto.IncompleteDocumentDTO; import org.raddatz.familienarchiv.dto.MatchOffset; import org.raddatz.familienarchiv.dto.SearchMatchData; import org.raddatz.familienarchiv.dto.TagOperator; +import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentStatus; import org.raddatz.familienarchiv.model.ScriptType; @@ -20,6 +23,10 @@ import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.Specification; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.springframework.stereotype.Service; @@ -56,6 +63,8 @@ public class DocumentService { private final TagService tagService; private final DocumentVersionService documentVersionService; private final AnnotationService annotationService; + private final AuditService auditService; + private final UserService userService; public record StoreResult(Document document, boolean isNew) {} @@ -108,11 +117,17 @@ public class DocumentService { document.setFilePath(upload.s3Key()); document.setFileHash(upload.fileHash()); document.setContentType(file.getContentType()); - if (document.getStatus() == DocumentStatus.PLACEHOLDER) { + boolean wasPlaceholder = document.getStatus() == DocumentStatus.PLACEHOLDER; + if (wasPlaceholder) { document.setStatus(DocumentStatus.UPLOADED); } - return new StoreResult(documentRepository.save(document), isNew); + Document saved = documentRepository.save(document); + if (wasPlaceholder) { + UUID actorId = resolveCurrentUserId(); + logAfterCommit(AuditKind.FILE_UPLOADED, actorId, saved.getId(), null); + } + return new StoreResult(saved, isNew); } @Transactional @@ -192,6 +207,8 @@ public class DocumentService { Document doc = documentRepository.findById(id) .orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id)); + DocumentStatus statusBefore = doc.getStatus(); + // 1. Einfache Felder Update doc.setTitle(dto.getTitle()); doc.setDocumentDate(dto.getDocumentDate()); @@ -245,6 +262,15 @@ public class DocumentService { Document saved = documentRepository.save(doc); documentVersionService.recordVersion(saved); + + UUID actorId = resolveCurrentUserId(); + if (saved.getStatus() != statusBefore) { + logAfterCommit(AuditKind.STATUS_CHANGED, actorId, saved.getId(), + Map.of("oldStatus", statusBefore.name(), "newStatus", saved.getStatus().name())); + } else { + logAfterCommit(AuditKind.METADATA_UPDATED, actorId, saved.getId(), null); + } + return saved; } @@ -300,11 +326,16 @@ public class DocumentService { doc.setFileHash(upload.fileHash()); doc.setOriginalFilename(file.getOriginalFilename()); doc.setContentType(file.getContentType()); - if (doc.getStatus() == DocumentStatus.PLACEHOLDER) { + boolean wasPlaceholder = doc.getStatus() == DocumentStatus.PLACEHOLDER; + if (wasPlaceholder) { doc.setStatus(DocumentStatus.UPLOADED); } Document saved = documentRepository.save(doc); documentVersionService.recordVersion(saved); + if (wasPlaceholder) { + UUID actorId = resolveCurrentUserId(); + logAfterCommit(AuditKind.FILE_UPLOADED, actorId, saved.getId(), null); + } return saved; } @@ -725,4 +756,26 @@ public class DocumentService { throw new IllegalStateException("SHA-256 not available", e); } } + + private UUID resolveCurrentUserId() { + Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + if (auth == null || !auth.isAuthenticated()) return null; + String email = auth.getName(); + if (email == null) return null; + AppUser user = userService.findByEmail(email); + return user != null ? user.getId() : null; + } + + private void logAfterCommit(AuditKind kind, UUID actorId, UUID documentId, Map payload) { + if (TransactionSynchronizationManager.isActualTransactionActive()) { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + auditService.log(kind, actorId, documentId, payload); + } + }); + } else { + auditService.log(kind, actorId, documentId, payload); + } + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java index 69e127e2..32f7e427 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java @@ -6,6 +6,8 @@ import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.dto.DocumentSearchResult; import org.raddatz.familienarchiv.dto.DocumentSort; import org.raddatz.familienarchiv.dto.DocumentUpdateDTO; @@ -13,6 +15,7 @@ import org.raddatz.familienarchiv.dto.IncompleteDocumentDTO; import org.raddatz.familienarchiv.dto.MatchOffset; import org.raddatz.familienarchiv.dto.SearchMatchData; import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentStatus; import org.raddatz.familienarchiv.model.Person; @@ -36,6 +39,7 @@ 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.ArgumentMatchers.isNull; import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) @@ -47,6 +51,8 @@ class DocumentServiceTest { @Mock TagService tagService; @Mock DocumentVersionService documentVersionService; @Mock AnnotationService annotationService; + @Mock AuditService auditService; + @Mock UserService userService; @InjectMocks DocumentService documentService; // ─── deleteDocument ─────────────────────────────────────────────────────── @@ -1499,4 +1505,86 @@ class DocumentServiceTest { assertThatThrownBy(() -> documentService.attachFile(id, file)) .isInstanceOf(DomainException.class); } + + // ─── audit events ───────────────────────────────────────────────────────── + + @Test + void updateDocument_logsMetadataUpdated_whenNoStatusChange() throws Exception { + UUID id = UUID.randomUUID(); + Document doc = Document.builder().id(id).status(DocumentStatus.UPLOADED).build(); + when(documentRepository.findById(id)).thenReturn(Optional.of(doc)); + when(documentRepository.save(any())).thenReturn(doc); + + documentService.updateDocument(id, new DocumentUpdateDTO(), null); + + verify(auditService).log(eq(AuditKind.METADATA_UPDATED), isNull(), eq(id), isNull()); + verify(auditService, never()).log(eq(AuditKind.STATUS_CHANGED), any(), any(), any()); + } + + @Test + void updateDocument_logsStatusChanged_whenFileCausesPlaceholderToUploadedTransition() throws Exception { + UUID id = UUID.randomUUID(); + Document doc = Document.builder().id(id).status(DocumentStatus.PLACEHOLDER).build(); + when(documentRepository.findById(id)).thenReturn(Optional.of(doc)); + when(documentRepository.save(any())).thenAnswer(inv -> { + Document saved = inv.getArgument(0); + saved.setStatus(DocumentStatus.UPLOADED); + return saved; + }); + MockMultipartFile file = new MockMultipartFile("file", "doc.pdf", "application/pdf", new byte[]{1}); + when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("key", "hash")); + + documentService.updateDocument(id, new DocumentUpdateDTO(), file); + + verify(auditService).log(eq(AuditKind.STATUS_CHANGED), isNull(), eq(id), any()); + verify(auditService, never()).log(eq(AuditKind.METADATA_UPDATED), any(), any(), any()); + } + + @Test + void storeDocument_logsFileUploaded_whenExistingDocumentWasPlaceholder() throws Exception { + String filename = "test.pdf"; + Document existing = Document.builder() + .id(UUID.randomUUID()).originalFilename(filename).status(DocumentStatus.PLACEHOLDER).build(); + when(documentRepository.findFirstByOriginalFilename(filename)).thenReturn(Optional.of(existing)); + when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("s3key", "hash")); + when(documentRepository.save(any())).thenReturn(existing); + + MockMultipartFile file = new MockMultipartFile("file", filename, "application/pdf", new byte[]{1}); + documentService.storeDocument(file); + + verify(auditService).log(eq(AuditKind.FILE_UPLOADED), isNull(), eq(existing.getId()), isNull()); + } + + @Test + void storeDocument_doesNotLogFileUploaded_whenDocumentIsAlreadyUploaded() throws Exception { + String filename = "test.pdf"; + Document existing = Document.builder() + .id(UUID.randomUUID()).originalFilename(filename).status(DocumentStatus.UPLOADED).build(); + when(documentRepository.findFirstByOriginalFilename(filename)).thenReturn(Optional.of(existing)); + when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("s3key", "hash")); + when(documentRepository.save(any())).thenReturn(existing); + + MockMultipartFile file = new MockMultipartFile("file", filename, "application/pdf", new byte[]{1}); + documentService.storeDocument(file); + + verify(auditService, never()).log(any(), any(), any(), any()); + } + + @Test + void attachFile_logsFileUploaded_whenDocumentWasPlaceholder() throws Exception { + UUID id = UUID.randomUUID(); + Document doc = Document.builder().id(id).status(DocumentStatus.PLACEHOLDER).build(); + when(documentRepository.findById(id)).thenReturn(Optional.of(doc)); + when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("s3key", "hash")); + when(documentRepository.save(any())).thenAnswer(inv -> { + Document saved = inv.getArgument(0); + saved.setStatus(DocumentStatus.UPLOADED); + return saved; + }); + + MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1}); + documentService.attachFile(id, file); + + verify(auditService).log(eq(AuditKind.FILE_UPLOADED), isNull(), eq(id), isNull()); + } } -- 2.49.1 From 5a3b5ff3c757ab5a8bd8fd24f8398a47db62cb5c Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 19 Apr 2026 14:07:20 +0200 Subject: [PATCH 4/5] fix(audit): address review cycle 1 feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract logAfterCommit() from AnnotationService and TranscriptionService into AuditService, eliminating duplicate boilerplate (Markus) - Remove UserService from DocumentService; add actorId param to storeDocument(), attachFile(), updateDocument() instead — resolves SecurityContextHolder coupling concern (Markus) - Update DocumentController to inject UserService and resolve actorId from Authentication, passing it through to service methods - Add logAfterCommit() tests to AuditServiceTest with MockedStatic - Update all test verify() calls to use logAfterCommit() (not log()) Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/audit/AuditService.java | 19 +++++ .../controller/DocumentController.java | 31 +++++-- .../service/AnnotationService.java | 17 +--- .../service/DocumentService.java | 44 ++-------- .../service/TranscriptionService.java | 18 +---- .../audit/AuditServiceTest.java | 46 +++++++++++ .../controller/DocumentControllerTest.java | 19 +++-- .../service/AnnotationServiceTest.java | 4 +- .../service/DocumentServiceTest.java | 80 +++++++++---------- .../service/TranscriptionServiceTest.java | 8 +- 10 files changed, 159 insertions(+), 127 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditService.java b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditService.java index 0bd1e727..5f8c5a4e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditService.java @@ -4,6 +4,8 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Service; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; import java.util.Map; import java.util.UUID; @@ -17,6 +19,23 @@ public class AuditService { @Async("auditExecutor") public void log(AuditKind kind, UUID actorId, UUID documentId, Map payload) { + writeLog(kind, actorId, documentId, payload); + } + + public void logAfterCommit(AuditKind kind, UUID actorId, UUID documentId, Map payload) { + if (TransactionSynchronizationManager.isActualTransactionActive()) { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + writeLog(kind, actorId, documentId, payload); + } + }); + } else { + writeLog(kind, actorId, documentId, payload); + } + } + + private void writeLog(AuditKind kind, UUID actorId, UUID documentId, Map payload) { try { auditLogRepository.save(AuditLog.builder() .kind(kind) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java index ba34092b..e29e164f 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java @@ -25,12 +25,15 @@ import org.raddatz.familienarchiv.dto.DocumentSort; import org.raddatz.familienarchiv.model.DocumentStatus; import org.raddatz.familienarchiv.model.TrainingLabel; import org.raddatz.familienarchiv.model.DocumentVersion; +import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.security.Permission; import org.raddatz.familienarchiv.security.RequirePermission; import org.raddatz.familienarchiv.service.DocumentService; import org.raddatz.familienarchiv.service.DocumentVersionService; import org.raddatz.familienarchiv.service.FileService; +import org.raddatz.familienarchiv.service.UserService; import org.springframework.data.domain.Sort; +import org.springframework.security.core.Authentication; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; @@ -63,6 +66,7 @@ public class DocumentController { private final DocumentService documentService; private final DocumentVersionService documentVersionService; private final FileService fileService; + private final UserService userService; // --- DOWNLOAD --- @GetMapping("/{id}/file") @@ -112,9 +116,10 @@ public class DocumentController { public Document updateDocument( @PathVariable UUID id, @ModelAttribute DocumentUpdateDTO dto, - @RequestPart(value = "file", required = false) MultipartFile file) { + @RequestPart(value = "file", required = false) MultipartFile file, + Authentication authentication) { try { - return documentService.updateDocument(id, dto, file); + return documentService.updateDocument(id, dto, file, requireUserId(authentication)); } catch (IOException e) { throw DomainException.internal(ErrorCode.FILE_UPLOAD_FAILED, "Failed to upload file: " + e.getMessage()); } @@ -138,12 +143,13 @@ public class DocumentController { @RequirePermission(Permission.WRITE_ALL) public Document attachFile( @PathVariable UUID id, - @RequestPart("file") MultipartFile file) { + @RequestPart("file") MultipartFile file, + Authentication authentication) { String contentType = file.getContentType(); if (contentType == null || !ALLOWED_CONTENT_TYPES.contains(contentType)) { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Unsupported file type: " + contentType); } - return documentService.attachFile(id, file); + return documentService.attachFile(id, file, requireUserId(authentication)); } // --- QUICK UPLOAD --- @@ -154,7 +160,8 @@ public class DocumentController { @PostMapping(value = "/quick-upload", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) @RequirePermission(Permission.WRITE_ALL) public QuickUploadResult quickUpload( - @RequestPart(value = "files", required = false) List files) { + @RequestPart(value = "files", required = false) List files, + Authentication authentication) { List created = new ArrayList<>(); List updated = new ArrayList<>(); List errors = new ArrayList<>(); @@ -163,13 +170,14 @@ public class DocumentController { return new QuickUploadResult(created, updated, errors); } + UUID actorId = requireUserId(authentication); for (MultipartFile file : files) { if (!ALLOWED_CONTENT_TYPES.contains(file.getContentType())) { errors.add(new UploadError(file.getOriginalFilename(), "UNSUPPORTED_FILE_TYPE")); continue; } try { - DocumentService.StoreResult result = documentService.storeDocument(file); + DocumentService.StoreResult result = documentService.storeDocument(file, actorId); if (result.isNew()) { created.add(result.document()); } else { @@ -276,4 +284,15 @@ public class DocumentController { Sort sort = Sort.by(Sort.Direction.fromString(dir.toUpperCase()), "documentDate"); return documentService.getConversationFiltered(senderId, receiverId, from, to, sort); } + + private UUID requireUserId(Authentication authentication) { + if (authentication == null || !authentication.isAuthenticated()) { + throw DomainException.unauthorized("Authentication required"); + } + AppUser user = userService.findByEmail(authentication.getName()); + if (user == null) { + throw DomainException.unauthorized("User not found"); + } + return user.getId(); + } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java index ff3f9cb1..1c6da350 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java @@ -14,8 +14,6 @@ import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import org.springframework.transaction.support.TransactionSynchronization; -import org.springframework.transaction.support.TransactionSynchronizationManager; import java.util.List; import java.util.Map; @@ -49,7 +47,7 @@ public class AnnotationService { .build(); DocumentAnnotation saved = annotationRepository.save(annotation); - logAfterCommit(AuditKind.ANNOTATION_CREATED, userId, saved.getDocumentId(), + auditService.logAfterCommit(AuditKind.ANNOTATION_CREATED, userId, saved.getDocumentId(), Map.of("pageNumber", saved.getPageNumber())); return saved; } @@ -117,17 +115,4 @@ public class AnnotationService { }); } - private void logAfterCommit(AuditKind kind, UUID actorId, UUID documentId, Map payload) { - if (TransactionSynchronizationManager.isActualTransactionActive()) { - TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { - @Override - public void afterCommit() { - auditService.log(kind, actorId, documentId, payload); - } - }); - } else { - auditService.log(kind, actorId, documentId, payload); - } - } - } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java index fb52dd7a..0007fd5d 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -12,7 +12,6 @@ import org.raddatz.familienarchiv.dto.IncompleteDocumentDTO; import org.raddatz.familienarchiv.dto.MatchOffset; import org.raddatz.familienarchiv.dto.SearchMatchData; import org.raddatz.familienarchiv.dto.TagOperator; -import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentStatus; import org.raddatz.familienarchiv.model.ScriptType; @@ -23,10 +22,6 @@ import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.Specification; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.transaction.support.TransactionSynchronization; -import org.springframework.transaction.support.TransactionSynchronizationManager; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.springframework.stereotype.Service; @@ -64,7 +59,6 @@ public class DocumentService { private final DocumentVersionService documentVersionService; private final AnnotationService annotationService; private final AuditService auditService; - private final UserService userService; public record StoreResult(Document document, boolean isNew) {} @@ -84,7 +78,7 @@ public class DocumentService { * - Wenn NEIN: Erstellt neuen Eintrag — isNew = true. */ @Transactional - public StoreResult storeDocument(MultipartFile file) throws IOException { + public StoreResult storeDocument(MultipartFile file, UUID actorId) throws IOException { String originalFilename = file.getOriginalFilename(); // 1. Check for existing record (findFirst to survive duplicate filenames in the DB) @@ -124,8 +118,7 @@ public class DocumentService { Document saved = documentRepository.save(document); if (wasPlaceholder) { - UUID actorId = resolveCurrentUserId(); - logAfterCommit(AuditKind.FILE_UPLOADED, actorId, saved.getId(), null); + auditService.logAfterCommit(AuditKind.FILE_UPLOADED, actorId, saved.getId(), null); } return new StoreResult(saved, isNew); } @@ -203,7 +196,7 @@ public class DocumentService { } @Transactional - public Document updateDocument(UUID id, DocumentUpdateDTO dto, MultipartFile newFile) throws IOException { + public Document updateDocument(UUID id, DocumentUpdateDTO dto, MultipartFile newFile, UUID actorId) throws IOException { Document doc = documentRepository.findById(id) .orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id)); @@ -263,12 +256,11 @@ public class DocumentService { Document saved = documentRepository.save(doc); documentVersionService.recordVersion(saved); - UUID actorId = resolveCurrentUserId(); if (saved.getStatus() != statusBefore) { - logAfterCommit(AuditKind.STATUS_CHANGED, actorId, saved.getId(), + auditService.logAfterCommit(AuditKind.STATUS_CHANGED, actorId, saved.getId(), Map.of("oldStatus", statusBefore.name(), "newStatus", saved.getStatus().name())); } else { - logAfterCommit(AuditKind.METADATA_UPDATED, actorId, saved.getId(), null); + auditService.logAfterCommit(AuditKind.METADATA_UPDATED, actorId, saved.getId(), null); } return saved; @@ -313,7 +305,7 @@ public class DocumentService { } @Transactional - public Document attachFile(UUID id, MultipartFile file) { + public Document attachFile(UUID id, MultipartFile file, UUID actorId) { Document doc = documentRepository.findById(id) .orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id)); FileService.UploadResult upload; @@ -333,8 +325,7 @@ public class DocumentService { Document saved = documentRepository.save(doc); documentVersionService.recordVersion(saved); if (wasPlaceholder) { - UUID actorId = resolveCurrentUserId(); - logAfterCommit(AuditKind.FILE_UPLOADED, actorId, saved.getId(), null); + auditService.logAfterCommit(AuditKind.FILE_UPLOADED, actorId, saved.getId(), null); } return saved; } @@ -757,25 +748,4 @@ public class DocumentService { } } - private UUID resolveCurrentUserId() { - Authentication auth = SecurityContextHolder.getContext().getAuthentication(); - if (auth == null || !auth.isAuthenticated()) return null; - String email = auth.getName(); - if (email == null) return null; - AppUser user = userService.findByEmail(email); - return user != null ? user.getId() : null; - } - - private void logAfterCommit(AuditKind kind, UUID actorId, UUID documentId, Map payload) { - if (TransactionSynchronizationManager.isActualTransactionActive()) { - TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { - @Override - public void afterCommit() { - auditService.log(kind, actorId, documentId, payload); - } - }); - } else { - auditService.log(kind, actorId, documentId, payload); - } - } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java index 4ec7cf94..c52cf103 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java @@ -21,8 +21,6 @@ import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import org.springframework.transaction.support.TransactionSynchronization; -import org.springframework.transaction.support.TransactionSynchronizationManager; import java.util.List; import java.util.Map; @@ -144,7 +142,7 @@ public class TranscriptionService { if (!text.equals(previousText)) { Optional annotation = annotationRepository.findById(block.getAnnotationId()); int pageNumber = annotation.map(DocumentAnnotation::getPageNumber).orElse(0); - logAfterCommit(AuditKind.TEXT_SAVED, userId, documentId, Map.of("pageNumber", pageNumber)); + auditService.logAfterCommit(AuditKind.TEXT_SAVED, userId, documentId, Map.of("pageNumber", pageNumber)); } Document doc = documentService.getDocumentById(documentId); @@ -201,7 +199,7 @@ public class TranscriptionService { block.setReviewed(!wasReviewed); TranscriptionBlock saved = blockRepository.save(block); if (!wasReviewed && saved.isReviewed()) { - logAfterCommit(AuditKind.BLOCK_REVIEWED, userId, documentId, null); + auditService.logAfterCommit(AuditKind.BLOCK_REVIEWED, userId, documentId, null); } return saved; } @@ -228,16 +226,4 @@ public class TranscriptionService { return text; } - private void logAfterCommit(AuditKind kind, UUID actorId, UUID documentId, Map payload) { - if (TransactionSynchronizationManager.isActualTransactionActive()) { - TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { - @Override - public void afterCommit() { - auditService.log(kind, actorId, documentId, payload); - } - }); - } else { - auditService.log(kind, actorId, documentId, payload); - } - } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/audit/AuditServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/audit/AuditServiceTest.java index f8c85653..205f07eb 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/audit/AuditServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/audit/AuditServiceTest.java @@ -5,8 +5,13 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.UUID; @@ -71,4 +76,45 @@ class AuditServiceTest { verify(auditLogRepository).save(captor.capture()); assertThat(captor.getValue().getActorId()).isNull(); } + + // ─── logAfterCommit ─────────────────────────────────────────────────────── + + @Test + void logAfterCommit_savesDirectly_whenNoTransactionIsActive() { + when(auditLogRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + try (MockedStatic mocked = + mockStatic(TransactionSynchronizationManager.class)) { + mocked.when(TransactionSynchronizationManager::isActualTransactionActive).thenReturn(false); + + auditService.logAfterCommit(AuditKind.METADATA_UPDATED, null, null, null); + + verify(auditLogRepository).save(any()); + } + } + + @Test + void logAfterCommit_registersCallback_andSavesOnlyAfterCommit_whenTransactionIsActive() { + when(auditLogRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + try (MockedStatic mocked = + mockStatic(TransactionSynchronizationManager.class)) { + mocked.when(TransactionSynchronizationManager::isActualTransactionActive).thenReturn(true); + List captured = new ArrayList<>(); + mocked.when(() -> TransactionSynchronizationManager.registerSynchronization(any())) + .thenAnswer(inv -> { captured.add(inv.getArgument(0)); return null; }); + + auditService.logAfterCommit(AuditKind.TEXT_SAVED, null, null, null); + + // Callback registered but repo not yet called + assertThat(captured).hasSize(1); + verify(auditLogRepository, never()).save(any()); + + // Simulate transaction commit + captured.get(0).afterCommit(); + + // Now the row should be saved + verify(auditLogRepository).save(any()); + } + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java index 8bb86ed0..36b86449 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java @@ -10,10 +10,12 @@ import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentStatus; import org.raddatz.familienarchiv.model.DocumentVersion; import org.raddatz.familienarchiv.security.PermissionAspect; +import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.service.CustomUserDetailsService; import org.raddatz.familienarchiv.service.DocumentService; import org.raddatz.familienarchiv.service.DocumentVersionService; import org.raddatz.familienarchiv.service.FileService; +import org.raddatz.familienarchiv.service.UserService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; import org.raddatz.familienarchiv.config.SecurityConfig; @@ -51,6 +53,7 @@ class DocumentControllerTest { @MockitoBean DocumentService documentService; @MockitoBean DocumentVersionService documentVersionService; @MockitoBean FileService fileService; + @MockitoBean UserService userService; @MockitoBean CustomUserDetailsService customUserDetailsService; // ─── GET /api/documents/search ──────────────────────────────────────────── @@ -193,7 +196,8 @@ class DocumentControllerTest { .title("Updated") .originalFilename("test.pdf") .build(); - when(documentService.updateDocument(any(), any(), any())).thenReturn(doc); + when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); + when(documentService.updateDocument(any(), any(), any(), any())).thenReturn(doc); mockMvc.perform(multipart("/api/documents/" + id) .with(req -> { req.setMethod("PUT"); return req; })) @@ -246,7 +250,8 @@ class DocumentControllerTest { void quickUpload_returns200_withValidPdfFile() throws Exception { Document doc = Document.builder() .id(UUID.randomUUID()).title("scan001").originalFilename("scan001.pdf").build(); - when(documentService.storeDocument(any())) + when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); + when(documentService.storeDocument(any(), any())) .thenReturn(new DocumentService.StoreResult(doc, true)); org.springframework.mock.web.MockMultipartFile file = @@ -264,7 +269,8 @@ class DocumentControllerTest { void quickUpload_placesDocumentInUpdated_whenFilenameAlreadyExists() throws Exception { Document existing = Document.builder() .id(UUID.randomUUID()).title("Alter Brief").originalFilename("scan001.pdf").build(); - when(documentService.storeDocument(any())) + when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); + when(documentService.storeDocument(any(), any())) .thenReturn(new DocumentService.StoreResult(existing, false)); org.springframework.mock.web.MockMultipartFile file = @@ -280,6 +286,7 @@ class DocumentControllerTest { @Test @WithMockUser(authorities = "WRITE_ALL") void quickUpload_skipsUnsupportedFileType_andReturnsError() throws Exception { + when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); org.springframework.mock.web.MockMultipartFile file = new org.springframework.mock.web.MockMultipartFile("files", "report.docx", "application/vnd.openxmlformats-officedocument.wordprocessingml.document", new byte[]{1}); @@ -584,7 +591,8 @@ class DocumentControllerTest { Document doc = Document.builder() .id(id).title("Brief").originalFilename("brief.pdf") .filePath("docs/brief.pdf").status(DocumentStatus.UPLOADED).build(); - when(documentService.attachFile(eq(id), any())).thenReturn(doc); + when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); + when(documentService.attachFile(eq(id), any(), any())).thenReturn(doc); org.springframework.mock.web.MockMultipartFile file = new org.springframework.mock.web.MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1}); @@ -610,7 +618,8 @@ class DocumentControllerTest { @WithMockUser(authorities = "WRITE_ALL") void attachFile_returns404_whenDocumentDoesNotExist() throws Exception { UUID id = UUID.randomUUID(); - when(documentService.attachFile(eq(id), any())) + when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); + when(documentService.attachFile(eq(id), any(), any())) .thenThrow(DomainException.notFound( ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id)); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java index 0ac0118d..19277bec 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java @@ -109,7 +109,7 @@ class AnnotationServiceTest { @SuppressWarnings("unchecked") ArgumentCaptor> payloadCaptor = ArgumentCaptor.forClass(Map.class); - verify(auditService).log( + verify(auditService).logAfterCommit( org.mockito.ArgumentMatchers.eq(AuditKind.ANNOTATION_CREATED), org.mockito.ArgumentMatchers.eq(userId), org.mockito.ArgumentMatchers.eq(docId), @@ -126,7 +126,7 @@ class AnnotationServiceTest { annotationService.createOcrAnnotation(docId, dto, userId, null, null); - verify(auditService, never()).log(any(), any(), any(), any()); + verify(auditService, never()).logAfterCommit(any(), any(), any(), any()); } // ─── createOcrAnnotation ────────────────────────────────────────────────── diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java index 32f7e427..dc23df1a 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java @@ -15,7 +15,6 @@ import org.raddatz.familienarchiv.dto.IncompleteDocumentDTO; import org.raddatz.familienarchiv.dto.MatchOffset; import org.raddatz.familienarchiv.dto.SearchMatchData; import org.raddatz.familienarchiv.exception.DomainException; -import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentStatus; import org.raddatz.familienarchiv.model.Person; @@ -52,7 +51,6 @@ class DocumentServiceTest { @Mock DocumentVersionService documentVersionService; @Mock AnnotationService annotationService; @Mock AuditService auditService; - @Mock UserService userService; @InjectMocks DocumentService documentService; // ─── deleteDocument ─────────────────────────────────────────────────────── @@ -106,7 +104,7 @@ class DocumentServiceTest { UUID id = UUID.randomUUID(); when(documentRepository.findById(id)).thenReturn(Optional.empty()); - assertThatThrownBy(() -> documentService.updateDocument(id, new DocumentUpdateDTO(), null)) + assertThatThrownBy(() -> documentService.updateDocument(id, new DocumentUpdateDTO(), null, null)) .isInstanceOf(DomainException.class); } @@ -217,7 +215,7 @@ class DocumentServiceTest { when(fileService.uploadFile(any(), any())).thenReturn(uploadResult); when(documentRepository.save(any())).thenReturn(existing); - documentService.updateDocument(id, new DocumentUpdateDTO(), newFile); + documentService.updateDocument(id, new DocumentUpdateDTO(), newFile, null); assertThat(existing.getFileHash()).isEqualTo("cafebabe"); } @@ -250,7 +248,7 @@ class DocumentServiceTest { when(documentRepository.findById(id)).thenReturn(Optional.of(existing)); when(documentRepository.save(any())).thenReturn(existing); - documentService.updateDocument(id, new DocumentUpdateDTO(), null); + documentService.updateDocument(id, new DocumentUpdateDTO(), null, null); verify(documentVersionService).recordVersion(any(Document.class)); } @@ -269,7 +267,7 @@ class DocumentServiceTest { when(fileService.uploadFile(any(), any())).thenReturn(uploadResult); org.mockito.ArgumentCaptor captor = org.mockito.ArgumentCaptor.forClass(Document.class); - documentService.storeDocument(file); + documentService.storeDocument(file, null); verify(documentRepository).save(captor.capture()); assertThat(captor.getValue().getTitle()).isEqualTo("scan001"); @@ -288,7 +286,7 @@ class DocumentServiceTest { when(documentRepository.save(any())).thenReturn(placeholder); when(fileService.uploadFile(any(), any())).thenReturn(uploadResult); - documentService.storeDocument(file); + documentService.storeDocument(file, null); assertThat(placeholder.getTitle()).isEqualTo("Brief an Oma"); } @@ -303,7 +301,7 @@ class DocumentServiceTest { when(documentRepository.save(any())).thenReturn(saved); when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("documents/new.pdf", "hash")); - DocumentService.StoreResult result = documentService.storeDocument(file); + DocumentService.StoreResult result = documentService.storeDocument(file, null); assertThat(result.isNew()).isTrue(); } @@ -319,7 +317,7 @@ class DocumentServiceTest { when(documentRepository.save(any())).thenReturn(existing); when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("documents/existing.pdf", "hash")); - DocumentService.StoreResult result = documentService.storeDocument(file); + DocumentService.StoreResult result = documentService.storeDocument(file, null); assertThat(result.isNew()).isFalse(); } @@ -433,7 +431,7 @@ class DocumentServiceTest { when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("path", "hash")); ArgumentCaptor captor = ArgumentCaptor.forClass(Document.class); - documentService.storeDocument(file); + documentService.storeDocument(file, null); verify(documentRepository).save(captor.capture()); assertThat(captor.getValue().isMetadataComplete()).isFalse(); @@ -448,7 +446,7 @@ class DocumentServiceTest { when(documentRepository.save(any())).thenReturn(existing); when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("path", "hash")); - documentService.storeDocument(file); + documentService.storeDocument(file, null); assertThat(existing.isMetadataComplete()).isTrue(); } @@ -462,7 +460,7 @@ class DocumentServiceTest { when(personService.findByName(any(), any())).thenReturn(Optional.empty()); ArgumentCaptor captor = ArgumentCaptor.forClass(Document.class); - documentService.storeDocument(file); + documentService.storeDocument(file, null); verify(documentRepository).save(captor.capture()); assertThat(captor.getValue().getDocumentDate()).isEqualTo(java.time.LocalDate.of(1965, 3, 12)); @@ -479,7 +477,7 @@ class DocumentServiceTest { when(personService.findByName("Walter", "de Gruyter")).thenReturn(Optional.of(walter)); ArgumentCaptor captor = ArgumentCaptor.forClass(Document.class); - documentService.storeDocument(file); + documentService.storeDocument(file, null); verify(documentRepository).save(captor.capture()); assertThat(captor.getValue().getSender()).isEqualTo(walter); @@ -494,7 +492,7 @@ class DocumentServiceTest { when(personService.findByName(any(), any())).thenReturn(Optional.empty()); ArgumentCaptor captor = ArgumentCaptor.forClass(Document.class); - documentService.storeDocument(file); + documentService.storeDocument(file, null); verify(documentRepository).save(captor.capture()); assertThat(captor.getValue().getSender()).isNull(); @@ -621,7 +619,7 @@ class DocumentServiceTest { DocumentUpdateDTO dto = new DocumentUpdateDTO(); dto.setMetadataComplete(true); - documentService.updateDocument(id, dto, null); + documentService.updateDocument(id, dto, null, null); assertThat(existing.isMetadataComplete()).isTrue(); } @@ -636,7 +634,7 @@ class DocumentServiceTest { DocumentUpdateDTO dto = new DocumentUpdateDTO(); // metadataComplete not set → null - documentService.updateDocument(id, dto, null); + documentService.updateDocument(id, dto, null, null); assertThat(existing.isMetadataComplete()).isFalse(); } @@ -1004,7 +1002,7 @@ class DocumentServiceTest { dto.setTitle("T"); dto.setTags(" "); // not null but blank - documentService.updateDocument(id, dto, null); + documentService.updateDocument(id, dto, null, null); verify(tagService, never()).findOrCreate(any()); } @@ -1021,7 +1019,7 @@ class DocumentServiceTest { dto.setTitle("T"); dto.setReceiverIds(List.of()); // not null but empty → else → clear - documentService.updateDocument(id, dto, null); + documentService.updateDocument(id, dto, null, null); assertThat(doc.getReceivers()).isEmpty(); } @@ -1037,7 +1035,7 @@ class DocumentServiceTest { DocumentUpdateDTO dto = new DocumentUpdateDTO(); dto.setTitle("T"); - documentService.updateDocument(id, dto, emptyFile); + documentService.updateDocument(id, dto, emptyFile, null); verify(fileService, never()).uploadFile(any(), any()); } @@ -1068,7 +1066,7 @@ class DocumentServiceTest { dto.setTitle("T"); dto.setTags("Familie, ,Reise"); // blank middle segment filtered - documentService.updateDocument(id, dto, null); + documentService.updateDocument(id, dto, null, null); verify(tagService).findOrCreate("Familie"); verify(tagService).findOrCreate("Reise"); @@ -1116,7 +1114,7 @@ class DocumentServiceTest { dto.setTitle("T"); // senderId is null — should clear sender - documentService.updateDocument(id, dto, null); + documentService.updateDocument(id, dto, null, null); verify(documentRepository, atLeastOnce()).save(argThat(d -> d.getSender() == null)); } @@ -1136,7 +1134,7 @@ class DocumentServiceTest { dto.setTitle("T"); dto.setReceiverIds(List.of(r1Id)); - documentService.updateDocument(id, dto, null); + documentService.updateDocument(id, dto, null, null); verify(personService).getAllById(List.of(r1Id)); } @@ -1155,7 +1153,7 @@ class DocumentServiceTest { dto.setTitle("T"); dto.setTags("Reise"); - documentService.updateDocument(id, dto, null); + documentService.updateDocument(id, dto, null, null); verify(tagService).findOrCreate("Reise"); } @@ -1175,7 +1173,7 @@ class DocumentServiceTest { dto.setTitle("T"); dto.setSenderId(senderId); - documentService.updateDocument(id, dto, null); + documentService.updateDocument(id, dto, null, null); verify(personService).getById(senderId); assertThat(doc.getSender()).isEqualTo(sender); @@ -1443,7 +1441,7 @@ class DocumentServiceTest { when(documentRepository.findById(id)).thenReturn(Optional.empty()); MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1}); - assertThatThrownBy(() -> documentService.attachFile(id, file)) + assertThatThrownBy(() -> documentService.attachFile(id, file, null)) .isInstanceOf(DomainException.class) .hasMessageContaining(id.toString()); } @@ -1457,7 +1455,7 @@ class DocumentServiceTest { when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1}); - Document result = documentService.attachFile(id, file); + Document result = documentService.attachFile(id, file, null); assertThat(result.getStatus()).isEqualTo(DocumentStatus.UPLOADED); } @@ -1471,7 +1469,7 @@ class DocumentServiceTest { when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1}); - Document result = documentService.attachFile(id, file); + Document result = documentService.attachFile(id, file, null); assertThat(result.getStatus()).isEqualTo(DocumentStatus.UPLOADED); } @@ -1486,7 +1484,7 @@ class DocumentServiceTest { when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1}); - Document result = documentService.attachFile(id, file); + Document result = documentService.attachFile(id, file, null); assertThat(result.getFilePath()).isEqualTo("s3/brief.pdf"); assertThat(result.getFileHash()).isEqualTo("deadbeef"); @@ -1502,7 +1500,7 @@ class DocumentServiceTest { when(fileService.uploadFile(any(), any())).thenThrow(new java.io.IOException("storage unavailable")); MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1}); - assertThatThrownBy(() -> documentService.attachFile(id, file)) + assertThatThrownBy(() -> documentService.attachFile(id, file, null)) .isInstanceOf(DomainException.class); } @@ -1515,10 +1513,10 @@ class DocumentServiceTest { when(documentRepository.findById(id)).thenReturn(Optional.of(doc)); when(documentRepository.save(any())).thenReturn(doc); - documentService.updateDocument(id, new DocumentUpdateDTO(), null); + documentService.updateDocument(id, new DocumentUpdateDTO(), null, null); - verify(auditService).log(eq(AuditKind.METADATA_UPDATED), isNull(), eq(id), isNull()); - verify(auditService, never()).log(eq(AuditKind.STATUS_CHANGED), any(), any(), any()); + verify(auditService).logAfterCommit(eq(AuditKind.METADATA_UPDATED), isNull(), eq(id), isNull()); + verify(auditService, never()).logAfterCommit(eq(AuditKind.STATUS_CHANGED), any(), any(), any()); } @Test @@ -1534,10 +1532,10 @@ class DocumentServiceTest { MockMultipartFile file = new MockMultipartFile("file", "doc.pdf", "application/pdf", new byte[]{1}); when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("key", "hash")); - documentService.updateDocument(id, new DocumentUpdateDTO(), file); + documentService.updateDocument(id, new DocumentUpdateDTO(), file, null); - verify(auditService).log(eq(AuditKind.STATUS_CHANGED), isNull(), eq(id), any()); - verify(auditService, never()).log(eq(AuditKind.METADATA_UPDATED), any(), any(), any()); + verify(auditService).logAfterCommit(eq(AuditKind.STATUS_CHANGED), isNull(), eq(id), any()); + verify(auditService, never()).logAfterCommit(eq(AuditKind.METADATA_UPDATED), any(), any(), any()); } @Test @@ -1550,9 +1548,9 @@ class DocumentServiceTest { when(documentRepository.save(any())).thenReturn(existing); MockMultipartFile file = new MockMultipartFile("file", filename, "application/pdf", new byte[]{1}); - documentService.storeDocument(file); + documentService.storeDocument(file, null); - verify(auditService).log(eq(AuditKind.FILE_UPLOADED), isNull(), eq(existing.getId()), isNull()); + verify(auditService).logAfterCommit(eq(AuditKind.FILE_UPLOADED), isNull(), eq(existing.getId()), isNull()); } @Test @@ -1565,9 +1563,9 @@ class DocumentServiceTest { when(documentRepository.save(any())).thenReturn(existing); MockMultipartFile file = new MockMultipartFile("file", filename, "application/pdf", new byte[]{1}); - documentService.storeDocument(file); + documentService.storeDocument(file, null); - verify(auditService, never()).log(any(), any(), any(), any()); + verify(auditService, never()).logAfterCommit(any(), any(), any(), any()); } @Test @@ -1583,8 +1581,8 @@ class DocumentServiceTest { }); MockMultipartFile file = new MockMultipartFile("file", "brief.pdf", "application/pdf", new byte[]{1}); - documentService.attachFile(id, file); + documentService.attachFile(id, file, null); - verify(auditService).log(eq(AuditKind.FILE_UPLOADED), isNull(), eq(id), isNull()); + verify(auditService).logAfterCommit(eq(AuditKind.FILE_UPLOADED), isNull(), eq(id), isNull()); } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java index 4e374c41..e9d415a8 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java @@ -443,7 +443,7 @@ class TranscriptionServiceTest { transcriptionService.reviewBlock(docId, blockId, userId); - verify(auditService).log(AuditKind.BLOCK_REVIEWED, userId, docId, null); + verify(auditService).logAfterCommit(AuditKind.BLOCK_REVIEWED, userId, docId, null); } @Test @@ -458,7 +458,7 @@ class TranscriptionServiceTest { transcriptionService.reviewBlock(docId, blockId, userId); - verify(auditService, never()).log(any(), any(), any(), any()); + verify(auditService, never()).logAfterCommit(any(), any(), any(), any()); } @Test @@ -481,7 +481,7 @@ class TranscriptionServiceTest { @SuppressWarnings("unchecked") ArgumentCaptor> payloadCaptor = ArgumentCaptor.forClass(Map.class); - verify(auditService).log( + verify(auditService).logAfterCommit( org.mockito.ArgumentMatchers.eq(AuditKind.TEXT_SAVED), org.mockito.ArgumentMatchers.eq(userId), org.mockito.ArgumentMatchers.eq(docId), @@ -503,6 +503,6 @@ class TranscriptionServiceTest { transcriptionService.updateBlock(docId, blockId, new UpdateTranscriptionBlockDTO("same text", null), userId); - verify(auditService, never()).log(any(), any(), any(), any()); + verify(auditService, never()).logAfterCommit(any(), any(), any(), any()); } } -- 2.49.1 From 428c63a2f2d0aa513a1d57d2ae501b20d163e988 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 19 Apr 2026 15:43:51 +0200 Subject: [PATCH 5/5] feat(audit): add COMMENT_ADDED and MENTION_CREATED audit events Instruments CommentService.postComment(), postBlockComment(), and replyToComment() to fire COMMENT_ADDED after each successful save and MENTION_CREATED once per mentioned user. The shared logCommentPosted() helper avoids duplicating the two-call pattern across all three post methods. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/audit/AuditKind.java | 6 + .../service/CommentService.java | 18 +++ .../service/CommentServiceTest.java | 133 ++++++++++++++++++ 3 files changed, 157 insertions(+) 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 cac1ef48..d44513b5 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java @@ -19,4 +19,10 @@ public enum AuditKind { /** Payload: {@code {"pageNumber": 3}} */ ANNOTATION_CREATED, + + /** Payload: {@code {"commentId": "uuid"}} */ + COMMENT_ADDED, + + /** Payload: {@code {"commentId": "uuid", "mentionedUserId": "uuid"}} */ + MENTION_CREATED, } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/CommentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/CommentService.java index f80955e2..e6157509 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/CommentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/CommentService.java @@ -1,6 +1,8 @@ package org.raddatz.familienarchiv.service; import lombok.RequiredArgsConstructor; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.dto.MentionDTO; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; @@ -12,6 +14,7 @@ import org.springframework.transaction.annotation.Transactional; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.UUID; @@ -22,6 +25,7 @@ public class CommentService { private final CommentRepository commentRepository; private final UserService userService; private final NotificationService notificationService; + private final AuditService auditService; public List getCommentsForDocument(UUID documentId) { List roots = @@ -53,6 +57,7 @@ public class CommentService { DocumentComment saved = commentRepository.save(comment); withMentionDTOs(saved); notificationService.notifyMentions(mentionedUserIds, saved); + logCommentPosted(author, documentId, saved, mentionedUserIds); return saved; } @@ -70,6 +75,7 @@ public class CommentService { DocumentComment saved = commentRepository.save(comment); withMentionDTOs(saved); notificationService.notifyMentions(mentionedUserIds, saved); + logCommentPosted(author, documentId, saved, mentionedUserIds); return saved; } @@ -101,6 +107,7 @@ public class CommentService { participantIds.remove(author.getId()); notificationService.notifyReply(saved, participantIds); notificationService.notifyMentions(mentionedUserIds, saved); + logCommentPosted(author, documentId, saved, mentionedUserIds); return saved; } @@ -171,6 +178,17 @@ public class CommentService { ErrorCode.COMMENT_NOT_FOUND, "Comment not found: " + commentId)); } + private void logCommentPosted(AppUser author, UUID documentId, DocumentComment saved, List mentionedUserIds) { + UUID actorId = author != null ? author.getId() : null; + String commentId = saved.getId().toString(); + auditService.logAfterCommit(AuditKind.COMMENT_ADDED, actorId, documentId, Map.of("commentId", commentId)); + if (mentionedUserIds != null) { + mentionedUserIds.forEach(mentionedUserId -> + auditService.logAfterCommit(AuditKind.MENTION_CREATED, actorId, documentId, + Map.of("commentId", commentId, "mentionedUserId", mentionedUserId.toString()))); + } + } + private String resolveAuthorName(AppUser author) { String first = author.getFirstName(); String last = author.getLastName(); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/CommentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/CommentServiceTest.java index 6a846f03..a6596d80 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/CommentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/CommentServiceTest.java @@ -5,6 +5,8 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.DocumentComment; @@ -22,6 +24,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anySet; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -35,6 +38,7 @@ class CommentServiceTest { @Mock CommentRepository commentRepository; @Mock UserService userService; @Mock NotificationService notificationService; + @Mock AuditService auditService; @InjectMocks CommentService commentService; // ─── postComment ────────────────────────────────────────────────────────── @@ -489,6 +493,135 @@ class CommentServiceTest { .build(); } + // ─── audit: COMMENT_ADDED and MENTION_CREATED ───────────────────────────── + + @Test + void postComment_logsCommentAdded() { + UUID docId = UUID.randomUUID(); + UUID savedId = UUID.randomUUID(); + AppUser author = AppUser.builder().id(UUID.randomUUID()).email("hans@example.com").firstName("Hans").lastName("M").build(); + DocumentComment saved = DocumentComment.builder() + .id(savedId).documentId(docId).authorName("Hans M").content("Hello").build(); + when(commentRepository.save(any())).thenReturn(saved); + + commentService.postComment(docId, null, "Hello", List.of(), author); + + verify(auditService).logAfterCommit( + eq(AuditKind.COMMENT_ADDED), + eq(author.getId()), + eq(docId), + argThat(p -> savedId.toString().equals(p.get("commentId")))); + } + + @Test + void postComment_logsMentionCreated_oncePerMentionedUser() { + UUID docId = UUID.randomUUID(); + UUID savedId = UUID.randomUUID(); + UUID mentionedId1 = UUID.randomUUID(); + UUID mentionedId2 = UUID.randomUUID(); + AppUser author = AppUser.builder().id(UUID.randomUUID()).email("hans@example.com").firstName("Hans").lastName("M").build(); + AppUser mentioned1 = AppUser.builder().id(mentionedId1).email("anna@example.com").firstName("Anna").lastName("S").build(); + AppUser mentioned2 = AppUser.builder().id(mentionedId2).email("bob@example.com").firstName("Bob").lastName("J").build(); + DocumentComment saved = DocumentComment.builder() + .id(savedId).documentId(docId).authorName("Hans M").content("Hey @Anna @Bob").build(); + when(userService.findAllById(List.of(mentionedId1, mentionedId2))).thenReturn(List.of(mentioned1, mentioned2)); + when(commentRepository.save(any())).thenReturn(saved); + + commentService.postComment(docId, null, "Hey @Anna @Bob", List.of(mentionedId1, mentionedId2), author); + + verify(auditService).logAfterCommit( + eq(AuditKind.MENTION_CREATED), + eq(author.getId()), + eq(docId), + argThat(p -> mentionedId1.toString().equals(p.get("mentionedUserId")))); + verify(auditService).logAfterCommit( + eq(AuditKind.MENTION_CREATED), + eq(author.getId()), + eq(docId), + argThat(p -> mentionedId2.toString().equals(p.get("mentionedUserId")))); + } + + @Test + void postComment_doesNotLogMentionCreated_whenNoMentions() { + UUID docId = UUID.randomUUID(); + AppUser author = AppUser.builder().id(UUID.randomUUID()).email("hans@example.com").firstName("Hans").lastName("M").build(); + DocumentComment saved = DocumentComment.builder() + .id(UUID.randomUUID()).documentId(docId).authorName("Hans M").content("Hello").build(); + when(commentRepository.save(any())).thenReturn(saved); + + commentService.postComment(docId, null, "Hello", List.of(), author); + + verify(auditService, never()).logAfterCommit(eq(AuditKind.MENTION_CREATED), any(), any(), any()); + } + + @Test + void replyToComment_logsCommentAdded() { + UUID docId = UUID.randomUUID(); + UUID rootId = UUID.randomUUID(); + UUID savedId = UUID.randomUUID(); + AppUser author = AppUser.builder().id(UUID.randomUUID()).email("anna@example.com").firstName("Anna").lastName("S").build(); + DocumentComment root = DocumentComment.builder() + .id(rootId).documentId(docId).parentId(null).content("Root").authorName("Hans").build(); + DocumentComment saved = DocumentComment.builder() + .id(savedId).documentId(docId).parentId(rootId).content("Reply").authorName("Anna S").build(); + when(commentRepository.findById(rootId)).thenReturn(Optional.of(root)); + when(commentRepository.findByParentId(rootId)).thenReturn(List.of()); + when(commentRepository.save(any())).thenReturn(saved); + + commentService.replyToComment(docId, rootId, "Reply", List.of(), author); + + verify(auditService).logAfterCommit( + eq(AuditKind.COMMENT_ADDED), + eq(author.getId()), + eq(docId), + argThat(p -> savedId.toString().equals(p.get("commentId")))); + } + + @Test + void replyToComment_logsMentionCreated_whenMentioned() { + UUID docId = UUID.randomUUID(); + UUID rootId = UUID.randomUUID(); + UUID savedId = UUID.randomUUID(); + UUID mentionedId = UUID.randomUUID(); + AppUser author = AppUser.builder().id(UUID.randomUUID()).email("anna@example.com").firstName("Anna").lastName("S").build(); + AppUser mentioned = AppUser.builder().id(mentionedId).email("bob@example.com").firstName("Bob").lastName("J").build(); + DocumentComment root = DocumentComment.builder() + .id(rootId).documentId(docId).parentId(null).content("Root").authorName("Hans").build(); + DocumentComment saved = DocumentComment.builder() + .id(savedId).documentId(docId).parentId(rootId).content("Hey @Bob").authorName("Anna S").build(); + when(userService.findAllById(List.of(mentionedId))).thenReturn(List.of(mentioned)); + when(commentRepository.findById(rootId)).thenReturn(Optional.of(root)); + when(commentRepository.findByParentId(rootId)).thenReturn(List.of()); + when(commentRepository.save(any())).thenReturn(saved); + + commentService.replyToComment(docId, rootId, "Hey @Bob", List.of(mentionedId), author); + + verify(auditService).logAfterCommit( + eq(AuditKind.MENTION_CREATED), + eq(author.getId()), + eq(docId), + argThat(p -> mentionedId.toString().equals(p.get("mentionedUserId")))); + } + + @Test + void postBlockComment_logsCommentAdded() { + UUID docId = UUID.randomUUID(); + UUID blockId = UUID.randomUUID(); + UUID savedId = UUID.randomUUID(); + AppUser author = AppUser.builder().id(UUID.randomUUID()).email("felix@example.com").firstName("Felix").lastName("B").build(); + DocumentComment saved = DocumentComment.builder() + .id(savedId).documentId(docId).blockId(blockId).authorName("Felix B").content("Nice").build(); + when(commentRepository.save(any())).thenReturn(saved); + + commentService.postBlockComment(docId, blockId, "Nice", List.of(), author); + + verify(auditService).logAfterCommit( + eq(AuditKind.COMMENT_ADDED), + eq(author.getId()), + eq(docId), + argThat(p -> savedId.toString().equals(p.get("commentId")))); + } + // ─── Block-level comments ──────────────────────────────────────────────── @Test -- 2.49.1