[core] Keep the defaultToEmptyContainer default off schemas shared between properties

CP
4h ago
+82-74

Fixes #25522

With defaultToEmptyContainer, DefaultCodegen.fromProperty sets the empty-container default on the resolved schema. For a $ref property that is the shared component schema, and for allOf composition it is the shared inline property schema. Every later property that resolves to the same schema then finds a default already set, skips its own required check, and gets an initializer too. The first reference's required flag decides for all of them, so the output depends on property order.

The default is now taken off the schema again once toDefaultValue and toDefaultValueWithParam have read it, and the schema's default-set flag is restored. Each property is decided by its own required flag, and the exported api/openapi.yaml no longer carries default: [] entries the input never declared. Parameter and response metadata, which read the schema default again after fromProperty, now see the declared default only; the regenerated Java sample's API files are unchanged by that.

I chose this over cloning the schema before the update: ModelUtils.cloneSchema changes untyped {} schemas, which cost User.anyTypeProp and AdditionalPropertiesClass.anytype1 their = null initializer in the okhttp-gson sample. Happy to switch if you prefer the clone.

The okhttp-gson sample (the only config with this option) changes in two places. Its api/openapi.yaml loses the leaked default: [] lines. RequiredNullableBody.objectItemsNullable loses = new HashMap<>(): the config has map? and no map rule, so a required non-nullable map gets no default; it only had one because NullableClass was generated first and leaked it through the shared allOf schema. NullableClass.objectItemsNullable keeps its initializer.

Testing

  • The issue spec generated with -g java --library resttemplate and defaultToEmptyContainer=array on master gives optionalStrings = new ArrayList<>(), and optionalStrings; after swapping the two properties. With the fix both orders give optionalStrings; and requiredStrings = new ArrayList<>().
  • New JavaClientCodegenTest#testDefaultToEmptyContainerDoesNotLeakAcrossSharedSchemaRefs_issue25522 fails on master at the optionalStrings; assertion, and fails at the api/openapi.yaml assertion when only the flag restore is removed. It passes with the fix.
  • JavaClientCodegenTest, DefaultCodegenTest, SpringCodegenTest, DefaultGeneratorTest, JavaJAXRSSpecServerCodegenTest: 1225 tests, 0 failures, 0 errors.
  • Regenerated bin/configs/java-okhttp-gson.yaml with the master jar (no change) and with the fixed jar (the two files above). No other config uses defaultToEmptyContainer.

PR checklist

  • Read the contribution guidelines.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Summary by cubic

Fixes #25522: defaultToEmptyContainer no longer leaks empty-container defaults across schemas shared via $ref or allOf, so each property's initializer is decided by its own required flag instead of the first property that resolved the shared schema.

The default is now read and then removed from the schema again, restoring the declared state. The fix restores the schema's flag rather than cloning because ModelUtils.cloneSchema changes untyped {} schemas and would drop = null initializers in the okhttp-gson sample.

Side effects

  • The exported api/openapi.yaml no longer contains default: [] entries the input never declared.
  • RequiredNullableBody.objectItemsNullable in the okhttp-gson sample loses its = new HashMap<>() initializer; it only had one because NullableClass leaked it through the shared allOf schema.

Testing

  • New test testDefaultToEmptyContainerDoesNotLeakAcrossSharedSchemaRefs_issue25522 fails on master and passes with the fix.
  • 1225 tests pass across the affected test suites, 0 failures.