Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature request: HTTP resolver middleware should have access to path parameters #4609

Open
2 tasks done
kimsappi opened this issue Jun 23, 2024 · 8 comments
Open
2 tasks done

Feature request: HTTP resolver middleware should have access to path parameters #4609

kimsappi opened this issue Jun 23, 2024 · 8 comments
Labels
feature-request feature request help wanted Could use a second pair of eyes/hands

Comments

@kimsappi
Copy link

kimsappi commented Jun 23, 2024 •
edited
Loading

Use case

HTTP resolver middlewares should have access to path parameters to help with use cases like resource-specific authnz, logging enrichment, etc. Access is already available in an undocumented manner that may be considered "private" according to PEP 8 (see "User Experience" below).

Solution/User Experience

Current:

def middleware(app: APIGatewayRestResolver, next_middleware: NextMiddleware) -> Response:
    path_params = app.context.get("_route_args")
    logger.append_keys(path_params=path_params)
    return next_middleware(app)

New:

def middleware(app: APIGatewayRestResolver, next_middleware: NextMiddleware) -> Response:
    path_params = app.context.path_parameters
    logger.append_keys(path_params=path_params)
    return next_middleware(app)

Alternative solutions

Perhaps it would be ideal to have the parsed path parameters available via app.current_event instead (as with API Gateway resolvers), but using app.context is certainly easier and more in line with the current state. Providing data in the powertools app.current_event that is not present in a typical Lambda handler's event could also lead to confusion.

Acknowledgment
@kimsappi kimsappi added feature-request feature request triage Pending triage from maintainers labels Jun 23, 2024
@boring-cyborg boring-cyborg
Copy link

boring-cyborg bot commented Jun 23, 2024

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@kimsappi
Copy link
Author

Here's one solution for implementation (I can make a PR if this is deemed a valuable suggestion, tips for how to document it welcome):

diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py
index 7c4d6769..4cbd2bd8 100644
--- a/aws_lambda_powertools/event_handler/api_gateway.py
+++ b/aws_lambda_powertools/event_handler/api_gateway.py
@@ -423,7 +423,7 @@ class Route:
             print("=================")
 
         # Add Route Arguments to app context
-        app.append_context(_route_args=route_arguments)
+        app.context["_route_args"] = route_arguments
 
         # Call the Middleware Wrapped _call_stack function handler with the app
         return self._middleware_stack(app)
@@ -898,10 +898,17 @@ class ResponseBuilder(Generic[ResponseEventT]):
         }
 
 
+class RouterContext(dict):
+    @property
+    def path_parameters(self) -> Dict[str, Any]:
+        """Get path parameters for the current request."""
+        return self.get("_route_args", {})
+
+
 class BaseRouter(ABC):
     current_event: BaseProxyEvent
     lambda_context: LambdaContext
-    context: dict
+    context: RouterContext
     _router_middlewares: List[Callable] = []
     processed_stack_frames: List[str] = []
 
@@ -1317,6 +1324,11 @@ class BaseRouter(ABC):
 
     def append_context(self, **additional_context):
         """Append key=value data as routing context"""
+        if "_route_args" in additional_context:
+            warnings.warn(
+                "The '_route_args' context key is used internally by powertools and should not be overwrit
ten",
+                stacklevel=2,
+            )
         self.context.update(**additional_context)
 
     def clear_context(self):
