[REQ] Add parameter to standardize handling null types in all language

[ahmednfwela]

this is a problem that happens when a property has no default value, and is marked non-required and non-nullable
related: OAI/OpenAPI-Specification#2807

Describe the solution you'd like

1. Add a new parameter for all language generators (e.g. handlingStrategyForNonNullableNonRequired)
2. Its possible values are:
   1. FALLBACK old behavior so we won't cause a breaking change
   2. assumeNullable assume that it's nullable
   3. assumeRequired , assume that it's required
   4. assumeDefault , assumes a default value based on the clr (e.g. in c# that would be default(T) , in js it would be undefined)
   5. raiseError prevents handling this case altogether
3. languages will then opt-in to handle this new parameter by specifying in the FEATURE SET what of the 5 values they support (All languages will support fallback, raiseError, assumeNullable,assumeRequired by default, since they can be handled globally)

I can start working on the implmentation if someone can point me in the right direction
@wing328

[devhl-labs]

Is this issue regarding whether a property is nullable? Required only defines the nullability in regards to a codegen parameter. When dealing with a codegen property, use isNullable instead.

[ahmednfwela]

@devhl-labs that's not how the openapi spec works, it doesn't differentiate between a codegen parameter and a property, they are both part of the schema and they both have required/nullable status

[devhl-labs]

To be clear, your concern is what we set isNullable to when a property/parameter is both non-required and non-nullable? I also had this concern, and I toyed with using setting the nullability to !required && isNullable

[ahmednfwela]

the problem isn't the nullability, it's the default value
when a default value is absent and it's not required (the user doesn't have to enter it), the generator has to assume it
but how do you assume it in this case? you are suggesting that we just assume it as null (option 2 i declared above)

but shouldn't the user be able to customize this behavior ?

[devhl-labs]

The problem is that in the C# RestSharp templates, the parameters all get a default value. This is a mistake in my opinion, and it is one that I have fixed in my generichost library. If the definition states that a field is required and not nullable and no default value, then the user must provide a value. The mustache template should validate the input and throw where appropriate. If the property is nullable and the user provides no value, then use the default value. If the default value is null, then it's null.

[ahmednfwela]

but why "force" users to do anything? users should be able to customize the behavior they want, which is my suggestion here

[devhl-labs]

If there is a method, get_by_id, that takes an required non nullable with no default, and the user tries to run it without providing a value, the only reasonable thing to do is throw. Don't even send the request to the server. The spec says the parameter is required and not nullable. The user must obey that. If there is a default then the spec is wrong. Update the spec.

[ahmednfwela]

what we are talking about here is non-required non-nullable params with no default value

[devhl-labs]

Then you would mark the parameter as nullable and only add the value to the request when not null.

[ahmednfwela]

as I have said multiple times, I am giving the user an option to do what they want
they can either mark that specific parameter as nullable, required, or give it a default value

[spacether]

Can you give an example of this schema?

property has no default value, and is marked non-required and non-nullable

Isn't that just an optional property with a type defined?
nullable: false has no impact per:
A true value adds "null" to the allowed type specified by the type keyword, only if type is explicitly defined within the same Schema Object. Other Schema Object constraints retain their defined behavior, and therefore may disallow the use of null as a value. A false value leaves the specified or default type unmodified. The default value is false

Some examples here:

1

type: object
properties:
  foo:
    # required: false should not be here, this property should be omitted from the list of required property names
    # nullable: false has no impact because false by default
    type: string
required: []

the object can have an optional key foo with a string value. The key value pair for foo may be included or it may be omitted.
foos value may not be null.

2

type: object
properties:
  foo: {}
required: []

the object can have an optional key foo with a value containing any json type (string/int/float/null/array/object). The key value pair for foo may be included or it may be omitted.

If an optional value is omitted from an object client side it should not be sent to the server. Likewise if a server is sending back an object that lacks the optional value, the key value pair should not be included.

[ahmednfwela]

lets take the first case
when a generator attempts to generate this schema, the property foo doesn't have a default value and isn't required
so the generator needs to assume its default value.
IF it was nullable the default value would be null,
but since it isn't nullable, the generator has to do one of the 4 options I wrote above.
generators do one of them implicitly anyway and the most common is to assume the property nullable

[ahmednfwela]

