Skip to content

Commit 617a992

Browse files
committed
Update SharedCodegen to properly deal with inheritance
The base codegen that we're using does not really keep all the properties in sync, which is not great, but other templates are already applying this type of fixes. Trying to create a PR would eventually not be beneficial since we're still based on swagger-codegen version 2 (while version 3 has been released)
1 parent bf0e585 commit 617a992

File tree

2 files changed

+84
-1
lines changed

2 files changed

+84
-1
lines changed

gradle-plugin/plugin/src/main/java/com/yelp/codegen/KotlinGenerator.kt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ open class KotlinGenerator : SharedCodegen() {
210210
codegenModel.imports.add("com.squareup.moshi.Json")
211211
}
212212

213-
if (!codegenModel.isAlias) {
213+
if (!codegenModel.isAlias || codegenModel.parent != null) {
214214
// If we are rendering a model (or enum) we are annotating it with @JsonClass,
215215
// so we need to make sure that we're importing it
216216
codegenModel.imports.add("com.squareup.moshi.JsonClass")
@@ -223,6 +223,12 @@ open class KotlinGenerator : SharedCodegen() {
223223
break
224224
}
225225
}
226+
227+
if (supportsInheritance && codegenModel.discriminator != null) {
228+
// Import JsonClass annotation to enable Adapters generations via Kotlin Annotation Processor
229+
codegenModel.imports.add("$toolsPackage.optimisticHashCode")
230+
codegenModel.imports.add("$toolsPackage.Polymorphic")
231+
}
226232
}
227233

228234
/**

gradle-plugin/plugin/src/main/java/com/yelp/codegen/SharedCodegen.kt

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,20 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {
269269
.onEach { it.datatypeWithEnum = postProcessDataTypeWithEnum(codegenModel, it) }
270270
}
271271

272+
// Fix presence of duplicated variables into CodegenModel instances.
273+
// Apparently swagger-codegen does not really play well with such properties if
274+
// inheritance is enable and models have `allOf` attribute.
275+
CodegenModelVar.forEachVarAttribute(codegenModel) { _, properties ->
276+
val seenProperties = mutableSetOf<CodegenProperty>()
277+
val propertiesIterator = properties.iterator()
278+
while (propertiesIterator.hasNext()) {
279+
val property = propertiesIterator.next()
280+
if (!seenProperties.add(property)) {
281+
propertiesIterator.remove()
282+
}
283+
}
284+
}
285+
272286
return codegenModel
273287
}
274288

@@ -293,6 +307,31 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {
293307
}
294308
}
295309

310+
/**
311+
* Codegen does use the same [CodegenProperty] in the child models, is the property is inherited
312+
*
313+
* This method allows the creation of clones of the [CodegenProperty] instances in case they are
314+
* present due to inheritance.
315+
*
316+
* This additional computation is needed to allow further customisation of the child model
317+
* property definitions without parent model property modifications.
318+
* One example of use-case is the update of isInherited attribute of [CodegenProperty]
319+
*/
320+
private fun decoupleParentProperties(codegenModel: CodegenModel) {
321+
val nameToParentClone = codegenModel.parentVars.map {
322+
it.name to it.clone()
323+
}.toMap()
324+
CodegenModelVar.forEachVarAttribute(codegenModel) { type, properties ->
325+
if (type != CodegenModelVar.PARENT_VARS) {
326+
properties.forEachIndexed { index, property ->
327+
nameToParentClone[property.name]?.let {
328+
properties[index] = it
329+
}
330+
}
331+
}
332+
}
333+
}
334+
296335
override fun postProcessAllModels(objs: Map<String, Any>): Map<String, Any> {
297336
val postProcessedModels = super.postProcessAllModels(objs)
298337

@@ -319,6 +358,44 @@ abstract class SharedCodegen : DefaultCodegen(), CodegenConfig {
319358
.forEach { codegenModel ->
320359
// Ensure that after all the processing done on the CodegenModel.*Vars, hasMore does still make sense
321360
CodegenModelVar.forEachVarAttribute(codegenModel) { _, properties -> properties.fixHasMoreProperty() }
361+
362+
if (supportsInheritance && true == codegenModel.children?.isNotEmpty()) {
363+
// Codegen does update the children attribute but does not keep hasChildren consistent
364+
codegenModel.hasChildren = true
365+
}
366+
367+
if (supportsInheritance && codegenModel.parentModel != null) {
368+
val parentPropertyNames = codegenModel.parentModel.allVars.map { it.name }.toSet()
369+
370+
// Update parentVars (as codegen does not do it while updating the parents)
371+
codegenModel.parentVars?.addAll(codegenModel.parentModel.allVars)
372+
373+
decoupleParentProperties(codegenModel)
374+
375+
// Update all the *Vars attributes to keep track of parent-inherited parameters
376+
CodegenModelVar.forEachVarAttribute(codegenModel) { type, properties ->
377+
if (type != CodegenModelVar.PARENT_VARS) {
378+
properties.forEach {
379+
it.isInherited = it.name in parentPropertyNames
380+
}
381+
}
382+
}
383+
// Objects with a single item in allOf are recognized as aliases, as this is usually done
384+
// to override the content of metadata (ie. description).
385+
// The tool does already help avoid a new type creation by creating a type alias
386+
// In the case of Polymorphism this is actually a problem as the user would expect an
387+
// instance of a different class, even if the content is the same of the parent class
388+
if (codegenModel.isAlias) {
389+
codegenModel.isAlias = false
390+
codegenModel.hasVars = codegenModel.allVars.isNotEmpty()
391+
addRequiredImports(codegenModel)
392+
}
393+
394+
// codegenModel.hasEnums will be set to true even if the current model does not specify
395+
// an enum value but the parent does.
396+
// In order to avoid to consider this model as a model with enums we're re-evaluating it.
397+
codegenModel.hasEnums = codegenModel.vars.any { it.isEnum }
398+
}
322399
}
323400

324401
return postProcessedModels

0 commit comments

Comments
 (0)