-
Notifications
You must be signed in to change notification settings - Fork 450
feat(event_handler): add File parameter support for multipart/form-data uploads in OpenAPI utility #7132
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
base: develop
Are you sure you want to change the base?
feat(event_handler): add File parameter support for multipart/form-data uploads in OpenAPI utility #7132
Conversation
…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.
- 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.
- 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
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. |
- 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.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #7132 +/- ##
===========================================
- Coverage 96.35% 96.20% -0.16%
===========================================
Files 275 275
Lines 12980 13118 +138
Branches 965 990 +25
===========================================
+ Hits 12507 12620 +113
- Misses 366 384 +18
- Partials 107 114 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@leandrodamascena fixing some failing test and should be ready for a review and feed back |
Hi @oyiz-michael, a quick tip: run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey hey @oyiz-michael this is a super nice PR! I was playing with this code with a real Lambda and is working as expect, but I think we need to improve when customers are uploading files..
I left a comment to investigate. Lets see if we can improve it without breaking the current implementation.
- Add FastAPI-inspired UploadFile class with filename, content_type, size, headers properties - Enhance multipart parser to extract and preserve file metadata from Content-Disposition headers - Implement automatic type resolution for backward compatibility with existing bytes-based File parameters - Add comprehensive Pydantic schema validation for UploadFile class - Include 6 comprehensive test cases covering metadata access, backward compatibility, and file reconstruction scenarios - Update official example to showcase both new UploadFile and legacy bytes approaches - Maintain 100% backward compatibility - existing bytes code works unchanged - Address @leandrodamascena feedback about file reconstruction capabilities in Lambda environments Fixes: File parameter enhancement for metadata access in AWS Lambda file uploads
…_to_args - Extract helper functions to reduce cognitive complexity from 24 to under 15 - _get_field_location: Extract field location logic - _get_field_value: Extract value retrieval logic with error handling - _resolve_field_type: Extract Union type resolution logic - _convert_value_type: Extract UploadFile/bytes conversion logic - Maintain all existing functionality and test coverage - Improve code readability and maintainability
Hi @oyiz-michael I'll take a look at the changes on Monday. Thanks |
Hey @oyiz-michael I had some internal work to do and I need more days to finish my review here. Thanks for the patience. |
Hey @oyiz-michael a quick heads up! I see the code to upload files is working as expected, but the new class to implement UploadFile is not working and idk why, I'm still debugging the code. |
I would also have a look to fix the issue. Thanks for the review |
Hello @leandrodamascena Thanks for the heads up! I've just run extensive tests on the UploadFile implementation and everything appears to be working correctly on my end. Here's what I verified: All tests passing: The 6 comprehensive UploadFile tests are all green Quick debugging checklist: Make sure you're importing from the correct module: aws_lambda_powertools.event_handler.openapi.params Example usage: @app.post("/upload") Could you share the specific error message or behavior you're seeing? That would help me pinpoint what might be different in your setup. |
Hey @oyiz-michael! Days were super busy here but now I'm focusing on this PR! I see tests are passing, but I think it's missing a test to create the OpenAPI schema using the new
![]() The OpenAPI schema doesn't render well and it will fail the API call. Can you take a look pls? |
@leandrodamascena! I really appreciate your patience and effort in making this happen! I've fixed the OpenAPI schema generation for UploadFile. Here's what I did: Fixed UploadFile's JSON schema generation to properly work with Pydantic v2 Let me know if you need any other changes? |
@oyiz-michael the problem persist! Can you please generate the OpenAPI Schema and try to validate it using https://editor.swagger.io/? The problema is: when you annotate the function using
|
@leandrodamascena! I've successfully fixed the OpenAPI schema issue with UploadFile annotations! Problem Summary: You were absolutely right - when using UploadFile, the OpenAPI schema was generating references like #/components/schemas/aws_lambda_powertools__event_handler__openapi__compat__Body_upload_file_with_metadata_upload_with_metadata_post-Input__1 that didn't exist in the components section, breaking schema validation. Solution Implemented: I've created a comprehensive fix that: Automatically detects missing component references in OpenAPI schemas Key Files Added/Modified: upload_file_fix.py - Core fix implementation All existing tests pass (546 passed) Thanks for the detailed bug report - it helped me target the exact issue! |
…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
There was a problem hiding this 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @oyiz-michael! I see this file was created to fix the upload class and thats nice, but I think the name is not good and also I think it can be incorporated as part of the code, and not a isolated file. Can you pls try to add it in the params
file? I think we can have a better code doing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be removed and incorporated in the other test files.
|
Not all issues are linked correctly. Please link each issue to the PR either manually or using a closing keyword in the format If mentioning more than one issue, separate them with commas: i.e. |
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 inaws_lambda_powertools/event_handler/openapi/params.py
Form
withformat: binary
in OpenAPI schemaEnhanced multipart parsing in
aws_lambda_powertools/event_handler/middlewares/openapi_validation.py
_parse_multipart_data
method for parsing multipart/form-dataComprehensive test suite with 13 test scenarios covering:
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:format: binary
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number: N/AThis is not a breaking change - it's a new feature addition that doesn't modify existing functionality.
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.