Skip to content

Commit 8b33693

Browse files
lauzadislauzadis
authored
fix: address various failing protocol tests (#1223)
1 parent 0f8db44 commit 8b33693

File tree

11 files changed

+74
-63
lines changed

11 files changed

+74
-63
lines changed

‎codegen/protocol-tests/model/error-correction-tests.smithy‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ operation SayHelloXml { output: TestOutput, errors: [Error] }
3939

4040
structure TestOutputDocument with [TestStruct] {
4141
innerField: Nested,
42-
// FIXME: This trait fails smithy validator
43-
// @required
42+
43+
// Note: This shape _should_ be @required, but causes Smithy httpResponseTests validation to fail.
44+
// We expect `document` to be deserialized as `null` and enforce @required using a runtime check, but Smithy validator doesn't recognize / allow this.
4445
document: Document
4546
}
4647
structure TestOutput with [TestStruct] { innerField: Nested }
@@ -65,8 +66,8 @@ structure TestStruct {
6566
@required
6667
nestedListValue: NestedList
6768

68-
// FIXME: This trait fails smithy validator
69-
// @required
69+
// Note: This shape _should_ be @required, but causes Smithy httpResponseTests validation to fail.
70+
// We expect `nested` to be deserialized as `null` and enforce @required using a runtime check, but Smithy validator doesn't recognize / allow this.
7071
nested: Nested
7172

7273
@required
@@ -97,8 +98,7 @@ union MyUnion {
9798
}
9899

99100
structure Nested {
100-
// FIXME: This trait fails smithy validator
101-
// @required
101+
@required
102102
a: String
103103
}
104104

‎codegen/smithy-aws-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/aws/protocols/core/AwsHttpBindingProtocolGenerator.kt‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,7 @@ abstract class AwsHttpBindingProtocolGenerator : HttpBindingProtocolGenerator()
4040
// val targetedTest = TestMemberDelta(setOf("RestJsonComplexErrorWithNoMessage"), TestContainmentMode.RUN_TESTS)
4141

4242
val ignoredTests = TestMemberDelta(
43-
setOf(
44-
"AwsJson10ClientErrorCorrectsWithDefaultValuesWhenServerFailsToSerializeRequiredValues",
45-
"RestJsonNullAndEmptyHeaders",
46-
"NullAndEmptyHeaders",
47-
"RpcV2CborClientPopulatesDefaultsValuesWhenMissingInResponse",
48-
"RpcV2CborClientPopulatesDefaultValuesInInput",
49-
),
43+
setOf(),
5044
)
5145

5246
val requestTestBuilder = HttpProtocolUnitTestRequestGenerator.Builder()

‎codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinSymbolProvider.kt‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
193193
} else {
194194
// only use @default if type is `T`
195195
shape.getTrait<DefaultTrait>()?.let {
196-
defaultValue(it.getDefaultValue(targetShape))
196+
setDefaultValue(it, targetShape)
197197
}
198198
}
199199
}
@@ -219,9 +219,10 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
219219
}
220220
}
221221

