feat: enum and int enums as event headers #1443
Conversation
This comment has been minimized.
This comment has been minimized.
| when (target.type) { | ||
| ShapeType.ENUM, ShapeType.INT_ENUM -> { | ||
| writer.write( | ||
| "?.let { #T.fromValue(it) }", | ||
| targetSymbol, | ||
| ) | ||
| } | ||
| else -> writer.write("") | ||
| } |
There was a problem hiding this comment.
simplification: Can this ?.let { #T.fromValue(it) } be made part of the conversionFn for enum and int enum? It feels wrong to have extra logic doing type conversion when we already have conversionFn
There was a problem hiding this comment.
I don't think we can, since the conversion functions are not code-generated, I'm not sure how it would replace #T.fromValue(it).
There was a problem hiding this comment.
something like this should be possible:
@InternalApi
fun <T> HeaderValue.expectEnumValue(fromValue: (String) -> T): T {
val stringValue = expectString()
return fromValue(stringValue)
}it's not a strong opinion but I think this could be done cleaner
|
@ianbotsf I added some codegen tests downstream: aws/aws-sdk-kotlin#1710. I'm not sure if you saw them. |
Those are great but smithy-kotlin will soon have other use cases besides the SDK. We should ensure this repo is capable of testing all of its features, whether the SDK uses them or not. |
This comment has been minimized.
This comment has been minimized.
| val expectedEnumSerializer = "input.value.enum?.let { addHeader(\"enum\", HeaderValue.String(it.value)) }" | ||
| val expectedIntEnumSerializer = "input.value.intEnum?.let { addHeader(\"intEnum\", HeaderValue.Int32(it.value)) }" | ||
| val actualSerializer = testCtx.manifest.expectFileString("src/main/kotlin/com/test/serde/TestStreamOpOperationSerializer.kt") | ||
| actualSerializer.shouldContainOnlyOnceWithDiff(expectedEnumSerializer) | ||
| actualSerializer.shouldContainOnlyOnceWithDiff(expectedIntEnumSerializer) | ||
|
|
||
| val expectedEnumDeserializer = "eb.enum = message.headers.find { it.name == \"enum\" }?.value?.expectEnumValue(Enum::fromValue)" | ||
| val expectedIntEnumDeserializer = "eb.intEnum = message.headers.find { it.name == \"intEnum\" }?.value?.expectIntEnumValue(IntEnum::fromValue)" | ||
| val actualDeserializer = testCtx.manifest.expectFileString("src/main/kotlin/com/test/serde/TestStreamOpOperationDeserializer.kt") | ||
| actualDeserializer.shouldContainOnlyOnceWithDiff(expectedEnumDeserializer) | ||
| actualDeserializer.shouldContainOnlyOnceWithDiff(expectedIntEnumDeserializer) |
There was a problem hiding this comment.
Style: Strings with double-quotes inside are generally more readable with triple quotes, which don't require escaping. For example:
// harder to read
val expectedEnumSerializer = "input.value.enum?.let { addHeader(\"enum\", HeaderValue.String(it.value)) }"
// easier to read
val expectedEnumSerializer = """input.value.enum?.let { addHeader("enum", HeaderValue.String(it.value)) }"""
| /** | ||
| * A concrete implementation of AwsHttpBindingProtocolGenerator to exercise: | ||
| * renderThrowOperationError() | ||
| * getProtocolHttpBindingResolver() | ||
| */ | ||
| class TestableAwsHttpBindingProtocolGenerator : AwsHttpBindingProtocolGenerator() { |
There was a problem hiding this comment.
Comment: Thanks for adding new KDocs to old code!
Affected ArtifactsChanged in size
|
0marperez
dismissed
lauzadis’s stale review
Implemented requested changes and already got required approvals.
4 participants
Issue #
N/A
Description of changes
Supports smithy enums and int enums as event headers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.