Open kevinoid opened 4 years ago
👍 Thanks for opening this issue! 🏷 I have applied any labels matching special text in your issue.
The team will review the labels and make any necessary changes.
I have noticed the exact same issue.
I have pinpointed where the issue was introduced:
As mentioned by @kevinoid a non-required Date property is populated instead with the Min value for a date i.e. 1900-01-01 when not present in a request
@wing328 I've found your following statement in the PR that probably introduced this regression:
Upgrade Note
To make a property nullable in OpenAPI/Swagger spec 2.0, please use
x-nullable: true
https://github.com/OpenAPITools/openapi-generator/pull/3537#issuecomment-523529169
Does that mean that openapi-generator will always require x:nullable:true
from now on? Or could this issue be fixed?
Hi has anyone had any luck with this? Currently facing same issue
Also having this issue. I have found that I have to use nullable: true
instead of relying on the required which sadly means a lot of updates to our OpenApi specs if the issue can't be found.
@wing328 I've found your following statement in the PR that probably introduced this regression:
Upgrade Note
To make a property nullable in OpenAPI/Swagger spec 2.0, please use
x-nullable: true
#3537 (comment)Does that mean that openapi-generator will always require
x:nullable:true
from now on? Or could this issue be fixed?
Can anyone comment on this question? It seems to me that if a property is not required and has no default then it must, by definition, could be null. What other alternative is there? Requiring a vendor extension (and updates to API definition) doesn't seem right.
Geez, can we get a fix for this please? It's a pretty major bug to my eyes.
We have a problem that might be related. We have a PATCH request where the body has an optional nullable string - i.e. it could be a string, it could be null, or it could be undefined. We have no way of choosing undefined when we create the PatchRequest()
object.
Requiring a vendor extension (and updates to API definition) doesn't seem right.
In OpenAPI spec 3.0, nullable
is part of the official spec.
One possible way is to introduce an option to use nullable type without using nullable
in the spec.
Let me know if anyone wants to contribute such enhancement or sponsor it.
Requiring a vendor extension (and updates to API definition) doesn't seem right.
In OpenAPI spec 3.0,
nullable
is part of the official spec.
Yes, but for Swagger 2.0 it's a vendor extension. From the upgrade note:
To make a property nullable in OpenAPI/Swagger spec 2.0, please use x-nullable: true
This to me, does not seem right.
Also, an optional parameter without a default value must be nullable. Nothing else makes sense. The API spec for such a field says "this does not have to have value" which is exactly the same meaning as "this is nullable". Anything else is overriding the spec to effectively make the field required.
I see the nullable
as specifying that a field can be null _even though it has a default value. Otherwise you end up with a catch-22: A field which is not required, has no default value but is not nullable - what is that? In practice, you have to give a value (since null will fail) so you're violating the API spec and making the field required. Or am I missing something - what value can be given to such a field?
At a bear minimum there should be a generation-time flag to make it behave like this.
As I see it, for v2 we are requiring a vendor extension and even for v3 we are requiring that the spec is written in a specific way to get around the generator implementation.
Bumping this. Other generators offer a property when generating code for 'Generate optional properties as nullable'. Can this be added as a feature to OpenApi Generator?
Bug Report Checklist
Description
The C# generators currently generate model classes using non-nullable types for properties which are not required, which can't represent instances where those properties are not present.
openapi-generator version
v4.0.0 and later
OpenAPI declaration file content or url
Example OpenAPI 3.0.2 document
```yaml openapi: '3.0.2' info: title: non-required property example version: '1.0.0' components: schemas: DateRange: description: A possibly open-ended date range. type: object properties: start: type: string format: date-time end: type: string format: date-time required: - start paths: /date-ranges: get: operationId: getDateRanges responses: default: description: Get date ranges content: application/json: schema: type: array items: $ref: '#/components/schemas/DateRange' post: operationId: addDateRange requestBody: required: true content: application/json: schema: $ref: '#/components/schemas/DateRange' responses: '201': description: Success ```Note that
end
is not declarednullable: true
becauseend
is nevernull
in the JSON produced or consumed by the API. It is either a date string, or not present.Command line used for generation
java -jar openapi-generator-cli.jar generate -g csharp-netcore -i openapi.yaml -o generated
Steps to reproduce
DateRange
object without anend
property).GetDateRanges
and note thatEnd
for the open-ended range isDateTime(1900-01-01)
, which is problematic since it is indistinguishable from"end":"1900-01-01"
and likely violates the constraint thatEnd
is not beforeStart
.AddDateRange
with an open-ended range, sinceEnd
will always have a value.Related issues/PRs
The regression occurred between v3.0.2 and v4.0.0. Bisect says the first bad commit is 3744273312 (
v4.0.0
), so I'm obviously doing something wrong. (Maybecli
is using publishedcore
of same version, rather than locally-built version?) Advice on how to bisect would be appreciated.The issue was also discussed in https://github.com/OpenAPITools/openapi-generator/issues/3725#issuecomment-545039145.
Suggest a fix
I believe nullable types should be used for properties which are either
nullable
or notrequired
, sincenull
in C# is a reasonable representation of both JSON properties which arenull
and properties which are not present.Thanks for considering, Kevin