fix(analyses): filter list endpoint by documentId — prevent cross-doc bbox leak
GET /api/analyses?documentId=X retournait toutes les analyses (le query param était ignoré). Le frontend prenait alors la première COMPLETED de la réponse, qui pouvait être celle d'un autre doc → bboxes d'un autre doc projetées sur l'image en cours. Backend - analysis_repo: nouvelle méthode find_by_document(document_id, limit, offset) - analysis_service: expose find_by_document - api/analyses: GET /api/analyses accepte ?documentId=... (alias Pydantic pour respecter la règle ruff N803). Si présent, filtre via la nouvelle méthode; sinon comportement inchangé. - test: test_list_analyses_filtered_by_document vérifie le routing vers find_by_document quand le query param est fourni. Frontend (defensive) - DocInspectTab / DocAskTab: filtre client-side analyses.find(a => a.documentId === requestedId && a.status === 'COMPLETED') pour rester safe même si un backend antérieur ignore le param.
This commit is contained in:
parent
8ac81b9461
commit
aa1a195a61
6 changed files with 51 additions and 6 deletions
|
|
@ -5,7 +5,7 @@ from __future__ import annotations
|
||||||
import logging
|
import logging
|
||||||
from typing import Annotated
|
from typing import Annotated
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, Request
|
from fastapi import APIRouter, Depends, HTTPException, Query, Request
|
||||||
|
|
||||||
from api.schemas import (
|
from api.schemas import (
|
||||||
AnalysisResponse,
|
AnalysisResponse,
|
||||||
|
|
@ -75,9 +75,15 @@ async def create_analysis(body: CreateAnalysisRequest, service: ServiceDep) -> A
|
||||||
|
|
||||||
|
|
||||||
@router.get("", response_model=list[AnalysisResponse])
|
@router.get("", response_model=list[AnalysisResponse])
|
||||||
async def list_analyses(service: ServiceDep) -> list[AnalysisResponse]:
|
async def list_analyses(
|
||||||
"""List all analysis jobs."""
|
service: ServiceDep,
|
||||||
jobs = await service.find_all()
|
document_id: str | None = Query(default=None, alias="documentId"),
|
||||||
|
) -> list[AnalysisResponse]:
|
||||||
|
"""List analysis jobs, optionally filtered by documentId."""
|
||||||
|
if document_id:
|
||||||
|
jobs = await service.find_by_document(document_id)
|
||||||
|
else:
|
||||||
|
jobs = await service.find_all()
|
||||||
return [_to_response(j) for j in jobs]
|
return [_to_response(j) for j in jobs]
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -66,6 +66,19 @@ class SqliteAnalysisRepository:
|
||||||
rows = await cursor.fetchall()
|
rows = await cursor.fetchall()
|
||||||
return [_row_to_job(r) for r in rows]
|
return [_row_to_job(r) for r in rows]
|
||||||
|
|
||||||
|
async def find_by_document(
|
||||||
|
self, document_id: str, *, limit: int = 200, offset: int = 0
|
||||||
|
) -> list[AnalysisJob]:
|
||||||
|
"""Return analysis jobs for a given document, newest first."""
|
||||||
|
async with get_connection() as db:
|
||||||
|
cursor = await db.execute(
|
||||||
|
f"{_SELECT_WITH_DOC} WHERE aj.document_id = ? "
|
||||||
|
"ORDER BY aj.created_at DESC LIMIT ? OFFSET ?",
|
||||||
|
(document_id, limit, offset),
|
||||||
|
)
|
||||||
|
rows = await cursor.fetchall()
|
||||||
|
return [_row_to_job(r) for r in rows]
|
||||||
|
|
||||||
async def find_by_id(self, job_id: str) -> AnalysisJob | None:
|
async def find_by_id(self, job_id: str) -> AnalysisJob | None:
|
||||||
"""Find an analysis job by ID (with document filename), or return None."""
|
"""Find an analysis job by ID (with document filename), or return None."""
|
||||||
async with get_connection() as db:
|
async with get_connection() as db:
|
||||||
|
|
|
||||||
|
|
@ -149,6 +149,10 @@ class AnalysisService:
|
||||||
"""Return all analysis jobs, newest first."""
|
"""Return all analysis jobs, newest first."""
|
||||||
return await self._analysis_repo.find_all()
|
return await self._analysis_repo.find_all()
|
||||||
|
|
||||||
|
async def find_by_document(self, document_id: str) -> list[AnalysisJob]:
|
||||||
|
"""Return analysis jobs for a given document, newest first."""
|
||||||
|
return await self._analysis_repo.find_by_document(document_id)
|
||||||
|
|
||||||
async def find_by_id(self, job_id: str) -> AnalysisJob | None:
|
async def find_by_id(self, job_id: str) -> AnalysisJob | None:
|
||||||
"""Find an analysis job by ID, or return None."""
|
"""Find an analysis job by ID, or return None."""
|
||||||
return await self._analysis_repo.find_by_id(job_id)
|
return await self._analysis_repo.find_by_id(job_id)
|
||||||
|
|
|
||||||
|
|
@ -199,6 +199,22 @@ class TestAnalysisEndpoints:
|
||||||
assert data[0]["documentFilename"] == "test.pdf"
|
assert data[0]["documentFilename"] == "test.pdf"
|
||||||
assert data[0]["status"] == "PENDING"
|
assert data[0]["status"] == "PENDING"
|
||||||
|
|
||||||
|
def test_list_analyses_filtered_by_document(self, client, mock_analysis_service):
|
||||||
|
mock_analysis_service.find_all = AsyncMock(return_value=[])
|
||||||
|
mock_analysis_service.find_by_document = AsyncMock(
|
||||||
|
return_value=[
|
||||||
|
AnalysisJob(id="j2", document_id="d42", document_filename="paper.pdf"),
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
resp = client.get("/api/analyses?documentId=d42")
|
||||||
|
assert resp.status_code == 200
|
||||||
|
data = resp.json()
|
||||||
|
assert len(data) == 1
|
||||||
|
assert data[0]["documentId"] == "d42"
|
||||||
|
mock_analysis_service.find_by_document.assert_awaited_once_with("d42")
|
||||||
|
mock_analysis_service.find_all.assert_not_awaited()
|
||||||
|
|
||||||
def test_get_analysis(self, client, mock_analysis_service):
|
def test_get_analysis(self, client, mock_analysis_service):
|
||||||
job = AnalysisJob(id="j1", document_id="d1", document_filename="test.pdf")
|
job = AnalysisJob(id="j1", document_id="d1", document_filename="test.pdf")
|
||||||
job.mark_running()
|
job.mark_running()
|
||||||
|
|
|
||||||
|
|
@ -79,7 +79,10 @@ async function loadAnalysis(): Promise<void> {
|
||||||
try {
|
try {
|
||||||
const analyses = await fetchDocumentAnalyses(requestedId)
|
const analyses = await fetchDocumentAnalyses(requestedId)
|
||||||
if (requestedId !== props.docId) return
|
if (requestedId !== props.docId) return
|
||||||
analysis.value = analyses.find((a) => a.status === 'COMPLETED') ?? null
|
// Defensive: filter by documentId because the backend currently ignores
|
||||||
|
// the ?documentId= query param and returns all analyses.
|
||||||
|
analysis.value =
|
||||||
|
analyses.find((a) => a.documentId === requestedId && a.status === 'COMPLETED') ?? null
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
if (requestedId !== props.docId) return
|
if (requestedId !== props.docId) return
|
||||||
error.value = (e as Error).message || 'Failed to load analysis'
|
error.value = (e as Error).message || 'Failed to load analysis'
|
||||||
|
|
|
||||||
|
|
@ -50,7 +50,10 @@ async function load(): Promise<void> {
|
||||||
try {
|
try {
|
||||||
const analyses = await fetchDocumentAnalyses(requestedId)
|
const analyses = await fetchDocumentAnalyses(requestedId)
|
||||||
if (requestedId !== props.docId) return
|
if (requestedId !== props.docId) return
|
||||||
analysis.value = analyses.find((a) => a.status === 'COMPLETED') ?? null
|
// Defensive: filter by documentId because the backend currently ignores
|
||||||
|
// the ?documentId= query param and returns all analyses.
|
||||||
|
analysis.value =
|
||||||
|
analyses.find((a) => a.documentId === requestedId && a.status === 'COMPLETED') ?? null
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
if (requestedId !== props.docId) return
|
if (requestedId !== props.docId) return
|
||||||
error.value = (e as Error).message || 'Failed to load analysis'
|
error.value = (e as Error).message || 'Failed to load analysis'
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue