From d77e3350b666ba7e4ae868da296c8795f92b9b0c Mon Sep 17 00:00:00 2001 From: matusdrobuliak66 <60785969+matusdrobuliak66@users.noreply.github.com> Date: Fri, 10 Nov 2023 18:06:35 +0100 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20improve=20error=20handling?= =?UTF-8?q?=20of=20pricing=20plans=20in=20webserver=20(#4980)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .env-devel | 2 +- .../services/payments.py | 2 +- .../core/errors.py | 2 +- .../db/repositories/resource_tracker.py | 4 +- .../catalog/_constants.py | 2 + .../catalog/client.py | 11 ++- .../resource_usage/_client.py | 4 + .../resource_usage/_constants.py | 2 + .../resource_usage/_utils.py | 13 ++- .../01/test_catalog_api__pricing_plan.py | 99 +++++++++++++++++++ 10 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 services/web/server/tests/unit/with_dbs/01/test_catalog_api__pricing_plan.py diff --git a/.env-devel b/.env-devel index 2b837fdfcc9..5bc2e21c741 100644 --- a/.env-devel +++ b/.env-devel @@ -144,7 +144,7 @@ TRAEFIK_SIMCORE_ZONE=internal_simcore_stack # NOTE: WEBSERVER_SESSION_SECRET_KEY = $(python3 -c "from cryptography.fernet import Fernet; print(Fernet.generate_key())") -WEBSERVER_CREDIT_COMPUTATION_ENABLED=0 +WEBSERVER_CREDIT_COMPUTATION_ENABLED=1 WEBSERVER_LOGLEVEL=INFO diff --git a/services/payments/src/simcore_service_payments/services/payments.py b/services/payments/src/simcore_service_payments/services/payments.py index 2f3a7cc8d67..dcb0dceac17 100644 --- a/services/payments/src/simcore_service_payments/services/payments.py +++ b/services/payments/src/simcore_service_payments/services/payments.py @@ -157,7 +157,7 @@ async def on_payment_completed( credit_transaction_id = await rut_api.create_credit_transaction( product_name=transaction.product_name, wallet_id=transaction.wallet_id, - wallet_name="id={transaction.wallet_id}", + wallet_name=f"id={transaction.wallet_id}", user_id=transaction.user_id, user_email=transaction.user_email, osparc_credits=transaction.osparc_credits, diff --git a/services/resource-usage-tracker/src/simcore_service_resource_usage_tracker/core/errors.py b/services/resource-usage-tracker/src/simcore_service_resource_usage_tracker/core/errors.py index 93cc0230698..5c05b064824 100644 --- a/services/resource-usage-tracker/src/simcore_service_resource_usage_tracker/core/errors.py +++ b/services/resource-usage-tracker/src/simcore_service_resource_usage_tracker/core/errors.py @@ -19,4 +19,4 @@ def http404_error_handler( request: Request, # pylint: disable=unused-argument error: CustomResourceUsageTrackerError, ) -> JSONResponse: - return JSONResponse(status_code=404, content={"message": error.msg_template}) + return JSONResponse(status_code=404, content={"message": f"{error.msg_template}"}) diff --git a/services/resource-usage-tracker/src/simcore_service_resource_usage_tracker/modules/db/repositories/resource_tracker.py b/services/resource-usage-tracker/src/simcore_service_resource_usage_tracker/modules/db/repositories/resource_tracker.py index 9965b855f44..86ef25bd9bf 100644 --- a/services/resource-usage-tracker/src/simcore_service_resource_usage_tracker/modules/db/repositories/resource_tracker.py +++ b/services/resource-usage-tracker/src/simcore_service_resource_usage_tracker/modules/db/repositories/resource_tracker.py @@ -690,7 +690,7 @@ async def get_pricing_unit( row = result.first() if row is None: raise CustomResourceUsageTrackerError( - msg=f"Pricing unit id {pricing_unit_id} not found" + msg=f"Pricing plan {pricing_plan_id} and pricing unit {pricing_unit_id} for product {product_name} not found" ) return PricingUnitsDB.from_orm(row) @@ -723,6 +723,6 @@ async def get_pricing_unit_cost_by_id( row = result.first() if row is None: raise CustomResourceUsageTrackerError( - msg=f"Pricing unit cosd id {pricing_unit_cost_id} not found in the resource_tracker_pricing_unit_costs table", + msg=f"Pricing unit cost id {pricing_unit_cost_id} not found in the resource_tracker_pricing_unit_costs table", ) return PricingUnitCostsDB.from_orm(row) diff --git a/services/web/server/src/simcore_service_webserver/catalog/_constants.py b/services/web/server/src/simcore_service_webserver/catalog/_constants.py index 6cef02b06c2..371c56ce46d 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/_constants.py +++ b/services/web/server/src/simcore_service_webserver/catalog/_constants.py @@ -3,3 +3,5 @@ MSG_CATALOG_SERVICE_UNAVAILABLE: Final[ str ] = "Currently catalog service is unavailable, please try again later" + +MSG_CATALOG_SERVICE_NOT_FOUND: Final[str] = "Not Found" diff --git a/services/web/server/src/simcore_service_webserver/catalog/client.py b/services/web/server/src/simcore_service_webserver/catalog/client.py index ac09c88e781..89f841dc798 100644 --- a/services/web/server/src/simcore_service_webserver/catalog/client.py +++ b/services/web/server/src/simcore_service_webserver/catalog/client.py @@ -26,7 +26,7 @@ from yarl import URL from .._meta import api_version_prefix -from ._constants import MSG_CATALOG_SERVICE_UNAVAILABLE +from ._constants import MSG_CATALOG_SERVICE_NOT_FOUND, MSG_CATALOG_SERVICE_UNAVAILABLE from .settings import get_plugin_settings _logger = logging.getLogger(__name__) @@ -39,7 +39,14 @@ def _handle_client_exceptions(app: web.Application) -> Iterator[ClientSession]: yield session - except (asyncio.TimeoutError, ClientConnectionError, ClientResponseError) as err: + except ClientResponseError as err: + if err.status == 404: + raise web.HTTPNotFound(reason=MSG_CATALOG_SERVICE_NOT_FOUND) + raise web.HTTPServiceUnavailable( + reason=MSG_CATALOG_SERVICE_UNAVAILABLE + ) from err + + except (asyncio.TimeoutError, ClientConnectionError) as err: _logger.debug("Request to catalog service failed: %s", err) raise web.HTTPServiceUnavailable( reason=MSG_CATALOG_SERVICE_UNAVAILABLE diff --git a/services/web/server/src/simcore_service_webserver/resource_usage/_client.py b/services/web/server/src/simcore_service_webserver/resource_usage/_client.py index 8b8e3cc4f14..9eb150859eb 100644 --- a/services/web/server/src/simcore_service_webserver/resource_usage/_client.py +++ b/services/web/server/src/simcore_service_webserver/resource_usage/_client.py @@ -95,6 +95,7 @@ async def get_default_service_pricing_plan( ) with handle_client_exceptions(app) as session: async with session.get(url) as response: + response.raise_for_status() body: dict = await response.json() return parse_obj_as(ServicePricingPlanGet, body) @@ -119,6 +120,7 @@ async def get_pricing_plan_unit( ) with handle_client_exceptions(app) as session: async with session.get(url) as response: + response.raise_for_status() body: dict = await response.json() return parse_obj_as(PricingUnitGet, body) @@ -139,6 +141,7 @@ async def sum_total_available_credits_in_the_wallet( ) with handle_client_exceptions(app) as session: async with session.post(url) as response: + response.raise_for_status() body: dict = await response.json() return WalletTotalCredits.construct(**body) @@ -168,6 +171,7 @@ async def add_credits_to_wallet( } with handle_client_exceptions(app) as session: async with session.post(url, json=body) as response: + response.raise_for_status() output: dict = await response.json() return output diff --git a/services/web/server/src/simcore_service_webserver/resource_usage/_constants.py b/services/web/server/src/simcore_service_webserver/resource_usage/_constants.py index 1e070a35285..64586ad4c49 100644 --- a/services/web/server/src/simcore_service_webserver/resource_usage/_constants.py +++ b/services/web/server/src/simcore_service_webserver/resource_usage/_constants.py @@ -5,3 +5,5 @@ MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE: Final[ str ] = "Currently resource usage tracker service is unavailable, please try again later" + +MSG_RESOURCE_USAGE_TRACKER_NOT_FOUND: Final[str] = "Not Found" diff --git a/services/web/server/src/simcore_service_webserver/resource_usage/_utils.py b/services/web/server/src/simcore_service_webserver/resource_usage/_utils.py index 3d8c824da73..2232f9bec6e 100644 --- a/services/web/server/src/simcore_service_webserver/resource_usage/_utils.py +++ b/services/web/server/src/simcore_service_webserver/resource_usage/_utils.py @@ -7,7 +7,10 @@ from aiohttp.client_exceptions import ClientConnectionError, ClientResponseError from servicelib.aiohttp.client_session import get_client_session -from ._constants import MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE +from ._constants import ( + MSG_RESOURCE_USAGE_TRACKER_NOT_FOUND, + MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE, +) _logger = logging.getLogger(__name__) @@ -18,8 +21,14 @@ def handle_client_exceptions(app: web.Application) -> Iterator[ClientSession]: session: ClientSession = get_client_session(app) yield session + except (ClientResponseError) as err: + if err.status == 404: + raise web.HTTPNotFound(reason=MSG_RESOURCE_USAGE_TRACKER_NOT_FOUND) + raise web.HTTPServiceUnavailable( + reason=MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE + ) from err - except (asyncio.TimeoutError, ClientConnectionError, ClientResponseError) as err: + except (asyncio.TimeoutError, ClientConnectionError) as err: _logger.debug("Request to resource usage tracker service failed: %s", err) raise web.HTTPServiceUnavailable( reason=MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE diff --git a/services/web/server/tests/unit/with_dbs/01/test_catalog_api__pricing_plan.py b/services/web/server/tests/unit/with_dbs/01/test_catalog_api__pricing_plan.py new file mode 100644 index 00000000000..08ed576f038 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/01/test_catalog_api__pricing_plan.py @@ -0,0 +1,99 @@ +# pylint:disable=unused-variable +# pylint:disable=unused-argument +# pylint:disable=redefined-outer-name + +import re +import urllib.parse + +import pytest +from aiohttp import web +from aiohttp.test_utils import TestClient +from models_library.api_schemas_resource_usage_tracker.pricing_plans import ( + ServicePricingPlanGet, +) +from models_library.utils.fastapi_encoders import jsonable_encoder +from pydantic import parse_obj_as +from pytest_simcore.aioresponses_mocker import AioResponsesMock +from pytest_simcore.helpers.utils_assert import assert_status +from pytest_simcore.helpers.utils_login import UserInfoDict +from settings_library.resource_usage_tracker import ResourceUsageTrackerSettings +from simcore_service_webserver.db.models import UserRole +from simcore_service_webserver.resource_usage.settings import get_plugin_settings + + +@pytest.fixture +def mock_rut_api_responses( + client: TestClient, aioresponses_mocker: AioResponsesMock +) -> AioResponsesMock: + assert client.app + settings: ResourceUsageTrackerSettings = get_plugin_settings(client.app) + + service_pricing_plan_get = parse_obj_as( + ServicePricingPlanGet, + ServicePricingPlanGet.Config.schema_extra["examples"][0], + ) + aioresponses_mocker.get( + re.compile(f"^{settings.api_base_url}/services/+.+$"), + payload=jsonable_encoder(service_pricing_plan_get), + ) + + return aioresponses_mocker + + +@pytest.mark.parametrize( + "user_role,expected", + [ + (UserRole.ANONYMOUS, web.HTTPUnauthorized), + (UserRole.GUEST, web.HTTPOk), + (UserRole.USER, web.HTTPOk), + (UserRole.TESTER, web.HTTPOk), + ], +) +async def test_get_service_pricinp_plan_role_access_rights( + client: TestClient, + logged_user: UserInfoDict, + mock_rut_api_responses: AioResponsesMock, + expected: type[web.HTTPException], +): + assert client.app + assert client.app.router + url = client.app.router["get_service_pricing_plan"].url_for( + service_key=urllib.parse.quote("simcore/services/dynamic/someservice", safe=""), + service_version="3.4.5", + ) + response = await client.get(f"{url}") + await assert_status(response, expected) + + +@pytest.fixture +def mock_catalog_get_service_pricing_plan_not_found( + client: TestClient, aioresponses_mocker: AioResponsesMock +) -> AioResponsesMock: + assert client.app + settings: ResourceUsageTrackerSettings = get_plugin_settings(client.app) + url_pattern = re.compile(f"^{settings.base_url}+/.*$") + + aioresponses_mocker.get(url_pattern, exception=web.HTTPNotFound) + return aioresponses_mocker + + +@pytest.mark.parametrize( + "user_role,expected", + [ + (UserRole.TESTER, web.HTTPNotFound), + ], +) +async def test_get_service_pricing_plan_raises_not_found_error( + client: TestClient, + logged_user: UserInfoDict, + mock_catalog_get_service_pricing_plan_not_found: AioResponsesMock, + expected: type[web.HTTPException], +): + assert client.app + assert client.app.router + url = client.app.router["get_service_pricing_plan"].url_for( + service_key="simcore%2Fservices%2Fdynamic%2Fsomeservice", + service_version="3.4.5", + ) + response = await client.get(f"{url}") + await assert_status(response, expected)