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

Allow enums with nullable defaults #1571

Closed
wants to merge 2 commits into from

Conversation

hauntsaninja
Copy link

@hauntsaninja hauntsaninja commented Aug 6, 2022

The following two PRs added better support for nullable defaults:
#1463
#1464

However, it looks like it missed adding NullableEnumValidator to create_spec_validator.

Also see OAI/OpenAPI-Specification#1900 , which establishes that you can't just put null into the enum.

I'd never used openapi or jsonschema or whatever before this week, so maybe I'm totally misunderstanding something. Apologies if so.

@hauntsaninja
Copy link
Author

cc @Ruwann :-)

@hauntsaninja hauntsaninja changed the title Allow nullable enums Allow enums with nullable defaults Aug 6, 2022
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2814084668

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.683%

Totals Coverage Status
Change from base Build 2612692278: 0.0%
Covered Lines: 2707
Relevant Lines: 2859

💛 - Coveralls

@hauntsaninja
Copy link
Author

Thanks RobbeSneyders for triggering CI on this :-)

If this is a valid fix, I'm also curious about whether a release could be made... I wasn't able to find any docs about connexion's release schedule. Thank you again!

@RobbeSneyders
Copy link
Member

Hi @hauntsaninja, thanks for the PR.

It seems like there was a clarification to the OpenAPI spec last year that specifies that this behavior is not allowed though:

If a schema specifies nullable: true and enum: [1, 2, 3], does that schema allow null values? (See OAI/OpenAPI-Specification#1900.)
No. The nullable: true assertion folds into the type assertion, which presumably specifies integer or number.
While the modified type now allows null, the enum does not. Consistent with JSON schema, a value conforms to the schema only if it is valid against all constraints. Any constraint, in this case enum, can cause a value to fail validation, even if that value meets all of the other constraints.

We should probably remove the NullableEnumValidator instead of expanding its usage.

@hauntsaninja
Copy link
Author

Okie dokie, seems clear enough! Thank you

@hauntsaninja hauntsaninja deleted the nullable-enum branch August 23, 2022 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants