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()); } }