From f8daf73c8cb50e173cd3a168bb8ae07074f1242a Mon Sep 17 00:00:00 2001 From: Leandro Damascena Date: Mon, 6 Nov 2023 21:56:01 +0000 Subject: [PATCH 1/5] fix(event_handler): fixing middleware execution when using route prefix --- .../event_handler/api_gateway.py | 3 +- .../event_handler/test_api_middlewares.py | 38 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index 37e2265ea8..82fb0085ad 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -2017,7 +2017,8 @@ def include_router(self, router: "Router", prefix: Optional[str] = None) -> None new_route = (rule, *route[1:]) # Middlewares are stored by route separately - must grab them to include - middlewares = router._routes_with_middleware.get(new_route) + # Middleware store the route without prefix, so we must not include prefix when grabbing + middlewares = router._routes_with_middleware.get(route) # Need to use "type: ignore" here since mypy does not like a named parameter after # tuple expansion since may cause duplicate named parameters in the function signature. diff --git a/tests/functional/event_handler/test_api_middlewares.py b/tests/functional/event_handler/test_api_middlewares.py index 8f98b93343..c6a1581391 100644 --- a/tests/functional/event_handler/test_api_middlewares.py +++ b/tests/functional/event_handler/test_api_middlewares.py @@ -397,6 +397,44 @@ def dummy_route(): assert result["statusCode"] == 200 +@pytest.mark.parametrize( + "app, event", + [ + (ApiGatewayResolver(proxy_type=ProxyEventType.APIGatewayProxyEvent), API_REST_EVENT), + (APIGatewayRestResolver(), API_REST_EVENT), + (APIGatewayHttpResolver(), API_RESTV2_EVENT), + ], +) +def test_api_gateway_middleware_with_include_router_prefix(app: EventHandlerInstance, event): + # GIVEN two global middlewares: one for App and one for Router + router = Router() + + def app_middleware(app: EventHandlerInstance, next_middleware: NextMiddleware): + # inject a variable into resolver context + app.append_context(injected="injected_value") + response = next_middleware(app) + + return response + + @router.get("/path", middlewares=[app_middleware]) + def dummy_route(): + # if the middleware is executed, the context variable will be available + # Otherwise, an exception will be raised + injected_context = app.context["injected"] + + assert injected_context == "injected_value" + + return Response(status_code=200, body="works!") + + # WHEN register the route with a prefix + app.include_router(router, prefix="/my") + + # THEN resolving a request must execute the middleware + result = app(event, {}) + + assert result["statusCode"] == 200 + + @pytest.mark.parametrize( "app, event", [ From 826ae4e0523c0e8e1a91274a87bec6db4fe29bef Mon Sep 17 00:00:00 2001 From: Leandro Damascena Date: Mon, 6 Nov 2023 22:40:08 +0000 Subject: [PATCH 2/5] improving comment --- tests/functional/event_handler/test_api_middlewares.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/functional/event_handler/test_api_middlewares.py b/tests/functional/event_handler/test_api_middlewares.py index c6a1581391..92c4b4f922 100644 --- a/tests/functional/event_handler/test_api_middlewares.py +++ b/tests/functional/event_handler/test_api_middlewares.py @@ -406,7 +406,7 @@ def dummy_route(): ], ) def test_api_gateway_middleware_with_include_router_prefix(app: EventHandlerInstance, event): - # GIVEN two global middlewares: one for App and one for Router + # GIVEN a router instance router = Router() def app_middleware(app: EventHandlerInstance, next_middleware: NextMiddleware): @@ -416,6 +416,7 @@ def app_middleware(app: EventHandlerInstance, next_middleware: NextMiddleware): return response + # WHEN we register a route with a middleware @router.get("/path", middlewares=[app_middleware]) def dummy_route(): # if the middleware is executed, the context variable will be available @@ -430,6 +431,7 @@ def dummy_route(): app.include_router(router, prefix="/my") # THEN resolving a request must execute the middleware + # THEN return a successful response http 200 status code result = app(event, {}) assert result["statusCode"] == 200 From 8d2a8f0a5660be9c01f8088f59fc925b0073dea8 Mon Sep 17 00:00:00 2001 From: Leandro Damascena Date: Tue, 7 Nov 2023 10:21:53 +0000 Subject: [PATCH 3/5] Addessing Heitor's feedback --- .../event_handler/api_gateway.py | 1 + .../event_handler/test_api_gateway.py | 21 +++++++++++++ .../event_handler/test_api_middlewares.py | 30 ++++++------------- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index 82fb0085ad..0ddf287f26 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -2008,6 +2008,7 @@ def include_router(self, router: "Router", prefix: Optional[str] = None) -> None # use pointer to allow context clearance after event is processed e.g., resolve(evt, ctx) router.context = self.context + # Iterate through the routes defined in the router to configure and apply middlewares for each route for route, func in router._routes.items(): new_route = route diff --git a/tests/functional/event_handler/test_api_gateway.py b/tests/functional/event_handler/test_api_gateway.py index 18d10ec416..77fe49c8ae 100644 --- a/tests/functional/event_handler/test_api_gateway.py +++ b/tests/functional/event_handler/test_api_gateway.py @@ -1197,6 +1197,27 @@ def base(): assert result["multiValueHeaders"]["Content-Type"] == [content_types.APPLICATION_JSON] +def test_api_gateway_app_with_strip_prefix_and_route_prefix(): + # GIVEN a strip_prefix matches the request path + # and a router instance + app = ApiGatewayResolver(strip_prefixes=["/foo"]) + router = Router() + event = {"httpMethod": "GET", "path": "/foo/bar/users", "resource": "/users"} + + @router.get("/users") + def base(): + return {} + + # THEN we can register the router with a prefix + app.include_router(router, prefix="/bar") + + result = app(event, {}) + + # THEN process event correctly + assert result["statusCode"] == 200 + assert result["multiValueHeaders"]["Content-Type"] == [content_types.APPLICATION_JSON] + + def test_api_gateway_app_router(): # GIVEN a Router with registered routes app = ApiGatewayResolver() diff --git a/tests/functional/event_handler/test_api_middlewares.py b/tests/functional/event_handler/test_api_middlewares.py index 92c4b4f922..58bec25907 100644 --- a/tests/functional/event_handler/test_api_middlewares.py +++ b/tests/functional/event_handler/test_api_middlewares.py @@ -397,33 +397,21 @@ def dummy_route(): assert result["statusCode"] == 200 -@pytest.mark.parametrize( - "app, event", - [ - (ApiGatewayResolver(proxy_type=ProxyEventType.APIGatewayProxyEvent), API_REST_EVENT), - (APIGatewayRestResolver(), API_REST_EVENT), - (APIGatewayHttpResolver(), API_RESTV2_EVENT), - ], -) -def test_api_gateway_middleware_with_include_router_prefix(app: EventHandlerInstance, event): - # GIVEN a router instance +def test_api_gateway_middleware_with_include_router_prefix(): + # GIVEN an App and Router instance + app = ApiGatewayResolver() router = Router() def app_middleware(app: EventHandlerInstance, next_middleware: NextMiddleware): - # inject a variable into resolver context + # AND a variable injected into resolver context app.append_context(injected="injected_value") - response = next_middleware(app) - - return response + return next_middleware(app) # WHEN we register a route with a middleware @router.get("/path", middlewares=[app_middleware]) def dummy_route(): - # if the middleware is executed, the context variable will be available - # Otherwise, an exception will be raised - injected_context = app.context["injected"] - - assert injected_context == "injected_value" + # THEN we should have access to the middleware's injected variable + assert app.context["injected"] == "injected_value" return Response(status_code=200, body="works!") @@ -431,8 +419,8 @@ def dummy_route(): app.include_router(router, prefix="/my") # THEN resolving a request must execute the middleware - # THEN return a successful response http 200 status code - result = app(event, {}) + # and return a successful response http 200 status code + result = app(API_REST_EVENT, {}) assert result["statusCode"] == 200 From a9c49ef25b5a2bdcf9d4b048d21a5d67b0afa6a2 Mon Sep 17 00:00:00 2001 From: Leandro Damascena Date: Tue, 7 Nov 2023 10:38:10 +0000 Subject: [PATCH 4/5] Addessing Heitor's feedback --- tests/functional/event_handler/test_api_gateway.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/event_handler/test_api_gateway.py b/tests/functional/event_handler/test_api_gateway.py index 77fe49c8ae..29313824b0 100644 --- a/tests/functional/event_handler/test_api_gateway.py +++ b/tests/functional/event_handler/test_api_gateway.py @@ -1215,7 +1215,6 @@ def base(): # THEN process event correctly assert result["statusCode"] == 200 - assert result["multiValueHeaders"]["Content-Type"] == [content_types.APPLICATION_JSON] def test_api_gateway_app_router(): From b7278a368ec027d24e474ac63ca346f4b6990575 Mon Sep 17 00:00:00 2001 From: Leandro Damascena Date: Tue, 7 Nov 2023 11:09:12 +0000 Subject: [PATCH 5/5] Addessing Heitor's feedback --- .../event_handler/test_api_gateway.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/functional/event_handler/test_api_gateway.py b/tests/functional/event_handler/test_api_gateway.py index 29313824b0..8ad5ac35b1 100644 --- a/tests/functional/event_handler/test_api_gateway.py +++ b/tests/functional/event_handler/test_api_gateway.py @@ -1198,22 +1198,21 @@ def base(): def test_api_gateway_app_with_strip_prefix_and_route_prefix(): - # GIVEN a strip_prefix matches the request path - # and a router instance - app = ApiGatewayResolver(strip_prefixes=["/foo"]) + # GIVEN all routes are stripped from its version e.g., /v1 + app = ApiGatewayResolver(strip_prefixes=["/v1"]) router = Router() - event = {"httpMethod": "GET", "path": "/foo/bar/users", "resource": "/users"} - @router.get("/users") - def base(): - return {} + event = {"httpMethod": "GET", "path": "/v1/users/leandro", "resource": "/users"} - # THEN we can register the router with a prefix - app.include_router(router, prefix="/bar") + @router.get("") + def base(user_id: str): + return {"user": user_id} + # WHEN a router is included prefixing all routes with "/users/" + app.include_router(router, prefix="/users/") result = app(event, {}) - # THEN process event correctly + # THEN route correctly to the registered route after stripping each prefix (global + router) assert result["statusCode"] == 200