Repository: bkbnio/kompendium
Pull request #202 (2022-02-19 22:54:21)

fix: nullability breaks object comparison
# Description

This is not a real fix, more of a hotpatch for a bug that is going to be a real pain.  The switch to reference schemas completely broken Kompendiums ability to handle null fields.  Since each type gets stored only once, but nullability is stored on the object schema level, this leads to a situation where defining one field of Type A that _is_ nullable and another field of type A that _is not_ nullable cannot be handled.  

This PR doesn't fix that, but it _will_ at least stop kompendium from crashing when a null field exists.

## Type of change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Documentation 
- [ ] Chore

# How Has This Been Tested?

Added a test

# Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] I have updated the CHANGELOG in the `Unreleased` section
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9ae2f43f8..bfabc395a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,7 +5,6 @@
 ### Added
 
 ### Changed
-- Fixed sealed typed collections schema generation
 
 ### Remove
 
@@ -13,6 +12,11 @@
 
 ## Released
 
+## [2.1.1] - February 19th, 2022
+### Changed
+- Fixed sealed typed collections schema generation
+- Nullability no longer breaks object schema comparison
+
 ## [2.1.0] - February 18th, 2022
 ### Added
 - Ability to override serializer via custom route
diff --git a/gradle.properties b/gradle.properties
index 10e9a49e5..917b0789e 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -1,5 +1,5 @@
 # Kompendium
-project.version=2.1.0
+project.version=2.1.1
 # Kotlin
 kotlin.code.style=official
 # Gradle
diff --git a/kompendium-core/src/main/kotlin/io/bkbn/kompendium/core/handler/ObjectHandler.kt b/kompendium-core/src/main/kotlin/io/bkbn/kompendium/core/handler/ObjectHandler.kt
index 7db7912ae..9a5e85817 100644
--- a/kompendium-core/src/main/kotlin/io/bkbn/kompendium/core/handler/ObjectHandler.kt
+++ b/kompendium-core/src/main/kotlin/io/bkbn/kompendium/core/handler/ObjectHandler.kt
@@ -51,7 +51,7 @@ object ObjectHandler : SchemaHandler {
         .plus(clazz.generateUndeclaredFieldMap(cache))
         .mapValues { (_, fieldSchema) ->
           val fieldSlug = cache.filter { (_, vv) -> vv == fieldSchema }.keys.firstOrNull()
-          postProcessSchema(fieldSchema, fieldSlug ?: "Fine if blank, will be ignored")
+          postProcessSchema(fieldSchema, fieldSlug)
         }
       logger.debug("$slug contains $fieldMap")
       val schema = ObjectSchema(fieldMap).adjustForRequiredParams(clazz)
diff --git a/kompendium-core/src/main/kotlin/io/bkbn/kompendium/core/handler/SchemaHandler.kt b/kompendium-core/src/main/kotlin/io/bkbn/kompendium/core/handler/SchemaHandler.kt
index 5ef27abe0..d29e24c91 100644
--- a/kompendium-core/src/main/kotlin/io/bkbn/kompendium/core/handler/SchemaHandler.kt
+++ b/kompendium-core/src/main/kotlin/io/bkbn/kompendium/core/handler/SchemaHandler.kt
@@ -24,9 +24,15 @@ interface SchemaHandler {
     }
   }
 
-  fun postProcessSchema(schema: ComponentSchema, slug: String): ComponentSchema = when (schema) {
-    is ObjectSchema -> ReferencedSchema(COMPONENT_SLUG.plus("/").plus(slug))
-    is EnumSchema -> ReferencedSchema(COMPONENT_SLUG.plus("/").plus(slug))
+  fun postProcessSchema(schema: ComponentSchema, slug: String?): ComponentSchema = when (schema) {
+    is ObjectSchema -> {
+      require(slug != null) { "Slug cannot be null for an object schema! $schema" }
+      ReferencedSchema(COMPONENT_SLUG.plus("/").plus(slug))
+    }
+    is EnumSchema -> {
+      require(slug != null) { "Slug cannot be null for an enum schema! $schema" }
+      ReferencedSchema(COMPONENT_SLUG.plus("/").plus(slug))
+    }
     else -> schema
   }
 }
