Skip to content

Commit 7fc39bb

Browse files
authored
Fix issue with generation of inlined items schema in arrays. (#353)
* Fix issue with generation of inlined items schema in arrays. Add failing test * Fix issue by only prefixing enclosing schema name when it is not an array type. Also remove all references to the wrapper class EnclosedSchemaInfo. It is unclear what value it adds, and all tests pass without it
1 parent da04fb6 commit 7fc39bb

File tree

14 files changed

+97
-68
lines changed

14 files changed

+97
-68
lines changed

src/main/kotlin/com/cjbooms/fabrikt/generators/model/ModelGenerator.kt

+3-8
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import com.cjbooms.fabrikt.model.PropertyInfo.Companion.HTTP_SETTINGS
3030
import com.cjbooms.fabrikt.model.PropertyInfo.Companion.topLevelProperties
3131
import com.cjbooms.fabrikt.model.SchemaInfo
3232
import com.cjbooms.fabrikt.model.SourceApi
33-
import com.cjbooms.fabrikt.model.toEnclosingSchemaInfo
3433
import com.cjbooms.fabrikt.util.KaizenParserExtensions.findOneOfSuperInterface
3534
import com.cjbooms.fabrikt.util.KaizenParserExtensions.getDiscriminatorForInLinedObjectUnderAllOf
3635
import com.cjbooms.fabrikt.util.KaizenParserExtensions.getSchemaRefName
@@ -192,7 +191,6 @@ class ModelGenerator(
192191
schemaName = it.schema.safeName(),
193192
enclosingSchema = it.schema,
194193
apiDocUrl = it.schema.getDocumentUrl(),
195-
enclosingSchemaInfoName = it.name,
196194
)
197195
}
198196
else -> {
@@ -278,7 +276,7 @@ class ModelGenerator(
278276
} else {
279277
val props = it.schema.topLevelProperties(HTTP_SETTINGS, sourceApi.openApi3, enclosingSchema)
280278
val currentModel = standardDataClass(
281-
ModelNameRegistry.getOrRegister(it.schema, enclosingSchema.toEnclosingSchemaInfo()),
279+
ModelNameRegistry.getOrRegister(it.schema, enclosingSchema),
282280
it.name,
283281
props,
284282
it.schema.extensions,
@@ -331,11 +329,8 @@ class ModelGenerator(
331329
schemaName: String,
332330
enclosingSchema: Schema,
333331
apiDocUrl: String,
334-
enclosingSchemaInfoName: String? = null,
335332
): Collection<TypeSpec> =
336333
schema.itemsSchema.let { items ->
337-
val enclosingSchemaInfo = enclosingSchemaInfoName?.toEnclosingSchemaInfo()
338-
?: enclosingSchema.toEnclosingSchemaInfo()
339334
when {
340335
items.isInlinedObjectDefinition() ->
341336
items.topLevelProperties(HTTP_SETTINGS, sourceApi.openApi3, enclosingSchema).let { props ->
@@ -344,7 +339,7 @@ class ModelGenerator(
344339
enclosingSchema = enclosingSchema,
345340
apiDocUrl = apiDocUrl,
346341
) + standardDataClass(
347-
modelName = ModelNameRegistry.getOrRegister(schema, enclosingSchemaInfo),
342+
modelName = ModelNameRegistry.getOrRegister(schema, enclosingSchema),
348343
schemaName = schemaName,
349344
properties = props,
350345
extensions = schema.extensions,
@@ -355,7 +350,7 @@ class ModelGenerator(
355350
items.isInlinedEnumDefinition() ->
356351
setOf(
357352
buildEnumClass(
358-
KotlinTypeInfo.from(items, "items", enclosingSchemaInfo) as KotlinTypeInfo.Enum,
353+
KotlinTypeInfo.from(items, "items", enclosingSchema) as KotlinTypeInfo.Enum,
359354
),
360355
)
361356

src/main/kotlin/com/cjbooms/fabrikt/model/KotlinTypeInfo.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ sealed class KotlinTypeInfo(val modelKClass: KClass<*>, val generatedModelClassN
7070
companion object {
7171
private val logger = Logger.getGlobal()
7272

73-
fun from(schema: Schema, oasKey: String = "", enclosingSchema: EnclosingSchemaInfo? = null): KotlinTypeInfo =
73+
fun from(schema: Schema, oasKey: String = "", enclosingSchema: Schema? = null): KotlinTypeInfo =
7474
when (schema.toOasType(oasKey)) {
7575
OasType.Date -> {
7676
if (MutableSettings.serializationLibrary() == KOTLINX_SERIALIZATION) KotlinxLocalDate

src/main/kotlin/com/cjbooms/fabrikt/model/PropertyInfo.kt

+5-5
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ sealed class PropertyInfo {
167167
val enclosingSchema: Schema? = null
168168
) : PropertyInfo() {
169169
override val typeInfo: KotlinTypeInfo =
170-
KotlinTypeInfo.from(schema, oasKey, enclosingSchema?.toEnclosingSchemaInfo())
170+
KotlinTypeInfo.from(schema, oasKey, enclosingSchema)
171171
val pattern: String? = schema.safeField(Schema::getPattern)
172172
val maxLength: Int? = schema.safeField(Schema::getMaxLength)
173173
val minLength: Int? = schema.safeField(Schema::getMinLength)
@@ -194,9 +194,9 @@ sealed class PropertyInfo {
194194
) : PropertyInfo(), CollectionValidation {
195195
override val typeInfo: KotlinTypeInfo =
196196
if (isInherited) {
197-
KotlinTypeInfo.from(schema, oasKey, parentSchema.toEnclosingSchemaInfo())
197+
KotlinTypeInfo.from(schema, oasKey, parentSchema)
198198
} else {
199-
KotlinTypeInfo.from(schema, oasKey, enclosingSchema?.toEnclosingSchemaInfo())
199+
KotlinTypeInfo.from(schema, oasKey, enclosingSchema)
200200
}
201201
override val minItems: Int? = schema.minItems
202202
override val maxItems: Int? = schema.maxItems
@@ -232,9 +232,9 @@ sealed class PropertyInfo {
232232
) : PropertyInfo() {
233233
override val typeInfo: KotlinTypeInfo =
234234
if (isInherited) {
235-
KotlinTypeInfo.from(schema, oasKey, parentSchema.toEnclosingSchemaInfo())
235+
KotlinTypeInfo.from(schema, oasKey, parentSchema)
236236
} else {
237-
KotlinTypeInfo.from(schema, oasKey, enclosingSchema?.toEnclosingSchemaInfo())
237+
KotlinTypeInfo.from(schema, oasKey, enclosingSchema)
238238
}
239239
}
240240

src/main/kotlin/com/cjbooms/fabrikt/model/SourceApi.kt

-26
Original file line numberDiff line numberDiff line change
@@ -83,29 +83,3 @@ data class SourceApi(
8383
}
8484
}
8585
}
86-
87-
/**
88-
* Enclosing schema can either be provided as:
89-
* - a schema from Kaizen as OAS3 model.
90-
* - provided as schema name based on names provided in spec file from SchemaInfo
91-
*/
92-
enum class EnclosingSchemaInfoType {
93-
NAME,
94-
OAS_MODEL,
95-
}
96-
97-
interface EnclosingSchemaInfo {
98-
val type: EnclosingSchemaInfoType
99-
}
100-
101-
data class EnclosingSchemaInfoName(val name: String) : EnclosingSchemaInfo {
102-
override val type = EnclosingSchemaInfoType.NAME
103-
}
104-
105-
data class EnclosingSchemaInfoOasModel(val schema: Schema) : EnclosingSchemaInfo {
106-
override val type = EnclosingSchemaInfoType.OAS_MODEL
107-
}
108-
109-
fun Schema.toEnclosingSchemaInfo() = EnclosingSchemaInfoOasModel(this)
110-
111-
fun String.toEnclosingSchemaInfo() = EnclosingSchemaInfoName(this)

src/main/kotlin/com/cjbooms/fabrikt/util/ModelNameRegistry.kt

+8-17
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
package com.cjbooms.fabrikt.util
22

33
import com.cjbooms.fabrikt.generators.MutableSettings
4-
import com.cjbooms.fabrikt.model.EnclosingSchemaInfo
5-
import com.cjbooms.fabrikt.model.EnclosingSchemaInfoName
6-
import com.cjbooms.fabrikt.model.EnclosingSchemaInfoOasModel
74
import com.cjbooms.fabrikt.model.SchemaInfo
85
import com.cjbooms.fabrikt.util.KaizenParserExtensions.safeName
96
import com.cjbooms.fabrikt.util.NormalisedString.toModelClassName
@@ -26,11 +23,11 @@ object ModelNameRegistry {
2623
*/
2724
private fun register(
2825
schema: Schema,
29-
enclosingSchema: EnclosingSchemaInfo? = null,
26+
enclosingSchema: Schema? = null,
3027
valueSuffix: Boolean = false,
3128
schemaInfoName: String? = null,
3229
): String {
33-
val modelClassName = schema.toModelClassName(schemaInfoName, enclosingSchema?.toModelClassName(), valueSuffix)
30+
val modelClassName = schema.toModelClassName(schemaInfoName, enclosingSchema, valueSuffix)
3431
var suggestion = modelClassName
3532
while (allocatedNames.contains(suggestion)) {
3633
suggestion += SUFFIX
@@ -49,10 +46,11 @@ object ModelNameRegistry {
4946

5047
private fun Schema.toModelClassName(
5148
schemaInfoName: String? = null,
52-
enclosingClassName: String? = null,
49+
enclosingSchema: Schema? = null,
5350
valueSuffix: Boolean = false,
5451
): String = buildString {
55-
if (enclosingClassName != null) {
52+
val enclosingClassName = enclosingSchema?.toModelClassName()
53+
if (enclosingClassName != null && enclosingSchema.type != "array") {
5654
append(enclosingClassName)
5755
}
5856
val modelClassName = schemaInfoName?.toModelClassName() ?: safeName().toModelClassName()
@@ -64,20 +62,13 @@ object ModelNameRegistry {
6462
append(modelClassNameSuffix)
6563
}
6664

67-
private fun EnclosingSchemaInfo.toModelClassName() =
68-
when (this) {
69-
is EnclosingSchemaInfoName -> this.name
70-
is EnclosingSchemaInfoOasModel -> this.schema.toModelClassName()
71-
else -> ""
72-
}
73-
7465
private fun resolveTag(
7566
schema: Schema,
76-
enclosingSchema: EnclosingSchemaInfo? = null,
67+
enclosingSchema: Schema? = null,
7768
valueSuffix: Boolean = false,
7869
schemaInfoName: String? = null,
7970
): String =
80-
resolveTag(schema, schema.toModelClassName(schemaInfoName, enclosingSchema?.toModelClassName(), valueSuffix))
71+
resolveTag(schema, schema.toModelClassName(schemaInfoName, enclosingSchema, valueSuffix))
8172

8273
private fun resolveTag(schema: Schema, modelClassName: String): String {
8374
val overlay = Overlay.of(schema)
@@ -92,7 +83,7 @@ object ModelNameRegistry {
9283

9384
fun getOrRegister(
9485
schema: Schema,
95-
enclosingSchema: EnclosingSchemaInfo? = null,
86+
enclosingSchema: Schema? = null,
9687
valueSuffix: Boolean = false,
9788
) = this[resolveTag(schema, enclosingSchema, valueSuffix)]
9889
.getOrElse { register(schema, enclosingSchema, valueSuffix) }

src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ class ModelGeneratorTest {
7777
ModelNameRegistry.clear()
7878
}
7979

80-
// @Test
81-
// fun `debug single test`() = `correct models are generated for different OpenApi Specifications`("insert test case")
80+
@Test
81+
fun `debug single test`() = `correct models are generated for different OpenApi Specifications`("enumExamples")
8282

8383
@ParameterizedTest
8484
@MethodSource("testCases")

src/test/kotlin/com/cjbooms/fabrikt/generators/SpringControllerGeneratorTest.kt

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class SpringControllerGeneratorTest {
3333

3434
@Suppress("unused")
3535
private fun testCases(): Stream<String> = Stream.of(
36+
"arrays",
3637
"githubApi",
3738
"singleAllOf",
3839
"pathLevelParameters",

src/test/resources/examples/arrays/api.yaml

+27-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,24 @@
11
openapi: 3.0.0
22
info:
33
paths:
4+
/items:
5+
get:
6+
summary: Get items with a status filter
7+
description: Retrieve a list of items filtered by their status.
8+
parameters:
9+
- name: status
10+
in: query
11+
required: false
12+
description: Filter items by status.
13+
schema:
14+
$ref: '#/components/schemas/ArrayOfSomething'
15+
responses:
16+
"200":
17+
description: A list of items.
18+
content:
19+
application/json:
20+
schema:
21+
$ref: '#/components/schemas/ArrayContainingComplexInlined'
422
components:
523
schemas:
624
ContainsArrayOfArrays:
@@ -76,4 +94,12 @@ components:
7694
properties:
7795
grams:
7896
type: integer
79-
example: 100
97+
example: 100
98+
99+
ArrayContainingComplexInlined:
100+
type: array
101+
items:
102+
type: object
103+
properties:
104+
prop_one:
105+
type: string
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package examples.arrays.controllers
2+
3+
import examples.arrays.models.ArrayContainingComplexInlined
4+
import examples.arrays.models.Something
5+
import org.springframework.http.ResponseEntity
6+
import org.springframework.stereotype.Controller
7+
import org.springframework.validation.`annotation`.Validated
8+
import org.springframework.web.bind.`annotation`.RequestMapping
9+
import org.springframework.web.bind.`annotation`.RequestMethod
10+
import org.springframework.web.bind.`annotation`.RequestParam
11+
import javax.validation.Valid
12+
import kotlin.collections.List
13+
14+
@Controller
15+
@Validated
16+
@RequestMapping("")
17+
public interface ItemsController {
18+
/**
19+
* Get items with a status filter
20+
* Retrieve a list of items filtered by their status.
21+
* @param status Filter items by status.
22+
*/
23+
@RequestMapping(
24+
value = ["/items"],
25+
produces = ["application/json"],
26+
method = [RequestMethod.GET],
27+
)
28+
public fun `get`(
29+
@Valid @RequestParam(value = "status", required = false)
30+
status: List<Something>?,
31+
): ResponseEntity<List<ArrayContainingComplexInlined>>
32+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package examples.arrays.models
2+
3+
import com.fasterxml.jackson.`annotation`.JsonProperty
4+
import kotlin.String
5+
6+
public data class ArrayContainingComplexInlined(
7+
@param:JsonProperty("prop_one")
8+
@get:JsonProperty("prop_one")
9+
public val propOne: String? = null,
10+
)

src/test/resources/examples/enumExamples/models/BarBar.kt src/test/resources/examples/enumExamples/models/Bar.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package examples.enumExamples.models
33
import com.fasterxml.jackson.`annotation`.JsonProperty
44
import kotlin.String
55

6-
public data class BarBar(
6+
public data class Bar(
77
@param:JsonProperty("bar_prop")
88
@get:JsonProperty("bar_prop")
99
public val barProp: String? = null,

src/test/resources/examples/enumExamples/models/FooFoo.kt src/test/resources/examples/enumExamples/models/Foo.kt

+3-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import com.fasterxml.jackson.`annotation`.JsonValue
44
import kotlin.String
55
import kotlin.collections.Map
66

7-
public enum class FooFoo(
7+
public enum class Foo(
88
@JsonValue
99
public val `value`: String,
1010
) {
@@ -13,8 +13,8 @@ public enum class FooFoo(
1313
;
1414

1515
public companion object {
16-
private val mapping: Map<String, FooFoo> = entries.associateBy(FooFoo::value)
16+
private val mapping: Map<String, Foo> = entries.associateBy(Foo::value)
1717

18-
public fun fromValue(`value`: String): FooFoo? = mapping[value]
18+
public fun fromValue(`value`: String): Foo? = mapping[value]
1919
}
2020
}

src/test/resources/examples/modelSuffix/models/BarBarDto.kt src/test/resources/examples/modelSuffix/models/BarDto.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package examples.modelSuffix.models
33
import com.fasterxml.jackson.`annotation`.JsonProperty
44
import kotlin.String
55

6-
public data class BarBarDto(
6+
public data class BarDto(
77
@param:JsonProperty("bar_prop")
88
@get:JsonProperty("bar_prop")
99
public val barProp: String? = null,

src/test/resources/examples/modelSuffix/models/FooFooDto.kt src/test/resources/examples/modelSuffix/models/FooDto.kt

+3-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import com.fasterxml.jackson.`annotation`.JsonValue
44
import kotlin.String
55
import kotlin.collections.Map
66

7-
public enum class FooFooDto(
7+
public enum class FooDto(
88
@JsonValue
99
public val `value`: String,
1010
) {
@@ -13,8 +13,8 @@ public enum class FooFooDto(
1313
;
1414

1515
public companion object {
16-
private val mapping: Map<String, FooFooDto> = entries.associateBy(FooFooDto::value)
16+
private val mapping: Map<String, FooDto> = entries.associateBy(FooDto::value)
1717

18-
public fun fromValue(`value`: String): FooFooDto? = mapping[value]
18+
public fun fromValue(`value`: String): FooDto? = mapping[value]
1919
}
2020
}

0 commit comments

Comments
 (0)