OpenAPITools / OpenAPITools/openapi-generator

[BUG][Java][Spring] Wrong `NotNull` annotation for property received through `allOf` reference

Open
#22,150 6 comments 3 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?
  • Have you tested with the latest master to confirm the issue still exists? (but tested with 7.16.0)
  • 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

We have encountered the following issue with the Java Spring OpenAPI generator.

First of all, we override the beanValidation.mustache template to fix this issue. In short: required should define whether a property exists, type: null specifies the nullability of a property. The only change is this:

# customized beanValidation.mustache:
{{^isNullable}}{{^isReadOnly}}@NotNull {{/isReadOnly}}{{/isNullable}}...

# original beanValidation.mustache:
{{#required}}{{^isReadOnly}}@NotNull {{/isReadOnly}}{{/required}}...

The actual problem is this: We have a base type Recipe with a nullable property category:

Recipe:
  type: object
  properties:
    category:
      oneOf: # 👈 category is either Category or null
        - $ref: "#/components/schemas/Category"
        - type: "null"

We have another type InternetRecipe which "inherits" all of Recipe's properties through an allOf ref:

InternetRecipe:
  type: object
  allOf:
    - $ref: "#/components/schemas/Recipe" # 👈 InternetRecipe inherits the category property from Recipe

In the generated code (with openApiNullable set to true), the Recipe class has a properly annotated getter (no @NotNull):

  @Valid 
  @Schema(name = "category", requiredMode = Schema.RequiredMode.REQUIRED)
  @JsonProperty("category")
  public Category getCategory() {
    return category;
  }

The InternetRecipe class however has a getter for the same property annotated with @NotNull even though the property is defined as nullable.

  @NotNull @Valid 
  @Schema(name = "category", requiredMode = Schema.RequiredMode.REQUIRED)
  @JsonProperty("category")
  public Category getCategory() {
    return category;
  }
openapi-generator version

7.16.0

OpenAPI declaration file content or url
openapi: 3.1.0
info:
  version: 1.0.0
  title: NotNull Annotation Reproduction
paths:
  /:
    get:
      operationId: getInternetRecipe
      responses:
        "200":
          description: OK
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/InternetRecipe"
components:
  schemas:
    InternetRecipe:
      type: object
      allOf:
        - $ref: "#/components/schemas/Recipe" # 👈 InternetRecipe inherits the category property from Recipe
      properties:
        url:
          type: string
      required:
        - url
    Recipe:
      type: object
      properties:
        category:
          oneOf: # 👈 category is either Category or null
            - $ref: "#/components/schemas/Category"
            - type: "null"
      required: # 👈 all properties are required in the sense, that they should be present in JSON
        - category
    Category:
      type: object
      properties:
        name:
          type: string
        slug:
          type: string
      required:
        - name
        - slug
Generation Details
Steps to reproduce
git clone git@github.com:luchsamapparat/not-null-annotation-reproduction.git
cd not-null-annotation-reproduction
./mvnw clean generate-sources
Related issues/PRs

https://github.com/OpenAPITools/openapi-generator/issues/21050
https://github.com/OpenAPITools/openapi-generator/issues/14765

Suggest a fix What we've discovered

org.openapitools.codegen.utils.ModelUtils#simplifyOneOfAnyOfWithOnlyOneNonNullSubSchema technically produces an invalid schema. It sets the nullable property of the category's Schema instance to true and pulls the single remaining $ref up from the oneOf array resulting in nullable (not type: null even though we have an OpenAPI 3.1 spec file!) and $ref to both be set on the same property.

https://github.com/OpenAPITools/openapi-generator/blob/57324efd48fccfab54abcdf4718aa17d06be3769/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java#L2264

org.openapitools.codegen.utils.ModelUtils#cloneSchema is called for the allOf in InternetRecipe which clones the category property of Recipe. This delegates to swagger-core which serializes via io.swagger.v3.core.jackson.SchemaSerializer#serialize (see SchemaSerializer.java#L41) the Schema instance from above according to the OpenAPI specification which does not allow $ref and nullable to be on the same property. Thus, it drops the nullable flag and retains only the $ref. This effectively makes this property @NotNull.

https://github.com/OpenAPITools/openapi-generator/blob/57324efd48fccfab54abcdf4718aa17d06be3769/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java#L2240

Finally, when org.openapitools.codegen.DefaultCodegen#fromProperty is invoked for the cloned property schema, it does not set the isNullable template variable to true resulting in the @NotNull annotation being set through our overridden beanValidation mustache template.

https://github.com/OpenAPITools/openapi-generator/blob/57324efd48fccfab54abcdf4718aa17d06be3769/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java#L4046

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

Reproduce the issue with the linked not-null-annotation-reproduction project using ./mvnw clean generate-sources. Read ModelUtils#simplifyOneOfAnyOfWithOnlyOneNonNullSubSchema and cloneSchema, then follow DefaultCodegen#fromProperty for the allOf-cloned property. Done means the nullable category inherited by InternetRecipe retains its nullability and is not generated with @NotNull.

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
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.