From 04c2c9715d9a3647b10de4f3beb2de014a99d158 Mon Sep 17 00:00:00 2001 From: Sufiyan Date: Mon, 11 Nov 2024 22:53:23 +0530 Subject: [PATCH 1/3] Modify OpenApi allOf parsing logic. - Only top level discriminator information should be considered. - Fix potential bug with PatternBasedAttributeSelectionGenerator --- .../io/specmatic/conversions/OpenApiSpecification.kt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/core/src/main/kotlin/io/specmatic/conversions/OpenApiSpecification.kt b/core/src/main/kotlin/io/specmatic/conversions/OpenApiSpecification.kt index 5b1ed78db..265a5f5a2 100644 --- a/core/src/main/kotlin/io/specmatic/conversions/OpenApiSpecification.kt +++ b/core/src/main/kotlin/io/specmatic/conversions/OpenApiSpecification.kt @@ -1161,12 +1161,12 @@ class OpenApiSpecification( private fun toSpecmaticPattern(mediaType: MediaType, section: String, jsonInFormData: Boolean = false): Pattern = toSpecmaticPattern(mediaType.schema ?: throw ContractException("${section.capitalizeFirstChar()} body definition is missing"), emptyList(), jsonInFormData = jsonInFormData) - private fun resolveDeepAllOfs(schema: Schema, discriminatorDetails: DiscriminatorDetails, typeStack: Set): Pair>, DiscriminatorDetails> { + private fun resolveDeepAllOfs(schema: Schema, discriminatorDetails: DiscriminatorDetails, typeStack: Set, topLevel: Boolean): Pair>, DiscriminatorDetails> { if (schema.allOf == null) return listOf(schema) to discriminatorDetails // Pair [reffed schema]>>> - val newDiscriminatorDetailsDetails: Triple>>>, DiscriminatorDetails>? = schema.discriminator?.let { rawDiscriminator -> + val newDiscriminatorDetailsDetails: Triple>>>, DiscriminatorDetails>? = if (!topLevel) null else schema.discriminator?.let { rawDiscriminator -> rawDiscriminator.propertyName?.let { propertyName -> val mapping = rawDiscriminator.mapping ?: emptyMap() @@ -1178,7 +1178,8 @@ class OpenApiSpecification( val value = mappedSchemaName to resolveDeepAllOfs( mappedSchema, discriminatorDetails, - typeStack + mappedComponentName + typeStack + mappedComponentName, + topLevel = false ) discriminatorValue to value } else { @@ -1212,7 +1213,8 @@ class OpenApiSpecification( resolveDeepAllOfs( referredSchema, discriminatorDetails.plus(newDiscriminatorDetailsDetails), - typeStack + componentName + typeStack + componentName, + topLevel = false ) } else null @@ -1299,7 +1301,7 @@ class OpenApiSpecification( is ComposedSchema -> { if (schema.allOf != null) { - val (deepListOfAllOfs, allDiscriminators) = resolveDeepAllOfs(schema, DiscriminatorDetails(), emptySet()) + val (deepListOfAllOfs, allDiscriminators) = resolveDeepAllOfs(schema, DiscriminatorDetails(), emptySet(), topLevel = true) val explodedDiscriminators = allDiscriminators.explode() From 0180231dfa5b9ed047c149db8ce871c8301fc72f Mon Sep 17 00:00:00 2001 From: Sufiyan Date: Mon, 11 Nov 2024 22:56:50 +0530 Subject: [PATCH 2/3] Add tests for top-level discriminator allOf logic. - Add tests for generating examples using the Interactive Server for discriminator based scenarios top and deep levels. - Add test for the OpenApiSpecification parsing toSpecmaticPattern. --- .../conversions/OpenApiSpecificationTest.kt | 35 ++++++++ .../server/ExamplesInteractiveServerTest.kt | 53 ++++++++++++ .../resources/openapi/vehicle_deep_allof.yaml | 82 +++++++++++++++++++ 3 files changed, 170 insertions(+) create mode 100644 core/src/test/resources/openapi/vehicle_deep_allof.yaml diff --git a/core/src/test/kotlin/io/specmatic/conversions/OpenApiSpecificationTest.kt b/core/src/test/kotlin/io/specmatic/conversions/OpenApiSpecificationTest.kt index 0b357df05..469681fde 100644 --- a/core/src/test/kotlin/io/specmatic/conversions/OpenApiSpecificationTest.kt +++ b/core/src/test/kotlin/io/specmatic/conversions/OpenApiSpecificationTest.kt @@ -9352,6 +9352,40 @@ paths: assertThat(resolvedBodyPattern.pattern.keys).doesNotContain("id?") } + @Test + fun `should not resolve deep discriminators in an allOf schema`() { + val specFile = "src/test/resources/openapi/vehicle_deep_allof.yaml" + val feature = OpenApiSpecification.fromFile(specFile).toFeature() + + assertThat(feature.scenarios).allSatisfy { scenario -> + val responseBodyPattern = resolvedHop(scenario.httpResponsePattern.body, scenario.resolver).let { + when (it) { + is ListPattern -> resolvedHop(it.pattern, scenario.resolver) + else -> it + } + } + val requestBodyPattern = scenario.httpRequestPattern.body.takeIf { it !is NoBodyPattern }?.let { + resolvedHop(scenario.httpRequestPattern.body, scenario.resolver) + } + + assertThat(responseBodyPattern).isNotInstanceOf(AnyPattern::class.java) + if (requestBodyPattern != null) { + when (scenario.method) { + "POST" -> { + assertThat(requestBodyPattern).isInstanceOf(AnyPattern::class.java) + (requestBodyPattern as AnyPattern).let { + assertThat(it.pattern).hasSize(2) + assertThat(it.discriminator!!.property).isEqualTo("type") + assertThat(it.discriminator!!.values).containsExactlyInAnyOrder("car", "truck") + } + } + "PATCH" -> assertThat(requestBodyPattern).isNotInstanceOf(AnyPattern::class.java) + else -> Exception("Unexpected method: ${scenario.method}") + } + } + } + } + private fun ignoreButLogException(function: () -> OpenApiSpecification) { try { function() @@ -9361,3 +9395,4 @@ paths: } } + diff --git a/core/src/test/kotlin/io/specmatic/core/examples/server/ExamplesInteractiveServerTest.kt b/core/src/test/kotlin/io/specmatic/core/examples/server/ExamplesInteractiveServerTest.kt index aefbdbf68..735330047 100644 --- a/core/src/test/kotlin/io/specmatic/core/examples/server/ExamplesInteractiveServerTest.kt +++ b/core/src/test/kotlin/io/specmatic/core/examples/server/ExamplesInteractiveServerTest.kt @@ -7,6 +7,7 @@ import io.specmatic.core.value.JSONObjectValue import io.specmatic.core.value.NumberValue import io.specmatic.core.value.StringValue import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test import java.io.File @@ -65,4 +66,56 @@ class ExamplesInteractiveServerTest { } } } + + @Nested + inner class DiscriminatorExamplesGenerationTests { + private val specFile = File("src/test/resources/openapi/vehicle_deep_allof.yaml") + private val examplesDir = specFile.parentFile.resolve("vehicle_deep_allof_examples") + + @AfterEach + fun cleanUp() { + if (examplesDir.exists()) { + examplesDir.listFiles()?.forEach { it.delete() } + examplesDir.delete() + } + } + + @Test + fun `should generate multiple examples for top level discriminator`() { + val generatedExamples = ExamplesInteractiveServer.generate( + contractFile = specFile, + method = "POST", path = "/vehicles", responseStatusCode = 201, + bulkMode = false, allowOnlyMandatoryKeysInJSONObject = false + ) + + assertThat(generatedExamples).hasSize(2) + assertThat(generatedExamples).allSatisfy { + assertThat(it.created).isTrue() + assertThat(it.path).satisfiesAnyOf( + { path -> assertThat(path).contains("car") }, { path -> assertThat(path).contains("truck") } + ) + } + } + + @Test + fun `should not generate multiple examples for deep nested discriminator`() { + val generatedGetExamples = ExamplesInteractiveServer.generate( + contractFile = specFile, + method = "GET", path = "/vehicles", responseStatusCode = 200, + bulkMode = false, allowOnlyMandatoryKeysInJSONObject = false + ) + + assertThat(generatedGetExamples).hasSize(1) + assertThat(generatedGetExamples.first().path).contains("GET") + + val generatedPatchExamples = ExamplesInteractiveServer.generate( + contractFile = specFile, + method = "PATCH", path = "/vehicles", responseStatusCode = 200, + bulkMode = false, allowOnlyMandatoryKeysInJSONObject = false + ) + + assertThat(generatedPatchExamples).hasSize(1) + assertThat(generatedPatchExamples.first().path).contains("PATCH") + } + } } \ No newline at end of file diff --git a/core/src/test/resources/openapi/vehicle_deep_allof.yaml b/core/src/test/resources/openapi/vehicle_deep_allof.yaml new file mode 100644 index 000000000..399fcd918 --- /dev/null +++ b/core/src/test/resources/openapi/vehicle_deep_allof.yaml @@ -0,0 +1,82 @@ +openapi: 3.0.3 +info: + title: Vehicle Inventory API + description: An API to manage a catalog of vehicles available for sale + version: 1.0.0 +paths: + /vehicles: + get: + responses: + '200': + description: A list of vehicles + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/VehicleResponse' + post: + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/Vehicle' + responses: + '201': + description: Vehicle created + content: + application/json: + schema: + $ref: '#/components/schemas/VehicleResponse' + patch: + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/VehicleResponse' + responses: + '200': + description: Vehicle updated + content: + application/json: + schema: + $ref: '#/components/schemas/VehicleResponse' +components: + schemas: + VehicleResponse: + allOf: + - $ref: '#/components/schemas/Vehicle' + required: + - id + BaseVehicle: + properties: + id: + type: string + type: + type: string + required: + - type + Vehicle: + allOf: + - $ref: '#/components/schemas/BaseVehicle' + discriminator: + propertyName: type + mapping: + car: '#/components/schemas/Car' + truck: '#/components/schemas/Truck' + Car: + allOf: + - $ref: '#/components/schemas/BaseVehicle' + - type: object + properties: + seatingCapacity: + type: integer + Truck: + allOf: + - $ref: '#/components/schemas/BaseVehicle' + - type: object + properties: + trailer: + type: boolean From 11e04beeee2e661943b0800cfabab73294a2d2e3 Mon Sep 17 00:00:00 2001 From: Sufiyan Date: Mon, 11 Nov 2024 23:51:14 +0530 Subject: [PATCH 3/3] Fix - Interactive ExternaliseInlineExamplesTests - Reset ExamplesInteractiveServer Internal Examples Atomic Counter Before each test, resetting the count to initial value. --- .../core/examples/server/ExamplesInteractiveServerTest.kt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/src/test/kotlin/io/specmatic/core/examples/server/ExamplesInteractiveServerTest.kt b/core/src/test/kotlin/io/specmatic/core/examples/server/ExamplesInteractiveServerTest.kt index 735330047..2017c84cc 100644 --- a/core/src/test/kotlin/io/specmatic/core/examples/server/ExamplesInteractiveServerTest.kt +++ b/core/src/test/kotlin/io/specmatic/core/examples/server/ExamplesInteractiveServerTest.kt @@ -8,11 +8,17 @@ import io.specmatic.core.value.NumberValue import io.specmatic.core.value.StringValue import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test import java.io.File class ExamplesInteractiveServerTest { + @BeforeEach + fun resetCounter() { + ExamplesInteractiveServer.resetExampleFileNameCounter() + } + @Nested inner class ExternaliseInlineExamplesTests {