Support serializing traits into specification extensions in OpenAPI

[Xtansia]

Description of changes

Adds a "meta-trait" smithy.openapi#specificationExtension that can be used to annotate other traits to indicate they should be serialized into specification extension (x-*) properties when converting to OpenAPI. This is supported on both shapes/"schemas" as well as operations. By default the extension will be named by the shape ID replacing # & . with - prefixed with x-, otherwise there is an as: member on the trait that allows renaming.

I believe this could be useful in bridging the gap where custom traits are currently unsupported in OpenAPI conversion, by allowing some opt in.

Adds a JsonSchemaMapperV2 interface that extends the original, and adds an overload of updateSchema that accepts a context object that includes the model being converted.

Example

Some operation:

$version: "2.0"

namespace smithy.example

list StringList {
    member: String
}

@trait
@smithy.openapi#specificationExtension
structure structuredExtension {
    stringMember: String
    integerMember: Integer
    listMember: StringList
}

@trait
@smithy.openapi#specificationExtension(as: "x-metadata")
string stringExtension

@stringExtension("hello world")
structure SpecificationExtensionsOperationInput {
    name: String
    language: String
}

@structuredExtension(
    stringMember: "first field"
    integerMember: 17
    listMember: ["item1", "item2", "item3"]
)
@http(method: "PUT", uri: "/")
operation SpecificationExtensionsOperation {
    input: SpecificationExtensionsOperationInput
}

@aws.protocols#restJson1
service SpecificationExtensions {
    operations: [SpecificationExtensionsOperation]
}

Becomes:

{
    "openapi": "3.0.2",
    "info": {
        "title": "SpecificationExtensions",
        "version": ""
    },
    "paths": {
        "/": {
            "put": {
                "operationId": "SpecificationExtensionsOperation",
                "requestBody": {
                    "content": {
                        "application/json": {
                            "schema": {
                                "$ref": "#/components/schemas/SpecificationExtensionsOperationRequestContent"
                            }
                        }
                    }
                },
                "responses": {
                    "200": {
                        "description": "SpecificationExtensionsOperation 200 response"
                    }
                },
                "x-smithy-example-structuredExtension": {
                    "stringMember": "first field",
                    "integerMember": 17,
                    "listMember": [
                        "item1",
                        "item2",
                        "item3"
                    ]
                }
            }
        }
    },
    "components": {
        "schemas": {
            "SpecificationExtensionsOperationRequestContent": {
                "type": "object",
                "properties": {
                    "name": {
                        "type": "string"
                    },
                    "language": {
                        "type": "string"
                    }
                },
               "x-metadata": "hello world"
            }
        }
    }
}

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[syall]

Hi @Xtansia, thanks for the contribution! This PR adds a lot of new functionality to OpenAPI conversions which is powerful and exciting.

Overall feedback for the changes besides the comments:

• For new files, use the following copyright header:

  /*
   * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
   * SPDX-License-Identifier: Apache-2.0
   */
  

• Some classes / methods are missing updated javadoc comments.
• The tests should be expanded to cover:
  • Custom extension traits for all applicable shape types, or constrained via selector the shape types used to define custom extension traits.
  • Applying these custom extension traits to different parts of the smithy model, e.g. operations, input / output, members of structures and unions, etc.
• This feature and trait should be renamed to "Specification Extensions" rather than "Vendor Extensions" to match the official OpenAPI specification.

The trait also needs a dedicated section in the "Converting Smithy to OpenAPI" guide at docs/source-2.0/guides/converting-to-openapi.rst similar to other trait definitions (arbitrary example: httpApiKeyAuth). This would include:

• How to use it in a smithy model,
• Where to get the trait (the new smithy-openapi-traits package!),
• The expected OpenAPI conversion (similar to the @examples trait conversion section), and
• What subset of OpenAPI conversions are supported by this trait compared to the OpenAPI Specification (behavior captured in the expanded tests).

For unsupported features, I believe the jsonAdd configuration setting documented in the "Converting Smithy to OpenAPI" guide should cover most cases.

[Xtansia]

@syall Thank you for the super thorough feedback, I'll get to work addressing those 👍

[Xtansia]

@syall I've had a first go at fixing all the changes you requested and drafting some documentation, would get your thoughts on anything that needs further improving.

[syall]

Hi @Xtansia, all of my comments are on the documentation section added.

One item I missed from last review is that SpecificationExtensionsMapper is missing a corresponding SpecificationExtensionsMapperTest. Besides this, the code changes look good!

[Xtansia]

Hi @Xtansia, all of my comments are on the documentation section added.

One item I missed from last review is that SpecificationExtensionsMapper is missing a corresponding SpecificationExtensionsMapperTest. Besides this, the code changes look good!

@syall Is there something specific you'd want to be covered by a SpecificationExtensionsMapperTest or just moving the current test I'd added out of OpenApiConverterTest?

[syall]

@Xtansia Good question. I think moving out the test to SpecificationExtensionsMapperTest makes sense.

The specification-extensions.smithy test model could also be split into separate "unit test" models that cover all of the specification extension smithy types at the different OpenAPI specification extension locations documented:

• Service shape
• Operation shape
• Simple & Aggregate shapes

[Xtansia]

@syall I've refactored those tests and added cases covering all trait shapes on services, operation, structure and string (as an "inlined" schema). Do you think that's sufficient or would you like coverage of applying the extensions to all simple & aggregate types?

[syall]

@Xtansia The current tests are sufficient, I believe that the Simple & Aggregate shapes fall under the same testing category for "inlined" schema.

The JsonSchemaMapper::updateSchema breaking change has too much risk, looking into alternatives.

[Xtansia]

@syall Other possibility I can think of for not breaking current implementations of JsonSchemaMapper is:

Add overload with default implementation to interface:

public interface JsonSchemaMapper {
    default byte getOrder() { return 0; }

    default Schema.Builder updateSchema(Shape shape, Schema.Builder schemaBuilder, JsonSchemaConfig config) { return schemaBuilder; }

    default Schema.Builder updateSchema(Shape shape, Schema.Builder schemaBuilder, JsonSchemaConfig config, Model model) { return schemaBuilder; }
}

And change JsonSchemaShapeVisitor::buildSchema to:

    private Schema buildSchema(Shape shape, Schema.Builder builder) {
        JsonSchemaConfig config = converter.getConfig();
        
        for (JsonSchemaMapper mapper : mappers) {
            builder = mapper.updateSchema(shape, builder, config);
            builder = mapper.updateSchema(shape, builder, config, model);
        }

        return builder.build();
    }

Though it's a tad gross in duplication, and I'm not sure if adding a default implementation to the original method def is class compatible with code compiled with the previous interface version. Alternatively could do similar thing but with a new interface (ie. JsonSchemaMapperWithContext) that extends old JsonSchemaMapper and then do instanceof differentiation in buildSchema

[Xtansia]

@syall Have pushed up an a version of what I suggested, also changed to passing in a "context" object to the new interface to minimise breaking changes in future.

[Xtansia]

@syall Have you been able to review my alternative approach?

[syall]

Changed the previous approach to use an overloaded JsonSchemaMapper::updateSchema() that uses the new JsonSchemaMapperContext.

JsonSchemaMapperContext is a class that contains all the info needed to update schemas, and is additive by adding fields to the class (e.g. in this PR Model). This avoids adding overloaded methods with parameters.

[syall]

Changed JsonSchemaMapper tests to class implementation rather than functional interface.

[syall]

There is a breaking change in JsonSchemaMapper from a functional interface to a normal interface.

Although this is a breaking change, I think this is a reasonable compromise to enable this feature.