diff --git a/tests/conftest.py b/tests/conftest.py index 20f395b3..6dace120 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -167,6 +167,55 @@ startxref return pdf_path +@pytest.fixture +def sample_pdf_file(test_workdir): + """Create a sample PDF file for testing, returning Path object.""" + from pathlib import Path + + pdf_path = Path(test_workdir) / "test.pdf" + + # Create a minimal valid 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] +>> +endobj +xref +0 4 +0000000000 65535 f +0000000009 00000 n +0000000058 00000 n +0000000115 00000 n +trailer +<< +/Size 4 +/Root 1 0 R +>> +startxref +197 +%%EOF +""" + + pdf_path.write_bytes(pdf_content) + return pdf_path + + @pytest.fixture def sample_text_file(test_workdir) -> str: """Create a sample text file for testing.""" diff --git a/tests/test_endpoint_registration.py b/tests/test_endpoint_registration.py index 2d571c6e..20e95b80 100644 --- a/tests/test_endpoint_registration.py +++ b/tests/test_endpoint_registration.py @@ -6,6 +6,7 @@ that endpoints remain accessible after code refactoring or reorganization. """ import pytest +from unittest.mock import Mock, patch # Test constants TEST_URL = "https://example.com/test.pdf" @@ -15,14 +16,29 @@ TEST_URL = "https://example.com/test.pdf" class TestEndpointRegistration: """Verify that critical API endpoints are registered in the FastAPI app""" - def test_process_url_endpoint_exists(self, client): + @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.process_document") + def test_process_url_endpoint_exists(self, mock_process_document, mock_requests_get, client): """Verify that /api/process-url endpoint is registered and accessible""" + # Mock successful download to prevent actual HTTP requests + mock_response = Mock() + mock_response.status_code = 200 + mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "1024"} + mock_response.iter_content = Mock(return_value=[b"PDF content"]) + mock_response.raise_for_status = Mock() + mock_requests_get.return_value = mock_response + + # Mock Celery task + mock_task = Mock() + mock_task.id = "test-task-id" + mock_process_document.delay.return_value = mock_task + # Make a request to the endpoint - it should not return 404 response = client.post( "/api/process-url", json={"url": TEST_URL} ) - + # The endpoint exists if we don't get a 404 # We may get other errors (401, 400, 500, etc.) due to validation or missing mocks, # but 404 specifically means the endpoint is not registered @@ -32,34 +48,64 @@ class TestEndpointRegistration: f"Verify that url_upload_router is included in app/api/__init__.py" ) - def test_process_url_endpoint_accepts_post(self, client): + @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.process_document") + def test_process_url_endpoint_accepts_post(self, mock_process_document, mock_requests_get, client): """Verify that /api/process-url accepts POST requests""" + # Mock successful download to prevent actual HTTP requests + mock_response = Mock() + mock_response.status_code = 200 + mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "1024"} + mock_response.iter_content = Mock(return_value=[b"PDF content"]) + mock_response.raise_for_status = Mock() + mock_requests_get.return_value = mock_response + + # Mock Celery task + mock_task = Mock() + mock_task.id = "test-task-id" + mock_process_document.delay.return_value = mock_task + # Try POST request response = client.post( "/api/process-url", json={"url": TEST_URL} ) - + # Should not return 405 (Method Not Allowed) assert response.status_code != 405, ( f"Endpoint /api/process-url returned 405 (Method Not Allowed) for POST. " f"Verify the endpoint is decorated with @router.post()" ) - def test_api_router_included_in_app(self, client): + @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.process_document") + def test_api_router_included_in_app(self, mock_process_document, mock_requests_get, client): """Verify that the main API router is included in the FastAPI app""" + # Mock successful download for /api/process-url test + mock_response = Mock() + mock_response.status_code = 200 + mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "1024"} + mock_response.iter_content = Mock(return_value=[b"PDF content"]) + mock_response.raise_for_status = Mock() + mock_requests_get.return_value = mock_response + + # Mock Celery task + mock_task = Mock() + mock_task.id = "test-task-id" + mock_process_document.delay.return_value = mock_task + # Test a few known API endpoints to ensure the /api prefix works endpoints_to_check = [ ("/api/process-url", "post"), ("/api/diagnostic/settings", "get"), ] - + for endpoint, method in endpoints_to_check: if method == "get": response = client.get(endpoint) else: response = client.post(endpoint, json={"url": TEST_URL}) - + # None of these should return 404 assert response.status_code != 404, ( f"Endpoint {endpoint} returned 404. " diff --git a/tests/test_file_queries.py b/tests/test_file_queries.py index 0438cbbe..d5eabf3f 100644 --- a/tests/test_file_queries.py +++ b/tests/test_file_queries.py @@ -9,7 +9,7 @@ from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker from app.database import Base -from app.models import FileRecord, ProcessingLog +from app.models import FileRecord, FileProcessingStep from app.utils.file_queries import apply_status_filter @@ -28,7 +28,7 @@ def db_session(): @pytest.fixture def sample_files(db_session): """Create sample files with different processing statuses.""" - # File 1: pending (no logs) + # File 1: pending (no steps) file1 = FileRecord( filehash="hash1", original_filename="pending.pdf", @@ -39,7 +39,7 @@ def sample_files(db_session): db_session.add(file1) db_session.flush() - # File 2: processing (has in_progress log) + # File 2: processing (has in_progress step) file2 = FileRecord( filehash="hash2", original_filename="processing.pdf", @@ -50,16 +50,14 @@ def sample_files(db_session): db_session.add(file2) db_session.flush() - log2 = ProcessingLog( + step2 = FileProcessingStep( file_id=file2.id, - task_id="task2", step_name="extract_text", status="in_progress", - message="Processing...", ) - db_session.add(log2) + db_session.add(step2) - # File 3: failed (has failure log) + # File 3: failed (has failure step) file3 = FileRecord( filehash="hash3", original_filename="failed.pdf", @@ -70,16 +68,15 @@ def sample_files(db_session): db_session.add(file3) db_session.flush() - log3 = ProcessingLog( + step3 = FileProcessingStep( file_id=file3.id, - task_id="task3", step_name="extract_text", status="failure", - message="Error occurred", + error_message="Error occurred", ) - db_session.add(log3) + db_session.add(step3) - # File 4: completed (has success log, no failures) + # File 4: completed (has success step, no failures) file4 = FileRecord( filehash="hash4", original_filename="completed.pdf", @@ -90,10 +87,10 @@ def sample_files(db_session): 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) + step4 = FileProcessingStep(file_id=file4.id, step_name="extract_text", status="success") + db_session.add(step4) - # File 5: completed with multiple success logs + # File 5: completed with multiple success steps file5 = FileRecord( filehash="hash5", original_filename="completed2.pdf", @@ -104,21 +101,17 @@ def sample_files(db_session): db_session.add(file5) db_session.flush() - log5a = ProcessingLog( + step5a = FileProcessingStep( file_id=file5.id, - task_id="task5a", step_name="extract_text", status="success", - message="Step 1 done", ) - log5b = ProcessingLog( + step5b = FileProcessingStep( 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]) + db_session.add_all([step5a, step5b]) # File 6: has success but also failure (should be filtered out from completed) file6 = FileRecord( @@ -131,21 +124,18 @@ def sample_files(db_session): db_session.add(file6) db_session.flush() - log6a = ProcessingLog( + step6a = FileProcessingStep( file_id=file6.id, - task_id="task6a", step_name="extract_text", status="success", - message="Step 1 done", ) - log6b = ProcessingLog( + step6b = FileProcessingStep( file_id=file6.id, - task_id="task6b", step_name="upload_to_s3", status="failure", - message="Upload failed", + error_message="Upload failed", ) - db_session.add_all([log6a, log6b]) + db_session.add_all([step6a, step6b]) db_session.commit() @@ -172,12 +162,12 @@ def test_apply_status_filter_none(db_session, sample_files): @pytest.mark.unit def test_apply_status_filter_pending(db_session, sample_files): - """Test filtering for pending files (no logs).""" + """Test filtering for pending files (no steps).""" query = db_session.query(FileRecord) filtered_query = apply_status_filter(query, db_session, "pending") results = filtered_query.all() - # Should return only file1 (no logs) + # Should return only file1 (no steps) assert len(results) == 1 assert results[0].filehash == "hash1" assert results[0].original_filename == "pending.pdf" @@ -190,7 +180,7 @@ def test_apply_status_filter_processing(db_session, sample_files): filtered_query = apply_status_filter(query, db_session, "processing") results = filtered_query.all() - # Should return only file2 (has in_progress log) + # Should return only file2 (has in_progress step) assert len(results) == 1 assert results[0].filehash == "hash2" assert results[0].original_filename == "processing.pdf" @@ -203,7 +193,7 @@ def test_apply_status_filter_failed(db_session, sample_files): filtered_query = apply_status_filter(query, db_session, "failed") results = filtered_query.all() - # Should return file3 and file6 (both have failure logs) + # Should return file3 and file6 (both have failure steps) assert len(results) == 2 filehashes = {r.filehash for r in results} assert "hash3" in filehashes @@ -217,7 +207,7 @@ def test_apply_status_filter_completed(db_session, sample_files): filtered_query = apply_status_filter(query, db_session, "completed") results = filtered_query.all() - # Should return file4 and file5 (success logs, no failures) + # Should return file4 and file5 (success steps, no failures) # file6 should NOT be included (has both success and failure) assert len(results) == 2 filehashes = {r.filehash for r in results} diff --git a/tests/test_file_status_fix.py b/tests/test_file_status_fix.py index b44ed09b..86dbe89a 100644 --- a/tests/test_file_status_fix.py +++ b/tests/test_file_status_fix.py @@ -1,331 +1,203 @@ """ -Tests for file status and metrics calculation bug fixes. +Tests for file status and metrics calculation using FileProcessingStep model. This test module verifies that: -1. Status calculation only considers the latest status per unique step -2. Metrics counting only uses the latest status per unique step +1. Status calculation uses FileProcessingStep entries correctly +2. Metrics counting uses FileProcessingStep entries correctly 3. Files with completed steps show "completed" not "processing" """ from datetime import datetime, timedelta import pytest +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker -from app.utils.file_status import _compute_status_from_logs -from app.views.files import _compute_step_summary +from app.database import Base +from app.models import FileRecord, FileProcessingStep +from app.utils.step_manager import get_file_overall_status, get_step_summary, initialize_file_steps, update_step_status + + +@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.mark.unit -class TestFileStatusBugFixes: - """Test fixes for status calculation bugs.""" +class TestFileStatusCalculation: + """Test file status calculation using FileProcessingStep.""" - def test_status_not_stuck_on_old_in_progress(self): + def test_status_completed_when_all_steps_success(self, db_session): """ - Test that status doesn't show "processing" when old in_progress logs exist - but latest status for all steps is success. - - This simulates the bug where a file shows "Processing" even though - all steps have completed successfully. + Test that status shows "completed" when all steps are success. """ + # Create file and initialize steps + file_record = FileRecord( + filehash="test1", + original_filename="test.pdf", + local_filename="/tmp/test.pdf", + file_size=1024 + ) + db_session.add(file_record) + db_session.commit() - class MockLog: - def __init__(self, step_name, status, timestamp): - self.step_name = step_name - self.status = status - self.timestamp = timestamp + initialize_file_steps(db_session, file_record.id) - now = datetime.now() - # Simulate logs ordered by timestamp desc (latest first) - logs = [ - # Latest logs (all success) - MockLog("upload_to_dropbox", "success", now - timedelta(minutes=1)), - MockLog("extract_metadata_with_gpt", "success", now - timedelta(minutes=2)), - MockLog("check_text", "success", now - timedelta(minutes=3)), - # Older in_progress logs that should be ignored - MockLog("upload_to_dropbox", "in_progress", now - timedelta(minutes=5)), - MockLog("extract_metadata_with_gpt", "in_progress", now - timedelta(minutes=6)), - MockLog("check_text", "in_progress", now - timedelta(minutes=7)), - ] + # Mark all steps as success + from app.utils.step_manager import MAIN_PROCESSING_STEPS + for step_name in MAIN_PROCESSING_STEPS: + update_step_status(db_session, file_record.id, step_name, "success") - result = _compute_status_from_logs(logs) + result = get_file_overall_status(db_session, file_record.id) - # Should be completed, not processing assert result["status"] == "completed" assert result["has_errors"] is False - def test_status_shows_processing_for_active_tasks(self): + def test_status_processing_when_steps_in_progress(self, db_session): """ - Test that status correctly shows "processing" when there are - actually in-progress tasks (based on latest status). + Test that status shows "processing" when there are in_progress steps. """ + file_record = FileRecord( + filehash="test2", + original_filename="test2.pdf", + local_filename="/tmp/test2.pdf", + file_size=2048 + ) + db_session.add(file_record) + db_session.commit() - class MockLog: - def __init__(self, step_name, status, timestamp): - self.step_name = step_name - self.status = status - self.timestamp = timestamp + initialize_file_steps(db_session, file_record.id) - now = datetime.now() - logs = [ - # One task actually in progress - MockLog("upload_to_dropbox", "in_progress", now - timedelta(minutes=1)), - # Other tasks completed - MockLog("extract_metadata_with_gpt", "success", now - timedelta(minutes=2)), - MockLog("check_text", "success", now - timedelta(minutes=3)), - ] + # Mark some steps as success, one as in_progress + update_step_status(db_session, file_record.id, "create_file_record", "success") + update_step_status(db_session, file_record.id, "check_text", "in_progress") - result = _compute_status_from_logs(logs) + result = get_file_overall_status(db_session, file_record.id) - # Should be processing because one task is actually in progress assert result["status"] == "processing" assert result["has_errors"] is False - def test_status_shows_failed_when_latest_has_failure(self): + def test_status_failed_when_steps_have_failure(self, db_session): """ - Test that status shows "failed" when the latest status for any step is failure. + Test that status shows "failed" when any step has failure status. """ + file_record = FileRecord( + filehash="test3", + original_filename="test3.pdf", + local_filename="/tmp/test3.pdf", + file_size=3072 + ) + db_session.add(file_record) + db_session.commit() - class MockLog: - def __init__(self, step_name, status, timestamp): - self.step_name = step_name - self.status = status - self.timestamp = timestamp + initialize_file_steps(db_session, file_record.id) - now = datetime.now() - logs = [ - # One task failed (latest status) - MockLog("upload_to_s3", "failure", now - timedelta(minutes=1)), - # Other tasks completed - MockLog("upload_to_dropbox", "success", now - timedelta(minutes=2)), - MockLog("extract_metadata_with_gpt", "success", now - timedelta(minutes=3)), - ] + # Mark some steps as success, one as failure + update_step_status(db_session, file_record.id, "create_file_record", "success") + update_step_status(db_session, file_record.id, "check_text", "success") + update_step_status(db_session, file_record.id, "extract_text", "failure", error_message="OCR failed") - result = _compute_status_from_logs(logs) + result = get_file_overall_status(db_session, file_record.id) assert result["status"] == "failed" assert result["has_errors"] is True @pytest.mark.unit -class TestMetricsCountingBugFixes: - """Test fixes for metrics counting bugs.""" +class TestMetricsCounting: + """Test metrics counting using FileProcessingStep.""" - def test_main_steps_not_double_counted(self): + def test_main_steps_counted_correctly(self, db_session): """ - Test that main processing steps are only counted once per step, - using the latest status, not counting all historical logs. - - This simulates the bug where metrics show incorrect counts because - they count all logs instead of just the latest per step. + Test that main processing steps are counted correctly. """ - from datetime import datetime, timedelta + file_record = FileRecord( + filehash="test4", + original_filename="test4.pdf", + local_filename="/tmp/test4.pdf", + file_size=4096 + ) + db_session.add(file_record) + db_session.commit() - class MockLog: - def __init__(self, step_name, status, timestamp): - self.step_name = step_name - self.status = status - self.timestamp = timestamp + initialize_file_steps(db_session, file_record.id) - now = datetime.now() - # Simulate logs ordered by timestamp desc (latest first) - logs = [ - # Latest status for each step (all success) - MockLog("hash_file", "success", now - timedelta(minutes=1)), - MockLog("create_file_record", "success", now - timedelta(minutes=2)), - MockLog("extract_metadata_with_gpt", "success", now - timedelta(minutes=3)), - # Older in_progress logs that should be ignored - MockLog("hash_file", "in_progress", now - timedelta(minutes=5)), - MockLog("create_file_record", "in_progress", now - timedelta(minutes=6)), - MockLog("extract_metadata_with_gpt", "in_progress", now - timedelta(minutes=7)), - ] + # Mark some steps with different statuses + update_step_status(db_session, file_record.id, "create_file_record", "success") + update_step_status(db_session, file_record.id, "check_text", "success") + update_step_status(db_session, file_record.id, "extract_text", "in_progress") - summary = _compute_step_summary(logs) + summary = get_step_summary(db_session, file_record.id) - # Should count each main step only once - assert summary["total_main_steps"] == 3 - assert summary["main"]["success"] == 3 - assert summary["main"]["in_progress"] == 0 # No steps actually in progress - assert summary["main"]["failure"] == 0 + # Should count each step once + from app.utils.step_manager import MAIN_PROCESSING_STEPS + assert summary["total_main_steps"] == len(MAIN_PROCESSING_STEPS) + assert summary["main"]["success"] == 2 + assert summary["main"]["in_progress"] == 1 - def test_upload_tasks_not_double_counted(self): + def test_upload_tasks_counted_correctly(self, db_session): """ - Test that upload tasks are only counted once per destination, - using the latest status. + Test that upload tasks are counted correctly. """ - from datetime import datetime, timedelta + file_record = FileRecord( + filehash="test5", + original_filename="test5.pdf", + local_filename="/tmp/test5.pdf", + file_size=5120 + ) + db_session.add(file_record) + db_session.commit() - class MockLog: - def __init__(self, step_name, status, timestamp): - self.step_name = step_name - self.status = status - self.timestamp = timestamp + initialize_file_steps(db_session, file_record.id) + from app.utils.step_manager import add_upload_steps + add_upload_steps(db_session, file_record.id, ["dropbox", "s3", "nextcloud"]) - now = datetime.now() - # Logs ordered by timestamp desc (latest first) - logs = [ - # Latest status for uploads - MockLog("upload_to_dropbox", "success", now - timedelta(minutes=1)), - MockLog("upload_to_s3", "success", now - timedelta(minutes=2)), - MockLog("upload_to_nextcloud", "success", now - timedelta(minutes=3)), - # Queue logs (older, should use upload_to_ as latest) - MockLog("queue_dropbox", "success", now - timedelta(minutes=4)), - MockLog("queue_s3", "in_progress", now - timedelta(minutes=5)), - MockLog("queue_nextcloud", "success", now - timedelta(minutes=6)), - # Even older in_progress logs - MockLog("upload_to_dropbox", "in_progress", now - timedelta(minutes=7)), - MockLog("upload_to_s3", "in_progress", now - timedelta(minutes=8)), - ] + # Mark upload steps with different statuses + update_step_status(db_session, file_record.id, "upload_to_dropbox", "success") + update_step_status(db_session, file_record.id, "upload_to_s3", "failure") + update_step_status(db_session, file_record.id, "upload_to_nextcloud", "in_progress") - summary = _compute_step_summary(logs) + summary = get_step_summary(db_session, file_record.id) - # Should count unique upload destinations - # Note: queue_X and upload_to_X are separate steps - assert summary["total_upload_tasks"] == 6 # 3 upload_to + 3 queue - assert summary["uploads"]["success"] == 5 # All uploads success, 2 queue success - assert summary["uploads"]["in_progress"] == 1 # 1 queue in_progress - - def test_accurate_metrics_for_completed_file(self): - """ - Test the scenario from the issue: File with 6 actual uploads - should show 6, not 12. - """ - from datetime import datetime, timedelta - - class MockLog: - def __init__(self, step_name, status, timestamp): - self.step_name = step_name - self.status = status - self.timestamp = timestamp - - now = datetime.now() - # Simulate 6 successful uploads with their queue steps - logs = [] - services = ["dropbox", "s3", "nextcloud", "google_drive", "onedrive", "webdav"] - - # Add latest status (all success) - most recent - for i, service in enumerate(services): - logs.append(MockLog(f"upload_to_{service}", "success", now - timedelta(minutes=i * 2))) - logs.append(MockLog(f"queue_{service}", "success", now - timedelta(minutes=i * 2 + 1))) - - # Add some older in_progress logs - base_offset = len(services) * 2 - for i, service in enumerate(services): - logs.append(MockLog(f"upload_to_{service}", "in_progress", now - timedelta(minutes=base_offset + i * 2))) - logs.append(MockLog(f"queue_{service}", "in_progress", now - timedelta(minutes=base_offset + i * 2 + 1))) - - summary = _compute_step_summary(logs) - - # Should have 12 total upload tasks (6 upload_to + 6 queue) - assert summary["total_upload_tasks"] == 12 - # All should be success (latest status) - assert summary["uploads"]["success"] == 12 - assert summary["uploads"]["in_progress"] == 0 - - def test_mixed_upload_statuses(self): - """ - Test that upload metrics correctly reflect mixed statuses. - """ - from datetime import datetime, timedelta - - class MockLog: - def __init__(self, step_name, status, timestamp): - self.step_name = step_name - self.status = status - self.timestamp = timestamp - - now = datetime.now() - logs = [ - # Latest statuses (ordered by timestamp desc) - MockLog("upload_to_dropbox", "success", now - timedelta(minutes=1)), - MockLog("upload_to_s3", "failure", now - timedelta(minutes=2)), - MockLog("upload_to_nextcloud", "in_progress", now - timedelta(minutes=3)), - MockLog("queue_dropbox", "success", now - timedelta(minutes=4)), - MockLog("queue_s3", "success", now - timedelta(minutes=5)), - MockLog("queue_nextcloud", "success", now - timedelta(minutes=6)), - ] - - summary = _compute_step_summary(logs) - - assert summary["total_upload_tasks"] == 6 - assert summary["uploads"]["success"] == 4 # 1 upload + 3 queue - assert summary["uploads"]["failure"] == 1 # 1 upload - assert summary["uploads"]["in_progress"] == 1 # 1 upload - - def test_order_independent_ascending(self): - """ - Test that _compute_step_summary works correctly with ascending order logs - (as used in production by file_detail_page). - - This test ensures the function correctly selects the latest status per step - based on timestamp, not position in the list. - """ - - class MockLog: - def __init__(self, step_name, status, timestamp): - self.step_name = step_name - self.status = status - self.timestamp = timestamp - - now = datetime.now() - # Logs ordered by timestamp ASCENDING (oldest first) - like production - logs = [ - # Older in_progress logs (should be ignored) - MockLog("hash_file", "in_progress", now - timedelta(minutes=7)), - MockLog("create_file_record", "in_progress", now - timedelta(minutes=6)), - MockLog("extract_metadata_with_gpt", "in_progress", now - timedelta(minutes=5)), - MockLog("upload_to_dropbox", "in_progress", now - timedelta(minutes=4)), - # Latest status for each step (all success) - at the end - MockLog("hash_file", "success", now - timedelta(minutes=3)), - MockLog("create_file_record", "success", now - timedelta(minutes=2)), - MockLog("extract_metadata_with_gpt", "success", now - timedelta(minutes=1)), - MockLog("upload_to_dropbox", "success", now), - ] - - summary = _compute_step_summary(logs) - - # Should use latest status (success) not first seen (in_progress) - assert summary["total_main_steps"] == 3 - assert summary["main"]["success"] == 3 - assert summary["main"]["in_progress"] == 0 - - assert summary["total_upload_tasks"] == 1 + # Should count only upload_to_* steps (not queue_* steps) + assert summary["total_upload_tasks"] == 3 assert summary["uploads"]["success"] == 1 - assert summary["uploads"]["in_progress"] == 0 + assert summary["uploads"]["failure"] == 1 + assert summary["uploads"]["in_progress"] == 1 - def test_order_independent_mixed(self): + def test_accurate_metrics_for_completed_file(self, db_session): """ - Test that _compute_step_summary works correctly with randomly ordered logs. - - This ensures the function truly is order-independent. + Test metrics for a file with multiple successful uploads. """ + file_record = FileRecord( + filehash="test6", + original_filename="test6.pdf", + local_filename="/tmp/test6.pdf", + file_size=6144 + ) + db_session.add(file_record) + db_session.commit() - class MockLog: - def __init__(self, step_name, status, timestamp): - self.step_name = step_name - self.status = status - self.timestamp = timestamp + initialize_file_steps(db_session, file_record.id) + from app.utils.step_manager import add_upload_steps + services = ["dropbox", "s3", "nextcloud", "google_drive", "onedrive", "webdav"] + add_upload_steps(db_session, file_record.id, services) - now = datetime.now() - # Logs in mixed order - logs = [ - MockLog("upload_to_s3", "in_progress", now - timedelta(minutes=8)), - MockLog("hash_file", "success", now - timedelta(minutes=1)), # Latest for hash_file - MockLog("upload_to_dropbox", "in_progress", now - timedelta(minutes=7)), - MockLog("hash_file", "in_progress", now - timedelta(minutes=5)), # Older, should be ignored - MockLog("upload_to_s3", "failure", now - timedelta(minutes=2)), # Latest for S3 - MockLog("create_file_record", "in_progress", now - timedelta(minutes=6)), - MockLog("upload_to_dropbox", "success", now), # Latest for Dropbox - MockLog("create_file_record", "success", now - timedelta(minutes=3)), # Latest for create - ] + # Mark all uploads as success + for service in services: + update_step_status(db_session, file_record.id, f"upload_to_{service}", "success") - summary = _compute_step_summary(logs) + summary = get_step_summary(db_session, file_record.id) - # Should correctly identify latest status for each step - assert summary["total_main_steps"] == 2 - assert summary["main"]["success"] == 2 # hash_file and create_file_record - assert summary["main"]["in_progress"] == 0 - - assert summary["total_upload_tasks"] == 2 - assert summary["uploads"]["success"] == 1 # dropbox - assert summary["uploads"]["failure"] == 1 # s3 + # Should have 6 upload_to_* tasks + assert summary["total_upload_tasks"] == 6 + assert summary["uploads"]["success"] == 6 + assert summary["uploads"]["failure"] == 0 assert summary["uploads"]["in_progress"] == 0 diff --git a/tests/test_step_manager.py b/tests/test_step_manager.py index 37c13305..c56722fc 100644 --- a/tests/test_step_manager.py +++ b/tests/test_step_manager.py @@ -51,13 +51,13 @@ class TestFileProcessingStepModel: db_session.commit() # Create a processing step - step = FileProcessingStep(file_id=file_record.id, step_name="hash_file", status="success") + step = FileProcessingStep(file_id=file_record.id, step_name="create_file_record", status="success") db_session.add(step) db_session.commit() assert step.id is not None assert step.file_id == file_record.id - assert step.step_name == "hash_file" + assert step.step_name == "create_file_record" assert step.status == "success" def test_unique_constraint(self, db_session: Session): @@ -70,12 +70,12 @@ class TestFileProcessingStepModel: db_session.commit() # Create first step - step1 = FileProcessingStep(file_id=file_record.id, step_name="hash_file", status="in_progress") + step1 = FileProcessingStep(file_id=file_record.id, step_name="create_file_record", status="in_progress") db_session.add(step1) db_session.commit() # Try to create duplicate step - should fail - step2 = FileProcessingStep(file_id=file_record.id, step_name="hash_file", status="success") + step2 = FileProcessingStep(file_id=file_record.id, step_name="create_file_record", status="success") db_session.add(step2) with pytest.raises(Exception): # SQLAlchemy will raise an integrity error @@ -145,12 +145,12 @@ class TestStepManager: # Update a step that doesn't exist yet now = datetime.now() - update_step_status(db_session, file_record.id, "hash_file", "in_progress", started_at=now) + update_step_status(db_session, file_record.id, "create_file_record", "in_progress", started_at=now) # Verify step was created step = ( db_session.query(FileProcessingStep) - .filter(FileProcessingStep.file_id == file_record.id, FileProcessingStep.step_name == "hash_file") + .filter(FileProcessingStep.file_id == file_record.id, FileProcessingStep.step_name == "create_file_record") .first() ) @@ -172,13 +172,13 @@ class TestStepManager: # Update an existing step now = datetime.now() update_step_status( - db_session, file_record.id, "hash_file", "success", started_at=now - timedelta(seconds=5), completed_at=now + db_session, file_record.id, "create_file_record", "success", started_at=now - timedelta(seconds=5), completed_at=now ) # Verify step was updated step = ( db_session.query(FileProcessingStep) - .filter(FileProcessingStep.file_id == file_record.id, FileProcessingStep.step_name == "hash_file") + .filter(FileProcessingStep.file_id == file_record.id, FileProcessingStep.step_name == "create_file_record") .first() ) @@ -197,21 +197,21 @@ class TestStepManager: initialize_file_steps(db_session, file_record.id) - # Update some steps + # Update some steps - use actual step names from MAIN_PROCESSING_STEPS now = datetime.now() - update_step_status(db_session, file_record.id, "hash_file", "success", completed_at=now) - update_step_status(db_session, file_record.id, "create_file_record", "in_progress", started_at=now) - update_step_status(db_session, file_record.id, "check_text", "failure", error_message="Failed to check text") + update_step_status(db_session, file_record.id, "create_file_record", "success", completed_at=now) + update_step_status(db_session, file_record.id, "check_text", "in_progress", started_at=now) + update_step_status(db_session, file_record.id, "extract_text", "failure", error_message="Failed to extract text") # Get all step statuses status_map = get_file_step_status(db_session, file_record.id) assert len(status_map) == len(MAIN_PROCESSING_STEPS) - assert status_map["hash_file"]["status"] == "success" - assert status_map["hash_file"]["completed_at"] == now - assert status_map["create_file_record"]["status"] == "in_progress" - assert status_map["check_text"]["status"] == "failure" - assert status_map["check_text"]["error_message"] == "Failed to check text" + assert status_map["create_file_record"]["status"] == "success" + assert status_map["create_file_record"]["completed_at"] == now + assert status_map["check_text"]["status"] == "in_progress" + assert status_map["extract_text"]["status"] == "failure" + assert status_map["extract_text"]["error_message"] == "Failed to extract text" def test_get_file_overall_status_pending(self, db_session: Session): """Test overall status for a file with pending steps.""" @@ -242,9 +242,9 @@ class TestStepManager: initialize_file_steps(db_session, file_record.id) # Mark some steps as complete and one as in progress - update_step_status(db_session, file_record.id, "hash_file", "success") update_step_status(db_session, file_record.id, "create_file_record", "success") - update_step_status(db_session, file_record.id, "check_text", "in_progress") + update_step_status(db_session, file_record.id, "check_text", "success") + update_step_status(db_session, file_record.id, "extract_text", "in_progress") status = get_file_overall_status(db_session, file_record.id) @@ -285,9 +285,9 @@ class TestStepManager: initialize_file_steps(db_session, file_record.id) # Mark some steps as success and one as failure - update_step_status(db_session, file_record.id, "hash_file", "success") update_step_status(db_session, file_record.id, "create_file_record", "success") - update_step_status(db_session, file_record.id, "check_text", "failure", error_message="OCR failed") + update_step_status(db_session, file_record.id, "check_text", "success") + update_step_status(db_session, file_record.id, "extract_text", "failure", error_message="OCR failed") status = get_file_overall_status(db_session, file_record.id) @@ -308,12 +308,12 @@ class TestStepManager: add_upload_steps(db_session, file_record.id, ["dropbox", "s3", "nextcloud"]) # Update statuses - update_step_status(db_session, file_record.id, "hash_file", "success") update_step_status(db_session, file_record.id, "create_file_record", "success") - update_step_status(db_session, file_record.id, "check_text", "in_progress") + update_step_status(db_session, file_record.id, "check_text", "success") + update_step_status(db_session, file_record.id, "extract_text", "in_progress") update_step_status(db_session, file_record.id, "upload_to_dropbox", "success") update_step_status(db_session, file_record.id, "upload_to_s3", "failure") - update_step_status(db_session, file_record.id, "queue_nextcloud", "in_progress") + update_step_status(db_session, file_record.id, "upload_to_nextcloud", "in_progress") summary = get_step_summary(db_session, file_record.id) @@ -322,10 +322,10 @@ class TestStepManager: assert summary["main"]["in_progress"] == 1 assert summary["main"]["queued"] >= 5 # Remaining pending steps - # Check upload counts + # Check upload counts (only upload_to_* steps, not queue_* steps) assert summary["uploads"]["success"] == 1 assert summary["uploads"]["failure"] == 1 assert summary["uploads"]["in_progress"] == 1 assert summary["total_main_steps"] == len(MAIN_PROCESSING_STEPS) - assert summary["total_upload_tasks"] == 6 # 3 destinations x 2 steps each + assert summary["total_upload_tasks"] == 3 # 3 upload_to_* steps counted diff --git a/tests/test_storage_reorganization.py b/tests/test_storage_reorganization.py index 8dd30d13..de9f5a5f 100644 --- a/tests/test_storage_reorganization.py +++ b/tests/test_storage_reorganization.py @@ -119,15 +119,74 @@ startxref def test_reprocessing_preserves_original(self, db_session, tmp_path): """Test that reprocessing doesn't create a duplicate original""" from app.tasks.process_document import process_document - - # Create test file and original + + # Create test file and original with valid PDF content + 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 = tmp_path / "test.pdf" - test_pdf.write_bytes(b"%PDF-1.4\ntest") - + test_pdf.write_bytes(pdf_content) + original_dir = tmp_path / "original" original_dir.mkdir() original_file = original_dir / "existing-original.pdf" - original_file.write_bytes(b"%PDF-1.4\noriginal") + original_file.write_bytes(pdf_content) # Create existing file record file_record = FileRecord( diff --git a/tests/test_url_upload.py b/tests/test_url_upload.py index f3d4fdac..bf46e75b 100644 --- a/tests/test_url_upload.py +++ b/tests/test_url_upload.py @@ -167,11 +167,26 @@ class TestURLUploadValidation: class TestURLUploadEndpoint: """Integration tests for URL upload endpoint""" - def test_process_url_requires_authentication(self, client, monkeypatch): + @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.process_document") + def test_process_url_requires_authentication(self, mock_process_document, mock_requests_get, client, monkeypatch): """Test that endpoint requires authentication when auth is enabled""" + # Mock successful download to prevent actual HTTP requests + mock_response = Mock() + mock_response.status_code = 200 + mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "1024"} + mock_response.iter_content = Mock(return_value=[b"PDF content"]) + mock_response.raise_for_status = Mock() + mock_requests_get.return_value = mock_response + + # Mock Celery task + mock_task = Mock() + mock_task.id = "test-task-id" + mock_process_document.delay.return_value = mock_task + # Temporarily enable auth for this test monkeypatch.setenv("AUTH_ENABLED", "True") - + # Since we can't easily reload the app config, we'll just test that the endpoint exists # In production with auth enabled, it would redirect or return 401 response = client.post("/api/process-url", json={"url": "https://example.com/file.pdf"})