diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index c2d14ee6..88405d26 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -32,6 +32,7 @@ }, "mounts": [ "source=${localEnv:HOME}/.claude,target=/home/vscode/.claude,type=bind", + "source=${localEnv:HOME}/.gitconfig,target=/home/vscode/.gitconfig,type=bind,readonly", "source=fileglancer-pixi,target=${containerWorkspaceFolder}/.pixi,type=volume" ], "remoteEnv": { diff --git a/fileglancer/alembic/versions/c4e8a7d92b15_add_user_apps_table.py b/fileglancer/alembic/versions/c4e8a7d92b15_add_user_apps_table.py new file mode 100644 index 00000000..16943782 --- /dev/null +++ b/fileglancer/alembic/versions/c4e8a7d92b15_add_user_apps_table.py @@ -0,0 +1,124 @@ +"""add user_apps table + +Revision ID: c4e8a7d92b15 +Revises: 20b763c28c4f +Create Date: 2026-05-24 00:00:00.000000 + +""" +from datetime import datetime, UTC + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'c4e8a7d92b15' +down_revision = '20b763c28c4f' +branch_labels = None +depends_on = None + + +def _parse_iso(value): + """Parse an ISO 8601 timestamp string into a naive UTC datetime. + + Returns None if value is falsy or cannot be parsed. + """ + if not value: + return None + if isinstance(value, datetime): + return value + try: + dt = datetime.fromisoformat(str(value).replace("Z", "+00:00")) + except ValueError: + return None + if dt.tzinfo is not None: + dt = dt.astimezone(UTC).replace(tzinfo=None) + return dt + + +def upgrade() -> None: + op.create_table( + 'user_apps', + sa.Column('id', sa.Integer(), primary_key=True, autoincrement=True), + sa.Column('username', sa.String(), nullable=False), + sa.Column('url', sa.String(), nullable=False), + sa.Column('manifest_path', sa.String(), nullable=False, server_default=''), + sa.Column('name', sa.String(), nullable=False), + sa.Column('description', sa.String(), nullable=True), + sa.Column('branch', sa.String(), nullable=True), + sa.Column('manifest', sa.JSON(), nullable=True), + sa.Column('added_at', sa.DateTime(), nullable=False), + sa.Column('updated_at', sa.DateTime(), nullable=True), + sa.UniqueConstraint('username', 'url', 'manifest_path', name='uq_user_app'), + ) + op.create_index('ix_user_apps_username', 'user_apps', ['username']) + + # Data migration: move user_preferences['apps'] into user_apps rows. + user_preferences = sa.table( + 'user_preferences', + sa.column('id', sa.Integer), + sa.column('username', sa.String), + sa.column('key', sa.String), + sa.column('value', sa.JSON), + ) + user_apps = sa.table( + 'user_apps', + sa.column('username', sa.String), + sa.column('url', sa.String), + sa.column('manifest_path', sa.String), + sa.column('name', sa.String), + sa.column('description', sa.String), + sa.column('branch', sa.String), + sa.column('manifest', sa.JSON), + sa.column('added_at', sa.DateTime), + sa.column('updated_at', sa.DateTime), + ) + + conn = op.get_bind() + rows = conn.execute( + sa.select( + user_preferences.c.id, + user_preferences.c.username, + user_preferences.c.value, + ).where(user_preferences.c.key == 'apps') + ).fetchall() + + now = datetime.now(UTC).replace(tzinfo=None) + seen: set[tuple[str, str, str]] = set() + inserts = [] + for _pref_id, username, value in rows: + app_list = (value or {}).get('apps', []) if isinstance(value, dict) else [] + for entry in app_list: + if not isinstance(entry, dict): + continue + url = entry.get('url') + if not url: + continue + manifest_path = entry.get('manifest_path') or '' + key = (username, url, manifest_path) + if key in seen: + continue + seen.add(key) + inserts.append({ + 'username': username, + 'url': url, + 'manifest_path': manifest_path, + 'name': entry.get('name') or 'Unknown', + 'description': entry.get('description'), + 'branch': None, + 'manifest': None, + 'added_at': _parse_iso(entry.get('added_at')) or now, + 'updated_at': _parse_iso(entry.get('updated_at')), + }) + + if inserts: + conn.execute(user_apps.insert(), inserts) + + conn.execute( + user_preferences.delete().where(user_preferences.c.key == 'apps') + ) + + +def downgrade() -> None: + op.drop_index('ix_user_apps_username', table_name='user_apps') + op.drop_table('user_apps') diff --git a/fileglancer/apps/__init__.py b/fileglancer/apps/__init__.py index b101295e..d3906774 100644 --- a/fileglancer/apps/__init__.py +++ b/fileglancer/apps/__init__.py @@ -11,6 +11,8 @@ discover_app_manifests, fetch_app_manifest, get_app_branch, + get_or_load_manifest, + refresh_cached_manifest, get_job_file_content, get_job_file_paths, get_service_url, diff --git a/fileglancer/apps/core.py b/fileglancer/apps/core.py index 89e679cb..a117b7f2 100644 --- a/fileglancer/apps/core.py +++ b/fileglancer/apps/core.py @@ -317,6 +317,86 @@ async def fetch_app_manifest(url: str, manifest_path: str = "", return _read_manifest_file(target_dir) +async def get_or_load_manifest(username: str, url: str, + manifest_path: str = "") -> AppManifest: + """Return the manifest for an app, preferring the DB cache. + + Hot path: a single SELECT plus model_validate — no disk I/O, + no worker dispatch. + + If the cached manifest is missing (NULL) or fails validation + (schema drift), falls back to reading from disk via + fetch_app_manifest and writes the fresh value back to the row. + + If no row exists for (username, url, manifest_path), reads from + disk and returns the manifest without creating a row (preview + semantics for not-yet-installed apps). + """ + from pydantic import ValidationError + + settings = get_settings() + + with db.get_db_session(settings.db_url) as session: + row = db.get_user_app(session, username, url, manifest_path) + stored = row.manifest if row else None + row_exists = row is not None + + if stored is not None: + try: + return AppManifest(**stored) + except ValidationError as e: + logger.warning(f"Stored manifest schema mismatch for {url}: {e}") + + manifest = await fetch_app_manifest(url, manifest_path, username=username) + + if row_exists: + branch = await get_app_branch(url) + with db.get_db_session(settings.db_url) as session: + db.upsert_user_app( + session, username, + url=url, manifest_path=manifest_path, + name=manifest.name, description=manifest.description, + branch=branch, + manifest=manifest.model_dump(mode="json"), + bump_updated_at=False, + ) + + return manifest + + +async def refresh_cached_manifest(username: str, url: str, + manifest_path: str = "", + bump_updated_at: bool = False + ) -> tuple[AppManifest, str]: + """Re-read the manifest from disk and sync the cache. + + Call this after any operation that mutates the on-disk YAML + (clone or git pull) so the DB cache stays in lockstep with disk. + + No-op on the DB if (username, url, manifest_path) has no row — + callers that need to insert a new row should use upsert_user_app + directly. + + Returns (manifest, branch). + """ + manifest = await fetch_app_manifest(url, manifest_path, username=username) + branch = await get_app_branch(url) + + settings = get_settings() + with db.get_db_session(settings.db_url) as session: + if db.get_user_app(session, username, url, manifest_path) is not None: + db.upsert_user_app( + session, username, + url=url, manifest_path=manifest_path, + name=manifest.name, description=manifest.description, + branch=branch, + manifest=manifest.model_dump(mode="json"), + bump_updated_at=bump_updated_at, + ) + + return manifest, branch + + async def get_app_branch(url: str) -> str: """Return the branch name for a GitHub app URL. @@ -688,8 +768,11 @@ def build_command(entry_point: AppEntryPoint, parameters: dict, session=None) -> # (bjobs, bsub, bkill) due to HPC root-squash policy. All LSF # operations go through the persistent per-user worker pool. # -# The poll loop picks any user with active jobs and dispatches ``bjobs -# -u all`` through that user's worker to get statuses for ALL users' jobs. +# The poll loop picks any user with active jobs and dispatches a ``poll`` +# action through that user's worker, passing the explicit list of +# cluster_job_ids to query. py-cluster-api's executor then runs ``bjobs`` +# for just those IDs. LSF normally allows querying jobs by ID across +# users, so one worker's call returns statuses for all users' jobs. _poll_task = None _POLL_LOCK_PATH = os.path.join(tempfile.gettempdir(), "fileglancer_poll.lock") @@ -767,8 +850,10 @@ def _get_any_active_username(settings) -> str | None: async def _reconnect_as_any_user(settings): """Reconnect to existing cluster jobs via the persistent worker. - Picks any user with active jobs to run bjobs as. If no active jobs - exist, reconnection is skipped (nothing to reconnect to). + Picks any user with active jobs in the DB and dispatches a ``reconnect`` + action through their worker; py-cluster-api re-attaches to the jobs it + finds. If no active jobs exist in the DB, reconnection is skipped + (nothing to reconnect to). """ username = _get_any_active_username(settings) if not username: @@ -881,7 +966,9 @@ async def _poll_jobs(settings): if settings.cluster.executor == "local": return _poll_local_jobs(session, jobs_to_poll) - # Pick any user to run bjobs as (bjobs -u all sees all users' jobs) + # Pick any user to run the poll through. py-cluster-api will query + # each cluster_job_id explicitly; LSF allows querying jobs by ID + # across users, so one worker's call covers everyone's jobs. poll_username = jobs_to_poll[0].username # Pass current known statuses so stubs are seeded correctly. # Without this, stubs default to PENDING and jobs whose status @@ -1102,8 +1189,8 @@ async def submit_job( """ settings = get_settings() - # Fetch and validate manifest (clones repo into user's cache) - manifest = await fetch_app_manifest(app_url, manifest_path, username=username) + # Read manifest from the cache when available; fall back to disk. + manifest = await get_or_load_manifest(username, app_url, manifest_path) # Find entry point entry_point = None @@ -1183,14 +1270,23 @@ async def submit_job( session.commit() # Clone/pull repo into the user's cache (~username/.fileglancer/apps). - if manifest.repo_url: + if manifest.repo_url and manifest.repo_url != app_url: cached_repo_dir = await _ensure_repo_cache(manifest.repo_url, pull=pull_latest, username=username) cd_suffix = "repo" + pulled_manifest_repo = False else: cached_repo_dir = await _ensure_repo_cache(app_url, pull=pull_latest, username=username) cd_suffix = f"repo/{manifest_path}" if manifest_path else "repo" + pulled_manifest_repo = pull_latest + + # If the pull just changed the YAML on disk, sync the cache. + if pulled_manifest_repo: + try: + await refresh_cached_manifest(username, app_url, manifest_path) + except Exception as e: + logger.warning(f"Failed to refresh cached manifest after pull: {e}") # Build environment variable export lines env_lines = "" diff --git a/fileglancer/database.py b/fileglancer/database.py index 4a52ca3f..cc090e5c 100644 --- a/fileglancer/database.py +++ b/fileglancer/database.py @@ -166,6 +166,26 @@ class JobDB(Base): finished_at = Column(DateTime, nullable=True) +class UserAppDB(Base): + """Database model for a user's installed apps with cached manifests.""" + __tablename__ = 'user_apps' + + id = Column(Integer, primary_key=True, autoincrement=True) + username = Column(String, nullable=False, index=True) + url = Column(String, nullable=False) + manifest_path = Column(String, nullable=False, server_default="") + name = Column(String, nullable=False) + description = Column(String, nullable=True) + branch = Column(String, nullable=True) + manifest = Column(JSON, nullable=True) + added_at = Column(DateTime, nullable=False, default=lambda: datetime.now(UTC)) + updated_at = Column(DateTime, nullable=True) + + __table_args__ = ( + UniqueConstraint('username', 'url', 'manifest_path', name='uq_user_app'), + ) + + class SessionDB(Base): """Database model for storing user sessions""" __tablename__ = 'sessions' @@ -986,3 +1006,76 @@ def delete_old_jobs(session: Session, days: int = 30) -> int: ).delete(synchronize_session='fetch') session.commit() return deleted + + +# --- User app database functions --- + +def list_user_apps(session: Session, username: str) -> List[UserAppDB]: + """Get all apps installed by a user, oldest first.""" + return ( + session.query(UserAppDB) + .filter_by(username=username) + .order_by(UserAppDB.added_at.asc()) + .all() + ) + + +def get_user_app(session: Session, username: str, url: str, + manifest_path: str = "") -> Optional[UserAppDB]: + """Get a single user app by (username, url, manifest_path).""" + return session.query(UserAppDB).filter_by( + username=username, + url=url, + manifest_path=manifest_path, + ).first() + + +def upsert_user_app(session: Session, username: str, url: str, + manifest_path: str = "", *, + name: str, + description: Optional[str] = None, + branch: Optional[str] = None, + manifest: Optional[Dict] = None, + bump_updated_at: bool = True) -> UserAppDB: + """Insert or update a user app row. + + On insert, added_at is set to now and updated_at stays NULL. + On update, added_at is preserved. updated_at is bumped only when + bump_updated_at is True (the default) — set False for invisible + refreshes like a lazy manifest backfill. + """ + now = datetime.now(UTC) + row = get_user_app(session, username, url, manifest_path) + if row is None: + row = UserAppDB( + username=username, + url=url, + manifest_path=manifest_path, + name=name, + description=description, + branch=branch, + manifest=manifest, + added_at=now, + ) + session.add(row) + else: + row.name = name + row.description = description + row.branch = branch + row.manifest = manifest + if bump_updated_at: + row.updated_at = now + session.commit() + return row + + +def delete_user_app(session: Session, username: str, url: str, + manifest_path: str = "") -> bool: + """Delete a user app row. Returns True if a row was deleted.""" + deleted = session.query(UserAppDB).filter_by( + username=username, + url=url, + manifest_path=manifest_path, + ).delete() + session.commit() + return deleted > 0 diff --git a/fileglancer/server.py b/fileglancer/server.py index 92b29d54..a91f3700 100644 --- a/fileglancer/server.py +++ b/fileglancer/server.py @@ -21,7 +21,7 @@ import yaml from loguru import logger -from pydantic import HttpUrl +from pydantic import HttpUrl, ValidationError from contextlib import asynccontextmanager from fastapi import FastAPI, HTTPException, Request, Query, Path, Body, Depends from fastapi.middleware.cors import CORSMiddleware @@ -1521,9 +1521,9 @@ async def fetch_manifest(body: ManifestFetchRequest, username: str = Depends(get_current_user)): try: logger.info(f"Fetching manifest for URL: '{body.url}' path: '{body.manifest_path}'") - manifest = await apps_module.fetch_app_manifest(body.url, body.manifest_path, - username=username) - return manifest + return await apps_module.get_or_load_manifest( + username, body.url, body.manifest_path, + ) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) except Exception as e: @@ -1533,32 +1533,63 @@ async def fetch_manifest(body: ManifestFetchRequest, description="Get the user's configured apps with their manifests") async def get_user_apps(username: str = Depends(get_current_user)): with db.get_db_session(settings.db_url) as session: - pref = db.get_user_preference(session, username, "apps") - - app_list = pref.get("apps", []) if pref else [] - result = [] - for app_entry in app_list: - user_app = UserApp( - url=app_entry["url"], - manifest_path=app_entry.get("manifest_path", ""), - name=app_entry.get("name", "Unknown"), - description=app_entry.get("description"), - added_at=app_entry.get("added_at", datetime.now(UTC).isoformat()), - updated_at=app_entry.get("updated_at"), - ) - # Try to fetch manifest from local clone + rows = db.list_user_apps(session, username) + snapshots = [ + { + "url": row.url, + "manifest_path": row.manifest_path, + "name": row.name, + "description": row.description, + "branch": row.branch, + "manifest": row.manifest, + "added_at": row.added_at, + "updated_at": row.updated_at, + } + for row in rows + ] + + result: list[UserApp] = [] + needs_backfill: list[int] = [] + for idx, snap in enumerate(snapshots): + manifest_obj: Optional[AppManifest] = None + stored = snap["manifest"] + if stored is not None: + try: + manifest_obj = AppManifest(**stored) + except ValidationError as e: + logger.warning( + f"Stored manifest schema mismatch for {snap['url']}: {e}" + ) + needs_backfill.append(idx) + else: + needs_backfill.append(idx) + + result.append(UserApp( + url=snap["url"], + manifest_path=snap["manifest_path"], + name=snap["name"], + description=snap["description"], + branch=snap["branch"], + added_at=snap["added_at"], + updated_at=snap["updated_at"], + manifest=manifest_obj, + )) + + for idx in needs_backfill: + snap = snapshots[idx] try: - user_app.manifest = await apps_module.fetch_app_manifest( - app_entry["url"], app_entry.get("manifest_path", ""), - username=username, + manifest, branch = await apps_module.refresh_cached_manifest( + username, snap["url"], snap["manifest_path"], ) - # Update name/description from manifest - user_app.name = user_app.manifest.name - user_app.description = user_app.manifest.description - user_app.branch = await apps_module.get_app_branch(app_entry["url"]) except Exception as e: - logger.warning(f"Failed to fetch manifest for {app_entry['url']}: {e}") - result.append(user_app) + logger.warning(f"Failed to fetch manifest for {snap['url']}: {e}") + continue + + user_app = result[idx] + user_app.manifest = manifest + user_app.name = manifest.name + user_app.description = manifest.description + user_app.branch = branch return result @app.post("/api/apps", response_model=list[UserApp], @@ -1582,48 +1613,36 @@ async def add_user_app(body: AppAddRequest, f"Make sure a manifest exists in the repository.", ) - now = datetime.now(UTC) + branch = await apps_module.get_app_branch(body.url) + new_apps: list[UserApp] = [] with db.get_db_session(settings.db_url) as session: - pref = db.get_user_preference(session, username, "apps") - app_list = pref.get("apps", []) if pref else [] - - # Build set of existing (url, manifest_path) for dedup - existing_keys = { - (a["url"], a.get("manifest_path", "")) for a in app_list - } - - branch = await apps_module.get_app_branch(body.url) - new_apps: list[UserApp] = [] for manifest_path, manifest in discovered: - if (body.url, manifest_path) in existing_keys: + if db.get_user_app(session, username, body.url, manifest_path) is not None: continue # silently skip duplicates - - new_entry = { - "url": body.url, - "manifest_path": manifest_path, - "name": manifest.name, - "description": manifest.description, - "added_at": now.isoformat(), - } - app_list.append(new_entry) - new_apps.append(UserApp( - url=body.url, - manifest_path=manifest_path, + row = db.upsert_user_app( + session, username, + url=body.url, manifest_path=manifest_path, + name=manifest.name, description=manifest.description, branch=branch, - name=manifest.name, - description=manifest.description, - added_at=now, + manifest=manifest.model_dump(mode="json"), + ) + new_apps.append(UserApp( + url=row.url, + manifest_path=row.manifest_path, + branch=row.branch, + name=row.name, + description=row.description, + added_at=row.added_at, + updated_at=row.updated_at, manifest=manifest, )) - if not new_apps: - raise HTTPException( - status_code=409, - detail="All apps in this repository have already been added.", - ) - - db.set_user_preference(session, username, "apps", {"apps": app_list}) + if not new_apps: + raise HTTPException( + status_code=409, + detail="All apps in this repository have already been added.", + ) return new_apps @@ -1633,18 +1652,8 @@ async def remove_user_app(url: str = Query(..., description="URL of the app to r manifest_path: str = Query("", description="Manifest path within the repo"), username: str = Depends(get_current_user)): with db.get_db_session(settings.db_url) as session: - pref = db.get_user_preference(session, username, "apps") - app_list = pref.get("apps", []) if pref else [] - - new_list = [ - a for a in app_list - if not (a["url"] == url and a.get("manifest_path", "") == manifest_path) - ] - if len(new_list) == len(app_list): + if not db.delete_user_app(session, username, url, manifest_path): raise HTTPException(status_code=404, detail="App not found") - - db.set_user_preference(session, username, "apps", {"apps": new_list}) - return {"message": "App removed"} @app.post("/api/apps/update", response_model=UserApp, @@ -1664,33 +1673,26 @@ async def update_user_app(body: ManifestFetchRequest, except Exception as e: raise HTTPException(status_code=400, detail=f"Failed to read manifest after update: {str(e)}") - now = datetime.now(UTC) + branch = await apps_module.get_app_branch(body.url) - # Update stored name/description/updated_at from refreshed manifest with db.get_db_session(settings.db_url) as session: - pref = db.get_user_preference(session, username, "apps") - app_list = pref.get("apps", []) if pref else [] - added_at = now # fallback - for entry in app_list: - if entry["url"] == body.url and entry.get("manifest_path", "") == body.manifest_path: - entry["name"] = manifest.name - entry["description"] = manifest.description - entry["updated_at"] = now.isoformat() - added_at = entry.get("added_at", now.isoformat()) - break - db.set_user_preference(session, username, "apps", {"apps": app_list}) - - branch = await apps_module.get_app_branch(body.url) - return UserApp( - url=body.url, - manifest_path=body.manifest_path, - branch=branch, - name=manifest.name, - description=manifest.description, - added_at=added_at, - updated_at=now, - manifest=manifest, - ) + row = db.upsert_user_app( + session, username, + url=body.url, manifest_path=body.manifest_path, + name=manifest.name, description=manifest.description, + branch=branch, + manifest=manifest.model_dump(mode="json"), + ) + return UserApp( + url=row.url, + manifest_path=row.manifest_path, + branch=row.branch, + name=row.name, + description=row.description, + added_at=row.added_at, + updated_at=row.updated_at, + manifest=manifest, + ) @app.post("/api/apps/validate-paths", response_model=PathValidationResponse, description="Validate file/directory paths for app parameters") diff --git a/frontend/src/components/designSystem/atoms/FgButton.stories.tsx b/frontend/src/components/designSystem/atoms/FgButton.stories.tsx index 82965949..af9fb000 100644 --- a/frontend/src/components/designSystem/atoms/FgButton.stories.tsx +++ b/frontend/src/components/designSystem/atoms/FgButton.stories.tsx @@ -1,5 +1,5 @@ import { expect, fn, userEvent, within } from 'storybook/test'; -import { HiDownload, HiOutlinePlus, HiSearch } from 'react-icons/hi'; +import { HiDownload, HiSearch } from 'react-icons/hi'; import type { Meta, StoryObj } from '@storybook/react-vite'; import FgButton from './FgButton'; diff --git a/tests/test_apps_endpoints.py b/tests/test_apps_endpoints.py new file mode 100644 index 00000000..3baf8912 --- /dev/null +++ b/tests/test_apps_endpoints.py @@ -0,0 +1,525 @@ +"""Tests for /api/apps endpoints backed by the user_apps table.""" + +import os +import shutil +import tempfile +from datetime import datetime, UTC, timedelta +from unittest.mock import AsyncMock, patch + +import pytest +from fastapi.testclient import TestClient + +from fileglancer.settings import Settings +from fileglancer.server import create_app, get_current_user +from fileglancer.database import ( + Base, + UserAppDB, + UserPreferenceDB, + create_engine, + sessionmaker, + dispose_engine, + get_db_session, + list_user_apps, +) +from fileglancer.model import AppEntryPoint, AppManifest + + +TEST_USERNAME = "testuser" + + +def _make_manifest(name="Demo App", description="Demo"): + return AppManifest( + name=name, + description=description, + runnables=[AppEntryPoint(id="run", name="Run", command="echo hi")], + ) + + +@pytest.fixture +def temp_dir(): + d = tempfile.mkdtemp() + yield d + shutil.rmtree(d) + + +@pytest.fixture +def test_app(temp_dir): + db_path = os.path.join(temp_dir, "test.db") + db_url = f"sqlite:///{db_path}" + engine = create_engine(db_url) + Base.metadata.create_all(engine) + + settings = Settings(db_url=db_url, file_share_mounts=[], cli_mode=True) + + import fileglancer.settings + import fileglancer.database + import fileglancer.apps.core + original_get_settings = fileglancer.settings.get_settings + fileglancer.settings.get_settings = lambda: settings + fileglancer.database.get_settings = lambda: settings + fileglancer.apps.core.get_settings = lambda: settings + # Migrations are unneeded here since create_all built the schema. + fileglancer.database._migrations_run = True + + app = create_app(settings) + yield app, db_url + + engine.dispose() + dispose_engine(db_url) + fileglancer.settings.get_settings = original_get_settings + fileglancer.database.get_settings = original_get_settings + fileglancer.apps.core.get_settings = original_get_settings + fileglancer.database._migrations_run = False + + +@pytest.fixture +def test_client(test_app): + app, _ = test_app + app.dependency_overrides[get_current_user] = lambda: TEST_USERNAME + yield TestClient(app) + app.dependency_overrides.clear() + + +@pytest.fixture +def db_session(test_app): + _, db_url = test_app + session = get_db_session(db_url) + yield session + session.close() + + +def _seed_app(db_session, *, url="https://github.com/owner/repo", + manifest_path="", manifest=None, name="Demo App", + description="Demo", branch="main", + added_at=None, updated_at=None): + row = UserAppDB( + username=TEST_USERNAME, + url=url, + manifest_path=manifest_path, + name=name, + description=description, + branch=branch, + manifest=manifest, + added_at=added_at or datetime.now(UTC), + updated_at=updated_at, + ) + db_session.add(row) + db_session.commit() + return row + + +def test_get_apps_empty(test_client): + response = test_client.get("/api/apps") + assert response.status_code == 200 + assert response.json() == [] + + +def test_get_apps_uses_db_cache(test_client, db_session): + manifest = _make_manifest() + _seed_app(db_session, manifest=manifest.model_dump(mode="json")) + + with patch("fileglancer.apps.fetch_app_manifest", + new=AsyncMock()) as mock_fetch, \ + patch("fileglancer.apps.get_app_branch", + new=AsyncMock()) as mock_branch: + response = test_client.get("/api/apps") + + assert response.status_code == 200 + assert mock_fetch.await_count == 0 + assert mock_branch.await_count == 0 + + body = response.json() + assert len(body) == 1 + assert body[0]["name"] == "Demo App" + assert body[0]["branch"] == "main" + assert body[0]["manifest"]["name"] == "Demo App" + + +def test_get_apps_backfills_null_manifest(test_client, db_session): + _seed_app(db_session, manifest=None, branch=None, name="Stale Name") + manifest = _make_manifest(name="Fresh Name", description="Fresh") + + # refresh_cached_manifest calls these directly inside apps/core.py, so + # patches must target the core namespace, not the apps re-export. + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock(return_value=manifest)) as mock_fetch, \ + patch("fileglancer.apps.core.get_app_branch", + new=AsyncMock(return_value="dev")) as mock_branch: + response = test_client.get("/api/apps") + + assert response.status_code == 200 + assert mock_fetch.await_count == 1 + assert mock_branch.await_count == 1 + + body = response.json() + assert body[0]["name"] == "Fresh Name" + assert body[0]["branch"] == "dev" + assert body[0]["manifest"]["name"] == "Fresh Name" + + # Row is persisted; subsequent reads hit the cache. + rows = list_user_apps(db_session, TEST_USERNAME) + assert len(rows) == 1 + assert rows[0].manifest is not None + assert rows[0].manifest["name"] == "Fresh Name" + assert rows[0].branch == "dev" + # Backfill should NOT bump updated_at (invisible refresh). + assert rows[0].updated_at is None + + +def test_get_apps_handles_schema_drift(test_client, db_session): + # Manifest missing required field 'runnables' → ValidationError. + _seed_app(db_session, manifest={"name": "Broken"}, branch=None) + fresh = _make_manifest(name="Recovered") + + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock(return_value=fresh)) as mock_fetch, \ + patch("fileglancer.apps.core.get_app_branch", + new=AsyncMock(return_value="main")): + response = test_client.get("/api/apps") + + assert response.status_code == 200 + assert mock_fetch.await_count == 1 + + body = response.json() + assert body[0]["name"] == "Recovered" + assert body[0]["manifest"]["name"] == "Recovered" + + +def test_get_apps_backfill_handles_fetch_failure(test_client, db_session): + _seed_app(db_session, manifest=None, branch=None, name="Cached Name") + + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock(side_effect=RuntimeError("network down"))), \ + patch("fileglancer.apps.core.get_app_branch", + new=AsyncMock(side_effect=RuntimeError("nope"))): + response = test_client.get("/api/apps") + + assert response.status_code == 200 + body = response.json() + assert len(body) == 1 + # Falls back to stored values; manifest stays unpopulated. + assert body[0]["name"] == "Cached Name" + assert body[0]["manifest"] is None + + +def test_add_app_persists_manifest_and_branch(test_client, db_session): + manifest = _make_manifest(name="From Add") + with patch("fileglancer.apps.discover_app_manifests", + new=AsyncMock(return_value=[("", manifest)])), \ + patch("fileglancer.apps.get_app_branch", + new=AsyncMock(return_value="main")): + response = test_client.post( + "/api/apps", + json={"url": "https://github.com/owner/repo"}, + ) + + assert response.status_code == 200 + body = response.json() + assert len(body) == 1 + assert body[0]["name"] == "From Add" + assert body[0]["branch"] == "main" + assert body[0]["manifest"]["name"] == "From Add" + + rows = list_user_apps(db_session, TEST_USERNAME) + assert len(rows) == 1 + assert rows[0].manifest["name"] == "From Add" + assert rows[0].branch == "main" + + +def test_add_app_dedups(test_client, db_session): + """Adding the same repo twice returns 409 and inserts no new rows.""" + manifest = _make_manifest() + _seed_app(db_session, url="https://github.com/owner/repo", + manifest=manifest.model_dump(mode="json")) + + with patch("fileglancer.apps.discover_app_manifests", + new=AsyncMock(return_value=[("", manifest)])), \ + patch("fileglancer.apps.get_app_branch", + new=AsyncMock(return_value="main")): + response = test_client.post( + "/api/apps", + json={"url": "https://github.com/owner/repo"}, + ) + + assert response.status_code == 409 + assert len(list_user_apps(db_session, TEST_USERNAME)) == 1 + + +def test_update_app_persists_manifest(test_client, db_session): + older = datetime.now(UTC) - timedelta(days=1) + _seed_app(db_session, manifest=None, name="Old", added_at=older) + + fresh = _make_manifest(name="New", description="New") + + with patch("fileglancer.apps.fetch_app_manifest", + new=AsyncMock(return_value=fresh)), \ + patch("fileglancer.apps.get_app_branch", + new=AsyncMock(return_value="main")), \ + patch("fileglancer.apps._ensure_repo_cache", + new=AsyncMock(return_value="/tmp/x")): + response = test_client.post( + "/api/apps/update", + json={"url": "https://github.com/owner/repo", "manifest_path": ""}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["name"] == "New" + assert body["updated_at"] is not None + + rows = list_user_apps(db_session, TEST_USERNAME) + assert len(rows) == 1 + assert rows[0].name == "New" + assert rows[0].manifest["name"] == "New" + assert rows[0].updated_at is not None + # added_at preserved across update. + assert rows[0].added_at.replace(tzinfo=None) == older.replace(tzinfo=None) + + +def test_delete_app_removes_row(test_client, db_session): + _seed_app(db_session, manifest=_make_manifest().model_dump(mode="json")) + assert len(list_user_apps(db_session, TEST_USERNAME)) == 1 + + response = test_client.delete( + "/api/apps", + params={"url": "https://github.com/owner/repo", "manifest_path": ""}, + ) + assert response.status_code == 200 + assert len(list_user_apps(db_session, TEST_USERNAME)) == 0 + + # Second delete → 404 + response = test_client.delete( + "/api/apps", + params={"url": "https://github.com/owner/repo", "manifest_path": ""}, + ) + assert response.status_code == 404 + + +def test_fetch_manifest_uses_cache_for_installed_app(test_client, db_session): + """POST /api/apps/manifest returns cached manifest without disk read.""" + cached = _make_manifest(name="Cached App") + _seed_app(db_session, manifest=cached.model_dump(mode="json")) + + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock()) as mock_fetch: + response = test_client.post("/api/apps/manifest", json={ + "url": "https://github.com/owner/repo", + "manifest_path": "", + }) + + assert response.status_code == 200 + assert mock_fetch.await_count == 0 + assert response.json()["name"] == "Cached App" + + +def test_fetch_manifest_reads_disk_for_uninstalled(test_client, db_session): + """Preview of an uninstalled URL reads disk and does not create a row.""" + fresh = _make_manifest(name="Preview Only") + + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock(return_value=fresh)) as mock_fetch: + response = test_client.post("/api/apps/manifest", json={ + "url": "https://github.com/new/repo", + "manifest_path": "", + }) + + assert response.status_code == 200 + assert mock_fetch.await_count == 1 + assert response.json()["name"] == "Preview Only" + # No row was created for the preview. + assert list_user_apps(db_session, TEST_USERNAME) == [] + + +def test_fetch_manifest_backfills_null_cache(test_client, db_session): + """If row exists with NULL manifest, endpoint reads disk and writes back.""" + _seed_app(db_session, manifest=None, name="Stale", branch=None) + fresh = _make_manifest(name="Backfilled") + + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock(return_value=fresh)) as mock_fetch, \ + patch("fileglancer.apps.core.get_app_branch", + new=AsyncMock(return_value="main")): + response = test_client.post("/api/apps/manifest", json={ + "url": "https://github.com/owner/repo", + "manifest_path": "", + }) + + assert response.status_code == 200 + assert mock_fetch.await_count == 1 + assert response.json()["name"] == "Backfilled" + + # Row was updated silently (updated_at stays NULL). + rows = list_user_apps(db_session, TEST_USERNAME) + assert len(rows) == 1 + assert rows[0].manifest["name"] == "Backfilled" + assert rows[0].updated_at is None + + +@pytest.mark.asyncio +async def test_get_or_load_manifest_cache_hit(test_app, db_session): + """Cache hit returns parsed manifest without any disk read.""" + from fileglancer.apps import get_or_load_manifest + + cached = _make_manifest(name="From Cache") + _seed_app(db_session, manifest=cached.model_dump(mode="json")) + + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock()) as mock_fetch: + manifest = await get_or_load_manifest( + TEST_USERNAME, "https://github.com/owner/repo", "", + ) + + assert manifest.name == "From Cache" + assert mock_fetch.await_count == 0 + + +@pytest.mark.asyncio +async def test_get_or_load_manifest_preview_no_row(test_app, db_session): + """Preview of uninstalled URL reads disk, no row created.""" + from fileglancer.apps import get_or_load_manifest + + fresh = _make_manifest(name="Preview") + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock(return_value=fresh)) as mock_fetch: + manifest = await get_or_load_manifest( + TEST_USERNAME, "https://github.com/x/y", "", + ) + + assert manifest.name == "Preview" + assert mock_fetch.await_count == 1 + assert list_user_apps(db_session, TEST_USERNAME) == [] + + +@pytest.mark.asyncio +async def test_refresh_cached_manifest_syncs_existing_row(test_app, db_session): + """refresh_cached_manifest updates an existing row from disk.""" + from fileglancer.apps import refresh_cached_manifest + + _seed_app(db_session, manifest=None, name="Stale") + fresh = _make_manifest(name="Synced") + + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock(return_value=fresh)), \ + patch("fileglancer.apps.core.get_app_branch", + new=AsyncMock(return_value="main")): + manifest, branch = await refresh_cached_manifest( + TEST_USERNAME, "https://github.com/owner/repo", "", + ) + + assert manifest.name == "Synced" + assert branch == "main" + + rows = list_user_apps(db_session, TEST_USERNAME) + assert rows[0].manifest["name"] == "Synced" + assert rows[0].branch == "main" + # Silent refresh by default — updated_at stays NULL. + assert rows[0].updated_at is None + + +@pytest.mark.asyncio +async def test_refresh_cached_manifest_no_op_for_uninstalled(test_app, db_session): + """refresh_cached_manifest doesn't create rows for uninstalled apps.""" + from fileglancer.apps import refresh_cached_manifest + + fresh = _make_manifest() + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock(return_value=fresh)), \ + patch("fileglancer.apps.core.get_app_branch", + new=AsyncMock(return_value="main")): + manifest, branch = await refresh_cached_manifest( + TEST_USERNAME, "https://github.com/new/repo", "", + ) + + assert manifest.name == "Demo App" + assert list_user_apps(db_session, TEST_USERNAME) == [] + + +@pytest.mark.asyncio +async def test_refresh_cached_manifest_bumps_updated_at(test_app, db_session): + """bump_updated_at=True is the explicit-user-update path.""" + from fileglancer.apps import refresh_cached_manifest + + _seed_app(db_session, manifest=None, name="Old") + fresh = _make_manifest(name="Updated") + + with patch("fileglancer.apps.core.fetch_app_manifest", + new=AsyncMock(return_value=fresh)), \ + patch("fileglancer.apps.core.get_app_branch", + new=AsyncMock(return_value="main")): + await refresh_cached_manifest( + TEST_USERNAME, "https://github.com/owner/repo", "", + bump_updated_at=True, + ) + + rows = list_user_apps(db_session, TEST_USERNAME) + assert rows[0].updated_at is not None + + +def test_alembic_migration_moves_legacy_apps(temp_dir, monkeypatch): + """The migration relocates user_preferences['apps'] into user_apps.""" + from alembic.config import Config + from alembic import command + + db_path = os.path.join(temp_dir, "legacy.db") + db_url = f"sqlite:///{db_path}" + + # env.py forces the DB URL from FILEGLANCER_MIGRATION_DB_URL or settings, + # so set_main_option('sqlalchemy.url', ...) is not enough — use the env + # var that env.py actually reads. + monkeypatch.setenv("FILEGLANCER_MIGRATION_DB_URL", db_url) + + pkg_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + alembic_ini = os.path.join(pkg_dir, "alembic.ini") + if not os.path.exists(alembic_ini): + alembic_ini = os.path.join(pkg_dir, "fileglancer", "alembic.ini") + assert os.path.exists(alembic_ini), f"alembic.ini not found near {pkg_dir}" + + cfg = Config(alembic_ini) + cfg.set_main_option("sqlalchemy.url", db_url) + + # 1) Upgrade to the revision just before ours. + command.upgrade(cfg, "20b763c28c4f") + + # 2) Seed legacy apps preference. + engine = create_engine(db_url) + Session = sessionmaker(bind=engine) + s = Session() + s.add(UserPreferenceDB( + username=TEST_USERNAME, + key="apps", + value={"apps": [ + { + "url": "https://github.com/owner/repo", + "manifest_path": "", + "name": "Legacy", + "description": "From prefs", + "added_at": "2025-01-01T00:00:00+00:00", + }, + { + "url": "https://github.com/owner/repo", + "manifest_path": "sub", + "name": "Legacy Sub", + "added_at": "2025-01-02T00:00:00", + }, + ]}, + )) + s.commit() + s.close() + + # 3) Run our migration. + command.upgrade(cfg, "c4e8a7d92b15") + + # 4) Verify rows moved and preference is gone. + s = Session() + apps = s.query(UserAppDB).filter_by(username=TEST_USERNAME).order_by(UserAppDB.manifest_path).all() + assert len(apps) == 2 + assert apps[0].name == "Legacy" + assert apps[0].manifest_path == "" + assert apps[0].manifest is None # backfilled lazily + assert apps[0].branch is None + assert apps[1].manifest_path == "sub" + + prefs = s.query(UserPreferenceDB).filter_by(username=TEST_USERNAME, key="apps").all() + assert prefs == [] + s.close() + engine.dispose()