From eb666d8325394829898aa6233c52650e8d3fe81c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 20:35:19 +0000 Subject: [PATCH 1/2] Initial plan From 8edc11da0457110da88e4171d719c651673e240b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 20:40:11 +0000 Subject: [PATCH 2/2] refactor: extract shared file-status query logic into utility function - Created app/utils/file_queries.py with apply_status_filter function - Updated app/api/files.py to use shared function - Updated app/views/files.py to use shared function - Removed unused 'or_' import from app/api/files.py - Added comprehensive tests in tests/test_file_queries.py - All tests pass (10 new tests, 14 existing tests verified) Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/files.py | 34 +---- app/utils/file_queries.py | 72 +++++++++ app/views/files.py | 32 +--- tests/test_file_queries.py | 291 +++++++++++++++++++++++++++++++++++++ 4 files changed, 368 insertions(+), 61 deletions(-) create mode 100644 app/utils/file_queries.py create mode 100644 tests/test_file_queries.py diff --git a/app/api/files.py b/app/api/files.py index c00c3212..d055782e 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -9,7 +9,7 @@ import uuid from typing import List, Optional from fastapi import APIRouter, Depends, File, HTTPException, Query, Request, UploadFile -from sqlalchemy import asc, desc, or_ +from sqlalchemy import asc, desc from sqlalchemy.orm import Session from app.auth import require_login @@ -18,6 +18,7 @@ from app.database import get_db from app.models import FileRecord, ProcessingLog from app.tasks.convert_to_pdf import convert_to_pdf from app.tasks.process_document import process_document +from app.utils.file_queries import apply_status_filter from app.utils.file_status import get_files_processing_status from app.utils.filename_utils import sanitize_filename @@ -85,36 +86,7 @@ def list_files_api( 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 - if status == "pending": - # Files with no logs - subq = db.query(ProcessingLog.file_id).distinct() - 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() - 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() - 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( - ~FileRecord.id.in_(db.query(failed_files.c.file_id)) - ) + query = apply_status_filter(query, db, status) # Get total count before pagination (after all filters) total_items = query.count() diff --git a/app/utils/file_queries.py b/app/utils/file_queries.py new file mode 100644 index 00000000..a6af8b26 --- /dev/null +++ b/app/utils/file_queries.py @@ -0,0 +1,72 @@ +""" +Shared file query utilities for filtering files by processing status. + +This module contains reusable query logic for filtering FileRecord objects +based on their processing status (pending, processing, failed, completed). +""" + +from typing import Optional + +from sqlalchemy import or_ +from sqlalchemy.orm import Query, Session + +from app.models import FileRecord, ProcessingLog + + +def apply_status_filter(query: Query, db: Session, status: Optional[str]) -> Query: + """ + Apply status filter to a FileRecord query. + + This function modifies a SQLAlchemy query to filter files based on their + processing status by examining associated ProcessingLog entries. + + Args: + query: The base SQLAlchemy query for FileRecord objects + db: Database session for creating subqueries + status: Status filter to apply. Valid values: + - "pending": Files with no ProcessingLog entries + - "processing": Files with in_progress logs + - "failed": Files with failure logs + - "completed": Files with success logs but no failures or in_progress + - None: No filter applied (returns query unchanged) + + Returns: + Modified query with status filter applied + + Examples: + >>> query = db.query(FileRecord) + >>> query = apply_status_filter(query, db, "completed") + >>> files = query.all() + """ + if not status: + return query + + # Subquery to get file IDs matching the status + if status == "pending": + # Files with no logs + subq = db.query(ProcessingLog.file_id).distinct() + 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() + 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() + 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( + ~FileRecord.id.in_(db.query(failed_files.c.file_id)) + ) + + return query diff --git a/app/views/files.py b/app/views/files.py index 8e29e5c4..b3b2056b 100644 --- a/app/views/files.py +++ b/app/views/files.py @@ -8,6 +8,7 @@ from fastapi import Depends, Query, Request from sqlalchemy.orm import Session from app.config import settings +from app.utils.file_queries import apply_status_filter from app.utils.file_status import get_files_processing_status from app.views.base import APIRouter, get_db, logger, require_login, templates @@ -48,36 +49,7 @@ def files_page( 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 - if status == "pending": - # Files with no logs - subq = db.query(ProcessingLog.file_id).distinct() - 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() - 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() - 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( - ~FileRecord.id.in_(db.query(failed_files.c.file_id)) - ) + query = apply_status_filter(query, db, status) # Get total count before pagination total_items = query.count() diff --git a/tests/test_file_queries.py b/tests/test_file_queries.py new file mode 100644 index 00000000..0438cbbe --- /dev/null +++ b/tests/test_file_queries.py @@ -0,0 +1,291 @@ +""" +Tests for file query utilities. + +This module tests the shared file status filtering logic in app/utils/file_queries.py. +""" + +import pytest +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +from app.database import Base +from app.models import FileRecord, ProcessingLog +from app.utils.file_queries import apply_status_filter + + +@pytest.fixture +def db_session(): + """Create an in-memory SQLite database for testing.""" + engine = create_engine("sqlite:///:memory:") + Base.metadata.create_all(engine) + SessionLocal = sessionmaker(bind=engine) + session = SessionLocal() + yield session + session.close() + Base.metadata.drop_all(engine) + + +@pytest.fixture +def sample_files(db_session): + """Create sample files with different processing statuses.""" + # File 1: pending (no logs) + file1 = FileRecord( + filehash="hash1", + original_filename="pending.pdf", + local_filename="/tmp/pending.pdf", + file_size=1024, + mime_type="application/pdf", + ) + db_session.add(file1) + db_session.flush() + + # File 2: processing (has in_progress log) + file2 = FileRecord( + filehash="hash2", + original_filename="processing.pdf", + local_filename="/tmp/processing.pdf", + file_size=2048, + mime_type="application/pdf", + ) + db_session.add(file2) + db_session.flush() + + log2 = ProcessingLog( + file_id=file2.id, + task_id="task2", + step_name="extract_text", + status="in_progress", + message="Processing...", + ) + db_session.add(log2) + + # File 3: failed (has failure log) + file3 = FileRecord( + filehash="hash3", + original_filename="failed.pdf", + local_filename="/tmp/failed.pdf", + file_size=3072, + mime_type="application/pdf", + ) + db_session.add(file3) + db_session.flush() + + log3 = ProcessingLog( + file_id=file3.id, + task_id="task3", + step_name="extract_text", + status="failure", + message="Error occurred", + ) + db_session.add(log3) + + # File 4: completed (has success log, no failures) + file4 = FileRecord( + filehash="hash4", + original_filename="completed.pdf", + local_filename="/tmp/completed.pdf", + file_size=4096, + mime_type="application/pdf", + ) + db_session.add(file4) + db_session.flush() + + log4 = ProcessingLog(file_id=file4.id, task_id="task4", step_name="extract_text", status="success", message="Done") + db_session.add(log4) + + # File 5: completed with multiple success logs + file5 = FileRecord( + filehash="hash5", + original_filename="completed2.pdf", + local_filename="/tmp/completed2.pdf", + file_size=5120, + mime_type="application/pdf", + ) + db_session.add(file5) + db_session.flush() + + log5a = ProcessingLog( + file_id=file5.id, + task_id="task5a", + step_name="extract_text", + status="success", + message="Step 1 done", + ) + log5b = ProcessingLog( + file_id=file5.id, + task_id="task5b", + step_name="extract_metadata_with_gpt", + status="success", + message="Step 2 done", + ) + db_session.add_all([log5a, log5b]) + + # File 6: has success but also failure (should be filtered out from completed) + file6 = FileRecord( + filehash="hash6", + original_filename="mixed.pdf", + local_filename="/tmp/mixed.pdf", + file_size=6144, + mime_type="application/pdf", + ) + db_session.add(file6) + db_session.flush() + + log6a = ProcessingLog( + file_id=file6.id, + task_id="task6a", + step_name="extract_text", + status="success", + message="Step 1 done", + ) + log6b = ProcessingLog( + file_id=file6.id, + task_id="task6b", + step_name="upload_to_s3", + status="failure", + message="Upload failed", + ) + db_session.add_all([log6a, log6b]) + + db_session.commit() + + return { + "pending": file1, + "processing": file2, + "failed": file3, + "completed": file4, + "completed2": file5, + "mixed": file6, + } + + +@pytest.mark.unit +def test_apply_status_filter_none(db_session, sample_files): + """Test that no filter is applied when status is None.""" + query = db_session.query(FileRecord) + filtered_query = apply_status_filter(query, db_session, None) + results = filtered_query.all() + + # Should return all files + assert len(results) == 6 + + +@pytest.mark.unit +def test_apply_status_filter_pending(db_session, sample_files): + """Test filtering for pending files (no logs).""" + query = db_session.query(FileRecord) + filtered_query = apply_status_filter(query, db_session, "pending") + results = filtered_query.all() + + # Should return only file1 (no logs) + assert len(results) == 1 + assert results[0].filehash == "hash1" + assert results[0].original_filename == "pending.pdf" + + +@pytest.mark.unit +def test_apply_status_filter_processing(db_session, sample_files): + """Test filtering for files in processing state.""" + query = db_session.query(FileRecord) + filtered_query = apply_status_filter(query, db_session, "processing") + results = filtered_query.all() + + # Should return only file2 (has in_progress log) + assert len(results) == 1 + assert results[0].filehash == "hash2" + assert results[0].original_filename == "processing.pdf" + + +@pytest.mark.unit +def test_apply_status_filter_failed(db_session, sample_files): + """Test filtering for failed files.""" + query = db_session.query(FileRecord) + filtered_query = apply_status_filter(query, db_session, "failed") + results = filtered_query.all() + + # Should return file3 and file6 (both have failure logs) + assert len(results) == 2 + filehashes = {r.filehash for r in results} + assert "hash3" in filehashes + assert "hash6" in filehashes + + +@pytest.mark.unit +def test_apply_status_filter_completed(db_session, sample_files): + """Test filtering for completed files (success with no failures).""" + query = db_session.query(FileRecord) + filtered_query = apply_status_filter(query, db_session, "completed") + results = filtered_query.all() + + # Should return file4 and file5 (success logs, no failures) + # file6 should NOT be included (has both success and failure) + assert len(results) == 2 + filehashes = {r.filehash for r in results} + assert "hash4" in filehashes + assert "hash5" in filehashes + assert "hash6" not in filehashes # mixed status should be excluded + + +@pytest.mark.unit +def test_apply_status_filter_empty_string(db_session, sample_files): + """Test that empty string status is treated like None (no filter).""" + query = db_session.query(FileRecord) + filtered_query = apply_status_filter(query, db_session, "") + results = filtered_query.all() + + # Should return all files + assert len(results) == 6 + + +@pytest.mark.unit +def test_apply_status_filter_invalid_status(db_session, sample_files): + """Test that invalid status values don't crash, just return unfiltered.""" + query = db_session.query(FileRecord) + filtered_query = apply_status_filter(query, db_session, "invalid_status") + results = filtered_query.all() + + # Should return all files (no filter applied for unknown status) + assert len(results) == 6 + + +@pytest.mark.unit +def test_apply_status_filter_with_other_filters(db_session, sample_files): + """Test that status filter can be combined with other query filters.""" + # Filter for completed PDFs + query = db_session.query(FileRecord).filter(FileRecord.mime_type == "application/pdf") + filtered_query = apply_status_filter(query, db_session, "completed") + results = filtered_query.all() + + # Should return file4 and file5 (both are completed PDFs) + assert len(results) == 2 + filehashes = {r.filehash for r in results} + assert "hash4" in filehashes + assert "hash5" in filehashes + + +@pytest.mark.unit +def test_apply_status_filter_empty_database(db_session): + """Test filtering on empty database doesn't crash.""" + query = db_session.query(FileRecord) + filtered_query = apply_status_filter(query, db_session, "completed") + results = filtered_query.all() + + # Should return empty list + assert len(results) == 0 + + +@pytest.mark.unit +def test_apply_status_filter_preserves_query_order(db_session, sample_files): + """Test that status filter preserves existing query ordering.""" + from sqlalchemy import desc + + # Query with ordering + query = db_session.query(FileRecord).order_by(desc(FileRecord.file_size)) + filtered_query = apply_status_filter(query, db_session, "completed") + results = filtered_query.all() + + # Should return file5 before file4 (larger file size) + assert len(results) == 2 + assert results[0].file_size > results[1].file_size + assert results[0].filehash == "hash5" + assert results[1].filehash == "hash4"