From e1adee2d6a7939340851ea6219f88704ec265b31 Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Fri, 26 Jul 2024 00:10:03 -0400 Subject: [PATCH 01/10] add tests for routes with and without trailing slashes --- tests/test_attributes.py | 9 +++++++++ tests/test_query.py | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/tests/test_attributes.py b/tests/test_attributes.py index 3be37a2..98cbdfa 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -242,3 +242,12 @@ def mock_load_json(path): "namespace_prefix": expected_namespace_pfx, "term_labels": mock_term_labels, } + + +def test_request_with_trailing_slash_not_redirected(test_app): + """ + Test that given a request to a route with a trailing slash '/' where none is expected, + the request is not redirected and returns a 404 status code. + """ + response = test_app.get("/attributes/nb:SomeClass/") + assert response.status_code == 404 diff --git a/tests/test_query.py b/tests/test_query.py index 752b5a3..1cd781f 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -509,3 +509,24 @@ def test_query_without_token_succeeds_when_auth_disabled( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get("/query/") assert response.status_code == 200 + + +def test_request_without_trailing_slash_not_redirected( + test_app, monkeypatch, mock_successful_get, disable_auth +): + """Test that a request to the /query route is not redirected to have a trailing slash.""" + monkeypatch.setattr(crud, "get", mock_successful_get) + response = test_app.get("/query", follow_redirects=False) + assert response.status_code == 200 + + +def test_query_params_missing_leading_slash_succeeds( + test_app, monkeypatch, mock_successful_get, disable_auth +): + """ + Test that a request to the /query route with query parameters but missing a leading slash + is redirected to include the slash, and succeeds. + """ + monkeypatch.setattr(crud, "get", mock_successful_get) + response = test_app.get("/query?min_age=20") + assert response.status_code == 200 From ce6adfe9176a54db4cbceed6c250ae183e5fb047 Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Mon, 29 Jul 2024 11:10:04 -0400 Subject: [PATCH 02/10] disable redirect slashes and remove trailing slash from /query route --- app/api/routers/query.py | 2 +- app/main.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/api/routers/query.py b/app/api/routers/query.py index 539b27a..0fcb39d 100644 --- a/app/api/routers/query.py +++ b/app/api/routers/query.py @@ -23,7 +23,7 @@ ) -@router.get("/", response_model=List[CohortQueryResponse]) +@router.get("", response_model=List[CohortQueryResponse]) async def get_query( query: QueryModel = Depends(QueryModel), token: str | None = Depends(oauth2_scheme), diff --git a/app/main.py b/app/main.py index c4a88cd..5d0ceb3 100644 --- a/app/main.py +++ b/app/main.py @@ -16,7 +16,10 @@ from .api.security import check_client_id app = FastAPI( - default_response_class=ORJSONResponse, docs_url=None, redoc_url=None + default_response_class=ORJSONResponse, + docs_url=None, + redoc_url=None, + redirect_slashes=False, ) favicon_url = "https://raw.githubusercontent.com/neurobagel/documentation/main/docs/imgs/logo/neurobagel_favicon.png" From b6fa8904bedb53040df77a676e3b7b9dec085ac1 Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Mon, 29 Jul 2024 11:17:54 -0400 Subject: [PATCH 03/10] update routes in tests --- tests/test_app_events.py | 2 +- tests/test_attributes.py | 4 +-- tests/test_query.py | 61 ++++++++++++++++++++-------------------- 3 files changed, 33 insertions(+), 34 deletions(-) diff --git a/tests/test_app_events.py b/tests/test_app_events.py index eaaf001..943e798 100644 --- a/tests/test_app_events.py +++ b/tests/test_app_events.py @@ -39,7 +39,7 @@ def mock_httpx_post(**kwargs): return httpx.Response(status_code=401) monkeypatch.setattr(httpx, "post", mock_httpx_post) - response = test_app.get("/query/", headers=mock_auth_header) + response = test_app.get("/query", headers=mock_auth_header) assert response.status_code == 401 diff --git a/tests/test_attributes.py b/tests/test_attributes.py index 98cbdfa..c6ab639 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -246,8 +246,8 @@ def mock_load_json(path): def test_request_with_trailing_slash_not_redirected(test_app): """ - Test that given a request to a route with a trailing slash '/' where none is expected, - the request is not redirected and returns a 404 status code. + Test that a request to a route with a trailing slash '/', where none is expected, + is *not* redirected and returns a 404. """ response = test_app.get("/attributes/nb:SomeClass/") assert response.status_code == 404 diff --git a/tests/test_query.py b/tests/test_query.py index 1cd781f..e40405d 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -6,6 +6,8 @@ import app.api.utility as util from app.api import crud +ROUTE = "/query" + def test_get_subjects_by_query(monkeypatch): """Test that graph results for dataset size queries are correctly parsed into a dictionary.""" @@ -68,7 +70,7 @@ def test_null_modalities( crud, "query_matching_dataset_sizes", mock_query_matching_dataset_sizes ) - response = test_app.get("/query/", headers=mock_auth_header) + response = test_app.get(f"{ROUTE}", headers=mock_auth_header) assert response.json()[0]["image_modals"] == [ "http://purl.org/nidash/nidm#T1Weighted" ] @@ -84,7 +86,7 @@ def test_get_all( """Given no input for any query parameters, returns a 200 status code and a non-empty list of results (should correspond to all subjects in graph).""" monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get("/query/", headers=mock_auth_header) + response = test_app.get(f"{ROUTE}", headers=mock_auth_header) assert response.status_code == 200 assert response.json() != [] @@ -106,7 +108,7 @@ def test_get_valid_age_range( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?min_age={valid_min_age}&max_age={valid_max_age}", + f"{ROUTE}?min_age={valid_min_age}&max_age={valid_max_age}", headers=mock_auth_header, ) assert response.status_code == 200 @@ -128,7 +130,7 @@ def test_get_valid_age_single_bound( """Given only a valid lower/upper age bound, returns a 200 status code and a non-empty list of results.""" monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get(f"/query/?{age_keyval}", headers=mock_auth_header) + response = test_app.get(f"{ROUTE}?{age_keyval}", headers=mock_auth_header) assert response.status_code == 200 assert response.json() != [] @@ -155,7 +157,7 @@ def test_get_invalid_age( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?min_age={invalid_min_age}&max_age={invalid_max_age}", + f"{ROUTE}?min_age={invalid_min_age}&max_age={invalid_max_age}", headers=mock_auth_header, ) assert response.status_code == 422 @@ -177,7 +179,7 @@ def test_get_valid_sex( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?sex={valid_sex}", headers=mock_auth_header + f"{ROUTE}?sex={valid_sex}", headers=mock_auth_header ) assert response.status_code == 200 assert response.json() != [] @@ -190,7 +192,7 @@ def test_get_invalid_sex( """Given an invalid sex string, returns a 422 status code.""" monkeypatch.setattr(crud, "get", mock_get) - response = test_app.get("/query/?sex=apple", headers=mock_auth_header) + response = test_app.get(f"{ROUTE}?sex=apple", headers=mock_auth_header) assert response.status_code == 422 @@ -209,7 +211,7 @@ def test_get_valid_diagnosis( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?diagnosis={valid_diagnosis}", headers=mock_auth_header + f"{ROUTE}?diagnosis={valid_diagnosis}", headers=mock_auth_header ) assert response.status_code == 200 assert response.json() != [] @@ -231,7 +233,7 @@ def test_get_invalid_diagnosis( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?diagnosis={invalid_diagnosis}", headers=mock_auth_header + f"{ROUTE}?diagnosis={invalid_diagnosis}", headers=mock_auth_header ) assert response.status_code == 422 @@ -249,7 +251,7 @@ def test_get_valid_iscontrol( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?is_control={valid_iscontrol}", headers=mock_auth_header + f"{ROUTE}?is_control={valid_iscontrol}", headers=mock_auth_header ) assert response.status_code == 200 assert response.json() != [] @@ -263,7 +265,7 @@ def test_get_invalid_iscontrol( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - "/query/?is_control=apple", headers=mock_auth_header + f"{ROUTE}?is_control=apple", headers=mock_auth_header ) assert response.status_code == 422 @@ -276,7 +278,7 @@ def test_get_invalid_control_diagnosis_pair( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - "/query/?diagnosis=snomed:35489007&is_control=True", + f"{ROUTE}?diagnosis=snomed:35489007&is_control=True", headers=mock_auth_header, ) assert response.status_code == 422 @@ -305,7 +307,7 @@ def test_get_valid_min_num_sessions( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?{session_param}={valid_min_num_sessions}", + f"{ROUTE}?{session_param}={valid_min_num_sessions}", headers=mock_auth_header, ) assert response.status_code == 200 @@ -331,7 +333,7 @@ def test_get_invalid_min_num_sessions( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?{session_param}={invalid_min_num_sessions}", + f"{ROUTE}?{session_param}={invalid_min_num_sessions}", headers=mock_auth_header, ) response.status_code = 422 @@ -348,7 +350,7 @@ def test_get_valid_assessment( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - "/query/?assessment=nb:cogAtlas-1234", headers=mock_auth_header + f"{ROUTE}?assessment=nb:cogAtlas-1234", headers=mock_auth_header ) assert response.status_code == 200 assert response.json() != [] @@ -370,7 +372,7 @@ def test_get_invalid_assessment( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?assessment={invalid_assessment}", headers=mock_auth_header + f"{ROUTE}?assessment={invalid_assessment}", headers=mock_auth_header ) assert response.status_code == 422 @@ -397,7 +399,7 @@ def test_get_valid_available_image_modal( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get( - f"/query/?image_modal={valid_available_image_modal}", + f"{ROUTE}?image_modal={valid_available_image_modal}", headers=mock_auth_header, ) assert response.status_code == 200 @@ -421,7 +423,7 @@ def test_get_valid_unavailable_image_modal( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?image_modal={valid_unavailable_image_modal}", + f"{ROUTE}?image_modal={valid_unavailable_image_modal}", headers=mock_auth_header, ) @@ -445,7 +447,7 @@ def test_get_invalid_image_modal( monkeypatch.setattr(crud, "get", mock_get) response = test_app.get( - f"/query/?image_modal={invalid_image_modal}", headers=mock_auth_header + f"{ROUTE}?image_modal={invalid_image_modal}", headers=mock_auth_header ) assert response.status_code == 422 @@ -469,7 +471,7 @@ def test_get_undefined_prefix_image_modal( monkeypatch.setattr(crud, "get", mock_get_with_exception) response = test_app.get( - f"/query/?image_modal={undefined_prefix_image_modal}", + f"{ROUTE}?image_modal={undefined_prefix_image_modal}", headers=mock_auth_header, ) assert response.status_code == 500 @@ -491,7 +493,7 @@ def test_aggregate_query_response_structure( crud, "query_matching_dataset_sizes", mock_query_matching_dataset_sizes ) - response = test_app.get("/query/", headers=mock_auth_header) + response = test_app.get(f"{ROUTE}", headers=mock_auth_header) assert all( dataset["subject_data"] == "protected" for dataset in response.json() ) @@ -507,7 +509,7 @@ def test_query_without_token_succeeds_when_auth_disabled( Test that when authentication is disabled, a request to the /query route without a token succeeds. """ monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get("/query/") + response = test_app.get(f"{ROUTE}") assert response.status_code == 200 @@ -516,17 +518,14 @@ def test_request_without_trailing_slash_not_redirected( ): """Test that a request to the /query route is not redirected to have a trailing slash.""" monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get("/query", follow_redirects=False) + response = test_app.get(f"{ROUTE}", follow_redirects=False) assert response.status_code == 200 -def test_query_params_missing_leading_slash_succeeds( - test_app, monkeypatch, mock_successful_get, disable_auth -): +def test_query_params_with_leading_slash_fails(test_app, disable_auth): """ - Test that a request to the /query route with query parameters but missing a leading slash - is redirected to include the slash, and succeeds. + Test that a request to the /query route with a slash before query parameters + is *not* redirected to exclude the slash, and fails with a 404. """ - monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get("/query?min_age=20") - assert response.status_code == 200 + response = test_app.get(f"{ROUTE}/?min_age=20") + assert response.status_code == 404 From d33c8c1bbed94b4e370decba7f85cc80a3648040 Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Mon, 29 Jul 2024 11:30:00 -0400 Subject: [PATCH 04/10] fix - explicitly enable auth in tests of request headers using fixture --- tests/conftest.py | 6 ++++++ tests/test_security.py | 9 ++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5d03c2d..dd92310 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,6 +11,12 @@ def test_app(): yield client +@pytest.fixture +def enable_auth(monkeypatch): + """Enable the authentication requirement for the API.""" + monkeypatch.setattr("app.api.security.AUTH_ENABLED", True) + + @pytest.fixture def disable_auth(monkeypatch): """ diff --git a/tests/test_security.py b/tests/test_security.py index 6eb5b32..0f6dbb5 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -6,14 +6,13 @@ @pytest.mark.filterwarnings("ignore:.*NB_API_ALLOWED_ORIGINS") def test_missing_client_id_raises_error_when_auth_enabled( - monkeypatch, test_app, set_test_credentials + monkeypatch, test_app, set_test_credentials, enable_auth ): """Test that a missing client ID raises an error on startup when authentication is enabled.""" # We're using what should be default values of CLIENT_ID and AUTH_ENABLED here # (if the corresponding environment variables are unset), # but we set the values explicitly here for clarity monkeypatch.setattr("app.api.security.CLIENT_ID", None) - monkeypatch.setattr("app.api.security.AUTH_ENABLED", True) with pytest.raises(ValueError) as exc_info: with test_app: @@ -52,12 +51,12 @@ def test_invalid_token_raises_error(invalid_token): [{}, {"Authorization": ""}, {"badheader": "badvalue"}], ) def test_query_with_malformed_auth_header_fails( - test_app, set_mock_verify_token, invalid_auth_header + test_app, set_mock_verify_token, enable_auth, invalid_auth_header ): - """Test that a request to the /query route with a missing or malformed authorization header, fails .""" + """Test that a request to the /query route with a missing or malformed authorization header, fails.""" response = test_app.get( - "/query/", + "/query", headers=invalid_auth_header, ) From 74d13a42878dcce2503a844cc1c8392dc3565fe2 Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Mon, 29 Jul 2024 12:02:25 -0400 Subject: [PATCH 05/10] trust X-Forwarded... proxy headers from all remote IPs --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 795bb51..f60fa33 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,4 +12,4 @@ COPY ./vocab /usr/src/vocab # NB_API_PORT, representing the port on which the API will be exposed, # is an environment variable that will always have a default value of 8000 when building the image # but can be overridden when running the container. -ENTRYPOINT uvicorn app.main:app --proxy-headers --host 0.0.0.0 --port ${NB_API_PORT:-8000} +ENTRYPOINT uvicorn app.main:app --proxy-headers --forwarded-allow-ips=* --host 0.0.0.0 --port ${NB_API_PORT:-8000} From 82e3975376a798261e8b09f45bda028a82a9c82a Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Mon, 29 Jul 2024 16:55:23 -0400 Subject: [PATCH 06/10] revert change to mocking client ID when auth is enabled --- tests/test_security.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/test_security.py b/tests/test_security.py index 949cb88..57cbe3e 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -51,9 +51,18 @@ def test_invalid_token_raises_error(invalid_token): [{}, {"Authorization": ""}, {"badheader": "badvalue"}], ) def test_query_with_malformed_auth_header_fails( - test_app, set_mock_verify_token, enable_auth, invalid_auth_header + test_app, + set_mock_verify_token, + enable_auth, + invalid_auth_header, + monkeypatch, ): - """Test that a request to the /query route with a missing or malformed authorization header, fails.""" + """ + Test that when authentication is enabled, a request to the /query route with a + missing or malformed authorization header fails. + """ + monkeypatch.setattr("app.api.security.CLIENT_ID", "foo.id") + response = test_app.get( "/query", headers=invalid_auth_header, From 2f9b208df59d32c5dd2f73ee730c86c299ad926b Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Mon, 29 Jul 2024 18:00:10 -0400 Subject: [PATCH 07/10] add comment explaining empty string path prefix --- app/api/routers/query.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/api/routers/query.py b/app/api/routers/query.py index 0fcb39d..f746b7b 100644 --- a/app/api/routers/query.py +++ b/app/api/routers/query.py @@ -23,6 +23,11 @@ ) +# We (unconventionally) use an "" path prefix here because we have globally disabled +# redirection of trailing slashes in the main app file. We use an empty string here +# to ensure that a request without a trailing slash (e.g., to /query instead of /query/) +# is correctly routed to this endpoint. +# For more context, see https://github.com/neurobagel/api/issues/327. @router.get("", response_model=List[CohortQueryResponse]) async def get_query( query: QueryModel = Depends(QueryModel), From d22027e9bfd53966c4361c136cf6117b73af0d78 Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Tue, 30 Jul 2024 14:01:43 -0400 Subject: [PATCH 08/10] update slashes in README examples --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b68931c..88c9592 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,8 @@ Please refer to our [**official documentation**](https://neurobagel.org/api/) fo ## Quickstart The API is hosted at https://api.neurobagel.org/ and interfaces with Neurobagel's graph database. Queries of the graph can be run using the `/query` route. -Example: **I want to query for only female participants in the graph.** The URL for such a query would be https://api.neurobagel.org/query/?sex=snomed:248152002, where `snomed:248152002` is a [controlled term from the SNOMED CT database](http://purl.bioontology.org/ontology/SNOMEDCT/248152002) corresponding to female sex. +Example: **I want to query for only female participants in the graph.** +The URL for such a query would be https://api.neurobagel.org/query?sex=snomed:248152002, where `snomed:248152002` is a [controlled term from the SNOMED CT database](http://purl.bioontology.org/ontology/SNOMEDCT/248152002) corresponding to female sex. Interactive documentation for the API is available at https://api.neurobagel.org/docs. @@ -118,7 +119,7 @@ docker run -d --name=api -p ${NB_API_PORT_HOST}:${NB_API_PORT} --env-file=.env n By default, after running the above steps, the API should be served at localhost, http://127.0.0.1:8000/query, on the machine where you launched the Dockerized app. To check that the API is running and can access the knowledge graph as expected, you can navigate to the interactive API docs in your local browser (http://127.0.0.1:8000/docs) and enter a sample query, or send an HTTP request in your terminal using `curl`: ``` bash # example: query for female subjects in graph -curl -L http://127.0.0.1:8000/query/?sex=snomed:248152002 +curl http://127.0.0.1:8000/query?sex=snomed:248152002 ``` The response should be a list of dictionaries containing info about datasets with participants matching the query. From 17c390872a270c51972b5b83e5d31488bf6b3bef Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Tue, 30 Jul 2024 23:00:13 -0400 Subject: [PATCH 09/10] remove unneeded fstrings --- tests/test_query.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_query.py b/tests/test_query.py index e40405d..d77576b 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -70,7 +70,7 @@ def test_null_modalities( crud, "query_matching_dataset_sizes", mock_query_matching_dataset_sizes ) - response = test_app.get(f"{ROUTE}", headers=mock_auth_header) + response = test_app.get(ROUTE, headers=mock_auth_header) assert response.json()[0]["image_modals"] == [ "http://purl.org/nidash/nidm#T1Weighted" ] @@ -86,7 +86,7 @@ def test_get_all( """Given no input for any query parameters, returns a 200 status code and a non-empty list of results (should correspond to all subjects in graph).""" monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get(f"{ROUTE}", headers=mock_auth_header) + response = test_app.get(ROUTE, headers=mock_auth_header) assert response.status_code == 200 assert response.json() != [] @@ -493,7 +493,7 @@ def test_aggregate_query_response_structure( crud, "query_matching_dataset_sizes", mock_query_matching_dataset_sizes ) - response = test_app.get(f"{ROUTE}", headers=mock_auth_header) + response = test_app.get(ROUTE, headers=mock_auth_header) assert all( dataset["subject_data"] == "protected" for dataset in response.json() ) @@ -509,7 +509,7 @@ def test_query_without_token_succeeds_when_auth_disabled( Test that when authentication is disabled, a request to the /query route without a token succeeds. """ monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get(f"{ROUTE}") + response = test_app.get(ROUTE) assert response.status_code == 200 @@ -518,7 +518,7 @@ def test_request_without_trailing_slash_not_redirected( ): """Test that a request to the /query route is not redirected to have a trailing slash.""" monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get(f"{ROUTE}", follow_redirects=False) + response = test_app.get(ROUTE, follow_redirects=False) assert response.status_code == 200 From 9e29589389c55bd60e911fbe24db75727b6ef7bb Mon Sep 17 00:00:00 2001 From: Alyssa Dai Date: Wed, 31 Jul 2024 14:29:40 -0400 Subject: [PATCH 10/10] refactor tests of trailing slash behaviour into separate module --- tests/test_attributes.py | 19 ----------------- tests/test_query.py | 18 ---------------- tests/test_routing.py | 45 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 37 deletions(-) create mode 100644 tests/test_routing.py diff --git a/tests/test_attributes.py b/tests/test_attributes.py index c6ab639..3cde96f 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -7,16 +7,6 @@ from app.api import utility as util -def test_root(test_app): - """Given a GET request to the root endpoint, Check for 200 status and expected content.""" - - response = test_app.get("/") - - assert response.status_code == 200 - assert "Welcome to the Neurobagel REST API!" in response.text - assert 'documentation' in response.text - - @pytest.mark.filterwarnings("ignore:.*NB_API_ALLOWED_ORIGINS") @pytest.mark.parametrize( "valid_data_element_URI", @@ -242,12 +232,3 @@ def mock_load_json(path): "namespace_prefix": expected_namespace_pfx, "term_labels": mock_term_labels, } - - -def test_request_with_trailing_slash_not_redirected(test_app): - """ - Test that a request to a route with a trailing slash '/', where none is expected, - is *not* redirected and returns a 404. - """ - response = test_app.get("/attributes/nb:SomeClass/") - assert response.status_code == 404 diff --git a/tests/test_query.py b/tests/test_query.py index d77576b..2e92ec0 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -511,21 +511,3 @@ def test_query_without_token_succeeds_when_auth_disabled( monkeypatch.setattr(crud, "get", mock_successful_get) response = test_app.get(ROUTE) assert response.status_code == 200 - - -def test_request_without_trailing_slash_not_redirected( - test_app, monkeypatch, mock_successful_get, disable_auth -): - """Test that a request to the /query route is not redirected to have a trailing slash.""" - monkeypatch.setattr(crud, "get", mock_successful_get) - response = test_app.get(ROUTE, follow_redirects=False) - assert response.status_code == 200 - - -def test_query_params_with_leading_slash_fails(test_app, disable_auth): - """ - Test that a request to the /query route with a slash before query parameters - is *not* redirected to exclude the slash, and fails with a 404. - """ - response = test_app.get(f"{ROUTE}/?min_age=20") - assert response.status_code == 404 diff --git a/tests/test_routing.py b/tests/test_routing.py new file mode 100644 index 0000000..7b03001 --- /dev/null +++ b/tests/test_routing.py @@ -0,0 +1,45 @@ +import pytest + +from app.api import crud + + +@pytest.mark.parametrize( + "root_path", + ["/", ""], +) +def test_root(test_app, root_path): + """Given a GET request to the root endpoint, Check for 200 status and expected content.""" + + response = test_app.get(root_path, follow_redirects=False) + + assert response.status_code == 200 + assert "Welcome to the Neurobagel REST API!" in response.text + assert 'documentation' in response.text + + +@pytest.mark.parametrize( + "valid_route", + ["/query", "/query?min_age=20"], +) +def test_request_without_trailing_slash_not_redirected( + test_app, monkeypatch, mock_successful_get, disable_auth, valid_route +): + """Test that a request to a route without a / is not redirected to have a trailing slash.""" + monkeypatch.setattr(crud, "get", mock_successful_get) + response = test_app.get(valid_route, follow_redirects=False) + assert response.status_code == 200 + + +@pytest.mark.parametrize( + "invalid_route", + ["/query/", "/query/?min_age=20", "/attributes/nb:SomeClass/"], +) +def test_request_with_trailing_slash_not_redirected( + test_app, disable_auth, invalid_route +): + """ + Test that a request to routes including a trailing slash, where none is expected, + is *not* redirected to exclude the slash, and returns a 404. + """ + response = test_app.get(invalid_route) + assert response.status_code == 404