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 58d6493a..e09aaa76 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java @@ -69,9 +69,15 @@ public class MassImportService { @Schema(requiredMode = Schema.RequiredMode.REQUIRED) List skippedFiles, LocalDateTime startedAt ) { - @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + // Note: @Schema on a record accessor method is not picked up by SpringDoc; the + // "skipped" count is a computed convenience field derived from skippedFiles.size(). @JsonProperty("skipped") public int skipped() { return skippedFiles.size(); } + + /** Defensive-copy constructor — callers cannot mutate the stored list after construction. */ + public ImportStatus { + skippedFiles = List.copyOf(skippedFiles); + } } record ProcessResult(int processed, List skippedFiles) {} @@ -304,8 +310,10 @@ public class MassImportService { } } - boolean imported = importSingleDocument(cells, fileOnDisk, filename, index); - if (imported) { + Optional skipReason = importSingleDocument(cells, fileOnDisk, filename, index); + if (skipReason.isPresent()) { + skippedFiles.add(new SkippedFile(filename, skipReason.get())); + } else { processed++; } } @@ -328,12 +336,17 @@ public class MassImportService { } } + /** + * Imports a single document row. + * + * @return empty Optional on success; an Optional containing the skip reason on failure/skip. + */ @Transactional - protected boolean 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 false; + return Optional.of("ALREADY_EXISTS"); } String archiveBox = getCell(cells, colBox); @@ -369,7 +382,7 @@ public class MassImportService { status = DocumentStatus.UPLOADED; } catch (Exception e) { log.error("S3 Upload Fehler für {}", file.get().getName(), e); - return false; + return Optional.of("S3_UPLOAD_FAILED"); } } @@ -411,7 +424,7 @@ public class MassImportService { thumbnailAsyncRunner.dispatchAfterCommit(saved.getId()); } log.info("Importiert{}: {}", file.isEmpty() ? " (nur Metadaten)" : "", originalFilename); - return true; + return Optional.empty(); } // --- Helpers --- 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 31cd2842..26d38679 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java @@ -154,9 +154,49 @@ class MassImportServiceTest { .build(); when(documentService.findByOriginalFilename("doc001.pdf")).thenReturn(Optional.of(existing)); - 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"); + } + + // ─── importSingleDocument — S3 failure surfaced in skippedFiles ────────── + + @Test + void runImportAsync_addsS3UploadFailed_toSkippedFiles_whenS3Throws(@TempDir Path tempDir) throws Exception { + byte[] pdfHeader = {0x25, 0x50, 0x44, 0x46, 0x2D}; // %PDF- + Files.write(tempDir.resolve("upload_fail.pdf"), pdfHeader); + buildMinimalImportXlsx(tempDir, "upload_fail.pdf"); + ReflectionTestUtils.setField(service, "importDir", tempDir.toString()); + when(documentService.findByOriginalFilename("upload_fail.pdf")).thenReturn(Optional.empty()); + doThrow(new RuntimeException("S3 unavailable")) + .when(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class)); + + service.runImportAsync(); + + 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")); + } + + @Test + void runImportAsync_addsAlreadyExists_toSkippedFiles_whenDocumentAlreadyUploaded(@TempDir Path tempDir) throws Exception { + buildMinimalImportXlsx(tempDir, "existing.pdf"); + ReflectionTestUtils.setField(service, "importDir", tempDir.toString()); + Document existing = Document.builder() + .id(UUID.randomUUID()) + .originalFilename("existing.pdf") + .status(DocumentStatus.UPLOADED) + .build(); + when(documentService.findByOriginalFilename("existing.pdf")).thenReturn(Optional.of(existing)); + + service.runImportAsync(); + + assertThat(service.getStatus().skipped()).isEqualTo(1); + assertThat(service.getStatus().skippedFiles()) + .extracting(MassImportService.SkippedFile::reason) + .containsExactly("ALREADY_EXISTS"); } // ─── importSingleDocument — create new document (metadata only) ─────────── @@ -208,7 +248,7 @@ class MassImportServiceTest { } @Test - void importSingleDocument_returnsEarly_whenS3UploadFails(@TempDir Path tempDir) throws Exception { + void importSingleDocument_returnsS3UploadFailed_whenS3UploadFails(@TempDir Path tempDir) throws Exception { Path tempFile = tempDir.resolve("fail.pdf"); Files.write(tempFile, "data".getBytes()); @@ -216,10 +256,11 @@ class MassImportServiceTest { doThrow(new RuntimeException("S3 error")) .when(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class)); - 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"); } // ─── importSingleDocument — sender handling ─────────────────────────────── diff --git a/frontend/messages/de.json b/frontend/messages/de.json index e225997f..72254cf3 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -355,6 +355,8 @@ "admin_system_import_skipped_label": "übersprungen", "import_reason_invalid_pdf_signature": "Keine gültige PDF-Signatur", "import_reason_file_read_error": "Fehler beim Lesen der Datei", + "import_reason_s3_upload_failed": "Upload-Fehler (S3)", + "import_reason_already_exists": "Bereits importiert", "admin_system_import_status_failed": "Import fehlgeschlagen", "admin_system_import_failed_no_spreadsheet": "Keine Tabellendatei gefunden.", "admin_system_import_failed_internal": "Interner Fehler beim Import.", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 894c60f8..b8b6dd68 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -355,6 +355,8 @@ "admin_system_import_skipped_label": "skipped", "import_reason_invalid_pdf_signature": "Invalid PDF signature", "import_reason_file_read_error": "File read error", + "import_reason_s3_upload_failed": "Upload error (S3)", + "import_reason_already_exists": "Already imported", "admin_system_import_status_failed": "Import failed", "admin_system_import_failed_no_spreadsheet": "No spreadsheet file found.", "admin_system_import_failed_internal": "Import failed due to an internal error.", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index a2409dbf..0943943e 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -355,6 +355,8 @@ "admin_system_import_skipped_label": "omitidos", "import_reason_invalid_pdf_signature": "Firma PDF no válida", "import_reason_file_read_error": "Error al leer el archivo", + "import_reason_s3_upload_failed": "Error de carga (S3)", + "import_reason_already_exists": "Ya importado", "admin_system_import_status_failed": "Importación fallida", "admin_system_import_failed_no_spreadsheet": "No se encontró ninguna hoja de cálculo.", "admin_system_import_failed_internal": "Error interno durante la importación.", diff --git a/frontend/src/routes/admin/system/ImportStatusCard.svelte b/frontend/src/routes/admin/system/ImportStatusCard.svelte index f6e9ab82..c18fcef3 100644 --- a/frontend/src/routes/admin/system/ImportStatusCard.svelte +++ b/frontend/src/routes/admin/system/ImportStatusCard.svelte @@ -19,6 +19,8 @@ const failureMessage = $derived( function reasonLabel(code: string): string { if (code === 'INVALID_PDF_SIGNATURE') return m.import_reason_invalid_pdf_signature(); if (code === 'FILE_READ_ERROR') return m.import_reason_file_read_error(); + if (code === 'S3_UPLOAD_FAILED') return m.import_reason_s3_upload_failed(); + if (code === 'ALREADY_EXISTS') return m.import_reason_already_exists(); return code; } @@ -54,38 +56,41 @@ function reasonLabel(code: string): string {

{m.admin_system_import_status_done()}

- {#if importStatus.skipped > 0} -
- - - - -
- {importStatus.skipped} + {#if importStatus.skipped > 0} +
+ + - {m.admin_system_import_skipped_label()} - -
- -
    - {#each importStatus.skippedFiles as skipped (skipped.filename)} -
  • - {skipped.filename} — {reasonLabel(skipped.reason)} -
    • - {/each} -
-
- {/if} + + +
+ {importStatus.skipped} + + {m.admin_system_import_skipped_label()} + +
+ +
    + {#each importStatus.skippedFiles as skipped (skipped.filename)} +
  • + {skipped.filename} — {reasonLabel(skipped.reason)} +
    • + {/each} +
+ + {/if} +