Skip to content

Commit 3c259ef

Browse files
0marperez0marperez
committed
PR feeback checkpoint
1 parent d398801 commit 3c259ef

File tree

8 files changed

+17
-21
lines changed

8 files changed

+17
-21
lines changed

‎aws-runtime/aws-http/common/src/aws/sdk/kotlin/runtime/http/interceptors/S3CompositeChecksumsInterceptor.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public class S3CompositeChecksumsInterceptor : HttpInterceptor {
3232
internal fun HeadersBuilder.removeCompositeChecksums(logger: Logger): Unit =
3333
names().forEach { header ->
3434
if (header.startsWith(CHECKSUM_HEADER_PREFIX, ignoreCase = true) && get(header)!!.isCompositeChecksum()) {
35-
logger.warn { "S3 returned a composite checksum, composite checksums are not supported, removing checksum" }
35+
logger.warn { "S3 returned a composite checksum ($header), composite checksums are not supported, removing checksum" }
3636
remove(header)
3737
}
3838
}

‎codegen/aws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/PresignerGenerator.kt‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ import software.amazon.smithy.codegen.core.Symbol
1212
import software.amazon.smithy.kotlin.codegen.core.*
1313
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Core.Hashing.isSupportedForFlexibleChecksums
1414
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Core.Hashing.toHashFunctionOrThrow
15-
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Core.IllegalStateException
1615
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Core.Text.Encoding.encodeBase64String
17-
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Core.Text.lowercase
1816
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Core.Utils.runBlocking
1917
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Http.HttpBody
2018
import software.amazon.smithy.kotlin.codegen.core.RuntimeTypes.Http.readAll
@@ -278,10 +276,10 @@ class PresignerGenerator : KotlinIntegration {
278276
}
279277

280278
checksumAlgorithmMember(op, ctx)?.let { checksumAlgorithmMember ->
281-
withBlock("input.#L?.value?.let { checksumAlgorithmString ->", "}", checksumAlgorithmMember) {
279+
withBlock("input.#L?.value?.let { checksumAlgorithmName ->", "}", checksumAlgorithmMember) {
282280
withBlock("when (unsignedRequest.body) {", "}") {
283281
withBlock("is #1T.Bytes, is #1T.Empty -> {", "}", HttpBody) {
284-
write("val checksumAlgorithm = checksumAlgorithmString.#T()", toHashFunctionOrThrow)
282+
write("val checksumAlgorithm = checksumAlgorithmName.#T()", toHashFunctionOrThrow)
285283
withInlineBlock(
286284
"if (checksumAlgorithm.#T) {",
287285
"}",
@@ -293,9 +291,8 @@ class PresignerGenerator : KotlinIntegration {
293291
}
294292
}
295293
write(
296-
"checksum = #S.#T() to checksumAlgorithm.digest().#T()",
297-
"x-amz-checksum-\${checksumAlgorithmString}",
298-
lowercase,
294+
"checksum = #S.lowercase() to checksumAlgorithm.digest().#T()",
295+
"x-amz-checksum-\${checksumAlgorithmName}",
299296
encodeBase64String,
300297
)
301298
}
@@ -306,12 +303,13 @@ class PresignerGenerator : KotlinIntegration {
306303
"#T.#T<Presigner> { #S }",
307304
coroutineContext,
308305
warn,
309-
"The requested checksum algorithm is not supported for pre-signed URL checksums, sending request without checksum.",
306+
"The requested checksum algorithm (\${checksumAlgorithmName}) is not supported for pre-signed URL checksums, sending request without checksum. " +
307+
"Supported checksums for pre-signed URLs include the following: ",
310308
)
311309
}
312310
}
313311
}
314-
write("else -> throw #T(#S)", IllegalStateException, "HTTP body type unsupported for pre-signed URL checksums.")
312+
write("else -> throw IllegalStateException(#S)", "HTTP body type unsupported for pre-signed URL checksums.")
315313
}
316314
}
317315
}
@@ -340,7 +338,7 @@ class PresignerGenerator : KotlinIntegration {
340338
private fun normalizeUriPath(service: ServiceShape) = service.sdkId != "S3"
341339

342340
/**
343-
* Gets the checksum algorithm member if a user can configure request checksums otherwise null
341+
* Gets the checksum algorithm member if configured on a request, otherwise null
344342
*/
345343
private fun checksumAlgorithmMember(
346344
operationShape: OperationShape,

‎codegen/aws-sdk-codegen/src/main/kotlin/aws/sdk/kotlin/codegen/customization/flexiblechecksums/FlexibleChecksumsRequest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class FlexibleChecksumsRequest : KotlinIntegration {
6969
}
7070

7171
private val flexibleChecksumsRequestMiddleware = object : ProtocolMiddleware {
72-
override val name: String = "FlexibleChecksumsRequest"
72+
override val name: String = "flexibleChecksumsRequestMiddleware"
7373

7474
override fun isEnabledFor(ctx: ProtocolGenerator.GenerationContext, op: OperationShape): Boolean {
7575
val httpChecksumTrait = op.getTrait<HttpChecksumTrait>()

‎services/s3/common/src/aws/sdk/kotlin/services/s3/express/S3ExpressDisableChecksumInterceptor.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ private const val S3_EXPRESS_ENDPOINT_PROPERTY_KEY = "backend"
1919
private const val S3_EXPRESS_ENDPOINT_PROPERTY_VALUE = "S3Express"
2020

2121
/**
22-
* Disables checksums for s3:UploadPart requests that use S3 express.
22+
* Disables checksums for s3:UploadPart requests that use S3 Express.
2323
*/
2424
internal class S3ExpressDisableChecksumInterceptor(
2525
private val userConfiguredChecksum: Boolean,

‎services/s3/e2eTest/src/PaginatorTest.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,8 @@ class PaginatorTest {
4141
S3TestUtils.deleteBucketAndAllContents(client, testBucket)
4242
}
4343

44-
// FIXME: Enable test when motorcade is ready
44+
// FIXME: Enable test
4545
// Seeing: S3Exception: Checksum Type mismatch occurred, expected checksum Type: null, actual checksum Type: crc32
46-
// Cause: "Post-motorcade SDK is expected not to work against Pre-Motorcade S3"
4746
// ListParts has a strange pagination termination condition via [IsTerminated]. Verify it actually works correctly.
4847
@Ignore
4948
@Test

‎services/s3/e2eTest/src/S3ChecksumTest.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class S3ChecksumTest {
3131
@BeforeAll
3232
private fun setUp(): Unit = runBlocking {
3333
val accountId = getAccountId()
34-
// FIXME: Use randomly generated bucket instead of hardcoded one when motorcade is ready
34+
// FIXME: Use randomly generated bucket instead of hardcoded one
3535
getBucketByName(client, testBucket, "us-west-2", accountId)
3636
}
3737

@@ -169,6 +169,7 @@ class S3ChecksumTest {
169169
val unsignedPutRequest = PutObjectRequest {
170170
bucket = testBucket
171171
key = testKey()
172+
checksumCrc32 = "`"
172173
}
173174
val presignedPutRequest = client.presignPutObject(unsignedPutRequest, 60.seconds)
174175
val contents = "presign-test"

‎services/s3/e2eTest/src/S3IntegrationTest.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,8 @@ class S3BucketOpsIntegrationTest {
188188
}
189189
}
190190

191-
// FIXME: Enable test when motorcade is ready
191+
// FIXME: Enable test
192192
// Seeing: S3Exception: Checksum Type mismatch occurred, expected checksum Type: null, actual checksum Type: crc32
193-
// Cause: "Post-motorcade SDK is expected not to work against Pre-Motorcade S3"
194193
@Ignore
195194
@Test
196195
fun testMultipartUpload(): Unit = runBlocking {

‎services/s3/e2eTest/src/S3TestUtils.kt‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,8 @@ object S3TestUtils {
3434
const val DEFAULT_REGION = "us-west-2"
3535

3636
// The E2E test account only has permission to operate on buckets with the prefix "s3-test-bucket-"
37-
// Motorcade allow-listed bucket: "s3-test-bucket-ci-motorcade".
38-
// Non-checksum E2E tests will use it and delete it if TEST_BUCKET_PREFIX="s3-test-bucket-" via `deleteBucketAndAllContents`
39-
// TODO: Change back to "s3-test-bucket-" after motorcade is released.
37+
// Non-checksum E2E tests will use and delete hardcoded bucket required for checksum tests if TEST_BUCKET_PREFIX="s3-test-bucket-" via `deleteBucketAndAllContents`
38+
// TODO: Change back to "s3-test-bucket-"
4039
private const val TEST_BUCKET_PREFIX = "s3-test-bucket-temp-"
4140

4241
private const val S3_MAX_BUCKET_NAME_LENGTH = 63 // https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html

0 commit comments

Comments
 (0)