Merge branch 'main' into perf/async-url-upload-7099409897484162483

2026-03-16 12:12:21 +01:00
parent 8ce41d723e 66fdb11e39
commit b8a1ac52b3
46 changed files with 2356 additions and 258 deletions
@@ -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
@@ -6,7 +6,7 @@ import logging
 import os
 from typing import Annotated, Optional

-import requests
+import httpx
 from fastapi import APIRouter, Depends, Form, HTTPException, Request, status
 from sqlalchemy.orm import Session

@@ -132,57 +132,60 @@ async def test_dropbox_token(request: Request):
                "message": "Dropbox credentials are not fully configured",
            }

-        # Check token validity by getting current account info
-        headers = {"Authorization": f"Bearer {settings.dropbox_refresh_token}"}
-        response = requests.post(
-            "https://api.dropboxapi.com/2/users/get_current_account",
-            headers=headers,
-            timeout=settings.http_request_timeout,
-        )
-
-        # If token is invalid, try refreshing it
-        if response.status_code == 401:
-            logger.info("Dropbox access token invalid or expired, trying to refresh")
-
-            # Get a new access token using the refresh token
-            refresh_url = "https://api.dropbox.com/oauth2/token"
-            refresh_data = {
-                "grant_type": "refresh_token",
-                "refresh_token": settings.dropbox_refresh_token,
-                "client_id": settings.dropbox_app_key,
-                "client_secret": settings.dropbox_app_secret,
-            }
-
-            refresh_response = requests.post(refresh_url, data=refresh_data, timeout=settings.http_request_timeout)
-
-            if refresh_response.status_code != 200:
-                logger.error(f"Failed to refresh Dropbox token: {refresh_response.text}")
-                return {
-                    "status": "error",
-                    "message": "Refresh token has expired or is invalid",
-                    "needs_reauth": True,
-                }
-
-            token_info = refresh_response.json()
-            access_token = token_info.get("access_token")
-
-            # Try again with the new access token
-            headers = {"Authorization": f"Bearer {access_token}"}
-            response = requests.post(
+        async with httpx.AsyncClient() as client:
+            # Check token validity by getting current account info
+            headers = {"Authorization": f"Bearer {settings.dropbox_refresh_token}"}
+            response = await client.post(
                "https://api.dropboxapi.com/2/users/get_current_account",
                headers=headers,
                timeout=settings.http_request_timeout,
            )

-        if response.status_code != 200:
-            logger.error(f"Dropbox token test failed: {response.status_code} {response.text}")
-            return {
-                "status": "error",
-                "message": f"Token validation failed with status {response.status_code}: {response.text}",
-            }
+            # If token is invalid, try refreshing it
+            if response.status_code == 401:
+                logger.info("Dropbox access token invalid or expired, trying to refresh")

-        # Get account info
-        account_info = response.json()
+                # Get a new access token using the refresh token
+                refresh_url = "https://api.dropbox.com/oauth2/token"
+                refresh_data = {
+                    "grant_type": "refresh_token",
+                    "refresh_token": settings.dropbox_refresh_token,
+                    "client_id": settings.dropbox_app_key,
+                    "client_secret": settings.dropbox_app_secret,
+                }
+
+                refresh_response = await client.post(
+                    refresh_url, data=refresh_data, timeout=settings.http_request_timeout
+                )
+
+                if refresh_response.status_code != 200:
+                    logger.error(f"Failed to refresh Dropbox token: {refresh_response.text}")
+                    return {
+                        "status": "error",
+                        "message": "Refresh token has expired or is invalid",
+                        "needs_reauth": True,
+                    }
+
+                token_info = refresh_response.json()
+                access_token = token_info.get("access_token")
+
+                # Try again with the new access token
+                headers = {"Authorization": f"Bearer {access_token}"}
+                response = await client.post(
+                    "https://api.dropboxapi.com/2/users/get_current_account",
+                    headers=headers,
+                    timeout=settings.http_request_timeout,
+                )
+
+            if response.status_code != 200:
+                logger.error(f"Dropbox token test failed: {response.status_code} {response.text}")
+                return {
+                    "status": "error",
+                    "message": f"Token validation failed with status {response.status_code}: {response.text}",
+                }
+
+            # Get account info
+            account_info = response.json()
        account_email = account_info.get("email", "Unknown account")
        account_name = account_info.get("name", {}).get("display_name", "Unknown user")

@@ -73,33 +73,38 @@ def list_duplicate_groups(
    groups = []
    total_duplicate_files = 0

-    for filehash in dup_hashes:
-        # Find the original (non-duplicate) record with this hash
-        original = (
-            db.query(FileRecord)
-            .filter(FileRecord.filehash == filehash, FileRecord.is_duplicate.is_(False))
-            .order_by(FileRecord.id.asc())
-            .first()
+    if dup_hashes:
+        # Fetch all matching files (both original and duplicates) in a single batch query
+        all_records = (
+            db.query(FileRecord).filter(FileRecord.filehash.in_(dup_hashes)).order_by(FileRecord.id.asc()).all()
        )

-        # Find all duplicate records for this hash
-        duplicates = (
-            db.query(FileRecord)
-            .filter(FileRecord.filehash == filehash, FileRecord.is_duplicate.is_(True))
-            .order_by(FileRecord.id.asc())
-            .all()
-        )
+        # Group records by hash
+        originals_by_hash = {}
+        duplicates_by_hash = {h: [] for h in dup_hashes}

-        total_duplicate_files += len(duplicates)
+        for record in all_records:
+            h = record.filehash
+            if not record.is_duplicate:
+                # Store only the first original record per hash, matching the old .first() behaviour
+                if h not in originals_by_hash:
+                    originals_by_hash[h] = record
+            else:
+                duplicates_by_hash[h].append(record)
+                total_duplicate_files += 1

-        groups.append(
-            {
-                "filehash": filehash,
-                "original": _file_record_to_dict(original) if original else None,
-                "duplicates": [_file_record_to_dict(d) for d in duplicates],
-                "duplicate_count": len(duplicates),
-            }
-        )
+        for filehash in dup_hashes:
+            original = originals_by_hash.get(filehash)
+            duplicates = duplicates_by_hash.get(filehash, [])
+
+            groups.append(
+                {
+                    "filehash": filehash,
+                    "original": _file_record_to_dict(original) if original else None,
+                    "duplicates": [_file_record_to_dict(d) for d in duplicates],
+                    "duplicate_count": len(duplicates),
+                }
+            )

    total_pages = (total_groups + per_page - 1) // per_page if total_groups > 0 else 1

@@ -11,6 +11,7 @@ import zipfile
 from datetime import datetime, timezone
 from typing import Annotated, List, Optional

+import aiofiles
 from fastapi import APIRouter, Depends, File, HTTPException, Query, Request, UploadFile, status
 from fastapi.responses import StreamingResponse
 from sqlalchemy import asc, desc
@@ -1277,7 +1278,7 @@ async def ui_upload(request: Request, db: DbSession, file: UploadFile = File(...
    # enforcing the size limit during the read so memory usage stays bounded.
    try:
        written_size = 0
-        with open(target_path, "wb") as f:
+        async with aiofiles.open(target_path, "wb") as f:
            chunk_size = 65536  # 64 KB chunks
            while True:
                chunk = await file.read(chunk_size)
@@ -1286,14 +1287,14 @@ async def ui_upload(request: Request, db: DbSession, file: UploadFile = File(...
                written_size += len(chunk)
                if written_size > max_size:
                    # Exceeded limit mid-stream; clean up and reject
-                    f.close()
+                    await 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)
+                await f.write(chunk)
    except HTTPException:
        raise
    except Exception as e:
@@ -363,7 +363,7 @@ def format_time_remaining(time_delta):

@router.post("/google-drive/save-settings")
@require_login
-async def save_dropbox_settings(
+async def save_google_drive_settings(
    request: Request,
    refresh_token: Annotated[str, Form(...)],
    client_id: Annotated[Optional[str], Form()] = None,
@@ -452,17 +452,16 @@ async def update_preferences(
            )

    try:
+        # Pre-fetch existing preferences for this user to avoid N+1 queries
+        existing_prefs = (
+            db.query(UserNotificationPreference).filter(UserNotificationPreference.owner_id == owner_id).all()
+        )
+
+        # Build a fast lookup dictionary keyed by (event_type, channel_type, target_id)
+        prefs_dict = {(pref.event_type, pref.channel_type, pref.target_id): pref for pref in existing_prefs}
+
        for item in body.preferences:
-            existing = (
-                db.query(UserNotificationPreference)
-                .filter(
-                    UserNotificationPreference.owner_id == owner_id,
-                    UserNotificationPreference.event_type == item.event_type,
-                    UserNotificationPreference.channel_type == item.channel_type,
-                    UserNotificationPreference.target_id == item.target_id,
-                )
-                .first()
-            )
+            existing = prefs_dict.get((item.event_type, item.channel_type, item.target_id))
            if existing:
                existing.is_enabled = item.is_enabled
            else:
@@ -3,7 +3,6 @@ OneDrive API endpoints
 """

 import logging
-import os
 from datetime import datetime, timedelta
 from typing import Annotated, Optional

@@ -14,6 +13,7 @@ from sqlalchemy.orm import Session
 from app.auth import require_login
 from app.config import settings
 from app.database import get_db
+from app.utils.env_utils import update_env_file
 from app.utils.oauth_helper import exchange_oauth_token
 from app.utils.settings_service import save_setting_to_db
 from app.utils.settings_sync import notify_settings_updated
@@ -115,32 +115,7 @@ async def test_onedrive_token(request: Request):
            settings.onedrive_refresh_token = new_refresh_token

            # Also try to update .env file if it exists
-            try:
-                env_path = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(__file__))), ".env")
-                if os.path.exists(env_path):
-                    with open(env_path, "r") as f:
-                        env_lines = f.readlines()
-
-                    updated_lines = []
-                    updated = False
-
-                    for line in env_lines:
-                        if line.startswith("ONEDRIVE_REFRESH_TOKEN="):
-                            updated_lines.append(f"ONEDRIVE_REFRESH_TOKEN={new_refresh_token}\n")
-                            updated = True
-                        else:
-                            updated_lines.append(line)
-
-                    if not updated:
-                        updated_lines.append(f"ONEDRIVE_REFRESH_TOKEN={new_refresh_token}\n")
-
-                    with open(env_path, "w") as f:
-                        f.writelines(updated_lines)
-
-                    logger.info("Updated refresh token in .env file")
-
-            except Exception as e:
-                logger.warning(f"Failed to update refresh token in .env file: {e}")
+            update_env_file({"ONEDRIVE_REFRESH_TOKEN": new_refresh_token})

            # Persist the rotated refresh token to the database
            try:
@@ -246,75 +221,26 @@ async def save_onedrive_settings(
            user.get("preferred_username") or user.get("username") or user.get("email") or user.get("id") or "wizard"
        )

-        # Best-effort .env file write
-        try:
-            env_path = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(__file__))), ".env")
-            if not os.path.exists(env_path):
-                logger.warning(f".env file not found at {env_path}, skipping file write")
-            else:
-                logger.info(f"Updating OneDrive settings in {env_path}")
+        # Build settings dictionary mapped to database/memory keys
+        onedrive_settings = {
+            "onedrive_refresh_token": refresh_token,
+            "onedrive_client_id": client_id,
+            "onedrive_client_secret": client_secret,
+            "onedrive_tenant_id": tenant_id,
+            "onedrive_folder_path": folder_path,
+        }

-                with open(env_path, "r") as f:
-                    env_lines = f.readlines()
+        # Filter out None values
+        onedrive_settings = {k: v for k, v in onedrive_settings.items() if v is not None}

-                onedrive_settings = {"ONEDRIVE_REFRESH_TOKEN": refresh_token}
-                if client_id:
-                    onedrive_settings["ONEDRIVE_CLIENT_ID"] = client_id
-                if client_secret:
-                    onedrive_settings["ONEDRIVE_CLIENT_SECRET"] = client_secret
-                if tenant_id:
-                    onedrive_settings["ONEDRIVE_TENANT_ID"] = tenant_id
-                if folder_path:
-                    onedrive_settings["ONEDRIVE_FOLDER_PATH"] = folder_path
+        # Best-effort .env file write using the new utility
+        env_settings = {k.upper(): v for k, v in onedrive_settings.items()}
+        update_env_file(env_settings)

-                updated = set()
-                new_env_lines = []
-                for line in env_lines:
-                    stripped_line = line.rstrip()
-                    is_updated = False
-                    for key, value in onedrive_settings.items():
-                        if stripped_line.startswith(f"{key}=") or stripped_line.startswith(f"# {key}="):
-                            new_env_lines.append(f"{key}={value}")
-                            updated.add(key)
-                            is_updated = True
-                            break
-                    if not is_updated:
-                        new_env_lines.append(stripped_line)
-
-                for key, value in onedrive_settings.items():
-                    if key not in updated:
-                        new_env_lines.append(f"{key}={value}")
-
-                with open(env_path, "w") as f:
-                    f.write("\n".join(new_env_lines) + "\n")
-
-                logger.info("Successfully updated OneDrive settings in .env file")
-        except Exception as env_err:
-            logger.warning(f"Failed to write .env file (non-fatal): {env_err}")
-
-        # Update the settings in memory
-        if refresh_token:
-            settings.onedrive_refresh_token = refresh_token
-        if client_id:
-            settings.onedrive_client_id = client_id
-        if client_secret:
-            settings.onedrive_client_secret = client_secret
-        if tenant_id:
-            settings.onedrive_tenant_id = tenant_id
-        if folder_path:
-            settings.onedrive_folder_path = folder_path
-
-        # Persist to database (primary)
-        if refresh_token:
-            save_setting_to_db(db, "onedrive_refresh_token", refresh_token, changed_by=changed_by)
-        if client_id:
-            save_setting_to_db(db, "onedrive_client_id", client_id, changed_by=changed_by)
-        if client_secret:
-            save_setting_to_db(db, "onedrive_client_secret", client_secret, changed_by=changed_by)
-        if tenant_id:
-            save_setting_to_db(db, "onedrive_tenant_id", tenant_id, changed_by=changed_by)
-        if folder_path:
-            save_setting_to_db(db, "onedrive_folder_path", folder_path, changed_by=changed_by)
+        # Update in-memory settings and persist to database dynamically
+        for key, value in onedrive_settings.items():
+            setattr(settings, key, value)
+            save_setting_to_db(db, key, value, changed_by=changed_by)

        notify_settings_updated()

@@ -196,11 +196,20 @@ def seed_plans(db: DbSession, _admin: AdminUser) -> dict[str, Any]:
 def reorder_plans(body: ReorderBody, db: DbSession, _admin: AdminUser) -> dict[str, Any]:
    """Update sort_order for each plan_id in *body.order* (position = index in list)."""
    updated = 0
-    for sort_order, plan_id in enumerate(body.order):
-        plan = db.query(SubscriptionPlan).filter(SubscriptionPlan.plan_id == plan_id).first()
+
+    # Fetch all requested plans in a single query to avoid N+1
+    plan_ids = body.order
+    plans = db.query(SubscriptionPlan).filter(SubscriptionPlan.plan_id.in_(plan_ids)).all()
+
+    # Build a map for fast O(1) lookup
+    plan_map = {p.plan_id: p for p in plans}
+
+    for sort_order, plan_id in enumerate(plan_ids):
+        plan = plan_map.get(plan_id)
        if plan:
            plan.sort_order = sort_order
            updated += 1
+
    try:
        db.commit()
    except Exception:
@@ -313,16 +313,18 @@ async def list_shared_links(
    active_only: bool = Query(False, description="When true, only return active (non-revoked) links"),
 ) -> list[dict[str, Any]]:
    """List all shared links created by the authenticated user."""
-    q = db.query(SharedLink).filter(SharedLink.owner_id == owner_id)
+    q = (
+        db.query(SharedLink, FileRecord.original_filename)
+        .outerjoin(FileRecord, SharedLink.file_id == FileRecord.id)
+        .filter(SharedLink.owner_id == owner_id)
+    )
    if active_only:
        q = q.filter(SharedLink.is_active.is_(True))
-    links = q.order_by(SharedLink.created_at.desc()).all()
+    links_with_filenames = q.order_by(SharedLink.created_at.desc()).all()

    base_url = str(request.base_url).rstrip("/")
    result = []
-    for link in links:
-        file_record = db.query(FileRecord).filter(FileRecord.id == link.file_id).first()
-        filename = file_record.original_filename if file_record else None
+    for link, filename in links_with_filenames:
        result.append(_link_to_dict(link, base_url, filename))
    return result