Skip to content

Conversation

oyiz-michael
Copy link
Contributor

@oyiz-michael oyiz-michael commented Aug 6, 2025

Issue number: #7124

closes #7124

Summary

This PR adds comprehensive File parameter support for handling file uploads in multipart/form-data requests within the AWS Lambda Powertools Python Event Handler with OpenAPI validation.

Changes

  • Added File class in aws_lambda_powertools/event_handler/openapi/params.py

    • New parameter type specifically for file uploads
    • Inherits from Form with format: binary in OpenAPI schema
    • Supports validation constraints (max_length, etc.)
  • Enhanced multipart parsing in aws_lambda_powertools/event_handler/middlewares/openapi_validation.py

    • Added _parse_multipart_data method for parsing multipart/form-data
    • WebKit boundary support for Safari/Chrome compatibility
    • Base64 decoding support for Lambda event handling
    • Distinguishes between file fields and form fields
  • Comprehensive test suite with 13 test scenarios covering:

    • Basic file uploads and multiple file handling
    • File + form data combinations
    • WebKit boundary parsing and base64 encoded content
    • Validation constraints and error handling
    • Optional file parameters
  • Complete usage example in examples/event_handler_rest/src/file_parameter_example.py

User experience

Before: Users could not handle file uploads in multipart/form-data requests with OpenAPI validation. They had to manually parse request bodies or disable validation entirely.

After: Users can now use type-annotated File parameters that automatically:

  • Parse multipart/form-data file uploads
  • Generate proper OpenAPI schema with format: binary
  • Apply validation constraints
  • Work seamlessly with existing form parameters
from typing import Annotated
from aws_lambda_powertools.event_handler import APIGatewayRestResolver
from aws_lambda_powertools.event_handler.openapi.params import File, Form

app = APIGatewayRestResolver(enable_validation=True)

@app.post("/upload")
def upload_file(
    file: Annotated[bytes, File(description="File to upload", max_length=1000000)],
    title: Annotated[str, Form(description="File title")]
):
    return {"file_size": len(file), "title": title, "status": "uploaded"}

Checklist

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

Is this a breaking change? RFC issue number: N/A

This is not a breaking change - it's a new feature addition that doesn't modify existing functionality.

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.

…ploads

- Add public File parameter class extending _File
- Support multipart/form-data parsing with WebKit boundary compatibility
- OpenAPI schema generation with format: binary for file uploads
- Enhanced dependant logic to handle File + Form parameter combinations
- Clean implementation based on upstream develop branch

Changes:
- params.py: Add File(_File) public class with proper documentation
- dependant.py: Add File parameter support in body field info logic
- openapi_validation.py: Add multipart parsing with boundary detection
- test_file_form_validation.py: Basic test coverage for File parameters

This provides customers with File parameter support using the same
pattern as Query, Path, Header parameters with Annotated types.
- Add File parameter class in openapi/params.py with binary format schema
- Implement comprehensive multipart/form-data parsing in openapi_validation.py
  * Support for WebKit and standard boundary formats
  * Base64-encoded request handling for AWS Lambda
  * Mixed file and form data parsing
- Update dependant.py to handle File parameters in body field resolution
- Add comprehensive test suite (13 tests) covering:
  * Basic file upload parsing and validation
  * WebKit boundary format support
  * Base64-encoded multipart data
  * Multiple file uploads
  * File size constraints validation
  * Optional file parameters
  * Error handling for invalid boundaries and missing files
- Add file_parameter_example.py demonstrating various File parameter use cases
- Clean up unnecessary imports and pragma comments

Resolves file upload functionality with full OpenAPI schema generation and validation support.
@oyiz-michael oyiz-michael requested a review from a team as a code owner August 6, 2025 22:12
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 6, 2025
- Break down _parse_multipart_data method into smaller helper methods
- Reduce cognitive complexity from 43 to under 15 per SonarCloud requirement
- Improve code readability and maintainability
- All existing tests continue to pass

Helper methods created:
- _decode_request_body: Handle base64 decoding
- _extract_boundary_bytes: Extract multipart boundary
- _parse_multipart_sections: Parse sections into data dict
- _parse_multipart_section: Handle individual section parsing
- _split_section_headers_and_content: Split headers/content
- _decode_form_field_content: Decode form field as string

