feat: MinIO-backed media libraries, and file the last 316 questions
Storage Media now goes through `storage_service`, which has two backends: the container volume, and S3/MinIO. A volume can only be mounted by one host, has no presigned URLs and no lifecycle rules, none of which suits ~860 MB of media. Reads fall back to the volume when an object is missing, so the existing uploads keep working and files can migrate gradually rather than in one risky pass. A row stores the object key, never a URL: a URL embeds the backend, so a row holding `http://minio:9000/...` breaks the moment the backend changes. MinIO publishes no host ports — the backend reaches it over the compose network, and 9000/9001 are already taken on this host by other stacks. Image libraries (migration d2e3f4a5b6c7) An image belongs to a library, and 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. Tags reuse the shared `question_tags` vocabulary rather than inventing a media-only one. Uploads are type- and size-checked, stored through the service, and embedded so an image can be found by what it shows. Classification finished The 316 questions the chooser had declined are now filed with `--force`, which takes the nearest candidate from the same shortlist the chooser saw. 306 were forced, 10 the chooser accepted on this pass. No question sits on a bare system any more: system only 2,730 -> 0 condition/subsystem 214 -> 1,782 full depth 4 -> 1,166 A forced match is a weaker signal than a chosen one, so expect more errors among those 306 — but the original system stays as a cross-link, so nothing is lost and they can be corrected by hand. Tests: 8 new backend covering library scoping, edit confinement, shared-vocabulary tags, storage indirection on upload, and type/size limits. 131 backend green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WgRcMaScVEL7TBLpnAoSV9
This commit is contained in:
parent
3b1ac9aea8
commit
db2df87fc6
10 changed files with 708 additions and 10 deletions
55
backend/alembic/versions/d2e3f4a5b6c7_media_libraries.py
Normal file
55
backend/alembic/versions/d2e3f4a5b6c7_media_libraries.py
Normal file
|
|
@ -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")
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"])
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
270
backend/app/routers/media.py
Normal file
270
backend/app/routers/media.py
Normal file
|
|
@ -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()
|
||||
148
backend/app/services/storage_service.py
Normal file
148
backend/app/services/storage_service.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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:")
|
||||
|
|
|
|||
149
backend/tests/test_media_library.py
Normal file
149
backend/tests/test_media_library.py
Normal file
|
|
@ -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()
|
||||
|
|
@ -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:
|
||||
|
|
|
|||
11
docs/TODO.md
11
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.
|
||||
|
|
|
|||
Loading…
Reference in a new issue