Skip to content
Merged
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
1 change: 1 addition & 0 deletions .github/workflows/samples-kotlin-server-jdk17.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ jobs:
matrix:
sample:
# server
- samples/server/others/kotlin-springboot/allOf-multilevel
- samples/server/others/kotlin-springboot/oneOf-discriminator
- samples/server/others/kotlin-springboot/oneOf-discriminator-const
- samples/server/others/kotlin-springboot/oneOf-enum-discriminator
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/samples-kotlin-server-jdk21.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ jobs:
fail-fast: false
matrix:
sample:
- samples/server/others/kotlin-springboot/allOf-multilevel
- samples/server/others/kotlin-springboot/oneOf-discriminator
- samples/server/others/kotlin-springboot/oneOf-discriminator-const
- samples/server/others/kotlin-springboot/oneOf-enum-discriminator
Expand Down
11 changes: 11 additions & 0 deletions bin/configs/kotlin-spring-boot-allof-multilevel.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
generatorName: kotlin-spring
outputDir: samples/server/others/kotlin-springboot/allOf-multilevel
library: spring-boot
inputSpec: modules/openapi-generator/src/test/resources/3_1/allof-multilevel-inheritance.yaml
templateDir: modules/openapi-generator/src/main/resources/kotlin-spring
additionalProperties:
documentationProvider: none
annotationLibrary: none
useSwaggerUI: "false"
interfaceOnly: "true"
useSpringBoot3: "true"
Original file line number Diff line number Diff line change
Expand Up @@ -1507,6 +1507,67 @@ public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs)
}
}

// Multi-level allOf inheritance: detect "mid-level" models — models that (a) have at least
// one child via allOf and (b) are themselves a child (have a parent) but are NOT a
// discriminator root. A data class cannot be subclassed, so these must become `open class`
// for their own subclasses to compile.
// Example: Animal (interface) <- Dog (open class, has child BigDog) <- BigDog (data class)
for (CodegenModel cm : allModelsMap.values()) {
boolean isMidLevel = cm.hasChildren
&& cm.discriminator == null
&& cm.parent != null
&& !Boolean.TRUE.equals(cm.vendorExtensions.get(CodegenConstants.X_IS_ONE_OF_INTERFACE));
if (isMidLevel) {
// Mark for `open class` rendering in the template
cm.vendorExtensions.put("x-is-open-class", true);
// Mark every *own* (non-inherited) property as `open` so subclasses can override it.
// Inherited properties (override) are implicitly open in an open class.
Stream.of(cm.vars, cm.requiredVars, cm.optionalVars, cm.allVars)
.flatMap(List::stream)
.filter(p -> !p.isInherited)
.forEach(p -> p.vendorExtensions.put("x-is-open-property", true));
// An open class gets none of the compiler-generated data class members, so the
// template writes equals/hashCode/toString/copy itself. They must list properties
// in the same order the constructor declares them (required before optional), so
// that positional arguments to copy() bind to the properties the caller expects.
List<CodegenProperty> constructorOrder = new ArrayList<>(cm.getRequiredVars());
constructorOrder.addAll(cm.getOptionalVars());
// toString() prints the property name as a label. Kotlin back-ticks names that are
// not valid identifiers (`2ndField`), but the back-ticks are only source syntax and
// a data class does not print them, so strip them for the label.
constructorOrder.forEach(p -> p.vendorExtensions.put(
"x-open-class-label", p.getName().replace("`", "")));
cm.vendorExtensions.put("x-open-class-vars", constructorOrder);
}
}

