From 5759f2c4659d1adcb423bfa541758ab805b975ea Mon Sep 17 00:00:00 2001 From: Jeff Lewis Date: Mon, 31 Jan 2022 10:20:24 -0700 Subject: [PATCH] add support for discriminated union conversion to openapi (#86) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add support for discriminated unions to openapi * create missing headers * rename union test operation * remove incorrect union shape * fix json tests * Add discriminator field to alternatives in openapi Co-authored-by: Olivier Mélois --- .../http/json/SchematicJCodecTests.scala | 1 - .../smithy4s/openapi/DiscriminatedUnion.scala | 83 +++++ .../openapi/Smithy4sOpenApiExtension.scala | 3 +- modules/openapi/test/resources/foo.json | 291 +++++++++++------- modules/openapi/test/resources/foo.smithy | 27 +- ...re.amazon.smithy.model.traits.TraitService | 1 + .../DiscriminatedUnionValidator.java | 39 ++- .../DiscriminatedUnionValidatorSpec.scala | 39 +++ sampleSpecs/discriminated.smithy | 7 - 9 files changed, 354 insertions(+), 137 deletions(-) create mode 100644 modules/openapi/src/smithy4s/openapi/DiscriminatedUnion.scala diff --git a/modules/json/test/src/smithy4s/http/json/SchematicJCodecTests.scala b/modules/json/test/src/smithy4s/http/json/SchematicJCodecTests.scala index 10d77dc2c..070d476d7 100644 --- a/modules/json/test/src/smithy4s/http/json/SchematicJCodecTests.scala +++ b/modules/json/test/src/smithy4s/http/json/SchematicJCodecTests.scala @@ -195,7 +195,6 @@ object SchematicJCodecTests extends SimpleIOSuite { expect( result == PayloadData( - None, Some(TestBiggerUnion.OneCase(One(Some("hello")))) ) ) diff --git a/modules/openapi/src/smithy4s/openapi/DiscriminatedUnion.scala b/modules/openapi/src/smithy4s/openapi/DiscriminatedUnion.scala new file mode 100644 index 000000000..bcbc2d739 --- /dev/null +++ b/modules/openapi/src/smithy4s/openapi/DiscriminatedUnion.scala @@ -0,0 +1,83 @@ +/* + * Copyright 2021 Disney Streaming + * + * Licensed under the Tomorrow Open Source Technology License, Version 1.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://disneystreaming.github.io/TOST-1.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package smithy4s.openapi + +import _root_.software.amazon.smithy.jsonschema.JsonSchemaConfig +import _root_.software.amazon.smithy.jsonschema.JsonSchemaMapper +import _root_.software.amazon.smithy.jsonschema.Schema.Builder +import _root_.software.amazon.smithy.model.shapes.Shape + +import scala.jdk.CollectionConverters._ +import software.amazon.smithy.model.node.ObjectNode +import smithy4s.api.DiscriminatedUnionTrait +import software.amazon.smithy.jsonschema.Schema + +class DiscriminatedUnions() extends JsonSchemaMapper { + private final val COMPONENTS = "components" + + def updateSchema( + shape: Shape, + schemaBuilder: Builder, + config: JsonSchemaConfig + ): Builder = { + val maybeDiscriminated = shape.getTrait(classOf[DiscriminatedUnionTrait]) + if (maybeDiscriminated.isPresent()) { + val discriminated = maybeDiscriminated.get() + val discriminatorField = discriminated.getValue() + val unionSchema = schemaBuilder.build() + + val alternatives = unionSchema.getOneOf().asScala + val discriminatedAlts = + alternatives.flatMap(alt => alt.getProperties().asScala) + + val transformedAlts = discriminatedAlts.map { case (label, altSchema) => + val localDiscriminator = Schema + .builder() + .`type`("string") + .enumValues(List(label).asJava) + .build() + val discObject = Schema + .builder() + .`type`("object") + .properties( + Map( + discriminatorField -> localDiscriminator + ).asJava + ) + .required(List(discriminatorField).asJava) + .build() + Schema + .builder() + .allOf( + List(altSchema, discObject).asJava + ) + .build() + }.asJava + + schemaBuilder + .oneOf(transformedAlts) + .putExtension( + "discriminator", + ObjectNode + .builder() + .withMember("propertyName", discriminated.getValue()) + .build() + ) + + } else schemaBuilder + } +} diff --git a/modules/openapi/src/smithy4s/openapi/Smithy4sOpenApiExtension.scala b/modules/openapi/src/smithy4s/openapi/Smithy4sOpenApiExtension.scala index 2f8a836c0..d463340d9 100644 --- a/modules/openapi/src/smithy4s/openapi/Smithy4sOpenApiExtension.scala +++ b/modules/openapi/src/smithy4s/openapi/Smithy4sOpenApiExtension.scala @@ -44,7 +44,8 @@ final class Smithy4sOpenApiExtension() extends Smithy2OpenApiExtension { ).asJava override def getJsonSchemaMappers(): ju.List[JsonSchemaMapper] = List( - new OpenApiJsonSchemaMapper(): JsonSchemaMapper + new OpenApiJsonSchemaMapper(): JsonSchemaMapper, + new DiscriminatedUnions() ).asJava } diff --git a/modules/openapi/test/resources/foo.json b/modules/openapi/test/resources/foo.json index 604d7c081..bfbf95b6b 100644 --- a/modules/openapi/test/resources/foo.json +++ b/modules/openapi/test/resources/foo.json @@ -1,133 +1,204 @@ { "openapi": "3.0.2", "info": { - "title": "HelloWorldService", - "version": "0.0.1" + "title": "HelloWorldService", + "version": "0.0.1" }, "paths": { - "/hello/{name}": { - "get": { - "operationId": "Greet", - "parameters": [ - { - "name": "name", - "in": "path", - "schema": { - "type": "string" - }, - "required": true - }, - { - "name": "X-Bamtech-Partner", - "in": "header", - "schema": { - "type": "string" - } - } - ], - "responses": { - "200": { - "description": "Greet 200 response", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/GreetOutputPayload" - } - } - } - }, - "500": { - "description": "GeneralServerError 500 response", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/GeneralServerErrorResponseContent" - } - } - } - } + "/hello/{name}": { + "get": { + "operationId": "Greet", + "parameters": [ + { + "name": "name", + "in": "path", + "schema": { + "type": "string" + }, + "required": true + }, + { + "name": "X-Bamtech-Partner", + "in": "header", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "Greet200response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GreetOutputPayload" + } + } + } + }, + "500": { + "description": "GeneralServerError500response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GeneralServerErrorResponseContent" + } } + } } - }, - "/untagged": { - "get": { - "operationId": "GetIntOrString", - "responses": { - "200": { - "description": "GetIntOrString 200 response", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/GetIntOrStringResponseContent" - } - } - } - }, - "500": { - "description": "GeneralServerError 500 response", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/GeneralServerErrorResponseContent" - } - } - } - } + } + } + }, + "/untagged": { + "get": { + "operationId": "GetUnion", + "responses": { + "200": { + "description": "GetUnion200response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GetUnionResponseContent" + } } + } + }, + "500": { + "description": "GeneralServerError500response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GeneralServerErrorResponseContent" + } + } + } } + } } + } }, "components": { - "schemas": { - "GeneralServerErrorResponseContent": { - "type": "object", - "properties": { - "message": { - "type": "string" + "schemas": { + "Cat": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, + "CatOrDog": { + "oneOf": [ + { + "allOf": [ + { + "$ref": "#/components/schemas/Cat" + }, + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "cat" + ] } + }, + "required": [ + "type" + ] } + ] }, - "GetIntOrStringResponseContent": { - "type": "object", - "properties": { - "intOrString": { - "$ref": "#/components/schemas/IntOrString" + { + "allOf": [ + { + "$ref": "#/components/schemas/Dog" + }, + { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "dog" + ] } + }, + "required": [ + "type" + ] } + ] + } + ], + "discriminator": { + "propertyName": "type" + } + }, + "Dog": { + "type": "object", + "properties": { + "name": { + "type": "string" }, - "GreetOutputPayload": { - "type": "string" + "breed": { + "type": "string" + } + } + }, + "GeneralServerErrorResponseContent": { + "type": "object", + "properties": { + "message": { + "type": "string" + } + } + }, + "GetUnionResponseContent": { + "type": "object", + "properties": { + "intOrString": { + "$ref": "#/components/schemas/IntOrString" }, - "IntOrString": { - "oneOf": [ - { - "type": "object", - "title": "int", - "properties": { - "int": { - "type": "number", - "format": "int32", - "nullable": true - } - }, - "required": [ - "int" - ] - }, - { - "type": "object", - "title": "string", - "properties": { - "string": { - "type": "string" - } - }, - "required": [ - "string" - ] - } - ] + "catOrDog": { + "$ref": "#/components/schemas/CatOrDog" + } + } + }, + "GreetOutputPayload": { + "type": "string" + }, + "IntOrString": { + "oneOf": [ + { + "type": "object", + "title": "int", + "properties": { + "int": { + "type": "number", + "format": "int32", + "nullable": true + } + }, + "required": [ + "int" + ] + }, + { + "type": "object", + "title": "string", + "properties": { + "string": { + "type": "string" + } + }, + "required": [ + "string" + ] } + ] } + } } } diff --git a/modules/openapi/test/resources/foo.smithy b/modules/openapi/test/resources/foo.smithy index e38903e8c..1949e5c41 100644 --- a/modules/openapi/test/resources/foo.smithy +++ b/modules/openapi/test/resources/foo.smithy @@ -1,12 +1,13 @@ namespace foo use smithy4s.api#simpleRestJson +use smithy4s.api#discriminated @simpleRestJson service HelloWorldService { version: "0.0.1", errors: [GeneralServerError], - operations: [Greet, GetIntOrString] + operations: [Greet, GetUnion] } @readonly @@ -18,8 +19,8 @@ operation Greet { @readonly @http(method: "GET", uri: "/untagged") -operation GetIntOrString { - output: GetIntOrStringResponse +operation GetUnion { + output: GetUnionResponse } structure Person { @@ -43,8 +44,9 @@ structure GeneralServerError { message: String, } -structure GetIntOrStringResponse { - intOrString: IntOrString +structure GetUnionResponse { + intOrString: IntOrString, + catOrDog: CatOrDog } union IntOrString { @@ -52,4 +54,19 @@ union IntOrString { string: String } +structure Cat { + name: String +} + +structure Dog { + name: String, + breed: String +} + +@discriminated("type") +union CatOrDog { + cat: Cat, + dog: Dog +} + diff --git a/modules/protocol/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService b/modules/protocol/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService index 42a68ff45..cd5d88999 100644 --- a/modules/protocol/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService +++ b/modules/protocol/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService @@ -1,3 +1,4 @@ smithy4s.api.SimpleRestJsonTrait$Provider smithy4s.api.UncheckedExamplesTrait$Provider smithy4s.api.UuidFormatTrait$Provider +smithy4s.api.DiscriminatedUnionTrait$Provider diff --git a/modules/protocol/src/smithy4s/api/validation/DiscriminatedUnionValidator.java b/modules/protocol/src/smithy4s/api/validation/DiscriminatedUnionValidator.java index c9b415a9d..3552584a8 100644 --- a/modules/protocol/src/smithy4s/api/validation/DiscriminatedUnionValidator.java +++ b/modules/protocol/src/smithy4s/api/validation/DiscriminatedUnionValidator.java @@ -17,29 +17,42 @@ package smithy4s.api.validation; import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.validation.AbstractValidator; import software.amazon.smithy.model.validation.ValidationEvent; import smithy4s.api.DiscriminatedUnionTrait; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; public final class DiscriminatedUnionValidator extends AbstractValidator { - @Override - public List validate(Model model) { - return model.getShapesWithTrait(DiscriminatedUnionTrait.class).stream().flatMap(unionShape -> { - return unionShape.asUnionShape().get().getAllMembers().entrySet().stream().flatMap(entry -> { - Optional maybeTarget = model.getShape(entry.getValue().getTarget()); - if (maybeTarget.isPresent() && maybeTarget.get().isStructureShape()) { // if not defined then shape won't be structure - return Stream.empty(); - } else { - return Stream.of(error(entry.getValue(), String.format("Target of member '%s' is not a structure shape", entry.getKey()))); - } - }); - }).collect(Collectors.toList()); - } + @Override + public List validate(Model model) { + return model.getShapesWithTrait(DiscriminatedUnionTrait.class).stream().flatMap(unionShape -> { + DiscriminatedUnionTrait discriminated = unionShape.getTrait(DiscriminatedUnionTrait.class).get(); + return unionShape.asUnionShape().get().getAllMembers().entrySet().stream().flatMap(entry -> { + Optional maybeTarget = model.getShape(entry.getValue().getTarget()); + if (maybeTarget.isPresent() && maybeTarget.get().isStructureShape()) { // if not defined then shape + // won't be structure + Map structureMembers = maybeTarget.get().asStructureShape().get() + .getAllMembers(); + if (structureMembers.get(discriminated.getValue()) != null) { + return Stream.of(error(entry.getValue(), + String.format("Target of member '%s' contains discriminator '%s'", entry.getKey(), + discriminated.getValue()))); + } else { + return Stream.empty(); + } + } else { + return Stream.of(error(entry.getValue(), + String.format("Target of member '%s' is not a structure shape", entry.getKey()))); + } + }); + }).collect(Collectors.toList()); + } } diff --git a/modules/protocol/test/src/smithy4s/api/validation/DiscriminatedUnionValidatorSpec.scala b/modules/protocol/test/src/smithy4s/api/validation/DiscriminatedUnionValidatorSpec.scala index e4f615363..623219057 100644 --- a/modules/protocol/test/src/smithy4s/api/validation/DiscriminatedUnionValidatorSpec.scala +++ b/modules/protocol/test/src/smithy4s/api/validation/DiscriminatedUnionValidatorSpec.scala @@ -24,6 +24,7 @@ import scala.jdk.CollectionConverters._ import software.amazon.smithy.model.validation.ValidationEvent import software.amazon.smithy.model.validation.Severity import software.amazon.smithy.model.shapes.StructureShape +import software.amazon.smithy.model.shapes.ShapeId object DiscriminatedUnionValidatorSpec extends weaver.FunSuite { @@ -62,6 +63,44 @@ object DiscriminatedUnionValidatorSpec extends weaver.FunSuite { expect(result == expected) } + test("return error when target structures contain discriminator") { + val struct = StructureShape + .builder() + .id("test#struct") + .addMember("type", ShapeId.fromParts("smithy.api", "String")) + .build() + val member = + MemberShape + .builder() + .target("test#struct") + .id("test#test$test") + .build() + val union = UnionShape + .builder() + .addTrait(new DiscriminatedUnionTrait("type")) + .id("test#test") + .addMember(member) + .build() + + val model = + Model.builder().addShapes(struct, union).build() + + val result = validator.validate(model).asScala.toList + + val expected = List( + ValidationEvent + .builder() + .id("DiscriminatedUnion") + .shape(member) + .severity(Severity.ERROR) + .message( + "Target of member 'test' contains discriminator 'type'" + ) + .build() + ) + expect(result == expected) + } + test("return no error when union contains only structure members") { val struct = StructureShape.builder().id("test#struct").build() val member = diff --git a/sampleSpecs/discriminated.smithy b/sampleSpecs/discriminated.smithy index 095db959e..49dc508ca 100644 --- a/sampleSpecs/discriminated.smithy +++ b/sampleSpecs/discriminated.smithy @@ -30,16 +30,9 @@ structure TestDiscriminatedOutput { } structure PayloadData { - testUnion: TestUnion, testBiggerUnion: TestBiggerUnion } -@discriminated("type") -union TestUnion { - one: String, - two: Integer -} - @discriminated("tpe") union TestBiggerUnion { one: One,