222-
private fun DefaultTrait.getDefaultValue(targetShape: Shape): String? {
223-
val node = toNode()
224-
return when {
222+
private fun Symbol.Builder.setDefaultValue(defaultTrait: DefaultTrait, targetShape: Shape) {
223+
val node = defaultTrait.toNode()
224+
225+
val defaultValue = when {
225226
node.toString() == "null" -> null
226227

227228
// Check if target is an enum before treating the default like a regular number/string
@@ -235,13 +236,20 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
235236
"${enumSymbol.fullName}.fromValue($arg)"
236237
}
237238

238-
targetShape.isBlobShape && targetShape.isStreaming ->
239-
node
240-
.toString()
241-
.takeUnless { it.isEmpty() }
242-
?.let { "ByteStream.fromString(${it.dq()})" }
239+
targetShape.isBlobShape -> {
240+
addReferences(RuntimeTypes.Core.Text.Encoding.decodeBase64)
243241

244-
targetShape.isBlobShape -> "${node.toString().dq()}.encodeToByteArray()"
242+
if (targetShape.isStreaming) {
243+
node.toString()
244+
.takeUnless { it.isEmpty() }
245+
?.let {
246+
addReferences(RuntimeTypes.Core.Content.ByteStream)
247+
"ByteStream.fromString(${it.dq()}.decodeBase64())"
248+
}
249+
} else {
250+
"${node.toString().dq()}.decodeBase64().encodeToByteArray()"
251+
}
252+
}
245253

246254
targetShape.isDocumentShape -> getDefaultValueForDocument(node)
247255
targetShape.isTimestampShape -> getDefaultValueForTimestamp(node.asNumberNode().get())
@@ -252,6 +260,8 @@ class KotlinSymbolProvider(private val model: Model, private val settings: Kotli
252260
node.isStringNode -> node.toString().dq()
253261
else -> node.toString()
254262
}
263+
264+
defaultValue(defaultValue)
255265
}
256266

257267
private fun getDefaultValueForTimestamp(node: NumberNode): String {

‎codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/StructureGenerator.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ class StructureGenerator(
248248
} else {
249249
memberSymbol
250250
}
251+
251252
write("public var #L: #E", memberName, builderMemberSymbol)
252253
}
253254
write("")

‎codegen/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/protocol/HttpStringValuesMapSerializer.kt‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,8 @@ class HttpStringValuesMapSerializer(
157157
val paramName = binding.locationName
158158
// addAll collection parameter 2
159159
val param2 = if (mapFnContents.isEmpty()) "input.$memberName" else "input.$memberName.map { $mapFnContents }"
160-
val nullCheck = if (memberSymbol.isNullable) "?" else ""
161160
writer.write(
162-
"if (input.#L$nullCheck.isNotEmpty() == true) #L(#S, #L)",
161+
"if (input.#L != null) #L(#S, #L)",
163162
memberName,
164163
binding.location.addAllFnName,
165164
paramName,
@@ -174,8 +173,7 @@ class HttpStringValuesMapSerializer(
174173
val paramName = binding.locationName
175174
val memberSymbol = symbolProvider.toSymbol(binding.member)
176175

177-
// NOTE: query parameters are allowed to be empty, whereas headers should omit empty string
178-
// values from serde
176+
// NOTE: query parameters are allowed to be empty
179177
if ((location == HttpBinding.Location.QUERY || location == HttpBinding.Location.HEADER) && binding.member.hasTrait<IdempotencyTokenTrait>()) {
180178
// Call the idempotency token function if no supplied value.
181179
writer.addImport(RuntimeTypes.SmithyClient.IdempotencyTokenProviderExt)
@@ -185,18 +183,7 @@ class HttpStringValuesMapSerializer(
185183
paramName,
186184
)
187185
} else {
188-
val nullCheck =
189-
if (location == HttpBinding.Location.QUERY ||
190-
memberTarget.hasTrait<
191-
@Suppress("DEPRECATION")
192-
software.amazon.smithy.model.traits.EnumTrait,
193-
>()
194-
) {
195-
if (memberSymbol.isNullable) "input.$memberName != null" else ""
196-
} else {
197-
val nullCheck = if (memberSymbol.isNullable) "?" else ""
198-
"input.$memberName$nullCheck.isNotEmpty() == true"
199-
}
186+
val nullCheck = if (memberSymbol.isNullable) "input.$memberName != null" else ""
200187

201188
val cond = defaultCheck(binding.member) ?: nullCheck
202189

‎codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/SymbolProviderTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ class SymbolProviderTest {
182182
"double,2.71828,2.71828",
183183
"byte,10,10.toByte()",
184184
"string,\"hello\",\"hello\"",
185-
"blob,\"abcdefg\",\"abcdefg\".encodeToByteArray()",
185+
"blob,\"abcdefg\",\"abcdefg\".decodeBase64().encodeToByteArray()",
186186
"boolean,true,true",
187187
"bigInteger,5,5",
188188
"bigDecimal,9.0123456789,9.0123456789",

‎codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/protocol/HttpBindingProtocolGeneratorTest.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ internal class SmokeTestOperationSerializer: HttpSerializer.NonStreaming<SmokeTe
5757
}
5858
5959
builder.headers {
60-
if (input.header1?.isNotEmpty() == true) append("X-Header1", input.header1)
61-
if (input.header2?.isNotEmpty() == true) append("X-Header2", input.header2)
60+
if (input.header1 != null) append("X-Header1", input.header1)
61+
if (input.header2 != null) append("X-Header2", input.header2)
6262
}
6363
6464
val payload = serializeSmokeTestOperationBody(context, input)
@@ -264,7 +264,7 @@ internal class TimestampInputOperationSerializer: HttpSerializer.NonStreaming<Ti
264264
}
265265
parameters.decodedParameters(PercentEncoding.SmithyLabel) {
266266
if (input.queryTimestamp != null) add("qtime", input.queryTimestamp.format(TimestampFormat.ISO_8601))
267-
if (input.queryTimestampList?.isNotEmpty() == true) addAll("qtimeList", input.queryTimestampList.map { it.format(TimestampFormat.ISO_8601) })
267+
if (input.queryTimestampList != null) addAll("qtimeList", input.queryTimestampList.map { it.format(TimestampFormat.ISO_8601) })
268268
}
269269
}
270270
@@ -304,7 +304,7 @@ internal class BlobInputOperationSerializer: HttpSerializer.NonStreaming<BlobInp
304304
}
305305
306306
builder.headers {
307-
if (input.headerMediaType?.isNotEmpty() == true) append("X-Blob", input.headerMediaType.encodeBase64())
307+
if (input.headerMediaType != null) append("X-Blob", input.headerMediaType.encodeBase64())
308308
}
309309
310310
val payload = serializeBlobInputOperationBody(context, input)

‎codegen/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/protocol/HttpStringValuesMapSerializerTest.kt‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ class HttpStringValuesMapSerializerTest {
145145
contents.assertBalancedBracesAndParens()
146146

147147
val expectedContents = """
148-
if (input.header1?.isNotEmpty() == true) append("X-Header1", input.header1)
149-
if (input.header2?.isNotEmpty() == true) append("X-Header2", input.header2)
148+
if (input.header1 != null) append("X-Header1", input.header1)
149+
if (input.header2 != null) append("X-Header2", input.header2)
150150
""".trimIndent()
151151
contents.shouldContainOnlyOnceWithDiff(expectedContents)
152152
}
@@ -157,7 +157,7 @@ class HttpStringValuesMapSerializerTest {
157157
contents.assertBalancedBracesAndParens()
158158

159159
val expectedContents = """
160-
if (input.headerMediaType?.isNotEmpty() == true) append("X-Blob", input.headerMediaType.encodeBase64())
160+
if (input.headerMediaType != null) append("X-Blob", input.headerMediaType.encodeBase64())
161161
""".trimIndent()
162162
contents.shouldContainOnlyOnceWithDiff(expectedContents)
163163
}
@@ -168,10 +168,10 @@ class HttpStringValuesMapSerializerTest {
168168
contents.assertBalancedBracesAndParens()
169169

170170
val expectedContents = """
171-
if (input.enumList?.isNotEmpty() == true) appendAll("x-enumList", input.enumList.map { quoteHeaderValue(it.value) })
172-
if (input.intList?.isNotEmpty() == true) appendAll("x-intList", input.intList.map { it.toString() })
173-
if (input.strList?.isNotEmpty() == true) appendAll("x-strList", input.strList.map { quoteHeaderValue(it) })
174-
if (input.tsList?.isNotEmpty() == true) appendAll("x-tsList", input.tsList.map { it.format(TimestampFormat.RFC_5322) })
171+
if (input.enumList != null) appendAll("x-enumList", input.enumList.map { quoteHeaderValue(it.value) })
172+
if (input.intList != null) appendAll("x-intList", input.intList.map { it.toString() })
173+
if (input.strList != null) appendAll("x-strList", input.strList.map { quoteHeaderValue(it) })
174+
if (input.tsList != null) appendAll("x-tsList", input.tsList.map { it.format(TimestampFormat.RFC_5322) })
175175
""".trimIndent()
176176
contents.shouldContainOnlyOnceWithDiff(expectedContents)
177177
}
@@ -190,7 +190,7 @@ class HttpStringValuesMapSerializerTest {
190190
val queryContents = getTestContents(defaultModel, "com.test#TimestampInput", HttpBinding.Location.QUERY)
191191
val expectedQueryContents = """
192192
if (input.queryTimestamp != null) add("qtime", input.queryTimestamp.format(TimestampFormat.ISO_8601))
193-
if (input.queryTimestampList?.isNotEmpty() == true) addAll("qtimeList", input.queryTimestampList.map { it.format(TimestampFormat.ISO_8601) })
193+
if (input.queryTimestampList != null) addAll("qtimeList", input.queryTimestampList.map { it.format(TimestampFormat.ISO_8601) })
194194
""".trimIndent()
195195
queryContents.shouldContainOnlyOnceWithDiff(expectedQueryContents)
196196
}

‎gradle/libs.versions.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ micrometer-version = "1.14.2"
1818
binary-compatibility-validator-version = "0.16.3"
1919

2020
# codegen
21-
smithy-version = "1.53.0"
21+
smithy-version = "1.54.0"
2222
smithy-gradle-version = "0.9.0"
2323

2424
# testing

‎runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/text/encoding/Base64.kt‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,17 @@ public fun String.decodeBase64Bytes(): ByteArray = encodeToByteArray().decodeBas
9999
* Decode [ByteArray] from base64 format
100100
*/
101101
public fun ByteArray.decodeBase64(): ByteArray {
102-
val encoded = this
102+
// Calculate the padding needed to make the length a multiple of 4
103+
val remainder = size % 4
104+
val encoded: ByteArray = if (remainder == 0) {
105+
this
106+
} else {
107+
this + ByteArray(4 - remainder) { BASE64_PAD.code.toByte() }
108+
}
109+
103110
val decodedLen = base64DecodedLen(encoded)
104111
val decoded = ByteArray(decodedLen)
105-
val blockCnt = size / 4
112+
val blockCnt = encoded.size / 4
106113
var readIdx = 0
107114
var writeIdx = 0
108115

0 commit comments

Comments
 (0)