From 7d24b7c668a8ff72f4569d1f83e8b82ccdae7f0b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Feb 2026 17:37:50 +0000 Subject: [PATCH] Apply code formatting and fix linting issues Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/files.py | 355 +++++++++---------- app/tasks/convert_to_pdf.py | 163 +++++---- app/tasks/process_document.py | 39 +- tests/test_original_filename_preservation.py | 31 +- 4 files changed, 288 insertions(+), 300 deletions(-) diff --git a/app/api/files.py b/app/api/files.py index 3ea4e98b..4f28fa6a 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -1,28 +1,31 @@ """ File-related API endpoints """ -from fastapi import APIRouter, Request, HTTPException, Depends, UploadFile, File, Query -from sqlalchemy.orm import Session -from sqlalchemy import desc, asc, or_, func -from typing import Optional, List + import logging +import mimetypes import os import uuid -import mimetypes +from typing import List, Optional + +from fastapi import APIRouter, Depends, File, HTTPException, Query, Request, UploadFile +from sqlalchemy import asc, desc, or_ +from sqlalchemy.orm import Session -from app.auth import require_login -from app.models import FileRecord, ProcessingLog -from app.config import settings from app.api.common import get_db -from app.tasks.process_document import process_document +from app.auth import require_login +from app.config import settings +from app.models import FileRecord, ProcessingLog from app.tasks.convert_to_pdf import convert_to_pdf -from app.utils.file_status import get_file_processing_status, get_files_processing_status +from app.tasks.process_document import process_document +from app.utils.file_status import get_files_processing_status # Set up logging logger = logging.getLogger(__name__) router = APIRouter() + @router.get("/files") @require_login def list_files_api( @@ -30,16 +33,18 @@ def list_files_api( db: Session = Depends(get_db), page: int = Query(1, ge=1, description="Page number"), per_page: int = Query(50, ge=1, le=200, description="Items per page"), - sort_by: str = Query("created_at", description="Sort field: id, original_filename, file_size, mime_type, created_at, status"), + sort_by: str = Query( + "created_at", description="Sort field: id, original_filename, file_size, mime_type, created_at, status" + ), sort_order: str = Query("desc", description="Sort order: asc or desc"), search: Optional[str] = Query(None, description="Search in filename"), mime_type: Optional[str] = Query(None, description="Filter by MIME type"), - status: Optional[str] = Query(None, description="Filter by processing status") + status: Optional[str] = Query(None, description="Filter by processing status"), ): """ Returns a paginated JSON list of FileRecord entries with processing status. Supports server-side sorting, filtering, and searching. - + Query Parameters: - page: Page number (default: 1) - per_page: Items per page (default: 50, max: 200) @@ -48,7 +53,7 @@ def list_files_api( - search: Search in filename - mime_type: Filter by MIME type - status: Filter by processing status (pending, processing, completed, failed) - + Example response: { "files": [...], @@ -62,15 +67,15 @@ def list_files_api( """ # Start with base query query = db.query(FileRecord) - + # Apply search filter if search: query = query.filter(FileRecord.original_filename.ilike(f"%{search}%")) - + # Apply MIME type filter if mime_type: query = query.filter(FileRecord.mime_type == mime_type) - + # Apply status filter (before pagination for correct counts) if status: # Subquery to get file IDs matching the status @@ -80,87 +85,78 @@ def list_files_api( query = query.filter(~FileRecord.id.in_(subq)) elif status == "processing": # Files with in_progress logs - subq = db.query(ProcessingLog.file_id).filter( - ProcessingLog.status == "in_progress" - ).distinct() + subq = db.query(ProcessingLog.file_id).filter(ProcessingLog.status == "in_progress").distinct() query = query.filter(FileRecord.id.in_(subq)) elif status == "failed": # Files with failure logs - subq = db.query(ProcessingLog.file_id).filter( - ProcessingLog.status == "failure" - ).distinct() + subq = db.query(ProcessingLog.file_id).filter(ProcessingLog.status == "failure").distinct() query = query.filter(FileRecord.id.in_(subq)) elif status == "completed": # Files with success logs but no failures or in_progress - success_files = db.query(ProcessingLog.file_id).filter( - ProcessingLog.status == "success" - ).distinct().subquery() - - failed_files = db.query(ProcessingLog.file_id).filter( - or_(ProcessingLog.status == "failure", ProcessingLog.status == "in_progress") - ).distinct().subquery() - - query = query.filter( - FileRecord.id.in_(db.query(success_files.c.file_id)) - ).filter( + success_files = ( + db.query(ProcessingLog.file_id).filter(ProcessingLog.status == "success").distinct().subquery() + ) + + failed_files = ( + db.query(ProcessingLog.file_id) + .filter(or_(ProcessingLog.status == "failure", ProcessingLog.status == "in_progress")) + .distinct() + .subquery() + ) + + query = query.filter(FileRecord.id.in_(db.query(success_files.c.file_id))).filter( ~FileRecord.id.in_(db.query(failed_files.c.file_id)) ) - + # Get total count before pagination (after all filters) total_items = query.count() - + # Apply sorting sort_column = { "id": FileRecord.id, "original_filename": FileRecord.original_filename, "file_size": FileRecord.file_size, "mime_type": FileRecord.mime_type, - "created_at": FileRecord.created_at + "created_at": FileRecord.created_at, }.get(sort_by, FileRecord.created_at) - + if sort_order == "asc": query = query.order_by(asc(sort_column)) else: query = query.order_by(desc(sort_column)) - + # Apply pagination offset = (page - 1) * per_page files = query.offset(offset).limit(per_page).all() - + # Get processing status for all files efficiently file_ids = [f.id for f in files] statuses = get_files_processing_status(db, file_ids) - + # Build result with processing status result = [] for f in files: - result.append({ - "id": f.id, - "filehash": f.filehash, - "original_filename": f.original_filename, - "local_filename": f.local_filename, - "file_size": f.file_size, - "mime_type": f.mime_type, - "created_at": f.created_at.isoformat() if f.created_at else None, - "processing_status": statuses.get(f.id, { - "status": "pending", - "last_step": None, - "has_errors": False, - "total_steps": 0 - }) - }) - + result.append( + { + "id": f.id, + "filehash": f.filehash, + "original_filename": f.original_filename, + "local_filename": f.local_filename, + "file_size": f.file_size, + "mime_type": f.mime_type, + "created_at": f.created_at.isoformat() if f.created_at else None, + "processing_status": statuses.get( + f.id, {"status": "pending", "last_step": None, "has_errors": False, "total_steps": 0} + ), + } + ) + # Calculate pagination info total_pages = (total_items + per_page - 1) // per_page - + return { "files": result, - "pagination": { - "page": page, - "per_page": per_page, - "total_items": total_items, - "total_pages": total_pages - } + "pagination": {"page": page, "per_page": per_page, "total_items": total_items, "total_pages": total_pages}, } @@ -170,8 +166,8 @@ def _get_file_processing_status(db: Session, file_id: int) -> dict: Kept for backward compatibility with file detail endpoint. """ from app.utils.file_status import get_file_processing_status - return get_file_processing_status(db, file_id) + return get_file_processing_status(db, file_id) @router.get("/files/{file_id}") @@ -182,38 +178,35 @@ def get_file_details(request: Request, file_id: int, db: Session = Depends(get_d """ # Find the file record file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first() - + if not file_record: - raise HTTPException( - status_code=404, - detail=f"File record with ID {file_id} not found" - ) - + raise HTTPException(status_code=404, detail=f"File record with ID {file_id} not found") + # Get processing logs - logs = db.query(ProcessingLog).filter( - ProcessingLog.file_id == file_id - ).order_by(ProcessingLog.timestamp.desc()).all() - + logs = ( + db.query(ProcessingLog).filter(ProcessingLog.file_id == file_id).order_by(ProcessingLog.timestamp.desc()).all() + ) + # Build log list log_list = [] for log in logs: - log_list.append({ - "id": log.id, - "task_id": log.task_id, - "step_name": log.step_name, - "status": log.status, - "message": log.message, - "timestamp": log.timestamp.isoformat() if log.timestamp else None - }) - + log_list.append( + { + "id": log.id, + "task_id": log.task_id, + "step_name": log.step_name, + "status": log.status, + "message": log.message, + "timestamp": log.timestamp.isoformat() if log.timestamp else None, + } + ) + # Get processing status processing_status = _get_file_processing_status(db, file_id) - + # Check if files exist on disk - files_on_disk = { - "original": os.path.exists(file_record.local_filename) if file_record.local_filename else False - } - + files_on_disk = {"original": os.path.exists(file_record.local_filename) if file_record.local_filename else False} + return { "file": { "id": file_record.id, @@ -222,13 +215,14 @@ def get_file_details(request: Request, file_id: int, db: Session = Depends(get_d "local_filename": file_record.local_filename, "file_size": file_record.file_size, "mime_type": file_record.mime_type, - "created_at": file_record.created_at.isoformat() if file_record.created_at else None + "created_at": file_record.created_at.isoformat() if file_record.created_at else None, }, "processing_status": processing_status, "logs": log_list, - "files_on_disk": files_on_disk + "files_on_disk": files_on_disk, } + @router.delete("/files/{file_id}") @require_login def delete_file_record(request: Request, file_id: int, db: Session = Depends(get_db)): @@ -238,42 +232,31 @@ def delete_file_record(request: Request, file_id: int, db: Session = Depends(get """ # Check if file deletion is allowed if not settings.allow_file_delete: - raise HTTPException( - status_code=403, - detail="File deletion is disabled in the configuration" - ) + raise HTTPException(status_code=403, detail="File deletion is disabled in the configuration") try: # Find the file record file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first() - + if not file_record: - raise HTTPException( - status_code=404, - detail=f"File record with ID {file_id} not found" - ) - + raise HTTPException(status_code=404, detail=f"File record with ID {file_id} not found") + # Log the deletion logger.info(f"Deleting file record: ID={file_id}, Filename={file_record.original_filename}") - + # Delete the record db.delete(file_record) db.commit() - - return { - "status": "success", - "message": f"File record {file_id} deleted successfully" - } - + + return {"status": "success", "message": f"File record {file_id} deleted successfully"} + except HTTPException: raise except Exception as e: db.rollback() logger.exception(f"Error deleting file record {file_id}: {str(e)}") - raise HTTPException( - status_code=500, - detail=f"Error deleting file record: {str(e)}" - ) + raise HTTPException(status_code=500, detail=f"Error deleting file record: {str(e)}") + @router.post("/files/bulk-delete") @require_login @@ -284,48 +267,39 @@ def bulk_delete_files(request: Request, file_ids: List[int], db: Session = Depen """ # Check if file deletion is allowed if not settings.allow_file_delete: - raise HTTPException( - status_code=403, - detail="File deletion is disabled in the configuration" - ) + raise HTTPException(status_code=403, detail="File deletion is disabled in the configuration") try: # Find all file records file_records = db.query(FileRecord).filter(FileRecord.id.in_(file_ids)).all() - + if not file_records: - raise HTTPException( - status_code=404, - detail="No files found with the provided IDs" - ) - + raise HTTPException(status_code=404, detail="No files found with the provided IDs") + deleted_count = len(file_records) deleted_ids = [f.id for f in file_records] - + # Log the deletion logger.info(f"Bulk deleting {deleted_count} file records: IDs={deleted_ids}") - + # Delete all records for file_record in file_records: db.delete(file_record) - + db.commit() - + return { "status": "success", "message": f"Successfully deleted {deleted_count} file records", - "deleted_ids": deleted_ids + "deleted_ids": deleted_ids, } - + except HTTPException: raise except Exception as e: db.rollback() logger.exception(f"Error bulk deleting file records: {str(e)}") - raise HTTPException( - status_code=500, - detail=f"Error bulk deleting file records: {str(e)}" - ) + raise HTTPException(status_code=500, detail=f"Error bulk deleting file records: {str(e)}") @router.post("/files/bulk-reprocess") @@ -337,63 +311,56 @@ def bulk_reprocess_files(request: Request, file_ids: List[int], db: Session = De try: # Find all file records file_records = db.query(FileRecord).filter(FileRecord.id.in_(file_ids)).all() - + if not file_records: - raise HTTPException( - status_code=404, - detail="No files found with the provided IDs" - ) - + raise HTTPException(status_code=404, detail="No files found with the provided IDs") + task_ids = [] processed_files = [] errors = [] - + for file_record in file_records: try: # Check if local file exists if not file_record.local_filename or not os.path.exists(file_record.local_filename): - errors.append({ - "file_id": file_record.id, - "filename": file_record.original_filename, - "error": "Local file not found" - }) + errors.append( + { + "file_id": file_record.id, + "filename": file_record.original_filename, + "error": "Local file not found", + } + ) continue - + # Queue the file for processing task = process_document.delay(file_record.local_filename) task_ids.append(task.id) - processed_files.append({ - "file_id": file_record.id, - "filename": file_record.original_filename, - "task_id": task.id - }) - - logger.info(f"Reprocessing file: ID={file_record.id}, Filename={file_record.original_filename}, TaskID={task.id}") - + processed_files.append( + {"file_id": file_record.id, "filename": file_record.original_filename, "task_id": task.id} + ) + + logger.info( + f"Reprocessing file: ID={file_record.id}, " + f"Filename={file_record.original_filename}, TaskID={task.id}" + ) + except Exception as e: logger.exception(f"Error reprocessing file {file_record.id}: {str(e)}") - errors.append({ - "file_id": file_record.id, - "filename": file_record.original_filename, - "error": str(e) - }) - + errors.append({"file_id": file_record.id, "filename": file_record.original_filename, "error": str(e)}) + return { "status": "success" if processed_files else "error", "message": f"Successfully queued {len(processed_files)} files for reprocessing", "processed_files": processed_files, "errors": errors if errors else None, - "task_ids": task_ids + "task_ids": task_ids, } - + except HTTPException: raise except Exception as e: logger.exception(f"Error bulk reprocessing files: {str(e)}") - raise HTTPException( - status_code=500, - detail=f"Error bulk reprocessing files: {str(e)}" - ) + raise HTTPException(status_code=500, detail=f"Error bulk reprocessing files: {str(e)}") @router.post("/ui-upload") @@ -401,10 +368,10 @@ def bulk_reprocess_files(request: Request, file_ids: List[int], db: Session = De async def ui_upload(request: Request, file: UploadFile = File(...)): """Endpoint to accept a user-uploaded file and enqueue it for processing.""" workdir = settings.workdir - + # Extract just the filename without any path components to prevent path traversal safe_filename = os.path.basename(file.filename) - + # Generate a unique filename with UUID to prevent overwriting and filename conflicts unique_id = str(uuid.uuid4()) # Keep the original extension if present @@ -413,34 +380,28 @@ async def ui_upload(request: Request, file: UploadFile = File(...)): target_filename = f"{unique_id}.{file_extension}" else: target_filename = unique_id - + # Store both the safe original name and the unique name target_path = os.path.join(workdir, target_filename) - + try: with open(target_path, "wb") as f: content = await file.read() f.write(content) except Exception as e: - raise HTTPException( - status_code=500, - detail=f"Failed to save file: {e}" - ) + raise HTTPException(status_code=500, detail=f"Failed to save file: {e}") # Log the mapping between original and safe filename logger.info(f"Saved uploaded file '{safe_filename}' as '{target_filename}'") - + # Check file size file_size = os.path.getsize(target_path) max_size = 500 * 1024 * 1024 # 500MB if file_size > max_size: # Remove the file if it's too large os.remove(target_path) - raise HTTPException( - status_code=413, - detail=f"File too large: {file_size} bytes (max {max_size} bytes)" - ) - + raise HTTPException(status_code=413, detail=f"File too large: {file_size} bytes (max {max_size} bytes)") + # Same set of allowed file types as in the IMAP task ALLOWED_MIME_TYPES = { "application/pdf", @@ -455,30 +416,40 @@ async def ui_upload(request: Request, file: UploadFile = File(...)): "application/rtf", "text/rtf", } - + # Image MIME types that need conversion IMAGE_MIME_TYPES = { - 'image/jpeg', 'image/jpg', 'image/png', - 'image/gif', 'image/bmp', 'image/tiff', - 'image/webp', 'image/svg+xml' + "image/jpeg", + "image/jpg", + "image/png", + "image/gif", + "image/bmp", + "image/tiff", + "image/webp", + "image/svg+xml", } - + # Determine if the file is a PDF or needs conversion mime_type, _ = mimetypes.guess_type(target_path) file_ext = os.path.splitext(target_path)[1].lower() - + # Check if it's a PDF by extension or MIME type is_pdf = file_ext == ".pdf" or mime_type == "application/pdf" - + if is_pdf: # If it's a PDF, process directly 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']): + 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, 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']): + 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, original_filename=safe_filename) logger.info(f"Enqueued office document for PDF conversion: {target_path}") @@ -486,10 +457,10 @@ async def ui_upload(request: Request, file: UploadFile = File(...)): # 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, original_filename=safe_filename) - + return { - "task_id": task.id, - "status": "queued", + "task_id": task.id, + "status": "queued", "original_filename": safe_filename, - "stored_filename": target_filename + "stored_filename": target_filename, } diff --git a/app/tasks/convert_to_pdf.py b/app/tasks/convert_to_pdf.py index 5ac29515..3d52ce5b 100644 --- a/app/tasks/convert_to_pdf.py +++ b/app/tasks/convert_to_pdf.py @@ -1,23 +1,25 @@ #!/usr/bin/env python3 -import os -import requests import logging import mimetypes -import json +import os + +import requests from celery import shared_task + from app.config import settings from app.tasks.process_document import process_document from app.utils import log_task_progress logger = logging.getLogger(__name__) + @shared_task(bind=True) 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) @@ -25,7 +27,7 @@ def convert_to_pdf(self, file_path, original_filename=None): task_id = self.request.id logger.info(f"[{task_id}] Starting PDF conversion: {file_path}") log_task_progress(task_id, "convert_to_pdf", "in_progress", f"Converting file: {os.path.basename(file_path)}") - + gotenberg_url = getattr(settings, "gotenberg_url", None) if not gotenberg_url: logger.error(f"[{task_id}] Gotenberg URL is not configured in settings.") @@ -42,68 +44,89 @@ def convert_to_pdf(self, file_path, original_filename=None): endpoint = None form_data = {} files = {} - + # Dictionary mapping file extensions to their handlers OFFICE_EXTENSIONS = { - '.doc', '.docx', '.docm', '.dot', '.dotx', '.dotm', # Word - '.xls', '.xlsx', '.xlsm', '.xlsb', '.xlt', '.xltx', '.xlw', # Excel - '.ppt', '.pptx', '.pptm', '.pps', '.ppsx', '.pot', '.potx', # PowerPoint - '.odt', '.ods', '.odp', '.odg', '.odf', # OpenOffice/LibreOffice - '.rtf', '.txt', '.csv', # Text formats - '.pdf', # PDF (already in PDF format but can be processed) + ".doc", + ".docx", + ".docm", + ".dot", + ".dotx", + ".dotm", # Word + ".xls", + ".xlsx", + ".xlsm", + ".xlsb", + ".xlt", + ".xltx", + ".xlw", # Excel + ".ppt", + ".pptx", + ".pptm", + ".pps", + ".ppsx", + ".pot", + ".potx", # PowerPoint + ".odt", + ".ods", + ".odp", + ".odg", + ".odf", # OpenOffice/LibreOffice + ".rtf", + ".txt", + ".csv", # Text formats + ".pdf", # PDF (already in PDF format but can be processed) } - - IMAGE_EXTENSIONS = { - '.jpg', '.jpeg', '.png', '.gif', '.bmp', '.tiff', '.tif', '.webp', '.svg' - } - - HTML_EXTENSIONS = { - '.html', '.htm' - } - + + IMAGE_EXTENSIONS = {".jpg", ".jpeg", ".png", ".gif", ".bmp", ".tiff", ".tif", ".webp", ".svg"} + + HTML_EXTENSIONS = {".html", ".htm"} + # Use LibreOffice endpoint for office documents and images - if (mime_type and 'office' in mime_type) or \ - (mime_type and 'opendocument' in mime_type) or \ - (mime_type and mime_type.startswith('image/')) or \ - file_ext in OFFICE_EXTENSIONS or \ - file_ext in IMAGE_EXTENSIONS: + if ( + (mime_type and "office" in mime_type) + or (mime_type and "opendocument" in mime_type) + or (mime_type and mime_type.startswith("image/")) + or file_ext in OFFICE_EXTENSIONS + or file_ext in IMAGE_EXTENSIONS + ): endpoint = f"{gotenberg_url}/forms/libreoffice/convert" - files = {'files': (os.path.basename(file_path), open(file_path, 'rb'))} - + files = {"files": (os.path.basename(file_path), open(file_path, "rb"))} + # Add some quality settings for better PDF output form_data = { - 'landscape': 'false', - 'exportBookmarks': 'true', - 'exportNotes': 'false', - 'losslessImageCompression': 'true', # Use lossless compression for images - 'pdfa': 'PDF/A-2b', # Produce PDF/A-2b compatible output + "landscape": "false", + "exportBookmarks": "true", + "exportNotes": "false", + "losslessImageCompression": "true", # Use lossless compression for images + "pdfa": "PDF/A-2b", # Produce PDF/A-2b compatible output } - + # Use Chromium endpoint for HTML documents - elif (mime_type and mime_type == 'text/html') or file_ext in HTML_EXTENSIONS: + elif (mime_type and mime_type == "text/html") or file_ext in HTML_EXTENSIONS: endpoint = f"{gotenberg_url}/forms/chromium/convert/html" # Gotenberg requires the form field to be exactly 'index.html' # The content filename doesn't matter, just the form field key - files = {'index.html': ('index.html', open(file_path, 'rb'))} - + files = {"index.html": ("index.html", open(file_path, "rb"))} + # Add options for better HTML to PDF conversion form_data = { - 'paperWidth': '8.27', # A4 width in inches - 'paperHeight': '11.7', # A4 height in inches - 'marginTop': '0.4', - 'marginBottom': '0.4', - 'marginLeft': '0.4', - 'marginRight': '0.4', - 'printBackground': 'true', - 'preferCssPageSize': 'false', - 'waitDelay': '2s', # Wait for JavaScript to execute + "paperWidth": "8.27", # A4 width in inches + "paperHeight": "11.7", # A4 height in inches + "marginTop": "0.4", + "marginBottom": "0.4", + "marginLeft": "0.4", + "marginRight": "0.4", + "printBackground": "true", + "preferCssPageSize": "false", + "waitDelay": "2s", # Wait for JavaScript to execute } - + # Use Markdown route for markdown files - elif (mime_type and mime_type in ['text/markdown', 'text/x-markdown']) or file_ext in ['.md', '.markdown']: + elif (mime_type and mime_type in ["text/markdown", "text/x-markdown"]) or file_ext in [".md", ".markdown"]: # For Markdown, we need both the markdown file and an HTML wrapper endpoint = f"{gotenberg_url}/forms/chromium/convert/markdown" - + # Create a simple HTML wrapper for the markdown # IMPORTANT: The filename in the template must match the key used in the files dictionary markdown_filename = os.path.basename(file_path) @@ -125,35 +148,35 @@ def convert_to_pdf(self, file_path, original_filename=None): {{{{ toHTML "{markdown_filename}" }}}} """ - + # Create a temporary HTML wrapper file wrapper_path = os.path.join(os.path.dirname(file_path), "md_wrapper.html") - with open(wrapper_path, 'w') as f: + with open(wrapper_path, "w") as f: f.write(html_wrapper) - + try: files = { - 'index.html': ('index.html', open(wrapper_path, 'rb')), - markdown_filename: (markdown_filename, open(file_path, 'rb')) + "index.html": ("index.html", open(wrapper_path, "rb")), + markdown_filename: (markdown_filename, open(file_path, "rb")), } - + form_data = { - 'paperWidth': '8.27', # A4 width in inches - 'paperHeight': '11.7', # A4 height in inches - 'marginTop': '0.4', - 'marginBottom': '0.4', - 'marginLeft': '0.4', - 'marginRight': '0.4', + "paperWidth": "8.27", # A4 width in inches + "paperHeight": "11.7", # A4 height in inches + "marginTop": "0.4", + "marginBottom": "0.4", + "marginLeft": "0.4", + "marginRight": "0.4", } finally: # Clean up the temporary wrapper file after preparing the request if os.path.exists(wrapper_path): os.remove(wrapper_path) - + # Fallback to LibreOffice for everything else else: endpoint = f"{gotenberg_url}/forms/libreoffice/convert" - files = {'files': (os.path.basename(file_path), open(file_path, 'rb'))} + files = {"files": (os.path.basename(file_path), open(file_path, "rb"))} logger.warning(f"Using fallback conversion for unknown type: {mime_type} / {file_ext}") if not endpoint: @@ -164,20 +187,22 @@ def convert_to_pdf(self, file_path, original_filename=None): try: logger.info(f"[{task_id}] Converting {file_path} using endpoint: {endpoint}") log_task_progress(task_id, "call_gotenberg", "in_progress", "Calling Gotenberg API") - + # Send the conversion request to Gotenberg response = requests.post(endpoint, files=files, data=form_data) - + if response.status_code == 200: # Save the converted PDF converted_file_path = os.path.splitext(file_path)[0] + ".pdf" with open(converted_file_path, "wb") as out_file: out_file.write(response.content) - + logger.info(f"[{task_id}] Converted file saved as PDF: {converted_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)}") - + 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, preserving original filename if provided if original_filename: # Change extension to .pdf for the original filename @@ -186,7 +211,7 @@ def convert_to_pdf(self, file_path, original_filename=None): process_document.delay(converted_file_path, original_filename=pdf_original_filename) else: process_document.delay(converted_file_path) - + return converted_file_path else: error_msg = f"Status code: {response.status_code}" diff --git a/app/tasks/process_document.py b/app/tasks/process_document.py index 0bd7a061..c43de473 100644 --- a/app/tasks/process_document.py +++ b/app/tasks/process_document.py @@ -1,21 +1,22 @@ #!/usr/bin/env python3 -import os -import uuid -import shutil -import mimetypes import logging +import mimetypes +import os +import shutil +import uuid + import PyPDF2 # Replace fitz with PyPDF2 +from app.celery_app import celery from app.config import settings -from app.tasks.retry_config import BaseTaskWithRetry +from app.database import SessionLocal +from app.models import FileRecord +from app.tasks.extract_metadata_with_gpt import extract_metadata_with_gpt from app.tasks.process_with_azure_document_intelligence import ( process_with_azure_document_intelligence, ) -from app.tasks.extract_metadata_with_gpt import extract_metadata_with_gpt -from app.celery_app import celery -from app.database import SessionLocal -from app.models import FileRecord +from app.tasks.retry_config import BaseTaskWithRetry from app.utils import hash_file, log_task_progress logger = logging.getLogger(__name__) @@ -63,9 +64,7 @@ def process_document(self, original_local_file: str, original_filename: str = No if not mime_type: mime_type = "application/octet-stream" - logger.info( - f"[{task_id}] File hash: {filehash[:10]}..., Size: {file_size} bytes, MIME: {mime_type}" - ) + logger.info(f"[{task_id}] File hash: {filehash[:10]}..., Size: {file_size} bytes, MIME: {mime_type}") log_task_progress( task_id, "hash_file", @@ -77,9 +76,7 @@ def process_document(self, original_local_file: str, original_filename: str = No with SessionLocal() as db: existing = db.query(FileRecord).filter_by(filehash=filehash).one_or_none() if existing: - logger.info( - f"[{task_id}] Duplicate file detected (hash={filehash[:10]}...) Skipping processing." - ) + logger.info(f"[{task_id}] Duplicate file detected (hash={filehash[:10]}...) Skipping processing.") log_task_progress( task_id, "process_document", @@ -95,9 +92,7 @@ def process_document(self, original_local_file: str, original_filename: str = No # Not a duplicate -> insert a new record logger.info(f"[{task_id}] Creating new file record in database") - log_task_progress( - task_id, "create_file_record", "in_progress", "Creating file record" - ) + log_task_progress(task_id, "create_file_record", "in_progress", "Creating file record") new_record = FileRecord( filehash=filehash, original_filename=original_filename, @@ -169,9 +164,7 @@ def process_document(self, original_local_file: str, original_filename: str = No break if has_text: - logger.info( - f"[{task_id}] PDF {original_local_file} contains embedded text. Processing locally." - ) + logger.info(f"[{task_id}] PDF {original_local_file} contains embedded text. Processing locally.") log_task_progress( task_id, "check_text", @@ -221,9 +214,7 @@ def process_document(self, original_local_file: str, original_filename: str = No } # 3. If no embedded text, queue Azure Document Intelligence processing - logger.info( - f"[{task_id}] No embedded text found. Queueing Azure Document Intelligence processing" - ) + logger.info(f"[{task_id}] No embedded text found. Queueing Azure Document Intelligence processing") log_task_progress( task_id, "check_text", diff --git a/tests/test_original_filename_preservation.py b/tests/test_original_filename_preservation.py index e3bf97a2..0778f6d2 100644 --- a/tests/test_original_filename_preservation.py +++ b/tests/test_original_filename_preservation.py @@ -5,13 +5,12 @@ 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 unittest.mock import MagicMock, patch + +import pytest -from app.tasks.process_document import process_document from app.models import FileRecord +from app.tasks.process_document import process_document @pytest.mark.unit @@ -83,15 +82,16 @@ startxref %%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: + 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) @@ -109,7 +109,7 @@ startxref # 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 @@ -187,10 +187,11 @@ startxref 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: + 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)