marcel/familienarchiv
1
0
Fork 0
Code Issues 105 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix(import): address PR review concerns

Browse Source 
- remove duplicate List import in AdminControllerTest
- derive skipped() from skippedFiles.size() — drop redundant int field
- use machine codes for SkippedFile.reason (INVALID_PDF_SIGNATURE, FILE_READ_ERROR)
- map reason codes to i18n strings in ImportStatusCard (de/en/es)
- replace raw amber Tailwind classes with warning semantic token
- fix <summary> accessibility: replace list-none with rotating chevron SVG
- replace <p> with <span> inside <summary> (phrasing content rule)
- extract setupOneValidOneFakeImport() helper — remove 3x copy-paste
- add lenient mock to short-file test for defensive coverage
- add IOException path test for isPdfMagicBytes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-05-18 14:45:21 +02:00
committed by marcel
parent 0451b6630c
commit 5587722800
8 changed files with 94 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java
View File

@@ -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()) {

5
backend/src/test/java/org/raddatz/familienarchiv/user/AdminControllerTest.java
View File

@@ -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;
@@ -48,7 +47,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"))
@@ -62,7 +61,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"))