-
Notifications
You must be signed in to change notification settings - Fork 7
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
OpenAPI generation of OptionalValue
may be incorrect
#821
Comments
One thing to consider is whether JSON Patch would make sense for the API's patch operations. |
It could be that this is fixed by switching from |
Though now that I read this through again, I think it may be the other way around, the OpenAPI spec is generated almost correctly: But at least in my testing the API actually expected and accepted a payload of |
The inconsistency should be cause by the fact that there is custom serializer for the |
If we need to be able to set already present value to |
Yes, that's done by explicitly setting the value to |
The `OptionalValue` class is only used in Kotlin code to model the difference between `null` values and not present values for PATCH requests. In the JSON request bodies such properties can simply be omitted. Configure type redirects for `OptionalValue` to its generic types used in the API classes to eliminate the class from the generated OpenAPI spec. Fixes eclipse-apoapsis#821. Relates to eclipse-apoapsis#605. Signed-off-by: Martin Nonnenmacher <[email protected]>
The `OptionalValue` class is only used in Kotlin code to model the difference between `null` values and not present values for PATCH requests. In the JSON request bodies such properties can simply be omitted. Configure type redirects for `OptionalValue` to its generic types used in the API classes to eliminate the class from the generated OpenAPI spec. Fixes eclipse-apoapsis#821. Relates to eclipse-apoapsis#605. Signed-off-by: Martin Nonnenmacher <[email protected]>
|
The `OptionalValue` class is only used in Kotlin code to model the difference between `null` values and not present values for PATCH requests. In the JSON request bodies such properties can simply be omitted. Configure type redirects for `OptionalValue` to its generic types used in the API classes to eliminate the class from the generated OpenAPI spec. Fixes #821. Relates to #605. Signed-off-by: Martin Nonnenmacher <[email protected]>
The issue with new client generation is now fixed, but I think the spec is still incorrect. New spec for update: "UpdateOrganization" : {
"title" : "UpdateOrganization",
"type" : "object",
"properties" : {
"description" : {
"title" : "String",
"type" : "string"
},
"name" : {
"title" : "String",
"type" : "string"
}
}
}, I believe the correct |
Actually, name is probably correct as is, but description should be |
This is a bug in |
There may be something in OpenAPI generation in Kotlin or the API handler, specifically for
OptionalValue
. @mnonnenmacher can you take a look?The relevant parts of current OpenaAPI for UpdateOrganization:
The correct payload for updating organization name should then be:
For some reason, the old version of
@7nohe/openapi-react-query-codegen
doesn't correctly generate the TypeScript interfaces forOptionalValue
, so it has ended accepting the following that, according to the OpenAPI spec, should not be accepted by the API :For some reason, the API does accept it and modifies the organization name accordingly. If the code is modified to abide to the OpenAPI spec (that the new version of openapi-react-query-codegen does), we get a payload of
that returns the following error:
This leads me to believe that the OpenAPI spec representation of OptionalValue does not match with what the API actually accepts.
Originally posted by @mmurto in #797 (comment)
The text was updated successfully, but these errors were encountered: