diff --git a/backend/app/redis_client.py b/backend/app/redis_client.py new file mode 100644 index 0000000..589db3e --- /dev/null +++ b/backend/app/redis_client.py @@ -0,0 +1,75 @@ +""" +Redis helper — returns a connected client or None when Redis is unavailable. + +Key namespace design (prevents cross-file / cross-user data leakage): + + ann:{pdf_id}:{page_number}:{user_id} + +Every segment is mandatory, so: + - User A opening file 1 never touches User A's data on file 2 + - User A opening file 1 never touches User B's data on file 1 + - Data for page 3 never affects page 7 + +TTL: 24 h — temp entry expires automatically if the user never returns. +On permanent save the entry is deleted immediately. +""" + +import logging +import os +from typing import Optional + +import redis as redis_lib + +logger = logging.getLogger(__name__) + +_client: Optional[redis_lib.Redis] = None +_warned = False # log the "unavailable" warning only once + + +def get_redis() -> Optional[redis_lib.Redis]: + """Return a live Redis client, or None if Redis is unreachable.""" + global _client, _warned + + if _client is not None: + try: + _client.ping() + return _client + except Exception: + _client = None # connection dropped — try to reconnect below + + url = os.getenv("REDIS_URL", "redis://localhost:6379/0") + try: + r: redis_lib.Redis = redis_lib.from_url( + url, + decode_responses=True, + socket_connect_timeout=2, + socket_timeout=2, + ) + r.ping() + _client = r + _warned = False + logger.info("Redis connected: %s", url) + return _client + except Exception as exc: + if not _warned: + logger.warning( + "Redis unavailable (%s) — temp annotation cache disabled. " + "Set REDIS_URL or start a local Redis instance to enable it.", + exc, + ) + _warned = True + return None + + +# Key helpers — centralised so there's one place to change the format. +ANN_TTL = 86_400 # 24 hours + + +def ann_temp_key(pdf_id: int, page_number: int, user_id: int) -> str: + """Fully-namespaced Redis key for one user's unsaved canvas on one PDF page.""" + return f"ann:{pdf_id}:{page_number}:{user_id}" + + +def ann_temp_page_pattern(pdf_id: int, page_number: int) -> str: + """Glob pattern to list all users' temp entries for a given page.""" + return f"ann:{pdf_id}:{page_number}:*" diff --git a/backend/app/routers/annotations.py b/backend/app/routers/annotations.py index ff010c7..fb17062 100644 --- a/backend/app/routers/annotations.py +++ b/backend/app/routers/annotations.py @@ -1,11 +1,13 @@ from datetime import datetime, timezone +import json -from fastapi import APIRouter, Depends, HTTPException +from fastapi import APIRouter, Depends, HTTPException, status as http_status from sqlalchemy.orm import Session from ..database import get_db from ..dependencies import get_current_user from ..models import Annotation, PDF, User +from ..redis_client import ANN_TTL, ann_temp_key, ann_temp_page_pattern, get_redis from ..schemas import AnnotationIn, AnnotationOut router = APIRouter(prefix="/annotations", tags=["annotations"]) @@ -82,7 +84,35 @@ def get_all_annotations( .all() ) - if not rows: + # Build user_id → canvas_data from DB (permanent saves) + # Key = pdf_id + page_number + user_id → completely isolated per file/page/user + user_data: dict[int, dict] = {} + for row in rows: + user_data[row.user_id] = dict(row.canvas_data) if row.canvas_data else {} + + # Overlay with Redis temp data — unsaved strokes written by path:created / clear. + # If a user has BOTH a DB record and a temp entry, Redis wins (more recent). + r = get_redis() + if r is not None: + try: + pattern = ann_temp_page_pattern(pdf_id, page_number) + temp_keys: list[str] = r.keys(pattern) + for key in temp_keys: + # key format: ann:{pdf_id}:{page_number}:{user_id} + try: + uid = int(key.split(":")[-1]) + except ValueError: + continue + raw = r.get(key) + if raw: + try: + user_data[uid] = json.loads(raw) + except Exception: + pass + except Exception: + pass # Redis hiccup — fall through with DB data only + + if not user_data: return AnnotationOut( id=0, pdf_id=pdf_id, @@ -91,28 +121,50 @@ def get_all_annotations( updated_at=datetime.now(timezone.utc), ) - # Merge all objects arrays; use canvas metadata (background etc.) from latest record - latest = max(rows, key=lambda r: r.updated_at) - base: dict = dict(latest.canvas_data) if latest.canvas_data else {} + # Canvas-level metadata (version, background…) comes from the latest DB record + latest = max(rows, key=lambda row: row.updated_at) if rows else None + base: dict = dict(latest.canvas_data) if latest and latest.canvas_data else {} + merged_objects: list = [] - for row in rows: - for obj in (row.canvas_data or {}).get("objects", []): - # Tag each object with its owner so the frontend can avoid re-saving - # other users' objects under the current user's record (prevents duplicates). + for uid, canvas in user_data.items(): + for obj in (canvas or {}).get("objects", []): obj_copy = dict(obj) - obj_copy["_owner_id"] = row.user_id + obj_copy["_owner_id"] = uid merged_objects.append(obj_copy) base["objects"] = merged_objects return AnnotationOut( - id=latest.id, + id=latest.id if latest else 0, pdf_id=pdf_id, page_number=page_number, canvas_data=base, - updated_at=latest.updated_at, + updated_at=latest.updated_at if latest else datetime.now(timezone.utc), ) +# ── PUT /api/annotations/{pdf_id}/{page_number}/temp ────────────────────────── +# Writes unsaved canvas state to Redis so every subsequent /all call includes it. +# Called automatically (debounced) by the frontend after each stroke. + +@router.put("/{pdf_id}/{page_number}/temp", status_code=http_status.HTTP_204_NO_CONTENT) +def upsert_temp_annotation( + pdf_id: int, + page_number: int, + body: AnnotationIn, + current_user: User = Depends(get_current_user), + db: Session = Depends(get_db), +): + _any_pdf_or_404(db, pdf_id) + r = get_redis() + if r is None: + return # Redis unavailable — silently skip + key = ann_temp_key(pdf_id, page_number, current_user.id) + try: + r.setex(key, ANN_TTL, json.dumps(body.canvas_data)) + except Exception: + pass # non-critical + + # ── PUT /api/annotations/{pdf_id}/{page_number} ─────────────────────────────── @router.put("/{pdf_id}/{page_number}", response_model=AnnotationOut) @@ -149,4 +201,13 @@ def upsert_annotation( db.commit() db.refresh(ann) + + # Data is now in DB — remove the Redis temp entry so /all doesn't double-count + r = get_redis() + if r is not None: + try: + r.delete(ann_temp_key(pdf_id, page_number, current_user.id)) + except Exception: + pass + return ann diff --git a/backend/app/routers/ws.py b/backend/app/routers/ws.py index 3048865..79b5f42 100644 --- a/backend/app/routers/ws.py +++ b/backend/app/routers/ws.py @@ -161,7 +161,7 @@ async def pdf_collaboration(websocket: WebSocket, pdf_id: int): continue # Inject sender identity and forward to all other clients - if msg_type in {"object_add", "object_remove", "clear", "cursor"}: + if msg_type in {"object_add", "object_remove", "clear", "cursor", "color_sync"}: msg.update(user_info) await room.broadcast(msg, exclude=websocket) diff --git a/backend/requirements.txt b/backend/requirements.txt index 35f9bce..4ac8e78 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -7,3 +7,4 @@ python-jose[cryptography]==3.3.0 pydantic[email]==2.9.2 python-multipart==0.0.12 alembic==1.14.1 +redis>=5.0 diff --git a/docker-compose.yml b/docker-compose.yml index 0fa17ca..0f92a57 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,6 +18,19 @@ services: networks: - lms_net + # ── Redis (temp annotation cache) ──────────────────────────────────────────── + redis: + image: redis:7-alpine + restart: unless-stopped + command: redis-server --save "" --appendonly no # in-memory only, no disk writes + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 5s + timeout: 3s + retries: 5 + networks: + - lms_net + # ── FastAPI backend ─────────────────────────────────────────────────────────── backend: image: ${BACKEND_IMAGE:-lms-backend:latest} @@ -28,12 +41,15 @@ services: depends_on: db: condition: service_healthy + redis: + condition: service_healthy environment: DATABASE_URL: postgresql+psycopg2://${POSTGRES_USER:-lms_user}:${POSTGRES_PASSWORD}@db:5432/${POSTGRES_DB:-lms_db} JWT_SECRET_KEY: ${JWT_SECRET_KEY} JWT_ALGORITHM: HS256 ACCESS_TOKEN_EXPIRE_MINUTES: ${ACCESS_TOKEN_EXPIRE_MINUTES:-60} UPLOAD_DIR: /uploads + REDIS_URL: redis://redis:6379/0 volumes: - pdf_uploads:/uploads expose: diff --git a/frontend/src/components/WorkbookViewer.tsx b/frontend/src/components/WorkbookViewer.tsx index b522aaa..9cc351a 100644 --- a/frontend/src/components/WorkbookViewer.tsx +++ b/frontend/src/components/WorkbookViewer.tsx @@ -1,10 +1,33 @@ "use client"; import { useCallback, useEffect, useRef, useState } from "react"; +import { createPortal } from "react-dom"; import { useRouter } from "next/navigation"; import { api } from "@/lib/api"; import { useCollaboration, type RemoteEvent } from "@/hooks/useCollaboration"; +// ───────────────────────────────────────────────────────────────────────────── +// Preset color palette (30 hues, 3 brightness levels × 10 hue families) +// Each color is visually distinct to avoid confusion between collaborators. +// ───────────────────────────────────────────────────────────────────────────── + +const PRESET_COLORS = [ + "#e63946", "#9d0208", "#ff6b6b", // Red + "#f4572a", "#a23b17", "#ff9472", // Orange-red + "#f7b731", "#a07800", "#ffe066", // Yellow + "#80b918", "#4a6c0a", "#b5e853", // Yellow-green + "#2dc653", "#0a7a2e", "#6ee08a", // Green + "#00b4d8", "#005f73", "#48cae4", // Cyan + "#4361ee", "#1a237e", "#7b9cff", // Blue + "#7209b7", "#3a0068", "#b56aff", // Purple + "#e040fb", "#880e62", "#f48fff", // Magenta + "#ff6392", "#a3003f", "#ffadc5", // Rose +] as const; + +function userIdToPresetColor(id: number): string { + return PRESET_COLORS[id % PRESET_COLORS.length]; +} + // ───────────────────────────────────────────────────────────────────────────── // Types // ───────────────────────────────────────────────────────────────────────────── @@ -122,6 +145,14 @@ export default function WorkbookViewer({ pdfId }: Props) { // Buffer for remote events that arrive while renderPageWithAnnotations is in progress const renderingRef = useRef(false); const pendingRemoteEvents = useRef([]); + // True once api.me() has resolved and pen color has been set from user id + const [userColorReady, setUserColorReady] = useState(false); + + // Color picker + const [colorPickerOpen, setColorPickerOpen] = useState(false); + const [colorPickerPos, setColorPickerPos] = useState<{ top: number; left: number } | null>(null); + const colorBtnRef = useRef(null); + const colorPickerRef = useRef(null); // Ref for auto-scrolling thumbnail panel const thumbRefs = useRef<(HTMLButtonElement | null)[]>([]); @@ -137,7 +168,12 @@ export default function WorkbookViewer({ pdfId }: Props) { // Collab: stable ref to send functions (set after hook initializes) // eslint-disable-next-line @typescript-eslint/no-explicit-any - const collabSendRef = useRef<{ objectAdd: any; objectRemove: any; clear: any } | null>(null); + const collabSendRef = useRef<{ objectAdd: any; objectRemove: any; clear: any; colorSync: (c: string) => void } | null>(null); + + // Temp-cache sync: debounce unsaved canvas state to Redis (1.5 s after last stroke) + const tempSyncTimerRef = useRef | null>(null); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const syncTempCanvasRef = useRef<(page: number) => void>(() => {}); // Keep currentPageRef in sync useEffect(() => { currentPageRef.current = currentPage; }, [currentPage]); @@ -149,6 +185,27 @@ export default function WorkbookViewer({ pdfId }: Props) { useEffect(() => { fitModeRef.current = fitMode; }, [fitMode]); useEffect(() => { scrollModeRef.current = scrollMode; }, [scrollMode]); + // Keep syncTempCanvasRef current so closures inside Fabric events always see latest pdfId + useEffect(() => { + syncTempCanvasRef.current = (page: number) => { + const fc = fabricRef.current; + if (!fc) return; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fullData: any = fc.toJSON(["_owner_id"]); + const uid = currentUserIdRef.current; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const ownObjects = ((fullData.objects as any[]) ?? []).filter( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (o: any) => o._owner_id == null || o._owner_id === uid + ); + const canvasData = { ...fullData, objects: ownObjects }; + if (tempSyncTimerRef.current) clearTimeout(tempSyncTimerRef.current); + tempSyncTimerRef.current = setTimeout(() => { + api.upsertTempAnnotation(pdfId, page, { canvas_data: canvasData }).catch(() => {}); + }, 1500); + }; + }, [pdfId]); + // Auto-scroll thumbnail sidebar to keep current page visible useEffect(() => { if (showThumbnails) { @@ -166,9 +223,32 @@ export default function WorkbookViewer({ pdfId }: Props) { // Fetch current user id — used to distinguish own from remote objects when merging /all useEffect(() => { - api.me().then((u) => { currentUserIdRef.current = u.id; }).catch(() => {}); + api.me().then((u) => { + currentUserIdRef.current = u.id; + // Assign deterministic color from preset palette based on user id + const assigned = userIdToPresetColor(u.id); + setPenColor(assigned); + penColorRef.current = assigned; + setUserColorReady(true); // unblocks the color-sync broadcast + }).catch(() => {}); }, []); + // Close color picker when clicking outside + useEffect(() => { + if (!colorPickerOpen) return; + const handler = (e: MouseEvent) => { + if ( + colorPickerRef.current && !colorPickerRef.current.contains(e.target as Node) && + colorBtnRef.current && !colorBtnRef.current.contains(e.target as Node) + ) { + setColorPickerOpen(false); + setColorPickerPos(null); + } + }; + document.addEventListener("mousedown", handler); + return () => document.removeEventListener("mousedown", handler); + }, [colorPickerOpen]); + // Handle incoming remote annotation events const onRemoteEvent = useCallback((event: RemoteEvent) => { const fabric = fabricNSRef.current; @@ -260,16 +340,49 @@ export default function WorkbookViewer({ pdfId }: Props) { const fc = fabricRef.current; if (fc) broadcastCanvas(fc, currentPageRef.current); } + // Also broadcast our current pen color so the new peer can disable our swatch + collabSendRef.current?.colorSync(penColorRef.current); }, []); // Collaboration hook - const { users: collabUsers, connected: collabConnected, sendObjectAdd, sendObjectRemove, sendClear } = + const { users: collabUsers, connected: collabConnected, peerColors, sendObjectAdd, sendObjectRemove, sendClear, sendColorSync } = useCollaboration({ pdfId, token: collabToken, onEvent: onRemoteEvent, onPeerJoined: handlePeerJoined }); // Keep send functions accessible in stable refs (used in Fabric event handlers) useEffect(() => { - collabSendRef.current = { objectAdd: sendObjectAdd, objectRemove: sendObjectRemove, clear: sendClear }; - }, [sendObjectAdd, sendObjectRemove, sendClear]); + collabSendRef.current = { objectAdd: sendObjectAdd, objectRemove: sendObjectRemove, clear: sendClear, colorSync: sendColorSync }; + }, [sendObjectAdd, sendObjectRemove, sendClear, sendColorSync]); + + // Broadcast pen color only when BOTH WS is connected AND user color has been fetched. + // This prevents both users broadcasting the same default "#e63946" before api.me() resolves. + // Also re-broadcasts whenever the user manually picks a new color. + useEffect(() => { + if (collabConnected && userColorReady) { + sendColorSync(penColor); + } + }, [collabConnected, userColorReady, penColor, sendColorSync]); + + // If our color collides with a peer's color, pick the first free preset color and rebroadcast. + useEffect(() => { + if (!collabConnected || !userColorReady) return; + const uid = currentUserIdRef.current; + if (uid == null) return; + + const taken = new Set( + Object.entries(peerColors) + .filter(([id]) => Number(id) !== uid) + .map(([, c]) => c.toLowerCase()) + ); + const current = penColor.toLowerCase(); + if (!taken.has(current)) return; + + const replacement = PRESET_COLORS.find(c => !taken.has(c.toLowerCase())) ?? penColor; + if (replacement.toLowerCase() === current) return; + + setPenColor(replacement); + penColorRef.current = replacement; + sendColorSync(replacement); + }, [collabConnected, userColorReady, peerColors, penColor, sendColorSync]); // Re-render current page when fit mode changes — moved below renderAllPages definition @@ -607,6 +720,14 @@ export default function WorkbookViewer({ pdfId }: Props) { const obj = (options.path as any).toJSON(["collab_id"]); collabSendRef.current?.objectAdd(obj, currentPageRef.current); } + // Sync unsaved canvas to Redis so late-joining users see this stroke + syncTempCanvasRef.current(currentPageRef.current); + }); + + // Object removed (eraser / undo) — sync temp cache + fc.on("object:removed", () => { + if (skipObjectTracking.current || skipRemoteRef.current) return; + syncTempCanvasRef.current(currentPageRef.current); }); // ── Init PDF.js ─────────────────────────────────────────────────── @@ -883,6 +1004,8 @@ export default function WorkbookViewer({ pdfId }: Props) { try { await api.upsertAnnotation(pdfId, currentPage, { canvas_data: canvasData }); + // Data is now in DB — cancel any pending debounce (backend deletes Redis key too) + if (tempSyncTimerRef.current) { clearTimeout(tempSyncTimerRef.current); tempSyncTimerRef.current = null; } // Cache the full display state (including remote objects) so the page looks correct on revisit localAnnotations.current[currentPage] = fullCanvasData; setSaveNotice(true); @@ -913,6 +1036,8 @@ export default function WorkbookViewer({ pdfId }: Props) { addedObjects.current = []; fc.renderAll(); collabSendRef.current?.clear(currentPageRef.current); + // Push empty canvas to Redis so other users see the clear immediately on next load + syncTempCanvasRef.current(currentPageRef.current); }, []); // ───────────────────────────────────────────────────────────────────────── @@ -1129,12 +1254,22 @@ export default function WorkbookViewer({ pdfId }: Props) { - setPenColor(e.target.value)} - title="Color" - className="w-7 h-7 rounded cursor-pointer border border-gray-300 p-0.5 bg-white flex-shrink-0" + {/* Color picker button */} +