fix(upload): structured error codes for quick-upload, fix duplicate filename crash
Some checks failed
CI / Unit & Component Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / E2E Tests (push) Has been cancelled
CI / Unit & Component Tests (pull_request) Has been cancelled
CI / Backend Unit Tests (pull_request) Has been cancelled
CI / E2E Tests (pull_request) Has been cancelled
Some checks failed
CI / Unit & Component Tests (push) Has been cancelled
CI / Backend Unit Tests (push) Has been cancelled
CI / E2E Tests (push) Has been cancelled
CI / Unit & Component Tests (pull_request) Has been cancelled
CI / Backend Unit Tests (pull_request) Has been cancelled
CI / E2E Tests (pull_request) Has been cancelled
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -110,14 +110,15 @@ public class DocumentController {
|
||||
private static final Set<String> ALLOWED_CONTENT_TYPES = Set.of(
|
||||
"application/pdf", "image/jpeg", "image/png", "image/tiff");
|
||||
|
||||
public record QuickUploadResult(List<Document> created, List<String> errors) {}
|
||||
public record UploadError(String filename, String code) {}
|
||||
public record QuickUploadResult(List<Document> created, List<UploadError> errors) {}
|
||||
|
||||
@PostMapping(value = "/quick-upload", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
|
||||
@RequirePermission(Permission.WRITE_ALL)
|
||||
public QuickUploadResult quickUpload(
|
||||
@RequestPart(value = "files", required = false) List<MultipartFile> files) {
|
||||
List<Document> created = new ArrayList<>();
|
||||
List<String> errors = new ArrayList<>();
|
||||
List<UploadError> 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());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -21,6 +21,9 @@ public interface DocumentRepository extends JpaRepository<Document, UUID>, JpaSp
|
||||
// Wichtig für den Abgleich beim Excel-Import & Datei-Upload
|
||||
Optional<Document> findByOriginalFilename(String originalFilename);
|
||||
|
||||
// Wie oben, gibt aber nur das erste Ergebnis zurück — sicher wenn doppelte Dateinamen existieren
|
||||
Optional<Document> findFirstByOriginalFilename(String originalFilename);
|
||||
|
||||
// Findet alle Dokumente mit einem bestimmten Status
|
||||
// z.B. um alle offenen "PLACEHOLDER" zu finden
|
||||
List<Document> findByStatus(DocumentStatus status);
|
||||
|
||||
@@ -52,8 +52,8 @@ public class DocumentService {
|
||||
public Document storeDocument(MultipartFile file) throws IOException {
|
||||
String originalFilename = file.getOriginalFilename();
|
||||
|
||||
// 1. Check for existing record
|
||||
Optional<Document> existingDoc = documentRepository.findByOriginalFilename(originalFilename);
|
||||
// 1. Check for existing record (findFirst to survive duplicate filenames in the DB)
|
||||
Optional<Document> existingDoc = documentRepository.findFirstByOriginalFilename(originalFilename);
|
||||
Document document;
|
||||
|
||||
if (existingDoc.isPresent()) {
|
||||
|
||||
@@ -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 ────────────────────────────────────
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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.",
|
||||
|
||||
@@ -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.",
|
||||
|
||||
@@ -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.",
|
||||
|
||||
@@ -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':
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user