[BUG][MULTIPLE] 3.1.0 spec, additionalProperties: false with nullable properties generates invalid models

[lesteenman]

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

For our OpenAPI spec using the 3.1.0 specification, we make heavy use of the additionalProperties: false keyword on most of our objects to make validation stricter, both while validating our examples and in the generated server code. By adding they keyword, we ensure we don't get false negatives while validating examples with non-required properties with a typo in the key, and make issues in usage of the APIs clearer.

However, when this keyword is combined with an object with nullable properties, the generated models are invalid (see the example below). I've tested both the Typescript-Angular client generator, the Java OKHTTP client generator and the Java spring-boot API spec generator, and each of them shows the same behaviour: the nullable property is inlined to a new, invalid object resulting in broken code.

openapi-generator version

Tested with version 7.9.0 and 7.10.0, both show the same behaviour.

OpenAPI declaration file content or url

# filename: nullable-properties-with-additional-properties-false.yaml
openapi: 3.1.0
info:
  title: ""
  version: 1.0.0
components:
  schemas:
    # For reference, an object without additionalProperties: false, but with nullable properties
    SampleObject:
      properties:
        someString:
          anyOf:
            - type: string
            - type: 'null'

    # For reference, an object with additionalProperties: false, but without any nullable properties
    ReferenceObject:
      additionalProperties: false
      properties:
        someString:
          type: string

    # The broken case: an object with additionalProperties: false and nullable properties
    SampleObjectWithAdditionalFalse:
      additionalProperties: false
      properties:
        someString:
          anyOf:
            - type: string
            - type: 'null'

Generation Details

Verified with the following configs, and no further settings. Run with java -jar openapi-generator-cli-7.10.0.jar generate --config <config-file-as-shown-in-examples>.yaml and no further customization. I will add additional samples in comments for other generators.

Spring Generator

generatorName: spring
outputDir: ./spring-boot
inputSpec: nullable-properties-with-additional-properties-false.yaml

Generated code below. Note the reference objects to show the expected behaviour:

• For the simplest reference object, a String field with a NOT_REQUIRED annotation.
• For the object

// Example objects leave out their imports, the equals(), hashCode(), toString() and toIntendedString() methods for
some brevity.

/**
 * ReferenceObject
 */
@Generated(value = "org.openapitools.codegen.languages.SpringCodegen", date = "2024-12-01T14:17:54.855204+01:00[Europe/Amsterdam]", comments = "Generator version: 7.10.0")
public class ReferenceObject {

  private String someString;

  public ReferenceObject someString(String someString) {
    this.someString = someString;
    return this;
  }

  /**
   * Get someString
   * @return someString
   */

  @Schema(name = "someString", requiredMode = Schema.RequiredMode.NOT_REQUIRED)
  @JsonProperty("someString")
  public String getSomeString() {
    return someString;
  }

  public void setSomeString(String someString) {
    this.someString = someString;
  }
}

/**
 * ObjectWithAdditionalFalse
 */
@Generated(value = "org.openapitools.codegen.languages.SpringCodegen", date = "2024-12-01T14:17:54.855204+01:00[Europe/Amsterdam]", comments = "Generator version: 7.10.0")
public class ObjectWithAdditionalFalse {

  private JsonNullable<String> someString = JsonNullable.<String>undefined();

  public ObjectWithAdditionalFalse someString(String someString) {
    this.someString = JsonNullable.of(someString);
    return this;
  }

  /**
   * Get someString
   * @return someString
   */

  @Schema(name = "someString", requiredMode = Schema.RequiredMode.NOT_REQUIRED)
  @JsonProperty("someString")
  public JsonNullable<String> getSomeString() {
    return someString;
  }

  public void setSomeString(JsonNullable<String> someString) {
    this.someString = someString;
  }
}

/**
 * ObjectWithNullableProperties
 */
@Generated(value = "org.openapitools.codegen.languages.SpringCodegen", date = "2024-12-01T14:17:54.855204+01:00[Europe/Amsterdam]", comments = "Generator version: 7.10.0")
public class ObjectWithNullableProperties {

  private JsonNullable<String> someString = JsonNullable.<String>undefined();

  public ObjectWithNullableProperties someString(String someString) {
    this.someString = JsonNullable.of(someString);
    return this;
  }

  /**
   * Get someString
   * @return someString
   */

  @Schema(name = "someString", requiredMode = Schema.RequiredMode.NOT_REQUIRED)
  @JsonProperty("someString")
  public JsonNullable<String> getSomeString() {
    return someString;
  }

  public void setSomeString(JsonNullable<String> someString) {
    this.someString = someString;
  }
}

/**
 * BrokenObject
 */
@Generated(value = "org.openapitools.codegen.languages.SpringCodegen", date = "2024-12-01T14:17:54.855204+01:00[Europe/Amsterdam]", comments = "Generator version: 7.10.0")
public class BrokenObject {

  private BrokenObjectSomeString someString;

  public BrokenObject someString(BrokenObjectSomeString someString) {
    this.someString = someString;
    return this;
  }

  /**
   * Get someString
   * @return someString
   */
  @Valid 
  @Schema(name = "someString", requiredMode = Schema.RequiredMode.NOT_REQUIRED)
  @JsonProperty("someString")
  public BrokenObjectSomeString getSomeString() {
    return someString;
  }

  public void setSomeString(BrokenObjectSomeString someString) {
    this.someString = someString;
  }
    /**
    * A container for additional, undeclared properties.
    * This is a holder for any undeclared properties as specified with
    * the 'additionalProperties' keyword in the OAS document.
    */
    private Map<String, Object> additionalProperties;

