Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Iddodo opened this issue Nov 19, 2024 · 3 comments
Labels
feature-request A feature should be added or improved.

Comments

@Iddodo
Copy link

Iddodo commented Nov 19, 2024

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"
                                }
                            }
                        }
                    },
                    // ...
@Iddodo Iddodo changed the title Retain operation input/output Smithy symbol (or allow customization) upon conversion to OpenAPI Retain operation input/output structure symbol (or allow customization) upon conversion to OpenAPI Nov 19, 2024
@mtdowling
Copy link
Member

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
Copy link
Author

Iddodo commented Nov 19, 2024

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
Copy link
Author

Iddodo commented Nov 21, 2024

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()
    }
}

@sugmanue sugmanue added the feature-request A feature should be added or improved. label Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

3 participants