the problem is that this specific case itself is illegal, since non-required means the server might not send it, but at the same time, it does not tell the generator what to do when the server does not send it (by not specifying a default value).

if it was just validating an object against a schema that would be fine, but openapi handles generating the object as well.

[spacether]

So what you have described sounds like it comes from the context of a compiled language. Is that correct? What you have said is not true for all generators or all compiled languages.
Below are some solutions that don't require your proposed solutions.
@wing328

Interpreted

In python-experiental

• unset optional typed values are unset in dict payloads, they do not need an unset sentinal value
• unset optional nullable typed values are unset in dict payloads, they do not need an unset sentinal value
  One could store an unset sentinal value if one needed to store a value in a model instance
  • if the value is present and None it is stored

Compiled

Java, use JsonNullable per #3435
Because one can store undefined (unset) values in it per:
https://github.com/OpenAPITools/jackson-databind-nullable
jersey2 uses this

Go-experimental nullable support:
#5414

[ahmednfwela]

@spacether having a "special" undefined value is also another solution that's similar to assuming a default value.

this can be added as an option number 6

• assumeDefaultIsUndefined that is different from assumeDefault in that it creates a special value called undefined

However this won't be supported by all languages (especially strictly typed ones)

[spacether]

If this is a feature that we add, then python-experimental would need an option like:

• omittedFromPayloadIfUnset
  python-experimental does not use a sentinel value. I was just mentioning it as a possible solution.

What does raiseError mean?

• would the tooling (generator) raise an error if a spec schema had this use case in it
• would a client raise an error if one attempted to access an unset optional value?
• what raises the error and when?

[ahmednfwela]

raiseError means the tooling (generator) would raise an error if a spec schema had this use case in it.

also by default, if a variable is marked as not required, it shouldn't be serialized if value == defaultValue this applies to all generators, not just python

[spacether]

Thank you for explaining what would raise the error.

it shouldn't be serialized if value == defaultValue this applies to all generators, not just python

That sounds like an implementation decision. I disagree that it should be done that way. If the defaultValue is allowed, then I think that the client should also be able to send it if a developer manually specifies it. As an implementation one could choose not to send the value when it is == to defaultValue; one can do it that way if one wants.

[ahmednfwela]

but isn't that what the required parameter was made for ?

• in serialization: if a non-required parameter has a default value (usually null) it should be omitted
• in deserialization: if a non-required parameter isn't present in the json object sent by the server, it should be set to the default value

this is already the default behavior in many generators including dart-dio-next (now dart-dio) and charp-netcore