Addresses SonarCloud cognitive complexity violation while maintaining
all existing functionality for File parameter multipart parsing.
@oyiz-michael oyiz-michael changed the title Feature/file parameter clean feat: add File parameter support for multipart/form-data uploads Aug 6, 2025
@github-actions github-actions bot added the feature New feature or functionality label Aug 6, 2025
@oyiz-michael oyiz-michael changed the title feat: add File parameter support for multipart/form-data uploads feat(event_handler): add File parameter support for multipart/form-data uploads in OpenAPI utility Aug 6, 2025
- Add missing __future__ annotations imports
- Remove unused pytest imports from test files
- Remove unused json import from example
- Fix line length violations in test files
- All File parameter tests continue to pass (13/13)

Addresses ruff linting violations:
- FA102: Missing future annotations for PEP 604 unions
- F401: Unused imports
- E501: Line too long violations
@leandrodamascena
Copy link
Contributor

Hi @oyiz-michael, I see you are working on this PR and please let me know when you need a first round of review or any help.

@leandrodamascena leandrodamascena linked an issue Aug 7, 2025 that may be closed by this pull request
2 tasks
- Replace bytes | None with Union[bytes, None] for broader compatibility
- Replace str | None with Union[str, None] in examples
- Add noqa: UP007 comments to suppress linter preference for newer syntax
- Ensures compatibility with Python environments that don't support PEP 604 unions
- Fixes test failure: 'Unable to evaluate type annotation bytes | None'

All File parameter tests continue to pass (13/13) across Python versions.
Copy link

codecov bot commented Aug 7, 2025

Codecov Report

❌ Patch coverage is 97.57785% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.40%. Comparing base (4a0269e) to head (1b307f8).
⚠️ Report is 11 commits behind head on develop.

Files with missing lines Patch % Lines
..._lambda_powertools/event_handler/openapi/params.py 96.36% 1 Missing and 3 partials ⚠️
...ls/event_handler/middlewares/openapi_validation.py 97.29% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7132      +/-   ##
===========================================
+ Coverage    96.37%   96.40%   +0.02%     
===========================================
  Files          275      276       +1     
  Lines        13048    13295     +247     
  Branches       974     1019      +45     
===========================================
+ Hits         12575    12817     +242     
- Misses         366      368       +2     
- Partials       107      110       +3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@oyiz-michael
Copy link
Contributor Author

Hi @oyiz-michael, I see you are working on this PR and please let me know when you need a first round of review or any help.

@leandrodamascena fixing some failing test and should be ready for a review and feed back

@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 Aug 7, 2025
@leandrodamascena
Copy link
Contributor

Hi @oyiz-michael, a quick tip: run make pr in your local environment and then you can catch errors before committing and pushing the files.

…t references in OpenAPI schemas

- Add upload_file_fix.py module to detect and generate missing component schemas
- Integrate fix into APIGatewayRestResolver.get_openapi_schema() method
- Create comprehensive tests for UploadFile OpenAPI schema validation
- Add examples demonstrating the fix functionality
- Ensure generated schemas pass Swagger Editor validation
- Fix issue where UploadFile annotations created schema references without corresponding components
- All tests passing (546 passed, 9 skipped)

Fixes: Missing component references like #/components/schemas/aws_lambda_powertools__event_handler__openapi__compat__Body_*
Resolves: OpenAPI schema validation failures when using UploadFile annotations
- Refactored get_openapi_schema method to reduce cognitive complexity from 27 to 15
  - Split into 7 helper methods for better maintainability
  - Enhanced error handling and code organization

- Refactored find_missing_component_references to reduce complexity from 23 to 15
  - Split into 5 helper methods with proper separation of concerns
  - Fixed null pointer bug in _get_existing_schemas function

- Reorganized examples directory structure
  - Moved upload file examples to examples/event_handler/
  - Updated documentation and imports accordingly

- All OpenAPI tests passing (220/220)
- Refactored _resolve_openapi_config method to reduce complexity from 17 to 15
  - Split into 4 helper methods for better maintainability
  - Improved separation of concerns for config resolution logic

- Refactored get_openapi_schema method in example file to reduce complexity from 23 to 15
  - Split into 6 helper methods with clear responsibilities
  - Enhanced readability and maintainability of schema fixing logic

- All OpenAPI tests continue to pass (220/220)
- Example functionality validated and working correctly
Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

Hi @oyiz-michael! Thank you so much for all your hard work on this. I've been testing this code extensively, and it seems to be working as expected, but I have a few comments that might be helpful for you when iterating with the GenAI tool.

1/ The code is missing a docstring in general. You've created new methods that are somewhat difficult to read and understand without proper comments/docstring. Can you improve this?

2/ I left a comment about the upload_file_fix.py file.

