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

fix: ignore __type when deserializing union for AWS Json protocols #1054

Merged
merged 5 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changes/ac672b00-f8bb-4235-8db4-aad0ae2f157d.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "ac672b00-f8bb-4235-8db4-aad0ae2f157d",
"type": "bugfix",
"description": "ignore __type when deserializing union for AWS Json protocols",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also applies for RestJSON, so I wouldn't say just "AWS Json protocols"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider wrapping __type in backticks: __type

"issues": [
"awslabs/aws-sdk-kotlin#1044"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@ package aws.sdk.kotlin.codegen.protocols
import aws.sdk.kotlin.codegen.protocols.core.AwsHttpBindingProtocolGenerator
import aws.sdk.kotlin.codegen.protocols.json.AwsJsonHttpBindingResolver
import aws.sdk.kotlin.codegen.protocols.json.AwsJsonProtocolMiddleware
import aws.sdk.kotlin.codegen.protocols.json.AwsJsonProtocolParserGenerator
import aws.sdk.kotlin.codegen.protocols.json.JsonHttpBindingProtocolGenerator
import software.amazon.smithy.aws.traits.protocols.AwsJson1_0Trait
import software.amazon.smithy.kotlin.codegen.rendering.protocol.HttpBindingResolver
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolGenerator
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolMiddleware
import software.amazon.smithy.kotlin.codegen.rendering.serde.StructuredDataParserGenerator
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.*
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.ShapeId

/**
* Handles generating the aws.protocols#awsJson1_0 protocol for services.
Expand All @@ -36,4 +39,7 @@ class AwsJson1_0 : JsonHttpBindingProtocolGenerator() {

override fun getProtocolHttpBindingResolver(model: Model, serviceShape: ServiceShape): HttpBindingResolver =
AwsJsonHttpBindingResolver(model, serviceShape, "application/x-amz-json-1.0")

override fun structuredDataParser(ctx: ProtocolGenerator.GenerationContext): StructuredDataParserGenerator =
AwsJsonProtocolParserGenerator(this, supportsJsonNameTrait)
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ package aws.sdk.kotlin.codegen.protocols
import aws.sdk.kotlin.codegen.protocols.core.AwsHttpBindingProtocolGenerator
import aws.sdk.kotlin.codegen.protocols.json.AwsJsonHttpBindingResolver
import aws.sdk.kotlin.codegen.protocols.json.AwsJsonProtocolMiddleware
import aws.sdk.kotlin.codegen.protocols.json.AwsJsonProtocolParserGenerator
import aws.sdk.kotlin.codegen.protocols.json.JsonHttpBindingProtocolGenerator
import software.amazon.smithy.aws.traits.protocols.AwsJson1_1Trait
import software.amazon.smithy.kotlin.codegen.rendering.protocol.HttpBindingResolver
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolGenerator
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolMiddleware
import software.amazon.smithy.kotlin.codegen.rendering.serde.StructuredDataParserGenerator
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.ShapeId
Expand All @@ -38,4 +40,7 @@ class AwsJson1_1 : JsonHttpBindingProtocolGenerator() {

override fun getProtocolHttpBindingResolver(model: Model, serviceShape: ServiceShape): HttpBindingResolver =
AwsJsonHttpBindingResolver(model, serviceShape, "application/x-amz-json-1.1")

override fun structuredDataParser(ctx: ProtocolGenerator.GenerationContext): StructuredDataParserGenerator =
AwsJsonProtocolParserGenerator(this, supportsJsonNameTrait)
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
package aws.sdk.kotlin.codegen.protocols

import aws.sdk.kotlin.codegen.protocols.core.AwsHttpBindingProtocolGenerator
import aws.sdk.kotlin.codegen.protocols.json.AwsJsonProtocolParserGenerator
import aws.sdk.kotlin.codegen.protocols.json.JsonHttpBindingProtocolGenerator
import software.amazon.smithy.aws.traits.protocols.RestJson1Trait
import software.amazon.smithy.kotlin.codegen.core.KotlinWriter
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes
import software.amazon.smithy.kotlin.codegen.core.defaultName
import software.amazon.smithy.kotlin.codegen.core.withBlock
import software.amazon.smithy.kotlin.codegen.rendering.protocol.*
import software.amazon.smithy.kotlin.codegen.rendering.serde.StructuredDataParserGenerator
import software.amazon.smithy.model.Model
import software.amazon.smithy.model.knowledge.HttpBinding
import software.amazon.smithy.model.shapes.OperationShape
Expand Down Expand Up @@ -62,4 +64,7 @@ class RestJson1 : JsonHttpBindingProtocolGenerator() {
}
}
}

override fun structuredDataParser(ctx: ProtocolGenerator.GenerationContext): StructuredDataParserGenerator =
AwsJsonProtocolParserGenerator(this, supportsJsonNameTrait)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package aws.sdk.kotlin.codegen.protocols.json

import software.amazon.smithy.kotlin.codegen.core.KotlinWriter
import software.amazon.smithy.kotlin.codegen.rendering.protocol.ProtocolGenerator
import software.amazon.smithy.kotlin.codegen.rendering.protocol.toRenderingContext
import software.amazon.smithy.kotlin.codegen.rendering.serde.JsonParserGenerator
import software.amazon.smithy.kotlin.codegen.rendering.serde.JsonSerdeDescriptorGenerator
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.Shape

class AwsJsonProtocolParserGenerator(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix Missing documentation.

Should we rename this (and AwsJsonProtocolSerdeDescriptorGenerator) since these aren't specific to the "aws json" protocols? It's a bit confusing but I also don't know that I have a better suggestion at the moment. At the very least we should document that this isn't specific to awsJson protocols.

private val protocolGenerator: ProtocolGenerator,
private val supportsJsonNameTrait: Boolean = true,
) : JsonParserGenerator(protocolGenerator, supportsJsonNameTrait) {

override fun descriptorGenerator(
ctx: ProtocolGenerator.GenerationContext,
shape: Shape,
members: List<MemberShape>,
writer: KotlinWriter,
): JsonSerdeDescriptorGenerator = AwsJsonProtocolSerdeDescriptorGenerator(ctx.toRenderingContext(protocolGenerator, shape, writer), members, supportsJsonNameTrait)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package aws.sdk.kotlin.codegen.protocols.json

import software.amazon.smithy.kotlin.codegen.core.RenderingContext
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes
import software.amazon.smithy.kotlin.codegen.rendering.serde.JsonSerdeDescriptorGenerator
import software.amazon.smithy.kotlin.codegen.rendering.serde.SdkFieldDescriptorTrait
import software.amazon.smithy.kotlin.codegen.rendering.serde.add
import software.amazon.smithy.kotlin.codegen.utils.dq
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.Shape

class AwsJsonProtocolSerdeDescriptorGenerator(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing docs

ctx: RenderingContext<Shape>,
memberShapes: List<MemberShape>? = null,
supportsJsonNameTrait: Boolean = true,
) : JsonSerdeDescriptorGenerator(ctx, memberShapes, supportsJsonNameTrait) {

/**
* Adds a trait to ignore `__type` in union shapes for AWS specific JSON protocols
* Sometimes the unnecessary trait `__type` is added and needs to be ignored
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: "Sometimes the unnecessary field `__type` is added and needs to be ignored"

*
* NOTE: Will be ignored unless it's in the model
*
* Source: https://github.com/smithy-lang/smithy/pull/1945
*/
override fun getObjectDescriptorTraits(): List<SdkFieldDescriptorTrait> {
val traitList = super.getObjectDescriptorTraits().toMutableList()
if (ctx.shape?.isUnionShape == true) traitList.add(RuntimeTypes.Serde.IgnoreKey, "__type".dq(), "false")
return traitList
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package aws.sdk.kotlin.codegen.protocols.json

import software.amazon.smithy.kotlin.codegen.test.*
import software.amazon.smithy.model.shapes.ShapeId
import kotlin.test.Test

class AwsJsonProtocolSerdeDescriptorGeneratorTest {
@Test
fun itHandlesUnionsAndAddsIgnoreKeysTrait() {
val model = """
@http(method: "POST", uri: "/foo")
operation Foo {
input: FooRequest
}

structure FooRequest {
strVal: String,
intVal: Integer
}

union Bar {
x: String,
y: String,
}
""".prependNamespaceAndService(operations = listOf("Foo")).toSmithyModel()

val testCtx = model.newTestContext()
val writer = testCtx.newWriter()
val shape = model.expectShape(ShapeId.from("com.test#Bar"))
val renderingCtx = testCtx.toRenderingContext(writer, shape)

AwsJsonProtocolSerdeDescriptorGenerator(renderingCtx).render()

val expectedDescriptors = """
val X_DESCRIPTOR = SdkFieldDescriptor(SerialKind.String, JsonSerialName("x"))
val Y_DESCRIPTOR = SdkFieldDescriptor(SerialKind.String, JsonSerialName("y"))
val OBJ_DESCRIPTOR = SdkObjectDescriptor.build {
trait(IgnoreKey("__type", false))
field(X_DESCRIPTOR)
field(Y_DESCRIPTOR)
}
""".formatForTest("")

val contents = writer.toString()
contents.shouldContainOnlyOnceWithDiff(expectedDescriptors)
}
}
Loading