diff --git a/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java b/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java index 64b09294..ccd62649 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java @@ -56,9 +56,17 @@ public class MassImportService { public enum State { IDLE, RUNNING, DONE, FAILED } + public enum SkipReason { + INVALID_FILENAME_PATH_TRAVERSAL, + INVALID_PDF_SIGNATURE, + FILE_READ_ERROR, + ALREADY_EXISTS, + S3_UPLOAD_FAILED + } + public record SkippedFile( @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String filename, - @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String reason + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) SkipReason reason ) {} public record ImportStatus( @@ -293,7 +301,7 @@ public class MassImportService { String filename = index.contains(".") ? index : index + ".pdf"; if (!isValidImportFilename(filename)) { log.warn("Skipping import row {}: filename rejected — {}", i, filename); - skippedFiles.add(new SkippedFile(filename, "INVALID_FILENAME_PATH_TRAVERSAL")); + skippedFiles.add(new SkippedFile(filename, SkipReason.INVALID_FILENAME_PATH_TRAVERSAL)); continue; } Optional fileOnDisk = findFileRecursive(filename); @@ -305,17 +313,17 @@ public class MassImportService { try { if (!isPdfMagicBytes(fileOnDisk.get())) { log.warn("Überspringe {}: Datei beginnt nicht mit %PDF-Signatur", filename); - skippedFiles.add(new SkippedFile(filename, "INVALID_PDF_SIGNATURE")); + skippedFiles.add(new SkippedFile(filename, SkipReason.INVALID_PDF_SIGNATURE)); continue; } } catch (IOException e) { log.error("Fehler beim Prüfen der Magic-Bytes für {}", filename, e); - skippedFiles.add(new SkippedFile(filename, "FILE_READ_ERROR")); + skippedFiles.add(new SkippedFile(filename, SkipReason.FILE_READ_ERROR)); continue; } } - Optional skipReason = importSingleDocument(cells, fileOnDisk, filename, index); + Optional skipReason = importSingleDocument(cells, fileOnDisk, filename, index); if (skipReason.isPresent()) { skippedFiles.add(new SkippedFile(filename, skipReason.get())); } else { @@ -364,11 +372,11 @@ public class MassImportService { * @return empty Optional on success; an Optional containing the skip reason on failure/skip. */ @Transactional - protected Optional importSingleDocument(List cells, Optional file, String originalFilename, String index) { + protected Optional importSingleDocument(List cells, Optional file, String originalFilename, String index) { Optional existing = documentService.findByOriginalFilename(originalFilename); if (existing.isPresent() && existing.get().getStatus() != DocumentStatus.PLACEHOLDER) { log.info("Dokument {} existiert bereits, überspringe.", originalFilename); - return Optional.of("ALREADY_EXISTS"); + return Optional.of(SkipReason.ALREADY_EXISTS); } String archiveBox = getCell(cells, colBox); @@ -404,7 +412,7 @@ public class MassImportService { status = DocumentStatus.UPLOADED; } catch (Exception e) { log.error("S3 Upload Fehler für {}", file.get().getName(), e); - return Optional.of("S3_UPLOAD_FAILED"); + return Optional.of(SkipReason.S3_UPLOAD_FAILED); } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java index 0fb50a8e..575f568c 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java @@ -154,10 +154,10 @@ class MassImportServiceTest { .build(); when(documentService.findByOriginalFilename("doc001.pdf")).thenReturn(Optional.of(existing)); - Optional result = service.importSingleDocument(minimalCells("doc001.pdf"), Optional.empty(), "doc001.pdf", "doc001"); + Optional result = service.importSingleDocument(minimalCells("doc001.pdf"), Optional.empty(), "doc001.pdf", "doc001"); verify(documentService, never()).save(any()); - assertThat(result).isPresent().contains("ALREADY_EXISTS"); + assertThat(result).isPresent().contains(MassImportService.SkipReason.ALREADY_EXISTS); } // ─── importSingleDocument — already-exists guard fires before file I/O ───── @@ -179,10 +179,10 @@ class MassImportServiceTest { byte[] pdfHeader = {0x25, 0x50, 0x44, 0x46, 0x2D}; // %PDF- Files.write(physicalFile, pdfHeader); - Optional result = service.importSingleDocument( + Optional result = service.importSingleDocument( minimalCells("present.pdf"), Optional.of(physicalFile.toFile()), "present.pdf", "present"); - assertThat(result).isPresent().contains("ALREADY_EXISTS"); + assertThat(result).isPresent().contains(MassImportService.SkipReason.ALREADY_EXISTS); verify(s3Client, never()).putObject(any(PutObjectRequest.class), any(RequestBody.class)); verify(documentService, never()).save(any()); } @@ -204,7 +204,7 @@ class MassImportServiceTest { assertThat(service.getStatus().skipped()).isEqualTo(1); assertThat(service.getStatus().skippedFiles()) .extracting(MassImportService.SkippedFile::filename, MassImportService.SkippedFile::reason) - .containsExactly(org.assertj.core.groups.Tuple.tuple("upload_fail.pdf", "S3_UPLOAD_FAILED")); + .containsExactly(org.assertj.core.groups.Tuple.tuple("upload_fail.pdf", MassImportService.SkipReason.S3_UPLOAD_FAILED)); } @Test @@ -223,7 +223,7 @@ class MassImportServiceTest { assertThat(service.getStatus().skipped()).isEqualTo(1); assertThat(service.getStatus().skippedFiles()) .extracting(MassImportService.SkippedFile::reason) - .containsExactly("ALREADY_EXISTS"); + .containsExactly(MassImportService.SkipReason.ALREADY_EXISTS); } // ─── importSingleDocument — create new document (metadata only) ─────────── @@ -283,11 +283,11 @@ class MassImportServiceTest { doThrow(new RuntimeException("S3 error")) .when(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class)); - Optional result = service.importSingleDocument( + Optional result = service.importSingleDocument( minimalCells("fail.pdf"), Optional.of(tempFile.toFile()), "fail.pdf", "fail"); verify(documentService, never()).save(any()); - assertThat(result).isPresent().contains("S3_UPLOAD_FAILED"); + assertThat(result).isPresent().contains(MassImportService.SkipReason.S3_UPLOAD_FAILED); } // ─── importSingleDocument — sender handling ─────────────────────────────── @@ -539,7 +539,7 @@ class MassImportServiceTest { assertThat(result.processed()).isEqualTo(1); assertThat(result.skippedFiles()) .extracting(MassImportService.SkippedFile::reason) - .containsExactly("INVALID_FILENAME_PATH_TRAVERSAL"); + .containsExactly(MassImportService.SkipReason.INVALID_FILENAME_PATH_TRAVERSAL); } // ─── importSingleDocument — non-blank optional fields ──────────────────── @@ -755,7 +755,7 @@ class MassImportServiceTest { assertThat(spyService.getStatus().skipped()).isEqualTo(1); assertThat(spyService.getStatus().skippedFiles()) .extracting(MassImportService.SkippedFile::reason) - .containsExactly("FILE_READ_ERROR"); + .containsExactly(MassImportService.SkipReason.FILE_READ_ERROR); } // ─── readOds — XXE security regression ───────────────────────────────────