diff --git a/BUILD_DATE b/BUILD_DATE index 7d0d30b7..64d12fb0 100644 --- a/BUILD_DATE +++ b/BUILD_DATE @@ -1 +1 @@ -2026-03-16T10:45:13Z +2026-03-16T11:41:49Z diff --git a/CHANGELOG.md b/CHANGELOG.md index 493efb99..606ac39c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,89 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## v0.148.0 (2026-03-16) + +### Documentation + +- Improve docstring and comment clarity in extract_metadata_from_file + ([`f2255f9`](https://github.com/christianlouis/DocuElevate/commit/f2255f9a1c28f138eae7836e9cf2df97f03b7b17)) + +### Features + +- **tasks**: Extract and map embedded PDF metadata in upload_to_email + ([`9d6bfde`](https://github.com/christianlouis/DocuElevate/commit/9d6bfde2882fe74356bef8b64fb79bfa99870f56)) + + +## v0.147.3 (2026-03-16) + +### Code Style + +- Apply ruff auto-fix + ([`ca2d023`](https://github.com/christianlouis/DocuElevate/commit/ca2d023d8130000fb82482fb8fbe0d7e218c94fb)) + +### Performance Improvements + +- **onedrive**: Use async httpx for token refresh + ([`8279795`](https://github.com/christianlouis/DocuElevate/commit/827979598eaeab765a1d24e6011deb26fc804b95)) + +- **onedrive**: Use async httpx for token refresh + ([`2471921`](https://github.com/christianlouis/DocuElevate/commit/24719212042a65ed4a0d99776c6b3974ef97f0e8)) + +- **onedrive**: Use async httpx for token refresh + ([`d1f64eb`](https://github.com/christianlouis/DocuElevate/commit/d1f64ebfba6bb353ea6f75e1535a42fd26a8fe0a)) + +- **onedrive**: Use async httpx for token refresh + ([`7242f3c`](https://github.com/christianlouis/DocuElevate/commit/7242f3c168396aa5400fd46ad531ede93024b467)) + + +## v0.147.2 (2026-03-16) + +### Code Style + +- Apply ruff auto-fix + ([`46c4031`](https://github.com/christianlouis/DocuElevate/commit/46c403127641c1b5c729ff69ab5505efa5c9b54d)) + +### Documentation + +- **changelog**: Update changelog [skip ci] + ([`66fdb11`](https://github.com/christianlouis/DocuElevate/commit/66fdb11e39bc63f5a1d2b652649fd39d2a7e7469)) + +- **changelog**: Update changelog [skip ci] + ([`0f31216`](https://github.com/christianlouis/DocuElevate/commit/0f312160bcb2e46e29c90dd055c4ebc9aaf01ad4)) + +- **changelog**: Update changelog [skip ci] + ([`5734df2`](https://github.com/christianlouis/DocuElevate/commit/5734df2d5046158a23b987fdc4235d3f3f6b4042)) + + +## Unreleased + +### Code Style + +- Apply ruff auto-fix + ([`46c4031`](https://github.com/christianlouis/DocuElevate/commit/46c403127641c1b5c729ff69ab5505efa5c9b54d)) + +### Documentation + +- **changelog**: Update changelog [skip ci] + ([`0f31216`](https://github.com/christianlouis/DocuElevate/commit/0f312160bcb2e46e29c90dd055c4ebc9aaf01ad4)) + +- **changelog**: Update changelog [skip ci] + ([`5734df2`](https://github.com/christianlouis/DocuElevate/commit/5734df2d5046158a23b987fdc4235d3f3f6b4042)) + + +## Unreleased + +### Code Style + +- Apply ruff auto-fix + ([`46c4031`](https://github.com/christianlouis/DocuElevate/commit/46c403127641c1b5c729ff69ab5505efa5c9b54d)) + +### Documentation + +- **changelog**: Update changelog [skip ci] + ([`5734df2`](https://github.com/christianlouis/DocuElevate/commit/5734df2d5046158a23b987fdc4235d3f3f6b4042)) + + ## Unreleased diff --git a/GIT_SHA b/GIT_SHA index b730cdfe..e0b609b3 100644 --- a/GIT_SHA +++ b/GIT_SHA @@ -1 +1 @@ -fd15c36 +1b1cbfc diff --git a/RUNTIME_INFO b/RUNTIME_INFO index 20398c70..0f4c1a92 100644 --- a/RUNTIME_INFO +++ b/RUNTIME_INFO @@ -1,10 +1,10 @@ DocuElevate Build Information ============================== -Version: 0.147.1 -Build Date: 2026-03-16T10:45:13Z -Git Commit: fd15c3666547405bb0a3af37e98be4727ff635bb -Git Short SHA: fd15c36 +Version: 0.148.0 +Build Date: 2026-03-16T11:41:49Z +Git Commit: 1b1cbfce3914277bb8f982d78ef11890f34c9c04 +Git Short SHA: 1b1cbfc Git Branch: main -Commit Date: 2026-03-16T11:44:51+01:00 -Build Timestamp: 2026-03-16T10:45:13Z +Commit Date: 2026-03-16T12:41:28+01:00 +Build Timestamp: 2026-03-16T11:41:49Z ============================== diff --git a/VERSION b/VERSION index c6915b0c..5edc0a74 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.147.1 +0.148.0 diff --git a/app/api/audit_logs.py b/app/api/audit_logs.py index 41a01bcd..3f6fa9a7 100644 --- a/app/api/audit_logs.py +++ b/app/api/audit_logs.py @@ -20,14 +20,16 @@ logger = logging.getLogger(__name__) router = APIRouter() -DbSession = Annotated[Session, Depends(get_db)] +# Module-level dependency singleton to satisfy Ruff B008 while maintaining default values for manual calls (e.g. in decorators). +_db_dep = Depends(get_db) +DbSession = Annotated[Session, _db_dep] @router.get("/audit-logs") @require_login async def list_audit_logs( request: Request, - db: DbSession, + db: DbSession = _db_dep, action: Annotated[str | None, Query(description="Filter by action (exact match)")] = None, user: Annotated[str | None, Query(description="Filter by username")] = None, resource_type: Annotated[str | None, Query(description="Filter by resource type")] = None, @@ -73,7 +75,7 @@ async def list_audit_logs( @require_login async def list_distinct_actions( request: Request, - db: DbSession, + db: DbSession = _db_dep, ) -> list[str]: """Return the distinct action values present in the audit log.""" from app.models import AuditLog @@ -86,7 +88,7 @@ async def list_distinct_actions( @require_login async def list_distinct_users( request: Request, - db: DbSession, + db: DbSession = _db_dep, ) -> list[str]: """Return the distinct user values present in the audit log.""" from app.models import AuditLog diff --git a/app/api/files.py b/app/api/files.py index ddfc2cbb..21e3c18a 100644 --- a/app/api/files.py +++ b/app/api/files.py @@ -1218,6 +1218,66 @@ def download_file( raise HTTPException(status_code=500, detail=f"Error downloading file: {str(e)}") +async def _save_upload_file_chunks(file: UploadFile, target_path: str, max_size: int) -> int: + """Save an uploaded file in chunks and enforce the maximum size limit.""" + try: + written_size = 0 + with open(target_path, "wb") as f: + chunk_size = 65536 # 64 KB chunks + while True: + chunk = await file.read(chunk_size) + if not chunk: + break + written_size += len(chunk) + if written_size > max_size: + # Exceeded limit mid-stream; clean up and reject + f.close() + os.remove(target_path) + raise HTTPException( + status_code=413, + detail=f"File too large: exceeded {max_size} bytes during upload. " + f"See SECURITY_AUDIT.md for configuration details.", + ) + f.write(chunk) + return written_size + except HTTPException: + raise + except Exception as e: + if os.path.exists(target_path): + os.remove(target_path) + raise HTTPException(status_code=500, detail=f"Failed to save file: {e}") + + +def _check_for_exact_duplicate(db: DbSession, target_path: str, safe_filename: str) -> dict | None: + """Check for an exact duplicate of the uploaded file and return a warning if found.""" + if not settings.enable_deduplication: + return None + + try: + filehash = hash_file(target_path) + existing = ( + db.query(FileRecord) + .filter(FileRecord.filehash == filehash, FileRecord.is_duplicate.is_(False)) + .order_by(FileRecord.id.asc()) + .first() + ) + if existing: + logger.info(f"Exact duplicate detected on upload: '{safe_filename}' matches file ID {existing.id}") + return { + "duplicate_type": "exact", + "original_file_id": existing.id, + "original_filename": existing.original_filename, + "message": ( + "This file appears to be an exact duplicate of an already-processed document. " + "It will still be queued but will be flagged as a duplicate." + ), + } + except Exception as e: + logger.warning(f"Duplicate check failed for uploaded file '{safe_filename}': {e}") + + return None + + @router.post("/ui-upload") @require_login async def ui_upload(request: Request, db: DbSession, file: UploadFile = File(...)): @@ -1385,29 +1445,7 @@ async def ui_upload(request: Request, db: DbSession, file: UploadFile = File(... # Check for exact duplicates (same SHA-256 hash) before returning. # This gives the caller an immediate warning without waiting for the pipeline. # Only performed when deduplication is enabled in settings. - exact_duplicate_warning = None - if settings.enable_deduplication: - try: - filehash = hash_file(target_path) - existing = ( - db.query(FileRecord) - .filter(FileRecord.filehash == filehash, FileRecord.is_duplicate.is_(False)) - .order_by(FileRecord.id.asc()) - .first() - ) - if existing: - exact_duplicate_warning = { - "duplicate_type": "exact", - "original_file_id": existing.id, - "original_filename": existing.original_filename, - "message": ( - "This file appears to be an exact duplicate of an already-processed document. " - "It will still be queued but will be flagged as a duplicate." - ), - } - logger.info(f"Exact duplicate detected on upload: '{safe_filename}' matches file ID {existing.id}") - except Exception as e: - logger.warning(f"Duplicate check failed for uploaded file '{safe_filename}': {e}") + exact_duplicate_warning = _check_for_exact_duplicate(db, target_path, safe_filename) response: dict = { "task_id": task.id, diff --git a/app/api/onedrive.py b/app/api/onedrive.py index a9543897..25429344 100644 --- a/app/api/onedrive.py +++ b/app/api/onedrive.py @@ -6,7 +6,7 @@ import logging from datetime import datetime, timedelta from typing import Annotated, Optional -import requests +import httpx from fastapi import APIRouter, Depends, Form, HTTPException, Request, status from sqlalchemy.orm import Session @@ -92,17 +92,18 @@ async def test_onedrive_token(request: Request): "scope": "offline_access Files.ReadWrite", } - response = requests.post(token_url, data=refresh_data, timeout=settings.http_request_timeout) + async with httpx.AsyncClient(timeout=settings.http_request_timeout) as client: + response = await client.post(token_url, data=refresh_data) - if response.status_code != 200: - logger.error(f"Failed to refresh OneDrive token: {response.text}") - return { - "status": "error", - "message": "Refresh token has expired or is invalid", - "needs_reauth": True, - } + if response.status_code != 200: + logger.error(f"Failed to refresh OneDrive token: {response.text}") + return { + "status": "error", + "message": "Refresh token has expired or is invalid", + "needs_reauth": True, + } - token_data = response.json() + token_data = response.json() access_token = token_data.get("access_token") expires_in = token_data.get("expires_in", 3600) # Default to 1 hour if not specified @@ -139,17 +140,18 @@ async def test_onedrive_token(request: Request): user_info_url = "https://graph.microsoft.com/v1.0/me" headers = {"Authorization": f"Bearer {access_token}"} - user_response = requests.get(user_info_url, headers=headers, timeout=settings.http_request_timeout) + async with httpx.AsyncClient(timeout=settings.http_request_timeout) as client: + user_response = await client.get(user_info_url, headers=headers) - if user_response.status_code != 200: - logger.error(f"OneDrive token test failed: {user_response.status_code} {user_response.text}") - return { - "status": "error", - "message": f"Token validation failed with status {user_response.status_code}: {user_response.text}", - } + if user_response.status_code != 200: + logger.error(f"OneDrive token test failed: {user_response.status_code} {user_response.text}") + return { + "status": "error", + "message": f"Token validation failed with status {user_response.status_code}: {user_response.text}", + } - # Get user info - user_info = user_response.json() + # Get user info + user_info = user_response.json() display_name = user_info.get("displayName", "Unknown user") email = user_info.get("userPrincipalName", "Unknown email") diff --git a/app/api/url_upload.py b/app/api/url_upload.py index 78598027..ae286ad3 100644 --- a/app/api/url_upload.py +++ b/app/api/url_upload.py @@ -9,7 +9,8 @@ import urllib.parse import uuid from typing import Optional -import requests +import aiofiles +import httpx from fastapi import APIRouter, HTTPException, Request from pydantic import BaseModel, HttpUrl, field_validator @@ -153,67 +154,73 @@ async def process_url(request: Request, url_request: URLUploadRequest): logger.info(f"Downloading file from URL: {url}") # Use configured timeout to prevent hanging - response = requests.get( - url, + async with httpx.AsyncClient( timeout=settings.http_request_timeout, - stream=True, # Stream to handle large files - allow_redirects=True, # Follow redirects + follow_redirects=True, headers={ "User-Agent": "DocuElevate/1.0", # Identify ourselves }, - ) - response.raise_for_status() + ) as client: + async with client.stream("GET", url) as response: + response.raise_for_status() - # Validate content type - content_type = response.headers.get("Content-Type", "") - if not validate_file_type(content_type, safe_filename): - raise HTTPException( - status_code=400, - detail=f"Unsupported file type: {content_type}. " - "Supported types: PDF, Office documents, images, plain text", - ) + # Validate content type + content_type = response.headers.get("Content-Type", "") + if not validate_file_type(content_type, safe_filename): + raise HTTPException( + status_code=400, + detail=f"Unsupported file type: {content_type}. " + "Supported types: PDF, Office documents, images, plain text", + ) - # Check content length before downloading - content_length = response.headers.get("Content-Length") - if content_length: - file_size = int(content_length) - max_size = settings.max_upload_size - if file_size > max_size: - raise HTTPException( - status_code=413, - detail=f"File too large: {file_size} bytes (max {max_size} bytes)", - ) - - # Generate unique filename - unique_id = str(uuid.uuid4()) - if "." in safe_filename: - file_extension = safe_filename.rsplit(".", 1)[1] - target_filename = f"{unique_id}.{file_extension}" - else: - target_filename = unique_id - - target_path = os.path.join(settings.workdir, target_filename) - - # Download file in chunks to handle large files - downloaded_size = 0 - max_size = settings.max_upload_size - - with open(target_path, "wb") as f: - for chunk in response.iter_content(chunk_size=8192): - if chunk: - f.write(chunk) - downloaded_size += len(chunk) - - # Check size during download - if downloaded_size > max_size: - # Remove partial file - f.close() - os.remove(target_path) + # Check content length before downloading + content_length = response.headers.get("Content-Length") + if content_length: + file_size = int(content_length) + max_size = settings.max_upload_size + if file_size > max_size: raise HTTPException( status_code=413, - detail=f"File too large: exceeded {max_size} bytes during download", + detail=f"File too large: {file_size} bytes (max {max_size} bytes)", ) + # Generate unique filename + unique_id = str(uuid.uuid4()) + + # Check for extension using original_filename to avoid any CodeQL issues + # with safe_filename which is derived from the URL directly. + if "." in original_filename: + _, ext = os.path.splitext(original_filename) + # Strip out the leading dot and any non-alphanumeric chars + clean_ext = "".join(c for c in ext if c.isalnum()) + if not clean_ext: + clean_ext = "bin" + target_filename = f"{unique_id}.{clean_ext}" + else: + target_filename = unique_id + + target_path = os.path.join(settings.workdir, target_filename) + + # Download file in chunks to handle large files + downloaded_size = 0 + max_size = settings.max_upload_size + + async with aiofiles.open(target_path, "wb") as f: + async for chunk in response.aiter_bytes(chunk_size=8192): + if chunk: + await f.write(chunk) + downloaded_size += len(chunk) + + # Check size during download + if downloaded_size > max_size: + # Remove partial file + await f.close() + os.remove(target_path) + raise HTTPException( + status_code=413, + detail=f"File too large: exceeded {max_size} bytes during download", + ) + logger.info(f"Downloaded file from URL '{url}' as '{target_filename}' ({downloaded_size} bytes)") # Enqueue for processing @@ -227,19 +234,19 @@ async def process_url(request: Request, url_request: URLUploadRequest): "size": downloaded_size, } - except requests.exceptions.Timeout: + except httpx.TimeoutException: logger.error(f"Timeout while downloading file from URL: {url}") raise HTTPException(status_code=408, detail="Request timeout: server took too long to respond") - except requests.exceptions.ConnectionError as e: + except httpx.ConnectError as e: logger.error(f"Connection error while downloading file from URL: {url} - {str(e)}") raise HTTPException(status_code=502, detail=f"Failed to connect to URL: {str(e)}") - except requests.exceptions.HTTPError as e: + except httpx.HTTPStatusError as e: logger.error(f"HTTP error while downloading file from URL: {url} - {str(e)}") raise HTTPException(status_code=e.response.status_code, detail=f"HTTP error: {str(e)}") - except requests.exceptions.RequestException as e: + except httpx.RequestError as e: logger.error(f"Error downloading file from URL: {url} - {str(e)}") raise HTTPException(status_code=500, detail=f"Failed to download file: {str(e)}") diff --git a/app/tasks/upload_to_email.py b/app/tasks/upload_to_email.py index c68e4bf7..378ec7d7 100644 --- a/app/tasks/upload_to_email.py +++ b/app/tasks/upload_to_email.py @@ -24,6 +24,15 @@ logger = logging.getLogger(__name__) # Constants _LOGO_FILENAME = "logo.png" +# Mapping from PDF metadata keys (with leading slash stripped) to application-specific names. +# This mirrors the inverse of the mapping used in app/tasks/embed_metadata_into_pdf.py. +_PDF_METADATA_KEY_MAP = { + "Title": "filename", + "Author": "absender", + "Subject": "document_type", + "Keywords": "tags", +} + def get_email_template(template_name="default.html"): """ @@ -64,9 +73,12 @@ def extract_metadata_from_file(file_path): """ Try to extract metadata from a file using several methods: 1. Check for a .json metadata file with the same name - 2. Extract metadata from PDF if it's embedded + 2. Extract embedded metadata from PDF using pypdf - Returns a dictionary of metadata or None if not found + JSON metadata takes precedence; embedded PDF metadata fills in any missing + fields using the application's standard key mapping (e.g., /Title → filename). + + Returns a dictionary of metadata (may be empty if none found). """ metadata = {} @@ -77,7 +89,6 @@ def extract_metadata_from_file(file_path): with open(metadata_path, "r", encoding="utf-8") as f: metadata = json.load(f) logger.info(f"Loaded metadata from external JSON file: {metadata_path}") - return metadata except Exception as e: logger.warning(f"Failed to load metadata from JSON file: {str(e)}") @@ -88,11 +99,14 @@ def extract_metadata_from_file(file_path): pdf_reader = pypdf.PdfReader(f) pdf_metadata = pdf_reader.metadata if pdf_metadata: - # Convert metadata to a standard dictionary for key, value in pdf_metadata.items(): # Remove the leading slash from PDF metadata keys (e.g., '/Title' -> 'Title') clean_key = key[1:] if key.startswith("/") else key - metadata[clean_key] = str(value) + # Map to application-specific key names where possible + mapped_key = _PDF_METADATA_KEY_MAP.get(clean_key, clean_key) + # Only set if not already present (JSON metadata takes precedence) + if mapped_key not in metadata: + metadata[mapped_key] = str(value) logger.info(f"Extracted embedded metadata from PDF: {file_path}") except Exception as e: diff --git a/benchmark_onedrive.py b/benchmark_onedrive.py new file mode 100644 index 00000000..7dc81447 --- /dev/null +++ b/benchmark_onedrive.py @@ -0,0 +1,69 @@ +import asyncio +import time +import httpx +from unittest.mock import patch, MagicMock, AsyncMock +from app.api.onedrive import test_onedrive_token +from app.config import settings + +settings.onedrive_refresh_token = "dummy" +settings.onedrive_client_id = "dummy" +settings.onedrive_client_secret = "dummy" + +class DummyRequest: + def __init__(self): + self.session = {"user": "dummy"} + +async def run_benchmark(func_name, mock_post, mock_get): + mock_post_resp = MagicMock() + mock_post_resp.status_code = 200 + mock_post_resp.json.return_value = { + "access_token": "dummy_access", + "expires_in": 3600 + } + mock_post.return_value = mock_post_resp + + mock_get_resp = MagicMock() + mock_get_resp.status_code = 200 + mock_get_resp.json.return_value = { + "displayName": "Test User", + "userPrincipalName": "test@example.com" + } + mock_get.return_value = mock_get_resp + + start_time = time.time() + for _ in range(100): + await test_onedrive_token(DummyRequest()) + end_time = time.time() + print(f"{func_name} took {end_time - start_time:.4f} seconds") + +async def run_benchmark_async(func_name, mock_post, mock_get): + mock_post_resp = MagicMock() + mock_post_resp.status_code = 200 + mock_post_resp.json = MagicMock(return_value={ + "access_token": "dummy_access", + "expires_in": 3600 + }) + mock_post.return_value = mock_post_resp + + mock_get_resp = MagicMock() + mock_get_resp.status_code = 200 + mock_get_resp.json = MagicMock(return_value={ + "displayName": "Test User", + "userPrincipalName": "test@example.com" + }) + mock_get.return_value = mock_get_resp + + start_time = time.time() + for _ in range(100): + await test_onedrive_token(DummyRequest()) + end_time = time.time() + print(f"{func_name} took {end_time - start_time:.4f} seconds") + + +@patch('app.api.onedrive.requests.get') +@patch('app.api.onedrive.requests.post') +def benchmark_sync(mock_post, mock_get): + asyncio.run(run_benchmark("Sync requests (baseline)", mock_post, mock_get)) + +if __name__ == "__main__": + benchmark_sync() diff --git a/benchmark_url_upload.py b/benchmark_url_upload.py new file mode 100644 index 00000000..e41a33bd --- /dev/null +++ b/benchmark_url_upload.py @@ -0,0 +1,49 @@ +import asyncio +import time +from unittest.mock import Mock, patch + +from app.api.url_upload import process_url, URLUploadRequest +from app.config import settings + +async def main(): + # Mock request and URLUploadRequest + request = Mock() + url_request = URLUploadRequest(url="https://example.com/file.pdf") + + # Generate a large chunk + large_chunk = b"A" * 8192 + num_chunks = 10000 # 8192 * 10000 = ~80MB + + mock_response = Mock() + mock_response.status_code = 200 + mock_response.headers = {"Content-Type": "application/pdf"} + mock_response.iter_content = Mock(return_value=[large_chunk] * num_chunks) + + # For async client later + class AsyncMockResponse: + def __init__(self): + self.status_code = 200 + self.headers = {"Content-Type": "application/pdf"} + def raise_for_status(self): + pass + async def aiter_bytes(self, chunk_size): + for _ in range(num_chunks): + yield large_chunk + + async_mock_response = AsyncMockResponse() + + # We will mock requests.get for synchronous, httpx.AsyncClient.get for asynchronous + + # Test sync + start_time = time.time() + with patch("app.api.url_upload.requests.get", return_value=mock_response), \ + patch("app.api.url_upload.process_document"): + try: + await process_url(request=request, url_request=url_request) + except Exception as e: + print(f"Error: {e}") + end_time = time.time() + print(f"Original execution time (sync writing): {end_time - start_time:.4f} seconds") + +if __name__ == "__main__": + asyncio.run(main()) diff --git a/benchmark_url_upload2.py b/benchmark_url_upload2.py new file mode 100644 index 00000000..29dcdde2 --- /dev/null +++ b/benchmark_url_upload2.py @@ -0,0 +1,75 @@ +import asyncio +import time +import os +import shutil +import tempfile +from unittest.mock import Mock, patch +from fastapi import HTTPException + +from app.api.url_upload import process_url, URLUploadRequest +from app.config import settings + +async def main(): + # Setup test dir + test_dir = tempfile.mkdtemp() + settings.workdir = test_dir + + # Mock request and URLUploadRequest + request = Mock() + url_request = URLUploadRequest(url="https://example.com/file.pdf") + + # Generate a large chunk + chunk_size = 8192 + num_chunks = 20000 # 20000 * 8192 = ~160MB + large_chunk = b"A" * chunk_size + + class SyncMockResponse: + def __init__(self): + self.status_code = 200 + self.headers = {"Content-Type": "application/pdf"} + def raise_for_status(self): + pass + def iter_content(self, chunk_size): + for _ in range(num_chunks): + # sleep slightly to simulate network latency, otherwise OS file cache obscures the difference + time.sleep(0.0001) + yield large_chunk + + sync_mock_response = SyncMockResponse() + + class AsyncMockResponse: + def __init__(self): + self.status_code = 200 + self.headers = {"Content-Type": "application/pdf"} + self.is_success = True + self.status_code = 200 + def raise_for_status(self): + pass + async def aiter_bytes(self, chunk_size=8192): + for _ in range(num_chunks): + await asyncio.sleep(0.0001) + yield large_chunk + + class AsyncMockContext: + async def __aenter__(self): + return AsyncMockResponse() + async def __aexit__(self, exc_type, exc_val, exc_tb): + pass + + async_mock_response = AsyncMockResponse() + + # Test sync + start_time = time.time() + with patch("app.api.url_upload.requests.get", return_value=sync_mock_response), \ + patch("app.api.url_upload.process_document"): + try: + await process_url(request=request, url_request=url_request) + except Exception as e: + print(f"Error (sync): {e}") + end_time = time.time() + print(f"Original execution time (sync writing): {end_time - start_time:.4f} seconds") + + shutil.rmtree(test_dir) + +if __name__ == "__main__": + asyncio.run(main()) diff --git a/fix_test7.py b/fix_test7.py new file mode 100644 index 00000000..ba0c6587 --- /dev/null +++ b/fix_test7.py @@ -0,0 +1,53 @@ +import re + +with open("tests/test_api_saved_searches.py", "r") as f: + content = f.read() + +# We need to mock get_current_user in app.api.saved_searches (which is imported from app.auth) +# because saved searches uses `_get_user_id` which calls `get_current_user(request)`. +# But `_get_user_id` is NOT a dependency injected via `Depends`! +# Let's verify `app/api/saved_searches.py` uses `Depends` or just calls it. + +# In `app/api/saved_searches.py`: +# def _get_user_id(request: Request) -> str: +# user = get_current_user(request) +# if user: +# return user.get("preferred_username") ... +# It's called directly inside the routes: `user_id = _get_user_id(request)` +# It doesn't use `Depends(_get_user_id)`. +# Ah! But earlier I saw `_get_user_id` wasn't mocked properly. Let's use patch to mock `_get_user_id`. + +# Wait, `TestClient` can be given an active session, but `app.auth.get_current_user` uses `request.session.get("user")` or Bearer token. +# Is `AUTH_ENABLED` false? The test env has `os.environ["AUTH_ENABLED"] = "False"` in `tests/conftest.py`. +# If `AUTH_ENABLED` is false, `require_login` is a no-op, and `_get_user_id` falls back to "anonymous". +# Actually, `_get_user_id` returns "anonymous" if `get_current_user(request)` is None. +# If `_OWNER` is "test_user@example.com", we should probably just patch `_get_user_id`. + +replacement = """def _make_client(int_engine, owner_id: str = _OWNER): + \"\"\"Return a TestClient with *owner_id* injected as the authenticated user.\"\"\" + from app.main import app + from unittest.mock import patch + + def override_db(): + Session = sessionmaker(bind=int_engine) + session = Session() + try: + yield session + finally: + session.close() + + app.dependency_overrides[get_db] = override_db + with patch("app.api.saved_searches._get_user_id", return_value=owner_id): + with TestClient(app, base_url="http://localhost", raise_server_exceptions=False) as client: + yield client + app.dependency_overrides.clear()""" + +content = re.sub( + r"def _make_client\(int_engine, owner_id: str = _OWNER\):.*?(?=@pytest\.fixture\(\)\ndef int_client\(int_engine\):)", + replacement + "\n\n\n", + content, + flags=re.DOTALL +) + +with open("tests/test_api_saved_searches.py", "w") as f: + f.write(content) diff --git a/requirements-dev.txt b/requirements-dev.txt index 43a5ca21..bf7a8924 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -38,4 +38,5 @@ pip-licenses==5.5.1 # For license compliance checking # Release automation python-semantic-release>=9.0.0 + types-aiofiles>=24.1.0.20240311 # Type stubs for aiofiles diff --git a/requirements.txt b/requirements.txt index f65c9262..b4f35b22 100644 --- a/requirements.txt +++ b/requirements.txt @@ -50,12 +50,13 @@ litellm>=1.0.0,<2.0.0 pytesseract>=0.3.10 # Python wrapper for Tesseract OCR pdf2image>=1.17.0 # Convert PDF pages to images (used by Tesseract and EasyOCR providers) ocrmypdf>=16.0.0,<18.0.0 # Post-processing: embeds searchable text layers into PDFs via Tesseract -meilisearch>=0.31.0 # Full-text search engine client +meilisearch>=0.31.0 # Full-text search engine client stripe>=7.0.0,<15.0.0 # Stripe billing SDK (MIT license) # Error and performance monitoring sentry-sdk[fastapi,celery,sqlalchemy]>=2.20.0,<3.0.0 # GraphQL API -strawberry-graphql[fastapi]>=0.243.0,<1.0.0 -aiofiles>=24.1.0 # Asynchronous file I/O support +strawberry-graphql[fastapi]>=0.243.0,<1.0.0 + +aiofiles>=24.1.0 # Asynchronous file I/O support diff --git a/tests/test_api_onedrive_coverage.py b/tests/test_api_onedrive_coverage.py index 6e0ed1bc..edf97b31 100644 --- a/tests/test_api_onedrive_coverage.py +++ b/tests/test_api_onedrive_coverage.py @@ -5,7 +5,7 @@ Focuses on uncovered lines: 98-99, 121-143, 160-161, 170-171, 324-326, 400-402, 436-438. """ -from unittest.mock import MagicMock, PropertyMock, patch +from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch import pytest from fastapi.testclient import TestClient @@ -15,7 +15,7 @@ from fastapi.testclient import TestClient class TestTestTokenRefreshFailed: """Cover lines 98-99: token refresh returns non-200.""" - @patch("app.api.onedrive.requests.post") + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) def test_test_token_refresh_returns_non_200(self, mock_post, client: TestClient): """Test token refresh returning a failure status hits the error branch.""" from app.config import settings @@ -42,8 +42,8 @@ class TestTestTokenRefreshFailed: class TestTestTokenRotation: """Cover lines 121-143, 160-161: token rotation with .env and DB persist.""" - @patch("app.api.onedrive.requests.get") - @patch("app.api.onedrive.requests.post") + @patch("httpx.AsyncClient.get", new_callable=AsyncMock) + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) def test_token_rotation_env_file_exists(self, mock_post, mock_get, client: TestClient, tmp_path): """When a new refresh token is received and .env file exists, it should be updated.""" from app.config import settings @@ -90,8 +90,8 @@ class TestTestTokenRotation: data = response.json() assert data["status"] == "success" - @patch("app.api.onedrive.requests.get") - @patch("app.api.onedrive.requests.post") + @patch("httpx.AsyncClient.get", new_callable=AsyncMock) + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) def test_token_rotation_env_not_existing(self, mock_post, mock_get, client: TestClient): """Token rotation when .env doesn't exist still succeeds.""" from app.config import settings @@ -130,8 +130,8 @@ class TestTestTokenRotation: assert response.status_code == 200 assert response.json()["status"] == "success" - @patch("app.api.onedrive.requests.get") - @patch("app.api.onedrive.requests.post") + @patch("httpx.AsyncClient.get", new_callable=AsyncMock) + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) def test_token_rotation_env_write_failure(self, mock_post, mock_get, client: TestClient): """Token rotation when .env write fails (lines 142-143) still continues.""" from app.config import settings @@ -171,8 +171,8 @@ class TestTestTokenRotation: assert response.status_code == 200 assert response.json()["status"] == "success" - @patch("app.api.onedrive.requests.get") - @patch("app.api.onedrive.requests.post") + @patch("httpx.AsyncClient.get", new_callable=AsyncMock) + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) def test_token_rotation_db_persist_failure(self, mock_post, mock_get, client: TestClient): """Token rotation when DB persist fails (lines 160-161) still continues.""" from app.config import settings @@ -211,8 +211,8 @@ class TestTestTokenRotation: class TestTestTokenUserInfoFailed: """Cover lines 170-171: user info request fails.""" - @patch("app.api.onedrive.requests.get") - @patch("app.api.onedrive.requests.post") + @patch("httpx.AsyncClient.get", new_callable=AsyncMock) + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) def test_user_info_returns_non_200(self, mock_post, mock_get, client: TestClient): """Test when user info request fails after successful token refresh.""" from app.config import settings @@ -247,8 +247,8 @@ class TestTestTokenUserInfoFailed: class TestTokenRotationEnvAppendLine: """Cover the branch at line 134 where token line is not found in .env and must be appended.""" - @patch("app.api.onedrive.requests.get") - @patch("app.api.onedrive.requests.post") + @patch("httpx.AsyncClient.get", new_callable=AsyncMock) + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) def test_token_rotation_appends_to_env(self, mock_post, mock_get, client: TestClient, tmp_path): """When .env exists but doesn't have ONEDRIVE_REFRESH_TOKEN, it should append.""" from app.config import settings diff --git a/tests/test_api_onedrive_extended.py b/tests/test_api_onedrive_extended.py index 3a4085a5..5a5d230a 100644 --- a/tests/test_api_onedrive_extended.py +++ b/tests/test_api_onedrive_extended.py @@ -1,7 +1,7 @@ """Comprehensive unit tests for app/api/onedrive.py module.""" from datetime import timedelta -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -48,8 +48,8 @@ class TestExchangeOneDriveToken: class TestTestOneDriveToken: """Tests for GET /onedrive/test-token endpoint.""" - @patch("app.api.onedrive.requests.post") - @patch("app.api.onedrive.requests.get") + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) + @patch("httpx.AsyncClient.get", new_callable=AsyncMock) def test_test_token_success(self, mock_get, mock_post): """Test successful token validation.""" from app.config import settings @@ -79,7 +79,7 @@ class TestTestOneDriveToken: # Should return success pass - @patch("app.api.onedrive.requests.post") + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) def test_test_token_not_configured(self, mock_post): """Test when credentials are not configured.""" from app.config import settings @@ -88,7 +88,7 @@ class TestTestOneDriveToken: # Should return error pass - @patch("app.api.onedrive.requests.post") + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) def test_test_token_refresh_failed(self, mock_post): """Test when token refresh fails.""" from app.config import settings @@ -104,8 +104,8 @@ class TestTestOneDriveToken: # Should return error with needs_reauth pass - @patch("app.api.onedrive.requests.post") - @patch("app.api.onedrive.requests.get") + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) + @patch("httpx.AsyncClient.get", new_callable=AsyncMock) def test_test_token_user_info_failed(self, mock_get, mock_post): """Test when user info request fails.""" from app.config import settings @@ -128,8 +128,8 @@ class TestTestOneDriveToken: # Should return error pass - @patch("app.api.onedrive.requests.post") - @patch("app.api.onedrive.requests.get") + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) + @patch("httpx.AsyncClient.get", new_callable=AsyncMock) @patch("builtins.open", create=True) @patch("os.path.exists") def test_test_token_updates_refresh_token(self, mock_exists, mock_open, mock_get, mock_post): @@ -167,8 +167,8 @@ class TestTestOneDriveToken: # Should update refresh token in memory and file pass - @patch("app.api.onedrive.requests.post") - @patch("app.api.onedrive.requests.get") + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) + @patch("httpx.AsyncClient.get", new_callable=AsyncMock) def test_test_token_expiration_info(self, mock_get, mock_post): """Test that expiration info is included.""" from app.config import settings @@ -195,7 +195,7 @@ class TestTestOneDriveToken: # token_info should include expiration details pass - @patch("app.api.onedrive.requests.post") + @patch("httpx.AsyncClient.post", new_callable=AsyncMock) def test_test_token_exception_handling(self, mock_post): """Test handling of exceptions.""" from app.config import settings diff --git a/tests/test_api_saved_searches.py b/tests/test_api_saved_searches.py new file mode 100644 index 00000000..d483390b --- /dev/null +++ b/tests/test_api_saved_searches.py @@ -0,0 +1,191 @@ +"""Tests for the saved searches API (app/api/saved_searches.py).""" + +import pytest +from fastapi.testclient import TestClient +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker +from sqlalchemy.pool import StaticPool + +from app.database import Base, get_db +from app.models import SavedSearch + +# --------------------------------------------------------------------------- +# Test data constants +# --------------------------------------------------------------------------- + +_OWNER = "test_user@example.com" +_OTHER_OWNER = "other_user@example.com" + + +# --------------------------------------------------------------------------- +# Shared fixture helpers +# --------------------------------------------------------------------------- + + +@pytest.fixture() +def int_engine(): + """In-memory SQLite engine for integration tests.""" + engine = create_engine( + "sqlite:///:memory:", + connect_args={"check_same_thread": False}, + poolclass=StaticPool, + ) + Base.metadata.create_all(bind=engine) + yield engine + Base.metadata.drop_all(bind=engine) + + +@pytest.fixture() +def int_session(int_engine): + """DB session scoped to one test.""" + Session = sessionmaker(bind=int_engine) + session = Session() + yield session + session.close() + + +def _make_client(int_engine, owner_id: str = _OWNER): + """Return a TestClient with *owner_id* injected as the authenticated user.""" + from unittest.mock import patch + + from app.main import app + + def override_db(): + Session = sessionmaker(bind=int_engine) + session = Session() + try: + yield session + finally: + session.close() + + app.dependency_overrides[get_db] = override_db + with patch("app.api.saved_searches._get_user_id", return_value=owner_id): + with TestClient(app, base_url="http://localhost", raise_server_exceptions=False) as client: + yield client + app.dependency_overrides.clear() + + +@pytest.fixture() +def int_client(int_engine): + """TestClient authenticated as _OWNER.""" + yield from _make_client(int_engine, _OWNER) + + +# --------------------------------------------------------------------------- +# CRUD tests +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +class TestSavedSearchesAPI: + """Tests for Saved Searches endpoints.""" + + def test_list_saved_searches_empty(self, int_client): + """No saved searches returns empty list.""" + resp = int_client.get("/api/saved-searches") + assert resp.status_code == 200 + assert resp.json() == [] + + def test_create_saved_search(self, int_client): + """Create a saved search and verify the response.""" + payload = {"name": "My Invoices", "filters": {"tags": "invoice", "document_type": "Invoice"}} + resp = int_client.post("/api/saved-searches", json=payload) + assert resp.status_code == 201 + data = resp.json() + assert data["name"] == "My Invoices" + assert data["filters"] == {"tags": "invoice", "document_type": "Invoice"} + assert "id" in data + + def test_create_saved_search_invalid_filters(self, int_client): + """Creating with invalid filters returns 422.""" + # Missing filters parameter (or empty after sanitization) + payload = {"name": "My Invoices", "filters": {}} + resp = int_client.post("/api/saved-searches", json=payload) + assert resp.status_code == 422 + + # Invalid filters format + payload2 = {"name": "My Invoices", "filters": "not_a_dict"} + resp2 = int_client.post("/api/saved-searches", json=payload2) + assert resp2.status_code == 422 + + def test_create_saved_search_duplicate(self, int_client): + """Creating a duplicate named search returns 409.""" + payload = {"name": "Duplicate", "filters": {"q": "test"}} + int_client.post("/api/saved-searches", json=payload) + resp = int_client.post("/api/saved-searches", json=payload) + assert resp.status_code == 409 + + def test_create_saved_search_limit(self, int_client, int_session): + """Exceeding MAX_SAVED_SEARCHES_PER_USER returns 409.""" + # Create 50 searches using the API to ensure they are visible + for i in range(50): + resp = int_client.post("/api/saved-searches", json={"name": f"Search LIMIT {i}", "filters": {"q": "test"}}) + assert resp.status_code == 201 + + payload = {"name": "One too many", "filters": {"q": "test"}} + resp = int_client.post("/api/saved-searches", json=payload) + assert resp.status_code == 409 + + def test_update_saved_search(self, int_client): + """Update an existing saved search.""" + payload = {"name": "Original Name", "filters": {"q": "test"}} + created = int_client.post("/api/saved-searches", json=payload).json() + search_id = created["id"] + + update_payload = {"name": "Updated Name", "filters": {"tags": "new"}} + resp = int_client.put(f"/api/saved-searches/{search_id}", json=update_payload) + assert resp.status_code == 200 + data = resp.json() + assert data["name"] == "Updated Name" + assert data["filters"] == {"tags": "new"} + + def test_update_saved_search_not_found(self, int_client): + """Updating a non-existent search returns 404.""" + update_payload = {"name": "Updated Name"} + resp = int_client.put("/api/saved-searches/999", json=update_payload) + assert resp.status_code == 404 + + def test_update_saved_search_duplicate_name(self, int_client): + """Updating name to an existing search name returns 409.""" + payload1 = {"name": "Search 1", "filters": {"q": "a"}} + payload2 = {"name": "Search 2", "filters": {"q": "b"}} + int_client.post("/api/saved-searches", json=payload1) + created2 = int_client.post("/api/saved-searches", json=payload2).json() + search2_id = created2["id"] + + update_payload = {"name": "Search 1"} + resp = int_client.put(f"/api/saved-searches/{search2_id}", json=update_payload) + assert resp.status_code == 409 + + def test_delete_saved_search(self, int_client, int_session): + """Delete an existing search.""" + payload = {"name": "To be deleted", "filters": {"q": "test"}} + created = int_client.post("/api/saved-searches", json=payload).json() + search_id = created["id"] + + resp = int_client.delete(f"/api/saved-searches/{search_id}") + assert resp.status_code == 204 + + assert int_session.query(SavedSearch).filter(SavedSearch.id == search_id).first() is None + + def test_delete_saved_search_not_found(self, int_client): + """Deleting a non-existent search returns 404.""" + resp = int_client.delete("/api/saved-searches/999") + assert resp.status_code == 404 + + def test_other_users_searches_isolated(self, int_engine, int_session): + """Users only see and can only modify their own saved searches.""" + int_session.add(SavedSearch(user_id=_OTHER_OWNER, name="Other Search", filters='{"q": "test"}')) + int_session.commit() + + client = next(_make_client(int_engine, _OWNER)) + resp = client.get("/api/saved-searches") + assert resp.status_code == 200 + assert len(resp.json()) == 0 + + other_search = int_session.query(SavedSearch).first() + resp = client.put(f"/api/saved-searches/{other_search.id}", json={"name": "Hacked"}) + assert resp.status_code == 404 + + resp = client.delete(f"/api/saved-searches/{other_search.id}") + assert resp.status_code == 404 diff --git a/tests/test_external_integrations.py b/tests/test_external_integrations.py index 5fab2016..2d7954b5 100644 --- a/tests/test_external_integrations.py +++ b/tests/test_external_integrations.py @@ -417,14 +417,20 @@ class TestOneDriveIntegration: def test_onedrive_token_refresh_and_user_info(self, original_env: dict) -> None: """Validate token refresh and user info retrieval.""" - import requests + import asyncio + + import httpx token = self._get_access_token(original_env) - resp = requests.get( - "https://graph.microsoft.com/v1.0/me", - headers={"Authorization": f"Bearer {token}"}, - timeout=30, - ) + + async def _test(): + async with httpx.AsyncClient(timeout=30) as client: + return await client.get( + "https://graph.microsoft.com/v1.0/me", + headers={"Authorization": f"Bearer {token}"}, + ) + + resp = asyncio.run(_test()) assert resp.status_code == 200, f"OneDrive user info failed: {resp.text}" def test_onedrive_upload_download_delete(self, original_env: dict) -> None: diff --git a/tests/test_upload_email.py b/tests/test_upload_email.py index d84dc59f..f1e57ef2 100644 --- a/tests/test_upload_email.py +++ b/tests/test_upload_email.py @@ -143,11 +143,72 @@ class TestExtractMetadataFromFile: result = extract_metadata_from_file(str(file_path)) - # Check that the leading slash is stripped and keys/values match - assert result.get("Title") == "Test Title" - assert result.get("Author") == "Test Author" - assert result.get("Subject") == "Test Document" - assert result.get("Keywords") == "test, metadata, pypdf" + # Keys are mapped to application-specific names + assert result.get("filename") == "Test Title" + assert result.get("absender") == "Test Author" + assert result.get("document_type") == "Test Document" + assert result.get("tags") == "test, metadata, pypdf" + + def test_extracts_embedded_metadata_from_pdf(self, tmp_path): + """Test that embedded PDF metadata is mapped to application-specific keys.""" + import pypdf + + file_path = tmp_path / "mapped.pdf" + + writer = pypdf.PdfWriter() + writer.add_blank_page(width=100, height=100) + writer.add_metadata( + { + "/Title": "Invoice 2024", + "/Author": "Acme Corp", + "/Subject": "invoice", + "/Keywords": "finance, billing", + } + ) + with open(file_path, "wb") as f: + writer.write(f) + + result = extract_metadata_from_file(str(file_path)) + + # Verify the PDF-to-app key mapping + assert result["filename"] == "Invoice 2024" + assert result["absender"] == "Acme Corp" + assert result["document_type"] == "invoice" + assert result["tags"] == "finance, billing" + + def test_pdf_metadata_does_not_overwrite_json(self, tmp_path): + """Test that JSON metadata takes precedence over embedded PDF metadata.""" + import pypdf + + file_path = tmp_path / "dual.pdf" + + # Create a PDF with embedded metadata + writer = pypdf.PdfWriter() + writer.add_blank_page(width=100, height=100) + writer.add_metadata( + { + "/Title": "PDF Title", + "/Author": "PDF Author", + "/Subject": "PDF Subject", + "/Keywords": "pdf, keywords", + } + ) + with open(file_path, "wb") as f: + writer.write(f) + + # Create a companion JSON file that sets some overlapping fields + json_metadata = {"filename": "JSON Filename", "absender": "JSON Author"} + json_path = tmp_path / "dual.json" + json_path.write_text(json.dumps(json_metadata)) + + result = extract_metadata_from_file(str(file_path)) + + # JSON values must not be overwritten by PDF metadata + assert result["filename"] == "JSON Filename" + assert result["absender"] == "JSON Author" + # Fields missing from JSON are filled from PDF metadata + assert result["document_type"] == "PDF Subject" + assert result["tags"] == "pdf, keywords" @pytest.mark.unit diff --git a/tests/test_url_upload.py b/tests/test_url_upload.py index 3204d8a9..7fead0ae 100644 --- a/tests/test_url_upload.py +++ b/tests/test_url_upload.py @@ -2,10 +2,10 @@ Tests for URL-based file upload functionality """ -from unittest.mock import Mock, patch +from unittest.mock import AsyncMock, MagicMock, Mock, patch +import httpx import pytest -import requests @pytest.mark.unit @@ -165,17 +165,24 @@ class TestURLUploadValidation: class TestURLUploadEndpoint: """Integration tests for URL upload endpoint""" - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_requires_authentication(self, mock_process_document, mock_requests_get, client, monkeypatch): + def test_process_url_requires_authentication(self, mock_process_document, mock_stream, client, monkeypatch): """Test that endpoint requires authentication when auth is enabled""" # Mock successful download to prevent actual HTTP requests - mock_response = Mock() + mock_response = AsyncMock() 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"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF content" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context # Mock Celery task mock_task = Mock() @@ -192,17 +199,24 @@ class TestURLUploadEndpoint: # (like no mocking). We're just checking the endpoint exists and is reachable. assert response.status_code != 404 # Endpoint should exist - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_success(self, mock_process_document, mock_requests_get, client, tmp_path): + def test_process_url_success(self, mock_process_document, mock_stream, client, tmp_path): """Test successful URL processing""" # Mock successful download - mock_response = Mock() + mock_response = AsyncMock() 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 here"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF content here" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context # Mock Celery task mock_task = Mock() @@ -219,8 +233,8 @@ class TestURLUploadEndpoint: assert "filename" in data assert "size" in data - @patch("app.api.url_upload.requests.get") - def test_process_url_blocks_private_ip(self, mock_requests_get, client): + @patch("app.api.url_upload.httpx.AsyncClient.stream") + def test_process_url_blocks_private_ip(self, mock_stream, client): """Test that private IPs are blocked""" response = client.post("/api/process-url", json={"url": "http://192.168.1.1/file.pdf"}) @@ -229,10 +243,10 @@ class TestURLUploadEndpoint: assert "private/internal" in data["detail"] # Should not make HTTP request - mock_requests_get.assert_not_called() + mock_stream.assert_not_called() - @patch("app.api.url_upload.requests.get") - def test_process_url_blocks_localhost(self, mock_requests_get, client): + @patch("app.api.url_upload.httpx.AsyncClient.stream") + def test_process_url_blocks_localhost(self, mock_stream, client): """Test that localhost is blocked""" response = client.post("/api/process-url", json={"url": "http://localhost/file.pdf"}) @@ -241,10 +255,10 @@ class TestURLUploadEndpoint: assert "private/internal" in data["detail"] # Should not make HTTP request - mock_requests_get.assert_not_called() + mock_stream.assert_not_called() - @patch("app.api.url_upload.requests.get") - def test_process_url_blocks_metadata_endpoint(self, mock_requests_get, client): + @patch("app.api.url_upload.httpx.AsyncClient.stream") + def test_process_url_blocks_metadata_endpoint(self, mock_stream, client): """Test that cloud metadata endpoints are blocked""" response = client.post("/api/process-url", json={"url": "http://169.254.169.254/latest/meta-data/"}) @@ -254,17 +268,20 @@ class TestURLUploadEndpoint: assert "metadata" in data["detail"] or "private" in data["detail"] # Should not make HTTP request - mock_requests_get.assert_not_called() + mock_stream.assert_not_called() - @patch("app.api.url_upload.requests.get") - def test_process_url_invalid_file_type(self, mock_requests_get, client): + @patch("app.api.url_upload.httpx.AsyncClient.stream") + def test_process_url_invalid_file_type(self, mock_stream, client): """Test that invalid file types are rejected""" # Mock response with executable content-type - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/x-executable"} mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context response = client.post("/api/process-url", json={"url": "https://example.com/malware.exe"}) @@ -272,21 +289,24 @@ class TestURLUploadEndpoint: data = response.json() assert "Unsupported file type" in data["detail"] - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_file_too_large_by_header(self, mock_process_document, mock_requests_get, client): + def test_process_url_file_too_large_by_header(self, mock_process_document, mock_stream, client): """Test that files too large are rejected based on Content-Length header""" from app.config import settings # Mock response with large content-length - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = { "Content-Type": "application/pdf", "Content-Length": str(settings.max_upload_size + 1000), } mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context response = client.post("/api/process-url", json={"url": "https://example.com/huge.pdf"}) @@ -297,10 +317,10 @@ class TestURLUploadEndpoint: # Should not process document mock_process_document.delay.assert_not_called() - @patch("app.api.url_upload.requests.get") - def test_process_url_timeout_error(self, mock_requests_get, client): + @patch("app.api.url_upload.httpx.AsyncClient.stream") + def test_process_url_timeout_error(self, mock_stream, client): """Test handling of timeout errors""" - mock_requests_get.side_effect = requests.exceptions.Timeout("Request timed out") + mock_stream.side_effect = httpx.TimeoutException("Request timed out") response = client.post("/api/process-url", json={"url": "https://example.com/slow.pdf"}) @@ -308,10 +328,10 @@ class TestURLUploadEndpoint: data = response.json() assert "timeout" in data["detail"].lower() - @patch("app.api.url_upload.requests.get") - def test_process_url_connection_error(self, mock_requests_get, client): + @patch("app.api.url_upload.httpx.AsyncClient.stream") + def test_process_url_connection_error(self, mock_stream, client): """Test handling of connection errors""" - mock_requests_get.side_effect = requests.exceptions.ConnectionError("Failed to connect") + mock_stream.side_effect = httpx.ConnectError("Failed to connect") response = client.post("/api/process-url", json={"url": "https://example.com/file.pdf"}) @@ -319,15 +339,16 @@ class TestURLUploadEndpoint: data = response.json() assert "connect" in data["detail"].lower() - @patch("app.api.url_upload.requests.get") - def test_process_url_http_error_404(self, mock_requests_get, client): + @patch("app.api.url_upload.httpx.AsyncClient.stream") + def test_process_url_http_error_404(self, mock_stream, client): """Test handling of HTTP 404 errors""" - mock_response = Mock() + # When raising HTTPStatusError, httpx requires request and response arguments + # For our code, we just need it to hit the exception handler and check status code + mock_request = MagicMock() + mock_response = MagicMock() mock_response.status_code = 404 - mock_response.raise_for_status.side_effect = requests.exceptions.HTTPError( - "404 Not Found", response=mock_response - ) - mock_requests_get.return_value = mock_response + + mock_stream.side_effect = httpx.HTTPStatusError("404 Not Found", request=mock_request, response=mock_response) response = client.post("/api/process-url", json={"url": "https://example.com/notfound.pdf"}) @@ -335,17 +356,24 @@ class TestURLUploadEndpoint: data = response.json() assert "HTTP error" in data["detail"] - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_with_custom_filename(self, mock_process_document, mock_requests_get, client, tmp_path): + def test_process_url_with_custom_filename(self, mock_process_document, mock_stream, client, tmp_path): """Test URL upload with custom filename""" # Mock successful download - mock_response = Mock() + mock_response = AsyncMock() 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"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF content" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context # Mock Celery task mock_task = Mock() @@ -361,17 +389,24 @@ class TestURLUploadEndpoint: data = response.json() assert data["filename"] == "my-document.pdf" - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_extracts_filename_from_url(self, mock_process_document, mock_requests_get, client, tmp_path): + def test_process_url_extracts_filename_from_url(self, mock_process_document, mock_stream, client, tmp_path): """Test that filename is extracted from URL when not provided""" # Mock successful download - mock_response = Mock() + mock_response = AsyncMock() 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"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF content" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context # Mock Celery task mock_task = Mock() @@ -386,9 +421,9 @@ class TestURLUploadEndpoint: # Should extract "annual-report.pdf" from URL assert "annual-report" in data["filename"] - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_file_size_during_download(self, mock_process_document, mock_requests_get, client): + def test_process_url_file_size_during_download(self, mock_process_document, mock_stream, client): """Test that file size is checked during download""" from app.config import settings @@ -396,12 +431,19 @@ class TestURLUploadEndpoint: large_chunk = b"x" * (settings.max_upload_size + 1000) # Mock response without Content-Length header - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/pdf"} # No Content-Length - mock_response.iter_content = Mock(return_value=[large_chunk]) + + async def mock_aiter_bytes(chunk_size=None): + yield large_chunk + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context response = client.post("/api/process-url", json={"url": "https://example.com/big.pdf"}) @@ -412,10 +454,10 @@ class TestURLUploadEndpoint: # Should not process document mock_process_document.delay.assert_not_called() - @patch("app.api.url_upload.requests.get") - def test_process_url_request_exception(self, mock_requests_get, client): - """Test handling of generic RequestException""" - mock_requests_get.side_effect = requests.exceptions.RequestException("Generic request error") + @patch("app.api.url_upload.httpx.AsyncClient.stream") + def test_process_url_request_exception(self, mock_stream, client): + """Test handling of generic RequestError""" + mock_stream.side_effect = httpx.RequestError("Generic request error") response = client.post("/api/process-url", json={"url": "https://example.com/file.pdf"}) @@ -423,16 +465,23 @@ class TestURLUploadEndpoint: data = response.json() assert "Failed to download file" in data["detail"] - @patch("app.api.url_upload.requests.get") - def test_process_url_oserror_during_save(self, mock_requests_get, client, tmp_path, monkeypatch): + @patch("app.api.url_upload.httpx.AsyncClient.stream") + def test_process_url_oserror_during_save(self, mock_stream, client, tmp_path, monkeypatch): """Test handling of OSError when saving file""" # Mock successful download - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "100"} - mock_response.iter_content = Mock(return_value=[b"PDF"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context # Mock workdir to a non-existent path to trigger OSError from app.config import settings @@ -450,17 +499,24 @@ class TestURLUploadEndpoint: # Restore original workdir monkeypatch.setattr(settings, "workdir", original_workdir) - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_unexpected_exception(self, mock_process_document, mock_requests_get, client): + def test_process_url_unexpected_exception(self, mock_process_document, mock_stream, client): """Test handling of unexpected exceptions""" # Mock successful download but process_document.delay raises unexpected error - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "100"} - mock_response.iter_content = Mock(return_value=[b"PDF"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context # Mock process_document.delay to raise an unexpected exception mock_process_document.delay.side_effect = RuntimeError("Unexpected processing error") @@ -471,17 +527,24 @@ class TestURLUploadEndpoint: data = response.json() assert "Unexpected error" in data["detail"] - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_filename_without_extension(self, mock_process_document, mock_requests_get, client): + def test_process_url_filename_without_extension(self, mock_process_document, mock_stream, client): """Test that files without extensions are handled correctly""" # Mock successful download - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "100"} - mock_response.iter_content = Mock(return_value=[b"PDF"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context # Mock Celery task mock_task = Mock() @@ -496,17 +559,24 @@ class TestURLUploadEndpoint: # Should still work, just without extension assert data["task_id"] == "test-task-id" - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_empty_path_uses_download(self, mock_process_document, mock_requests_get, client): + def test_process_url_empty_path_uses_download(self, mock_process_document, mock_stream, client): """Test that empty URL path defaults to 'download' filename""" # Mock successful download - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "100"} - mock_response.iter_content = Mock(return_value=[b"PDF"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context # Mock Celery task mock_task = Mock() @@ -560,17 +630,24 @@ class TestURLUploadEndpoint: # Link-local address assert is_private_ip("169.254.1.1") is True - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_sanitizes_dangerous_filename(self, mock_process_document, mock_requests_get, client): + def test_process_url_sanitizes_dangerous_filename(self, mock_process_document, mock_stream, client): """Test that dangerous filenames are sanitized""" # Mock successful download - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "100"} - mock_response.iter_content = Mock(return_value=[b"PDF"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context # Mock Celery task mock_task = Mock() @@ -671,18 +748,25 @@ class TestURLUploadCoverageGaps: assert validate_file_type("", "filename_without_extension") is False @patch("app.api.url_upload.sanitize_filename", return_value="") - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") def test_process_url_sanitize_filename_returns_empty( - self, mock_process_document, mock_requests_get, mock_sanitize, client + self, mock_process_document, mock_stream, mock_sanitize, client ): """Test that when sanitize_filename returns empty string, filename defaults to 'download' (line 177)""" - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "100"} - mock_response.iter_content = Mock(return_value=[b"PDF content"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF content" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context mock_task = Mock() mock_task.id = "test-task-id-sanitize" @@ -695,17 +779,26 @@ class TestURLUploadCoverageGaps: # When sanitize_filename returns "", safe_filename defaults to "download" assert data["filename"] == "download" - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") @patch("app.api.url_upload.process_document") - def test_process_url_skips_empty_chunks(self, mock_process_document, mock_requests_get, client): + def test_process_url_skips_empty_chunks(self, mock_process_document, mock_stream, client): """Test that empty bytes chunks are skipped during download (line 234->233 branch)""" - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/pdf"} # Mix empty bytes (falsy) with real content - covers the `if chunk:` False branch - mock_response.iter_content = Mock(return_value=[b"", b"PDF content", b""]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"" + yield b"PDF content" + yield b"" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context mock_task = Mock() mock_task.id = "test-task-id-chunks" @@ -719,9 +812,9 @@ class TestURLUploadCoverageGaps: @patch("app.api.url_upload.os.remove") @patch("app.api.url_upload.os.path.exists", return_value=True) - @patch("app.api.url_upload.requests.get") + @patch("app.api.url_upload.httpx.AsyncClient.stream") def test_process_url_oserror_cleanup_removes_existing_file( - self, mock_requests_get, mock_exists, mock_remove, client, tmp_path, monkeypatch + self, mock_stream, mock_exists, mock_remove, client, tmp_path, monkeypatch ): """Test OSError handler removes the partial file when it exists (line 285)""" import os @@ -735,12 +828,19 @@ class TestURLUploadCoverageGaps: monkeypatch.setattr(settings, "workdir", str(non_existent)) - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/pdf", "Content-Length": "100"} - mock_response.iter_content = Mock(return_value=[b"PDF"]) + + async def mock_aiter_bytes(chunk_size=None): + yield b"PDF" + + mock_response.aiter_bytes = mock_aiter_bytes mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context response = client.post("/api/process-url", json={"url": "https://example.com/file.pdf"}) @@ -750,14 +850,17 @@ class TestURLUploadCoverageGaps: mock_remove.assert_called_once() @patch("app.api.url_upload.validate_file_type", side_effect=ValueError("unexpected internal error")) - @patch("app.api.url_upload.requests.get") - def test_process_url_unexpected_exception_with_no_file_created(self, mock_requests_get, mock_validate, client): + @patch("app.api.url_upload.httpx.AsyncClient.stream") + def test_process_url_unexpected_exception_with_no_file_created(self, mock_stream, mock_validate, client): """Test unexpected exception before target_path is assigned; no file cleanup attempted (line 291->293)""" - mock_response = Mock() + mock_response = AsyncMock() mock_response.status_code = 200 mock_response.headers = {"Content-Type": "application/pdf"} mock_response.raise_for_status = Mock() - mock_requests_get.return_value = mock_response + + mock_context = AsyncMock() + mock_context.__aenter__.return_value = mock_response + mock_stream.return_value = mock_context response = client.post("/api/process-url", json={"url": "https://example.com/file.pdf"})