feat(apigateway): ignore trailing slashes in routes (APIGatewayRestResolver)

[walmsles]

Issue number: #1552

Summary

Ignore trailing slash for routers inheriting from APIGatewayResolver

Changes

AWS API Gateway routes the following 2 paths in the same way "/my/path", "/my/path/", i.e. when routing "/my/path/" it routes exactly as if "/my/path" had been called. The Lambda Powertools router currently treats these as 2 distinct paths so "/my/path/" returns HTTP statusCode of 404.

User experience

No change to user experience. Users will create routes as normal. In the given sample below making API gateway call to path "/my/path/" will correctly ensure the "get_lambda" route function is correctly called even though it is defined as "/my/path"

app = APIGatewayRestResolver()

    @app.get("/my/path")
    def get_lambda() -> Response:
        assert isinstance(app.current_event, APIGatewayProxyEvent)
        assert app.lambda_context == {}
        assert app.current_event.request_context.domain_name == "id.execute-api.us-east-1.amazonaws.com"
        return Response(200, content_types.APPLICATION_JSON, json.dumps({"foo": "value"}))

The code below will work in the same way when called using path "/my/path", "/my/path/"

app = APIGatewayRestResolver()

    @app.get("/my/path/")
    def get_lambda() -> Response:
        assert isinstance(app.current_event, APIGatewayProxyEvent)
        assert app.lambda_context == {}
        assert app.current_event.request_context.domain_name == "id.execute-api.us-east-1.amazonaws.com"
        return Response(200, content_types.APPLICATION_JSON, json.dumps({"foo": "value"}))

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• Meet tenets criteria
• I have performed a self-review of this change
• Changes have been tested
• Changes are documented
• PR title follows conventional commit semantics

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

---

View rendered docs/core/event_handler/api_gateway.md

---

View rendered MAINTAINERS.md
View rendered README.md
View rendered docs/core/event_handler/api_gateway.md
View rendered docs/core/tracer.md
View rendered docs/index.md
View rendered docs/upgrade.md
View rendered docs/utilities/batch.md
View rendered docs/utilities/idempotency.md
View rendered docs/utilities/parser.md
View rendered docs/utilities/validation.md

[walmsles]

I don't get why there are so many conflicts - my actual changes are not that large - will look at rebasing and see if that lowers the madness level.

[rubenfonseca]

@walmsles really sorry about this. As we are getting ready to release v2, we rebased the entire branch from the develop branch. Is it possible for you to re-create the PR, cherry-picking the commits into a new v2 fork? We really appreciate your time and effort on this!

[walmsles]

ahhhh, so it's not me, after all, 👀 - I just assumed I was failing hard today 🤣. No problem - My changes are actually across 2 commits so its very self contained. Been learning all along so it's all good - not a problem.

[walmsles]

I have hard reset my fork branches to develop/V2 from upstream and re-applied my changes so will see if can edit this PR and make the adjustment and see what it looks like or you think create a new fork is required?

[rubenfonseca]

You can definitely force push into this PR! Thank you so much :)

[walmsles]

You can definitely force push into this PR! Thank you so much :)

Thankyou 😄 I thought I was going crazy!

[walmsles]

Okay - have managed to wrangle my branches into a fit state of matching upstream now so this is now more sane.

Please refer to this comment on prior PR.

the TL;DR version:

I have decided to restrict the implementation of this feature to REST API Gateway (V1) since this is the default behaviour of this gateway implementation for a defined route i.e. both "/hello" and "/hello/" resolve to the same lambda integration. I will write E2E tests covering all the API behaviours around this feature so if anything changes it will be picked up.

I have added E2E tests for ALL the API routing implementations on AWS around the behaviour.

(👏 👏 👏 on E2E testing efforts - these are amazing!!!)

[heitorlessa]

Awesome!! Let me know when it's ready to review @walmsles. We're looking to close all V2 activities by tomorrow, and start writing the What's New (new features, non-backward compatible changes, etc.)

[walmsles]

Awesome!! Let me know when it's ready to review @walmsles. We're looking to close all V2 activities by tomorrow, and start writing the What's New (new features, non-backward compatible changes, etc.)

Hi @heitorlessa, this is ready for review. I am about to board a flight and will be back online in a couple of hours.

[heitorlessa]

Awesome, looking into this now!

[heitorlessa]

Thanks a lot for getting this implemented fast! I'm torn on whether we need an actual route + registration, since we're only testing the additional slash / at the end.

I've added some other minor suggestions you can either accept or drop.

[codecov-commenter]

Codecov Report

Base: 99.38% // Head: 99.40% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (ce588d3) compared to base (4b76eca).
Patch coverage: 91.58% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##               v2    #1609      +/-   ##
==========================================
+ Coverage   99.38%   99.40%   +0.01%     
==========================================
  Files         125      127       +2     
  Lines        5731     5842     +111     
  Branches      359      367       +8     
==========================================
+ Hits         5696     5807     +111     
+ Misses         18       17       -1     
- Partials       17       18       +1     

Impacted Files	Coverage Δ
aws_lambda_powertools/tracing/extensions.py	100.00% <ø> (ø)
aws_lambda_powertools/utilities/batch/__init__.py	100.00% <ø> (ø)
...ws_lambda_powertools/utilities/batch/exceptions.py	100.00% <ø> (ø)
...a_powertools/utilities/parser/envelopes/kinesis.py	100.00% <ø> (ø)
aws_lambda_powertools/shared/cookies.py	58.69% <58.69%> (ø)
...bda_powertools/utilities/data_classes/alb_event.py	90.47% <66.66%> (ø)
...ambda_powertools/utilities/parameters/appconfig.py	94.28% <86.66%> (ø)
aws_lambda_powertools/utilities/batch/base.py	97.79% <87.50%> (-1.42%)	⬇️
aws_lambda_powertools/event_handler/api_gateway.py	100.00% <100.00%> (ø)
aws_lambda_powertools/event_handler/appsync.py	100.00% <100.00%> (ø)
... and 41 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[heitorlessa]

LGTM @walmsles. As it's late in ANZ, happy to send the docs and merge it