Better support for complex inheritance (#229)

* Better support for complex inheritance The current implementation had several issues: 1. If there are classes `A` -> `B` -> `C`, `C` will be `sealed`, `A` and `B` will be `data`. This is incorrect, `B` should be `open`. 2. All properties in `B` and `C` should be open: descendants can override them 3. If class has no own properties, there can be two cases: 3.1. It's a leaf (like `A`), then it should be `data object`, not just object 3.2. It's a node (like `B`), then it should be `open class` This commit addresses all these cases. There is still one important issue I don't immediately know how to fix: all descendants should have an `override` modifier on fields, overriding fields in the whole tree. * additional test * no need for non-nullability for child classes * add missing approval files --------- Co-authored-by: Ivan Ponomarev <[email protected]>
CourseOrchestra · Aug 28, 2023 · 101eb4a · 101eb4a
1 parent 7342a04
commit 101eb4a
Show file tree

Hide file tree

Showing 7 changed files with 159 additions and 5 deletions.
diff --git a/src/main/java/ru/curs/hurdygurdy/KotlinTypeDefiner.kt b/src/main/java/ru/curs/hurdygurdy/KotlinTypeDefiner.kt
@@ -34,7 +34,6 @@ import com.squareup.kotlinpoet.TypeName
 import com.squareup.kotlinpoet.TypeSpec
 import com.squareup.kotlinpoet.asClassName
 import com.squareup.kotlinpoet.asTypeName
-import com.squareup.kotlinpoet.joinToCode
 import io.swagger.v3.oas.models.OpenAPI
 import io.swagger.v3.oas.models.media.ArraySchema
 import io.swagger.v3.oas.models.media.ComposedSchema
@@ -162,12 +161,17 @@ class KotlinTypeDefiner internal constructor(
     }
 
     private fun getDTOClass(name: String, schema: Schema<*>, openAPI: OpenAPI, baseClass: TypeName): TypeSpec {
+        // Define if any schema references to us as "allOf"
+        val isParent = openAPI.components.schemas.any { (_, schema) ->
+            schema.allOf?.any { it.`$ref`?.endsWith(name) ?: false } ?: false
+        }
         val classBuilder =
             (if (schema.properties.isNullOrEmpty() &&
                 schema.additionalProperties == null &&
-                schema.oneOf.isNullOrEmpty()
+                schema.oneOf.isNullOrEmpty() &&
+                !isParent
             )
-                TypeSpec.objectBuilder(name).superclass(baseClass)
+                TypeSpec.objectBuilder(name).superclass(baseClass).addModifiers(KModifier.DATA)
             else if (!schema.oneOf.isNullOrEmpty())
                 TypeSpec.interfaceBuilder(name)
             else
@@ -199,14 +203,19 @@ class KotlinTypeDefiner internal constructor(
         } else if (!(schema.properties.isNullOrEmpty() && schema.additionalProperties == null)) {
             classBuilder.addModifiers(KModifier.DATA)
         }
+        //Intermediate class, can't be data, should be open
+        if (isParent && !classBuilder.modifiers.contains(KModifier.SEALED)) {
+            classBuilder.addModifiers(KModifier.OPEN)
+            classBuilder.modifiers.remove(KModifier.DATA)
+        }
 
         val subclassMapping = getSubclassMapping(schema)
         if (subclassMapping.isNotEmpty()) {
             val mappings =
                 subclassMapping
                     .map { (key, value) ->
                         AnnotationSpec.builder(JsonSubTypes.Type::class)
-                            .addMember("value = %T::class", referencedTypeName(value, openAPI))
+                            .addMember("value = %T::class", referencedTypeName(value, openAPI).copy(nullable = false))
                             .addMember("name = %S", key).build()
                     }
                     .map { CodeBlock.of("%L", it) }
@@ -284,6 +293,8 @@ class KotlinTypeDefiner internal constructor(
 
                 val propertySpec = PropertySpec
                     .builder(propertyName, typeName)
+                    // If we're parent children can override even types!
+                    .addModifiers(listOfNotNull(KModifier.OPEN.takeIf { isParent }))
                     .initializer(propertyName).build()
                 classBuilder.addProperty(propertySpec)
             }

diff --git a/src/test/java/ru/curs/hurdygurdy/CodegenTest.deepInheritance.approved.txt b/src/test/java/ru/curs/hurdygurdy/CodegenTest.deepInheritance.approved.txt
@@ -0,0 +1,54 @@
+---
+
+---
+/com
+---
+/com/example
+---
+/com/example/dto
+---
+/com/example/dto/A.java
+package com.example.dto;
+
+import com.fasterxml.jackson.annotation.JsonSubTypes;
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
+import com.fasterxml.jackson.databind.PropertyNamingStrategies;
+import com.fasterxml.jackson.databind.annotation.JsonNaming;
+import lombok.Data;
+
+@Data
+@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
+@JsonTypeInfo(
+    use = JsonTypeInfo.Id.NAME,
+    include = JsonTypeInfo.As.PROPERTY,
+    property = "type"
+)
+@JsonSubTypes({
+    @JsonSubTypes.Type(value = B.class, name = "b"),
+    @JsonSubTypes.Type(value = C.class, name = "c")})
+public class A {
+}
+---
+/com/example/dto/B.java
+package com.example.dto;
+
+import com.fasterxml.jackson.databind.PropertyNamingStrategies;
+import com.fasterxml.jackson.databind.annotation.JsonNaming;
+import lombok.Data;
+
+@Data
+@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
+public class B extends A {
+}
+---
+/com/example/dto/C.java
+package com.example.dto;
+
+import com.fasterxml.jackson.databind.PropertyNamingStrategies;
+import com.fasterxml.jackson.databind.annotation.JsonNaming;
+import lombok.Data;
+
+@Data
+@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
+public class C extends B {
+}
diff --git a/src/test/java/ru/curs/hurdygurdy/CodegenTest.java b/src/test/java/ru/curs/hurdygurdy/CodegenTest.java
@@ -106,6 +106,12 @@ void noOwnTypes() throws IOException {
         Approvals.verify(getContent(result));
     }
 
+    @Test
+    void deepInheritance() throws IOException {
+        codegen.generate(Path.of("src/test/resources/deep_inheritance.yaml"), result);
+        Approvals.verify(getContent(result));
+    }
+
     String getContent(Path path) throws IOException {
         return Files.walk(path)
                 .sorted()

diff --git a/src/test/java/ru/curs/hurdygurdy/KCodegenTest.deepInheritance.approved.txt b/src/test/java/ru/curs/hurdygurdy/KCodegenTest.deepInheritance.approved.txt
@@ -0,0 +1,46 @@
+---
+
+---
+/com
+---
+/com/example
+---
+/com/example/dto
+---
+/com/example/dto/A.kt
+package com.example.dto
+
+import com.fasterxml.jackson.`annotation`.JsonSubTypes
+import com.fasterxml.jackson.`annotation`.JsonTypeInfo
+import com.fasterxml.jackson.databind.PropertyNamingStrategies
+import com.fasterxml.jackson.databind.`annotation`.JsonNaming
+
+@JsonNaming(value = PropertyNamingStrategies.SnakeCaseStrategy::class)
+@JsonTypeInfo(
+  use = JsonTypeInfo.Id.NAME,
+  include = JsonTypeInfo.As.PROPERTY,
+  property = "type",
+)
+@JsonSubTypes(
+  JsonSubTypes.Type(value = B::class, name = "b"),
+  JsonSubTypes.Type(value = C::class, name = "c"),
+)
+public sealed class A()
+---
+/com/example/dto/B.kt
+package com.example.dto
+
+import com.fasterxml.jackson.databind.PropertyNamingStrategies
+import com.fasterxml.jackson.databind.`annotation`.JsonNaming
+
+@JsonNaming(value = PropertyNamingStrategies.SnakeCaseStrategy::class)
+public open class B : A()
+---
+/com/example/dto/C.kt
+package com.example.dto
+
+import com.fasterxml.jackson.databind.PropertyNamingStrategies
+import com.fasterxml.jackson.databind.`annotation`.JsonNaming
+
+@JsonNaming(value = PropertyNamingStrategies.SnakeCaseStrategy::class)
+public data object C : B()
diff --git a/src/test/java/ru/curs/hurdygurdy/KCodegenTest.generateSample3.approved.txt b/src/test/java/ru/curs/hurdygurdy/KCodegenTest.generateSample3.approved.txt
@@ -69,7 +69,7 @@ import com.fasterxml.jackson.databind.PropertyNamingStrategies
 import com.fasterxml.jackson.databind.`annotation`.JsonNaming
 
 @JsonNaming(value = PropertyNamingStrategies.SnakeCaseStrategy::class)
-public object ClassWithNoFields
+public data object ClassWithNoFields
 ---
 /com/example/dto/CreateUpdateUserRequest.kt
 package com.example.dto

diff --git a/src/test/java/ru/curs/hurdygurdy/KCodegenTest.java b/src/test/java/ru/curs/hurdygurdy/KCodegenTest.java
@@ -119,6 +119,12 @@ void noOwnTypes() throws IOException {
         Approvals.verify(getContent(result));
     }
 
+    @Test
+    void deepInheritance() throws IOException {
+        codegen.generate(Path.of("src/test/resources/deep_inheritance.yaml"), result);
+        Approvals.verify(getContent(result));
+    }
+
     String getContent(Path path) throws IOException {
         return Files.walk(path)
                 .sorted()

diff --git a/src/test/resources/deep_inheritance.yaml b/src/test/resources/deep_inheritance.yaml
@@ -0,0 +1,31 @@
+openapi: 3.0.1
+info:
+  title: "Deep inheritance API"
+  description: |
+    Test
+  version: "0.1"
+
+paths:
+
+
+components:
+  schemas:
+    A:
+      description: A
+      properties:
+        type:
+          type: string
+      discriminator:
+        propertyName: type
+        mapping:
+          'b': '#/components/schemas/B'
+          'c': '#/components/schemas/C'
+    B:
+      description: B
+      allOf:
+        - $ref: "#/components/schemas/A"
+
+    C:
+      description: C
+      allOf:
+        - $ref: "#/components/schemas/B"