fix(api): use processed_file_path from DB in upload retry path resolution
The upload retry logic now checks file_record.processed_file_path first (the GPT-suggested filename stored during finalization), before falling back to legacy hash-based and original-filename-based path patterns. This fixes the case where the processed file has a different name than the original (e.g., '2023-10-01_Unknown.pdf' vs 'cable_graphic.pdf') and the retry couldn't find the file on disk. Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+33
-17
@@ -763,28 +763,44 @@ def retry_subtask(
|
||||
logger.info(
|
||||
f"Retrying upload task {subtask_name} for file {file_id}: "
|
||||
f"original_filename={file_record.original_filename!r}, "
|
||||
f"filehash={file_record.filehash!r}"
|
||||
f"filehash={file_record.filehash!r}, "
|
||||
f"processed_file_path={file_record.processed_file_path!r}"
|
||||
)
|
||||
workdir = settings.workdir
|
||||
processed_dir = os.path.join(workdir, "processed")
|
||||
|
||||
# Try to find the processed file
|
||||
base_filename = os.path.splitext(file_record.original_filename)[0]
|
||||
potential_paths = [
|
||||
os.path.join(processed_dir, f"{file_record.filehash}.pdf"),
|
||||
os.path.join(processed_dir, f"{base_filename}_processed.pdf"),
|
||||
os.path.join(processed_dir, file_record.original_filename),
|
||||
]
|
||||
|
||||
# Try to find the processed file using multiple strategies:
|
||||
# 1. DB-stored processed_file_path (most reliable - set during finalization)
|
||||
# 2. Hash-based path in processed dir
|
||||
# 3. Original filename with _processed suffix
|
||||
# 4. Original filename in processed dir
|
||||
file_path = None
|
||||
checked_paths = []
|
||||
for path in potential_paths:
|
||||
exists = os.path.exists(path)
|
||||
checked_paths.append(f"{path!r} (exists={exists})")
|
||||
logger.info(f"Checking processed file path: {path!r}, exists={exists}")
|
||||
|
||||
# Check 1: processed_file_path from DB
|
||||
if file_record.processed_file_path:
|
||||
exists = os.path.exists(file_record.processed_file_path)
|
||||
checked_paths.append(f"processed_file_path={file_record.processed_file_path!r} (exists={exists})")
|
||||
logger.info(f"Checking processed_file_path: {file_record.processed_file_path!r}, exists={exists}")
|
||||
if exists:
|
||||
file_path = path
|
||||
break
|
||||
file_path = file_record.processed_file_path
|
||||
|
||||
# Check 2-4: Legacy path patterns in processed directory
|
||||
if not file_path:
|
||||
workdir = settings.workdir
|
||||
processed_dir = os.path.join(workdir, "processed")
|
||||
base_filename = os.path.splitext(file_record.original_filename)[0]
|
||||
legacy_paths = [
|
||||
os.path.join(processed_dir, f"{file_record.filehash}.pdf"),
|
||||
os.path.join(processed_dir, f"{base_filename}_processed.pdf"),
|
||||
os.path.join(processed_dir, file_record.original_filename),
|
||||
]
|
||||
|
||||
for path in legacy_paths:
|
||||
exists = os.path.exists(path)
|
||||
checked_paths.append(f"{path!r} (exists={exists})")
|
||||
logger.info(f"Checking processed file path: {path!r}, exists={exists}")
|
||||
if exists:
|
||||
file_path = path
|
||||
break
|
||||
|
||||
if not file_path:
|
||||
paths_detail = "; ".join(checked_paths)
|
||||
|
||||
@@ -168,6 +168,7 @@ class TestRetrySubtaskEnhancedLogging:
|
||||
filehash="upload_error_test",
|
||||
original_filename="missing.pdf",
|
||||
local_filename="/nonexistent/tmp/missing.pdf",
|
||||
processed_file_path="/nonexistent/processed/missing_gpt.pdf",
|
||||
file_size=1024,
|
||||
mime_type="application/pdf",
|
||||
)
|
||||
@@ -183,8 +184,82 @@ class TestRetrySubtaskEnhancedLogging:
|
||||
# Verify the error message contains diagnostic information
|
||||
assert "Cannot retry upload" in error_detail
|
||||
assert "Paths checked:" in error_detail
|
||||
assert "processed_file_path" in error_detail
|
||||
assert "exists=False" in error_detail
|
||||
|
||||
def test_upload_retry_uses_processed_file_path_from_db(
|
||||
self, client: TestClient, db_session, sample_pdf_path, tmp_path
|
||||
):
|
||||
"""Test that upload retry uses processed_file_path from DB when legacy paths don't exist."""
|
||||
mock_task = MagicMock()
|
||||
mock_task.id = "upload-processed-path-task"
|
||||
|
||||
# Create processed file at the DB-stored path (GPT-suggested filename)
|
||||
processed_file = tmp_path / "2023-10-01_Unknown.pdf"
|
||||
shutil.copy(sample_pdf_path, processed_file)
|
||||
|
||||
# Create file record where only processed_file_path exists
|
||||
# (simulates the real scenario: original filename != GPT-suggested filename)
|
||||
file_record = FileRecord(
|
||||
filehash="upload_db_path_test",
|
||||
original_filename="cable_graphic.pdf",
|
||||
local_filename="/nonexistent/tmp/uuid.pdf",
|
||||
processed_file_path=str(processed_file),
|
||||
file_size=1024,
|
||||
mime_type="application/pdf",
|
||||
)
|
||||
db_session.add(file_record)
|
||||
db_session.commit()
|
||||
db_session.refresh(file_record)
|
||||
|
||||
with patch("app.tasks.upload_to_onedrive.upload_to_onedrive") as mock_upload:
|
||||
mock_upload.delay.return_value = mock_task
|
||||
response = client.post(f"/api/files/{file_record.id}/retry-subtask?subtask_name=upload_to_onedrive")
|
||||
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["status"] == "success"
|
||||
assert data["subtask_name"] == "upload_to_onedrive"
|
||||
|
||||
# Verify the task was called with the processed_file_path from DB
|
||||
mock_upload.delay.assert_called_once()
|
||||
call_args = mock_upload.delay.call_args
|
||||
assert call_args[0][0] == str(processed_file)
|
||||
|
||||
def test_upload_retry_processed_file_path_takes_priority(
|
||||
self, client: TestClient, db_session, sample_pdf_path, tmp_path
|
||||
):
|
||||
"""Test that processed_file_path from DB takes priority over legacy hash-based paths."""
|
||||
mock_task = MagicMock()
|
||||
mock_task.id = "upload-priority-task"
|
||||
|
||||
# Create the DB-stored processed file
|
||||
processed_file = tmp_path / "2024-01-01_Invoice.pdf"
|
||||
shutil.copy(sample_pdf_path, processed_file)
|
||||
|
||||
file_record = FileRecord(
|
||||
filehash="upload_priority_test",
|
||||
original_filename="scan001.pdf",
|
||||
local_filename="/nonexistent/tmp/uuid.pdf",
|
||||
processed_file_path=str(processed_file),
|
||||
file_size=1024,
|
||||
mime_type="application/pdf",
|
||||
)
|
||||
db_session.add(file_record)
|
||||
db_session.commit()
|
||||
db_session.refresh(file_record)
|
||||
|
||||
with patch("app.tasks.upload_to_dropbox.upload_to_dropbox") as mock_upload:
|
||||
mock_upload.delay.return_value = mock_task
|
||||
response = client.post(f"/api/files/{file_record.id}/retry-subtask?subtask_name=upload_to_dropbox")
|
||||
|
||||
assert response.status_code == 200
|
||||
|
||||
# Verify the task was called with the processed_file_path (first priority for uploads)
|
||||
mock_upload.delay.assert_called_once()
|
||||
call_args = mock_upload.delay.call_args
|
||||
assert call_args[0][0] == str(processed_file)
|
||||
|
||||
def test_embed_metadata_path_order(self, client: TestClient, db_session, sample_pdf_path, tmp_path):
|
||||
"""Test that embed_metadata_into_pdf checks paths in the correct order."""
|
||||
mock_task = MagicMock()
|
||||
|
||||
Reference in New Issue
Block a user