From 99b3734206c9bcdc9327f5c8cca82105730a8360 Mon Sep 17 00:00:00 2001 From: Chase Coalwell <782571+srchase@users.noreply.github.com> Date: Thu, 28 Sep 2023 16:21:12 -0600 Subject: [PATCH] Add option to deconflict errors with same status code for OpenAPI --- .../guides/converting-to-openapi.rst | 45 +-- .../DeconflictErrorsWithSharedStatusCode.java | 158 ++++++++++ .../model/transform/ModelTransformer.java | 11 + ...onflictErrorsWithSharedStatusCodeTest.java | 48 ++++ ...ing-errors-with-conflicting-headers.smithy | 28 ++ .../model/transform/conflicting-errors.smithy | 55 ++++ .../transform/deconflicted-errors.smithy | 54 ++++ .../amazon/smithy/openapi/OpenApiConfig.java | 32 +++ .../openapi/fromsmithy/OpenApiConverter.java | 6 + .../protocols/AwsRestJson1Protocol.java | 41 +++ .../protocols/AwsRestJson1ProtocolTest.java | 25 ++ ...code-collision-test-use-oneof.openapi.json | 272 ++++++++++++++++++ .../error-code-collision-test.smithy | 111 +++++++ 13 files changed, 856 insertions(+), 30 deletions(-) create mode 100644 smithy-model/src/main/java/software/amazon/smithy/model/transform/DeconflictErrorsWithSharedStatusCode.java create mode 100644 smithy-model/src/test/java/software/amazon/smithy/model/transform/DeconflictErrorsWithSharedStatusCodeTest.java create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/transform/conflicting-errors-with-conflicting-headers.smithy create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/transform/conflicting-errors.smithy create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/transform/deconflicted-errors.smithy create mode 100644 smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/protocols/error-code-collision-test-use-oneof.openapi.json create mode 100644 smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/protocols/error-code-collision-test.smithy diff --git a/docs/source-2.0/guides/converting-to-openapi.rst b/docs/source-2.0/guides/converting-to-openapi.rst index 5b79ed0dfb2..06f2569d63d 100644 --- a/docs/source-2.0/guides/converting-to-openapi.rst +++ b/docs/source-2.0/guides/converting-to-openapi.rst @@ -598,37 +598,22 @@ disableIntegerFormat (``boolean``) .. _generate-openapi-setting-onErrorStatusConflict: onErrorStatusConflict (``String``) - Specifies how to resolve multiple error responses that share a same HTTP status code. - This behavior can be customized using the following values for the ``onErrorStatusConflict`` setting: + Specifies how to resolve multiple error responses that share the same HTTP + status code. This behavior can be enabled using the following values for + the ``onErrorStatusConflict`` setting: ``oneOf`` - Use OpenAPI's ``oneOf`` keyword to combine error responses with same HTTP status code. The ``oneOf`` option - wraps schemas for contents of conflicting errors responses schemas into a synthetic union schema using - OpenAPI's ``oneOf`` keyword. - ``properties`` - Use ``properties`` field of OpenAPI schema object to combine error responses with same HTTP status code. - The ``properties`` option combines the conflicting error structure shapes into one union error shape that - contains all members from each and every conflicting error. - - .. note:: - ``oneOf`` keyword is not supported by Amazon API Gateway. - - Both options generate a single combined response object called "UnionError XXX Response" in the - OpenAPI model output, where "XXX" is the status code shared by multiple errors. Both options drop - the ``@required`` trait from all members of conflicting error structures, making them optional. - - .. warning:: - When using ``properties`` option, make sure that conflicting error structure shapes do not have member(s) - that have same name while having different target shapes. If member shapes with same name - (in conflicting error structures) target - different shapes, error shapes will not be able to be merged into one union error shape, and - an exception will be thrown. - - .. warning:: - Regardless of the setting, an exception will be thrown if any one of conflicting error structure shape - has a member shape with ``@httpPayload`` trait. - - By default, this setting is set to ``oneOf``. + Use OpenAPI's ``oneOf`` keyword to combine error responses with same + HTTP status code. The ``oneOf`` option wraps schemas for contents of + conflicting errors responses schemas into a synthetic union schema + using OpenAPI's ``oneOf`` keyword. + + By default, this setting is disabled. When enabled, a single combined + response object will be included in the OpenAPI model output. Any member of + the conflicting errors bound to a HTTP header will be added to the + top-level response. If any headers conflict, an error will be thrown. + Remaining members will be left in place on the conflicting errors. The + modified conflicting errors are then added to the combined response object. .. code-block:: json :caption: smithy-build.json @@ -638,7 +623,7 @@ onErrorStatusConflict (``String``) "plugins": { "openapi": { "service": "smithy.example#Weather", - "onErrorStatusConflict": "properties" + "onErrorStatusConflict": "oneOf" } } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/transform/DeconflictErrorsWithSharedStatusCode.java b/smithy-model/src/main/java/software/amazon/smithy/model/transform/DeconflictErrorsWithSharedStatusCode.java new file mode 100644 index 00000000000..5359fce8957 --- /dev/null +++ b/smithy-model/src/main/java/software/amazon/smithy/model/transform/DeconflictErrorsWithSharedStatusCode.java @@ -0,0 +1,158 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.model.transform; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.TopDownIndex; +import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.shapes.StructureShape; +import software.amazon.smithy.model.shapes.UnionShape; +import software.amazon.smithy.model.traits.ErrorTrait; +import software.amazon.smithy.model.traits.HttpErrorTrait; +import software.amazon.smithy.model.traits.HttpHeaderTrait; +import software.amazon.smithy.model.traits.Trait; +import software.amazon.smithy.utils.Pair; + +/** + * Deconflicts errors on operations that share the same status code by replacing + * the conflicting errors with a synthetic error structure that contains hoisted + * members that were bound to HTTP headers. The conflicting errors are added to + * a union on the synthetic error. + */ +public class DeconflictErrorsWithSharedStatusCode { + + private final ServiceShape forService; + + DeconflictErrorsWithSharedStatusCode(ServiceShape forService) { + this.forService = forService; + } + + Model transform(ModelTransformer transformer, Model model) { + // Copy any service errors to the operations to find all potential conflicts. + model = transformer.copyServiceErrorsToOperations(model, forService); + TopDownIndex topDownIndex = TopDownIndex.of(model); + List shapesToReplace = new ArrayList<>(); + + for (OperationShape operation : topDownIndex.getContainedOperations(forService)) { + OperationShape.Builder replacementOperation = operation.toBuilder(); + boolean replaceOperation = false; + + // Collect errors that share the same status code. + Map> statusCodesToErrors = new HashMap<>(); + for (ShapeId errorId : operation.getErrors()) { + StructureShape error = model.expectShape(errorId, StructureShape.class); + String statusCode = error.hasTrait(HttpErrorTrait.ID) + ? Integer.toString(error.getTrait(HttpErrorTrait.class).get().getCode()) + : Integer.toString(error.getTrait(ErrorTrait.class).get().getDefaultHttpStatusCode()); + statusCodesToErrors.computeIfAbsent(statusCode, k -> new ArrayList<>()).add(error); + } + + // Create union error for errors with same status code. + for (Map.Entry> statusCodeToErrors : statusCodesToErrors.entrySet()) { + if (statusCodeToErrors.getValue().size() > 1) { + replaceOperation = true; + List errors = statusCodeToErrors.getValue(); + // Create a new top-level synthetic error and all the shapes that need replaced for it. + Pair> syntheticErrorPair = synthesizeErrorUnion(operation.getId().getName(), + statusCodeToErrors.getKey(), errors); + for (StructureShape error : errors) { + replacementOperation.removeError(error.getId()); + } + replacementOperation.addError(syntheticErrorPair.getLeft()); + shapesToReplace.add(syntheticErrorPair.getLeft()); + shapesToReplace.addAll(syntheticErrorPair.getRight()); + } + } + // Replace the operation if it has been updated with a synthetic error. + if (replaceOperation) { + replacementOperation.build(); + shapesToReplace.add(replacementOperation.build()); + } + } + + return transformer.replaceShapes(model, shapesToReplace); + } + + // Return synthetic error, along with any updated shapes. + private Pair> synthesizeErrorUnion(String operationName, String statusCode, + List errors) { + List replacementShapes = new ArrayList<>(); + StructureShape.Builder errorResponse = StructureShape.builder(); + ShapeId errorResponseId = ShapeId.fromParts(forService.getId().getNamespace(), + operationName + statusCode + "Error"); + errorResponse.id(errorResponseId); + errorResponse.addTraits(getErrorTraitsFromStatusCode(statusCode)); + Map headerTraitMap = new HashMap<>(); + UnionShape.Builder errorUnion = UnionShape.builder().id( + ShapeId.fromParts(errorResponseId.getNamespace(), errorResponseId.getName() + "Content")); + for (StructureShape error : errors) { + StructureShape newError = createNewError(error, headerTraitMap); + replacementShapes.add(newError); + MemberShape newErrorMember = MemberShape.builder() + .id(errorUnion.getId().withMember(newError.getId().getName())) + .target(newError.getId()) + .build(); + replacementShapes.add(newErrorMember); + errorUnion.addMember(newErrorMember); + } + UnionShape union = errorUnion.build(); + replacementShapes.add(union); + errorResponse.addMember(MemberShape.builder() + .id(errorResponseId.withMember("errorUnion")) + .target(union.getId()) + .build()); + // Add members with hoisted HttpHeader traits. + for (Map.Entry entry : headerTraitMap.entrySet()) { + errorResponse.addMember(MemberShape.builder().id(errorResponseId.withMember(entry.getKey())) + .addTrait(entry.getValue()).target("smithy.api#String").build()); + } + StructureShape built = errorResponse.build(); + return Pair.of(built, replacementShapes); + } + + private StructureShape createNewError(StructureShape oldError, Map headerMap) { + StructureShape.Builder newErrorBuilder = oldError.toBuilder().clearMembers(); + for (MemberShape member : oldError.getAllMembers().values()) { + String name = member.getMemberName(); + // Collect HttpHeaderTraits to hoist. + if (member.hasTrait(HttpHeaderTrait.ID)) { + HttpHeaderTrait trait = member.expectTrait(HttpHeaderTrait.class); + if (headerMap.containsKey(name)) { + if (!headerMap.get(name).equals(trait)) { + throw new ModelTransformException("conflicting header when de-conflicting"); + } + } else { + headerMap.put(name, trait); + } + } else { + newErrorBuilder.addMember(member.toBuilder().id(newErrorBuilder.getId().withMember(name)).build()); + } + } + return newErrorBuilder.build(); + } + + private List getErrorTraitsFromStatusCode(String statusCode) { + List traits = new ArrayList<>(); + + if (statusCode.charAt(0) == '4') { + traits.add(new ErrorTrait("client")); + } else { + traits.add(new ErrorTrait("server")); + } + traits.add(new HttpErrorTrait(Integer.parseInt(statusCode))); + return traits; + } +} + + diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java b/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java index e822cb38ae9..44aff87eace 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/transform/ModelTransformer.java @@ -665,5 +665,16 @@ public Model downgradeToV1(Model model) { */ public Model removeInvalidDefaults(Model model) { return new RemoveInvalidDefaults().transform(this, model); + + } + + /** + * De-conflicts errors that share a status code. + * + * @param model Model to transform. + * @return Returns the transformed model. + */ + public Model deConflictErrorsWithSharedStatusCode(Model model, ServiceShape forService) { + return new DeconflictErrorsWithSharedStatusCode(forService).transform(this, model); } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/transform/DeconflictErrorsWithSharedStatusCodeTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/transform/DeconflictErrorsWithSharedStatusCodeTest.java new file mode 100644 index 00000000000..c9ef6bbf34d --- /dev/null +++ b/smithy-model/src/test/java/software/amazon/smithy/model/transform/DeconflictErrorsWithSharedStatusCodeTest.java @@ -0,0 +1,48 @@ +package software.amazon.smithy.model.transform; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.shapes.ModelSerializer; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.model.shapes.ShapeId; + +public class DeconflictErrorsWithSharedStatusCodeTest { + @Test + public void deconflictErrorsWithSharedStatusCodes() { + Model input = Model.assembler() + .addImport(getClass().getResource("conflicting-errors.smithy")) + .assemble() + .unwrap(); + Model output = Model.assembler() + .addImport(getClass().getResource("deconflicted-errors.smithy")) + .assemble() + .unwrap(); + + ModelTransformer transformer = ModelTransformer.create(); + + ServiceShape service = input.expectShape(ShapeId.from("smithy.example#MyService"), ServiceShape.class); + + Model result = transformer.deConflictErrorsWithSharedStatusCode(input, service); + + Node actual = ModelSerializer.builder().build().serialize(result); + Node expected = ModelSerializer.builder().build().serialize(output); + Node.assertEquals(actual, expected); + } + + @Test + public void throwsWhenHeadersConflict() { + Model model = Model.assembler() + .addImport(getClass().getResource("conflicting-errors-with-conflicting-headers.smithy")) + .assemble() + .unwrap(); + + ModelTransformer transformer = ModelTransformer.create(); + + ServiceShape service = model.expectShape(ShapeId.from("smithy.example#MyService"), ServiceShape.class); + assertThrows(ModelTransformException.class, + () -> transformer.deConflictErrorsWithSharedStatusCode(model, service)); + } +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/transform/conflicting-errors-with-conflicting-headers.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/conflicting-errors-with-conflicting-headers.smithy new file mode 100644 index 00000000000..0b858e38662 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/conflicting-errors-with-conflicting-headers.smithy @@ -0,0 +1,28 @@ +$version: "2.0" + +namespace smithy.example + +service MyService { + operations: [MyOperation] +} + +operation MyOperation { + input := { + string: String + } + errors: [FooError, BarError] +} + +@error("client") +@httpError(429) +structure FooError { + @httpHeader("x-foo") + xFoo: String +} + +@error("client") +@httpError(429) +structure BarError { + @httpHeader("x-bar") + xFoo: String +} \ No newline at end of file diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/transform/conflicting-errors.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/conflicting-errors.smithy new file mode 100644 index 00000000000..b6eb3f923b4 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/conflicting-errors.smithy @@ -0,0 +1,55 @@ +$version: "2.0" + +namespace smithy.example + +service MyService { + operations: [MyOperation] + errors: [FooServiceLevelError, BarServiceLevelError] +} + +operation MyOperation { + input := { + string: String + } + errors: [FooError, BarError] +} + +@error("client") +@httpError(429) +structure FooServiceLevelError { + @httpHeader("x-service-foo") + xServiceFoo: String + + @httpHeader("x-common") + xCommon: String +} + +@error("client") +@httpError(429) +structure BarServiceLevelError { + @httpHeader("x-service-bar") + xServiceBar: String + + @httpHeader("x-common") + xCommon: String +} + +@error("client") +@httpError(429) +structure FooError { + @httpHeader("x-foo") + xFoo: String + + @httpHeader("x-common") + xCommon: String +} + +@error("client") +@httpError(429) +structure BarError { + @httpHeader("x-bar") + xBar: String + + @httpHeader("x-common") + xCommon: String +} \ No newline at end of file diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/transform/deconflicted-errors.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/deconflicted-errors.smithy new file mode 100644 index 00000000000..dc76dad0ec5 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/deconflicted-errors.smithy @@ -0,0 +1,54 @@ +$version: "2.0" + +namespace smithy.example + +service MyService { + operations: [MyOperation] + errors: [FooServiceLevelError, BarServiceLevelError] +} + +operation MyOperation { + input := { + string: String + } + errors: [MyOperation429Error] +} + +@error("client") +@httpError(429) +structure MyOperation429Error { + errorUnion: MyOperation429ErrorContent + @httpHeader("x-foo") + xFoo: String + @httpHeader("x-bar") + xBar: String + @httpHeader("x-common") + xCommon: String + @httpHeader("x-service-foo") + xServiceFoo: String + @httpHeader("x-service-bar") + xServiceBar: String +} + +union MyOperation429ErrorContent { + FooError: FooError + BarError: BarError + FooServiceLevelError: FooServiceLevelError + BarServiceLevelError: BarServiceLevelError +} + +@error("client") +@httpError(429) +structure FooServiceLevelError {} + +@error("client") +@httpError(429) +structure BarServiceLevelError {} + +@error("client") +@httpError(429) +structure FooError {} + +@error("client") +@httpError(429) +structure BarError {} \ No newline at end of file diff --git a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java index 43f6ea6b420..cf0db189498 100644 --- a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java +++ b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java @@ -46,6 +46,23 @@ public enum HttpPrefixHeadersStrategy { WARN } + /** Specifies how to resolve multiple error responses with same error codes. */ + public enum ErrorStatusConflictHandlingStrategy { + /** The default setting that uses OpenAPI's oneOf keyword to combine multiple schemas for same media type. */ + ONE_OF("oneOf"); + + private final String stringValue; + + ErrorStatusConflictHandlingStrategy(String stringValue) { + this.stringValue = stringValue; + } + + @Override + public String toString() { + return stringValue; + } + } + /** The JSON pointer to where OpenAPI schema components should be written. */ private static final String SCHEMA_COMPONENTS_POINTER = "#/components/schemas"; @@ -84,6 +101,7 @@ public enum HttpPrefixHeadersStrategy { private List externalDocs = ListUtils.of( "Homepage", "API Reference", "User Guide", "Developer Guide", "Reference", "Guide"); private boolean disableIntegerFormat = false; + private ErrorStatusConflictHandlingStrategy onErrorStatusConflict; private OpenApiVersion version = OpenApiVersion.VERSION_3_0_2; public OpenApiConfig() { @@ -336,6 +354,20 @@ public void setDisableIntegerFormat(boolean disableIntegerFormat) { this.disableIntegerFormat = disableIntegerFormat; } + + public ErrorStatusConflictHandlingStrategy getOnErrorStatusConflict() { + return onErrorStatusConflict; + } + + /** + * Specifies what to do when multiple error responses share the same HTTP status code. + * + * @param onErrorStatusConflict Strategy to use for multiple errors with same status code. + */ + public void setOnErrorStatusConflict(ErrorStatusConflictHandlingStrategy onErrorStatusConflict) { + this.onErrorStatusConflict = Objects.requireNonNull(onErrorStatusConflict); + } + /** * Creates an OpenApiConfig from a Node value. * diff --git a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java index fa7855f681a..7807d6de1cb 100644 --- a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java +++ b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java @@ -189,6 +189,12 @@ private ConversionEnvironment createConversionEnvironment(Model // Remove mixins from the model. model = ModelTransformer.create().flattenAndRemoveMixins(model); + // De-conflict errors that share the same status code. + if (config.getOnErrorStatusConflict() != null && config.getOnErrorStatusConflict() + .equals(OpenApiConfig.ErrorStatusConflictHandlingStrategy.ONE_OF)) { + model = ModelTransformer.create().deConflictErrorsWithSharedStatusCode(model, service); + } + JsonSchemaConverter.Builder jsonSchemaConverterBuilder = JsonSchemaConverter.builder(); jsonSchemaConverterBuilder.model(model); diff --git a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/protocols/AwsRestJson1Protocol.java b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/protocols/AwsRestJson1Protocol.java index 5b589a0e686..c486f39be0f 100644 --- a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/protocols/AwsRestJson1Protocol.java +++ b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/protocols/AwsRestJson1Protocol.java @@ -15,6 +15,8 @@ package software.amazon.smithy.openapi.fromsmithy.protocols; +import static software.amazon.smithy.openapi.OpenApiConfig.ErrorStatusConflictHandlingStrategy.ONE_OF; + import java.util.List; import java.util.Set; import java.util.TreeSet; @@ -25,10 +27,13 @@ import software.amazon.smithy.model.Model; import software.amazon.smithy.model.knowledge.HttpBinding; import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.OperationShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.model.shapes.StructureShape; +import software.amazon.smithy.model.shapes.UnionShape; +import software.amazon.smithy.model.traits.HttpErrorTrait; import software.amazon.smithy.model.traits.TimestampFormatTrait; import software.amazon.smithy.openapi.OpenApiConfig; import software.amazon.smithy.openapi.fromsmithy.Context; @@ -129,9 +134,45 @@ Schema createDocumentSchema( } StructureShape cleanedShape = containerShapeBuilder.build(); + + // If errors with the same status code have been de-conflicted, + // hoist the members from the synthetic target union and convert + // the error structure to a union, so that a `oneOf` can be used + // in the output without added nesting. + if (context.getConfig().getOnErrorStatusConflict() != null + && context.getConfig().getOnErrorStatusConflict().equals(ONE_OF) + && targetsSyntheticError(cleanedShape, context)) { + UnionShape.Builder asUnion = UnionShape.builder().id(cleanedShape.getId()); + UnionShape targetUnion = context.getModel().expectShape( + cleanedShape.getAllMembers().values().stream().findFirst().get().getTarget(), UnionShape.class); + for (MemberShape member : targetUnion.getAllMembers().values()) { + String name = member.getMemberName(); + asUnion.addMember(member.toBuilder().id(cleanedShape.getId().withMember(name)).build()); + } + return context.getJsonSchemaConverter().convertShape(asUnion.build()).getRootSchema(); + } return context.getJsonSchemaConverter().convertShape(cleanedShape).getRootSchema(); } + private boolean targetsSyntheticError(StructureShape shape, Context context) { + if (shape.hasTrait(HttpErrorTrait.ID)) { + HttpErrorTrait trait = shape.expectTrait(HttpErrorTrait.class); + String suffix = trait.getCode() + "Error"; + if (shape.getId().getName().endsWith(suffix)) { + return hasSingleUnionMember(shape, context.getModel()); + } + } + return false; + } + + private boolean hasSingleUnionMember(StructureShape shape, Model model) { + long unionCount = shape.getAllMembers().values().stream() + .map(member -> model.expectShape(member.getTarget())) + .filter(Shape::isUnionShape) + .count(); + return unionCount == 1; + } + @Override Node transformSmithyValueToProtocolValue(Node value) { return value; diff --git a/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/protocols/AwsRestJson1ProtocolTest.java b/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/protocols/AwsRestJson1ProtocolTest.java index 0e31bb7c5e5..4b588f984e5 100644 --- a/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/protocols/AwsRestJson1ProtocolTest.java +++ b/smithy-openapi/src/test/java/software/amazon/smithy/openapi/fromsmithy/protocols/AwsRestJson1ProtocolTest.java @@ -288,4 +288,29 @@ public void convertsExamples() { Node.assertEquals(result, expectedNode); } } + + @Test + public void combinesErrorsWithSameStatusCode() { + Model model = Model.assembler() + .addImport(getClass().getResource("error-code-collision-test.smithy")) + .discoverModels() + .assemble() + .unwrap(); + OpenApiConfig config = new OpenApiConfig(); + config.setService(ShapeId.from("example#Example")); + config.setOnErrorStatusConflict(OpenApiConfig.ErrorStatusConflictHandlingStrategy.ONE_OF); + ObjectNode result = OpenApiConverter.create() + .config(config) + .convertToNode(model); + InputStream openApiStream = getClass() + .getResourceAsStream("error-code-collision-test-use-oneof.openapi.json"); + + if (openApiStream == null) { + throw new RuntimeException("OpenAPI model not found for test case: " + + "error-code-collision-test-use-properties.openapi.json"); + } else { + Node expectedNode = Node.parse(IoUtils.toUtf8String(openApiStream)); + Node.assertEquals(result, expectedNode); + } + } } diff --git a/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/protocols/error-code-collision-test-use-oneof.openapi.json b/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/protocols/error-code-collision-test-use-oneof.openapi.json new file mode 100644 index 00000000000..4c85edc69a0 --- /dev/null +++ b/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/protocols/error-code-collision-test-use-oneof.openapi.json @@ -0,0 +1,272 @@ +{ + "openapi": "3.0.2", + "info": { + "title": "Example", + "version": "2006-03-01" + }, + "paths": { + "/time": { + "get": { + "operationId": "GetCurrentTime", + "responses": { + "200": { + "description": "GetCurrentTime 200 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GetCurrentTimeResponseContent" + } + } + } + }, + "404": { + "description": "GetCurrentTime404Error 404 response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GetCurrentTime404ErrorResponseContent" + } + } + } + }, + "429": { + "description": "GetCurrentTime429Error 429 response", + "headers": { + "error1-header": { + "schema": { + "type": "string" + } + }, + "error2-header": { + "schema": { + "type": "string" + } + } + }, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/GetCurrentTime429ErrorResponseContent" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "Error1": { + "type": "object", + "properties": { + "message": { + "type": "string" + } + }, + "required": [ + "message" + ] + }, + "Error2": { + "type": "object", + "properties": { + "message": { + "type": "string" + }, + "message2": { + "type": "string" + }, + "message3": { + "type": "string" + } + }, + "required": [ + "message" + ] + }, + "Error3": { + "type": "object", + "properties": { + "message24": { + "type": "string" + } + }, + "required": [ + "message24" + ] + }, + "Error4": { + "type": "object", + "properties": { + "message365": { + "type": "string" + } + }, + "required": [ + "message365" + ] + }, + "Error5": { + "type": "object", + "properties": { + "message": { + "type": "string" + } + }, + "required": [ + "message" + ] + }, + "Error6": { + "type": "object", + "properties": { + "message": { + "type": "string" + } + }, + "required": [ + "message" + ] + }, + "Error7": { + "type": "object", + "properties": { + "message": { + "type": "string" + } + }, + "required": [ + "message" + ] + }, + "Error8": { + "type": "object", + "properties": { + "message2": { + "type": "string" + } + }, + "required": [ + "message2" + ] + }, + "GetCurrentTime404ErrorResponseContent": { + "oneOf": [ + { + "type": "object", + "title": "Error7", + "properties": { + "Error7": { + "$ref": "#/components/schemas/Error7" + } + }, + "required": [ + "Error7" + ] + }, + { + "type": "object", + "title": "Error8", + "properties": { + "Error8": { + "$ref": "#/components/schemas/Error8" + } + }, + "required": [ + "Error8" + ] + } + ] + }, + "GetCurrentTime429ErrorResponseContent": { + "oneOf": [ + { + "type": "object", + "title": "Error1", + "properties": { + "Error1": { + "$ref": "#/components/schemas/Error1" + } + }, + "required": [ + "Error1" + ] + }, + { + "type": "object", + "title": "Error2", + "properties": { + "Error2": { + "$ref": "#/components/schemas/Error2" + } + }, + "required": [ + "Error2" + ] + }, + { + "type": "object", + "title": "Error3", + "properties": { + "Error3": { + "$ref": "#/components/schemas/Error3" + } + }, + "required": [ + "Error3" + ] + }, + { + "type": "object", + "title": "Error4", + "properties": { + "Error4": { + "$ref": "#/components/schemas/Error4" + } + }, + "required": [ + "Error4" + ] + }, + { + "type": "object", + "title": "Error5", + "properties": { + "Error5": { + "$ref": "#/components/schemas/Error5" + } + }, + "required": [ + "Error5" + ] + }, + { + "type": "object", + "title": "Error6", + "properties": { + "Error6": { + "$ref": "#/components/schemas/Error6" + } + }, + "required": [ + "Error6" + ] + } + ] + }, + "GetCurrentTimeResponseContent": { + "type": "object", + "properties": { + "time": { + "type": "number", + "format": "double" + } + }, + "required": [ + "time" + ] + } + } + } +} \ No newline at end of file diff --git a/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/protocols/error-code-collision-test.smithy b/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/protocols/error-code-collision-test.smithy new file mode 100644 index 00000000000..b20bec28612 --- /dev/null +++ b/smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/protocols/error-code-collision-test.smithy @@ -0,0 +1,111 @@ +namespace example + +use aws.protocols#restJson1 +use smithy.framework#ValidationException + +@restJson1 +service Example { + version: "2006-03-01", + operations: [GetCurrentTime], + errors: [ + Error1, + Error2, + Error3, + Error4, + Error5, + Error6, + Error7, + Error8 + ] +} + +@error("client") +@retryable(throttling: true) +@httpError(429) // Too many requests +structure Error1 { + @httpHeader("error1-header") + @required + header: String, + + @required + message: String, +} + +@error("client") +@retryable(throttling: true) +@httpError(429) // Too many requests +structure Error2 { + @httpHeader("error2-header") + @required + header2: String, + + @required + message: String, + + message2: String, + + message3: String +} + +@error("client") +@retryable(throttling: true) +@httpError(429) // Too many requests +structure Error3 { + @required + message24 : String, +} + +@error("client") +@retryable(throttling: true) +@httpError(429) // Too many requests +structure Error4 { + @required + message365: String, +} + +@error("client") +@retryable(throttling: true) +@httpError(429) // Too many requests +structure Error5 { + @required + message: String, +} + +@error("client") +@retryable(throttling: true) +@httpError(429) // Too many requests +structure Error6 { + @required + message: String, +} + +@error("client") +@httpError(404) // Too many requests +structure Error7 { + @required + message: String, +} + +@error("client") +@httpError(404) // Too many requests +structure Error8 { + @required + message2: String, +} + + +@readonly +@http(uri: "/time", method: "GET") +operation GetCurrentTime { + input: GetCurrentTimeInput, + output: GetCurrentTimeOutput +} + +@input +structure GetCurrentTimeInput {} + +@output +structure GetCurrentTimeOutput { + @required + time: Timestamp +}