// For children of open (non-interface) parent classes, build a parent constructor call
// so the template can emit `: Dog(className = className, ...)`. This is a second pass
// because it reads x-is-open-class on the *parent*, which the loop above must have
// finished setting for every model first.
// x-parent-is-class tells the template the parent requires `()` (even when arg list is empty);
// x-parent-ctor-args holds the argument string. Kept separate so a parent with no properties
// still generates `: ParentClass()` rather than the compile-error `: ParentClass` (no parens).
for (CodegenModel cm : allModelsMap.values()) {
if (cm.parent != null) {
CodegenModel parentModel = allModelsMap.get(cm.parent);
if (parentModel != null
&& Boolean.TRUE.equals(parentModel.vendorExtensions.get("x-is-open-class"))) {
cm.vendorExtensions.put("x-parent-is-class", true);
List<String> ctorArgs = new ArrayList<>();
for (CodegenProperty prop : parentModel.getRequiredVars()) {
ctorArgs.add(prop.getName() + " = " + prop.getName());
}
for (CodegenProperty prop : parentModel.getOptionalVars()) {
ctorArgs.add(prop.getName() + " = " + prop.getName());
}
if (!ctorArgs.isEmpty()) {
cm.vendorExtensions.put("x-parent-ctor-args", String.join(", ", ctorArgs));
}
}
}
}

