Merge pull request #105 from christianlouis/copilot/fix-original-file-name-upload
Preserve original filenames for UI uploads while maintaining UUID-based storage
This commit is contained in:
@@ -0,0 +1,112 @@
|
|||||||
|
# Fix for Issue: Uploaded files do not maintain original file name
|
||||||
|
|
||||||
|
## Problem Summary
|
||||||
|
Users reported that files uploaded through the UI were being renamed to UUIDs during processing, making it impossible to recognize the original files. For example, a file originally named "Apostille Sverige.pdf" would appear as "e64b2825-9ff2-486b-aff1-08af2957140b.pdf" in the file detail view.
|
||||||
|
|
||||||
|
## Root Cause
|
||||||
|
The issue occurred because:
|
||||||
|
1. In `app/api/files.py`, the `ui_upload` endpoint saves uploaded files with UUID-based filenames for security (to prevent path traversal and filename conflicts)
|
||||||
|
2. This UUID-based path is passed to the `process_document` task
|
||||||
|
3. The `process_document` task extracts the filename from the path using `os.path.basename()`, which returns the UUID-based name
|
||||||
|
4. This UUID-based name is then stored in the database as the `original_filename`
|
||||||
|
|
||||||
|
## Solution Implemented
|
||||||
|
|
||||||
|
### Changes Made
|
||||||
|
|
||||||
|
#### 1. Modified `app/tasks/process_document.py`
|
||||||
|
- Added optional `original_filename` parameter to the `process_document` function
|
||||||
|
- When provided, uses the passed filename instead of extracting from path
|
||||||
|
- Falls back to `os.path.basename()` when parameter is not provided (backward compatibility)
|
||||||
|
|
||||||
|
```python
|
||||||
|
def process_document(self, original_local_file: str, original_filename: str = None):
|
||||||
|
# ...
|
||||||
|
if original_filename is None:
|
||||||
|
original_filename = os.path.basename(original_local_file)
|
||||||
|
```
|
||||||
|
|
||||||
|
#### 2. Modified `app/tasks/convert_to_pdf.py`
|
||||||
|
- Added optional `original_filename` parameter to the `convert_to_pdf` function
|
||||||
|
- Passes through the original filename to `process_document` after conversion
|
||||||
|
- Adjusts file extension to .pdf when passing to the next stage
|
||||||
|
|
||||||
|
```python
|
||||||
|
def convert_to_pdf(self, file_path, original_filename=None):
|
||||||
|
# ...
|
||||||
|
if 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)
|
||||||
|
```
|
||||||
|
|
||||||
|
#### 3. Modified `app/api/files.py`
|
||||||
|
- Updated `ui_upload` endpoint to pass the original safe filename to processing tasks
|
||||||
|
- Passes `original_filename=safe_filename` parameter to both `process_document` and `convert_to_pdf`
|
||||||
|
|
||||||
|
```python
|
||||||
|
# For PDFs
|
||||||
|
task = process_document.delay(target_path, original_filename=safe_filename)
|
||||||
|
|
||||||
|
# For images and office documents
|
||||||
|
task = convert_to_pdf.delay(target_path, original_filename=safe_filename)
|
||||||
|
```
|
||||||
|
|
||||||
|
### Testing
|
||||||
|
|
||||||
|
#### Unit Tests (`tests/test_original_filename_preservation.py`)
|
||||||
|
Created comprehensive unit tests to verify:
|
||||||
|
1. **Test 1**: Original filename is preserved when parameter is provided
|
||||||
|
- Uploads a file with UUID-based path but provides original filename "Apostille Sverige.pdf"
|
||||||
|
- Verifies the database stores the original filename, not the UUID-based path
|
||||||
|
|
||||||
|
2. **Test 2**: Backward compatibility is maintained
|
||||||
|
- Calls `process_document` without the optional parameter
|
||||||
|
- Verifies it falls back to extracting filename from path
|
||||||
|
|
||||||
|
All tests pass successfully.
|
||||||
|
|
||||||
|
#### Existing Tests
|
||||||
|
All existing tests in `tests/test_process_document.py` continue to pass, confirming backward compatibility.
|
||||||
|
|
||||||
|
### Benefits of This Solution
|
||||||
|
|
||||||
|
1. **Minimal Changes**: Only 3 files modified, optional parameter added to maintain backward compatibility
|
||||||
|
2. **Security Maintained**: Files are still stored with UUID-based names on disk to prevent:
|
||||||
|
- Filename conflicts
|
||||||
|
- Path traversal attacks
|
||||||
|
- Overwriting existing files
|
||||||
|
3. **User Experience Improved**: Users can now see their original filenames in the UI
|
||||||
|
4. **Backward Compatible**: Existing code that calls these tasks without the new parameter continues to work
|
||||||
|
|
||||||
|
### Example Flow
|
||||||
|
|
||||||
|
**Before the fix:**
|
||||||
|
```
|
||||||
|
User uploads: "Apostille Sverige.pdf"
|
||||||
|
→ Saved as: "e64b2825-9ff2-486b-aff1-08af2957140b.pdf"
|
||||||
|
→ process_document extracts: "e64b2825-9ff2-486b-aff1-08af2957140b.pdf"
|
||||||
|
→ Database stores: "e64b2825-9ff2-486b-aff1-08af2957140b.pdf" ❌
|
||||||
|
```
|
||||||
|
|
||||||
|
**After the fix:**
|
||||||
|
```
|
||||||
|
User uploads: "Apostille Sverige.pdf"
|
||||||
|
→ Saved as: "e64b2825-9ff2-486b-aff1-08af2957140b.pdf" (for security)
|
||||||
|
→ ui_upload passes original_filename="Apostille Sverige.pdf" to process_document
|
||||||
|
→ Database stores: "Apostille Sverige.pdf" ✅
|
||||||
|
```
|
||||||
|
|
||||||
|
### Files Changed
|
||||||
|
- `app/tasks/process_document.py`: Added optional parameter and logic to use it
|
||||||
|
- `app/tasks/convert_to_pdf.py`: Added optional parameter and pass-through logic
|
||||||
|
- `app/api/files.py`: Updated to pass original filename to tasks
|
||||||
|
- `tests/test_original_filename_preservation.py`: New comprehensive unit tests
|
||||||
|
|
||||||
|
### Code Quality
|
||||||
|
- All code formatted with Black (line length 120)
|
||||||
|
- All imports sorted with isort (Black profile)
|
||||||
|
- All linting issues resolved (flake8)
|
||||||
|
- All existing and new tests pass
|
||||||
+113
-142
@@ -1,28 +1,31 @@
|
|||||||
"""
|
"""
|
||||||
File-related API endpoints
|
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 logging
|
||||||
|
import mimetypes
|
||||||
import os
|
import os
|
||||||
import uuid
|
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.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.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
|
# Set up logging
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
router = APIRouter()
|
router = APIRouter()
|
||||||
|
|
||||||
|
|
||||||
@router.get("/files")
|
@router.get("/files")
|
||||||
@require_login
|
@require_login
|
||||||
def list_files_api(
|
def list_files_api(
|
||||||
@@ -30,11 +33,13 @@ def list_files_api(
|
|||||||
db: Session = Depends(get_db),
|
db: Session = Depends(get_db),
|
||||||
page: int = Query(1, ge=1, description="Page number"),
|
page: int = Query(1, ge=1, description="Page number"),
|
||||||
per_page: int = Query(50, ge=1, le=200, description="Items per page"),
|
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"),
|
sort_order: str = Query("desc", description="Sort order: asc or desc"),
|
||||||
search: Optional[str] = Query(None, description="Search in filename"),
|
search: Optional[str] = Query(None, description="Search in filename"),
|
||||||
mime_type: Optional[str] = Query(None, description="Filter by MIME type"),
|
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.
|
Returns a paginated JSON list of FileRecord entries with processing status.
|
||||||
@@ -80,29 +85,26 @@ def list_files_api(
|
|||||||
query = query.filter(~FileRecord.id.in_(subq))
|
query = query.filter(~FileRecord.id.in_(subq))
|
||||||
elif status == "processing":
|
elif status == "processing":
|
||||||
# Files with in_progress logs
|
# Files with in_progress logs
|
||||||
subq = db.query(ProcessingLog.file_id).filter(
|
subq = db.query(ProcessingLog.file_id).filter(ProcessingLog.status == "in_progress").distinct()
|
||||||
ProcessingLog.status == "in_progress"
|
|
||||||
).distinct()
|
|
||||||
query = query.filter(FileRecord.id.in_(subq))
|
query = query.filter(FileRecord.id.in_(subq))
|
||||||
elif status == "failed":
|
elif status == "failed":
|
||||||
# Files with failure logs
|
# Files with failure logs
|
||||||
subq = db.query(ProcessingLog.file_id).filter(
|
subq = db.query(ProcessingLog.file_id).filter(ProcessingLog.status == "failure").distinct()
|
||||||
ProcessingLog.status == "failure"
|
|
||||||
).distinct()
|
|
||||||
query = query.filter(FileRecord.id.in_(subq))
|
query = query.filter(FileRecord.id.in_(subq))
|
||||||
elif status == "completed":
|
elif status == "completed":
|
||||||
# Files with success logs but no failures or in_progress
|
# Files with success logs but no failures or in_progress
|
||||||
success_files = db.query(ProcessingLog.file_id).filter(
|
success_files = (
|
||||||
ProcessingLog.status == "success"
|
db.query(ProcessingLog.file_id).filter(ProcessingLog.status == "success").distinct().subquery()
|
||||||
).distinct().subquery()
|
)
|
||||||
|
|
||||||
failed_files = db.query(ProcessingLog.file_id).filter(
|
failed_files = (
|
||||||
or_(ProcessingLog.status == "failure", ProcessingLog.status == "in_progress")
|
db.query(ProcessingLog.file_id)
|
||||||
).distinct().subquery()
|
.filter(or_(ProcessingLog.status == "failure", ProcessingLog.status == "in_progress"))
|
||||||
|
.distinct()
|
||||||
|
.subquery()
|
||||||
|
)
|
||||||
|
|
||||||
query = query.filter(
|
query = query.filter(FileRecord.id.in_(db.query(success_files.c.file_id))).filter(
|
||||||
FileRecord.id.in_(db.query(success_files.c.file_id))
|
|
||||||
).filter(
|
|
||||||
~FileRecord.id.in_(db.query(failed_files.c.file_id))
|
~FileRecord.id.in_(db.query(failed_files.c.file_id))
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -115,7 +117,7 @@ def list_files_api(
|
|||||||
"original_filename": FileRecord.original_filename,
|
"original_filename": FileRecord.original_filename,
|
||||||
"file_size": FileRecord.file_size,
|
"file_size": FileRecord.file_size,
|
||||||
"mime_type": FileRecord.mime_type,
|
"mime_type": FileRecord.mime_type,
|
||||||
"created_at": FileRecord.created_at
|
"created_at": FileRecord.created_at,
|
||||||
}.get(sort_by, FileRecord.created_at)
|
}.get(sort_by, FileRecord.created_at)
|
||||||
|
|
||||||
if sort_order == "asc":
|
if sort_order == "asc":
|
||||||
@@ -134,33 +136,27 @@ def list_files_api(
|
|||||||
# Build result with processing status
|
# Build result with processing status
|
||||||
result = []
|
result = []
|
||||||
for f in files:
|
for f in files:
|
||||||
result.append({
|
result.append(
|
||||||
"id": f.id,
|
{
|
||||||
"filehash": f.filehash,
|
"id": f.id,
|
||||||
"original_filename": f.original_filename,
|
"filehash": f.filehash,
|
||||||
"local_filename": f.local_filename,
|
"original_filename": f.original_filename,
|
||||||
"file_size": f.file_size,
|
"local_filename": f.local_filename,
|
||||||
"mime_type": f.mime_type,
|
"file_size": f.file_size,
|
||||||
"created_at": f.created_at.isoformat() if f.created_at else None,
|
"mime_type": f.mime_type,
|
||||||
"processing_status": statuses.get(f.id, {
|
"created_at": f.created_at.isoformat() if f.created_at else None,
|
||||||
"status": "pending",
|
"processing_status": statuses.get(
|
||||||
"last_step": None,
|
f.id, {"status": "pending", "last_step": None, "has_errors": False, "total_steps": 0}
|
||||||
"has_errors": False,
|
),
|
||||||
"total_steps": 0
|
}
|
||||||
})
|
)
|
||||||
})
|
|
||||||
|
|
||||||
# Calculate pagination info
|
# Calculate pagination info
|
||||||
total_pages = (total_items + per_page - 1) // per_page
|
total_pages = (total_items + per_page - 1) // per_page
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"files": result,
|
"files": result,
|
||||||
"pagination": {
|
"pagination": {"page": page, "per_page": per_page, "total_items": total_items, "total_pages": total_pages},
|
||||||
"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.
|
Kept for backward compatibility with file detail endpoint.
|
||||||
"""
|
"""
|
||||||
from app.utils.file_status import get_file_processing_status
|
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}")
|
@router.get("/files/{file_id}")
|
||||||
@@ -184,35 +180,32 @@ def get_file_details(request: Request, file_id: int, db: Session = Depends(get_d
|
|||||||
file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first()
|
file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first()
|
||||||
|
|
||||||
if not file_record:
|
if not file_record:
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=404, detail=f"File record with ID {file_id} not found")
|
||||||
status_code=404,
|
|
||||||
detail=f"File record with ID {file_id} not found"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Get processing logs
|
# Get processing logs
|
||||||
logs = db.query(ProcessingLog).filter(
|
logs = (
|
||||||
ProcessingLog.file_id == file_id
|
db.query(ProcessingLog).filter(ProcessingLog.file_id == file_id).order_by(ProcessingLog.timestamp.desc()).all()
|
||||||
).order_by(ProcessingLog.timestamp.desc()).all()
|
)
|
||||||
|
|
||||||
# Build log list
|
# Build log list
|
||||||
log_list = []
|
log_list = []
|
||||||
for log in logs:
|
for log in logs:
|
||||||
log_list.append({
|
log_list.append(
|
||||||
"id": log.id,
|
{
|
||||||
"task_id": log.task_id,
|
"id": log.id,
|
||||||
"step_name": log.step_name,
|
"task_id": log.task_id,
|
||||||
"status": log.status,
|
"step_name": log.step_name,
|
||||||
"message": log.message,
|
"status": log.status,
|
||||||
"timestamp": log.timestamp.isoformat() if log.timestamp else None
|
"message": log.message,
|
||||||
})
|
"timestamp": log.timestamp.isoformat() if log.timestamp else None,
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
# Get processing status
|
# Get processing status
|
||||||
processing_status = _get_file_processing_status(db, file_id)
|
processing_status = _get_file_processing_status(db, file_id)
|
||||||
|
|
||||||
# Check if files exist on disk
|
# Check if files exist on disk
|
||||||
files_on_disk = {
|
files_on_disk = {"original": os.path.exists(file_record.local_filename) if file_record.local_filename else False}
|
||||||
"original": os.path.exists(file_record.local_filename) if file_record.local_filename else False
|
|
||||||
}
|
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"file": {
|
"file": {
|
||||||
@@ -222,13 +215,14 @@ def get_file_details(request: Request, file_id: int, db: Session = Depends(get_d
|
|||||||
"local_filename": file_record.local_filename,
|
"local_filename": file_record.local_filename,
|
||||||
"file_size": file_record.file_size,
|
"file_size": file_record.file_size,
|
||||||
"mime_type": file_record.mime_type,
|
"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,
|
"processing_status": processing_status,
|
||||||
"logs": log_list,
|
"logs": log_list,
|
||||||
"files_on_disk": files_on_disk
|
"files_on_disk": files_on_disk,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@router.delete("/files/{file_id}")
|
@router.delete("/files/{file_id}")
|
||||||
@require_login
|
@require_login
|
||||||
def delete_file_record(request: Request, file_id: int, db: Session = Depends(get_db)):
|
def delete_file_record(request: Request, file_id: int, db: Session = Depends(get_db)):
|
||||||
@@ -238,20 +232,14 @@ def delete_file_record(request: Request, file_id: int, db: Session = Depends(get
|
|||||||
"""
|
"""
|
||||||
# Check if file deletion is allowed
|
# Check if file deletion is allowed
|
||||||
if not settings.allow_file_delete:
|
if not settings.allow_file_delete:
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=403, detail="File deletion is disabled in the configuration")
|
||||||
status_code=403,
|
|
||||||
detail="File deletion is disabled in the configuration"
|
|
||||||
)
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# Find the file record
|
# Find the file record
|
||||||
file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first()
|
file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first()
|
||||||
|
|
||||||
if not file_record:
|
if not file_record:
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=404, detail=f"File record with ID {file_id} not found")
|
||||||
status_code=404,
|
|
||||||
detail=f"File record with ID {file_id} not found"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Log the deletion
|
# Log the deletion
|
||||||
logger.info(f"Deleting file record: ID={file_id}, Filename={file_record.original_filename}")
|
logger.info(f"Deleting file record: ID={file_id}, Filename={file_record.original_filename}")
|
||||||
@@ -260,20 +248,15 @@ def delete_file_record(request: Request, file_id: int, db: Session = Depends(get
|
|||||||
db.delete(file_record)
|
db.delete(file_record)
|
||||||
db.commit()
|
db.commit()
|
||||||
|
|
||||||
return {
|
return {"status": "success", "message": f"File record {file_id} deleted successfully"}
|
||||||
"status": "success",
|
|
||||||
"message": f"File record {file_id} deleted successfully"
|
|
||||||
}
|
|
||||||
|
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
raise
|
raise
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
db.rollback()
|
db.rollback()
|
||||||
logger.exception(f"Error deleting file record {file_id}: {str(e)}")
|
logger.exception(f"Error deleting file record {file_id}: {str(e)}")
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=500, detail=f"Error deleting file record: {str(e)}")
|
||||||
status_code=500,
|
|
||||||
detail=f"Error deleting file record: {str(e)}"
|
|
||||||
)
|
|
||||||
|
|
||||||
@router.post("/files/bulk-delete")
|
@router.post("/files/bulk-delete")
|
||||||
@require_login
|
@require_login
|
||||||
@@ -284,20 +267,14 @@ def bulk_delete_files(request: Request, file_ids: List[int], db: Session = Depen
|
|||||||
"""
|
"""
|
||||||
# Check if file deletion is allowed
|
# Check if file deletion is allowed
|
||||||
if not settings.allow_file_delete:
|
if not settings.allow_file_delete:
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=403, detail="File deletion is disabled in the configuration")
|
||||||
status_code=403,
|
|
||||||
detail="File deletion is disabled in the configuration"
|
|
||||||
)
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# Find all file records
|
# Find all file records
|
||||||
file_records = db.query(FileRecord).filter(FileRecord.id.in_(file_ids)).all()
|
file_records = db.query(FileRecord).filter(FileRecord.id.in_(file_ids)).all()
|
||||||
|
|
||||||
if not file_records:
|
if not file_records:
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=404, detail="No files found with the provided IDs")
|
||||||
status_code=404,
|
|
||||||
detail="No files found with the provided IDs"
|
|
||||||
)
|
|
||||||
|
|
||||||
deleted_count = len(file_records)
|
deleted_count = len(file_records)
|
||||||
deleted_ids = [f.id for f in file_records]
|
deleted_ids = [f.id for f in file_records]
|
||||||
@@ -314,7 +291,7 @@ def bulk_delete_files(request: Request, file_ids: List[int], db: Session = Depen
|
|||||||
return {
|
return {
|
||||||
"status": "success",
|
"status": "success",
|
||||||
"message": f"Successfully deleted {deleted_count} file records",
|
"message": f"Successfully deleted {deleted_count} file records",
|
||||||
"deleted_ids": deleted_ids
|
"deleted_ids": deleted_ids,
|
||||||
}
|
}
|
||||||
|
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
@@ -322,10 +299,7 @@ def bulk_delete_files(request: Request, file_ids: List[int], db: Session = Depen
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
db.rollback()
|
db.rollback()
|
||||||
logger.exception(f"Error bulk deleting file records: {str(e)}")
|
logger.exception(f"Error bulk deleting file records: {str(e)}")
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=500, detail=f"Error bulk deleting file records: {str(e)}")
|
||||||
status_code=500,
|
|
||||||
detail=f"Error bulk deleting file records: {str(e)}"
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
@router.post("/files/bulk-reprocess")
|
@router.post("/files/bulk-reprocess")
|
||||||
@@ -339,10 +313,7 @@ def bulk_reprocess_files(request: Request, file_ids: List[int], db: Session = De
|
|||||||
file_records = db.query(FileRecord).filter(FileRecord.id.in_(file_ids)).all()
|
file_records = db.query(FileRecord).filter(FileRecord.id.in_(file_ids)).all()
|
||||||
|
|
||||||
if not file_records:
|
if not file_records:
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=404, detail="No files found with the provided IDs")
|
||||||
status_code=404,
|
|
||||||
detail="No files found with the provided IDs"
|
|
||||||
)
|
|
||||||
|
|
||||||
task_ids = []
|
task_ids = []
|
||||||
processed_files = []
|
processed_files = []
|
||||||
@@ -352,48 +323,44 @@ def bulk_reprocess_files(request: Request, file_ids: List[int], db: Session = De
|
|||||||
try:
|
try:
|
||||||
# Check if local file exists
|
# Check if local file exists
|
||||||
if not file_record.local_filename or not os.path.exists(file_record.local_filename):
|
if not file_record.local_filename or not os.path.exists(file_record.local_filename):
|
||||||
errors.append({
|
errors.append(
|
||||||
"file_id": file_record.id,
|
{
|
||||||
"filename": file_record.original_filename,
|
"file_id": file_record.id,
|
||||||
"error": "Local file not found"
|
"filename": file_record.original_filename,
|
||||||
})
|
"error": "Local file not found",
|
||||||
|
}
|
||||||
|
)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Queue the file for processing
|
# Queue the file for processing
|
||||||
task = process_document.delay(file_record.local_filename)
|
task = process_document.delay(file_record.local_filename)
|
||||||
task_ids.append(task.id)
|
task_ids.append(task.id)
|
||||||
processed_files.append({
|
processed_files.append(
|
||||||
"file_id": file_record.id,
|
{"file_id": file_record.id, "filename": file_record.original_filename, "task_id": task.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}")
|
logger.info(
|
||||||
|
f"Reprocessing file: ID={file_record.id}, "
|
||||||
|
f"Filename={file_record.original_filename}, TaskID={task.id}"
|
||||||
|
)
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.exception(f"Error reprocessing file {file_record.id}: {str(e)}")
|
logger.exception(f"Error reprocessing file {file_record.id}: {str(e)}")
|
||||||
errors.append({
|
errors.append({"file_id": file_record.id, "filename": file_record.original_filename, "error": str(e)})
|
||||||
"file_id": file_record.id,
|
|
||||||
"filename": file_record.original_filename,
|
|
||||||
"error": str(e)
|
|
||||||
})
|
|
||||||
|
|
||||||
return {
|
return {
|
||||||
"status": "success" if processed_files else "error",
|
"status": "success" if processed_files else "error",
|
||||||
"message": f"Successfully queued {len(processed_files)} files for reprocessing",
|
"message": f"Successfully queued {len(processed_files)} files for reprocessing",
|
||||||
"processed_files": processed_files,
|
"processed_files": processed_files,
|
||||||
"errors": errors if errors else None,
|
"errors": errors if errors else None,
|
||||||
"task_ids": task_ids
|
"task_ids": task_ids,
|
||||||
}
|
}
|
||||||
|
|
||||||
except HTTPException:
|
except HTTPException:
|
||||||
raise
|
raise
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.exception(f"Error bulk reprocessing files: {str(e)}")
|
logger.exception(f"Error bulk reprocessing files: {str(e)}")
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=500, detail=f"Error bulk reprocessing files: {str(e)}")
|
||||||
status_code=500,
|
|
||||||
detail=f"Error bulk reprocessing files: {str(e)}"
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
@router.post("/ui-upload")
|
@router.post("/ui-upload")
|
||||||
@@ -422,10 +389,7 @@ async def ui_upload(request: Request, file: UploadFile = File(...)):
|
|||||||
content = await file.read()
|
content = await file.read()
|
||||||
f.write(content)
|
f.write(content)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=500, detail=f"Failed to save file: {e}")
|
||||||
status_code=500,
|
|
||||||
detail=f"Failed to save file: {e}"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Log the mapping between original and safe filename
|
# Log the mapping between original and safe filename
|
||||||
logger.info(f"Saved uploaded file '{safe_filename}' as '{target_filename}'")
|
logger.info(f"Saved uploaded file '{safe_filename}' as '{target_filename}'")
|
||||||
@@ -436,10 +400,7 @@ async def ui_upload(request: Request, file: UploadFile = File(...)):
|
|||||||
if file_size > max_size:
|
if file_size > max_size:
|
||||||
# Remove the file if it's too large
|
# Remove the file if it's too large
|
||||||
os.remove(target_path)
|
os.remove(target_path)
|
||||||
raise HTTPException(
|
raise HTTPException(status_code=413, detail=f"File too large: {file_size} bytes (max {max_size} bytes)")
|
||||||
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
|
# Same set of allowed file types as in the IMAP task
|
||||||
ALLOWED_MIME_TYPES = {
|
ALLOWED_MIME_TYPES = {
|
||||||
@@ -458,9 +419,14 @@ async def ui_upload(request: Request, file: UploadFile = File(...)):
|
|||||||
|
|
||||||
# Image MIME types that need conversion
|
# Image MIME types that need conversion
|
||||||
IMAGE_MIME_TYPES = {
|
IMAGE_MIME_TYPES = {
|
||||||
'image/jpeg', 'image/jpg', 'image/png',
|
"image/jpeg",
|
||||||
'image/gif', 'image/bmp', 'image/tiff',
|
"image/jpg",
|
||||||
'image/webp', 'image/svg+xml'
|
"image/png",
|
||||||
|
"image/gif",
|
||||||
|
"image/bmp",
|
||||||
|
"image/tiff",
|
||||||
|
"image/webp",
|
||||||
|
"image/svg+xml",
|
||||||
}
|
}
|
||||||
|
|
||||||
# Determine if the file is a PDF or needs conversion
|
# Determine if the file is a PDF or needs conversion
|
||||||
@@ -472,24 +438,29 @@ 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,
|
||||||
"status": "queued",
|
"status": "queued",
|
||||||
"original_filename": safe_filename,
|
"original_filename": safe_filename,
|
||||||
"stored_filename": target_filename
|
"stored_filename": target_filename,
|
||||||
}
|
}
|
||||||
|
|||||||
+87
-52
@@ -1,22 +1,28 @@
|
|||||||
#!/usr/bin/env python3
|
#!/usr/bin/env python3
|
||||||
import os
|
|
||||||
import requests
|
|
||||||
import logging
|
import logging
|
||||||
import mimetypes
|
import mimetypes
|
||||||
import json
|
import os
|
||||||
|
|
||||||
|
import requests
|
||||||
from celery import shared_task
|
from celery import shared_task
|
||||||
|
|
||||||
from app.config import settings
|
from app.config import settings
|
||||||
from app.tasks.process_document import process_document
|
from app.tasks.process_document import process_document
|
||||||
from app.utils import log_task_progress
|
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}")
|
||||||
@@ -41,62 +47,83 @@ def convert_to_pdf(self, file_path):
|
|||||||
|
|
||||||
# Dictionary mapping file extensions to their handlers
|
# Dictionary mapping file extensions to their handlers
|
||||||
OFFICE_EXTENSIONS = {
|
OFFICE_EXTENSIONS = {
|
||||||
'.doc', '.docx', '.docm', '.dot', '.dotx', '.dotm', # Word
|
".doc",
|
||||||
'.xls', '.xlsx', '.xlsm', '.xlsb', '.xlt', '.xltx', '.xlw', # Excel
|
".docx",
|
||||||
'.ppt', '.pptx', '.pptm', '.pps', '.ppsx', '.pot', '.potx', # PowerPoint
|
".docm",
|
||||||
'.odt', '.ods', '.odp', '.odg', '.odf', # OpenOffice/LibreOffice
|
".dot",
|
||||||
'.rtf', '.txt', '.csv', # Text formats
|
".dotx",
|
||||||
'.pdf', # PDF (already in PDF format but can be processed)
|
".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 = {
|
IMAGE_EXTENSIONS = {".jpg", ".jpeg", ".png", ".gif", ".bmp", ".tiff", ".tif", ".webp", ".svg"}
|
||||||
'.jpg', '.jpeg', '.png', '.gif', '.bmp', '.tiff', '.tif', '.webp', '.svg'
|
|
||||||
}
|
|
||||||
|
|
||||||
HTML_EXTENSIONS = {
|
HTML_EXTENSIONS = {".html", ".htm"}
|
||||||
'.html', '.htm'
|
|
||||||
}
|
|
||||||
|
|
||||||
# Use LibreOffice endpoint for office documents and images
|
# Use LibreOffice endpoint for office documents and images
|
||||||
if (mime_type and 'office' in mime_type) or \
|
if (
|
||||||
(mime_type and 'opendocument' in mime_type) or \
|
(mime_type and "office" in mime_type)
|
||||||
(mime_type and mime_type.startswith('image/')) or \
|
or (mime_type and "opendocument" in mime_type)
|
||||||
file_ext in OFFICE_EXTENSIONS or \
|
or (mime_type and mime_type.startswith("image/"))
|
||||||
file_ext in IMAGE_EXTENSIONS:
|
or file_ext in OFFICE_EXTENSIONS
|
||||||
|
or file_ext in IMAGE_EXTENSIONS
|
||||||
|
):
|
||||||
endpoint = f"{gotenberg_url}/forms/libreoffice/convert"
|
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
|
# Add some quality settings for better PDF output
|
||||||
form_data = {
|
form_data = {
|
||||||
'landscape': 'false',
|
"landscape": "false",
|
||||||
'exportBookmarks': 'true',
|
"exportBookmarks": "true",
|
||||||
'exportNotes': 'false',
|
"exportNotes": "false",
|
||||||
'losslessImageCompression': 'true', # Use lossless compression for images
|
"losslessImageCompression": "true", # Use lossless compression for images
|
||||||
'pdfa': 'PDF/A-2b', # Produce PDF/A-2b compatible output
|
"pdfa": "PDF/A-2b", # Produce PDF/A-2b compatible output
|
||||||
}
|
}
|
||||||
|
|
||||||
# Use Chromium endpoint for HTML documents
|
# 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"
|
endpoint = f"{gotenberg_url}/forms/chromium/convert/html"
|
||||||
# Gotenberg requires the form field to be exactly 'index.html'
|
# Gotenberg requires the form field to be exactly 'index.html'
|
||||||
# The content filename doesn't matter, just the form field key
|
# 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
|
# Add options for better HTML to PDF conversion
|
||||||
form_data = {
|
form_data = {
|
||||||
'paperWidth': '8.27', # A4 width in inches
|
"paperWidth": "8.27", # A4 width in inches
|
||||||
'paperHeight': '11.7', # A4 height in inches
|
"paperHeight": "11.7", # A4 height in inches
|
||||||
'marginTop': '0.4',
|
"marginTop": "0.4",
|
||||||
'marginBottom': '0.4',
|
"marginBottom": "0.4",
|
||||||
'marginLeft': '0.4',
|
"marginLeft": "0.4",
|
||||||
'marginRight': '0.4',
|
"marginRight": "0.4",
|
||||||
'printBackground': 'true',
|
"printBackground": "true",
|
||||||
'preferCssPageSize': 'false',
|
"preferCssPageSize": "false",
|
||||||
'waitDelay': '2s', # Wait for JavaScript to execute
|
"waitDelay": "2s", # Wait for JavaScript to execute
|
||||||
}
|
}
|
||||||
|
|
||||||
# Use Markdown route for markdown files
|
# 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
|
# For Markdown, we need both the markdown file and an HTML wrapper
|
||||||
endpoint = f"{gotenberg_url}/forms/chromium/convert/markdown"
|
endpoint = f"{gotenberg_url}/forms/chromium/convert/markdown"
|
||||||
|
|
||||||
@@ -124,22 +151,22 @@ def convert_to_pdf(self, file_path):
|
|||||||
|
|
||||||
# Create a temporary HTML wrapper file
|
# Create a temporary HTML wrapper file
|
||||||
wrapper_path = os.path.join(os.path.dirname(file_path), "md_wrapper.html")
|
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)
|
f.write(html_wrapper)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
files = {
|
files = {
|
||||||
'index.html': ('index.html', open(wrapper_path, 'rb')),
|
"index.html": ("index.html", open(wrapper_path, "rb")),
|
||||||
markdown_filename: (markdown_filename, open(file_path, 'rb'))
|
markdown_filename: (markdown_filename, open(file_path, "rb")),
|
||||||
}
|
}
|
||||||
|
|
||||||
form_data = {
|
form_data = {
|
||||||
'paperWidth': '8.27', # A4 width in inches
|
"paperWidth": "8.27", # A4 width in inches
|
||||||
'paperHeight': '11.7', # A4 height in inches
|
"paperHeight": "11.7", # A4 height in inches
|
||||||
'marginTop': '0.4',
|
"marginTop": "0.4",
|
||||||
'marginBottom': '0.4',
|
"marginBottom": "0.4",
|
||||||
'marginLeft': '0.4',
|
"marginLeft": "0.4",
|
||||||
'marginRight': '0.4',
|
"marginRight": "0.4",
|
||||||
}
|
}
|
||||||
finally:
|
finally:
|
||||||
# Clean up the temporary wrapper file after preparing the request
|
# Clean up the temporary wrapper file after preparing the request
|
||||||
@@ -149,7 +176,7 @@ def convert_to_pdf(self, file_path):
|
|||||||
# Fallback to LibreOffice for everything else
|
# Fallback to LibreOffice for everything else
|
||||||
else:
|
else:
|
||||||
endpoint = f"{gotenberg_url}/forms/libreoffice/convert"
|
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}")
|
logger.warning(f"Using fallback conversion for unknown type: {mime_type} / {file_ext}")
|
||||||
|
|
||||||
if not endpoint:
|
if not endpoint:
|
||||||
@@ -172,10 +199,18 @@ def convert_to_pdf(self, file_path):
|
|||||||
|
|
||||||
logger.info(f"[{task_id}] Converted file saved as PDF: {converted_file_path}")
|
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, "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:
|
||||||
|
|||||||
@@ -1,31 +1,36 @@
|
|||||||
#!/usr/bin/env python3
|
#!/usr/bin/env python3
|
||||||
|
|
||||||
import os
|
|
||||||
import uuid
|
|
||||||
import shutil
|
|
||||||
import mimetypes
|
|
||||||
import logging
|
import logging
|
||||||
|
import mimetypes
|
||||||
|
import os
|
||||||
|
import shutil
|
||||||
|
import uuid
|
||||||
|
|
||||||
import PyPDF2 # Replace fitz with PyPDF2
|
import PyPDF2 # Replace fitz with PyPDF2
|
||||||
|
|
||||||
|
from app.celery_app import celery
|
||||||
from app.config import settings
|
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 (
|
from app.tasks.process_with_azure_document_intelligence import (
|
||||||
process_with_azure_document_intelligence,
|
process_with_azure_document_intelligence,
|
||||||
)
|
)
|
||||||
from app.tasks.extract_metadata_with_gpt import extract_metadata_with_gpt
|
from app.tasks.retry_config import BaseTaskWithRetry
|
||||||
from app.celery_app import celery
|
|
||||||
from app.database import SessionLocal
|
|
||||||
from app.models import FileRecord
|
|
||||||
from app.utils import hash_file, log_task_progress
|
from app.utils import hash_file, log_task_progress
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
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,15 +56,15 @@ 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:
|
||||||
mime_type = "application/octet-stream"
|
mime_type = "application/octet-stream"
|
||||||
|
|
||||||
logger.info(
|
logger.info(f"[{task_id}] File hash: {filehash[:10]}..., Size: {file_size} bytes, MIME: {mime_type}")
|
||||||
f"[{task_id}] File hash: {filehash[:10]}..., Size: {file_size} bytes, MIME: {mime_type}"
|
|
||||||
)
|
|
||||||
log_task_progress(
|
log_task_progress(
|
||||||
task_id,
|
task_id,
|
||||||
"hash_file",
|
"hash_file",
|
||||||
@@ -71,9 +76,7 @@ def process_document(self, original_local_file: str):
|
|||||||
with SessionLocal() as db:
|
with SessionLocal() as db:
|
||||||
existing = db.query(FileRecord).filter_by(filehash=filehash).one_or_none()
|
existing = db.query(FileRecord).filter_by(filehash=filehash).one_or_none()
|
||||||
if existing:
|
if existing:
|
||||||
logger.info(
|
logger.info(f"[{task_id}] Duplicate file detected (hash={filehash[:10]}...) Skipping processing.")
|
||||||
f"[{task_id}] Duplicate file detected (hash={filehash[:10]}...) Skipping processing."
|
|
||||||
)
|
|
||||||
log_task_progress(
|
log_task_progress(
|
||||||
task_id,
|
task_id,
|
||||||
"process_document",
|
"process_document",
|
||||||
@@ -89,9 +92,7 @@ def process_document(self, original_local_file: str):
|
|||||||
|
|
||||||
# Not a duplicate -> insert a new record
|
# Not a duplicate -> insert a new record
|
||||||
logger.info(f"[{task_id}] Creating new file record in database")
|
logger.info(f"[{task_id}] Creating new file record in database")
|
||||||
log_task_progress(
|
log_task_progress(task_id, "create_file_record", "in_progress", "Creating file record")
|
||||||
task_id, "create_file_record", "in_progress", "Creating file record"
|
|
||||||
)
|
|
||||||
new_record = FileRecord(
|
new_record = FileRecord(
|
||||||
filehash=filehash,
|
filehash=filehash,
|
||||||
original_filename=original_filename,
|
original_filename=original_filename,
|
||||||
@@ -163,9 +164,7 @@ def process_document(self, original_local_file: str):
|
|||||||
break
|
break
|
||||||
|
|
||||||
if has_text:
|
if has_text:
|
||||||
logger.info(
|
logger.info(f"[{task_id}] PDF {original_local_file} contains embedded text. Processing locally.")
|
||||||
f"[{task_id}] PDF {original_local_file} contains embedded text. Processing locally."
|
|
||||||
)
|
|
||||||
log_task_progress(
|
log_task_progress(
|
||||||
task_id,
|
task_id,
|
||||||
"check_text",
|
"check_text",
|
||||||
@@ -215,9 +214,7 @@ def process_document(self, original_local_file: str):
|
|||||||
}
|
}
|
||||||
|
|
||||||
# 3. If no embedded text, queue Azure Document Intelligence processing
|
# 3. If no embedded text, queue Azure Document Intelligence processing
|
||||||
logger.info(
|
logger.info(f"[{task_id}] No embedded text found. Queueing Azure Document Intelligence processing")
|
||||||
f"[{task_id}] No embedded text found. Queueing Azure Document Intelligence processing"
|
|
||||||
)
|
|
||||||
log_task_progress(
|
log_task_progress(
|
||||||
task_id,
|
task_id,
|
||||||
"check_text",
|
"check_text",
|
||||||
|
|||||||
@@ -0,0 +1,211 @@
|
|||||||
|
"""
|
||||||
|
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.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from app.models import FileRecord
|
||||||
|
from app.tasks.process_document import process_document
|
||||||
|
|
||||||
|
|
||||||
|
@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