-
-
Notifications
You must be signed in to change notification settings - Fork 331
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
fix: OpenAPI 3.1: required
with type: [array, null]
#2216
Conversation
|
||
|
||
class Basket(BaseModel): | ||
apples: List[Apple] | None |
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.
previously, before the change in this PR, this would have been:
apples: List[Apple] | None | |
apples: List[Apple] |
items: | ||
$ref: '#/components/schemas/Apple' | ||
required: | ||
- apples |
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.
(1) the property is required, e.g. it must exist in the payload.
- array | ||
- 'null' |
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.
(2) but it's value can be either an array of Apple
s or null
.
@koxudaxi I am somewhat blocked by this - is there any chance we could discuss this issue, please, so I can try to gauge whether this can land and/or to use a fork and/or to use a patch file for the result? |
CodSpeed Performance ReportMerging #2216 will not alter performanceComparing Summary
Benchmarks breakdown
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2216 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 38 38
Lines 4285 4288 +3
Branches 987 988 +1
=========================================
+ Hits 4285 4288 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Thank you for creating the PR. I'm waiting for the PR will be marged. |
Amazing, thank you! |
Thank you for 0.26.4 @koxudaxi! |
OpenAPI 3.1 allows a union
type
field for properties. Such a union can be:for example.
Currently the generator ignores this when the field is
required
at the same time.The
required
condition, however, is only applied to the field itself, e.g. an object with a required fieldx
will need to contain the field, but it doesn't define what the value of that field is. Thetype
field defines valid values (in the above case the field can be of typearray
or of typenull
.Thus, a field can be nullable at the same time as being required.
I added a sample file and a potential fix.
The relevant section in the spec is here.
A real-world example of such a spec can be found here: https://github.com/planet-a-ventures/affinity-node/blob/8dd03eb299ff7d08c88405a95627dbc47280c3c5/openapi/2024-11-18.json#L2027-L2039