Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,12 @@ apiTemplateFiles are for API outputs only (controllers/handlers).
* keyword. For example, the Java code generator may generate 'extends HashMap'.
*/
protected boolean supportsInheritance;
/**
* True if the language generator should not merge oneOf children's properties
* into the parent schema when the parent has a discriminator. This prevents
* child-specific properties from leaking into the parent interface/class.
*/
protected boolean skipOneOfPropertyMergeInParent;
/**
* True if the language generator supports the 'additionalProperties' keyword
* as sibling of a composed (allOf/anyOf/oneOf) schema.
Expand Down Expand Up @@ -2741,6 +2747,10 @@ protected void updateModelForComposedSchema(CodegenModel m, Schema schema, Map<S
"For more details, see https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.9.2.1.3 and the OAS section on 'Composition and Inheritance'.");
}
addVars(m, unaliasPropertySchema(composed.getProperties()), composed.getRequired(), null, null);
if (skipOneOfPropertyMergeInParent) {
// include own properties in allProperties so they appear in allVars
allProperties.putAll(composed.getProperties());
}
}

// parent model
Expand Down Expand Up @@ -2837,6 +2847,10 @@ protected void updateModelForComposedSchema(CodegenModel m, Schema schema, Map<S
} else if (parentName != null && parentName.equals(ref) && supportsInheritance) {
// single inheritance
addProperties(allProperties, allRequired, refSchema, new HashSet<>());
} else if (skipOneOfPropertyMergeInParent && composed.getOneOf() != null && composed.getDiscriminator() != null) {
// polymorphic parent with discriminator and oneOf children —
// these are type alternatives (subtypes), not compositions,
// so their properties should not be merged into the parent
} else {
// composition
Map<String, Schema> newProperties = new LinkedHashMap<>();
Expand Down Expand Up @@ -3706,14 +3720,33 @@ protected void addAdditionPropertiesToCodeGenModel(CodegenModel codegenModel, Sc
}

/**
* Add schema's properties to "properties" and "required" list
* Add schema's properties to "properties" and "required" list.
*
* <p>oneOf/anyOf sub-schema properties are merged into the parent only when
* {@code skipOneOfPropertyMergeInParent} is disabled (model generation default).
*
* @param properties all properties
* @param required required property only
* @param schema schema in which the properties will be added to the lists
* @param visitedSchemas circuit-breaker - the schemas with which the method was called before for recursive structures
*/
protected void addProperties(Map<String, Schema> properties, List<String> required, Schema schema, Set<Schema> visitedSchemas) {
addProperties(properties, required, schema, visitedSchemas, false);
}

