OpenAPITools / OpenAPITools/openapi-generator

[BUG][Java][Spring] openapiNullable - JsonNullable usage is based on nullable property instead of required property

Open
#14,765 50 comments 9 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Issue: Bug
Dominant language
Java
Stars
26.8k
Forks
7.7k
PR merge metrics
PR metrics pending

Description

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?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

In Java CodeGeneration openapiNullable is used to determine if the special type JsonNullable should be used or not.
The description of this type from the original documentation is as follows:

The JsonNullable wrapper shall be used to wrap Java bean fields for which it is important to distinguish between an explicit "null" and the field not being present. A typical usage is when implementing Json Merge Patch where an explicit "null"has the meaning "set this field to null / remove this field" whereas a non-present field has the meaning "don't change the value of this field".

So, in a request body (especially in PATCH requests) it is utilized to signalize, if a specifc field was specified in the body or not. This can be used to just ignore any change to the property and keep the old value or to apply a new value to it.

BUT. The current implementation is using the nullable property of a field instead of the required property. So nullable should actually tell about if a field can be null or not IF it is specified. nullable tells nothing about if a field must be present or can be omitted. This is what required is for. required declares, if a field as to be present or not, independent of if it may be nullable or not.

The fix would be to swap nullable and required for the usage of JsonNullable.

openapi-generator version

6.4.0, but goes back until 5.6.0 minimum (probably since openapiNullable was implemented first).

OpenAPI declaration file content or url

Existing /3_0/petstore-with-nullable-required.yaml can be used as an example

Taking the following Pet model from the referency specification:

    Pet:
      title: a Pet
      description: A pet for sale in the pet store
      type: object
      required:
        - name
        - photoUrls
      properties:
        id:
          type: integer
          format: int64
        category:
          $ref: '#/components/schemas/Category'
        name:
          type: string
          example: doggie
          nullable: true
        photoUrls:
          type: array
          xml:
            name: photoUrl
            wrapped: true
          items:
            type: string

The Pet class would be generated like:

public class Pet {

  @JsonProperty("id")
  private Long id;

  @JsonProperty("category")
  private Category category;

  @JsonProperty("name")
  private String name;

  @JsonProperty("photoUrls")
  @Valid
  private List<String> photoUrls = new ArrayList<>();
}

Although, because only name and photoUrls are declared as required, all other properties should be wrapped in JsonNullable and name should be initialized with null as it is declared as nullable additionally.

public class Pet {

  @JsonProperty("id")
  private JsonNullable<Long> id = JsonNullable.undefined();

  @JsonProperty("category")
  private JsonNullable<Category> category = JsonNullable.undefined();

  @JsonProperty("name")
  private String name = null;

  @JsonProperty("photoUrls")
  @Valid
  private List<String> photoUrls = new ArrayList<>();
}
Generation Details
./bin/generate-samples.sh bin/configs/spring-openapi-nullable-required.yaml
Steps to reproduce

I already created a pull request: #14766

Related issues/PRs

Didn't find any existing issue to this (which is quite suprising).

Suggest a fix

I already created a pull request: #14766

I am totally aware that this is a huge breaking change so I would like to discuss this issue openly. Maybe we need some kind of flag for the new behaviour. Any critics to this behaviour are welcome.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with modules/openapi-generator/src/test/resources/3_0/petstore-with-nullable-required.yaml and run ./bin/generate-samples.sh bin/configs/spring-openapi-nullable-required.yaml. Compare the generated Pet model with the expected JsonNullable behavior described here, and review PR #14766 before making changes. Done means the required and nullable cases are generated as specified, with regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, openapi, spring
Domain
api, backend, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.