From e8ef1a855190271c2560324bbc8f33a75aa02727 Mon Sep 17 00:00:00 2001 From: root Date: Mon, 17 Aug 2026 14:06:33 +0200 Subject: [PATCH] fix(api): return 204 when an analysis has no request details Distinguish a missing analysis (404) from an existing one without collected requests, instead of returning 200 with null. --- bases/ecoindex/backend/routers/ecoindex.py | 17 ++-- projects/ecoindex_api/openapi.json | 15 ++-- .../backend/test_ecoindex_requests.py | 90 +++++++++++++++++++ 3 files changed, 107 insertions(+), 15 deletions(-) create mode 100644 test/bases/ecoindex/backend/test_ecoindex_requests.py diff --git a/bases/ecoindex/backend/routers/ecoindex.py b/bases/ecoindex/backend/routers/ecoindex.py index 55ba83e..a8ebd52 100644 --- a/bases/ecoindex/backend/routers/ecoindex.py +++ b/bases/ecoindex/backend/routers/ecoindex.py @@ -132,12 +132,19 @@ async def get_ecoindex_analysis_by_id( @router.get( name="Get ecoindex analysis requests by id", path="/{id}/requests", - response_model=RequestsDetailResponse | None, + response_model=RequestsDetailResponse, response_description="Request details of the ecoindex analysis", - responses={status.HTTP_404_NOT_FOUND: example_ecoindex_not_found}, + responses={ + status.HTTP_204_NO_CONTENT: { + "description": ( + "Analysis exists but request details were not collected" + ) + }, + status.HTTP_404_NOT_FOUND: example_ecoindex_not_found, + }, description=( "This returns the detailed list of requests made by the page, " - "aggregated by category and by domain. Returns `null` when the " + "aggregated by category and by domain. Returns 204 when the " "analysis exists but request details were not collected." ), ) @@ -145,7 +152,7 @@ async def get_ecoindex_analysis_requests_by_id( id: IdParameter, version: VersionParameter = Version.v1, session: AsyncSession = Depends(get_session), -) -> RequestsDetailResponse | None: +) -> RequestsDetailResponse | Response: ecoindex = await get_ecoindex_result_by_id_db( session=session, id=id, version=version ) @@ -160,7 +167,7 @@ async def get_ecoindex_analysis_requests_by_id( session=session, analysis_id=id ) if not request_rows: - return None + return Response(status_code=status.HTTP_204_NO_CONTENT) return aggregate_request_details( [ diff --git a/projects/ecoindex_api/openapi.json b/projects/ecoindex_api/openapi.json index e3796e4..a5bce1d 100644 --- a/projects/ecoindex_api/openapi.json +++ b/projects/ecoindex_api/openapi.json @@ -2032,7 +2032,7 @@ }, "/{version}/ecoindexes/{id}/requests": { "get": { - "description": "This returns the detailed list of requests made by the page, aggregated by category and by domain. Returns `null` when the analysis exists but request details were not collected.", + "description": "This returns the detailed list of requests made by the page, aggregated by category and by domain. Returns 204 when the analysis exists but request details were not collected.", "operationId": "Get_ecoindex_analysis_requests_by_id__version__ecoindexes__id__requests_get", "parameters": [ { @@ -2069,20 +2069,15 @@ "content": { "application/json": { "schema": { - "anyOf": [ - { - "$ref": "#/components/schemas/RequestsDetailResponse" - }, - { - "type": "null" - } - ], - "title": "Response Get Ecoindex Analysis Requests By Id Version Ecoindexes Id Requests Get" + "$ref": "#/components/schemas/RequestsDetailResponse" } } }, "description": "Request details of the ecoindex analysis" }, + "204": { + "description": "Analysis exists but request details were not collected" + }, "404": { "content": { "application/json": { diff --git a/test/bases/ecoindex/backend/test_ecoindex_requests.py b/test/bases/ecoindex/backend/test_ecoindex_requests.py new file mode 100644 index 0000000..2a1f51a --- /dev/null +++ b/test/bases/ecoindex/backend/test_ecoindex_requests.py @@ -0,0 +1,90 @@ +from unittest.mock import AsyncMock +from uuid import uuid4 + +import pytest +from ecoindex.backend.routers.ecoindex import get_ecoindex_analysis_requests_by_id +from ecoindex.models.enums import Version +from ecoindex.models.scraper import RequestDetail, RequestsDetailResponse +from fastapi import HTTPException, Response, status + + +@pytest.mark.asyncio +async def test_get_requests_returns_204_when_analysis_exists_without_rows( + monkeypatch, +): + monkeypatch.setattr( + "ecoindex.backend.routers.ecoindex.get_ecoindex_result_by_id_db", + AsyncMock(return_value=object()), + ) + monkeypatch.setattr( + "ecoindex.backend.routers.ecoindex.get_requests_by_analysis_id_db", + AsyncMock(return_value=[]), + ) + + result = await get_ecoindex_analysis_requests_by_id( + id=uuid4(), + version=Version.v1, + session=AsyncMock(), + ) + + assert isinstance(result, Response) + assert result.status_code == status.HTTP_204_NO_CONTENT + assert not result.body + + +@pytest.mark.asyncio +async def test_get_requests_raises_404_when_analysis_is_missing(monkeypatch): + analysis_id = uuid4() + monkeypatch.setattr( + "ecoindex.backend.routers.ecoindex.get_ecoindex_result_by_id_db", + AsyncMock(return_value=None), + ) + get_requests = AsyncMock() + monkeypatch.setattr( + "ecoindex.backend.routers.ecoindex.get_requests_by_analysis_id_db", + get_requests, + ) + + with pytest.raises(HTTPException) as exc_info: + await get_ecoindex_analysis_requests_by_id( + id=analysis_id, + version=Version.v1, + session=AsyncMock(), + ) + + assert exc_info.value.status_code == status.HTTP_404_NOT_FOUND + assert str(analysis_id) in str(exc_info.value.detail) + get_requests.assert_not_called() + + +@pytest.mark.asyncio +async def test_get_requests_returns_payload_when_rows_exist(monkeypatch): + analysis_id = uuid4() + monkeypatch.setattr( + "ecoindex.backend.routers.ecoindex.get_ecoindex_result_by_id_db", + AsyncMock(return_value=object()), + ) + monkeypatch.setattr( + "ecoindex.backend.routers.ecoindex.get_requests_by_analysis_id_db", + AsyncMock( + return_value=[ + RequestDetail( + category="html", + domain="www.ecoindex.fr", + status=200, + url="https://www.ecoindex.fr/", + size=1000, + ) + ] + ), + ) + + result = await get_ecoindex_analysis_requests_by_id( + id=analysis_id, + version=Version.v1, + session=AsyncMock(), + ) + + assert isinstance(result, RequestsDetailResponse) + assert len(result.items) == 1 + assert result.items[0].domain == "www.ecoindex.fr"