@@ -1421,7 +1433,7 @@ def _registered_api_adapter(
         The API Response Object
 
     """
-    route_args: Dict = app.context.get("_route_args", {})
+    route_args: Dict = app.context.path_parameters
     logger.debug(f"Calling API Route Handler: {route_args}")
     return app._to_response(next_middleware(**route_args))
 
@@ -1494,7 +1506,7 @@ class ApiGatewayResolver(BaseRouter):
         self._debug = self._has_debug(debug)
         self._enable_validation = enable_validation
         self._strip_prefixes = strip_prefixes
-        self.context: Dict = {}  # early init as customers might add context before event resolution
+        self.context = RouterContext()  # early init as customers might add context before event resolutio
n
         self.processed_stack_frames = []
         self._response_builder_class = ResponseBuilder[BaseProxyEvent]
 
@@ -2453,7 +2465,7 @@ class Router(BaseRouter):
         self._routes: Dict[tuple, Callable] = {}
         self._routes_with_middleware: Dict[tuple, List[Callable]] = {}
         self.api_resolver: Optional[BaseRouter] = None
-        self.context = {}  # early init as customers might add context before event resolution
+        self.context = RouterContext()  # early init as customers might add context before event resolutio
n
         self._exception_handlers: Dict[Type, Callable] = {}
 
     def route(
diff --git a/tests/functional/event_handler/required_dependencies/test_router.py b/tests/functional/event_h
andler/required_dependencies/test_router.py
index d96f5035..b6111017 100644
--- a/tests/functional/event_handler/required_dependencies/test_router.py
+++ b/tests/functional/event_handler/required_dependencies/test_router.py
@@ -1,16 +1,23 @@
+import pytest
+
 from aws_lambda_powertools.event_handler import (
     ALBResolver,
     APIGatewayHttpResolver,
+    ApiGatewayResolver,
     APIGatewayRestResolver,
     LambdaFunctionUrlResolver,
     Response,
 )
+from aws_lambda_powertools.event_handler.middlewares import NextMiddleware
+from aws_lambda_powertools.event_handler.openapi.params import Path
 from aws_lambda_powertools.event_handler.router import (
     ALBRouter,
     APIGatewayHttpRouter,
     APIGatewayRouter,
     LambdaFunctionUrlRouter,
+    Router,
 )
+from aws_lambda_powertools.shared.types import Annotated
 from aws_lambda_powertools.utilities.data_classes import (
     ALBEvent,
     APIGatewayProxyEvent,
@@ -74,3 +81,71 @@ def test_lambda_function_url_router_event_type():
     app.include_router(router)
     result = app(load_event("lambdaFunctionUrlEvent.json"), {})
     assert result["body"] == "routed"
+
+
+@pytest.mark.parametrize(
+    "router,resolver,event_file",
+    [
+        (ALBRouter, ALBResolver, "albEvent.json"),
+        (APIGatewayRouter, APIGatewayRestResolver, "apiGatewayProxyEvent.json"),
+        (APIGatewayHttpRouter, APIGatewayHttpResolver, "apiGatewayProxyV2Event_GET.json"),
+        (LambdaFunctionUrlRouter, LambdaFunctionUrlResolver, "lambdaFunctionUrlEvent.json"),
+    ],
+)
+def test_path_parameters_in_context(
+    router: Router,
+    resolver: ApiGatewayResolver,
+    event_file: str,
+) -> None:
+    app = resolver(enable_validation=True)
+    router = router()
+    path_params = {
+        "str_param": "str_value",
+        "int_param": 3,
+    }
+
+    def bar(app: APIGatewayRestResolver, next_middleware: NextMiddleware) -> Response[str]:
+        assert app.context.path_parameters == path_params
+        return next_middleware(app)
+
+    @router.route(rule="/<str_param>/<int_param>", method=["GET"], middlewares=[bar])
+    def foo(str_param: Annotated[str, Path()], int_param: Annotated[int, Path()]) -> Response[str]:
+        return Response(status_code=200, body="routed")
+
+    app.include_router(router)
+    event = load_event(event_file)
+    event["path"] = event["rawPath"] = f"/{path_params['str_param']}/{path_params['int_param']}"
+    result = app(event, {})
+    assert result["body"] == "routed"
+
+
+@pytest.mark.parametrize(
+    "router,resolver,event_file",
+    [
+        (ALBRouter, ALBResolver, "albEvent.json"),
+        (APIGatewayRouter, APIGatewayRestResolver, "apiGatewayProxyEvent.json"),
+        (APIGatewayHttpRouter, APIGatewayHttpResolver, "apiGatewayProxyV2Event_GET.json"),
+        (LambdaFunctionUrlRouter, LambdaFunctionUrlResolver, "lambdaFunctionUrlEvent.json"),
+    ],
+)
+def test_path_parameters_static_path(
+    router: Router,
+    resolver: ApiGatewayResolver,
+    event_file: str,
+) -> None:
+    app = resolver(enable_validation=True)
+    router = router()
+
+    def bar(app: APIGatewayRestResolver, next_middleware: NextMiddleware) -> Response[str]:
+        assert app.context.path_parameters == {}
+        return next_middleware(app)
+
+    @router.route(rule="/static", method=["GET"], middlewares=[bar])
+    def foo() -> Response[str]:
+        return Response(status_code=200, body="routed")
+
+    app.include_router(router)
+    event = load_event(event_file)
+    event["path"] = event["rawPath"] = "/static"
+    result = app(event, {})
+    assert result["body"] == "routed"

@heitorlessa
Copy link
Contributor

heitorlessa commented Jun 23, 2024 via email

@kimsappi
Copy link
Author

Hey Kim, did you face any issues accessing via the app.current_event within the middleware? The app instance is the same for both middlewares and routes by design.

No, there is no issue with that. The problem is that I would like to have access to the parsed path parameters in middlewares. This is available in app.current_event for API Gateway, where it's a native feature of API Gateway and part of the Lambda event ("pathParameters"): https://docs.aws.amazon.com/lambda/latest/dg/services-apigateway.html#apigateway-example-event

However, other resolvers like ALB don't have this luxury since routing is handled exclusively with ALBResolver and path parameters aren't part of the Lambda event: https://docs.aws.amazon.com/lambda/latest/dg/services-alb.html

In a route, path parameters can be accessed like this:

@router.route(rule="/<str_param>/<int_param>", method=["GET"], middlewares=[bar])
def foo(str_param: Annotated[str, Path()], int_param: Annotated[int, Path()]) -> Response[str]:
    return Response(status_code=200, body=f"{str_param}/{int_param}")

But the middleware bar would, as far as I'm aware, need to parse them from the raw path or use the undocumented app.context["_route_args"] (unless using an API Gateway resolver).

@heitorlessa
Copy link
Contributor

heitorlessa commented Jun 23, 2024 via email

@heitorlessa heitorlessa removed the triage Pending triage from maintainers label Jun 24, 2024
@heitorlessa heitorlessa moved this from Triage to Backlog in Powertools for AWS Lambda (Python) Jun 24, 2024
@heitorlessa heitorlessa added the help wanted Could use a second pair of eyes/hands label Jun 24, 2024
@kimsappi
Copy link
Author

I think that sounds very good. I wasn't able to find the issue right now, but I'll keep an eye out. Perhaps it doesn't make sense to implement a temporary solution if a better solution is being planned. Maybe it's best to close this ticket?

@heitorlessa heitorlessa mentioned this issue Jun 24, 2024
Closed
2 tasks
@heitorlessa
Copy link
Contributor

Turns out the issue I mentioned was the precursor to have our new Data Validation feature (#1955) - we inject request Payloads, Headers, Query String, Path parameters, etc. inferring from type annotation.

We should keep this issue altogether. Unless @leandrodamascena disagrees, I think we should add this feature into the data validation middleware only to not confuse customers with the DX. If we see a Request model, we inject it.

If we stick by WSGI "standard", we can keep it simple like Starlette while handling multi-value query string and multi-value headers.

@leandrodamascena
Copy link
Contributor

We can discuss this when you get back from the PTO @heitorlessa!

@leandrodamascena leandrodamascena self-assigned this Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature-request feature request help wanted Could use a second pair of eyes/hands
Projects
Powertools for AWS Lambda (Python)
Status: Backlog
Milestone
No milestone
Development

No branches or pull requests
3 participants
@heitorlessa @leandrodamascena @kimsappi