Skip to content

Commit f6aec65

Browse files
petermetzpetermetz
committed
fix(kotlin-spring): add parentCtorCallNeeded model template property
The new parentCtorCallNeeded property can be used by the template to determine if there should be a constructor invocation on the parent type. Right now its value is computed in a way that could probably use some improvement because it is language specifically checking if the base type is a kotlin map or not. This might seem overly brittle, but was the only way I could differentiate between the two different edge cases that are present in the repro spec. This was the previous commit that changed the default behavior from one edge case to the other (both are wrong because we can only know for sure at generation time whether a constructor call will be necessary or not). 0bb08a7 Below is a yaml OpenAPI spec document that can be used to test/verify the currently known edge cases (which were also mentioned in the GH issue tracker's comments) ```yaml openapi: 3.0.3 info: version: "1.0.0" title: "" paths: /documents/v1: get: responses: 200: description: lorem content: application/json: schema: type: array items: $ref: "#/components/schemas/ParentSchema" components: schemas: JarFiles: type: array items: $ref: "#/components/schemas/JarFile" JarFile: type: object required: - filename - contentBase64 - hasDbMigrations additionalProperties: true properties: filename: type: string nullable: false minLength: 1 maxLength: 255 hasDbMigrations: description: Indicates whether the cordapp jar in question contains any embedded migrations that Cactus can/should execute between copying the jar into the cordapp directory and starting the node back up. type: boolean nullable: false contentBase64: type: string format: base64 nullable: false minLength: 1 maxLength: 1073741824 ParentSchema: type: object properties: id: type: string type: $ref: "#/components/schemas/DiscriminatingType" required: - id - type discriminator: propertyName: type mapping: subtypeA: "#/components/schemas/SubtypeA" subtypeB: "#/components/schemas/SubtypeB" subtypeC: "#/components/schemas/SubtypeC" SubtypeA: type: object allOf: - $ref: '#/components/schemas/ParentSchema' - type: object properties: subtypeAproperty: type: integer SubtypeB: type: object allOf: - $ref: '#/components/schemas/ParentSchema' - type: object properties: subtypeBproperty: type: string SubtypeC: type: object additionalProperties: true allOf: - $ref: '#/components/schemas/ParentSchema' - type: object properties: subtypeAproperty: type: integer DiscriminatingType: type: string enum: - subtypeA - subtypeB - subtypeC ``` Fixes #8366 Signed-off-by: Peter Somogyvari <peter.metz@unarin.com> Signed-off-by: Peter Metz <peter.metz@unarin.com>
1 parent b3f0967 commit f6aec65

33 files changed

Lines changed: 1469 additions & 3 deletions

File tree

‎.github/workflows/samples-kotlin-server.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ jobs:
4141
- samples/server/petstore/kotlin-springboot-source-swagger2
4242
- samples/server/petstore/kotlin-springboot-x-kotlin-implements
4343
- samples/server/petstore/kotlin-springboot-include-http-request-context-delegate
44+
- samples/server/petstore/kotlin-spring-issue8366-inheritance-parent-ctor-call
4445
- samples/server/petstore/kotlin-server/ktor2
4546
- samples/server/petstore/kotlin-server/jaxrs-spec
4647
- samples/server/petstore/kotlin-server/jaxrs-spec-mutiny

‎bin/configs/kotlin-spring-issue8366-inheritance-parent-ctor-call.yaml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
generatorName: kotlin-spring
2+
outputDir: samples/server/petstore/kotlin-spring-issue8366-inheritance-parent-ctor-call
3+
library: spring-boot
4+
inputSpec: modules/openapi-generator/src/test/resources/3_0/kotlin/issue8366-inheritance-parent-ctor-call.yaml
5+
templateDir: modules/openapi-generator/src/main/resources/kotlin-spring
6+
additionalProperties:
7+
documentationProvider: springdoc
8+
annotationLibrary: swagger2
9+
useSwaggerUI: "true"
10+
serviceImplementation: "true"
11+
reactive: "true"
12+
beanValidations: "false"

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractKotlinCodegen.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ public enum KotlinEnumNamingType {
123123
// ref: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-hash-map/
124124
protected Set<String> propertyAdditionalKeywords = new HashSet<>(Arrays.asList("entries", "keys", "size", "values"));
125125

126+
private final List<Pattern> PARENT_TYPES_NEEDING_CTOR_CALL = List.of(Pattern.compile("^kotlin\\.collections\\.HashMap.*"));
127+
126128
private final Map<String, String> schemaKeyToModelNameCache = new HashMap<>();
127129
@Getter @Setter
128130
protected List<String> additionalModelTypeAnnotations = new LinkedList<>();
@@ -498,10 +500,46 @@ public ModelsMap postProcessModels(ModelsMap objs) {
498500
break;
499501
}
500502
}
503+
504+
final boolean isParentCtorCallNeeded = isParentCtorCallNeeded(cm);
505+
cm.vendorExtensions.put("x-is-parent-ctor-call-needed", isParentCtorCallNeeded);
501506
}
502507
return postProcessModelsEnum(objs);
503508
}
504509

