Add original_filename parameter to preserve user's filename
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+4
-4
@@ -472,20 +472,20 @@ async def ui_upload(request: Request, file: UploadFile = File(...)):
|
|||||||
|
|
||||||
if is_pdf:
|
if is_pdf:
|
||||||
# If it's a PDF, process directly
|
# If it's a PDF, process directly
|
||||||
task = process_document.delay(target_path)
|
task = process_document.delay(target_path, original_filename=safe_filename)
|
||||||
logger.info(f"Enqueued PDF for processing: {target_path}")
|
logger.info(f"Enqueued PDF for processing: {target_path}")
|
||||||
elif mime_type in IMAGE_MIME_TYPES or any(file_ext.endswith(ext) for ext in ['.jpg', '.jpeg', '.png', '.gif', '.bmp', '.tiff', '.webp', '.svg']):
|
elif mime_type in IMAGE_MIME_TYPES or any(file_ext.endswith(ext) for ext in ['.jpg', '.jpeg', '.png', '.gif', '.bmp', '.tiff', '.webp', '.svg']):
|
||||||
# If it's an image, convert to PDF first
|
# If it's an image, convert to PDF first
|
||||||
task = convert_to_pdf.delay(target_path)
|
task = convert_to_pdf.delay(target_path, original_filename=safe_filename)
|
||||||
logger.info(f"Enqueued image for PDF conversion: {target_path}")
|
logger.info(f"Enqueued image for PDF conversion: {target_path}")
|
||||||
elif mime_type in ALLOWED_MIME_TYPES or any(file_ext.endswith(ext) for ext in ['.doc', '.docx', '.xls', '.xlsx', '.ppt', '.pptx', '.odt', '.ods', '.odp', '.rtf', '.txt', '.csv']):
|
elif mime_type in ALLOWED_MIME_TYPES or any(file_ext.endswith(ext) for ext in ['.doc', '.docx', '.xls', '.xlsx', '.ppt', '.pptx', '.odt', '.ods', '.odp', '.rtf', '.txt', '.csv']):
|
||||||
# If it's an office document, convert to PDF first
|
# If it's an office document, convert to PDF first
|
||||||
task = convert_to_pdf.delay(target_path)
|
task = convert_to_pdf.delay(target_path, original_filename=safe_filename)
|
||||||
logger.info(f"Enqueued office document for PDF conversion: {target_path}")
|
logger.info(f"Enqueued office document for PDF conversion: {target_path}")
|
||||||
else:
|
else:
|
||||||
# For any other file type, attempt conversion but log a warning
|
# For any other file type, attempt conversion but log a warning
|
||||||
logger.warning(f"Unsupported MIME type {mime_type} for {target_path}, attempting conversion")
|
logger.warning(f"Unsupported MIME type {mime_type} for {target_path}, attempting conversion")
|
||||||
task = convert_to_pdf.delay(target_path)
|
task = convert_to_pdf.delay(target_path, original_filename=safe_filename)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"task_id": task.id,
|
"task_id": task.id,
|
||||||
|
|||||||
@@ -12,11 +12,15 @@ from app.utils import log_task_progress
|
|||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
@shared_task(bind=True)
|
@shared_task(bind=True)
|
||||||
def convert_to_pdf(self, file_path):
|
def convert_to_pdf(self, file_path, original_filename=None):
|
||||||
"""
|
"""
|
||||||
Converts a file to PDF using Gotenberg's API.
|
Converts a file to PDF using Gotenberg's API.
|
||||||
Determines the appropriate Gotenberg endpoint based on the file's MIME type.
|
Determines the appropriate Gotenberg endpoint based on the file's MIME type.
|
||||||
On success, saves the PDF locally and enqueues it for processing.
|
On success, saves the PDF locally and enqueues it for processing.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
file_path: Path to the file to convert
|
||||||
|
original_filename: Optional original filename (if different from path basename)
|
||||||
"""
|
"""
|
||||||
task_id = self.request.id
|
task_id = self.request.id
|
||||||
logger.info(f"[{task_id}] Starting PDF conversion: {file_path}")
|
logger.info(f"[{task_id}] Starting PDF conversion: {file_path}")
|
||||||
@@ -174,8 +178,14 @@ def convert_to_pdf(self, file_path):
|
|||||||
log_task_progress(task_id, "call_gotenberg", "success", "PDF conversion successful")
|
log_task_progress(task_id, "call_gotenberg", "success", "PDF conversion successful")
|
||||||
log_task_progress(task_id, "convert_to_pdf", "success", f"Converted to PDF: {os.path.basename(converted_file_path)}")
|
log_task_progress(task_id, "convert_to_pdf", "success", f"Converted to PDF: {os.path.basename(converted_file_path)}")
|
||||||
|
|
||||||
# Enqueue the PDF for further processing
|
# Enqueue the PDF for further processing, preserving original filename if provided
|
||||||
process_document.delay(converted_file_path)
|
if original_filename:
|
||||||
|
# Change extension to .pdf for the original filename
|
||||||
|
original_base = os.path.splitext(original_filename)[0]
|
||||||
|
pdf_original_filename = f"{original_base}.pdf"
|
||||||
|
process_document.delay(converted_file_path, original_filename=pdf_original_filename)
|
||||||
|
else:
|
||||||
|
process_document.delay(converted_file_path)
|
||||||
|
|
||||||
return converted_file_path
|
return converted_file_path
|
||||||
else:
|
else:
|
||||||
|
|||||||
@@ -22,10 +22,14 @@ logger = logging.getLogger(__name__)
|
|||||||
|
|
||||||
|
|
||||||
@celery.task(base=BaseTaskWithRetry, bind=True)
|
@celery.task(base=BaseTaskWithRetry, bind=True)
|
||||||
def process_document(self, original_local_file: str):
|
def process_document(self, original_local_file: str, original_filename: str = None):
|
||||||
"""
|
"""
|
||||||
Process a document file and trigger appropriate text extraction.
|
Process a document file and trigger appropriate text extraction.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
original_local_file: Path to the file on disk
|
||||||
|
original_filename: Optional original filename (if different from path basename)
|
||||||
|
|
||||||
Steps:
|
Steps:
|
||||||
1. Check if we have a FileRecord entry (via SHA-256 hash). If found, skip re-processing.
|
1. Check if we have a FileRecord entry (via SHA-256 hash). If found, skip re-processing.
|
||||||
2. If not found, insert a new DB row and continue with the pipeline:
|
2. If not found, insert a new DB row and continue with the pipeline:
|
||||||
@@ -51,7 +55,9 @@ def process_document(self, original_local_file: str):
|
|||||||
logger.info(f"[{task_id}] Computing file hash...")
|
logger.info(f"[{task_id}] Computing file hash...")
|
||||||
log_task_progress(task_id, "hash_file", "in_progress", "Computing file hash")
|
log_task_progress(task_id, "hash_file", "in_progress", "Computing file hash")
|
||||||
filehash = hash_file(original_local_file)
|
filehash = hash_file(original_local_file)
|
||||||
original_filename = os.path.basename(original_local_file)
|
# Use provided original_filename or fall back to basename of path
|
||||||
|
if original_filename is None:
|
||||||
|
original_filename = os.path.basename(original_local_file)
|
||||||
file_size = os.path.getsize(original_local_file)
|
file_size = os.path.getsize(original_local_file)
|
||||||
mime_type, _ = mimetypes.guess_type(original_local_file)
|
mime_type, _ = mimetypes.guess_type(original_local_file)
|
||||||
if not mime_type:
|
if not mime_type:
|
||||||
|
|||||||
@@ -0,0 +1,210 @@
|
|||||||
|
"""
|
||||||
|
Unit tests for verifying that original filenames are preserved during upload.
|
||||||
|
|
||||||
|
These tests verify the fix for the issue where uploaded files do not maintain
|
||||||
|
their original file names.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import os
|
||||||
|
import pytest
|
||||||
|
from unittest.mock import patch, MagicMock
|
||||||
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
|
from app.tasks.process_document import process_document
|
||||||
|
from app.models import FileRecord
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
@pytest.mark.requires_db
|
||||||
|
def test_process_document_preserves_original_filename_parameter(db_session, tmp_path):
|
||||||
|
"""
|
||||||
|
Test that process_document correctly uses the original_filename parameter
|
||||||
|
when provided, instead of extracting it from the file path.
|
||||||
|
"""
|
||||||
|
# Create a test PDF file with a UUID-based name
|
||||||
|
test_pdf = tmp_path / "e64b2825-9ff2-486b-aff1-08af2957140b.pdf"
|
||||||
|
pdf_content = b"""%PDF-1.4
|
||||||
|
1 0 obj
|
||||||
|
<<
|
||||||
|
/Type /Catalog
|
||||||
|
/Pages 2 0 R
|
||||||
|
>>
|
||||||
|
endobj
|
||||||
|
2 0 obj
|
||||||
|
<<
|
||||||
|
/Type /Pages
|
||||||
|
/Kids [3 0 R]
|
||||||
|
/Count 1
|
||||||
|
>>
|
||||||
|
endobj
|
||||||
|
3 0 obj
|
||||||
|
<<
|
||||||
|
/Type /Page
|
||||||
|
/Parent 2 0 R
|
||||||
|
/MediaBox [0 0 612 792]
|
||||||
|
/Resources <<
|
||||||
|
/Font <<
|
||||||
|
/F1 <<
|
||||||
|
/Type /Font
|
||||||
|
/Subtype /Type1
|
||||||
|
/BaseFont /Helvetica
|
||||||
|
>>
|
||||||
|
>>
|
||||||
|
>>
|
||||||
|
/Contents 4 0 R
|
||||||
|
>>
|
||||||
|
endobj
|
||||||
|
4 0 obj
|
||||||
|
<<
|
||||||
|
/Length 44
|
||||||
|
>>
|
||||||
|
stream
|
||||||
|
BT
|
||||||
|
/F1 12 Tf
|
||||||
|
100 700 Td
|
||||||
|
(Test content) Tj
|
||||||
|
ET
|
||||||
|
endstream
|
||||||
|
endobj
|
||||||
|
xref
|
||||||
|
0 5
|
||||||
|
0000000000 65535 f
|
||||||
|
0000000009 00000 n
|
||||||
|
0000000058 00000 n
|
||||||
|
0000000115 00000 n
|
||||||
|
0000000306 00000 n
|
||||||
|
trailer
|
||||||
|
<<
|
||||||
|
/Size 5
|
||||||
|
/Root 1 0 R
|
||||||
|
>>
|
||||||
|
startxref
|
||||||
|
399
|
||||||
|
%%EOF
|
||||||
|
"""
|
||||||
|
test_pdf.write_bytes(pdf_content)
|
||||||
|
|
||||||
|
# The original filename that the user uploaded
|
||||||
|
original_filename = "Apostille Sverige.pdf"
|
||||||
|
|
||||||
|
# Mock environment and dependencies
|
||||||
|
with patch("app.tasks.process_document.SessionLocal") as mock_session_local, \
|
||||||
|
patch("app.tasks.process_document.settings") as mock_settings, \
|
||||||
|
patch("app.tasks.process_document.log_task_progress"), \
|
||||||
|
patch("app.tasks.process_document.extract_metadata_with_gpt") as mock_extract:
|
||||||
|
|
||||||
|
# Setup mocks
|
||||||
|
mock_settings.workdir = str(tmp_path)
|
||||||
|
mock_session_local.return_value.__enter__.return_value = db_session
|
||||||
|
mock_session_local.return_value.__exit__.return_value = None
|
||||||
|
mock_extract.delay = MagicMock()
|
||||||
|
|
||||||
|
# Call the task with the original_filename parameter
|
||||||
|
result = process_document.run(str(test_pdf), original_filename=original_filename)
|
||||||
|
|
||||||
|
# Verify that the task completed successfully
|
||||||
|
assert "file_id" in result
|
||||||
|
assert result["status"] == "Text extracted locally"
|
||||||
|
|
||||||
|
# Verify that a FileRecord was created with the correct original filename
|
||||||
|
file_record = db_session.query(FileRecord).first()
|
||||||
|
assert file_record is not None
|
||||||
|
|
||||||
|
# This is the key assertion - the original filename should be preserved
|
||||||
|
assert file_record.original_filename == original_filename
|
||||||
|
# The filename should NOT be the UUID-based filename
|
||||||
|
assert file_record.original_filename != "e64b2825-9ff2-486b-aff1-08af2957140b.pdf"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
@pytest.mark.requires_db
|
||||||
|
def test_process_document_fallback_to_basename_when_no_parameter(db_session, tmp_path):
|
||||||
|
"""
|
||||||
|
Test that process_document falls back to extracting filename from path
|
||||||
|
when original_filename parameter is not provided (backward compatibility).
|
||||||
|
"""
|
||||||
|
# Create a test PDF file
|
||||||
|
test_pdf = tmp_path / "test_document.pdf"
|
||||||
|
pdf_content = b"""%PDF-1.4
|
||||||
|
1 0 obj
|
||||||
|
<<
|
||||||
|
/Type /Catalog
|
||||||
|
/Pages 2 0 R
|
||||||
|
>>
|
||||||
|
endobj
|
||||||
|
2 0 obj
|
||||||
|
<<
|
||||||
|
/Type /Pages
|
||||||
|
/Kids [3 0 R]
|
||||||
|
/Count 1
|
||||||
|
>>
|
||||||
|
endobj
|
||||||
|
3 0 obj
|
||||||
|
<<
|
||||||
|
/Type /Page
|
||||||
|
/Parent 2 0 R
|
||||||
|
/MediaBox [0 0 612 792]
|
||||||
|
/Resources <<
|
||||||
|
/Font <<
|
||||||
|
/F1 <<
|
||||||
|
/Type /Font
|
||||||
|
/Subtype /Type1
|
||||||
|
/BaseFont /Helvetica
|
||||||
|
>>
|
||||||
|
>>
|
||||||
|
>>
|
||||||
|
/Contents 4 0 R
|
||||||
|
>>
|
||||||
|
endobj
|
||||||
|
4 0 obj
|
||||||
|
<<
|
||||||
|
/Length 44
|
||||||
|
>>
|
||||||
|
stream
|
||||||
|
BT
|
||||||
|
/F1 12 Tf
|
||||||
|
100 700 Td
|
||||||
|
(Test content) Tj
|
||||||
|
ET
|
||||||
|
endstream
|
||||||
|
endobj
|
||||||
|
xref
|
||||||
|
0 5
|
||||||
|
0000000000 65535 f
|
||||||
|
0000000009 00000 n
|
||||||
|
0000000058 00000 n
|
||||||
|
0000000115 00000 n
|
||||||
|
0000000306 00000 n
|
||||||
|
trailer
|
||||||
|
<<
|
||||||
|
/Size 5
|
||||||
|
/Root 1 0 R
|
||||||
|
>>
|
||||||
|
startxref
|
||||||
|
399
|
||||||
|
%%EOF
|
||||||
|
"""
|
||||||
|
test_pdf.write_bytes(pdf_content)
|
||||||
|
|
||||||
|
# Mock environment and dependencies
|
||||||
|
with patch("app.tasks.process_document.SessionLocal") as mock_session_local, \
|
||||||
|
patch("app.tasks.process_document.settings") as mock_settings, \
|
||||||
|
patch("app.tasks.process_document.log_task_progress"), \
|
||||||
|
patch("app.tasks.process_document.extract_metadata_with_gpt") as mock_extract:
|
||||||
|
|
||||||
|
# Setup mocks
|
||||||
|
mock_settings.workdir = str(tmp_path)
|
||||||
|
mock_session_local.return_value.__enter__.return_value = db_session
|
||||||
|
mock_session_local.return_value.__exit__.return_value = None
|
||||||
|
mock_extract.delay = MagicMock()
|
||||||
|
|
||||||
|
# Call the task WITHOUT the original_filename parameter (old behavior)
|
||||||
|
result = process_document.run(str(test_pdf))
|
||||||
|
|
||||||
|
# Verify that the task completed successfully
|
||||||
|
assert "file_id" in result
|
||||||
|
|
||||||
|
# Verify that the filename was extracted from the path
|
||||||
|
file_record = db_session.query(FileRecord).first()
|
||||||
|
assert file_record is not None
|
||||||
|
assert file_record.original_filename == "test_document.pdf"
|
||||||
Reference in New Issue
Block a user