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.

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

feat(event_handler): support Header parameter validation in OpenAPI schema #3687

Conversation

leandrodamascena
Copy link
Contributor

@leandrodamascena leandrodamascena commented Jan 31, 2024

Issue number: #3661

Summary

Changes

This pull request aims to enhance the OpenAPI schema by allowing the definition of headers as part of API specifications. An API call may require that custom headers be sent with an HTTP request. OpenAPI lets you define custom request headers as in: header parameters.

Key Changes:
  • Added support for defining custom headers within the OpenAPI schema.
  • Updated the OpenAPI documentation to showcase the usage of custom header parameters.
  • Refactored the tests to reduce the number of the code

User experience

from typing import List

from aws_lambda_powertools.event_handler import APIGatewayRestResolver
from aws_lambda_powertools.event_handler.openapi.params import Header
from aws_lambda_powertools.shared.types import Annotated  
from aws_lambda_powertools.utilities.typing import LambdaContext

app = APIGatewayRestResolver(enable_validation=True)

@app.get("/hello")
def get_hello(header2: Annotated[List[str], Header()], header1: Annotated[str, Header()]):
    print(header2)

def lambda_handler(event: dict, context: LambdaContext) -> dict:
    return app.resolve(event, context)

Checklist

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

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

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.

@leandrodamascena leandrodamascena requested a review from a team as a code owner January 31, 2024 15:54
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 31, 2024
@leandrodamascena leandrodamascena linked an issue Jan 31, 2024 that may be closed by this pull request
2 tasks
@codecov-commenter
Copy link

codecov-commenter commented Jan 31, 2024

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (ced0a3d) 95.53% compared to head (5917ddf) 95.58%.

Files Patch % Lines
...ls/event_handler/middlewares/openapi_validation.py 90.00% 2 Missing ⚠️
.../utilities/data_classes/api_gateway_proxy_event.py 84.61% 1 Missing and 1 partial ⚠️
...lambda_powertools/utilities/data_classes/common.py 66.66% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3687      +/-   ##
===========================================
+ Coverage    95.53%   95.58%   +0.04%     
===========================================
  Files          215      215              
  Lines        10088    10136      +48     
  Branches       862     1865    +1003     
===========================================
+ Hits          9638     9688      +50     
+ Misses         340      339       -1     
+ Partials       110      109       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@leandrodamascena leandrodamascena marked this pull request as draft January 31, 2024 16:21
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jan 31, 2024
@pull-request-size pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 1, 2024
Copy link
Contributor

github-actions bot commented Feb 1, 2024

⚠️Large PR detected⚠️

Please consider breaking into smaller PRs to avoid significant review delays. Ignore if this PR has naturally grown to this size after reviews.

@leandrodamascena leandrodamascena marked this pull request as ready for review February 1, 2024 12:53
@leandrodamascena
Copy link
Contributor Author

leandrodamascena commented Feb 1, 2024

@rubenfonseca this PR is ready to review. I want to highlight one point that you may want to change/refactor.

According to the RFC, headers are case insensitive, so I lower the header names when resolving them in the DataClass and lower the parameter name as well when validating the OpenAPI schema. Even this is against Python standards, but a customer can create a function like this:

@app.get("/hello")
def get(
    HeADer1: Annotated[
        str,
        Header(
            description="This is multi value header parameter.",
        ),
    ],
):
    """Return validated multi-value header values."""
    return HeADer1

We must be able to compare "HeADer1" with the header name "header1" or "Heder1" or "HEader1".

Thanks

@leandrodamascena leandrodamascena changed the title feat(event_handler): Add support for Headers parameters and validation in OpenAPI schema feat(event_handler): Add support for Headers parameters validation in OpenAPI schema Feb 1, 2024
Copy link
Contributor

github-actions bot commented Feb 1, 2024

⚠️Large PR detected⚠️

Please consider breaking into smaller PRs to avoid significant review delays. Ignore if this PR has naturally grown to this size after reviews.

Copy link
Contributor

@rubenfonseca rubenfonseca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great work, I only need to check the documentation now.
As we spoke during peer review, try to see if we can move the .lower inside the Header's class alias setter. This way we can remove some if/elses in the code and streamline the feature.
Great work Leandro!

Copy link
Contributor

@rubenfonseca rubenfonseca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job with the code, much cleaner! Just left some final comments on the documentation.

docs/core/event_handler/api_gateway.md Outdated Show resolved Hide resolved
docs/core/event_handler/api_gateway.md Outdated Show resolved Hide resolved
docs/core/event_handler/api_gateway.md Outdated Show resolved Hide resolved
docs/core/event_handler/api_gateway.md Outdated Show resolved Hide resolved
docs/core/event_handler/api_gateway.md Outdated Show resolved Hide resolved
docs/core/event_handler/api_gateway.md Outdated Show resolved Hide resolved
docs/core/event_handler/api_gateway.md Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Feb 1, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@rubenfonseca rubenfonseca changed the title feat(event_handler): Add support for Headers parameters validation in OpenAPI schema feat(event_handler): support Header parameter validation in OpenAPI schema Feb 1, 2024
@leandrodamascena leandrodamascena merged commit 33820d1 into aws-powertools:develop Feb 1, 2024
18 checks passed
@leandrodamascena leandrodamascena deleted the openapi-headers-and-cookies branch February 1, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation event_handlers openapi-schema size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Add support for headers parameters in OpenAPI schema
3 participants