Handle un-defined nillable properties

[aneeshafedo]

Description:

According to Open API Specification if a given property is required or/and nullable we need to specifically mention it as explained in [1], [2].

Therefore If a given property is not specifically mentioned as required that property is optional by default. Similarly if a given property is not specifically mentioned as ”nullable: true” that property is not accepting null values by default.

We have experienced several scenarios, where the response we are receiving has null values even though that property is not configured as nullable: true. In such cases http target type binding fails, returning a type conversion error.

As a solution to this we have already introduced as --nullable flag to make all the fields nillable when it is not defined in the OpenAPI file. However this approach is not user friendly.

Please introduce a methodology to handle this scenario in more user friendly way. (Ex: internally binding null fields without returning any runtime error.)

Discussed this issue under the the email subjected [Handling nullable and required fields in connectors generated from Open API too] in 2021

Describe your problem(s)

Describe your solution(s)

Related Issues (optional):

Suggested Labels (optional):

Suggested Assignees (optional):

[NipunaRanasinghe]

Given that this issue has been a major cause of runtime failures in our generated connectors, it would be beneficial to prioritize this improvement as soon as possible.

I've identified the following potential solutions for the same:

1. Using the --nullable flag during generation

• This approach makes all fields in all types (i.e., records) nilable and optional.
• Although this is the current and simpler option, it affects the user experience. Users must implement additional logic to check for nil values and absent fields when utilizing the connector responses(i.e. records).
• This method might also introduce different types of errors (e.g. optional types are not supported as path params in Ballerina)

2. Exploring a relaxed data binding mechanism

• From a broader perspective, the distinction between the concepts of optionality and nullability can be overlooked in the context of payload binding, as both represent the case of the absence of data.
• Therefore instead of relying on existing JSON conversion langlib APIs (cloneWithType(), fromJsonWithType()) for data binding, we could consider introducing new langlib API(s) (or implementing our own) so that:
  • Null values would be mapped to optional fields (by omitting the null value),
  • Absent JSON fields could still be mapped to their corresponding nilable fields in the record (by assigning nil).

Although it will require some effort, option 2 seems like the better choice to me, assuming it is technically feasible.

@shafreenAnfar @lnash94 @TharmiganK Any thoughts?

[TharmiganK]

Adding on @NipunaRanasinghe's comment:

1. Using the --treatOptionalAsNullable Flag during Generation

• The OpenAPI spec says the following:

   In objects, a nullable property is not the same as an optional property, but some tools
   may choose to map an optional property to the null value.
  

  So if we are having an option to handle this particular issue, ideally we should only treat the optional fields of the object schema to be nullable, we should not make all the types nullable. This is more like we inject nullable: true to all the fields that are optional in a object schema and then generate the client/service. If we did that then the issue with path parameters should be resolved. So IMO we should have a flag like --treatOptionalAsNullable.

• Still there is an impact on the user experience where the users has to access the fields of the record using optional field access or member access. But due the simple syntax, IMO this is not a big impact. Also this same impact will be there if the OpenAPI specification is correctly written with nullable: true.

  public type User record {
      string name;
      int age;
      Organization? org?;
  };
  
  public function main() returns error?{
      User user = check clientEP->/users;
  
      Organization? org = user?.org;
      // Organization? org = user["org"];
  
      string? orgName = user?.org?.name;
      // string? orgName = user["org"]["name"];
  }

• Some endpoints can represent separate meaning for not sending a particular field and sending a field with the null values. In those scenarios, this is the appropriate option as the users can also differentiate the two behaviours. But this might be only 20% case.

2. Treat null field values as a special case in data-binding

• This is also a viable solution where we treat the fields with null values specific to the target type as you mentioned

• Since most of the standard libraries have data-binding support, to be consistent, we should have the same behaviour in all of the modules. Also libraries have a limited access to the langlib APIs to implement the data-binding logic by themselves. That's why we were using the langlib conversion APIs. So if we are going with this option, I prefer to have a new langlib API to support this scenario or a common library like json.data should expose some API to get this done.

I am ok with both options

[lnash94]

Meeting: 2024/03/19

Attendees: @shafreenAnfar, @hasithaa, @MaryamZi , @warunalakshitha , @NipunaRanasinghe, @prakanth97 , @HindujaB , @TharmiganK , @dilanSachi, @lnash94

Discuss points :
Problem:

• According to the client generation experience of 80% scenarios, the user has provided the payload value where the response we are receiving has null values even though that property is not configured as nullable: true. In such cases HTTP target type binding fails, returning a type conversion error.

Solution discussed:

• Using projection support from conversion APIs from the data module this issue will be addressed
• After having the fix from the lang side,
  • The HTTP module will add a fix by providing configurable to handle undefined nillable property scenarios where the user can choose strict data binding or relaxed data binding(support from projection).
  • Then the OpenAPI code generation will add the configuration to the generated client by executing the flag to the client generation command.

[NipunaRanasinghe]

To recap our last meeting discussion, here's the breakdown of the cases we need to reconsider within the context of payload data binding:

Scenario	Ballerina Representation	Current Behaviour	Expected Behaviour	Comments
1. Required and non-nullable	T foo;	both null values and absent fields cause runtime failures	same	(optionally) can introduce a new flag to the openAPI tool, so that the users can choose to generate all the required properties as nilable/optional to avoid runtime failures in case of poorly written contracts
2. Required and nullable	T? foo;	absent fields will cause runtime failures	need relaxed databinding to map absent fields as nil values	same support will be added to the underneath http client via new data module APIs
3. Optional and non-nullable	T foo?;	null values will cause runtime failures	need relaxed databinding to map null values as absent fields	same support will be added to the underneath http client via new data module APIs
4. Optional and nullable	T? foo?;	both nil values and absent fields are allowed	same	N/A

@shafreenAnfar @hasithaa @lnash94 feel free to add any missing points.

[prakanth97]

@NipunaRanasinghe
anydata foo; and anydata? foo; same since anydata has () as its union member. Hence the expected type for the field foo should be T where T is anydata - () to satisfy your requirement.

I have sent the PR for the jsondata module to support this requirement.

[TharmiganK]

Regarding the HTTP level support, it requires a stable version of jsondata package and we need to change our data-binding implementation to incorporate the APIs from this package. So, we need to check the possibility of changing all the lang APIs with this package and this requires some effort and not possible to add this to update 10. I will check this and draft a proposal for this and share it soon.