/**
* Add schema's properties to "properties" and "required" list.
*
* @param properties all properties
* @param required required property only
* @param schema schema in which the properties will be added to the lists
* @param visitedSchemas circuit-breaker - the schemas with which the method was called before for recursive structures
* @param flattenComposedSchemas when true, oneOf/anyOf sub-schema properties are always flattened into the
* parent regardless of {@code skipOneOfPropertyMergeInParent}. This is required
* for form-data parameters, which need an individual field per alternative,
* unlike model definitions where alternatives are represented as separate types.
*/
protected void addProperties(Map<String, Schema> properties, List<String> required, Schema schema, Set<Schema> visitedSchemas, boolean flattenComposedSchemas) {
if (schema == null) {
return;
}
Expand All @@ -3729,23 +3762,29 @@ protected void addProperties(Map<String, Schema> properties, List<String> requir

if (schema.getAllOf() != null) {
for (Object component : schema.getAllOf()) {
addProperties(properties, required, (Schema) component, visitedSchemas);
addProperties(properties, required, (Schema) component, visitedSchemas, flattenComposedSchemas);
}
}

if (schema.getRequired() != null) {
required.addAll(schema.getRequired());
}

if (schema.getOneOf() != null) {
for (Object component : schema.getOneOf()) {
addProperties(properties, required, (Schema) component, visitedSchemas);
// Note: oneOf and anyOf represent type alternatives, not compositions.
// When skipOneOfPropertyMergeInParent is enabled, their children's properties
// should NOT be merged into the parent schema - unless flattenComposedSchemas is
// requested (e.g. for form-data parameters, which need one field per alternative).
if (flattenComposedSchemas || !skipOneOfPropertyMergeInParent) {
if (schema.getOneOf() != null) {
for (Object component : schema.getOneOf()) {
addProperties(properties, required, (Schema) component, visitedSchemas, flattenComposedSchemas);
}
}
}

if (schema.getAnyOf() != null) {
for (Object component : schema.getAnyOf()) {
addProperties(properties, required, (Schema) component, visitedSchemas);
if (schema.getAnyOf() != null) {
for (Object component : schema.getAnyOf()) {
addProperties(properties, required, (Schema) component, visitedSchemas, flattenComposedSchemas);
}
}
}

Expand All @@ -3754,7 +3793,7 @@ protected void addProperties(Map<String, Schema> properties, List<String> requir

if (StringUtils.isNotBlank(schema.get$ref())) {
Schema interfaceSchema = ModelUtils.getReferencedSchema(this.openAPI, schema);
addProperties(properties, required, interfaceSchema, visitedSchemas);
addProperties(properties, required, interfaceSchema, visitedSchemas, flattenComposedSchemas);
return;
}
if (schema.getProperties() != null) {
Expand Down Expand Up @@ -7444,9 +7483,11 @@ public List<CodegenParameter> fromRequestBodyToFormParameters(RequestBody body,
// TODO in the future have this return one codegenParameter of type object or composed which includes all definition
// that will be needed for complex composition use cases
// https://github.com/OpenAPITools/openapi-generator/issues/10415
addProperties(properties, allRequired, schema, new HashSet<>());

// For form data, always flatten oneOf/anyOf alternatives into individual fields
// regardless of skipOneOfPropertyMergeInParent (which only applies to model generation).
boolean isOneOfOrAnyOf = ModelUtils.isOneOf(schema) || ModelUtils.isAnyOf(schema);
addProperties(properties, allRequired, schema, new HashSet<>(), isOneOfOrAnyOf);

if (!properties.isEmpty()) {
for (Map.Entry<String, Schema> entry : properties.entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@ public KotlinClientCodegen() {

cliOptions.add(CliOption.newBoolean(USE_JACKSON_3,
"Use Jackson 3 dependencies (tools.jackson package). Not yet supported for kotlin-client; reserved for future use."));

skipOneOfPropertyMergeInParent = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Setting skipOneOfPropertyMergeInParent = true unconditionally in the KotlinClientCodegen constructor affects all oneOf/anyOf schemas, not just the oneOf+allOf case described in the PR. In DefaultCodegen.addProperties(), this flag prevents merging oneOf AND anyOf child properties into parent models globally. Since KotlinClientCodegen only supports oneOf/anyOf wrappers for jvm-retrofit2 with gson/kotlinx_serialization, and the default is jvm-okhttp4 with moshi, plain oneOf/anyOf schemas under default or unsupported configurations may lose child properties with no wrapper fallback. Consider scoping this flag to the specific oneOf+allOf/discriminator case or to library/serialization combinations that provide wrapper support.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/KotlinClientCodegen.java, line 311:

<comment>Setting `skipOneOfPropertyMergeInParent = true` unconditionally in the KotlinClientCodegen constructor affects all oneOf/anyOf schemas, not just the oneOf+allOf case described in the PR. In `DefaultCodegen.addProperties()`, this flag prevents merging oneOf AND anyOf child properties into parent models globally. Since KotlinClientCodegen only supports oneOf/anyOf wrappers for `jvm-retrofit2` with `gson`/`kotlinx_serialization`, and the default is `jvm-okhttp4` with `moshi`, plain oneOf/anyOf schemas under default or unsupported configurations may lose child properties with no wrapper fallback. Consider scoping this flag to the specific oneOf+allOf/discriminator case or to library/serialization combinations that provide wrapper support.</comment>

<file context>
@@ -307,6 +307,8 @@ public KotlinClientCodegen() {
         cliOptions.add(CliOption.newBoolean(USE_JACKSON_3,
             "Use Jackson 3 dependencies (tools.jackson package). Not yet supported for kotlin-client; reserved for future use."));
+
+        skipOneOfPropertyMergeInParent = true;
     }
 
</file context>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug affects all serialization libraries equally. Without the fix, moshi, gson, and kotlinx_serialization all produce identical broken output — fat parent interfaces with leaked child properties and sibling contamination.

Plain oneOf without discriminator is NOT affected. These schemas generate a data class wrapper (not an interface), and addProperties() merging is correct — the wrapper IS supposed to contain all variant properties as a union type. Tested and confirmed.

Plain allOf without discriminator is NOT affected. Standard inheritance works identically with and without the fix.

Added 16 new parameterized tests covering all 4 patterns (plain oneOf, oneOf+discriminator, plain allOf, allOf+discriminator) × all 4 serialization libraries (jackson, moshi, gson, kotlinx_serialization) — all pass. The flag is safe to set unconditionally at the KotlinClientCodegen level.

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1955,8 +1955,8 @@ public GeneratorLanguage generatorLanguage() {
* In this case, the second allOf that defines a map with string keys and Pet values will be part of model's property.
*/
@Override
protected void addProperties(Map<String, Schema> properties, List<String> required, Schema schema, Set<Schema> visitedSchemas){
super.addProperties(properties, required, schema, visitedSchemas);
protected void addProperties(Map<String, Schema> properties, List<String> required, Schema schema, Set<Schema> visitedSchemas, boolean flattenComposedSchemas){
super.addProperties(properties, required, schema, visitedSchemas, flattenComposedSchemas);
if(schema.getAdditionalProperties() != null) {
String addtionalPropertiesName = "default_map";
if(schema.getTitle() != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,40 @@ public void paramJsonPropertyAnnotationWithDigitStartingPropertyName() throws IO
"@param:JsonProperty(\"2nd_field\")\n @get:JsonProperty(\"2nd_field\")\n val `2ndField`");
}

@Test
public void testOneOfAllOfDiscriminatorInheritancePropertiesNotLeakedToParent() throws IOException {
File output = Files.createTempDirectory("test").toFile();
output.deleteOnExit();

final CodegenConfigurator configurator = new CodegenConfigurator()
.setGeneratorName("kotlin")
.addAdditionalProperty("serializationLibrary", "jackson")
.setInputSpec("src/test/resources/3_1/polymorphism-allof-and-oneof-discriminator.yaml")
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));

DefaultGenerator generator = new DefaultGenerator();
generator.opts(configurator.toClientOptInput()).generate();

Path petModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Pet.kt");
// don't include child's fields into interface class
TestUtils.assertFileNotContains(petModel, "packSize");
TestUtils.assertFileNotContains(petModel, "huntingSkill");
TestUtils.assertFileContains(petModel, "name");
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.

// cat contains only cat properties + parent
Path catModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Cat.kt");
TestUtils.assertFileContains(catModel, "huntingSkill");
TestUtils.assertFileNotContains(catModel, "packSize");
TestUtils.assertFileContains(catModel, "name");

// dog contains only dog properties + parent
Path dogModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Dog.kt");
TestUtils.assertFileNotContains(dogModel, "huntingSkill");
TestUtils.assertFileContains(dogModel, "packSize");
TestUtils.assertFileContains(dogModel, "name");
}


private static class ModelNameTest {
private final String expectedName;
private final String expectedClassName;
Expand All @@ -1098,4 +1132,148 @@ private ModelNameTest(String expectedName, String expectedClassName) {
this.expectedClassName = expectedClassName;
}
}

@Test(dataProvider = "serializationLibraries")
public void testPlainOneOfWithoutDiscriminatorGeneratesWrapperModel(String serializationLibrary) throws Exception {
// Plain oneOf without discriminator generates a wrapper data class containing all variants' properties.
// This is intentional — without discriminator there's no polymorphism, just a union type.
File output = Files.createTempDirectory("test").toFile();
output.deleteOnExit();

final CodegenConfigurator configurator = new CodegenConfigurator()
.setGeneratorName("kotlin")
.addAdditionalProperty("serializationLibrary", serializationLibrary)
.setInputSpec("src/test/resources/3_0/oneOf.yaml")
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));

new DefaultGenerator().opts(configurator.toClientOptInput()).generate();

Path fruitModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Fruit.kt");
// Fruit is a data class (wrapper) that includes its own prop + all children's props
TestUtils.assertFileContains(fruitModel, "color");
TestUtils.assertFileContains(fruitModel, "kind"); // Apple's property — merged intentionally
TestUtils.assertFileContains(fruitModel, "count"); // Banana's property — merged intentionally
TestUtils.assertFileContains(fruitModel, "sweet"); // Orange's property — merged intentionally

// Children are standalone models with only their own properties
Path appleModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Apple.kt");
TestUtils.assertFileContains(appleModel, "kind");
TestUtils.assertFileNotContains(appleModel, "count");
TestUtils.assertFileNotContains(appleModel, "sweet");

Path bananaModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Banana.kt");
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
TestUtils.assertFileContains(bananaModel, "count");
TestUtils.assertFileNotContains(bananaModel, "kind");
TestUtils.assertFileNotContains(bananaModel, "sweet");

Path orangeModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Orange.kt");
TestUtils.assertFileContains(orangeModel, "sweet");
TestUtils.assertFileNotContains(orangeModel, "kind");
TestUtils.assertFileNotContains(orangeModel, "count");
}

@Test(dataProvider = "serializationLibraries")
public void testOneOfWithDiscriminatorPropertiesNotLeakedToParent(String serializationLibrary) throws Exception {
// oneOf with discriminator: FruitReqDisc has discriminator "fruitType", children have their own props
File output = Files.createTempDirectory("test").toFile();
output.deleteOnExit();

final CodegenConfigurator configurator = new CodegenConfigurator()
.setGeneratorName("kotlin")
.addAdditionalProperty("serializationLibrary", serializationLibrary)
.setInputSpec("src/test/resources/3_0/oneOfDiscriminator.yaml")
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));

new DefaultGenerator().opts(configurator.toClientOptInput()).generate();

Path parentModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/FruitReqDisc.kt");
// Parent should NOT have child-specific properties
TestUtils.assertFileNotContains(parentModel, "seeds"); // AppleReqDisc's property
TestUtils.assertFileNotContains(parentModel, "length"); // BananaReqDisc's property

// Children should have only their own properties + discriminator
Path appleModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/AppleReqDisc.kt");
TestUtils.assertFileContains(appleModel, "seeds");
TestUtils.assertFileNotContains(appleModel, "length");

Path bananaModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/BananaReqDisc.kt");
TestUtils.assertFileContains(bananaModel, "length");
TestUtils.assertFileNotContains(bananaModel, "seeds");
}

@Test(dataProvider = "serializationLibraries")
public void testPlainAllOfWithoutDiscriminatorInheritancePreserved(String serializationLibrary) throws Exception {
// Plain allOf without discriminator: children inherit parent's properties via override
File output = Files.createTempDirectory("test").toFile();
output.deleteOnExit();

final CodegenConfigurator configurator = new CodegenConfigurator()
.setGeneratorName("kotlin")
.addAdditionalProperty("serializationLibrary", serializationLibrary)
.setInputSpec("src/test/resources/3_0/allOf_extension_parent.yaml")
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));

new DefaultGenerator().opts(configurator.toClientOptInput()).generate();

Path personModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Person.kt");
// Person should have its own properties
TestUtils.assertFileContains(personModel, "lastName");
TestUtils.assertFileContains(personModel, "firstName");

// Adult should have its own property + parent properties via inheritance
Path adultModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Adult.kt");
TestUtils.assertFileContains(adultModel, "children");
TestUtils.assertFileContains(adultModel, "lastName");
TestUtils.assertFileContains(adultModel, "firstName");

// Child should have its own property + parent properties via inheritance
Path childModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Child.kt");
TestUtils.assertFileContains(childModel, "age");
TestUtils.assertFileContains(childModel, "lastName");
TestUtils.assertFileContains(childModel, "firstName");
}

@Test(dataProvider = "serializationLibraries")
public void testAllOfWithDiscriminatorPropertiesNotLeakedToParent(String serializationLibrary) throws Exception {
// allOf with discriminator: Pet is parent, Cat/Dog extend it
File output = Files.createTempDirectory("test").toFile();
output.deleteOnExit();

final CodegenConfigurator configurator = new CodegenConfigurator()
.setGeneratorName("kotlin")
.addAdditionalProperty("serializationLibrary", serializationLibrary)
.setInputSpec("src/test/resources/3_1/polymorphism-allof-and-discriminator.yaml")
.setOutputDir(output.getAbsolutePath().replace("\\", "/"));

new DefaultGenerator().opts(configurator.toClientOptInput()).generate();

Path petModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Pet.kt");
// Pet should have only its own property
TestUtils.assertFileContains(petModel, "name");
TestUtils.assertFileNotContains(petModel, "packSize"); // Dog's property
TestUtils.assertFileNotContains(petModel, "huntingSkill"); // Cat's property

// Cat should have its own + parent properties, NOT Dog's
Path catModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Cat.kt");
TestUtils.assertFileContains(catModel, "huntingSkill");
TestUtils.assertFileContains(catModel, "name");
TestUtils.assertFileNotContains(catModel, "packSize");

// Dog should have its own + parent properties, NOT Cat's
Path dogModel = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/client/models/Dog.kt");
TestUtils.assertFileContains(dogModel, "packSize");
TestUtils.assertFileContains(dogModel, "name");
TestUtils.assertFileNotContains(dogModel, "huntingSkill");
}

@DataProvider(name = "serializationLibraries")
public Object[][] serializationLibraries() {
return new Object[][]{
{"jackson"},
{"moshi"},
{"gson"},
{"kotlinx_serialization"},
};
}

}
Loading