From 963807ff05fa859877979c31c9454b49b46c071b Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 26 Mar 2026 10:38:30 +0100 Subject: [PATCH] fix(upload): structured error codes for quick-upload, fix duplicate filename crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Switch errors from plain strings to { filename, code } objects so the frontend can show translated messages instead of raw exception text - Add UNSUPPORTED_FILE_TYPE error code end-to-end (Java enum → errors.ts → de/en/es messages) - Fix IncorrectResultSizeDataAccessException when a filename exists more than once in the DB: use findFirstByOriginalFilename instead of findByOriginalFilename in storeDocument() Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/controller/DocumentController.java | 9 +++++---- .../org/raddatz/familienarchiv/exception/ErrorCode.java | 2 ++ .../familienarchiv/repository/DocumentRepository.java | 3 +++ .../raddatz/familienarchiv/service/DocumentService.java | 4 ++-- .../controller/DocumentControllerTest.java | 5 +++-- .../familienarchiv/service/DocumentServiceTest.java | 4 ++-- frontend/messages/de.json | 1 + frontend/messages/en.json | 1 + frontend/messages/es.json | 1 + frontend/src/lib/errors.ts | 3 +++ frontend/src/routes/+page.svelte | 6 +++++- 11 files changed, 28 insertions(+), 11 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 a405577d..e09810ae 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java @@ -110,14 +110,15 @@ public class DocumentController { private static final Set ALLOWED_CONTENT_TYPES = Set.of( "application/pdf", "image/jpeg", "image/png", "image/tiff"); - public record QuickUploadResult(List created, List errors) {} + public record UploadError(String filename, String code) {} + public record QuickUploadResult(List created, List errors) {} @PostMapping(value = "/quick-upload", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) @RequirePermission(Permission.WRITE_ALL) public QuickUploadResult quickUpload( @RequestPart(value = "files", required = false) List files) { List created = new ArrayList<>(); - List errors = new ArrayList<>(); + List errors = new ArrayList<>(); if (files == null || files.isEmpty()) { return new QuickUploadResult(created, errors); @@ -125,13 +126,13 @@ public class DocumentController { for (MultipartFile file : files) { if (!ALLOWED_CONTENT_TYPES.contains(file.getContentType())) { - errors.add(file.getOriginalFilename() + ": unsupported file type"); + errors.add(new UploadError(file.getOriginalFilename(), "UNSUPPORTED_FILE_TYPE")); continue; } try { created.add(documentService.storeDocument(file)); } catch (Exception e) { - errors.add(file.getOriginalFilename() + ": " + e.getMessage()); + errors.add(new UploadError(file.getOriginalFilename(), "FILE_UPLOAD_FAILED")); log.warn("Quick upload failed for file {}: {}", file.getOriginalFilename(), e.getMessage()); } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java index a26e5e5a..bcf72ef8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -17,6 +17,8 @@ public enum ErrorCode { FILE_NOT_FOUND, /** An error occurred while uploading a file to object storage. 500 */ FILE_UPLOAD_FAILED, + /** The uploaded file's content type is not supported (PDF/JPEG/PNG/TIFF only). 400 */ + UNSUPPORTED_FILE_TYPE, // --- Users --- /** A user with the given ID or username does not exist. 404 */ diff --git a/backend/src/main/java/org/raddatz/familienarchiv/repository/DocumentRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/repository/DocumentRepository.java index 46969526..25da2dcd 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/repository/DocumentRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/repository/DocumentRepository.java @@ -21,6 +21,9 @@ public interface DocumentRepository extends JpaRepository, JpaSp // Wichtig für den Abgleich beim Excel-Import & Datei-Upload Optional findByOriginalFilename(String originalFilename); + // Wie oben, gibt aber nur das erste Ergebnis zurück — sicher wenn doppelte Dateinamen existieren + Optional findFirstByOriginalFilename(String originalFilename); + // Findet alle Dokumente mit einem bestimmten Status // z.B. um alle offenen "PLACEHOLDER" zu finden List findByStatus(DocumentStatus status); 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 0d1481bd..4722ce1d 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -52,8 +52,8 @@ public class DocumentService { public Document storeDocument(MultipartFile file) throws IOException { String originalFilename = file.getOriginalFilename(); - // 1. Check for existing record - Optional existingDoc = documentRepository.findByOriginalFilename(originalFilename); + // 1. Check for existing record (findFirst to survive duplicate filenames in the DB) + Optional existingDoc = documentRepository.findFirstByOriginalFilename(originalFilename); Document document; if (existingDoc.isPresent()) { 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 0f64fddc..c1637f6c 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java @@ -23,7 +23,6 @@ import java.util.Collections; import java.util.List; import java.util.UUID; -import static org.hamcrest.Matchers.containsString; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -150,6 +149,7 @@ class DocumentControllerTest { mockMvc.perform(multipart("/api/documents/quick-upload").file(file)) .andExpect(status().isOk()) .andExpect(jsonPath("$.created[0].title").value("scan001")) + .andExpect(jsonPath("$.errors").isArray()) .andExpect(jsonPath("$.errors").isEmpty()); } @@ -163,7 +163,8 @@ class DocumentControllerTest { mockMvc.perform(multipart("/api/documents/quick-upload").file(file)) .andExpect(status().isOk()) .andExpect(jsonPath("$.created").isEmpty()) - .andExpect(jsonPath("$.errors[0]").value(containsString("report.docx"))); + .andExpect(jsonPath("$.errors[0].filename").value("report.docx")) + .andExpect(jsonPath("$.errors[0].code").value("UNSUPPORTED_FILE_TYPE")); } // ─── GET /api/documents/{id}/versions ──────────────────────────────────── 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 36e7c2eb..b8c889e5 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java @@ -221,7 +221,7 @@ class DocumentServiceTest { FileService.UploadResult uploadResult = new FileService.UploadResult("documents/uuid_scan001.pdf", "abc123"); Document saved = Document.builder().id(UUID.randomUUID()).title("scan001").originalFilename("scan001.pdf").build(); - when(documentRepository.findByOriginalFilename("scan001.pdf")).thenReturn(Optional.empty()); + when(documentRepository.findFirstByOriginalFilename("scan001.pdf")).thenReturn(Optional.empty()); when(documentRepository.save(any())).thenReturn(saved); when(fileService.uploadFile(any(), any())).thenReturn(uploadResult); @@ -241,7 +241,7 @@ class DocumentServiceTest { .id(UUID.randomUUID()).title("Brief an Oma").originalFilename("scan001.pdf") .status(org.raddatz.familienarchiv.model.DocumentStatus.PLACEHOLDER).build(); - when(documentRepository.findByOriginalFilename("scan001.pdf")).thenReturn(Optional.of(placeholder)); + when(documentRepository.findFirstByOriginalFilename("scan001.pdf")).thenReturn(Optional.of(placeholder)); when(documentRepository.save(any())).thenReturn(placeholder); when(fileService.uploadFile(any(), any())).thenReturn(uploadResult); diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 0c8f9838..b683a77b 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -7,6 +7,7 @@ "error_document_no_file": "Diesem Dokument ist noch keine Datei zugeordnet.", "error_file_not_found": "Die Datei konnte im Speicher nicht gefunden werden.", "error_file_upload_failed": "Die Datei konnte nicht hochgeladen werden.", + "error_unsupported_file_type": "Dieses Dateiformat wird nicht unterstützt.", "error_user_not_found": "Der Benutzer wurde nicht gefunden.", "error_import_already_running": "Ein Import läuft bereits. Bitte warten Sie, bis dieser abgeschlossen ist.", "error_unauthorized": "Sie sind nicht angemeldet.", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index eb5ba31a..2d83e6ab 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -7,6 +7,7 @@ "error_document_no_file": "No file is associated with this document.", "error_file_not_found": "The file could not be found in storage.", "error_file_upload_failed": "The file could not be uploaded.", + "error_unsupported_file_type": "This file format is not supported.", "error_user_not_found": "User not found.", "error_import_already_running": "An import is already running. Please wait for it to finish.", "error_unauthorized": "You are not logged in.", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index dfb5ec70..a8c4fd62 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -7,6 +7,7 @@ "error_document_no_file": "No hay ningún archivo asociado a este documento.", "error_file_not_found": "El archivo no pudo encontrarse en el almacenamiento.", "error_file_upload_failed": "No se pudo subir el archivo.", + "error_unsupported_file_type": "Este formato de archivo no está admitido.", "error_user_not_found": "Usuario no encontrado.", "error_import_already_running": "Ya hay una importación en curso. Por favor, espere a que finalice.", "error_unauthorized": "No ha iniciado sesión.", diff --git a/frontend/src/lib/errors.ts b/frontend/src/lib/errors.ts index 3e679fbc..3abe2b0c 100644 --- a/frontend/src/lib/errors.ts +++ b/frontend/src/lib/errors.ts @@ -9,6 +9,7 @@ export type ErrorCode = | 'DOCUMENT_NO_FILE' | 'FILE_NOT_FOUND' | 'FILE_UPLOAD_FAILED' + | 'UNSUPPORTED_FILE_TYPE' | 'USER_NOT_FOUND' | 'EMAIL_ALREADY_IN_USE' | 'WRONG_CURRENT_PASSWORD' @@ -54,6 +55,8 @@ export function getErrorMessage(code: ErrorCode | string | undefined): string { return m.error_file_not_found(); case 'FILE_UPLOAD_FAILED': return m.error_file_upload_failed(); + case 'UNSUPPORTED_FILE_TYPE': + return m.error_unsupported_file_type(); case 'USER_NOT_FOUND': return m.error_user_not_found(); case 'EMAIL_ALREADY_IN_USE': diff --git a/frontend/src/routes/+page.svelte b/frontend/src/routes/+page.svelte index 6099189d..2daaac17 100644 --- a/frontend/src/routes/+page.svelte +++ b/frontend/src/routes/+page.svelte @@ -7,6 +7,7 @@ import { untrack } from 'svelte'; import { SvelteURLSearchParams } from 'svelte/reactivity'; import { m } from '$lib/paraglide/messages.js'; import { formatDate } from '$lib/utils/date'; +import { getErrorMessage } from '$lib/errors'; let { data } = $props(); @@ -101,7 +102,10 @@ async function uploadFiles(files: File[]) { messages.push({ text: m.upload_success({ count: result.created.length }), isError: false }); } for (const err of result.errors ?? []) { - messages.push({ text: err, isError: true }); + messages.push({ + text: `${err.filename}: ${getErrorMessage(err.code)}`, + isError: true + }); } await invalidateAll(); } else {