diff --git a/kompendium-oas/src/main/kotlin/io/bkbn/kompendium/oas/schema/ObjectSchema.kt b/kompendium-oas/src/main/kotlin/io/bkbn/kompendium/oas/schema/ObjectSchema.kt
index 9771b41ca..0470a4975 100644
--- a/kompendium-oas/src/main/kotlin/io/bkbn/kompendium/oas/schema/ObjectSchema.kt
+++ b/kompendium-oas/src/main/kotlin/io/bkbn/kompendium/oas/schema/ObjectSchema.kt
@@ -13,4 +13,24 @@ data class ObjectSchema(
   val required: List<String>? = null
 ) : TypedSchema {
   override val type = "object"
+
+  override fun equals(other: Any?): Boolean {
+    if (other !is ObjectSchema) return false
+    if (properties != other.properties) return false
+    if (description != other.description) return false
+    // TODO Going to need some way to differentiate nullable vs non-nullable reference schemas 😬
+//    if (nullable != other.nullable) return false
+    if (required != other.required) return false
+    return true
+  }
+
+  override fun hashCode(): Int {
+    var result = properties.hashCode()
+    result = 31 * result + (default?.hashCode() ?: 0)
+    result = 31 * result + (description?.hashCode() ?: 0)
+    result = 31 * result + (nullable?.hashCode() ?: 0)
+    result = 31 * result + (required?.hashCode() ?: 0)
+    result = 31 * result + type.hashCode()
+    return result
+  }
 }
diff --git a/kompendium-playground/src/main/kotlin/io/bkbn/kompendium/playground/BasicPlayground.kt b/kompendium-playground/src/main/kotlin/io/bkbn/kompendium/playground/BasicPlayground.kt
index 0584572d2..ccd092b4a 100644
--- a/kompendium-playground/src/main/kotlin/io/bkbn/kompendium/playground/BasicPlayground.kt
+++ b/kompendium-playground/src/main/kotlin/io/bkbn/kompendium/playground/BasicPlayground.kt
@@ -189,3 +189,4 @@ object BasicModels {
     val d: Boolean
   )
 }
+


Test patch:
diff --git a/kompendium-core/src/test/kotlin/io/bkbn/kompendium/core/KompendiumTest.kt b/kompendium-core/src/test/kotlin/io/bkbn/kompendium/core/KompendiumTest.kt
index 94d51c1ac..096a34bb8 100644
--- a/kompendium-core/src/test/kotlin/io/bkbn/kompendium/core/KompendiumTest.kt
+++ b/kompendium-core/src/test/kotlin/io/bkbn/kompendium/core/KompendiumTest.kt
@@ -39,6 +39,7 @@ import io.bkbn.kompendium.core.util.notarizedPatchModule
 import io.bkbn.kompendium.core.util.notarizedPostModule
 import io.bkbn.kompendium.core.util.notarizedPutModule
 import io.bkbn.kompendium.core.util.nullableField
+import io.bkbn.kompendium.core.util.nullableNestedObject
 import io.bkbn.kompendium.core.util.overrideFieldInfo
 import io.bkbn.kompendium.core.util.pathParsingTestModule
 import io.bkbn.kompendium.core.util.polymorphicCollectionResponse
