From fa2c7d7525188153e4d869cb2998e9bd780cda68 Mon Sep 17 00:00:00 2001 From: Pier-Jean Malandrino Date: Thu, 30 Apr 2026 15:49:25 +0200 Subject: [PATCH] =?UTF-8?q?fix(analyses):=20filter=20list=20endpoint=20by?= =?UTF-8?q?=20documentId=20=E2=80=94=20prevent=20cross-doc=20bbox=20leak?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- document-parser/api/analyses.py | 14 ++++++++++---- document-parser/persistence/analysis_repo.py | 13 +++++++++++++ document-parser/services/analysis_service.py | 4 ++++ document-parser/tests/test_api_endpoints.py | 16 ++++++++++++++++ frontend/src/pages/DocAskTab.vue | 5 ++++- frontend/src/pages/DocInspectTab.vue | 5 ++++- 6 files changed, 51 insertions(+), 6 deletions(-) diff --git a/document-parser/api/analyses.py b/document-parser/api/analyses.py index 5ebddae..0f9328d 100644 --- a/document-parser/api/analyses.py +++ b/document-parser/api/analyses.py @@ -5,7 +5,7 @@ from __future__ import annotations import logging from typing import Annotated -from fastapi import APIRouter, Depends, HTTPException, Request +from fastapi import APIRouter, Depends, HTTPException, Query, Request from api.schemas import ( AnalysisResponse, @@ -75,9 +75,15 @@ async def create_analysis(body: CreateAnalysisRequest, service: ServiceDep) -> A @router.get("", response_model=list[AnalysisResponse]) -async def list_analyses(service: ServiceDep) -> list[AnalysisResponse]: - """List all analysis jobs.""" - jobs = await service.find_all() +async def list_analyses( + service: ServiceDep, + 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] diff --git a/document-parser/persistence/analysis_repo.py b/document-parser/persistence/analysis_repo.py index 7d0d106..7b9260e 100644 --- a/document-parser/persistence/analysis_repo.py +++ b/document-parser/persistence/analysis_repo.py @@ -66,6 +66,19 @@ class SqliteAnalysisRepository: rows = await cursor.fetchall() 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: """Find an analysis job by ID (with document filename), or return None.""" async with get_connection() as db: diff --git a/document-parser/services/analysis_service.py b/document-parser/services/analysis_service.py index ac90ed7..f8ea2ae 100644 --- a/document-parser/services/analysis_service.py +++ b/document-parser/services/analysis_service.py @@ -149,6 +149,10 @@ class AnalysisService: """Return all analysis jobs, newest first.""" 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: """Find an analysis job by ID, or return None.""" return await self._analysis_repo.find_by_id(job_id) diff --git a/document-parser/tests/test_api_endpoints.py b/document-parser/tests/test_api_endpoints.py index 29e2d6a..1b43cf4 100644 --- a/document-parser/tests/test_api_endpoints.py +++ b/document-parser/tests/test_api_endpoints.py @@ -199,6 +199,22 @@ class TestAnalysisEndpoints: assert data[0]["documentFilename"] == "test.pdf" 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): job = AnalysisJob(id="j1", document_id="d1", document_filename="test.pdf") job.mark_running() diff --git a/frontend/src/pages/DocAskTab.vue b/frontend/src/pages/DocAskTab.vue index b298840..173ab45 100644 --- a/frontend/src/pages/DocAskTab.vue +++ b/frontend/src/pages/DocAskTab.vue @@ -79,7 +79,10 @@ async function loadAnalysis(): Promise { try { const analyses = await fetchDocumentAnalyses(requestedId) 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) { if (requestedId !== props.docId) return error.value = (e as Error).message || 'Failed to load analysis' diff --git a/frontend/src/pages/DocInspectTab.vue b/frontend/src/pages/DocInspectTab.vue index a1a693e..112b42d 100644 --- a/frontend/src/pages/DocInspectTab.vue +++ b/frontend/src/pages/DocInspectTab.vue @@ -50,7 +50,10 @@ async function load(): Promise { try { const analyses = await fetchDocumentAnalyses(requestedId) 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) { if (requestedId !== props.docId) return error.value = (e as Error).message || 'Failed to load analysis'