smithy-lang / smithy

Smithy is a protocol-agnostic interface definition language and set of tools for generating clients, servers, and documentation for any programming language.
https://smithy.io
Apache License 2.0
1.83k stars 219 forks source link

Retain operation input/output structure symbol (or allow customization) upon conversion to OpenAPI #2469

Open Iddodo opened 1 week ago

Iddodo commented 1 week ago

This relates to a previous issue where we asked about the possibility of inlining map shapes. This has been quickly addressed, which we are very thankful for.

Is title currently supported? For example, Consider the following operation shape:

/// Do something given its name
@http(
    method: "POST"
    uri: "/do-something"
)
operation DoSomething {
    input := {
        @required
        nameOfSomething: String
    }
    output := {
        comment: String
    }
}

More or less, this would currently get converted to the following JSON Schema:

{
            "post": {
                "description": "Do something given its name",
                "operationId": "DoSomething",
                "requestBody": {
                    "content": {
                        "application/json": {
                            "schema": {
                                "$ref": "#/components/schemas/DoSomethingRequestContent"
                            }
                        }
                    },
                    "required": true
                },
                "responses": {
                    "200": {
                        "description": "DoSomething 200 response",
                        "content": {
                            "application/json": {
                                "schema": {
                                    "$ref": "#/components/schemas/DoSomethingResponseContent"
                                }
                            }
                        }
                    },
                    // ...

This is problematic because our build process cares about the schema name of an operation's input and output content (for code generation purposes).

Is there a way to change these schema names to follow the original Smithy input and output structure names? (I used the input :=, output := syntax for brevity, this also refers to scenarios where the structure is explicitly defined).

Meaning, the desired conversion would look something like this:

            "post": {
                "description": "Do something given its name",
                "operationId": "DoSomething",
                "requestBody": {
                    "content": {
                        "application/json": {
                            "schema": {
                                "$ref": "#/components/schemas/DoSomethingInput"
                            }
                        }
                    },
                    "required": true
                },
                "responses": {
                    "200": {
                        "description": "DoSomething 200 response",
                        "content": {
                            "application/json": {
                                "schema": {
                                    "$ref": "#/components/schemas/DoSomethingOutput"
                                }
                            }
                        }
                    },
                    // ...
mtdowling commented 1 week ago

I'm not sure that it's possible or a good idea to change this.

The Smithy to OpenAPI conversion has to create a "synthetic" input and output shape that contains just the members sent in the payload. That's because OpenAPI requires a separation between what goes in headers, query strings, path labels, and the body. I guess this makes sense for OpenAPI because its job is to describe HTTP messages, but in Smithy, we have no such imposed separation between inputs and outputs. If we converted Smithy inputs and outputs to OpenAPI and reused the same name from the Smithy model, then something like PutFooInput in Smithy would have more members in it than PutFooInput in OpenAPI due to members having to be stripped out and moved to other parts of the OpenAPI.

Iddodo commented 1 week ago

I'm not sure if I follow. In Smithy, we have defined distinct structures for inputs and outputs, along with their intended symbols. We just want to be able to use them directly as the referenced JSON schema, and not have to deal with unintended schema symbols floating around.

This doesn't have to be a sweeping change that alters the existing behavior, it can be an opt-in config option or a trait (opt-in config is probably easier).

This is also somewhat of a blocker for us on the way to migrating to Smithy.

Just to be clear, nothing is supposed to change on Smithy side. All we want is that input/output schemas retain their original names, or alternatively some method to relatively easy customize the naming convention of said "synthetic shape" to allow aforementioned.

They are already structurally identical to the original structures.

Iddodo commented 1 week ago

For now we might try to get by with the following workaround, but I think something like this ought to be customizable, seems a bit arbitrary as a constraint


import com.google.auto.service.AutoService
import software.amazon.smithy.openapi.fromsmithy.Smithy2OpenApiExtension
import software.amazon.smithy.model.node.ObjectNode
import software.amazon.smithy.model.node.StringNode
import software.amazon.smithy.openapi.fromsmithy.OpenApiMapper
import software.amazon.smithy.openapi.fromsmithy.Context
import software.amazon.smithy.openapi.model.OpenApi
import kotlin.jvm.optionals.getOrNull

@AutoService(Smithy2OpenApiExtension::class)
class CustomOpenApiExtension : Smithy2OpenApiExtension {
    override fun getOpenApiMappers() = defaultMappers

    companion object {
        private val defaultMappers: MutableList<OpenApiMapper> = mutableListOf(ContentStructureMapper())
    }
}

class ContentStructureMapper : OpenApiMapper {
    override fun updateNode(context: Context<*>, openapi: OpenApi, node: ObjectNode): ObjectNode {
        val schemas = node.getMember("components").getOrNull()
            ?.expectObjectNode()
            ?.getMember("schemas")?.getOrNull()
            ?.expectObjectNode()
            ?: return node

        // First collect the renames we'll do
        val renames = schemas.members.entries.mapNotNull { (key, _) ->
            val strKey = key.value
            when {
                strKey.endsWith("ErrorResponseContent") -> strKey to strKey.removeSuffix("ResponseContent")
                strKey.endsWith("RequestContent") -> strKey to strKey.removeSuffix("Content")
                strKey.endsWith("ResponseContent") -> strKey to strKey.removeSuffix("Content")
                else -> null
            }
        }.toMap()

        // Update the entire document
        val updatedNode = updateRefs(node, renames)

        // Finally update the schema names
        val updatedSchemas = schemas.members.entries.associate { (key, value) ->
            val newKey = renames[key.value]?.let { StringNode.from(it) } ?: key
            newKey to value
        }

        return updatedNode.toBuilder()
            .withMember("components", ObjectNode.builder()
                .withMember("schemas", ObjectNode.builder()
                    .also { builder ->
                        updatedSchemas.forEach { (key, value) ->
                            builder.withMember(key, value)
                        }
                    }
                    .build())
                .build())
            .build()
    }

    // Function to recursively update refs in a node
    private fun updateRefs(node: ObjectNode, renames: Map<String, String>): ObjectNode {
        val ref = node.getMember("\$ref").getOrNull()?.expectStringNode()?.value
        if (ref != null) {
            val schemaName = ref.removePrefix("#/components/schemas/")
            if (schemaName in renames) {
                return node.toBuilder()
                    .withMember("\$ref", "#/components/schemas/${renames[schemaName]}")
                    .build()
            }
        }

        // Recursively process all object members
        return node.toBuilder().also { builder ->
            node.members.forEach { (key, value) ->
                val updated = when {
                    value is ObjectNode -> updateRefs(value, renames)
                    else -> value
                }
                builder.withMember(key, updated)
            }
        }.build()
    }
}