3/ The tests are very verbose and duplicated. Can you iterate to improve this?

Really thanks for all the effort, I'm super excited to merge this when we address all the things.

leandrodamascena and others added 4 commits August 29, 2025 10:19
- Fixed Pydantic core schema generation for UploadFile class
- Updated JSON schema method for better OpenAPI compatibility
- Added proper type annotations for mypy compliance
- Maintains all existing functionality while fixing schema generation issues

Addresses reviewer feedback on code quality and type safety.
oyiz-michael and others added 3 commits September 1, 2025 20:59
- Replaced 2 verbose test files (434 lines) with 1 concise file (92 lines)
- Eliminated duplication and verbose helper methods
- Maintained 100% coverage of missing lines identified by codecov
- Reduced test count from 23 to 7 comprehensive tests
- All tests passing with improved organization and clarity

Addresses reviewer comment about verbose and duplicated tests
@oyiz-michael
Copy link
Contributor Author

Hi @leandrodamascena I have addressed feedback about verbose and duplicated tests, coverage of the missing lines identified by codecov.
All tests passing with Improved code quality with better organization and clarity.

I really appreciate your patience and effort !

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

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

Hey @oyiz-michael the CI is failing with some problem in format. Please make sure to add in your workflow make format and make pr before pushing a new commit, in this way you can catch early bugs.

Thanks

oyiz-michael and others added 3 commits September 3, 2025 10:08
- Remove unused pytest import
- Fix type comparison issues using 'is' instead of '==' for type checks
- Code now passes ruff format and lint checks

Addresses CI formatting failures in test_uploadfile_coverage.py
…rements

- Replaced verbose/duplicated tests with comprehensive single test file
- Added 21 comprehensive test methods covering UploadFile validation, schema generation, and helper functions
- Covers UploadFile._validate_with_info with OpenAPI generation context
- Tests UploadFile validation methods (_validate, __get_validators__)
- Covers JSON schema generation (__get_pydantic_json_schema__, __modify_schema__)
- Tests fix_upload_file_schema_references and helper functions
- Includes edge cases for endpoint extraction and component title generation
- Addresses reviewer feedback about verbose/duplicated tests
- Targets 96.36% codecov patch coverage requirement
oyiz-michael and others added 6 commits September 6, 2025 10:41
- Replaced 'assert file_param is not None' with meaningful assertions
- Check file_param.description and hasattr for json_schema_extra
- Addresses SonarCloud quality issue S5727: comparison to None should not be constant
- Improves test meaningfulness and code maintainability
- Added tests for WebKit boundary extraction in multipart parsing
- Added base64 decoding error handling test coverage
- Added multipart section name extraction tests
- Added AttributeError handling tests for field value extraction
- Targets specific missing lines identified in codecov patch report
- Increases total test count from 21 to 25 comprehensive tests
- All tests pass with proper formatting and linting validation
- Addresses codecov target of 96.36% diff coverage
- Removed 'mock_body is not None and value is None' redundant condition
- Simplified test to directly test AttributeError handling scenario
- Addresses linter warning about identity check that will always be True
- Maintains same test coverage while improving code quality
- Follows proper make format && make pr workflow validation

Not all issues are linked correctly.

Please link each issue to the PR either manually or using a closing keyword in the format fixes #<issue-number> format.

If mentioning more than one issue, separate them with commas: i.e. fixes #<issue-number-1>, closes #<issue-number-2>.

Copy link

@leandrodamascena
Copy link
Contributor

Hi @oyiz-michael, a quick update here.

We have several PRs making changes that directly affect the OpenAPI and the event handler, two critical utilities we have, and we can't merge all the code at once because if there's a regression, we can easily roll it back. Our coverage is good, we have e2e tests, but we never know all the edge cases.

So, the order in which I'll be merging the PRs is:

1/ Today I merged this PR #7227 - and it will be included in the next release on September 25th.

2/ I'll work to merge this PR #7253 sometime after the 25th and include it in the release on October 7th.

3/ After October 7th, I will work to merge this PR and include it in the release on October 21st.

Thank you very much for your work and patience. I know as a OpenSource contributor sometimes we want to have the code in production, but we care a lot do not to introduce breaking changes to our customers, and this PR of yours is a bit more complex than the others.

@leandrodamascena leandrodamascena added on-hold This item is on-hold and will be revisited in the future do-not-merge labels Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge event_handlers feature New feature or functionality on-hold This item is on-hold and will be revisited in the future size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tests
Projects
None yet
2 participants