-
Notifications
You must be signed in to change notification settings - Fork 1
fix: file upload UI issues in Swagger documentation #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -53,8 +53,8 @@ public class FileStorageService { | |||||||||||||||||||||||
| @Value("${app.elasticsearch.index:document-chunks}") | ||||||||||||||||||||||||
| private String indexName; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Supported file types for document processing | ||||||||||||||||||||||||
| private static final List<String> SUPPORTED_CONTENT_TYPES = List.of( | ||||||||||||||||||||||||
| // Supported file types for document processing (canonical) | ||||||||||||||||||||||||
| private static final List<String> ALLOWED_CANONICAL_CONTENT_TYPES = List.of( | ||||||||||||||||||||||||
| "application/pdf", | ||||||||||||||||||||||||
| "text/markdown", | ||||||||||||||||||||||||
| "text/plain" | ||||||||||||||||||||||||
|
|
@@ -69,7 +69,7 @@ public class FileStorageService { | |||||||||||||||||||||||
| public SourceDocument uploadFileWithMetadata(MultipartFile file) { | ||||||||||||||||||||||||
| String filename = file.getOriginalFilename(); | ||||||||||||||||||||||||
| long fileSize = file.getSize(); | ||||||||||||||||||||||||
| String contentType = file.getContentType(); | ||||||||||||||||||||||||
| String contentType = resolveContentType(file); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| log.info("📤 [UPLOAD] Starting file upload with metadata: filename={}, size={} bytes, contentType={}", | ||||||||||||||||||||||||
| filename, fileSize, contentType); | ||||||||||||||||||||||||
|
|
@@ -98,15 +98,15 @@ public SourceDocument uploadFileWithMetadata(MultipartFile file) { | |||||||||||||||||||||||
| try { | ||||||||||||||||||||||||
| // Upload file to MinIO | ||||||||||||||||||||||||
| log.debug("☁️ [UPLOAD] Step 4/5: Uploading file to MinIO: {}", filename); | ||||||||||||||||||||||||
| String objectKey = uploadFile(file); | ||||||||||||||||||||||||
| String objectKey = uploadFile(file, contentType); | ||||||||||||||||||||||||
| log.info("✅ [UPLOAD] File uploaded to MinIO successfully: {} -> {}", filename, objectKey); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Create SourceDocument entity | ||||||||||||||||||||||||
| log.debug("💾 [UPLOAD] Step 5/5: Creating database record: {}", filename); | ||||||||||||||||||||||||
| SourceDocument sourceDocument = SourceDocument.builder() | ||||||||||||||||||||||||
| .originalFilename(file.getOriginalFilename()) | ||||||||||||||||||||||||
| .fileStoragePath(objectKey) | ||||||||||||||||||||||||
| .fileType(determineFileType(file.getContentType())) | ||||||||||||||||||||||||
| .fileType(determineFileType(contentType)) | ||||||||||||||||||||||||
| .fileSize(file.getSize()) | ||||||||||||||||||||||||
| .fileChecksum(fileChecksum) | ||||||||||||||||||||||||
| .ingestionStatus(IngestionStatus.PENDING) | ||||||||||||||||||||||||
|
|
@@ -139,7 +139,7 @@ public SourceDocument uploadFileWithMetadata(MultipartFile file) { | |||||||||||||||||||||||
| * @param file the multipart file to upload | ||||||||||||||||||||||||
| * @return the object key where the file was stored | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| public String uploadFile(MultipartFile file) { | ||||||||||||||||||||||||
| public String uploadFile(MultipartFile file, String resolvedContentType) { | ||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||
| // Ensure bucket exists | ||||||||||||||||||||||||
| ensureBucketExists(); | ||||||||||||||||||||||||
|
|
@@ -152,7 +152,7 @@ public String uploadFile(MultipartFile file) { | |||||||||||||||||||||||
| .bucket(minioConfig.getBucketName()) | ||||||||||||||||||||||||
| .object(objectKey) | ||||||||||||||||||||||||
| .stream(file.getInputStream(), file.getSize(), -1) | ||||||||||||||||||||||||
| .contentType(file.getContentType()) | ||||||||||||||||||||||||
| .contentType(resolvedContentType) | ||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| minioClient.putObject(putObjectArgs); | ||||||||||||||||||||||||
|
|
@@ -562,18 +562,23 @@ private void validateFile(MultipartFile file) { | |||||||||||||||||||||||
| "File size exceeds maximum limit of 100MB"); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| String contentType = file.getContentType(); | ||||||||||||||||||||||||
| if (contentType == null || !SUPPORTED_CONTENT_TYPES.contains(contentType)) { | ||||||||||||||||||||||||
| throw new BusinessException(ErrorCode.UNSUPPORTED_MEDIA_TYPE, | ||||||||||||||||||||||||
| "Unsupported file type. Supported types: PDF, Markdown, and plain text files."); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| String filename = file.getOriginalFilename(); | ||||||||||||||||||||||||
| if (filename == null || filename.trim().isEmpty()) { | ||||||||||||||||||||||||
| throw new BusinessException(ErrorCode.INVALID_REQUEST, "Filename cannot be empty"); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| log.debug("✅ [UPLOAD] File validation passed: filename={}", filename); | ||||||||||||||||||||||||
| // Resolve canonical content type from both content-type header and filename extension | ||||||||||||||||||||||||
| String resolvedContentType = resolveContentType(file); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Check if the resolved content type is supported | ||||||||||||||||||||||||
| if (!ALLOWED_CANONICAL_CONTENT_TYPES.contains(resolvedContentType)) { | ||||||||||||||||||||||||
| log.debug("❌ [UPLOAD] Unsupported content type: filename={}, original={}, resolved={}", | ||||||||||||||||||||||||
| filename, file.getContentType(), resolvedContentType); | ||||||||||||||||||||||||
| throw new BusinessException(ErrorCode.UNSUPPORTED_MEDIA_TYPE, | ||||||||||||||||||||||||
| "Unsupported file type. Supported types: PDF, Markdown, and plain text files."); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| log.debug("✅ [UPLOAD] File validation passed: filename={}, resolved_type={}", filename, resolvedContentType); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
|
|
@@ -612,6 +617,46 @@ private String determineFileType(String contentType) { | |||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Resolves the effective content type for the uploaded file. | ||||||||||||||||||||||||
| * If the inbound content type is null or generic (e.g., application/octet-stream), | ||||||||||||||||||||||||
| * infer from the filename extension and normalize to a canonical, allowed type. | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| private String resolveContentType(MultipartFile file) { | ||||||||||||||||||||||||
| String inbound = file.getContentType(); | ||||||||||||||||||||||||
| // If clearly an allowed canonical type, return as-is | ||||||||||||||||||||||||
| if (ALLOWED_CANONICAL_CONTENT_TYPES.contains(inbound)) { | ||||||||||||||||||||||||
| return inbound; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Try to infer from file extension for generic or alternate types | ||||||||||||||||||||||||
| String filename = file.getOriginalFilename(); | ||||||||||||||||||||||||
| String ext = (filename == null) ? null : getFileExtension(filename).toLowerCase(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (ext != null) { | ||||||||||||||||||||||||
| return switch (ext) { | ||||||||||||||||||||||||
| case "pdf" -> "application/pdf"; | ||||||||||||||||||||||||
| case "md", "markdown" -> "text/markdown"; | ||||||||||||||||||||||||
| case "txt" -> "text/plain"; | ||||||||||||||||||||||||
| default -> (inbound != null ? inbound : "application/octet-stream"); | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Fallback to inbound or generic | ||||||||||||||||||||||||
| return inbound != null ? inbound : "application/octet-stream"; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| return inbound != null ? inbound : "application/octet-stream"; | |
| default -> { | |
| log.warn("Unknown file extension '{}', treating as unsupported type. Inbound content type: '{}'", ext, inbound); | |
| yield "unsupported/unknown"; | |
| } | |
| }; | |
| } | |
| // Fallback to unsupported type if extension is missing | |
| log.warn("Unable to determine file extension for filename '{}', treating as unsupported type. Inbound content type: '{}'", filename, inbound); | |
| return "unsupported/unknown"; |
Copilot
AI
Aug 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition lastDot == filename.length() - 1 will incorrectly return an empty string for files ending with a dot (e.g., 'file.'). This should return the empty string only when there's no extension, but a file ending with a dot should be handled differently or treated as having no valid extension.
| if (lastDot == -1 || lastDot == filename.length() - 1) { | |
| // No dot, or dot is the first character (dotfile), or dot is the last character (ends with dot) | |
| if (lastDot <= 0 || lastDot == filename.length() - 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing capitalization: 'the system' should be 'The system' as it starts a new sentence.