@@ -235,6 +236,9 @@ class KompendiumTest : DescribeSpec({
     it("Can serialize a recursive type") {
       openApiTestAllSerializers("simple_recursive.json") { simpleRecursive() }
     }
+    it("Nullable fields do not lead to doom") {
+      openApiTestAllSerializers("nullable_fields.json") { nullableNestedObject() }
+    }
   }
   describe("Constraints") {
     it("Can set a minimum and maximum integer value") {
diff --git a/kompendium-core/src/test/kotlin/io/bkbn/kompendium/core/util/TestModules.kt b/kompendium-core/src/test/kotlin/io/bkbn/kompendium/core/util/TestModules.kt
index 6c13d8f02..3436d8e9e 100644
--- a/kompendium-core/src/test/kotlin/io/bkbn/kompendium/core/util/TestModules.kt
+++ b/kompendium-core/src/test/kotlin/io/bkbn/kompendium/core/util/TestModules.kt
@@ -24,6 +24,7 @@ import io.bkbn.kompendium.core.fixtures.TestResponseInfo.defaultParam
 import io.bkbn.kompendium.core.fixtures.TestResponseInfo.formattedParam
 import io.bkbn.kompendium.core.fixtures.TestResponseInfo.minMaxString
 import io.bkbn.kompendium.core.fixtures.TestResponseInfo.nullableField
+import io.bkbn.kompendium.core.fixtures.TestResponseInfo.nullableNested
 import io.bkbn.kompendium.core.fixtures.TestResponseInfo.regexString
 import io.bkbn.kompendium.core.fixtures.TestResponseInfo.requiredParam
 import io.bkbn.kompendium.core.metadata.RequestInfo
@@ -415,6 +416,16 @@ fun Application.simpleRecursive() {
   }
 }
 
+fun Application.nullableNestedObject() {
+  routing {
+    route("/nullable/nested") {
+      notarizedPost(nullableNested) {
+        call.respond(HttpStatusCode.OK)
+      }
+    }
+  }
+}
+
 fun Application.constrainedIntInfo() {
   routing {
     route("/test/constrained_int") {
diff --git a/kompendium-core/src/test/resources/nullable_fields.json b/kompendium-core/src/test/resources/nullable_fields.json
new file mode 100644
index 000000000..b8e9c8e16
--- /dev/null
+++ b/kompendium-core/src/test/resources/nullable_fields.json
@@ -0,0 +1,119 @@
+{
+  "openapi": "3.0.3",
+  "info": {
+    "title": "Test API",
+    "version": "1.33.7",
+    "description": "An amazing, fully-ish 😉 generated API spec",
+    "termsOfService": "https://example.com",
+    "contact": {
+      "name": "Homer Simpson",
+      "url": "https://gph.is/1NPUDiM",
+      "email": "chunkylover53@aol.com"
+    },
+    "license": {
+      "name": "MIT",
+      "url": "https://github.com/bkbnio/kompendium/blob/main/LICENSE"
+    }
+  },
+  "servers": [
+    {
+      "url": "https://myawesomeapi.com",
+      "description": "Production instance of my API"
+    },
+    {
+      "url": "https://staging.myawesomeapi.com",
+      "description": "Where the fun stuff happens"
+    }
+  ],
+  "paths": {
+    "/nullable/nested": {
+      "post": {
+        "tags": [],
+        "summary": "Has a bunch of nullable fields",
+        "description": "Should still work!",
+        "parameters": [],
+        "requestBody": {
+          "description": "Cool",
+          "content": {
+            "application/json": {
+              "schema": {
+                "$ref": "#/components/schemas/ProfileUpdateRequest"
+              }
+            }
+          },
+          "required": true
+        },
+        "responses": {
+          "200": {
+            "description": "A successful endeavor",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/TestResponse"
+                }
+              }
+            }
+          }
+        },
+        "deprecated": false
+      }
+    }
+  },
+  "components": {
+    "schemas": {
+      "ProfileUpdateRequest": {
+        "properties": {
+          "metadata": {
+            "$ref": "#/components/schemas/ProfileMetadataUpdateRequest"
+          },
+          "mood": {
+            "type": "string",
+            "nullable": true
+          },
+          "viewCount": {
+            "format": "int64",
+            "type": "integer",
+            "nullable": true
+          }
+        },
+        "required": [
+          "mood",
+          "viewCount",
+          "metadata"
+        ],
+        "type": "object"
+      },
+      "ProfileMetadataUpdateRequest": {
+        "properties": {
+          "isPrivate": {
+            "type": "boolean",
+            "nullable": true
+          },
+          "otherThing": {
+            "type": "string",
+            "nullable": true
+          }
+        },
+        "required": [
+          "isPrivate",
+          "otherThing"
+        ],
+        "type": "object"
+      },
+      "TestResponse": {
+        "properties": {
+          "c": {
+            "type": "string"
+          }
+        },
+        "required": [
+          "c"
+        ],
+        "type": "object"
+      }
+    },
+    "securitySchemes": {}
+  },
+  "security": [],
+  "tags": []
+}
diff --git a/kompendium-core/src/testFixtures/kotlin/io/bkbn/kompendium/core/fixtures/TestModels.kt b/kompendium-core/src/testFixtures/kotlin/io/bkbn/kompendium/core/fixtures/TestModels.kt
index d13608443..cbff7f4ee 100644
--- a/kompendium-core/src/testFixtures/kotlin/io/bkbn/kompendium/core/fixtures/TestModels.kt
+++ b/kompendium-core/src/testFixtures/kotlin/io/bkbn/kompendium/core/fixtures/TestModels.kt
@@ -235,3 +235,17 @@ data class ColumnSchema(
   val mode: ColumnMode,
   val subColumns: List<ColumnSchema> = emptyList()
 )
+
+@Serializable
+public data class ProfileUpdateRequest(
+  public val mood: String?,
+  public val viewCount: Long?,
+  public val metadata: ProfileMetadataUpdateRequest?
+)
+
+
+@Serializable
+public data class ProfileMetadataUpdateRequest(
+  public val isPrivate: Boolean?,
+  public val otherThing: String?
+)
diff --git a/kompendium-core/src/testFixtures/kotlin/io/bkbn/kompendium/core/fixtures/TestResponseInfo.kt b/kompendium-core/src/testFixtures/kotlin/io/bkbn/kompendium/core/fixtures/TestResponseInfo.kt
index 4451d59bc..03a2a1da9 100644
--- a/kompendium-core/src/testFixtures/kotlin/io/bkbn/kompendium/core/fixtures/TestResponseInfo.kt
+++ b/kompendium-core/src/testFixtures/kotlin/io/bkbn/kompendium/core/fixtures/TestResponseInfo.kt
@@ -175,6 +175,15 @@ object TestResponseInfo {
     responseInfo = simpleOkResponse()
   )
 
+  val nullableNested = PostInfo<Unit, ProfileUpdateRequest, TestResponse>(
+    summary = "Has a bunch of nullable fields",
+    description = "Should still work!",
+    requestInfo = RequestInfo(
+      description = "Cool"
+    ),
+    responseInfo = simpleOkResponse()
+  )
+
   val minMaxInt = GetInfo<Unit, MinMaxInt>(
     summary = "Constrained int field",
     description = "Cool stuff",