From 0f96a22589949c344316b4d485f27cab91a0de70 Mon Sep 17 00:00:00 2001 From: Ran Isenberg <60175085+ran-isenberg@users.noreply.github.com> Date: Wed, 27 Sep 2023 09:42:31 +0200 Subject: [PATCH] feature: add delete product impl. and tests (#73) --- .../integration_pillar/crud/interface.py | 4 ++ product/crud/dal/db_handler.py | 4 ++ product/crud/dal/dynamo_dal_handler.py | 13 ++++ .../domain_logic/handle_create_request.py | 12 +--- .../domain_logic/handle_delete_request.py | 11 ++++ .../crud/domain_logic/handle_get_request.py | 15 +++++ product/crud/handlers/delete_product.py | 28 ++++++++- product/crud/handlers/get_product.py | 2 +- product/crud/handlers/schemas/env_vars.py | 4 ++ product/crud/schemas/input.py | 8 ++- tests/e2e/crud/test_delete_product.py | 27 +++++++-- tests/integration/crud/test_create_product.py | 4 +- tests/integration/crud/test_delete_product.py | 59 +++++++++++++++---- ...ut.py => test_product_path_parms_input.py} | 12 ++-- 14 files changed, 165 insertions(+), 38 deletions(-) create mode 100644 product/crud/domain_logic/handle_delete_request.py create mode 100644 product/crud/domain_logic/handle_get_request.py rename tests/unit/crud/{test_get_product_input.py => test_product_path_parms_input.py} (57%) diff --git a/docs/examples/integration_pillar/crud/interface.py b/docs/examples/integration_pillar/crud/interface.py index 20e7b8e..bb22b41 100644 --- a/docs/examples/integration_pillar/crud/interface.py +++ b/docs/examples/integration_pillar/crud/interface.py @@ -21,3 +21,7 @@ def create_product(self, product_id: str, product_name: str, product_price: int) @abstractmethod def get_product(self, product_id: str) -> ProductEntry: ... # pragma: no cover + + @abstractmethod + def delete_product(self, product_id: str) -> None: + ... # pragma: no cover diff --git a/product/crud/dal/db_handler.py b/product/crud/dal/db_handler.py index 20e7b8e..bb22b41 100644 --- a/product/crud/dal/db_handler.py +++ b/product/crud/dal/db_handler.py @@ -21,3 +21,7 @@ def create_product(self, product_id: str, product_name: str, product_price: int) @abstractmethod def get_product(self, product_id: str) -> ProductEntry: ... # pragma: no cover + + @abstractmethod + def delete_product(self, product_id: str) -> None: + ... # pragma: no cover diff --git a/product/crud/dal/dynamo_dal_handler.py b/product/crud/dal/dynamo_dal_handler.py index b1aa298..cb0e264 100644 --- a/product/crud/dal/dynamo_dal_handler.py +++ b/product/crud/dal/dynamo_dal_handler.py @@ -65,3 +65,16 @@ def get_product(self, product_id: str) -> ProductEntry: logger.info('got item successfully', extra={'product_id': product_id}) return db_entry + + @tracer.capture_method(capture_response=False) + def delete_product(self, product_id: str) -> None: + logger.info('trying to delete a product', extra={'product_id': product_id}) + try: + table: Table = self._get_db_handler(self.table_name) + table.delete_item(Key={'id': product_id}) + except ClientError as exc: # pragma: no cover (covered in integration test) + error_msg = 'failed to delete product from db' + logger.exception(error_msg, extra={'exception': str(exc)}) + raise InternalServerException(error_msg) from exc + + logger.info('deleted product successfully', extra={'product_id': product_id}) diff --git a/product/crud/domain_logic/handle_create_request.py b/product/crud/domain_logic/handle_create_request.py index 2123bbf..89a84ed 100644 --- a/product/crud/domain_logic/handle_create_request.py +++ b/product/crud/domain_logic/handle_create_request.py @@ -2,7 +2,7 @@ from product.crud.dal.db_handler import DalHandler from product.crud.dal.schemas.db import ProductEntry from product.crud.handlers.utils.observability import logger, tracer -from product.crud.schemas.output import CreateProductOutput, GetProductOutput +from product.crud.schemas.output import CreateProductOutput @tracer.capture_method(capture_response=False) @@ -13,13 +13,3 @@ def handle_create_request(product_id: str, product_name: str, product_price: int product: ProductEntry = dal_handler.create_product(product_id=product_id, product_name=product_name, product_price=product_price) # convert from db entry to output, they won't always be the same return CreateProductOutput(id=product.id) - - -@tracer.capture_method(capture_response=False) -def handle_get_request(product_id: str, table_name: str) -> GetProductOutput: - logger.info('handling get product request', extra={'product_id': product_id}) - - dal_handler: DalHandler = get_dal_handler(table_name) - product: ProductEntry = dal_handler.get_product(product_id=product_id) - # convert from db entry to output, they won't always be the same - return GetProductOutput(id=product.id, price=product.price, name=product.name) diff --git a/product/crud/domain_logic/handle_delete_request.py b/product/crud/domain_logic/handle_delete_request.py new file mode 100644 index 0000000..514ae46 --- /dev/null +++ b/product/crud/domain_logic/handle_delete_request.py @@ -0,0 +1,11 @@ +from product.crud.dal import get_dal_handler +from product.crud.dal.db_handler import DalHandler +from product.crud.handlers.utils.observability import logger, tracer + + +@tracer.capture_method(capture_response=False) +def handle_delete_request(product_id: str, table_name: str) -> None: + logger.info('handling get product request', extra={'product_id': product_id}) + + dal_handler: DalHandler = get_dal_handler(table_name) + dal_handler.delete_product(product_id=product_id) diff --git a/product/crud/domain_logic/handle_get_request.py b/product/crud/domain_logic/handle_get_request.py new file mode 100644 index 0000000..a22dd11 --- /dev/null +++ b/product/crud/domain_logic/handle_get_request.py @@ -0,0 +1,15 @@ +from product.crud.dal import get_dal_handler +from product.crud.dal.db_handler import DalHandler +from product.crud.dal.schemas.db import ProductEntry +from product.crud.handlers.utils.observability import logger, tracer +from product.crud.schemas.output import GetProductOutput + + +@tracer.capture_method(capture_response=False) +def handle_get_request(product_id: str, table_name: str) -> GetProductOutput: + logger.info('handling get product request', extra={'product_id': product_id}) + + dal_handler: DalHandler = get_dal_handler(table_name) + product: ProductEntry = dal_handler.get_product(product_id=product_id) + # convert from db entry to output, they won't always be the same + return GetProductOutput(id=product.id, price=product.price, name=product.name) diff --git a/product/crud/handlers/delete_product.py b/product/crud/handlers/delete_product.py index 2c47dbe..47ae9fa 100644 --- a/product/crud/handlers/delete_product.py +++ b/product/crud/handlers/delete_product.py @@ -1,19 +1,45 @@ from http import HTTPStatus from typing import Any, Dict +from aws_lambda_env_modeler import get_environment_variables, init_environment_variables from aws_lambda_powertools.metrics import MetricUnit +from aws_lambda_powertools.utilities.parser import ValidationError, parse from aws_lambda_powertools.utilities.typing import LambdaContext +from product.crud.domain_logic.handle_delete_request import handle_delete_request +from product.crud.handlers.schemas.env_vars import DeleteVars from product.crud.handlers.utils.http_responses import build_response from product.crud.handlers.utils.observability import logger, metrics, tracer +from product.crud.schemas.exceptions import InternalServerException +from product.crud.schemas.input import DeleteProductRequest +@init_environment_variables(model=DeleteVars) @metrics.log_metrics @tracer.capture_lambda_handler(capture_response=False) def delete_product(event: Dict[str, Any], context: LambdaContext) -> Dict[str, Any]: logger.set_correlation_id(context.aws_request_id) + env_vars: DeleteVars = get_environment_variables(model=DeleteVars) + logger.debug('environment variables', extra=env_vars.model_dump()) + + try: + # we want to extract and parse the HTTP body from the api gw envelope + delete_input: DeleteProductRequest = parse(event=event, model=DeleteProductRequest) + logger.info('got a delete product request', extra={'product': delete_input.model_dump()}) + except (ValidationError, TypeError) as exc: # pragma: no cover + logger.exception('event failed input validation', extra={'error': str(exc)}) + return build_response(http_status=HTTPStatus.BAD_REQUEST, body={}) metrics.add_metric(name='DeleteProductEvents', unit=MetricUnit.Count, value=1) + try: + handle_delete_request( + product_id=delete_input.pathParameters.product, + table_name=env_vars.TABLE_NAME, + ) + except InternalServerException: # pragma: no cover + logger.exception('finished handling delete product request with internal error') + return build_response(http_status=HTTPStatus.INTERNAL_SERVER_ERROR, body={}) + logger.info('finished handling delete product request') - return build_response(http_status=HTTPStatus.NOT_IMPLEMENTED, body={}) + return build_response(http_status=HTTPStatus.NO_CONTENT, body={}) diff --git a/product/crud/handlers/get_product.py b/product/crud/handlers/get_product.py index e6a8cee..96db4f4 100644 --- a/product/crud/handlers/get_product.py +++ b/product/crud/handlers/get_product.py @@ -6,7 +6,7 @@ from aws_lambda_powertools.utilities.parser import ValidationError, parse from aws_lambda_powertools.utilities.typing import LambdaContext -from product.crud.domain_logic.handle_create_request import handle_get_request +from product.crud.domain_logic.handle_get_request import handle_get_request from product.crud.handlers.schemas.env_vars import GetVars from product.crud.handlers.utils.http_responses import build_response from product.crud.handlers.utils.observability import logger, metrics, tracer diff --git a/product/crud/handlers/schemas/env_vars.py b/product/crud/handlers/schemas/env_vars.py index e08f4d6..12939b4 100644 --- a/product/crud/handlers/schemas/env_vars.py +++ b/product/crud/handlers/schemas/env_vars.py @@ -18,3 +18,7 @@ class CreateVars(Observability, Idempotency): class GetVars(Observability): TABLE_NAME: Annotated[str, Field(min_length=1)] + + +class DeleteVars(Observability): + TABLE_NAME: Annotated[str, Field(min_length=1)] diff --git a/product/crud/schemas/input.py b/product/crud/schemas/input.py index 195fb46..70c685c 100644 --- a/product/crud/schemas/input.py +++ b/product/crud/schemas/input.py @@ -20,9 +20,13 @@ class CreateProductRequest(APIGatewayProxyEventModel): pathParameters: PutPathParams # type: ignore -class GetPathParams(BaseModel): +class ProductPathParams(BaseModel): product: ProductId class GetProductRequest(APIGatewayProxyEventModel): - pathParameters: GetPathParams # type: ignore + pathParameters: ProductPathParams # type: ignore + + +class DeleteProductRequest(APIGatewayProxyEventModel): + pathParameters: ProductPathParams # type: ignore diff --git a/tests/e2e/crud/test_delete_product.py b/tests/e2e/crud/test_delete_product.py index 1958295..8a4a3af 100644 --- a/tests/e2e/crud/test_delete_product.py +++ b/tests/e2e/crud/test_delete_product.py @@ -2,11 +2,28 @@ import requests -from tests.crud_utils import generate_create_product_request_body +from tests.crud_utils import generate_product_id, generate_random_integer, generate_random_string +from tests.e2e.crud.utils import create_product -def test_handler_200_ok(api_gw_url_slash_product: str, product_id: str): - body = generate_create_product_request_body() +# create product and then delete it +def test_handler_204_success_delete(api_gw_url_slash_product: str) -> None: + product_id = generate_product_id() + price = generate_random_integer() + name = generate_random_string() + create_product(api_gw_url_slash_product=api_gw_url_slash_product, product_id=product_id, price=price, name=name) url_with_product_id = f'{api_gw_url_slash_product}/{product_id}' - response = requests.delete(url=url_with_product_id, data=body.model_dump_json(), timeout=10) - assert response.status_code == HTTPStatus.NOT_IMPLEMENTED + response: requests.Response = requests.delete(url=url_with_product_id, timeout=10) + assert response.status_code == HTTPStatus.NO_CONTENT + + +def test_handler_invalid_path(api_gw_url: str) -> None: + url_with_product_id = f'{api_gw_url}/dummy' + response = requests.delete(url=url_with_product_id) + assert response.status_code == HTTPStatus.FORBIDDEN + + +def test_handler_invalid_product_id(api_gw_url_slash_product: str) -> None: + url_with_product_id = f'{api_gw_url_slash_product}/aaaa' + response = requests.delete(url=url_with_product_id) + assert response.status_code == HTTPStatus.BAD_REQUEST diff --git a/tests/integration/crud/test_create_product.py b/tests/integration/crud/test_create_product.py index 654ed6f..941c008 100644 --- a/tests/integration/crud/test_create_product.py +++ b/tests/integration/crud/test_create_product.py @@ -10,12 +10,12 @@ from tests.utils import generate_context -def call_create_product(body: Dict[str, Any]) -> Dict[str, Any]: +def call_create_product(event: Dict[str, Any]) -> Dict[str, Any]: # important is done here since idempotency decorator requires an env. variable during import time # conf.test sets that env. variable (table name) but it runs after imports # this way, idempotency import runs after conftest sets the values already from product.crud.handlers.create_product import create_product - return create_product(body, generate_context()) + return create_product(event, generate_context()) def test_handler_200_ok(table_name: str): diff --git a/tests/integration/crud/test_delete_product.py b/tests/integration/crud/test_delete_product.py index eb93dd7..120c92c 100644 --- a/tests/integration/crud/test_delete_product.py +++ b/tests/integration/crud/test_delete_product.py @@ -1,18 +1,57 @@ +import json from http import HTTPStatus -from typing import Any, Dict +from typing import Any, Dict, Generator -from product.crud.handlers.delete_product import delete_product -from tests.crud_utils import generate_api_gw_event, generate_create_product_request_body, generate_product_id +import boto3 +import pytest +from botocore.stub import Stubber + +from product.crud.dal.dynamo_dal_handler import DynamoDalHandler +from product.crud.dal.schemas.db import ProductEntry +from tests.crud_utils import generate_api_gw_event, generate_product_id from tests.utils import generate_context -def call_delete_product(body: Dict[str, Any]) -> Dict[str, Any]: - return delete_product(body, generate_context()) +@pytest.fixture() +def add_product_entry_to_db(table_name: str) -> Generator[ProductEntry, None, None]: + product = ProductEntry(id=generate_product_id(), price=1, name='test') + table = boto3.resource('dynamodb').Table(table_name) + table.put_item(Item=product.model_dump()) + yield product + table.delete_item(Key={'id': product.id}) + + +def call_delete_product(event: Dict[str, Any]) -> Dict[str, Any]: + # important is done here since idempotency decorator requires an env. variable during import time + # conf.test sets that env. variable (table name) but it runs after imports + # this way, idempotency import runs after conftest sets the values already + from product.crud.handlers.delete_product import delete_product + return delete_product(event, generate_context()) -def test_handler_200_ok(): - body = generate_create_product_request_body() - product_id = generate_product_id() - response = call_delete_product(generate_api_gw_event(body=body.model_dump(), path_params={'product': product_id})) +def test_handler_204_success_delete(add_product_entry_to_db: ProductEntry): + product_id = add_product_entry_to_db.id + event = generate_api_gw_event(path_params={'product': product_id}) + response = call_delete_product(event) # assert response - assert response['statusCode'] == HTTPStatus.NOT_IMPLEMENTED + assert response['statusCode'] == HTTPStatus.NO_CONTENT + + +def test_internal_server_error(table_name): + db_handler: DynamoDalHandler = DynamoDalHandler(table_name) + table = db_handler._get_db_handler(table_name) + + with Stubber(table.meta.client) as stubber: + stubber.add_client_error(method='delete_item', service_error_code='ValidationException') + event = generate_api_gw_event(path_params={'product': generate_product_id()}) + response = call_delete_product(event) + + assert response['statusCode'] == HTTPStatus.INTERNAL_SERVER_ERROR + + +def test_handler_bad_request_invalid_path_params(): + event = generate_api_gw_event(path_params={'dummy': generate_product_id()}) + response = call_delete_product(event) + assert response['statusCode'] == HTTPStatus.BAD_REQUEST + body_dict = json.loads(response['body']) + assert body_dict == {} diff --git a/tests/unit/crud/test_get_product_input.py b/tests/unit/crud/test_product_path_parms_input.py similarity index 57% rename from tests/unit/crud/test_get_product_input.py rename to tests/unit/crud/test_product_path_parms_input.py index 4534925..d1f9665 100644 --- a/tests/unit/crud/test_get_product_input.py +++ b/tests/unit/crud/test_product_path_parms_input.py @@ -1,28 +1,28 @@ import pytest from aws_lambda_powertools.utilities.parser import ValidationError -from product.crud.schemas.input import GetPathParams +from product.crud.schemas.input import ProductPathParams def test_invalid_product_id_invalid_string(): with pytest.raises(ValidationError): - GetPathParams.model_validate({'product': 'aa'}) + ProductPathParams.model_validate({'product': 'aa'}) def test_invalid_product_empty(): with pytest.raises(ValidationError): - GetPathParams.model_validate({}) + ProductPathParams.model_validate({}) def test_invalid_product_type_mismatch(): with pytest.raises(ValidationError): - GetPathParams.model_validate({'product': 6}) + ProductPathParams.model_validate({'product': 6}) def test_invalid_json_key_but_valid_uuid(product_id): with pytest.raises(ValidationError): - GetPathParams.model_validate({'order': product_id}) + ProductPathParams.model_validate({'order': product_id}) def test_valid_uuid_input(product_id): - GetPathParams.model_validate({'product': product_id}) + ProductPathParams.model_validate({'product': product_id})