From 64c352064eeac80bf13f9ba43c14598064e937fe Mon Sep 17 00:00:00 2001 From: Ryan Brink <5607577+unredundant@users.noreply.github.com> Date: Sat, 19 Feb 2022 18:39:57 -0500 Subject: [PATCH] fix: nullability breaks object comparison (#202) --- CHANGELOG.md | 6 +- gradle.properties | 2 +- .../kompendium/core/handler/ObjectHandler.kt | 2 +- .../kompendium/core/handler/SchemaHandler.kt | 12 +- .../io/bkbn/kompendium/core/KompendiumTest.kt | 4 + .../bkbn/kompendium/core/util/TestModules.kt | 11 ++ .../src/test/resources/nullable_fields.json | 119 ++++++++++++++++++ .../kompendium/core/fixtures/TestModels.kt | 14 +++ .../core/fixtures/TestResponseInfo.kt | 9 ++ .../kompendium/oas/schema/ObjectSchema.kt | 20 +++ .../kompendium/playground/BasicPlayground.kt | 1 + 11 files changed, 194 insertions(+), 6 deletions(-) create mode 100644 kompendium-core/src/test/resources/nullable_fields.json 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-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 = 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( + summary = "Has a bunch of nullable fields", + description = "Should still work!", + requestInfo = RequestInfo( + description = "Cool" + ), + responseInfo = simpleOkResponse() + ) + val minMaxInt = GetInfo( summary = "Constrained int field", description = "Cool stuff", 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? = 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 ) } +