[BUG] Double nullable operator ("??") when combining "nullable" and "not required" field in body model

[JFCote]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?

Description

If you have a request body that contains field with the keyword: nullable and that they are not in the required list and that you generate AspNetCore server with the nullableReferenceTypes option set to true, it will generate something like that:

public string?? MyModel { get; set; }

openapi-generator version

7.4.0 (Not yet released, in master at the moment of writing this.)

OpenAPI declaration file content or url

openapi: 3.0.0
info:
  title: API Version Endpoint
  description: A simple endpoint that returns the version of the API
  version: 1.0.0

servers:
  - url: http{protocol}://{server}:{port}/api
    variables:
      protocol:
        enum:
          - ''
          - 's'
        default: ''
      server:
        enum:
          - 'localhost'
        default: 'localhost'
      port:
        default: '7154'
paths:
  /pets:
    patch:
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/MyStuff'
      responses:
        '200':
          description: Updated
components:
  schemas:
    MyStuff:
      required:
        - bark
        - breed
      type: object
      properties:
        bark:
          type: boolean
        breed:
          type: string
          enum: [Dingo, Husky, Retriever, Shepherd]
        uselessInfo:
          type: string
          nullable: true

Generation Details

openapi-generator-cli generate -g aspnetcore \
  --model-name-suffix ApiModel \
  --additional-properties nullableReferenceTypes=true \
  --additional-properties aspnetCoreVersion=6.0 \
  --additional-properties enumValueSuffix= \
  --additional-properties classModifier=abstract \
  --additional-properties operationModifier=abstract \
  --additional-properties useDateTimeOffset=true \
  -i ./openapi.yaml \
  -o ${PWD}

Steps to reproduce

Run the command above and notice it will have double "??" which is not valid C# code.

Related issues/PRs

This bug was introduced by me because I'm never using "nullable" in an OpenAPI spec: #17934
Other teams at work are using it and it's causing problem.

Suggest a fix

I'm hesitating between multiple solutions.

nullable was introduced in version 3.0.3 but was removed in version 3.1.0 for reasons I can only imagine. The fact that we already have required that implied that an object can be null when it is not required might be one.

Option 1: Completely remove my fix and start using "nullable" in my code to deal with nullable but it will only be the inverse of using required so it's kind of bad in my opinion.
Option 2: Stay away from "nullable" since it has been removed. I would keep my fix but add code that prints warning on the output telling that using "nullable" had been removed and that people should use the required list instead (or in this case, not use it to tell a field can be null).

I'm open to ideas!

@wing328 Please feel free to tag any expert in the C# / AspNet community.

[wing328]

@JFCote did you test the csharp client generator? if the output (models, parameters) look good to you, we can plot the implementation to aspnetcore server generator.

[devhl-labs]

The difference between nullable and required is best understood in the raw json.

{
  foo: null
}

{}

It is a subtle but important difference. For instance, if you only want to update a particular property of an object, the server may allow you to only include that one property and omit the other properties. Or you may want to specifically set a property to null. So both cases should be supported. To do that, we have to keep track of null and omitted separately. One such way we can do that is to wrap not required properties in a struct which keeps track of whether the property has been set yet.

public Option<string?> Foo { get; set; }

This is how I handled it in the client generator for the generichost library.

[JFCote]

@devhl-labs Thanks for the explanation, it makes a lot of sense. You idea to deal with it in a struct is also good. I will have to check you code to understand and will propose a PR next week.