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 d2f1e276..b4873b41 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java @@ -1,6 +1,7 @@ package org.raddatz.familienarchiv.importing; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.apache.poi.ss.usermodel.*; @@ -56,11 +57,14 @@ public class MassImportService { public record SkippedFile(String filename, String reason) {} - public record ImportStatus(State state, String statusCode, @JsonIgnore String message, int processed, int skipped, List skippedFiles, LocalDateTime startedAt) {} + public record ImportStatus(State state, String statusCode, @JsonIgnore String message, int processed, List skippedFiles, LocalDateTime startedAt) { + @JsonProperty("skipped") + public int skipped() { return skippedFiles.size(); } + } - record ProcessResult(int processed, int skipped, List skippedFiles) {} + record ProcessResult(int processed, List skippedFiles) {} - private volatile ImportStatus currentStatus = new ImportStatus(State.IDLE, "IMPORT_IDLE", "Kein Import gestartet.", 0, 0, List.of(), null); + private volatile ImportStatus currentStatus = new ImportStatus(State.IDLE, "IMPORT_IDLE", "Kein Import gestartet.", 0, List.of(), null); public ImportStatus getStatus() { return currentStatus; @@ -122,22 +126,22 @@ public class MassImportService { if (currentStatus.state() == State.RUNNING) { throw DomainException.conflict(ErrorCode.IMPORT_ALREADY_RUNNING, "A mass import is already in progress"); } - currentStatus = new ImportStatus(State.RUNNING, "IMPORT_RUNNING", "Import läuft...", 0, 0, List.of(), LocalDateTime.now()); + currentStatus = new ImportStatus(State.RUNNING, "IMPORT_RUNNING", "Import läuft...", 0, List.of(), LocalDateTime.now()); try { File spreadsheet = findSpreadsheetFile(); log.info("Starte Massenimport aus: {}", spreadsheet.getAbsolutePath()); ProcessResult result = processRows(readSpreadsheet(spreadsheet)); currentStatus = new ImportStatus(State.DONE, "IMPORT_DONE", "Import abgeschlossen. " + result.processed() + " Dokumente verarbeitet.", - result.processed(), result.skipped(), result.skippedFiles(), currentStatus.startedAt()); + result.processed(), result.skippedFiles(), currentStatus.startedAt()); } catch (NoSpreadsheetException e) { log.error("Massenimport fehlgeschlagen: keine Tabellendatei", e); currentStatus = new ImportStatus(State.FAILED, "IMPORT_FAILED_NO_SPREADSHEET", - "Fehler: " + e.getMessage(), 0, 0, List.of(), currentStatus.startedAt()); + "Fehler: " + e.getMessage(), 0, List.of(), currentStatus.startedAt()); } catch (Exception e) { log.error("Massenimport fehlgeschlagen", e); currentStatus = new ImportStatus(State.FAILED, "IMPORT_FAILED_INTERNAL", - "Fehler: " + e.getMessage(), 0, 0, List.of(), currentStatus.startedAt()); + "Fehler: " + e.getMessage(), 0, List.of(), currentStatus.startedAt()); } } @@ -278,12 +282,12 @@ public class MassImportService { try { if (!isPdfMagicBytes(fileOnDisk.get())) { log.warn("Überspringe {}: Datei beginnt nicht mit %PDF-Signatur", filename); - skippedFiles.add(new SkippedFile(filename, "Keine gültige PDF-Signatur")); + skippedFiles.add(new SkippedFile(filename, "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, "Fehler beim Lesen der Datei")); + skippedFiles.add(new SkippedFile(filename, "FILE_READ_ERROR")); continue; } } @@ -293,7 +297,7 @@ public class MassImportService { processed++; } } - return new ProcessResult(processed, skippedFiles.size(), skippedFiles); + return new ProcessResult(processed, skippedFiles); } private boolean isPdfMagicBytes(File file) throws IOException { 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 eef66a71..4442e15d 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java @@ -40,6 +40,7 @@ import java.util.zip.ZipOutputStream; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @@ -135,7 +136,7 @@ class MassImportServiceTest { @Test void runImportAsync_throwsConflict_whenAlreadyRunning() { MassImportService.ImportStatus running = new MassImportService.ImportStatus( - MassImportService.State.RUNNING, "IMPORT_RUNNING", "Running...", 0, 0, List.of(), LocalDateTime.now()); + MassImportService.State.RUNNING, "IMPORT_RUNNING", "Running...", 0, List.of(), LocalDateTime.now()); ReflectionTestUtils.setField(service, "currentStatus", running); assertThatThrownBy(() -> service.runImportAsync()) @@ -529,14 +530,7 @@ class MassImportServiceTest { @Test void runImportAsync_uploadsValidPdf_andSkipsFakeOne(@TempDir Path tempDir) throws Exception { - byte[] pdfHeader = {0x25, 0x50, 0x44, 0x46, 0x2D}; // %PDF- - Files.write(tempDir.resolve("real.pdf"), pdfHeader); - Files.writeString(tempDir.resolve("fake.pdf"), "not a pdf"); - buildMinimalImportXlsx(tempDir, "real.pdf", "fake.pdf"); - ReflectionTestUtils.setField(service, "importDir", tempDir.toString()); - - when(documentService.findByOriginalFilename(any())).thenReturn(Optional.empty()); - when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); + setupOneValidOneFakeImport(tempDir); service.runImportAsync(); @@ -545,14 +539,7 @@ class MassImportServiceTest { @Test void runImportAsync_setsSkippedCount_toOne_whenOneFakeFile(@TempDir Path tempDir) throws Exception { - byte[] pdfHeader = {0x25, 0x50, 0x44, 0x46, 0x2D}; // %PDF- - Files.write(tempDir.resolve("real.pdf"), pdfHeader); - Files.writeString(tempDir.resolve("fake.pdf"), "not a pdf"); - buildMinimalImportXlsx(tempDir, "real.pdf", "fake.pdf"); - ReflectionTestUtils.setField(service, "importDir", tempDir.toString()); - - when(documentService.findByOriginalFilename(any())).thenReturn(Optional.empty()); - when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); + setupOneValidOneFakeImport(tempDir); service.runImportAsync(); @@ -561,14 +548,7 @@ class MassImportServiceTest { @Test void runImportAsync_includesRejectedFilename_inSkippedFiles(@TempDir Path tempDir) throws Exception { - byte[] pdfHeader = {0x25, 0x50, 0x44, 0x46, 0x2D}; // %PDF- - Files.write(tempDir.resolve("real.pdf"), pdfHeader); - Files.writeString(tempDir.resolve("fake.pdf"), "not a pdf"); - buildMinimalImportXlsx(tempDir, "real.pdf", "fake.pdf"); - ReflectionTestUtils.setField(service, "importDir", tempDir.toString()); - - when(documentService.findByOriginalFilename(any())).thenReturn(Optional.empty()); - when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); + setupOneValidOneFakeImport(tempDir); service.runImportAsync(); @@ -582,12 +562,31 @@ class MassImportServiceTest { Files.write(tempDir.resolve("tiny.pdf"), new byte[]{0x25, 0x50, 0x44}); // only 3 bytes buildMinimalImportXlsx(tempDir, "tiny.pdf"); ReflectionTestUtils.setField(service, "importDir", tempDir.toString()); + lenient().when(documentService.findByOriginalFilename(any())).thenReturn(Optional.empty()); service.runImportAsync(); assertThat(service.getStatus().skipped()).isEqualTo(1); } + @Test + void runImportAsync_skipsFile_whenMagicBytesCheckThrowsIOException(@TempDir Path tempDir) throws Exception { + Files.writeString(tempDir.resolve("unreadable.pdf"), "some content"); + File unreadable = tempDir.resolve("unreadable.pdf").toFile(); + buildMinimalImportXlsx(tempDir, "unreadable.pdf"); + ReflectionTestUtils.setField(service, "importDir", tempDir.toString()); + assumeTrue(unreadable.setReadable(false), "Requires non-root file permissions"); + try { + service.runImportAsync(); + assertThat(service.getStatus().skipped()).isEqualTo(1); + assertThat(service.getStatus().skippedFiles()) + .extracting(MassImportService.SkippedFile::reason) + .containsExactly("FILE_READ_ERROR"); + } finally { + unreadable.setReadable(true); + } + } + // ─── readOds — XXE security regression ─────────────────────────────────── // Security regression — do not remove. @@ -685,6 +684,16 @@ class MassImportServiceTest { return destination.toFile(); } + private void setupOneValidOneFakeImport(Path tempDir) throws Exception { + byte[] pdfHeader = {0x25, 0x50, 0x44, 0x46, 0x2D}; // %PDF- + Files.write(tempDir.resolve("real.pdf"), pdfHeader); + Files.writeString(tempDir.resolve("fake.pdf"), "not a pdf"); + buildMinimalImportXlsx(tempDir, "real.pdf", "fake.pdf"); + ReflectionTestUtils.setField(service, "importDir", tempDir.toString()); + when(documentService.findByOriginalFilename(any())).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); + } + private void buildMinimalImportXlsx(Path dir, String... filenames) throws Exception { Path xlsx = dir.resolve("import.xlsx"); try (XSSFWorkbook wb = new XSSFWorkbook()) { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/user/AdminControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/user/AdminControllerTest.java index cc2b885a..9b7f84b2 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/user/AdminControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/user/AdminControllerTest.java @@ -19,7 +19,6 @@ import org.springframework.test.web.servlet.MockMvc; import java.time.LocalDateTime; import java.util.List; -import java.util.List; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.verify; @@ -47,7 +46,7 @@ class AdminControllerTest { @WithMockUser(authorities = "ADMIN") void importStatus_returns200_withStatusCode_whenAdmin() throws Exception { MassImportService.ImportStatus status = new MassImportService.ImportStatus( - MassImportService.State.IDLE, "IMPORT_IDLE", "Kein Import gestartet.", 0, 0, List.of(), null); + MassImportService.State.IDLE, "IMPORT_IDLE", "Kein Import gestartet.", 0, List.of(), null); when(massImportService.getStatus()).thenReturn(status); mockMvc.perform(get("/api/admin/import-status")) @@ -61,7 +60,7 @@ class AdminControllerTest { @WithMockUser(authorities = "ADMIN") void importStatus_messageField_notPresentInApiResponse() throws Exception { MassImportService.ImportStatus status = new MassImportService.ImportStatus( - MassImportService.State.IDLE, "IMPORT_IDLE", "Kein Import gestartet.", 0, 0, List.of(), null); + MassImportService.State.IDLE, "IMPORT_IDLE", "Kein Import gestartet.", 0, List.of(), null); when(massImportService.getStatus()).thenReturn(status); mockMvc.perform(get("/api/admin/import-status")) diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 55e93d40..ace72a7e 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -351,6 +351,8 @@ "admin_system_import_status_done": "Import abgeschlossen", "admin_system_import_status_done_label": "Dokumente verarbeitet", "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", "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 1ace8196..4bcc3ca7 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -351,6 +351,8 @@ "admin_system_import_status_done": "Import complete", "admin_system_import_status_done_label": "Documents processed", "admin_system_import_skipped_label": "skipped", + "import_reason_invalid_pdf_signature": "Invalid PDF signature", + "import_reason_file_read_error": "File read error", "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 286faf5b..b52b17c3 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -351,6 +351,8 @@ "admin_system_import_status_done": "Importación completada", "admin_system_import_status_done_label": "Documentos procesados", "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", "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 a7cbba65..69e267c5 100644 --- a/frontend/src/routes/admin/system/ImportStatusCard.svelte +++ b/frontend/src/routes/admin/system/ImportStatusCard.svelte @@ -15,6 +15,12 @@ const failureMessage = $derived( ? m.admin_system_import_failed_no_spreadsheet() : m.admin_system_import_failed_internal() ); + +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(); + return code; +}
@@ -49,16 +55,31 @@ const failureMessage = $derived(

{m.admin_system_import_status_done()}

{#if importStatus.skipped > 0} -
- -

{importStatus.skipped}

-

- {m.admin_system_import_skipped_label()} -

+
+ + + + +
+ {importStatus.skipped} + + {m.admin_system_import_skipped_label()} + +
    {#each importStatus.skippedFiles as { filename, reason } (filename)} -
  • {filename} — {reason}
    • +
  • {filename} — {reasonLabel(reason)}
    • {/each}
@@ -94,3 +115,9 @@ const failureMessage = $derived( {/if} + + diff --git a/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts b/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts index 7f28de0a..a786b787 100644 --- a/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts +++ b/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts @@ -140,9 +140,9 @@ describe('ImportStatusCard', () => { processed: 10, skipped: 3, skippedFiles: [ - { filename: 'fake.pdf', reason: 'Keine gültige PDF-Signatur' }, - { filename: 'other.pdf', reason: 'Keine gültige PDF-Signatur' }, - { filename: 'tiny.pdf', reason: 'Keine gültige PDF-Signatur' } + { filename: 'fake.pdf', reason: 'INVALID_PDF_SIGNATURE' }, + { filename: 'other.pdf', reason: 'INVALID_PDF_SIGNATURE' }, + { filename: 'tiny.pdf', reason: 'INVALID_PDF_SIGNATURE' } ] }), ontrigger: () => {} @@ -160,7 +160,7 @@ describe('ImportStatusCard', () => { statusCode: 'IMPORT_DONE', processed: 5, skipped: 1, - skippedFiles: [{ filename: 'fake.pdf', reason: 'Keine gültige PDF-Signatur' }] + skippedFiles: [{ filename: 'fake.pdf', reason: 'INVALID_PDF_SIGNATURE' }] }), ontrigger: () => {} }