Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.model.shapes.StructureShape
import software.amazon.smithy.model.traits.DeprecatedTrait
import software.amazon.smithy.model.transform.ModelTransformer
import software.amazon.smithy.rulesengine.traits.EndpointTestCase
import software.amazon.smithy.rulesengine.traits.EndpointTestOperationInput
Expand Down Expand Up @@ -60,6 +61,14 @@ class S3Decorator : ClientCodegenDecorator {
ShapeId.from("com.amazonaws.s3#ListDirectoryBucketsOutput"),
)

// GetBucketLocation is deprecated because AWS recommends using HeadBucket instead
// to determine a bucket's region
private val deprecatedOperations =
mapOf(
ShapeId.from("com.amazonaws.s3#GetBucketLocation") to
"Use HeadBucket operation instead to determine the bucket's region. For more information, see https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadBucket.html",
)

override fun protocols(
serviceId: ShapeId,
currentProtocols: ProtocolMap<OperationGenerator, ClientCodegenContext>,
Expand All @@ -81,6 +90,10 @@ class S3Decorator : ClientCodegenDecorator {
shape.letIf(isInInvalidXmlRootAllowList(shape)) {
logger.info("Adding AllowInvalidXmlRoot trait to $it")
(it as StructureShape).toBuilder().addTrait(AllowInvalidXmlRoot()).build()
}.letIf(isDeprecatedOperation(shape)) {
logger.info("Adding DeprecatedTrait to $it")
val message = deprecatedOperations[shape.id]!!
(it as OperationShape).toBuilder().addTrait(createDeprecatedTrait(message)).build()
}
}
// the model has the bucket in the path
Expand Down Expand Up @@ -182,6 +195,22 @@ class S3Decorator : ClientCodegenDecorator {
private fun isInInvalidXmlRootAllowList(shape: Shape): Boolean {
return shape.isStructureShape && invalidXmlRootAllowList.contains(shape.id)
}

/**
* Checks if the given shape is an operation that should be marked as deprecated.
*/
private fun isDeprecatedOperation(shape: Shape): Boolean {
return shape.isOperationShape && deprecatedOperations.containsKey(shape.id)
}

/**
* Creates a DeprecatedTrait with the specified deprecation message.
*/
private fun createDeprecatedTrait(message: String): DeprecatedTrait {
return DeprecatedTrait.builder()
.message(message)
.build()
}
}

class FilterEndpointTests(
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

As a general tip, if these are only format changes, I'd keep the one in main and not include it in the PR (as long as precommit hook is ok). It'll be hard to distinguish between the real changes vs. format changes.

Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ import software.amazon.smithy.rustsdk.InlineAwsDependency
import kotlin.streams.asSequence

/**
* Enforces that Expires fields have the DateTime type (since in the future the model will change to model them as String),
* and add an ExpiresString field to maintain the raw string value sent.
* Enforces that Expires fields have the DateTime type (since in the future the model will change to
* model them as String), and add an ExpiresString field to maintain the raw string value sent.
*/
class S3ExpiresDecorator : ClientCodegenDecorator {
override val name: String = "S3ExpiresDecorator"
Expand All @@ -55,42 +55,55 @@ class S3ExpiresDecorator : ClientCodegenDecorator {
.asSequence()
.mapNotNull { shape ->
shape.members()
.singleOrNull { member -> member.memberName.equals(expires, ignoreCase = true) }
.singleOrNull { member ->
member.memberName.equals(expires, ignoreCase = true)
}
?.target
}
.associateWith { ShapeType.TIMESTAMP }
var transformedModel = transformer.changeShapeType(model, expiresShapeTimestampMap)

// Add an `ExpiresString` string shape to the model
val expiresStringShape = StringShape.builder().id("aws.sdk.rust.s3.synthetic#$expiresString").build()
val expiresStringShape =
StringShape.builder().id("aws.sdk.rust.s3.synthetic#$expiresString").build()
transformedModel = transformedModel.toBuilder().addShape(expiresStringShape).build()

// For output shapes only, deprecate `Expires` and add a synthetic member that targets `ExpiresString`
// For output shapes only, deprecate `Expires` and add a synthetic member that targets
// `ExpiresString`
transformedModel =
transformer.mapShapes(transformedModel) { shape ->
if (shape.hasTrait<OutputTrait>() && shape.memberNames.any { it.equals(expires, ignoreCase = true) }) {
if (shape.hasTrait<OutputTrait>() &&
shape.memberNames.any { it.equals(expires, ignoreCase = true) }
) {
val builder = (shape as StructureShape).toBuilder()

// Deprecate `Expires`
val expiresMember = shape.members().single { it.memberName.equals(expires, ignoreCase = true) }
val expiresMember =
shape.members().single {
it.memberName.equals(expires, ignoreCase = true)
}

builder.removeMember(expiresMember.memberName)
val deprecatedTrait =
DeprecatedTrait.builder()
.message("Please use `expires_string` which contains the raw, unparsed value of this field.")
.message(
"Please use `expires_string` which contains the raw, unparsed value of this field.",
)
.build()

builder.addMember(
expiresMember.toBuilder()
.addTrait(deprecatedTrait)
.build(),
expiresMember.toBuilder().addTrait(deprecatedTrait).build(),
)

// Add a synthetic member targeting `ExpiresString`
val expiresStringMember = MemberShape.builder()
expiresStringMember.target(expiresStringShape.id)
expiresStringMember.id(expiresMember.id.toString() + "String") // i.e. com.amazonaws.s3.<MEMBER_NAME>$ExpiresString
expiresStringMember.addTrait(HttpHeaderTrait(expiresString)) // Add HttpHeaderTrait to ensure the field is deserialized
expiresStringMember.id(
expiresMember.id.toString() + "String",
) // i.e. com.amazonaws.s3.<MEMBER_NAME>$ExpiresString
expiresStringMember.addTrait(
HttpHeaderTrait(expiresString),
) // Add HttpHeaderTrait to ensure the field is deserialized
expiresMember.getTrait<DocumentationTrait>()?.let {
expiresStringMember.addTrait(it) // Copy documentation from `Expires`
}
Expand Down Expand Up @@ -132,8 +145,11 @@ class ParseExpiresFieldsCustomization(
section.registerInterceptor(codegenContext.runtimeConfig, this) {
val interceptor =
RuntimeType.forInlineDependency(
InlineAwsDependency.forRustFile("s3_expires_interceptor"),
).resolve("S3ExpiresInterceptor")
InlineAwsDependency.forRustFile(
"s3_expires_interceptor",
),
)
.resolve("S3ExpiresInterceptor")
rustTemplate(
"""
#{S3ExpiresInterceptor}
Expand All @@ -142,7 +158,6 @@ class ParseExpiresFieldsCustomization(
)
}
}

else -> {}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.rustsdk.customize.s3

import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.ShapeId
import software.amazon.smithy.model.traits.DeprecatedTrait
import software.amazon.smithy.rust.codegen.client.testutil.testClientRustSettings
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.util.hasTrait
import kotlin.jvm.optionals.getOrNull

internal class S3DecoratorTest {
/**
* Base S3 model for testing. Includes GetBucketLocation, HeadBucket, and CreateBucket operations.
* This minimal model allows us to test deprecation behavior without loading the full S3 model.
*/
private val baseModel =
"""
namespace com.amazonaws.s3

use aws.protocols#restXml
use aws.auth#sigv4
use aws.api#service
use smithy.rules#endpointRuleSet

@restXml
@sigv4(name: "s3")
@service(
sdkId: "S3"
arnNamespace: "s3"
)
@endpointRuleSet({
"version": "1.0",
"rules": [{ "type": "endpoint", "conditions": [], "endpoint": { "url": "https://s3.amazonaws.com" } }],
"parameters": {
"Region": { "required": false, "type": "String", "builtIn": "AWS::Region" }
}
})
service S3 {
version: "2006-03-01",
operations: [GetBucketLocation, HeadBucket, CreateBucket]
}

@http(method: "GET", uri: "/{Bucket}?location")
operation GetBucketLocation {
input: GetBucketLocationRequest
output: GetBucketLocationOutput
}

@http(method: "HEAD", uri: "/{Bucket}")
operation HeadBucket {
input: HeadBucketRequest
}

@http(method: "PUT", uri: "/{Bucket}")
operation CreateBucket {
input: CreateBucketRequest
}

structure GetBucketLocationRequest {
@required
@httpLabel
Bucket: String
}

@output
structure GetBucketLocationOutput {
LocationConstraint: String
}

structure HeadBucketRequest {
@required
@httpLabel
Bucket: String
}

structure CreateBucketRequest {
@required
@httpLabel
Bucket: String
}
""".asSmithyModel()

private val serviceShape = baseModel.expectShape(ShapeId.from("com.amazonaws.s3#S3"), ServiceShape::class.java)
private val settings = testClientRustSettings()

/**
* Helper method to apply the S3Decorator transformation to the base model.
* This simulates what happens during code generation.
*/
private fun transformModel() = S3Decorator().transformModel(serviceShape, baseModel, settings)

@Test
fun `GetBucketLocation operation has DeprecatedTrait`() {
// Apply the S3Decorator transformation
val transformedModel = transformModel()

// Get the GetBucketLocation operation from the transformed model
val getBucketLocationId = ShapeId.from("com.amazonaws.s3#GetBucketLocation")
val operation = transformedModel.expectShape(getBucketLocationId, OperationShape::class.java)

// Assert that the operation has the DeprecatedTrait
assertTrue(operation.hasTrait<DeprecatedTrait>(), "GetBucketLocation should have DeprecatedTrait")

// Get the trait and verify the message content
val trait = operation.expectTrait(DeprecatedTrait::class.java)
val message = trait.message.getOrNull()

// Assert that the message contains "HeadBucket"
assertTrue(
message?.contains("HeadBucket") == true,
"Deprecation message should mention HeadBucket",
)

// Assert that the message contains the AWS documentation URL
assertTrue(
message?.contains("https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadBucket.html") == true,
"Deprecation message should include AWS documentation URL",
)
}

@Test
fun `Other S3 operations do not have DeprecatedTrait`() {
Comment on lines +130 to +131
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also remove this test (as well as relevant operations from the test model), as long as we are testing GetBucketLocation to be annotated for deprecation. Since there are many other S3 operations in the real world, the fact this test passed doesn't necessarily guarantee those operations are without deprecated annotations.

// Apply the S3Decorator transformation
val transformedModel = transformModel()

// Check HeadBucket operation does NOT have DeprecatedTrait
val headBucketId = ShapeId.from("com.amazonaws.s3#HeadBucket")
val headBucket = transformedModel.expectShape(headBucketId, OperationShape::class.java)
assertFalse(
headBucket.hasTrait<DeprecatedTrait>(),
"HeadBucket should not have DeprecatedTrait",
)

// Check CreateBucket operation does NOT have DeprecatedTrait
val createBucketId = ShapeId.from("com.amazonaws.s3#CreateBucket")
val createBucket = transformedModel.expectShape(createBucketId, OperationShape::class.java)
assertFalse(
createBucket.hasTrait<DeprecatedTrait>(),
"CreateBucket should not have DeprecatedTrait",
)
}
}
Loading