510+
/**
511+
* Determines if a constructor call is needed for the parent type of a model
512+
* and if yes, the invocation syntax `()` is added in the generated code.
513+
*
514+
* Important note: This is kotlin specific at the moment, but could be
515+
* extended with to support detection for other languages as well.
516+
*
517+
* The generator used to default to issuing a supertype
518+
* constructor call and then it was changed to default to not doing it.
519+
* Both are wrong since there are different cases and the decision has to be
520+
* made at runtime based on the parent type itself.
521+
*
522+
* For example if it is a HashMap (because you set
523+
* additionalProperties: true for example) then it MUST HAVE a parent
524+
* constructor call. If the parent type is an interface because you are
525+
* using allOf with a discriminator property then the generated code for the
526+
* model will have it's parent type as the interface which MUST NOT HAVE a
527+
* parent constructor call.
528+
*
529+
* @see https://github.com/OpenAPITools/openapi-generator/issues/8366
530+
*
531+
* @return {boolean} `true` if the () call syntax is needed, false otherwise.
532+
*/
533+
protected boolean isParentCtorCallNeeded(CodegenModel cm) {
534+
final String parent = cm.getParent();
535+
536+
if (parent != null) {
537+
return PARENT_TYPES_NEEDING_CTOR_CALL.stream()
538+
.anyMatch(pattern -> pattern.matcher(parent).matches());
539+
}
540+
return false;
541+
}
542+
505543
@Override
506544
public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<ModelMap> allModels) {
507545
handleImplicitHeaders(objs);

‎modules/openapi-generator/src/main/resources/kotlin-spring/dataClass.mustache‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@
2121
{{/hasOptional}}{{/hasRequired}}{{#optionalVars}}{{>dataClassOptVar}}{{^-last}},
2222
{{/-last}}{{/optionalVars}}
2323
){{/discriminator}}{{! no newline
24-
}}{{#parent}} : {{{.}}}{{#isMap}}(){{/isMap}}{{! no newline
25-
}}{{#vendorExtensions.x-kotlin-implements}}, {{{.}}}{{/vendorExtensions.x-kotlin-implements}}{{! <- serializableModel is also handled via x-kotlin-implements
26-
}}{{#vendorExtensions.x-implements-sealed-interfaces}}{{#.}}, {{{.}}}{{/.}}{{/vendorExtensions.x-implements-sealed-interfaces}}{{! <- add sealed interface implementations
24+
}}{{#parent}} : {{{.}}}{{#vendorExtensions.x-is-parent-ctor-call-needed}}(){{/vendorExtensions.x-is-parent-ctor-call-needed}}{{! no newline
25+
}}{{#vendorExtensions.x-kotlin-implements}}, {{{.}}}{{/vendorExtensions.x-kotlin-implements}}{{! no newline
26+
}}{{#vendorExtensions.x-implements-sealed-interfaces}}{{#.}}, {{{.}}}{{/.}}{{/vendorExtensions.x-implements-sealed-interfaces}}{{! no newline
2727
}}{{/parent}}{{! no newline
2828
}}{{^parent}}{{! no newline
2929
}}{{#vendorExtensions.x-kotlin-implements}}{{! no newline

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/kotlin/spring/KotlinSpringServerCodegenTest.java‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,108 @@ public void generateSerializableModelWithXimplementsSkipAndSchemaImplements() th
12481248
);
12491249
}
12501250

1251+
@Test
1252+
public void generateSerializableModelWithParentCtorCallNeeded() throws Exception {
1253+
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
1254+
output.deleteOnExit();
1255+
String outputPath = output.getAbsolutePath().replace('\\', '/');
1256+
1257+
KotlinSpringServerCodegen codegen = new KotlinSpringServerCodegen();
1258+
codegen.setOutputDir(output.getAbsolutePath());
1259+
codegen.additionalProperties().put(CodegenConstants.SERIALIZABLE_MODEL, true);
1260+
1261+
ClientOptInput input = new ClientOptInput()
1262+
.openAPI(TestUtils.parseSpec("src/test/resources/3_0/kotlin/issue8366-inheritance-parent-ctor-call.yaml"))
1263+
.config(codegen);
1264+
DefaultGenerator generator = new DefaultGenerator();
1265+
1266+
generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
1267+
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_TESTS, "false");
1268+
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_DOCS, "false");
1269+
generator.setGeneratorPropertyDefault(CodegenConstants.APIS, "false");
1270+
generator.setGeneratorPropertyDefault(CodegenConstants.SUPPORTING_FILES, "false");
1271+
1272+
generator.opts(input).generate();
1273+
1274+
Path path = Paths.get(outputPath + "/src/main/kotlin/org/openapitools/model/JarFile.kt");
1275+
assertFileContains(
1276+
path,
1277+
") : kotlin.collections.HashMap<String, kotlin.Any>(), java.io.Serializable {",
1278+
"private const val serialVersionUID: kotlin.Long = 1"
1279+
);
1280+
assertFileNotContains(path, ") : kotlin.collections.HashMap<String, kotlin.Any> {");
1281+
}
1282+
1283+
@Test
1284+
public void generateNonSerializableModelWithParentCtorCallNeeded() throws Exception {
1285+
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
1286+
output.deleteOnExit();
1287+
String outputPath = output.getAbsolutePath().replace('\\', '/');
1288+
1289+
KotlinSpringServerCodegen codegen = new KotlinSpringServerCodegen();
1290+
codegen.setOutputDir(output.getAbsolutePath());
1291+
1292+
ClientOptInput input = new ClientOptInput()
1293+
.openAPI(TestUtils.parseSpec("src/test/resources/3_0/kotlin/issue8366-inheritance-parent-ctor-call.yaml"))
1294+
.config(codegen);
1295+
DefaultGenerator generator = new DefaultGenerator();
1296+
1297+
generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
1298+
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_TESTS, "false");
1299+
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_DOCS, "false");
1300+
generator.setGeneratorPropertyDefault(CodegenConstants.APIS, "false");
1301+
generator.setGeneratorPropertyDefault(CodegenConstants.SUPPORTING_FILES, "false");
1302+
1303+
generator.opts(input).generate();
1304+
1305+
Path path = Paths.get(outputPath + "/src/main/kotlin/org/openapitools/model/JarFile.kt");
1306+
assertFileContains(
1307+
path,
1308+
") : kotlin.collections.HashMap<String, kotlin.Any>() {"
1309+
);
1310+
assertFileNotContains(
1311+
path,
1312+
"java.io.Serializable",
1313+
") : kotlin.collections.HashMap<String, kotlin.Any> {"
1314+
);
1315+
}
1316+
1317+
@Test
1318+
public void generateNonSerializableModelWithParentCtorCallNeededAndXimplements() throws Exception {
1319+
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
1320+
output.deleteOnExit();
1321+
String outputPath = output.getAbsolutePath().replace('\\', '/');
1322+
1323+
KotlinSpringServerCodegen codegen = new KotlinSpringServerCodegen();
1324+
codegen.setOutputDir(output.getAbsolutePath());
1325+
codegen.additionalProperties().put(KotlinSpringServerCodegen.SCHEMA_IMPLEMENTS, Map.of(
1326+
"JarFile", List.of("com.some.pack.ExtraInterface")
1327+
));
1328+
1329+
ClientOptInput input = new ClientOptInput()
1330+
.openAPI(TestUtils.parseSpec("src/test/resources/3_0/kotlin/issue8366-inheritance-parent-ctor-call.yaml"))
1331+
.config(codegen);
1332+
DefaultGenerator generator = new DefaultGenerator();
1333+
1334+
generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
1335+
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_TESTS, "false");
1336+
generator.setGeneratorPropertyDefault(CodegenConstants.MODEL_DOCS, "false");
1337+
generator.setGeneratorPropertyDefault(CodegenConstants.APIS, "false");
1338+
generator.setGeneratorPropertyDefault(CodegenConstants.SUPPORTING_FILES, "false");
1339+
1340+
generator.opts(input).generate();
1341+
1342+
Path path = Paths.get(outputPath + "/src/main/kotlin/org/openapitools/model/JarFile.kt");
1343+
assertFileContains(
1344+
path,
1345+
") : kotlin.collections.HashMap<String, kotlin.Any>(), com.some.pack.ExtraInterface {"
1346+
);
1347+
assertFileNotContains(
1348+
path,
1349+
"java.io.Serializable"
1350+
);
1351+
}
1352+
12511353
@Test
12521354
public void generateHttpInterfaceReactiveWithReactorResponseEntity() throws Exception {
12531355
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();

‎modules/openapi-generator/src/test/resources/3_0/kotlin/issue8366-inheritance-parent-ctor-call.yaml‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
openapi: 3.0.3
2+
info:
3+
version: "1.0.0"
4+
title: ""
5+
paths:
6+
/documents/v1:
7+
get:
8+
responses:
9+
200:
10+
description: lorem
11+
content:
12+
application/json:
13+
schema:
14+
type: array
15+
items:
16+
$ref: "#/components/schemas/ParentSchema"
17+
components:
18+
schemas:
19+
JarFiles:
20+
type: array
21+
items:
22+
$ref: "#/components/schemas/JarFile"
23+
JarFile:
24+
type: object
25+
required:
26+
- filename
27+
- contentBase64
28+
- hasDbMigrations
29+
additionalProperties: true
30+
properties:
31+
filename:
32+
type: string
33+
nullable: false
34+
minLength: 1
35+
maxLength: 255
36+
hasDbMigrations:
37+
description: Indicates whether the cordapp jar in question contains any
38+
embedded migrations that Cactus can/should execute between copying the
39+
jar into the cordapp directory and starting the node back up.
40+
type: boolean
41+
nullable: false
42+
contentBase64:
43+
type: string
44+
format: base64
45+
nullable: false
46+
minLength: 1
47+
maxLength: 1073741824
48+
49+
ParentSchema:
50+
type: object
51+
properties:
52+
id:
53+
type: string
54+
type:
55+
$ref: "#/components/schemas/DiscriminatingType"
56+
discriminator:
57+
propertyName: type
58+
mapping:
59+
subtypeA: "#/components/schemas/SubtypeA"
60+
subtypeB: "#/components/schemas/SubtypeB"
61+
subtypeC: "#/components/schemas/SubtypeC"
62+
SubtypeA:
63+
type: object
64+
allOf:
65+
- $ref: '#/components/schemas/ParentSchema'
66+
- type: object
67+
properties:
68+
subtypeAproperty:
69+
type: integer
70+
SubtypeB:
71+
type: object
72+
allOf:
73+
- $ref: '#/components/schemas/ParentSchema'
74+
- type: object
75+
properties:
76+
subtypeBproperty:
77+
type: string
78+
SubtypeC:
79+
type: object
80+
additionalProperties: true
81+
allOf:
82+
- $ref: '#/components/schemas/ParentSchema'
83+
- type: object
84+
properties:
85+
subtypeAproperty:
86+
type: integer
87+
88+
DiscriminatingType:
89+
type: string
90+
enum:
91+
- subtypeA
92+
- subtypeB
93+
- subtypeC

‎samples/server/petstore/kotlin-spring-issue8366-inheritance-parent-ctor-call/.openapi-generator-ignore‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# OpenAPI Generator Ignore
2+
# Generated by openapi-generator https://github.com/openapitools/openapi-generator
3+
4+
# Use this file to prevent files from being overwritten by the generator.
5+
# The patterns follow closely to .gitignore or .dockerignore.
6+
7+
# As an example, the C# client generator defines ApiClient.cs.
8+
# You can make changes and tell OpenAPI Generator to ignore just this file by uncommenting the following line:
9+
#ApiClient.cs
10+
11+
# You can match any string of characters against a directory, file or extension with a single asterisk (*):
12+
#foo/*/qux
13+
# The above matches foo/bar/qux and foo/baz/qux, but not foo/bar/baz/qux
14+
15+
# You can recursively match patterns against a directory, file or extension with a double asterisk (**):
16+
#foo/**/qux
17+
# This matches foo/bar/qux, foo/baz/qux, and foo/bar/baz/qux
18+
19+
# You can also negate patterns with an exclamation (!).
20+
# For example, you can ignore all files in a docs folder with the file extension .md:
21+
#docs/*.md
22+
# Then explicitly reverse the ignore rule for a single file:
23+
#!docs/README.md

‎samples/server/petstore/kotlin-spring-issue8366-inheritance-parent-ctor-call/.openapi-generator/FILES‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
README.md
2+
build.gradle.kts
3+
gradle/wrapper/gradle-wrapper.jar
4+
gradle/wrapper/gradle-wrapper.properties
5+
gradlew
6+
gradlew.bat
7+
pom.xml
8+
settings.gradle
9+
src/main/kotlin/org/openapitools/Application.kt
10+
src/main/kotlin/org/openapitools/HomeController.kt
11+
src/main/kotlin/org/openapitools/api/ApiUtil.kt
12+
src/main/kotlin/org/openapitools/api/DocumentsApiController.kt
13+
src/main/kotlin/org/openapitools/api/DocumentsApiService.kt
14+
src/main/kotlin/org/openapitools/api/DocumentsApiServiceImpl.kt
15+
src/main/kotlin/org/openapitools/configuration/EnumConverterConfiguration.kt
16+
src/main/kotlin/org/openapitools/model/DiscriminatingType.kt
17+
src/main/kotlin/org/openapitools/model/JarFile.kt
18+
src/main/kotlin/org/openapitools/model/ParentSchema.kt
19+
src/main/kotlin/org/openapitools/model/SubtypeA.kt
20+
src/main/kotlin/org/openapitools/model/SubtypeB.kt
21+
src/main/kotlin/org/openapitools/model/SubtypeC.kt
22+
src/main/resources/application.yaml
23+
src/main/resources/openapi.yaml

‎samples/server/petstore/kotlin-spring-issue8366-inheritance-parent-ctor-call/.openapi-generator/VERSION‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
7.24.0-SNAPSHOT

‎samples/server/petstore/kotlin-spring-issue8366-inheritance-parent-ctor-call/README.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#
2+
3+
This Kotlin based [Spring Boot](https://spring.io/projects/spring-boot) application has been generated using the [OpenAPI Generator](https://github.com/OpenAPITools/openapi-generator).
4+
5+
## Getting Started
6+
7+
This document assumes you have either maven or gradle available, either via the wrapper or otherwise. This does not come with a gradle / maven wrapper checked in.
8+
9+
By default a [`pom.xml`](pom.xml) file will be generated. If you specified `gradleBuildFile=true` when generating this project, a `build.gradle.kts` will also be generated. Note this uses [Gradle Kotlin DSL](https://github.com/gradle/kotlin-dsl).
10+
11+
To build the project using maven, run:
12+
```bash
13+
mvn package && java -jar target/openapi-spring-1.0.0.jar
14+
```
15+
16+
To build the project using gradle, run:
17+
```bash
18+
gradle build && java -jar build/libs/openapi-spring-1.0.0.jar
19+
```
20+
21+
If all builds successfully, the server should run on [http://localhost:8080/](http://localhost:8080/)

0 commit comments

Comments
 (0)