    /**
    * Set the additional (undeclared) property with the specified name and value.
    * If the property does not already exist, create it otherwise replace it.
    */
    @JsonAnySetter
    public BrokenObject putAdditionalProperty(String key, Object value) {
        if (this.additionalProperties == null) {
            this.additionalProperties = new HashMap<String, Object>();
        }
        this.additionalProperties.put(key, value);
        return this;
    }

    /**
    * Return the additional (undeclared) property.
    */
    @JsonAnyGetter
    public Map<String, Object> getAdditionalProperties() {
        return additionalProperties;
    }

    /**
    * Return the additional (undeclared) property with the specified name.
    */
    public Object getAdditionalProperty(String key) {
        if (this.additionalProperties == null) {
            return null;
        }
        return this.additionalProperties.get(key);
    }

}

/**
 * BrokenObjectSomeString
 */
@JsonTypeName("SampleObjectWithAdditionalFalse_someString")
@Generated(value = "org.openapitools.codegen.languages.SpringCodegen", date = "2024-12-01T14:17:54.855204+01:00[Europe/Amsterdam]", comments = "Generator version: 7.10.0")
public class BrokenObjectSomeString {
  // This object only has the equals(), hashCode(), toString() and toIntendedString() methods.
}

Note that I've also tried the config below, which seems to make practically no difference - except that the incorrectly introduced wrapper object also gets additionalProperties if set to false:

generatorName: spring
outputDir: ./spring-boot
inputSpec: nullable-properties-with-additional-properties-false.yaml
additionalProperties:
  disallowAdditionalPropertiesIfNotPresent: true

Resulting in the following BrokenObjectSomeString.java:

/**
 * BrokenObjectSomeString
 */

@JsonTypeName("BrokenObject_someString")
@Generated(value = "org.openapitools.codegen.languages.SpringCodegen", date = "2024-12-01T14:35:50.542378+01:00[Europe/Amsterdam]", comments = "Generator version: 7.10.0")
public class BrokenObjectSomeString {
    /**
    * A container for additional, undeclared properties.
    * This is a holder for any undeclared properties as specified with
    * the 'additionalProperties' keyword in the OAS document.
    */
    private Map<String, Object> additionalProperties;

   // Along with all other fields and methods expected of a model which allows additionalProperties. Note that the underlying type was a String, not an object!
}

Steps to reproduce

Create a file with the contents as above - or just any OpenAPI 3.1.0 spec where an object has both additionalProperties: false and any nullable field, done by adding a oneOf or anyOf where one option is the actual type of the field, and the other is { "type": "null" }.

Run a generate task with any of the mentioned generators (likely others as well).

Related issues/PRs

None that I can find. Can supply one if I find a solution.

Suggest a fix

Researching one, though I already want to have the issue filed for reference.

[wing328]

if you down grade the spec to 3.0.1, did you get the same issue?

just want to know if the issue only occurs with 3.1.0 spec.

[lesteenman]

Under 3.0.1, the generated models behave as I would expect: no unnecessary additionalProperties maps, no incorrectly inlined wrapper for the nullable generated. So this definitely is an issue with the 3.1.0 spec. Given the large variety of odd behaviours I observe in this test case however, finding a fix does feel a little bit daunting - though given that multiple if not all generators seem impacted, definitely worth looking into.

I do remember us needing some other feature unique since 3.1.0 so downgrading is likely not an option for us, although I can't recall off the top of my head what it was.

[lesteenman]

Going by the discussion on OAI/OpenAPI-Specification#3148, which matches what I've found elsewhere but decently summarizes it, nullable properties in a 3.1 specification can be one of the following

openapi: '3.1.0'
components:
  schemas:
    Foo:
      type: 'object'
      properties:
        bar:
          type: ['string', 'null']
          # This style still allows us to set other properties only valid for the 'string' type
          format: 'uri'
    Bar:
      type: 'object'
      properties:
        bar:
          oneOf:
            - type: 'string'
              format: 'uri'
            - type: 'null'

As a comparison, in 3.0, it would be done with an explicit nullable attribute:

    User:
      type: object
      properties:
        address:
          type: object
          nullable: true
          properties:
            city:
              type: string

The above is from one of the tests' examples in this project.

Now, I think the easiest way to handle this would be to modify the default generator (assuming that's where this is parsed - but I haven't gotten to the implementation stage of a fix yet) to:

• consider an anyOf with only one other type and one null type to be the other type as nullable, so any further generators can just stick to their old behaviour.
• if there are multiple options in the anyOf block, still "consume" the type: null entry, and make all other options nullable.

I expect that the alternative would be to consider both nullable: true and an anyOf containing null at various other points downstream. I do expect my proposal to be a potential pain to implement if the anyOf is already converted to some inlined schema at that point, as initial testing did lead me to the InlineModelResolver a couple of times for 3.1 related bugs, but we'll see.

@wing328, I'm assuming you're the right person to ask here - does this proposed behaviour sound acceptable? If so, I can make an attempt for a PR.

[lesteenman]

Also - it seems that the example openapi validator in the Github issue template is no longer a valid URL.

[lesteenman]

Actually, it seems support for this has been in the project for a long time already:

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java

Line 1737 in a447b59

// In OAS 3.1, the recommended way to define a nullable property or object is to use oneOf.

, which was added almost 2 years back. However, it's a check that must be used by every individual codegen rather than modifying the schema type itself to be nullable, contrary to what I proposed.

Given that existing code, I suppose that implementing this same logic for Java would be necessary to support nullable types in 3.1... But I expect the original issue I reported to not be resolved by just that.