From d049b1bad24b220db449e91e862b428abece6139 Mon Sep 17 00:00:00 2001 From: Stelios Voutsinas Date: Mon, 21 Oct 2024 16:24:15 -0700 Subject: [PATCH 1/2] Fix for GET with list params & test case --- ...241021_232057_steliosvoutsinas_DM_47027.md | 5 ++++ src/vocutouts/dependencies.py | 2 +- tests/conftest.py | 30 +++++++++++++++++-- tests/handlers/sync_test.py | 16 ++++++++++ 4 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 changelog.d/20241021_232057_steliosvoutsinas_DM_47027.md diff --git a/changelog.d/20241021_232057_steliosvoutsinas_DM_47027.md b/changelog.d/20241021_232057_steliosvoutsinas_DM_47027.md new file mode 100644 index 0000000..c506d56 --- /dev/null +++ b/changelog.d/20241021_232057_steliosvoutsinas_DM_47027.md @@ -0,0 +1,5 @@ + + +### Bug fixes + +- Change get_params_dependency to fetch params using multi_items method diff --git a/src/vocutouts/dependencies.py b/src/vocutouts/dependencies.py index 7370e75..64bd8eb 100644 --- a/src/vocutouts/dependencies.py +++ b/src/vocutouts/dependencies.py @@ -67,7 +67,7 @@ async def get_params_dependency( """Parse GET parameters into job parameters for a cutout.""" return [ UWSJobParameter(parameter_id=k, value=v) - for k, v in request.query_params.items() + for k, v in request.query_params.multi_items() if k in {"id", "pos", "circle", "polygon"} ] diff --git a/tests/conftest.py b/tests/conftest.py index e0d8a96..3379c23 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,25 +4,50 @@ from collections.abc import AsyncIterator, Iterator from datetime import timedelta +from typing import Annotated import pytest import pytest_asyncio import respx import structlog from asgi_lifespan import LifespanManager -from fastapi import FastAPI +from fastapi import APIRouter, Depends, FastAPI from httpx import ASGITransport, AsyncClient from safir.arq import MockArqQueue from safir.testing.gcs import MockStorageClient, patch_google_storage from safir.testing.slack import MockSlackWebhook, mock_slack_webhook from safir.testing.uws import MockUWSJobRunner +from safir.uws import UWSJobParameter from vocutouts import main from vocutouts.config import config, uws +from vocutouts.dependencies import get_params_dependency + + +@pytest.fixture +def get_params_router() -> APIRouter: + """Return a router that echoes the parameters passed to it.""" + router = APIRouter() + + @router.get("/params") + async def get_params( + params: Annotated[ + list[UWSJobParameter], Depends(get_params_dependency) + ], + ) -> dict[str, list[dict[str, str]]]: + return { + "params": [ + {"id": p.parameter_id, "value": p.value} for p in params + ] + } + + return router @pytest_asyncio.fixture -async def app(arq_queue: MockArqQueue) -> AsyncIterator[FastAPI]: +async def app( + arq_queue: MockArqQueue, get_params_router: APIRouter +) -> AsyncIterator[FastAPI]: """Return a configured test application. Initialize the database before creating the app to ensure that data is @@ -35,6 +60,7 @@ async def app(arq_queue: MockArqQueue) -> AsyncIterator[FastAPI]: # Otherwise, the web application will use the one created in its # lifespan context manager. uws.override_arq_queue(arq_queue) + main.app.include_router(get_params_router) yield main.app diff --git a/tests/handlers/sync_test.py b/tests/handlers/sync_test.py index c0ab2df..e507834 100644 --- a/tests/handlers/sync_test.py +++ b/tests/handlers/sync_test.py @@ -83,3 +83,19 @@ async def test_bad_parameters( # None of these requests should have been reported to Slack. assert mock_slack.messages == [] + + +@pytest.mark.asyncio +async def test_get_dependency_multiple_params(client: AsyncClient) -> None: + response = await client.get( + "/params?id=image1&id=image2&pos=" "RANGE 10 20 30 40&circle=10 20 5" + ) + assert response.status_code == 200 + assert response.json() == { + "params": [ + {"id": "id", "value": "image1"}, + {"id": "id", "value": "image2"}, + {"id": "pos", "value": "RANGE 10 20 30 40"}, + {"id": "circle", "value": "10 20 5"}, + ] + } From 0e291dbfa084f35dd25acb0d41ee41a00df32004 Mon Sep 17 00:00:00 2001 From: Stelios Voutsinas Date: Mon, 21 Oct 2024 18:51:45 -0700 Subject: [PATCH 2/2] Add a prefix to the test route used for testing the params --- tests/conftest.py | 2 +- tests/handlers/sync_test.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 3379c23..77bdc59 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -60,7 +60,7 @@ async def app( # Otherwise, the web application will use the one created in its # lifespan context manager. uws.override_arq_queue(arq_queue) - main.app.include_router(get_params_router) + main.app.include_router(get_params_router, prefix="/test") yield main.app diff --git a/tests/handlers/sync_test.py b/tests/handlers/sync_test.py index e507834..0d7dbdb 100644 --- a/tests/handlers/sync_test.py +++ b/tests/handlers/sync_test.py @@ -88,7 +88,8 @@ async def test_bad_parameters( @pytest.mark.asyncio async def test_get_dependency_multiple_params(client: AsyncClient) -> None: response = await client.get( - "/params?id=image1&id=image2&pos=" "RANGE 10 20 30 40&circle=10 20 5" + "/test/params?id=image1&id=image2&pos=" + "RANGE 10 20 30 40&circle=10 20 5" ) assert response.status_code == 200 assert response.json() == {