Unverified Commit ad520b08 authored by Miles Ziemer's avatar Miles Ziemer Committed by GitHub
Browse files

Remove invalid defaults for some services (#3217)



## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->

Some services have generated types with properties that have a default
value of zero. This can cause invalid requests if the service expects a
value > 0.

## Description
<!--- Describe your changes in detail -->

Adds a customization that removes the default value for services with
this issue.

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
- Added a test for the customization to make sure defaults were removed.
- Generated clients for the impacted services and verified the expected
types had the default value removed.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

Co-authored-by: default avatarRussell Cohen <rcoh@amazon.com>
parent 7d1e3211
Loading
Loading
Loading
Loading
+7 −1
Original line number Diff line number Diff line
@@ -10,3 +10,9 @@
# references = ["smithy-rs#920"]
# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
# author = "rcoh"

[[aws-sdk-rust]]
message = "Make certain types for EMR Serverless optional. Previously, they defaulted to 0, but this created invalid requests."
references = ["smithy-rs#3217"]
meta = { "breaking" = true, "tada" = false, "bug" = true }
author = "milesziemer"
+2 −0
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ import software.amazon.smithy.rust.codegen.client.smithy.customizations.DocsRsMe
import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator
import software.amazon.smithy.rust.codegen.client.smithy.customize.CombinedClientCodegenDecorator
import software.amazon.smithy.rustsdk.customize.DisabledAuthDecorator
import software.amazon.smithy.rustsdk.customize.RemoveDefaultsDecorator
import software.amazon.smithy.rustsdk.customize.apigateway.ApiGatewayDecorator
import software.amazon.smithy.rustsdk.customize.applyDecorators
import software.amazon.smithy.rustsdk.customize.ec2.Ec2Decorator
@@ -53,6 +54,7 @@ val DECORATORS: List<ClientCodegenDecorator> = listOf(
        RecursionDetectionDecorator(),
        InvocationIdDecorator(),
        RetryInformationHeaderDecorator(),
        RemoveDefaultsDecorator(),
    ),

    // Service specific decorators
+58 −0
Original line number Diff line number Diff line
/*
 * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 * SPDX-License-Identifier: Apache-2.0
 */

package software.amazon.smithy.rustsdk.customize

import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.AbstractShapeBuilder
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.model.traits.DefaultTrait
import software.amazon.smithy.model.transform.ModelTransformer
import software.amazon.smithy.rust.codegen.core.util.hasTrait
import software.amazon.smithy.rust.codegen.core.util.letIf
import software.amazon.smithy.utils.ToSmithyBuilder
import java.util.logging.Logger

/**
 * Removes default values from specified root shapes, and any members that target those
 * root shapes.
 */
object RemoveDefaults {
    private val logger: Logger = Logger.getLogger(javaClass.name)

    fun processModel(model: Model, removeDefaultsFrom: Set<ShapeId>): Model {
        val removedRootDefaults: MutableSet<ShapeId> = HashSet()
        val removedRootDefaultsModel = ModelTransformer.create().mapShapes(model) { shape ->
            shape.letIf(shouldRemoveRootDefault(shape, removeDefaultsFrom)) {
                logger.info("Removing default trait from root $shape")
                removedRootDefaults.add(shape.id)
                removeDefault(shape)
            }
        }

        return ModelTransformer.create().mapShapes(removedRootDefaultsModel) { shape ->
            shape.letIf(shouldRemoveMemberDefault(shape, removedRootDefaults)) {
                logger.info("Removing default trait from member $shape")
                removeDefault(shape)
            }
        }
    }

    private fun shouldRemoveRootDefault(shape: Shape, removeDefaultsFrom: Set<ShapeId>): Boolean {
        return shape !is MemberShape && removeDefaultsFrom.contains(shape.id) && shape.hasTrait<DefaultTrait>()
    }

    private fun shouldRemoveMemberDefault(shape: Shape, removeDefaultsFrom: Set<ShapeId>): Boolean {
        return shape is MemberShape && removeDefaultsFrom.contains(shape.target) && shape.hasTrait<DefaultTrait>()
    }

    private fun removeDefault(shape: Shape): Shape {
        return ((shape as ToSmithyBuilder<*>).toBuilder() as AbstractShapeBuilder<*, *>)
            .removeTrait(DefaultTrait.ID)
            .build()
    }
}
+44 −0
Original line number Diff line number Diff line
/*
 * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 * SPDX-License-Identifier: Apache-2.0
 */

package software.amazon.smithy.rustsdk.customize

import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.rust.codegen.client.smithy.ClientRustSettings
import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegenDecorator
import software.amazon.smithy.rust.codegen.core.util.shapeId
import java.util.logging.Logger

/**
 * Removes default values from certain shapes, and any member that targets those shapes,
 * for some services where the default value causes serialization issues, validation
 * issues, or other unexpected behavior.
 */
class RemoveDefaultsDecorator : ClientCodegenDecorator {
    override val name: String = "RemoveDefaults"
    override val order: Byte = 0
    private val logger: Logger = Logger.getLogger(javaClass.name)

    // Service shape id -> Shape id of each root shape to remove the default from.
    // TODO(https://github.com/smithy-lang/smithy-rs/issues/3220): Remove this customization after model updates.
    private val removeDefaults = mapOf(
        "com.amazonaws.emrserverless#AwsToledoWebService".shapeId() to setOf(
            // Service expects this to have a min value > 0
            "com.amazonaws.emrserverless#WorkerCounts".shapeId(),
        ),
    )

    private fun applies(service: ServiceShape) =
        removeDefaults.containsKey(service.id)

    override fun transformModel(service: ServiceShape, model: Model, settings: ClientRustSettings): Model {
        if (!applies(service)) {
            return model
        }
        logger.info("Removing invalid defaults from ${service.id}")
        return RemoveDefaults.processModel(model, removeDefaults[service.id]!!)
    }
}
+41 −0
Original line number Diff line number Diff line
/*
 * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 * SPDX-License-Identifier: Apache-2.0
 */

package software.amazon.smithy.rustsdk.customize

import io.kotest.matchers.shouldBe
import org.junit.jupiter.api.Test
import software.amazon.smithy.model.shapes.IntegerShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.traits.DefaultTrait
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.util.hasTrait
import software.amazon.smithy.rust.codegen.core.util.lookup
import software.amazon.smithy.rust.codegen.core.util.shapeId

internal class RemoveDefaultsTest {
    @Test
    fun `defaults should be removed`() {
        val removeDefaults = setOf(
            "test#Bar".shapeId(),
        )
        val baseModel = """
            namespace test

            structure Foo {
                bar: Bar = 0
            }

            @default(0)
            integer Bar

        """.asSmithyModel(smithyVersion = "2.0")
        val model = RemoveDefaults.processModel(baseModel, removeDefaults)
        val member = model.lookup<MemberShape>("test#Foo\$bar")
        member.hasTrait<DefaultTrait>() shouldBe false
        val root = model.lookup<IntegerShape>("test#Bar")
        root.hasTrait<DefaultTrait>() shouldBe false
    }
}