what we are discussing, is what to do if the spec doesn't provide a default value, and the property is neither nullable (so we can't assume the default value to be null) nor required (so we can't force the user or the server to give us a value)

[spacether]

Null can be a valid value for an optional variable that needs to be sent to the server. In use case 2, foo can have the value null which should be sent to the server.

[ahmednfwela]

@spacether in that case, it should be marked required and nullable with a default: null (implicit)

[spacether]

The default is for if the value is not sent. What if it is optional, nullable int with a default of 2. It is still optional and null should be sent. It is not required. Values could not be sent.

[ahmednfwela]

according to the spec, a required property means that it must exist in the json, with no regard to its actual value.

giving a non-required property a default value should be equivalent to completely omitting it from the json

however this isn't true for all languages, e.g. js
where an omitted property will be assigned undefined unless specified otherwise.

check https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/dart/libraries/dio/serialization/built_value/class.mustache#L47

[spacether]

giving a non-required property a default value should be equivalent to completely omitting it from the json

Agreed, that's why I described this as not being sent.

however this isn't true for all languages, e.g. js
where an omitted property will be assigned undefined unless specified otherwise.

That’s an implementation detail. Just because our generator does something doesn't mean that it conforms to the spec. We should strive to do the best job that we can in the generators and conform to the spec. All languages support maps/dicts. Optional properties could be stored in them if they exist and omitted if they do not. I think go or go-experimental does that for additionalProperties which are optional.

[ahmednfwela]

the problem is, the spec doesn't explain what to do in this scenario, which is why my suggestion here to clarify and standardize that

[wing328]

@ahmednfwela first of all, thanks for the questions/suggestions.

To answer your question, I would suggest we first imagine there's no such thing called OpenAPI and back to days when the APIs are described by HTML documentation.

Let' consider https://developer.twitter.com/en/docs/twitter-api/tweets/timelines/api-reference/get-users-id-tweets#tab0. The response has some optional fields which do not specify the default value. Would that result in confusions to developers who create SDKS in a particular programming language (e.g. Java, C#)? I don't think so as I've seen many API clients in different programming languages created for Twitter API.

My understanding is that the owner of the API (Twitter API in this case) doesn't care how the optional fields are stored in the client SDKs (e.g. nil, undef, null, etc).

For the request body sent to the API server, it checks to see if it contains the required fields (or the "Default" fields in the Twitter API documentation), e.g. https://developer.twitter.com/en/docs/twitter-api/tweets/manage-tweets/api-reference/post-tweets#tab2. Notice that some optional fields are omitted in request body (JSON) sent to the server.

For your use cases in which you would like to explain clearly how to store the optional fields in the client side, you can specify the default field (assuming you're the owner of the API). If you foresee the default value is going to be different for different programming languages, you may want to use extensions, e.g. x-java-default, x-csharp-default.

For the situation in which we're the consumer of the API (not owner), we only need to ensure the auto-generated client is doing the right thing by ensuing the payload does not contain optional fields that are not set by the users of the client.

I hope this answer your question regarding "when a property has no default value, and is marked non-required and non-nullable"

[ahmednfwela]

@wing328 thanks for the great reply!

I understand the general consensus of

if it's not required and the user didn't set it, just don't send it, who cares what you have on client side

if we are talking about Json schemas that would be perfectly fine, since the server doesn't see nor control the consumer's behavior for handling non-required fields, maybe consumer is storing it in RAM as undefined if they were using js or setting it to null + some annotations if they were using c#, or maybe wrapping every field in a class like java's JsonNullable

but OAS isn't just Json schema, it builds on top of it and provides more information per schema.

taken from the official docs:

default - The default value represents what would be assumed by the consumer of the input as the value of the schema if one is not provided. Unlike JSON Schema, the value MUST conform to the defined type for the Schema Object defined at the same level. For example, if type is string, then default can be "foo" but cannot be 1.

so now we are telling the consumer what to do when an optional field is not provided and it's not just set it to null, something Json schema didn't do before.

so the only problem here is the case

• non-required - might not be present when receiving or sending a json
• no default - consumer doesn't have instructions on what to set it to when it's not present
• non-nullable - doesn't accept nulls

so my suggestion here is very simple:
give power to the developer to how to handle that specific case in ANY generator by using one of the approaches I wrote above, the only thing that needs to be handled per generator is the assumeDefault way, since each language has their way of handling this

[wing328]

but OAS isn't just Json schema, it builds on top of it and provides more information per schema.

That's referring to 3.0.3 spec. The latest 3.1.0 now supports 100% compatibility with the latest draft (2020-12) of JSON Schema: https://www.openapis.org/blog/2021/02/18/openapi-specification-3-1-released so the Unlike JSON Schema part is no longer true in the latest version.

I used JSON payload as an example but my understanding of the default value is independent of the payload format (XML, protobuf, etc). Imagine there's a payload format in this world in which the default value is crucial for "non-required and non-nullable" property and without it the API functionality may break, then the owner of the API must provide the default value in the OpenAPI spec/doc in such case.

I want to use #10913 to explain how I view implementation in the client side in particular. As you can see, the users do not want snake_case in the method naming in the Python code but snake_case is the official method naming method in the style guide. Clearly he's the right to do so. The code still works with non-snake case but if we accept such "enhancement", we need to provide a lot of options down the road. The PR author in this case (or other users have similar need) can maintain a customized version of openapi-generator to meet his unique requirement.

In your original post, you listed our 5 possible values for the option handlingStrategyForNonNullableNonRequired. No matter which one we choose, it may not be what the API owner have in mind.

In C#, there's something called DataMemberAttribute.EmitDefaultValue. I think the option is somehow similar to what you're trying to do. I think your use case at this stage is the Dart client generator. What about adding an option to the Dart client generator (after discussion with the Dart community in this project) to start with instead of adding an option to ALL generators (100+), which is a lot of work?

[ahmednfwela]

visiting on this after this PR #14172 we can add normalizers to handle all options mentioned above, what do you think ? @wing328