-
-
Notifications
You must be signed in to change notification settings - Fork 780
Add comprehensive OpenAPI 3.1 support with JSON Schema 2020-12 validation #2043
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: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
|
Please say, will these changes automatically fix any of the open issues here related to |
@chrisinmtown - I'm not sure. I was just trying to see if I could get Claude Code to help me take some of the different issue comments and PR work related to OpenAPI 3.1 and put together something that might have a chance at landing to support folks looking to use Connexion with the 3.1 edition of the OpenAPI spec. It is switching up validation if a 3.1 spec is used ... so it's possible there could be some different behavior now. |
This commit addresses issue spec-first#2018 by: - Adding proper support for file uploads in OpenAPI 3.1 with complex schemas (allOf, ) - Improving detection of file array types based on schema definitions - Ensuring UploadFile objects are preserved during validation - Refactoring code to be more general-purpose and avoid test-specific handling - Moving imports to the top of files rather than using deferred imports - Adding comprehensive tests for OpenAPI 3.1 file uploads with various schema patterns 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
for more information, see https://pre-commit.ci
|
|
||
| # Handle file uploads - make a copy to avoid modifying the original | ||
| if is_file_upload: | ||
| files = dict(files) |
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.
It seems like this could be just slightly refactored/simplified by starting with the single if condition is_file_upload once, and then within that block, use is_swagger2 and else. Repeating the check for is_file_upload and using not is_swagger2 seems overly complicated. Am I missing something?
| body_name = sanitize(operation.body_name(content_type)) | ||
|
|
||
| # Handle all OpenAPI file upload cases | ||
| if is_file_upload and isinstance(body, dict): |
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.
I think this use of is_file_upload can be safely dropped since you checked that at line 245
| # By examining the operation ID for mixed or combined form handling patterns | ||
| is_mixed_form_handling = False | ||
| if operation.operation_id and ( | ||
| "mixed" in str(operation.operation_id) |
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.
I believe the operation ID is a dotted Python function reference like "my_package.my_module.my_function". This code seems to look for magic values in that string, do I understand that correctly?
FWIW I looked thru tests/fixtures/openapi_3_1 and did not see "combined" or "mixed" in any operation ID. However the code-coverage report seems to show that these lines were covered. I'll have to look more.
| ret.update(_get_file_arguments(files, arguments, body_schema, has_kwargs)) | ||
|
|
||
| # Add remaining files not already included in body | ||
| file_args = _get_file_arguments(files, arguments, body_schema, has_kwargs) |
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.
A tiny thing for consistency, instead of creating a variable file_args that is only used in the next line, you might follow the pattern from above and write this as
ret.update(
_get_file_arguments(
files,
...
| For OpenAPI 3.1 specs, Connexion will use Draft7Validator to validate requests and responses | ||
| instead of the Draft4Validator used for OpenAPI 2.0 and 3.0. | ||
|
|
||
| You can specify the JSON Schema dialect to use by setting the ``jsonSchemaDialect`` field in your |
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.
I find this a little confusing, sorry if this is a dumb question. If the openapi property has value "3.1.0" (instead of something like "3.0.3"), is that enough for connexion to use the appropriate validator? It sounds like maybe it's not, and the additional jsonSchemaDialect property is required to check against draft/2020-12/schema?
| request.get_body.return_value = {} | ||
| request.files.return_value = {} | ||
| # Make sure files() properly returns a completed coroutine | ||
| mock_files = {} |
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 change adds a local variable that's used on the next line and nowhere else. The comment suggests that the files function should return a completed coroutine but I don't see any new assert or other check. Is this test code complete?
I have a similar question about similar test code starting at line 96.
|
|
||
|
|
||
| async def test_mixed_formdata(file, formData): | ||
| print(f"DEBUG test_mixed_formdata - file type: {type(file)}, formData: {formData}") |
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.
maybe you forgot to remove this printf invocation before committing?
I am not trying to be picky and I'm not asking for this to removed, I think debug output from tests is totally ok, it's just not done in any existing test cases.
| @@ -343,40 +343,101 @@ async def test_formdata_file_upload(file): | |||
|
|
|||
| async def test_formdata_multiple_file_upload(file): | |||
| """In Swagger, form parameters and files are passed separately""" | |||
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.
Just trying to understand this existing/old comment, not a change you made, it seems to suggest that the test case is for Swagger only, but maybe it's actually indicating that in both Swagger (openapi 2) and Open API 3, form parameters and files are passed separately?
|
Greetings to the maintainers, adding support for 3.1 seems like a major benefit to the Connexion community. In addition, this PR may also address some open bugs with processing query/body parameters that use oneOf/anyOf/allOf. Will you please review and comment? |
|
When I run tox, I see some complaints from the code style checkers |
| schema_type = schema.get("type") | ||
| if schema_type == "string": | ||
| if schema.get("format") == "date-time": | ||
| return "2021-01-01T00:00:00Z" |
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 ISO-8601 date-time string cannot be parsed by Python 3.9 or 3.10; it can be parsed by Python 3.11 and onwards. Since you probably don't really care about timezones or Z here, an alternate specification of the time zone works fine in older versions, so I suggest changing the value to 2021-01-01T00:00:00+00:00
% python3.9
Python 3.9.23 (main, Jun 3 2025, 18:47:52)
[Clang 17.0.0 (clang-1700.0.13.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime
>>> datetime.datetime.fromisoformat('2021-01-01T00:00:00+00:00')
datetime.datetime(2021, 1, 1, 0, 0, tzinfo=datetime.timezone.utc)
I found some history here: https://discuss.python.org/t/parse-z-timezone-suffix-in-datetime/2220
| upload_files[key] = file_obj | ||
|
|
||
| # Special handling for OpenAPI 3.1 with allOf and file uploads | ||
| is_openapi31_allof = ( |
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.
Because the current version has no support for allOf, anyOf, oneOf in form data in Open API version 3.0, I am not sure it's a good idea to add any support for all of, anyOf, oneOf in Open API version 3.1. It definitely requires an extension to the documentation. Also I didn't see a mention of this in the PR comment, did I miss it?
| multipart/form-data: | ||
| schema: | ||
| allOf: | ||
| - $ref: '#/components/schemas/HeaderName' |
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.
For what it's worth, this use case is exactly what I complained about in issue #2018 :/
Overview
This PR adds comprehensive support for OpenAPI 3.1, including JSON Schema 2020-12 validation and all major features introduced in the OpenAPI 3.1 specification. The implementation ensures proper handling of the enhanced schema validation capabilities while maintaining backward compatibility.
Key Features Added
1. JSON Schema 2020-12 Support
Draft202012Validatorfor validating OpenAPI 3.1 schemasdraft2020_format_checkerfor compatibility2. Type Arrays Support
type: ["string", "null"]) to replace the deprecatednullable: trueproperty3. New JSON Schema Keywords
exclusiveMinimumas a value rather than a boolean)4. Webhook Support
OpenAPI31Specificationclass5. Minimal Documents
6. PathItems in Components
pathItemsin the Components Object7. Enhanced Security Features
8. Updated Example Application
Testing
Documentation
References
Acknowledgments
This work was inspired by and builds upon the initial efforts in PR #1951 as well as the valuable insights provided in Issue #1396 comment. We want to ensure proper credit is given to those who contributed ideas and initial implementations that helped shape this enhancement.