From b2264de9493f38cbf768ce7ad434970db4de8961 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 24 Apr 2026 20:25:17 +0200 Subject: [PATCH] refactor(documents): move batch validation from controller into DocumentService Validation guards (BATCH_TOO_LARGE, titles > files) are domain rules and belong in the service where they can be unit-tested without the HTTP layer. Controller now delegates to documentService.validateBatch(). Co-Authored-By: Claude Sonnet 4.6 --- .../controller/DocumentController.java | 7 +----- .../service/DocumentService.java | 9 +++++++ .../controller/DocumentControllerTest.java | 8 ++++++ .../service/DocumentServiceTest.java | 25 +++++++++++++++++++ 4 files changed, 43 insertions(+), 6 deletions(-) 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 cd03a22f..ea943163 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java @@ -204,12 +204,7 @@ public class DocumentController { return new QuickUploadResult(created, updated, errors); } - if (files.size() > 50) { - throw DomainException.badRequest(ErrorCode.BATCH_TOO_LARGE, "Batch exceeds maximum of 50 files per request"); - } - if (metadata != null && metadata.getTitles() != null && metadata.getTitles().size() > files.size()) { - throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, "titles count must not exceed files count"); - } + documentService.validateBatch(files.size(), metadata); UUID actorId = requireUserId(authentication); long totalBytes = files.stream().mapToLong(MultipartFile::getSize).sum(); 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 36575f28..01ea544d 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -133,6 +133,15 @@ public class DocumentService { return new StoreResult(saved, isNew); } + public void validateBatch(int fileCount, DocumentBatchMetadataDTO metadata) { + if (fileCount > 50) { + throw DomainException.badRequest(ErrorCode.BATCH_TOO_LARGE, "Batch exceeds maximum of 50 files per request"); + } + if (metadata != null && metadata.getTitles() != null && metadata.getTitles().size() > fileCount) { + throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, "titles count must not exceed files count"); + } + } + @Transactional public StoreResult storeDocumentWithBatchMetadata( MultipartFile file, DocumentBatchMetadataDTO metadata, int fileIndex, UUID actorId) throws IOException { 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 1981b1b2..828e8402 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java @@ -869,6 +869,10 @@ class DocumentControllerTest { @WithMockUser(authorities = "WRITE_ALL") void quickUpload_withMetadata_rejects400_whenTitlesSizeExceedsFilesSize() throws Exception { when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); + org.mockito.Mockito.doThrow( + org.raddatz.familienarchiv.exception.DomainException.badRequest( + org.raddatz.familienarchiv.exception.ErrorCode.VALIDATION_ERROR, "titles count must not exceed files count")) + .when(documentService).validateBatch(eq(2), any()); org.springframework.mock.web.MockMultipartFile f1 = new org.springframework.mock.web.MockMultipartFile("files", "a.pdf", "application/pdf", new byte[]{1}); @@ -886,6 +890,10 @@ class DocumentControllerTest { @WithMockUser(authorities = "WRITE_ALL") void quickUpload_returns400_whenBatchExceedsCap() throws Exception { when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); + org.mockito.Mockito.doThrow( + org.raddatz.familienarchiv.exception.DomainException.badRequest( + org.raddatz.familienarchiv.exception.ErrorCode.BATCH_TOO_LARGE, "Batch exceeds maximum of 50 files per request")) + .when(documentService).validateBatch(eq(51), any()); var builder = multipart("/api/documents/quick-upload"); for (int i = 0; i < 51; i++) { 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 0e46196f..052658a8 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java @@ -1813,4 +1813,29 @@ class DocumentServiceTest { verify(auditService).logAfterCommit(eq(AuditKind.FILE_UPLOADED), isNull(), eq(id), isNull()); } + + // ─── validateBatch ─────────────────────────────────────────────────────── + + @Test + void validateBatch_throwsBatchTooLarge_whenFileCountExceedsCap() { + assertThatThrownBy(() -> documentService.validateBatch(51, null)) + .isInstanceOf(DomainException.class) + .hasMessageContaining("50"); + } + + @Test + void validateBatch_doesNotThrow_whenFileCountEqualsCapExactly() { + documentService.validateBatch(50, null); + } + + @Test + void validateBatch_throwsValidationError_whenTitlesSizeExceedsFileCount() { + org.raddatz.familienarchiv.dto.DocumentBatchMetadataDTO metadata = + new org.raddatz.familienarchiv.dto.DocumentBatchMetadataDTO(); + metadata.setTitles(java.util.List.of("A", "B", "C")); + + assertThatThrownBy(() -> documentService.validateBatch(2, metadata)) + .isInstanceOf(DomainException.class) + .hasMessageContaining("titles"); + } }