From 2db58afedfe9fb5994f749c22737b77e1d2bb25b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 17:36:36 +0000 Subject: [PATCH] Add original_filename parameter to preserve user's filename Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/files.py | 8 +- app/tasks/convert_to_pdf.py | 16 +- app/tasks/process_document.py | 10 +- tests/test_original_filename_preservation.py | 210 +++++++++++++++++++ 4 files changed, 235 insertions(+), 9 deletions(-) create mode 100644 tests/test_original_filename_preservation.py diff --git a/app/api/files.py b/app/api/files.py index c2df2a41..3ea4e98b 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -472,20 +472,20 @@ async def ui_upload(request: Request, file: UploadFile = File(...)): if is_pdf: # 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}") 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 - 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}") 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 - 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}") else: # For any other file type, attempt conversion but log a warning 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 { "task_id": task.id, diff --git a/app/tasks/convert_to_pdf.py b/app/tasks/convert_to_pdf.py index 1631ea1f..5ac29515 100644 --- a/app/tasks/convert_to_pdf.py +++ b/app/tasks/convert_to_pdf.py @@ -12,11 +12,15 @@ from app.utils import log_task_progress logger = logging.getLogger(__name__) @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. Determines the appropriate Gotenberg endpoint based on the file's MIME type. 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 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, "convert_to_pdf", "success", f"Converted to PDF: {os.path.basename(converted_file_path)}") - # Enqueue the PDF for further processing - process_document.delay(converted_file_path) + # Enqueue the PDF for further processing, preserving original filename if provided + 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 else: diff --git a/app/tasks/process_document.py b/app/tasks/process_document.py index 8b9f5921..0bd7a061 100644 --- a/app/tasks/process_document.py +++ b/app/tasks/process_document.py @@ -22,10 +22,14 @@ logger = logging.getLogger(__name__) @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. + Args: + original_local_file: Path to the file on disk + original_filename: Optional original filename (if different from path basename) + Steps: 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: @@ -51,7 +55,9 @@ def process_document(self, original_local_file: str): logger.info(f"[{task_id}] Computing file hash...") log_task_progress(task_id, "hash_file", "in_progress", "Computing file hash") 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) mime_type, _ = mimetypes.guess_type(original_local_file) if not mime_type: diff --git a/tests/test_original_filename_preservation.py b/tests/test_original_filename_preservation.py new file mode 100644 index 00000000..e3bf97a2 --- /dev/null +++ b/tests/test_original_filename_preservation.py @@ -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"