diff --git a/app/database.py b/app/database.py index f48b1ba5..83de3f1a 100644 --- a/app/database.py +++ b/app/database.py @@ -165,6 +165,40 @@ def _run_schema_migrations(engine: Any) -> None: conn.execute(text("CREATE INDEX IF NOT EXISTS ix_saved_searches_user_id ON saved_searches (user_id)")) logger.info("Migration complete: 'saved_searches' table created") + # Migration: Add performance indexes for common query patterns + _ensure_indexes(engine, inspector) + + +def _ensure_indexes(engine: Any, inspector: Any) -> None: + """ + Create performance indexes for common query patterns. + + Each ``CREATE INDEX IF NOT EXISTS`` is idempotent and safe to run + on every startup. The indexes target the columns most frequently + used in file listing/filtering, status computation and log retrieval. + """ + from sqlalchemy import text + + _PERF_INDEXES = [ + ("ix_files_created_at", "files", "created_at"), + ("ix_files_mime_type", "files", "mime_type"), + ("ix_processing_logs_file_id", "processing_logs", "file_id"), + ("ix_processing_logs_timestamp", "processing_logs", "timestamp"), + ("ix_file_processing_steps_status", "file_processing_steps", "status"), + ] + + table_names = inspector.get_table_names() + columns_by_table: dict[str, set[str]] = {} + with engine.begin() as conn: + for idx_name, table, column in _PERF_INDEXES: + if table in table_names: + if table not in columns_by_table: + columns_by_table[table] = {col["name"] for col in inspector.get_columns(table)} + if column in columns_by_table[table]: + conn.execute(text(f"CREATE INDEX IF NOT EXISTS {idx_name} ON {table} ({column})")) + + logger.info("Performance indexes ensured") + def get_db() -> Generator[Session, None, None]: """ diff --git a/app/models.py b/app/models.py index 81d0d5e5..03974588 100644 --- a/app/models.py +++ b/app/models.py @@ -46,7 +46,7 @@ class FileRecord(Base): file_size = Column(Integer, nullable=False) # MIME type or extension (optional) - mime_type = Column(String) + mime_type = Column(String, index=True) # Deduplication tracking: True if this file is a duplicate of another file # When a duplicate is detected, this file record is created but marked as duplicate @@ -68,7 +68,7 @@ class FileRecord(Base): document_title = Column(String, nullable=True) # Timestamp when we inserted this record - created_at = Column(DateTime(timezone=True), server_default=func.now()) + created_at = Column(DateTime(timezone=True), server_default=func.now(), index=True) class FileProcessingStep(Base): @@ -82,7 +82,7 @@ class FileProcessingStep(Base): id = Column(Integer, primary_key=True, index=True) file_id = Column(Integer, ForeignKey(_FILES_ID_FK), nullable=False, index=True) step_name = Column(String, nullable=False, index=True) # e.g., "hash_file", "upload_to_dropbox" - status = Column(String, nullable=False) # "pending", "in_progress", "success", "failure", "skipped" + status = Column(String, nullable=False, index=True) # "pending", "in_progress", "success", "failure", "skipped" started_at = Column(DateTime(timezone=True), nullable=True) # When step started completed_at = Column(DateTime(timezone=True), nullable=True) # When step finished (success/failure) error_message = Column(Text, nullable=True) # Error message if status is "failure" @@ -95,13 +95,13 @@ class FileProcessingStep(Base): class ProcessingLog(Base): __tablename__ = "processing_logs" id = Column(Integer, primary_key=True, index=True) - file_id = Column(Integer, ForeignKey(_FILES_ID_FK), nullable=True) # Optional file association + file_id = Column(Integer, ForeignKey(_FILES_ID_FK), nullable=True, index=True) # Optional file association task_id = Column(String, index=True) # Celery task ID step_name = Column(String) # e.g., "OCR", "convert_to_pdf", "upload_s3" status = Column(String) # "pending", "in_progress", "success", "failure" message = Column(String, nullable=True) # Error text or success note detail = Column(Text, nullable=True) # Verbose worker log output for diagnostics - timestamp = Column(DateTime(timezone=True), server_default=func.now()) + timestamp = Column(DateTime(timezone=True), server_default=func.now(), index=True) class ApplicationSettings(Base): diff --git a/app/utils/cache.py b/app/utils/cache.py new file mode 100644 index 00000000..6be1662b --- /dev/null +++ b/app/utils/cache.py @@ -0,0 +1,122 @@ +""" +Lightweight Redis caching layer for frequently accessed data. + +Provides a thin wrapper around Redis GET/SET with JSON serialization and +configurable TTLs. All operations are **fail-open**: if Redis is +unavailable the caller simply gets a cache miss and falls back to the +database or other source of truth. + +Usage:: + + from app.utils.cache import cache_get, cache_set, cache_delete + + # Try cache first + value = cache_get("my_key") + if value is None: + value = expensive_query() + cache_set("my_key", value, ttl=300) +""" + +import json +import logging +from typing import Any + +import redis + +logger = logging.getLogger(__name__) + +#: Prefix applied to all cache keys to avoid collisions with other Redis users. +_KEY_PREFIX = "docuelevate:cache:" + +#: Module-level Redis client – lazily initialised on first use. +_redis_client: redis.Redis | None = None + + +def _get_redis() -> redis.Redis | None: + """Return a shared Redis client, or *None* if Redis is unreachable.""" + global _redis_client + if _redis_client is not None: + return _redis_client + try: + from app.config import settings + + _redis_client = redis.from_url(settings.redis_url, socket_connect_timeout=2, decode_responses=True) + # Quick connectivity check + _redis_client.ping() + return _redis_client + except Exception as exc: + logger.debug(f"Redis cache unavailable: {exc}") + _redis_client = None + return None + + +def cache_get(key: str) -> Any | None: + """ + Retrieve a cached value by *key*. + + Returns the deserialised Python object, or ``None`` on cache miss or + Redis error. + """ + client = _get_redis() + if client is None: + return None + try: + raw = client.get(f"{_KEY_PREFIX}{key}") + if raw is None: + return None + return json.loads(raw) + except Exception as exc: + logger.debug(f"Cache get failed for {key}: {exc}") + return None + + +def cache_set(key: str, value: Any, ttl: int = 300) -> None: + """ + Store *value* under *key* with a time-to-live of *ttl* seconds. + + Silently ignores errors so callers are never blocked by cache issues. + """ + client = _get_redis() + if client is None: + return + try: + client.setex(f"{_KEY_PREFIX}{key}", ttl, json.dumps(value)) + except Exception as exc: + logger.debug(f"Cache set failed for {key}: {exc}") + + +def cache_delete(key: str) -> None: + """ + Remove *key* from the cache. + + Silently ignores errors. + """ + client = _get_redis() + if client is None: + return + try: + client.delete(f"{_KEY_PREFIX}{key}") + except Exception as exc: + logger.debug(f"Cache delete failed for {key}: {exc}") + + +def cache_delete_pattern(pattern: str) -> None: + """ + Remove all keys matching *pattern* (glob-style) from the cache. + + Silently ignores errors. + """ + client = _get_redis() + if client is None: + return + try: + full_pattern = f"{_KEY_PREFIX}{pattern}" + cursor = 0 + while True: + cursor, keys = client.scan(cursor, match=full_pattern, count=100) # type: ignore[misc] # sync Redis returns tuple + if keys: + client.delete(*keys) + if cursor == 0: + break + except Exception as exc: + logger.debug(f"Cache delete pattern failed for {pattern}: {exc}") diff --git a/app/utils/file_status.py b/app/utils/file_status.py index 75c84526..7b81ae77 100644 --- a/app/utils/file_status.py +++ b/app/utils/file_status.py @@ -106,9 +106,15 @@ def get_files_processing_status(db: Session, file_ids: List[int]) -> Dict[int, D if settings.enable_deduplication: REAL_STEPS.add("check_for_duplicates") - # Get all REAL steps for these files in one query + # Get all REAL steps for these files in one query (load only needed columns) steps = ( - db.query(FileProcessingStep) + db.query( + FileProcessingStep.file_id, + FileProcessingStep.step_name, + FileProcessingStep.status, + FileProcessingStep.updated_at, + FileProcessingStep.created_at, + ) .filter(FileProcessingStep.file_id.in_(file_ids), FileProcessingStep.step_name.in_(REAL_STEPS)) .all() ) diff --git a/app/views/files.py b/app/views/files.py index 64839983..92245809 100644 --- a/app/views/files.py +++ b/app/views/files.py @@ -8,6 +8,7 @@ from typing import Optional from fastapi import Depends, HTTPException, Query, Request from sqlalchemy.orm import Session +from app.utils.cache import cache_get, cache_set from app.utils.file_queries import apply_status_filter from app.utils.file_status import get_files_processing_status from app.views.base import APIRouter, get_db, logger, require_login, templates @@ -149,9 +150,12 @@ def files_page( # Calculate pagination info total_pages = (total_items + per_page - 1) // per_page - # Get unique MIME types for filter dropdown - mime_types = db.query(FileRecord.mime_type).distinct().filter(FileRecord.mime_type.isnot(None)).all() - mime_types = [mt[0] for mt in mime_types if mt[0]] + # Get unique MIME types for filter dropdown (cached) + mime_types = cache_get("mime_types") + if mime_types is None: + raw = db.query(FileRecord.mime_type).distinct().filter(FileRecord.mime_type.isnot(None)).all() + mime_types = [mt[0] for mt in raw if mt[0]] + cache_set("mime_types", mime_types, ttl=120) # Debug output logger.info(f"Retrieved {len(files_with_status)} files from database (page {page}/{total_pages})") diff --git a/docs/ConfigurationGuide.md b/docs/ConfigurationGuide.md index 212e3365..5134b7a2 100644 --- a/docs/ConfigurationGuide.md +++ b/docs/ConfigurationGuide.md @@ -799,6 +799,39 @@ Administrators can set the **site-wide default** colour scheme that is applied w UI_DEFAULT_COLOR_SCHEME=dark ``` +## Performance & Caching + +DocuElevate automatically optimizes database access and uses Redis as a +caching layer for frequently accessed data. + +### Database Indexes + +On startup the application creates indexes on columns used for filtering, +sorting, and joining in the file listing and status computation queries: + +| Table | Column | Purpose | +|---|---|---| +| `files` | `created_at` | Default sort order | +| `files` | `mime_type` | MIME type filter & dropdown | +| `processing_logs` | `file_id` | Log retrieval by file | +| `processing_logs` | `timestamp` | Log ordering | +| `file_processing_steps` | `status` | Status filter sub-queries | + +These indexes are created idempotently on every startup so no manual +migration step is required. + +### Redis Query Cache + +When Redis is available (configured via `REDIS_URL`), DocuElevate caches +selected query results to avoid redundant database round-trips: + +| Cache Key | TTL | Description | +|---|---|---| +| `mime_types` | 120 s | Distinct MIME types shown in the file-list filter dropdown | + +The cache is **fail-open**: if Redis is unreachable the application falls +back to querying the database directly with no user-visible impact. + ## Configuration Examples ### Minimal Configuration diff --git a/tests/test_cache.py b/tests/test_cache.py new file mode 100644 index 00000000..e869e027 --- /dev/null +++ b/tests/test_cache.py @@ -0,0 +1,212 @@ +""" +Tests for the Redis caching utility module. +""" + +import json +from unittest.mock import MagicMock, patch + +import pytest + +from app.utils.cache import ( + _KEY_PREFIX, + cache_delete, + cache_delete_pattern, + cache_get, + cache_set, +) + + +@pytest.fixture(autouse=True) +def _reset_redis_client(): + """Reset the module-level Redis client between tests.""" + import app.utils.cache as cache_mod + + cache_mod._redis_client = None + yield + cache_mod._redis_client = None + + +# --------------------------------------------------------------------------- +# cache_get +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +def test_cache_get_returns_none_when_redis_unavailable(): + """cache_get should return None when Redis is not reachable.""" + with patch("app.utils.cache._get_redis", return_value=None): + assert cache_get("some_key") is None + + +@pytest.mark.unit +def test_cache_get_returns_none_on_miss(): + """cache_get should return None when the key does not exist.""" + mock_client = MagicMock() + mock_client.get.return_value = None + with patch("app.utils.cache._get_redis", return_value=mock_client): + assert cache_get("nonexistent") is None + mock_client.get.assert_called_once_with(f"{_KEY_PREFIX}nonexistent") + + +@pytest.mark.unit +def test_cache_get_returns_deserialized_value(): + """cache_get should deserialize the stored JSON string.""" + mock_client = MagicMock() + mock_client.get.return_value = json.dumps(["application/pdf", "image/png"]) + with patch("app.utils.cache._get_redis", return_value=mock_client): + result = cache_get("mime_types") + assert result == ["application/pdf", "image/png"] + + +@pytest.mark.unit +def test_cache_get_returns_none_on_exception(): + """cache_get should not raise when Redis throws an error.""" + mock_client = MagicMock() + mock_client.get.side_effect = Exception("connection lost") + with patch("app.utils.cache._get_redis", return_value=mock_client): + assert cache_get("key") is None + + +# --------------------------------------------------------------------------- +# cache_set +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +def test_cache_set_stores_json_with_ttl(): + """cache_set should serialise value as JSON and set with TTL.""" + mock_client = MagicMock() + with patch("app.utils.cache._get_redis", return_value=mock_client): + cache_set("my_key", {"a": 1}, ttl=60) + mock_client.setex.assert_called_once_with(f"{_KEY_PREFIX}my_key", 60, json.dumps({"a": 1})) + + +@pytest.mark.unit +def test_cache_set_uses_default_ttl(): + """cache_set should default to 300 seconds TTL.""" + mock_client = MagicMock() + with patch("app.utils.cache._get_redis", return_value=mock_client): + cache_set("key", "val") + _, args, _ = mock_client.setex.mock_calls[0] + assert args[1] == 300 + + +@pytest.mark.unit +def test_cache_set_noop_when_redis_unavailable(): + """cache_set should silently do nothing when Redis is down.""" + with patch("app.utils.cache._get_redis", return_value=None): + cache_set("key", "val") # Should not raise + + +@pytest.mark.unit +def test_cache_set_ignores_exception(): + """cache_set should not raise on Redis errors.""" + mock_client = MagicMock() + mock_client.setex.side_effect = Exception("write failed") + with patch("app.utils.cache._get_redis", return_value=mock_client): + cache_set("key", "val") # Should not raise + + +# --------------------------------------------------------------------------- +# cache_delete +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +def test_cache_delete_removes_key(): + """cache_delete should delete the prefixed key.""" + mock_client = MagicMock() + with patch("app.utils.cache._get_redis", return_value=mock_client): + cache_delete("old_key") + mock_client.delete.assert_called_once_with(f"{_KEY_PREFIX}old_key") + + +@pytest.mark.unit +def test_cache_delete_noop_when_redis_unavailable(): + """cache_delete should silently do nothing when Redis is down.""" + with patch("app.utils.cache._get_redis", return_value=None): + cache_delete("key") # Should not raise + + +@pytest.mark.unit +def test_cache_delete_ignores_exception(): + """cache_delete should not raise on Redis errors.""" + mock_client = MagicMock() + mock_client.delete.side_effect = Exception("delete failed") + with patch("app.utils.cache._get_redis", return_value=mock_client): + cache_delete("key") # Should not raise + + +# --------------------------------------------------------------------------- +# cache_delete_pattern +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +def test_cache_delete_pattern_scans_and_deletes(): + """cache_delete_pattern should use SCAN to find and delete matching keys.""" + mock_client = MagicMock() + # Simulate SCAN returning keys in one batch then cursor 0 + mock_client.scan.return_value = (0, [f"{_KEY_PREFIX}mime_types", f"{_KEY_PREFIX}mime_list"]) + with patch("app.utils.cache._get_redis", return_value=mock_client): + cache_delete_pattern("mime_*") + mock_client.scan.assert_called_once_with(0, match=f"{_KEY_PREFIX}mime_*", count=100) + mock_client.delete.assert_called_once_with(f"{_KEY_PREFIX}mime_types", f"{_KEY_PREFIX}mime_list") + + +@pytest.mark.unit +def test_cache_delete_pattern_noop_when_redis_unavailable(): + """cache_delete_pattern should silently do nothing when Redis is down.""" + with patch("app.utils.cache._get_redis", return_value=None): + cache_delete_pattern("mime_*") # Should not raise + + +@pytest.mark.unit +def test_cache_delete_pattern_handles_empty_scan(): + """cache_delete_pattern should not call delete when SCAN returns no keys.""" + mock_client = MagicMock() + mock_client.scan.return_value = (0, []) + with patch("app.utils.cache._get_redis", return_value=mock_client): + cache_delete_pattern("none_*") + mock_client.delete.assert_not_called() + + +@pytest.mark.unit +def test_cache_delete_pattern_ignores_exception(): + """cache_delete_pattern should not raise on Redis errors.""" + mock_client = MagicMock() + mock_client.scan.side_effect = Exception("scan failed") + with patch("app.utils.cache._get_redis", return_value=mock_client): + cache_delete_pattern("x*") # Should not raise + + +# --------------------------------------------------------------------------- +# _get_redis +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +def test_get_redis_returns_none_on_connection_failure(): + """_get_redis should return None when Redis connection fails.""" + import app.utils.cache as cache_mod + + with patch("app.utils.cache.redis.from_url", side_effect=Exception("refused")): + result = cache_mod._get_redis() + assert result is None + + +@pytest.mark.unit +def test_get_redis_caches_client(): + """_get_redis should reuse the cached client on subsequent calls.""" + import app.utils.cache as cache_mod + + mock_client = MagicMock() + mock_client.ping.return_value = True + + with patch("app.utils.cache.redis.from_url", return_value=mock_client): + first = cache_mod._get_redis() + second = cache_mod._get_redis() + + assert first is second + # from_url should only have been called once + assert first is mock_client diff --git a/tests/test_database_indexes.py b/tests/test_database_indexes.py new file mode 100644 index 00000000..52eb5aab --- /dev/null +++ b/tests/test_database_indexes.py @@ -0,0 +1,82 @@ +""" +Tests for database performance indexes and schema migrations. +""" + +import pytest +from sqlalchemy import create_engine, inspect +from sqlalchemy.pool import StaticPool + +from app.database import Base, _ensure_indexes + + +@pytest.fixture +def engine_with_tables(): + """Create an in-memory SQLite engine with all tables.""" + engine = create_engine( + "sqlite:///:memory:", + connect_args={"check_same_thread": False}, + poolclass=StaticPool, + ) + # Import models to register them with Base + from app.models import ( # noqa: F401 + ApplicationSettings, + FileProcessingStep, + FileRecord, + ProcessingLog, + ) + + Base.metadata.create_all(bind=engine) + return engine + + +@pytest.mark.unit +def test_ensure_indexes_creates_expected_indexes(engine_with_tables): + """_ensure_indexes should create all performance indexes.""" + engine = engine_with_tables + inspector = inspect(engine) + + # Run the migration + _ensure_indexes(engine, inspector) + + # Refresh inspector after DDL + inspector = inspect(engine) + + expected = { + "files": {"ix_files_created_at", "ix_files_mime_type"}, + "processing_logs": {"ix_processing_logs_file_id", "ix_processing_logs_timestamp"}, + "file_processing_steps": {"ix_file_processing_steps_status"}, + } + + for table, idx_names in expected.items(): + actual_indexes = {idx["name"] for idx in inspector.get_indexes(table)} + for name in idx_names: + assert name in actual_indexes, f"Index {name} missing from {table}; found {actual_indexes}" + + +@pytest.mark.unit +def test_ensure_indexes_idempotent(engine_with_tables): + """_ensure_indexes should be safe to run multiple times.""" + engine = engine_with_tables + inspector = inspect(engine) + + _ensure_indexes(engine, inspector) + # Running again should not raise + inspector2 = inspect(engine) + _ensure_indexes(engine, inspector2) + + +@pytest.mark.unit +def test_model_indexes_declared(): + """Verify key indexes are declared in SQLAlchemy model metadata.""" + from app.models import FileProcessingStep, FileRecord, ProcessingLog + + # FileRecord.created_at should be indexed + assert FileRecord.__table__.c.created_at.index is True + # FileRecord.mime_type should be indexed + assert FileRecord.__table__.c.mime_type.index is True + # ProcessingLog.file_id should be indexed + assert ProcessingLog.__table__.c.file_id.index is True + # ProcessingLog.timestamp should be indexed + assert ProcessingLog.__table__.c.timestamp.index is True + # FileProcessingStep.status should be indexed + assert FileProcessingStep.__table__.c.status.index is True