if (substituteGenericPagedModel && !pagedModelRegistry.isEmpty()) {
if (getAnnotationLibrary() == AnnotationLibrary.NONE) {
// No @ApiResponse annotations are generated when annotationLibrary=none,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
{{#vendorExtensions.x-class-extra-annotation}}
{{{.}}}
{{/vendorExtensions.x-class-extra-annotation}}
{{#discriminator}}interface {{classname}}{{/discriminator}}{{^discriminator}}{{#hasVars}}data {{/hasVars}}class {{classname}}(
{{#discriminator}}interface {{classname}}{{/discriminator}}{{^discriminator}}{{#vendorExtensions.x-is-open-class}}open {{/vendorExtensions.x-is-open-class}}{{^vendorExtensions.x-is-open-class}}{{#hasVars}}data {{/hasVars}}{{/vendorExtensions.x-is-open-class}}class {{classname}}(
{{#requiredVars}}
{{>dataClassReqVar}}{{^-last}},
{{/-last}}{{/requiredVars}}{{#hasRequired}}{{#hasOptional}},
{{/hasOptional}}{{/hasRequired}}{{#optionalVars}}{{>dataClassOptVar}}{{^-last}},
{{/-last}}{{/optionalVars}}
){{/discriminator}}{{! no newline
}}{{#parent}} : {{{.}}}{{#isMap}}(){{/isMap}}{{! no newline
}}{{#parent}} : {{{.}}}{{#vendorExtensions.x-parent-is-class}}({{{vendorExtensions.x-parent-ctor-args}}}){{/vendorExtensions.x-parent-is-class}}{{^vendorExtensions.x-parent-is-class}}{{#isMap}}(){{/isMap}}{{/vendorExtensions.x-parent-is-class}}{{! no newline
}}{{#vendorExtensions.x-kotlin-implements}}, {{{.}}}{{/vendorExtensions.x-kotlin-implements}}{{! <- serializableModel is also handled via x-kotlin-implements
}}{{#vendorExtensions.x-implements-sealed-interfaces}}{{#.}}, {{{.}}}{{/.}}{{/vendorExtensions.x-implements-sealed-interfaces}}{{! <- add sealed interface implementations
}}{{/parent}}{{! no newline
Expand All @@ -43,6 +43,33 @@
{{>interfaceOptVar}}{{! prevent indent}}
{{/optionalVars}}
{{/discriminator}}
{{#vendorExtensions.x-is-open-class}}
override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other?.javaClass != javaClass) return false
other as {{classname}}
return {{#vendorExtensions.x-open-class-vars}}{{{name}}} == other.{{{name}}}{{^-last}}
&& {{/-last}}{{/vendorExtensions.x-open-class-vars}}{{^vendorExtensions.x-open-class-vars}}true{{/vendorExtensions.x-open-class-vars}}
}

override fun hashCode(): Int {
return Objects.hash({{#vendorExtensions.x-open-class-vars}}{{{name}}}{{^-last}}, {{/-last}}{{/vendorExtensions.x-open-class-vars}})
}

override fun toString(): String {
return "{{classname}}(" +
{{#vendorExtensions.x-open-class-vars}}
"{{{vendorExtensions.x-open-class-label}}}=" + {{{name}}}{{^-last}} + ", " +{{/-last}}{{#-last}} +{{/-last}}
{{/vendorExtensions.x-open-class-vars}}
")"
}

fun copy(
{{#vendorExtensions.x-open-class-vars}}
{{{name}}}: {{{dataType}}}{{^required}}?{{/required}} = this.{{{name}}}{{^-last}},{{/-last}}

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.

P1: copy does not mirror the constructor property type: inline enums use Class.Enum, required nullable fields use T?, and openApiNullable fields use JsonNullable<T>. These mismatches make generated mid-level models fail Kotlin compilation; render the same type branches as the property partials.

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/resources/kotlin-spring/dataClass.mustache, line 69:

<comment>`copy` does not mirror the constructor property type: inline enums use `Class.Enum`, required nullable fields use `T?`, and `openApiNullable` fields use `JsonNullable<T>`. These mismatches make generated mid-level models fail Kotlin compilation; render the same type branches as the property partials.</comment>

<file context>
@@ -43,6 +43,33 @@
+
+    fun copy(
+{{#vendorExtensions.x-open-class-vars}}
+        {{{name}}}: {{{dataType}}}{{^required}}?{{/required}} = this.{{{name}}}{{^-last}},{{/-last}}
+{{/vendorExtensions.x-open-class-vars}}
+    ): {{classname}} = {{classname}}({{#vendorExtensions.x-open-class-vars}}{{{name}}} = {{{name}}}{{^-last}}, {{/-last}}{{/vendorExtensions.x-open-class-vars}})
</file context>
Suggested change
{{{name}}}: {{{dataType}}}{{^required}}?{{/required}} = this.{{{name}}}{{^-last}},{{/-last}}
{{{name}}}: {{#vendorExtensions.x-is-jackson-optional-nullable}}JsonNullable<{{#isEnum}}{{#isArray}}{{baseType}}<{{/isArray}}{{classname}}.{{{nameInPascalCase}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}>{{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{#isEnum}}{{#isArray}}{{baseType}}<{{/isArray}}{{classname}}.{{{nameInPascalCase}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}{{#isNullable}}?{{/isNullable}}{{^required}}{{^isNullable}}?{{/isNullable}}{{/required}}{{/vendorExtensions.x-is-jackson-optional-nullable}} = this.{{{name}}}{{^-last}},{{/-last}}

{{/vendorExtensions.x-open-class-vars}}
): {{classname}} = {{classname}}({{#vendorExtensions.x-open-class-vars}}{{{name}}} = {{{name}}}{{^-last}}, {{/-last}}{{/vendorExtensions.x-open-class-vars}})
{{/vendorExtensions.x-is-open-class}}
{{#hasEnums}}{{#vars}}{{#isEnum}}
/**
* {{{description}}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@
@field:JsonSetter(nulls = Nulls.SKIP){{/vendorExtensions.x-has-json-setter-nulls-skip}}{{#vendorExtensions.x-has-json-setter-nulls-fail}}
@field:JsonSetter(nulls = Nulls.FAIL){{/vendorExtensions.x-has-json-setter-nulls-fail}}
@param:JsonProperty("{{{baseName}}}")
@get:JsonProperty("{{{baseName}}}"){{#isInherited}} override{{/isInherited}} {{>modelMutable}} {{{name}}}: {{#vendorExtensions.x-is-jackson-optional-nullable}}JsonNullable<{{#isEnum}}{{#isArray}}{{baseType}}<{{/isArray}}{{classname}}.{{{nameInPascalCase}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}>{{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{#isEnum}}{{#isArray}}{{baseType}}<{{/isArray}}{{classname}}.{{{nameInPascalCase}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}?{{/vendorExtensions.x-is-jackson-optional-nullable}} = {{#vendorExtensions.x-is-jackson-optional-nullable}}JsonNullable.undefined(){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{^defaultValue}}null{{/defaultValue}}{{#defaultValue}}{{^isNumber}}{{{defaultValue}}}{{/isNumber}}{{#isNumber}}{{{dataType}}}("{{{defaultValue}}}"){{/isNumber}}{{/defaultValue}}{{/vendorExtensions.x-is-jackson-optional-nullable}}
@get:JsonProperty("{{{baseName}}}"){{#isInherited}} override{{/isInherited}}{{^isInherited}}{{#vendorExtensions.x-is-open-property}} open{{/vendorExtensions.x-is-open-property}}{{/isInherited}} {{>modelMutable}} {{{name}}}: {{#vendorExtensions.x-is-jackson-optional-nullable}}JsonNullable<{{#isEnum}}{{#isArray}}{{baseType}}<{{/isArray}}{{classname}}.{{{nameInPascalCase}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}>{{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{#isEnum}}{{#isArray}}{{baseType}}<{{/isArray}}{{classname}}.{{{nameInPascalCase}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}?{{/vendorExtensions.x-is-jackson-optional-nullable}} = {{#vendorExtensions.x-is-jackson-optional-nullable}}JsonNullable.undefined(){{/vendorExtensions.x-is-jackson-optional-nullable}}{{^vendorExtensions.x-is-jackson-optional-nullable}}{{^defaultValue}}null{{/defaultValue}}{{#defaultValue}}{{^isNumber}}{{{defaultValue}}}{{/isNumber}}{{#isNumber}}{{{dataType}}}("{{{defaultValue}}}"){{/isNumber}}{{/defaultValue}}{{/vendorExtensions.x-is-jackson-optional-nullable}}
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
@field:JsonSetter(nulls = Nulls.SKIP){{/vendorExtensions.x-has-json-setter-nulls-skip}}{{#vendorExtensions.x-has-json-setter-nulls-fail}}
@field:JsonSetter(nulls = Nulls.FAIL){{/vendorExtensions.x-has-json-setter-nulls-fail}}
@param:JsonProperty("{{{baseName}}}", required = true)
@get:JsonProperty("{{{baseName}}}", required = true){{#isInherited}} override{{/isInherited}} {{>modelMutable}} {{{name}}}: {{#isEnum}}{{#isArray}}{{baseType}}<{{/isArray}}{{classname}}.{{{nameInPascalCase}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}{{#isNullable}}?{{/isNullable}}{{#defaultValue}} = {{^isNumber}}{{{defaultValue}}}{{/isNumber}}{{#isNumber}}{{{dataType}}}("{{{defaultValue}}}"){{/isNumber}}{{/defaultValue}}
@get:JsonProperty("{{{baseName}}}", required = true){{#isInherited}} override{{/isInherited}}{{^isInherited}}{{#vendorExtensions.x-is-open-property}} open{{/vendorExtensions.x-is-open-property}}{{/isInherited}} {{>modelMutable}} {{{name}}}: {{#isEnum}}{{#isArray}}{{baseType}}<{{/isArray}}{{classname}}.{{{nameInPascalCase}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}{{#isNullable}}?{{/isNullable}}{{#defaultValue}} = {{^isNumber}}{{{defaultValue}}}{{/isNumber}}{{#isNumber}}{{{dataType}}}("{{{defaultValue}}}"){{/isNumber}}{{/defaultValue}}
Original file line number Diff line number Diff line change
Expand Up @@ -7800,4 +7800,108 @@ public void extraImportsDedupAgainstGeneratedImports() throws IOException {
Assert.assertEquals(countOccurrences(widgets, "import org.openapitools.model.Widget"), 1L,
"Extra import duplicating a generated type import must be emitted only once");
}

// ==================== multi-level allOf inheritance tests (fixes #18206) ====================

@Test(description = "multi-level allOf: mid-level model becomes open class, leaf stays data class with parent ctor call")
public void testMultiLevelAllOfInheritance() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

new DefaultGenerator().opts(new ClientOptInput()
.openAPI(new OpenAPIParser().readLocation("src/test/resources/3_1/allof-multilevel-inheritance.yaml", null, new ParseOptions()).getOpenAPI())
.config(new KotlinSpringServerCodegen() {{
setOutputDir(output.getAbsolutePath());
}}))
.generate();

String outputPath = output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/model";

// Animal: discriminator parent interface, with Jackson annotations listing ALL descendants
assertFileContains(Paths.get(outputPath + "/Animal.kt"),
"interface Animal",
"@JsonTypeInfo", "property = \"className\"", "visible = true",
"BigDog::class",
"Dog::class",
"Cat::class"
);

// Dog: open class (not data class) so BigDog can extend it; no ctor call to Animal (interface parent)
assertFileContains(Paths.get(outputPath + "/Dog.kt"),
"open class Dog(",
") : Animal {",
"override val className: kotlin.String",
"open val breed: kotlin.String?"
);
assertFileNotContains(Paths.get(outputPath + "/Dog.kt"), "data class Dog");
assertFileNotContains(Paths.get(outputPath + "/Dog.kt"), ": Animal(");
// open class generates equals/hashCode/toString/copy since data class would normally provide these
assertFileContains(Paths.get(outputPath + "/Dog.kt"),
"override fun equals(other: Any?): Boolean",
"if (other?.javaClass != javaClass) return false",
"override fun hashCode(): Int",
"Objects.hash(",
"override fun toString(): String",
"fun copy(",
"): Dog = Dog("
);

// Cat: leaf (no children) — stays a data class, unaffected
assertFileContains(Paths.get(outputPath + "/Cat.kt"), "data class Cat");

// BigDog: data class extending open class Dog with constructor call passing all parent ctor args
assertFileContains(Paths.get(outputPath + "/BigDog.kt"),
"data class BigDog(",
"override val className: kotlin.String",
"override val breed",
"override val color"
);
// Must call Dog's constructor (not bare `: Dog`)
assertFileContains(Paths.get(outputPath + "/BigDog.kt"), ") : Dog(");
assertFileNotContains(Paths.get(outputPath + "/BigDog.kt"), ") : Dog {");

// copy() must declare parameters in constructor order (required before optional), so that
// positional arguments bind to the same properties the constructor takes them for.
assertFileContains(Paths.get(outputPath + "/Dog.kt"),
" fun copy(\n"
+ " className: kotlin.String = this.className,\n"
+ " breed: kotlin.String? = this.breed,\n"
+ " color: kotlin.String? = this.color\n"
+ " ): Dog = Dog(className = className, breed = breed, color = color)"
);
}

@Test(description = "multi-level allOf: generated open class members handle back-ticked property names")
public void testMultiLevelAllOfWithEscapedPropertyNames() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

new DefaultGenerator().opts(new ClientOptInput()
.openAPI(new OpenAPIParser().readLocation("src/test/resources/3_1/allof-multilevel-escaped-names.yaml", null, new ParseOptions()).getOpenAPI())
.config(new KotlinSpringServerCodegen() {{
setOutputDir(output.getAbsolutePath());
}}))
.generate();

Path mid = Paths.get(output.getAbsolutePath() + "/src/main/kotlin/org/openapitools/model/Mid.kt");

// "2nd_field" and "object" are not Kotlin identifiers, so the property is back-ticked.
// The generated members must emit the back-ticks literally, not HTML-escape them.
// (Scoped to the generated members: the KDoc @param block escapes them for every model,
// open class or not, which is a separate pre-existing issue.)
assertFileNotContains(mid,
"&#x60;2ndField&#x60; == other",
"Objects.hash(kind, &#x60;"
);
assertFileContains(mid,
"open val `2ndField`",
"`2ndField` == other.`2ndField`",
"Objects.hash(kind, `2ndField`, `object`)",
"`2ndField`: kotlin.String? = this.`2ndField`"
);
// toString builds by concatenation: "$`2ndField`" is not valid Kotlin interpolation,
// and the back-ticks are source syntax that a data class would not print.
assertFileContains(mid, "\"2ndField=\" + `2ndField`");
assertFileNotContains(mid, "$`2ndField`");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
openapi: 3.1.0
info:
title: Multi-level allOf with property names that are not Kotlin identifiers
version: "1.0"
paths: {}
components:
schemas:
Base:
type: object
discriminator:
propertyName: kind
required:
- kind
properties:
kind:
type: string
# Mid has a child, so it renders as an `open class` whose equals/hashCode/
# toString/copy are generated rather than supplied by the compiler. Its
# properties need back-ticks in Kotlin, which those members must handle.
Mid:
allOf:
- $ref: '#/components/schemas/Base'
- type: object
properties:
2nd_field:
type: string
object:
type: string
Leaf:
allOf:
- $ref: '#/components/schemas/Mid'
- type: object
properties:
extra:
type: string
Loading
Loading