From 3dee614560843a8362acaef63b33330c4ae3014b Mon Sep 17 00:00:00 2001 From: Marc Sommerhalder Date: Wed, 30 Oct 2024 08:13:24 +0100 Subject: [PATCH] PB-1108 Refine user deletion responses --- app/access/api.py | 4 ++- app/access/tests/test_api.py | 58 +++++++++++++++++++++++------------- app/config/api.py | 12 ++++++++ 3 files changed, 52 insertions(+), 22 deletions(-) diff --git a/app/access/api.py b/app/access/api.py index 642949d..ee108b3 100644 --- a/app/access/api.py +++ b/app/access/api.py @@ -55,10 +55,12 @@ def delete(request: HttpRequest, username: str) -> HttpResponse: Return HTTP status code - 204 (No Content) if the user has been deleted - 404 (Not Found) if there is no user with the given username + - 500 (Internal Server Error) if there is inconsistency with cognito + - 503 (Service Unavailable) if cognito cannot be reached """ model = get_object_or_404(User, username=username) deleted = delete_user(model) if not deleted: - raise HttpError(503, "Service Unavailable. Please retry later.") + raise HttpError(500, "Internal Server Error") model.delete() return HttpResponse(status=204) diff --git a/app/access/tests/test_api.py b/app/access/tests/test_api.py index 7f447ae..2e67284 100644 --- a/app/access/tests/test_api.py +++ b/app/access/tests/test_api.py @@ -4,6 +4,8 @@ from access.api import user_to_response from access.models import User from access.schemas import UserSchema +from botocore.exceptions import EndpointConnectionError +from config.api import cognito_connection_error from ninja.testing import TestClient from provider.models import Provider @@ -23,6 +25,13 @@ def setUp(self): } User.objects.create(**model_fields) + self.client = TestClient(router) + # Re-attach custom exception handlers https://github.com/vitalik/django-ninja/issues/1171 + _ = self.client.urls + self.client.router_or_app.api.add_exception_handler( + EndpointConnectionError, cognito_connection_error + ) + def test_user_to_response_maps_fields_correctly(self): model = User.objects.last() @@ -41,8 +50,7 @@ def test_user_to_response_maps_fields_correctly(self): def test_get_user_returns_existing_user(self): - client = TestClient(router) - response = client.get("users/dude") + response = self.client.get("users/dude") assert response.status_code == 200 assert response.data == { @@ -55,16 +63,14 @@ def test_get_user_returns_existing_user(self): def test_get_user_returns_404_if_nonexisting(self): - client = TestClient(router) - response = client.get("users/nihilist") + response = self.client.get("users/nihilist") assert response.status_code == 404 assert response.data == {"detail": "Not Found"} def test_get_users_returns_single_user(self): - client = TestClient(router) - response = client.get("users") + response = self.client.get("users") assert response.status_code == 200 assert response.data == { @@ -88,8 +94,7 @@ def test_get_users_returns_users_ordered_by_id(self): } User.objects.create(**model_fields) - client = TestClient(router) - response = client.get("users") + response = self.client.get("users") assert response.status_code == 200 assert response.data == { @@ -111,31 +116,42 @@ def test_get_users_returns_users_ordered_by_id(self): ] } - @patch('access.api.delete_user', return_value=True) - def test_delete_user_deletes_user(self, mock): - client = TestClient(router) - response = client.delete("users/dude") + @patch('access.api.delete_user') + def test_delete_user_deletes_user(self, delete_user): + delete_user.return_value = True + + response = self.client.delete("users/dude") assert response.status_code == 204 assert response.content == b'' assert User.objects.count() == 0 - @patch('access.api.delete_user', return_value=False) - def test_delete_user_returns_404_if_nonexisting(self, mock): + @patch('access.api.delete_user') + def test_delete_user_returns_404_if_nonexisting(self, delete_user): + delete_user.return_value = False - client = TestClient(router) - response = client.delete("users/lebowski") + response = self.client.delete("users/lebowski") assert response.status_code == 404 assert response.data == {"detail": "Not Found"} assert User.objects.count() == 1 - @patch('access.api.delete_user', return_value=False) - def test_delete_user_returns_503_if_cognito_down(self, mock): + @patch('access.api.delete_user') + def test_delete_user_returns_500_if_cognito_inconsistent(self, delete_user): + delete_user.return_value = False + + response = self.client.delete("users/dude") + + assert response.status_code == 500 + assert response.data == {"detail": "Internal Server Error"} + assert User.objects.count() == 1 + + @patch('access.api.delete_user') + def test_delete_user_returns_503_if_cognito_down(self, delete_user): + delete_user.side_effect = EndpointConnectionError(endpoint_url='http://localhost') - client = TestClient(router) - response = client.delete("users/dude") + response = self.client.delete("users/dude") assert response.status_code == 503 - assert response.data == {'detail': 'Service Unavailable. Please retry later.'} + assert response.data == {"code": 503, "description": "Service Unavailable"} assert User.objects.count() == 1 diff --git a/app/config/api.py b/app/config/api.py index 3102b7b..dfdc288 100644 --- a/app/config/api.py +++ b/app/config/api.py @@ -1,4 +1,5 @@ from access.api import router as access_router +from botocore.exceptions import EndpointConnectionError from distributions.api import router as distributions_router from ninja import NinjaAPI from provider.api import router as provider_router @@ -14,6 +15,17 @@ root = NinjaAPI(urls_namespace="root") +@api.exception_handler(EndpointConnectionError) +def cognito_connection_error(request: HttpRequest, exception: EndpointConnectionError): + return api.create_response( + request, + { + "code": 503, "description": "Service Unavailable" + }, + status=503, + ) + + @root.get("/checker") def checker(request: HttpRequest) -> dict[str, bool | str]: return {"success": True, "message": "OK"}