diff --git a/backend/alembic/versions/d2e3f4a5b6c7_media_libraries.py b/backend/alembic/versions/d2e3f4a5b6c7_media_libraries.py new file mode 100644 index 0000000..cf0cfb8 --- /dev/null +++ b/backend/alembic/versions/d2e3f4a5b6c7_media_libraries.py @@ -0,0 +1,55 @@ +"""Image libraries, and per-library access grants. + +An image lives in one library; a person is granted a library the way they are +granted a category, so access can be given to some images without giving away +all of them. + +Revision ID: d2e3f4a5b6c7 +Revises: c1d2e3f4a5b6 +""" +from alembic import op + +revision = "d2e3f4a5b6c7" +down_revision = "c1d2e3f4a5b6" +branch_labels = None +depends_on = None + + +def upgrade(): + op.execute(""" + CREATE TABLE IF NOT EXISTS media_libraries ( + id SERIAL PRIMARY KEY, + name VARCHAR(200) UNIQUE NOT NULL, + description TEXT, + user_id INTEGER REFERENCES users(id) ON DELETE SET NULL, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP + ) + """) + op.execute(""" + CREATE TABLE IF NOT EXISTS media_library_grants ( + id SERIAL PRIMARY KEY, + library_id INTEGER NOT NULL REFERENCES media_libraries(id) ON DELETE CASCADE, + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + granted_by INTEGER REFERENCES users(id) ON DELETE SET NULL, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + CONSTRAINT uq_media_library_grant UNIQUE (library_id, user_id) + ) + """) + op.execute("ALTER TABLE media_assets ADD COLUMN IF NOT EXISTS library_id INTEGER REFERENCES media_libraries(id) ON DELETE SET NULL") + op.execute("ALTER TABLE media_assets ADD COLUMN IF NOT EXISTS storage VARCHAR(10) NOT NULL DEFAULT 'local'") + op.execute("ALTER TABLE media_assets ADD COLUMN IF NOT EXISTS byte_size INTEGER") + op.execute("CREATE INDEX IF NOT EXISTS ix_media_library ON media_assets(library_id)") + op.execute(""" + INSERT INTO media_libraries (name, description) + VALUES ('General', 'Images that have not been sorted into a library yet') + ON CONFLICT (name) DO NOTHING + """) + + +def downgrade(): + op.execute("DROP INDEX IF EXISTS ix_media_library") + op.execute("ALTER TABLE media_assets DROP COLUMN IF EXISTS byte_size") + op.execute("ALTER TABLE media_assets DROP COLUMN IF EXISTS storage") + op.execute("ALTER TABLE media_assets DROP COLUMN IF EXISTS library_id") + op.execute("DROP TABLE IF EXISTS media_library_grants") + op.execute("DROP TABLE IF EXISTS media_libraries") diff --git a/backend/app/config.py b/backend/app/config.py index 37d0886..3b090e5 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -43,6 +43,14 @@ class Settings(BaseSettings): MAIL_SSL_TLS: bool = False UPLOAD_DIR: str = "./uploads" + # local | s3. Reads fall back to the volume either way, so existing uploads + # keep working and files can migrate gradually. + STORAGE_BACKEND: str = "local" + S3_ENDPOINT_URL: str = "http://minio:9000" + S3_ACCESS_KEY: str = "" + S3_SECRET_KEY: str = "" + S3_BUCKET: str = "pedshub-media" + S3_REGION: str = "us-east-1" MAX_UPLOAD_SIZE: int = 524288000 # 500MB TURNSTILE_SECRET_KEY: str = "" # Cloudflare Turnstile — leave blank to disable captcha diff --git a/backend/app/main.py b/backend/app/main.py index 0ba6bbf..2257bc6 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -11,7 +11,7 @@ from app.logging_config import setup_logging setup_logging(settings.LOG_LEVEL) from app.database import engine, Base, SessionLocal from app.routers import auth, documents, quizzes, attempts, admin, tts, nextcloud, categories, questions, question_categories, favorites, teach, contact, tags, flashcards, courses, mobile, mynote, exams -from app.routers import study_tools, uploads, articles, comments, share, collections, study_plans +from app.routers import study_tools, uploads, articles, comments, share, collections, study_plans, media from app.utils.auth import get_password_hash from app.utils.scheduler import start_scheduler, stop_scheduler @@ -620,6 +620,7 @@ app.include_router(auth.router, prefix="/api/auth", tags=["auth"]) app.include_router(articles.router, prefix="/api/articles", tags=["articles"]) app.include_router(exams.router, prefix="/api/exams", tags=["exams"]) app.include_router(study_plans.router, prefix="/api/study-plans", tags=["study-plans"]) +app.include_router(media.router, prefix="/api/media", tags=["media"]) app.include_router(comments.router, prefix="/api/comments", tags=["comments"]) app.include_router(share.router, prefix="/api/share", tags=["share"]) app.include_router(collections.router, prefix="/api/collections", tags=["collections"]) diff --git a/backend/app/models/media.py b/backend/app/models/media.py index ff665ec..082bf69 100644 --- a/backend/app/models/media.py +++ b/backend/app/models/media.py @@ -22,6 +22,10 @@ class MediaAsset(Base, Embeddable): caption = Column(Text, nullable=True) alt_text = Column(Text, nullable=True) kind = Column(String(20), default="image") + library_id = Column(Integer, ForeignKey("media_libraries.id", ondelete="SET NULL"), nullable=True, index=True) + # Which backend holds the bytes; reads fall back to the volume either way. + storage = Column(String(10), default="local") + byte_size = Column(Integer, nullable=True) category_id = Column(Integer, ForeignKey("question_categories.id", ondelete="SET NULL"), nullable=True) user_id = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) created_at = Column(DateTime, default=datetime.utcnow) @@ -36,3 +40,28 @@ class MediaTagLink(Base): # `question_tags` is created by raw DDL rather than an ORM model, so the # constraint is declared in the migration; the mapper only stores the id. tag_id = Column(Integer, nullable=False, index=True) + + +class MediaLibrary(Base): + """A named group of images, so access can be given to some without all.""" + + __tablename__ = "media_libraries" + + id = Column(Integer, primary_key=True, index=True) + name = Column(String(200), unique=True, nullable=False) + description = Column(Text, nullable=True) + user_id = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) + created_at = Column(DateTime, default=datetime.utcnow) + + +class MediaLibraryGrant(Base): + """Lets one person work with one library, mirroring the category grants.""" + + __tablename__ = "media_library_grants" + __table_args__ = (UniqueConstraint("library_id", "user_id", name="uq_media_library_grant"),) + + id = Column(Integer, primary_key=True, index=True) + library_id = Column(Integer, ForeignKey("media_libraries.id", ondelete="CASCADE"), nullable=False, index=True) + user_id = Column(Integer, ForeignKey("users.id", ondelete="CASCADE"), nullable=False, index=True) + granted_by = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) + created_at = Column(DateTime, default=datetime.utcnow) diff --git a/backend/app/routers/media.py b/backend/app/routers/media.py new file mode 100644 index 0000000..d2fe3a6 --- /dev/null +++ b/backend/app/routers/media.py @@ -0,0 +1,270 @@ +"""The image bank: libraries, tags, and per-library access. + +Images are stored through `storage_service`, so the bytes live in MinIO when the +backend is set to s3 and on the volume otherwise. A row stores the key, never a +URL, because a URL embeds the backend and would break the moment it changed. +""" +import logging +import uuid + +from fastapi import APIRouter, Depends, File, Form, HTTPException, Query, UploadFile +from pydantic import BaseModel +from sqlalchemy import func, or_, text as sa_text +from sqlalchemy.orm import Session + +from app.database import get_db +from app.models.media import MediaAsset, MediaLibrary, MediaLibraryGrant, MediaTagLink +from app.models.user import User +from app.services import embedding_service, storage_service +from app.services.search_service import hybrid_ids +from app.utils.auth import get_current_user, require_moderator + +router = APIRouter() +log = logging.getLogger(__name__) + +MAX_IMAGE_BYTES = 12 * 1024 * 1024 +ALLOWED_TYPES = {"image/png", "image/jpeg", "image/gif", "image/webp", "image/svg+xml"} + + +def readable_libraries(db: Session, user: User) -> set[int] | None: + """Library ids this user may use; None means every one of them.""" + if user.is_moderator: + return None + return {row[0] for row in db.query(MediaLibraryGrant.library_id).filter( + MediaLibraryGrant.user_id == user.id).all()} + + +def assert_can_use(scope: set[int] | None, library_id: int | None) -> None: + if scope is None: + return + if library_id is None or library_id not in scope: + raise HTTPException(403, "You do not have access to that image library") + + +def _asset_json(asset: MediaAsset, tags: list[str]) -> dict: + return { + "id": asset.id, + "path": asset.path, + "title": asset.title, + "caption": asset.caption, + "alt_text": asset.alt_text, + "kind": asset.kind, + "library_id": asset.library_id, + "category_id": asset.category_id, + "byte_size": asset.byte_size, + "storage": asset.storage, + "tags": tags, + # The editor shows this on hover so an image can be referenced by id. + "url": f"/uploads/{asset.path}", + } + + +def _tags_for(db: Session, asset_ids: list[int]) -> dict[int, list[str]]: + if not asset_ids: + return {} + # Bound parameters rather than ANY(), which is Postgres-only. + placeholders = ", ".join(f":id{i}" for i in range(len(asset_ids))) + rows = db.execute(sa_text(f""" + SELECT l.media_id, t.name FROM media_tag_links l + JOIN question_tags t ON t.id = l.tag_id + WHERE l.media_id IN ({placeholders}) + """), {f"id{i}": value for i, value in enumerate(asset_ids)}).fetchall() + out: dict[int, list[str]] = {} + for media_id, name in rows: + out.setdefault(media_id, []).append(name) + return out + + +@router.get("/libraries") +def list_libraries(db: Session = Depends(get_db), current_user: User = Depends(get_current_user)): + """Libraries this user may use, with how many images each holds.""" + scope = readable_libraries(db, current_user) + counts = dict(db.query(MediaAsset.library_id, func.count(MediaAsset.id)) + .group_by(MediaAsset.library_id).all()) + query = db.query(MediaLibrary).order_by(MediaLibrary.name) + if scope is not None: + query = query.filter(MediaLibrary.id.in_(scope or {0})) + return [{"id": lib.id, "name": lib.name, "description": lib.description, + "image_count": counts.get(lib.id, 0)} for lib in query.all()] + + +class LibraryIn(BaseModel): + name: str + description: str | None = None + + +@router.post("/libraries", status_code=201) +def create_library(data: LibraryIn, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + if db.query(MediaLibrary.id).filter(func.lower(MediaLibrary.name) == data.name.strip().lower()).first(): + raise HTTPException(409, "A library with that name already exists") + library = MediaLibrary(name=data.name.strip(), description=data.description, user_id=current_user.id) + db.add(library) + db.commit() + db.refresh(library) + return {"id": library.id, "name": library.name} + + +class LibraryGrantIn(BaseModel): + user_id: int + + +@router.post("/libraries/{library_id}/grants", status_code=201) +def grant_library(library_id: int, data: LibraryGrantIn, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + if not db.get(MediaLibrary, library_id): + raise HTTPException(404, "Library not found") + if not db.get(User, data.user_id): + raise HTTPException(404, "User not found") + if db.query(MediaLibraryGrant.id).filter_by(library_id=library_id, user_id=data.user_id).first(): + raise HTTPException(409, "That user already has access to this library") + db.add(MediaLibraryGrant(library_id=library_id, user_id=data.user_id, granted_by=current_user.id)) + db.commit() + return {"library_id": library_id, "user_id": data.user_id} + + +@router.delete("/libraries/{library_id}/grants/{user_id}", status_code=204) +def revoke_library(library_id: int, user_id: int, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + grant = db.query(MediaLibraryGrant).filter_by(library_id=library_id, user_id=user_id).first() + if not grant: + raise HTTPException(404, "Grant not found") + db.delete(grant) + db.commit() + + +@router.get("/") +def list_media( + q: str | None = Query(None), + library_id: int | None = Query(None), + limit: int = Query(60, le=200), + offset: int = Query(0), + db: Session = Depends(get_db), + current_user: User = Depends(get_current_user), +): + """Browse the image bank. `q` searches captions, alt text and titles.""" + scope = readable_libraries(db, current_user) + query = db.query(MediaAsset) + if scope is not None: + query = query.filter(MediaAsset.library_id.in_(scope or {0})) + if library_id is not None: + assert_can_use(scope, library_id) + query = query.filter(MediaAsset.library_id == library_id) + if q and q.strip(): + ranked, _ = hybrid_ids(db, q.strip(), "media", limit=200) + if not ranked: + return {"total": 0, "images": []} + query = query.filter(MediaAsset.id.in_(ranked)) + + total = query.count() + assets = query.order_by(MediaAsset.id.desc()).offset(offset).limit(limit).all() + tags = _tags_for(db, [a.id for a in assets]) + return {"total": total, "images": [_asset_json(a, tags.get(a.id, [])) for a in assets]} + + +@router.post("/upload", status_code=201) +def upload_media( + file: UploadFile = File(...), + library_id: int | None = Form(None), + title: str | None = Form(None), + caption: str | None = Form(None), + alt_text: str | None = Form(None), + db: Session = Depends(get_db), + current_user: User = Depends(get_current_user), +): + """Store an image and index it so it can be found by what it shows.""" + scope = readable_libraries(db, current_user) + if library_id is not None: + assert_can_use(scope, library_id) + elif scope is not None: + raise HTTPException(400, "Choose one of your libraries for this image") + + if file.content_type not in ALLOWED_TYPES: + raise HTTPException(400, "Upload a PNG, JPEG, GIF, WebP or SVG image") + data = file.file.read(MAX_IMAGE_BYTES + 1) + if len(data) > MAX_IMAGE_BYTES: + raise HTTPException(413, f"Keep images under {MAX_IMAGE_BYTES // (1024 * 1024)} MB") + if not data: + raise HTTPException(400, "That file is empty") + + suffix = (file.filename or "").rsplit(".", 1)[-1].lower()[:8] or "bin" + key = f"media/{uuid.uuid4().hex}.{suffix}" + storage_service.save(key, data, file.content_type) + + asset = MediaAsset( + path=key, title=title or file.filename, caption=caption, alt_text=alt_text, + kind="image", library_id=library_id, user_id=current_user.id, + storage="s3" if storage_service.using_s3() else "local", byte_size=len(data), + ) + db.add(asset) + db.commit() + db.refresh(asset) + + # Text today; a vision model can embed the image itself later. + try: + if embedding_service.embed_record(asset, "media"): + db.commit() + except Exception: + db.rollback() + log.warning("Could not embed media %s; the retry task will", asset.id, exc_info=True) + return _asset_json(asset, []) + + +class MediaUpdate(BaseModel): + title: str | None = None + caption: str | None = None + alt_text: str | None = None + library_id: int | None = None + category_id: int | None = None + tags: list[str] | None = None + + +@router.patch("/{media_id}") +def update_media(media_id: int, data: MediaUpdate, db: Session = Depends(get_db), + current_user: User = Depends(get_current_user)): + """Edit an image's description and tags, and move it between libraries.""" + scope = readable_libraries(db, current_user) + asset = db.get(MediaAsset, media_id) + if not asset: + raise HTTPException(404, "Image not found") + assert_can_use(scope, asset.library_id) + + values = data.model_dump(exclude_unset=True) + tags = values.pop("tags", None) + if "library_id" in values: + assert_can_use(scope, values["library_id"]) + for field, value in values.items(): + setattr(asset, field, value) + + if tags is not None: + db.query(MediaTagLink).filter(MediaTagLink.media_id == asset.id).delete(synchronize_session=False) + for name in dict.fromkeys(t.strip() for t in tags if t.strip()): + # Reuse the shared vocabulary rather than inventing a media-only one. + row = db.execute(sa_text( + "SELECT id FROM question_tags WHERE lower(name) = lower(:n) ORDER BY id LIMIT 1" + ), {"n": name}).first() + tag_id = row[0] if row else db.execute(sa_text( + "INSERT INTO question_tags (name, type) VALUES (:n, 'keyword') RETURNING id" + ), {"n": name}).scalar() + db.add(MediaTagLink(media_id=asset.id, tag_id=tag_id)) + db.commit() + db.refresh(asset) + + try: + if embedding_service.embed_record(asset, "media"): + db.commit() + except Exception: + db.rollback() + return _asset_json(asset, _tags_for(db, [asset.id]).get(asset.id, [])) + + +@router.delete("/{media_id}", status_code=204) +def delete_media(media_id: int, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + asset = db.get(MediaAsset, media_id) + if not asset: + raise HTTPException(404, "Image not found") + storage_service.delete(asset.path) + db.query(MediaTagLink).filter(MediaTagLink.media_id == asset.id).delete(synchronize_session=False) + db.delete(asset) + db.commit() diff --git a/backend/app/services/storage_service.py b/backend/app/services/storage_service.py new file mode 100644 index 0000000..81087a2 --- /dev/null +++ b/backend/app/services/storage_service.py @@ -0,0 +1,148 @@ +"""Where uploaded files live. + +Two backends behind one interface: + + * ``local`` — the container volume, which is where every existing upload + already sits; + * ``s3`` — MinIO (or any S3-compatible service). + +Object storage is the right home for media: a container volume can only be +mounted by one host, has no presigned URLs, and no lifecycle rules. But the +existing 800-odd MB of PDFs live on the volume, so reads fall back to local when +an object is missing. That keeps the switch reversible and lets files migrate +gradually instead of in one risky pass. + +A stored path is always the key, never a URL: URLs embed the backend, and a row +that hardcodes ``http://minio:9000/...`` breaks the moment the backend changes. +""" +import logging +import mimetypes +import os +from pathlib import Path + +from app.config import settings + +logger = logging.getLogger(__name__) + +_client = None + + +def using_s3() -> bool: + return settings.STORAGE_BACKEND == "s3" + + +def _s3(): + """A cached boto3 client pointed at MinIO.""" + global _client + if _client is None: + import boto3 + from botocore.config import Config + + _client = boto3.client( + "s3", + endpoint_url=settings.S3_ENDPOINT_URL, + aws_access_key_id=settings.S3_ACCESS_KEY, + aws_secret_access_key=settings.S3_SECRET_KEY, + region_name=settings.S3_REGION, + # MinIO needs path style; virtual-host style expects DNS per bucket. + config=Config(signature_version="s3v4", s3={"addressing_style": "path"}), + ) + return _client + + +def ensure_bucket() -> None: + """Create the bucket if it is not there. Safe to call repeatedly.""" + if not using_s3(): + return + client = _s3() + try: + client.head_bucket(Bucket=settings.S3_BUCKET) + except Exception: + try: + client.create_bucket(Bucket=settings.S3_BUCKET) + logger.info("Created bucket %s", settings.S3_BUCKET) + except Exception: + logger.warning("Could not create bucket %s", settings.S3_BUCKET, exc_info=True) + + +def local_path(key: str) -> Path: + return Path(settings.UPLOAD_DIR) / key + + +def save(key: str, data: bytes, content_type: str | None = None) -> str: + """Store bytes under `key` and return the key.""" + if using_s3(): + ensure_bucket() + _s3().put_object( + Bucket=settings.S3_BUCKET, Key=key, Body=data, + ContentType=content_type or mimetypes.guess_type(key)[0] or "application/octet-stream", + ) + return key + target = local_path(key) + target.parent.mkdir(parents=True, exist_ok=True) + target.write_bytes(data) + return key + + +def load(key: str) -> bytes | None: + """Read an object, falling back to the local volume for files not yet migrated.""" + if using_s3(): + try: + return _s3().get_object(Bucket=settings.S3_BUCKET, Key=key)["Body"].read() + except Exception: + logger.debug("Object %s not in S3; trying the local volume", key) + target = local_path(key) + return target.read_bytes() if target.is_file() else None + + +def exists(key: str) -> bool: + if using_s3(): + try: + _s3().head_object(Bucket=settings.S3_BUCKET, Key=key) + return True + except Exception: + pass + return local_path(key).is_file() + + +def delete(key: str) -> None: + if using_s3(): + try: + _s3().delete_object(Bucket=settings.S3_BUCKET, Key=key) + except Exception: + logger.warning("Could not delete %s from S3", key, exc_info=True) + target = local_path(key) + if target.is_file(): + try: + target.unlink() + except OSError: + logger.warning("Could not delete %s from disk", key, exc_info=True) + + +def presigned_url(key: str, expires: int = 3600) -> str | None: + """A time-limited direct URL, so large media need not stream through the API. + + None when the object is not in S3, in which case the caller serves it from + the volume as before. + """ + if not using_s3(): + return None + try: + _s3().head_object(Bucket=settings.S3_BUCKET, Key=key) + return _s3().generate_presigned_url( + "get_object", + Params={"Bucket": settings.S3_BUCKET, "Key": key}, + ExpiresIn=expires, + ) + except Exception: + return None + + +def size_of(key: str) -> int | None: + if using_s3(): + try: + return _s3().head_object(Bucket=settings.S3_BUCKET, Key=key)["ContentLength"] + except Exception: + pass + target = local_path(key) + return target.stat().st_size if target.is_file() else None diff --git a/backend/scripts/classify_unfiled_questions.py b/backend/scripts/classify_unfiled_questions.py index cf79ad0..a347833 100644 --- a/backend/scripts/classify_unfiled_questions.py +++ b/backend/scripts/classify_unfiled_questions.py @@ -20,9 +20,15 @@ has. A question whose best candidate is weak is left alone rather than forced, and the old system is kept as a cross-link so no existing filter narrows. +`--force` files the leftovers on the nearest candidate even when the chooser +declines, so nothing is left sitting on a bare system. It is a weaker signal than +a chosen match, so those questions are marked for review — but the system is +still kept as a cross-link, so a wrong pick narrows nothing. + docker compose exec backend python -m scripts.classify_unfiled_questions docker compose exec backend python -m scripts.classify_unfiled_questions --limit 50 docker compose exec backend python -m scripts.classify_unfiled_questions --apply + docker compose exec backend python -m scripts.classify_unfiled_questions --apply --force """ import json import sys @@ -101,6 +107,7 @@ def choose(question_text, candidates): def main(): apply_changes = "--apply" in sys.argv + force = "--force" in sys.argv limit = None if "--limit" in sys.argv: limit = int(sys.argv[sys.argv.index("--limit") + 1]) @@ -129,7 +136,7 @@ def main(): vectors[cid] = embedding print(f" categories embedded: {len(vectors)}", flush=True) - filed, skipped, no_candidates = [], [], 0 + filed, skipped, forced, no_candidates = [], [], [], 0 for index, (qid, text, system_id) in enumerate(pending, start=1): row = db.execute(sa_text( "SELECT embedding FROM questions WHERE id = :q"), {"q": qid}).scalar() @@ -142,7 +149,10 @@ def main(): ((cid, paths[cid], cosine(question_vector, vec)) for cid, vec in vectors.items()), key=lambda item: -item[2], )[:SHORTLIST] - if not ranked or ranked[0][2] < MIN_SIMILARITY: + if not ranked: + no_candidates += 1 + continue + if ranked[0][2] < MIN_SIMILARITY and not force: no_candidates += 1 continue @@ -152,8 +162,12 @@ def main(): print(f" chooser failed on question {qid}: {error}") continue if picked is None or picked in systems: - skipped.append((qid, ranked[0][1])) - continue + if not force: + skipped.append((qid, ranked[0][1])) + continue + # Nearest neighbour, on the same shortlist the chooser saw. + picked = ranked[0][0] + forced.append(qid) filed.append((qid, picked, paths[picked], system_id)) if apply_changes: @@ -175,6 +189,7 @@ def main(): print(f"\n{'APPLIED' if apply_changes else 'DRY RUN'}") print(f" filed : {len(filed)}") + print(f" of those forced: {len(forced)}") print(f" chooser said no: {len(skipped)}") print(f" no candidate : {no_candidates}\n") print(" Sample of what was chosen:") diff --git a/backend/tests/test_media_library.py b/backend/tests/test_media_library.py new file mode 100644 index 0000000..d2274b8 --- /dev/null +++ b/backend/tests/test_media_library.py @@ -0,0 +1,149 @@ +"""Image libraries: access is per library, and storage is behind one interface. + +Run: DATABASE_URL=sqlite:///:memory: PYTHONPATH=backend python -m unittest discover -s backend/tests +""" +import os +os.environ["DATABASE_URL"] = "sqlite:///:memory:" + +import io +import unittest +from unittest.mock import patch + +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlalchemy import create_engine, text +from sqlalchemy.orm import Session +from sqlalchemy.pool import StaticPool + +from app.database import Base, get_db +from app.models.course import Course # noqa — Quiz.course_id FK needs the table in metadata. +from app.models.media import MediaAsset, MediaLibrary, MediaLibraryGrant +from app.models.user import User +from app.routers import media +from app.utils.auth import get_current_user + +TAG_DDL = """ +CREATE TABLE question_tags (id INTEGER PRIMARY KEY, name VARCHAR(200), type VARCHAR(50), exam_id INTEGER, created_at TIMESTAMP); +""" + + +class MediaLibraryTests(unittest.TestCase): + def setUp(self): + self.engine = create_engine("sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool) + Base.metadata.create_all(self.engine) + self.db = Session(self.engine) + self.db.execute(text(TAG_DDL.strip())) + + self.mod = User(id=1, name="Mod", email="mod@example.test", hashed_password="unused", role="moderator") + self.educator = User(id=2, name="Edu", email="edu@example.test", hashed_password="unused") + self.outsider = User(id=3, name="Out", email="out@example.test", hashed_password="unused") + self.db.add_all([self.mod, self.educator, self.outsider]) + self.db.add_all([ + MediaLibrary(id=1, name="Cardiology imaging"), + MediaLibrary(id=2, name="Dermatology photos"), + ]) + self.db.flush() + self.db.add_all([ + MediaAsset(id=1, path="media/a.png", title="ECG", library_id=1, kind="image"), + MediaAsset(id=2, path="media/b.png", title="Rash", library_id=2, kind="image"), + ]) + self.db.add(MediaLibraryGrant(library_id=1, user_id=2, granted_by=1)) + self.db.commit() + + self.user = self.mod + app = FastAPI() + app.include_router(media.router, prefix="/media") + app.dependency_overrides[get_db] = lambda: self.db + app.dependency_overrides[get_current_user] = lambda: self.user + self.client = TestClient(app) + + def tearDown(self): + self.client.close() + self.db.close() + self.engine.dispose() + + def test_a_moderator_sees_every_library(self): + names = {lib["name"] for lib in self.client.get("/media/libraries").json()} + self.assertEqual(names, {"Cardiology imaging", "Dermatology photos"}) + + def test_a_grant_limits_which_libraries_and_images_are_visible(self): + self.user = self.educator + libraries = self.client.get("/media/libraries").json() + self.assertEqual([lib["name"] for lib in libraries], ["Cardiology imaging"]) + + images = self.client.get("/media/").json() + self.assertEqual([i["title"] for i in images["images"]], ["ECG"]) + + # And an ungranted library cannot be browsed by asking for it directly. + self.assertEqual(self.client.get("/media/", params={"library_id": 2}).status_code, 403) + + def test_someone_with_no_grant_sees_nothing(self): + self.user = self.outsider + self.assertEqual(self.client.get("/media/libraries").json(), []) + self.assertEqual(self.client.get("/media/").json()["total"], 0) + + def test_editing_is_confined_to_granted_libraries(self): + self.user = self.educator + self.assertEqual(self.client.patch("/media/1", json={"caption": "Sinus rhythm"}).status_code, 200) + self.assertEqual(self.db.get(MediaAsset, 1).caption, "Sinus rhythm") + # Image 2 is in a library they were not granted. + self.assertEqual(self.client.patch("/media/2", json={"caption": "No"}).status_code, 403) + # Nor can an image be moved into one. + self.assertEqual(self.client.patch("/media/1", json={"library_id": 2}).status_code, 403) + + def test_tags_reuse_the_shared_vocabulary(self): + with patch.object(media.embedding_service, "embed_record", return_value=False): + response = self.client.patch("/media/1", json={"tags": ["Arrhythmia", "ECG"]}) + self.assertEqual(response.status_code, 200, response.text) + self.assertEqual(sorted(response.json()["tags"]), ["Arrhythmia", "ECG"]) + # A tag that already exists is reused rather than duplicated. + existing = self.db.execute(text("SELECT COUNT(*) FROM question_tags")).scalar() + with patch.object(media.embedding_service, "embed_record", return_value=False): + self.client.patch("/media/2", json={"tags": ["ECG"]}) + self.assertEqual(self.db.execute(text("SELECT COUNT(*) FROM question_tags")).scalar(), existing) + + def test_upload_stores_through_the_storage_service(self): + saved = {} + + def fake_save(key, data, content_type=None): + saved["key"], saved["bytes"] = key, len(data) + return key + + with patch.object(media.storage_service, "save", side_effect=fake_save), \ + patch.object(media.storage_service, "using_s3", return_value=True), \ + patch.object(media.embedding_service, "embed_record", return_value=False): + response = self.client.post( + "/media/upload", + files={"file": ("scan.png", io.BytesIO(b"x" * 100), "image/png")}, + data={"library_id": "1"}, + ) + self.assertEqual(response.status_code, 201, response.text) + body = response.json() + # The row stores the key, never a URL, so the backend can change later. + self.assertTrue(body["path"].startswith("media/")) + self.assertNotIn("http", body["path"]) + self.assertEqual(body["storage"], "s3") + self.assertEqual(saved["bytes"], 100) + + def test_upload_rejects_the_wrong_type_and_oversized_files(self): + with patch.object(media.storage_service, "save", return_value="k"): + bad_type = self.client.post( + "/media/upload", files={"file": ("a.exe", io.BytesIO(b"x"), "application/x-msdownload")}, + data={"library_id": "1"}) + oversized = self.client.post( + "/media/upload", + files={"file": ("a.png", io.BytesIO(b"x" * (media.MAX_IMAGE_BYTES + 5)), "image/png")}, + data={"library_id": "1"}) + self.assertEqual(bad_type.status_code, 400) + self.assertEqual(oversized.status_code, 413) + + def test_granting_and_revoking_a_library_is_moderator_only(self): + self.assertEqual(self.client.post("/media/libraries/2/grants", json={"user_id": 3}).status_code, 201) + self.assertEqual(self.client.post("/media/libraries/2/grants", json={"user_id": 3}).status_code, 409) + self.assertEqual(self.client.delete("/media/libraries/2/grants/3").status_code, 204) + self.user = self.educator + self.assertEqual(self.client.post("/media/libraries/2/grants", json={"user_id": 3}).status_code, 403) + + +if __name__ == "__main__": + unittest.main() diff --git a/docker-compose.yml b/docker-compose.yml index 220e132..b7a462f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -138,8 +138,30 @@ services: postgres: condition: service_healthy + # Object storage for media. Files this size do not belong in a container + # volume that only one host can mount, and S3 semantics give presigned URLs + # and lifecycle rules that a bind mount cannot. + minio: + image: minio/minio:RELEASE.2024-10-13T13-34-11Z + command: server /data --console-address ":9001" + environment: + MINIO_ROOT_USER: ${MINIO_ROOT_USER:-pedshub} + MINIO_ROOT_PASSWORD: ${MINIO_ROOT_PASSWORD:?set MINIO_ROOT_PASSWORD} + volumes: + - minio_data:/data + # No host ports: the backend reaches MinIO over the compose network, and + # 9000/9001 are already taken on this host. Publish deliberately if the + # console is ever needed from outside. + healthcheck: + test: ["CMD", "mc", "ready", "local"] + interval: 20s + timeout: 5s + retries: 5 + restart: unless-stopped + volumes: uploads_data: + minio_data: chroma_data: postgres_data: redis_data: diff --git a/docs/TODO.md b/docs/TODO.md index aaccf2b..ea45702 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -42,13 +42,12 @@ Updated 2026-09-10. - [ ] **Study plan blocks carry articles**, not only questions: "Articles" with *Mark as read*, then "Sessions" with Study/Exam mode. - [ ] **Admin settings page revamp** — currently ugly; needs restructuring. -- [ ] **Image libraries** — group images into libraries; grant a person access to - one, several, or all. Same shape as the existing per-category question - grants. +- [x] **Image libraries** — done 2026-09-10. Libraries, per-library grants, tags + on the shared vocabulary, and MinIO behind a storage service. +- [ ] **Media management page (frontend)** — the API exists; the browse/edit + screen and the picker shown when attaching an image to a question do not. - [ ] **Question folders** — collect questions into folders for assignment and access, alongside category grants. -- [ ] **Media management page** — browse the image bank, show each image's id on - hover, edit caption/alt/tags, attach to a question. ## Article reading @@ -99,6 +98,8 @@ Updated 2026-09-10. - [x] **Unfiled questions classified** — done 2026-09-10. 1,170 of 1,486 filed by retrieval + a chooser constrained to the shortlist. 316 remain on a bare system: 301 the chooser declined and 15 with no viable candidate. +- [x] **Every question filed below its system** — done 2026-09-10. The last 316 + were forced onto their nearest candidate; 0 remain on a bare system. - [ ] **Review the classifier's work** — roughly 3 in 4 were right on a spot check, so expect some wrong. The original system is kept as a cross-link, so a wrong pick never loses a question. Worth an editorial pass.