Skip to content

Fixing bug with enums as eventHeaders #4337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
landonxjames merged 5 commits into from
Oct 13, 2025
Merged

Fixing bug with enums as eventHeaders #4337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8d810d2
Fixing bug with enums as eventHeaders
landonxjames Oct 8, 2025
038c6cc
Make enum eventHeader tests client only
landonxjames Oct 9, 2025
532f87e
Merge branch 'main' into landonxjames/eventheader-enum
landonxjames Oct 10, 2025
60d06cd
Merge branch 'main' into landonxjames/eventheader-enum
landonxjames Oct 13, 2025
9a79b66
Merge branch 'main' into landonxjames/eventheader-enum
landonxjames Oct 13, 2025
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
8 changes: 6 additions & 2 deletions ...t/codegen/client/smithy/protocols/eventstream/ClientEventStreamMarshallerGeneratorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,11 @@ class ClientEventStreamMarshallerGeneratorTest {
.is_some());
assert_eq!(
msg.payload(),
&bytes::Bytes::from_static(${testCase.generateRustPayloadInitializer(rpcEventStreamTestCase.expectedInInitialRequest)})
&bytes::Bytes::from_static(${
testCase.generateRustPayloadInitializer(
rpcEventStreamTestCase.expectedInInitialRequest,
)
})
);
""",
)
Expand All @@ -178,7 +182,7 @@ class ClientEventStreamMarshallerGeneratorTest {

class TestCasesProvider : ArgumentsProvider {
override fun provideArguments(context: ExtensionContext?): Stream<out Arguments> =
EventStreamTestModels.TEST_CASES.map { Arguments.of(it) }.stream()
EventStreamTestModels.TEST_CASES.map { Arguments.of(it.withEnumMembers()) }.stream()
}

/**
Expand Down
5 changes: 4 additions & 1 deletion ...mazon/smithy/rust/codegen/core/smithy/protocols/parse/EventStreamUnmarshallerGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import software.amazon.smithy.codegen.core.Symbol
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.BooleanShape
import software.amazon.smithy.model.shapes.ByteShape
import software.amazon.smithy.model.shapes.EnumShape
import software.amazon.smithy.model.shapes.IntegerShape
import software.amazon.smithy.model.shapes.LongShape
import software.amazon.smithy.model.shapes.MemberShape
Expand Down Expand Up @@ -261,6 +262,7 @@ class EventStreamUnmarshallerGenerator(
is IntegerShape -> rustTemplate("#{expect_fns}::expect_int32(header)?", *codegenScope)
is LongShape -> rustTemplate("#{expect_fns}::expect_int64(header)?", *codegenScope)
is BlobShape -> rustTemplate("#{expect_fns}::expect_byte_array(header)?", *codegenScope)
is EnumShape -> rustTemplate("#{expect_fns}::expect_string(header)?.as_str().into()", *codegenScope)
Copy link
Collaborator

@rcoh rcoh Oct 9, 2025

Choose a reason for hiding this comment

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

does this handle intEnum?

Copy link
Contributor Author

@landonxjames landonxjames Oct 9, 2025

Choose a reason for hiding this comment

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

I don't think any codegen actually supports intEnum (see #1700) so it isn't handled here. I did add it to the test model though so when we do support it we will get an obvious error in these tests if it doesn't work as an @eventHeader.

is StringShape -> rustTemplate("#{expect_fns}::expect_string(header)?", *codegenScope)
is TimestampShape -> rustTemplate("#{expect_fns}::expect_timestamp(header)?", *codegenScope)
else -> throw IllegalStateException("unsupported event stream header shape type: $target")
Expand Down Expand Up @@ -383,7 +385,8 @@ class EventStreamUnmarshallerGenerator(
"builder", target,
mapErr = {
rustTemplate(
"""|err|#{Error}::unmarshalling(format!("{}", err))""", *codegenScope,
"""|err|#{Error}::unmarshalling(format!("{}", err))""",
*codegenScope,
)
},
),
Expand Down
2 changes: 2 additions & 0 deletions ...zon/smithy/rust/codegen/core/smithy/protocols/serialize/EventStreamMarshallerGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import software.amazon.smithy.model.Model
import software.amazon.smithy.model.shapes.BlobShape
import software.amazon.smithy.model.shapes.BooleanShape
import software.amazon.smithy.model.shapes.ByteShape
import software.amazon.smithy.model.shapes.EnumShape
import software.amazon.smithy.model.shapes.IntegerShape
import software.amazon.smithy.model.shapes.LongShape
import software.amazon.smithy.model.shapes.MemberShape
Expand Down Expand Up @@ -243,6 +244,7 @@ open class EventStreamMarshallerGenerator(
is IntegerShape -> "Int32($inputName)"
is LongShape -> "Int64($inputName)"
is BlobShape -> "ByteArray($inputName.into_inner().into())"
is EnumShape -> "String($inputName.to_string().into())"
is StringShape -> "String($inputName.into())"
is TimestampShape -> "Timestamp($inputName)"
else -> throw IllegalStateException("unsupported event stream header shape type: $target")
Expand Down
26 changes: 26 additions & 0 deletions ...rc/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/EventStreamTestModels.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ private fun fillInBaseModel(
namespacedProtocolName: String,
extraServiceAnnotations: String = "",
nonEventStreamMembers: String = "",
extraEventHeaderMembers: String = "",
extraShapes: String = "",
): String =
"""
${"\$version: \"2\""}
namespace test

use smithy.framework#ValidationException
Expand All @@ -42,6 +45,8 @@ private fun fillInBaseModel(
Message: String,
}

$extraShapes

structure MessageWithBlob { @eventPayload data: Blob }
structure MessageWithString { @eventPayload data: String }
structure MessageWithStruct { @eventPayload someStruct: TestStruct }
Expand All @@ -55,6 +60,7 @@ private fun fillInBaseModel(
@eventHeader short: Short,
@eventHeader string: String,
@eventHeader timestamp: Timestamp,
$extraEventHeaderMembers
}
structure MessageWithHeaderAndPayload {
@eventHeader header: String,
Expand Down Expand Up @@ -129,6 +135,26 @@ object EventStreamTestModels {

fun withNonEventStreamMembers(nonEventStreamMembers: String): TestCase =
this.copy(model = fillInBaseModel(this.protocolShapeId, "", nonEventStreamMembers).asSmithyModel())

// Server doesn't support enum members in event streams, so this util allows Clients to test with those shapes
fun withEnumMembers(): TestCase =
this.copy(
model =
fillInBaseModel(
this.protocolShapeId,
extraEventHeaderMembers = "@eventHeader enum: TheEnum,\n@eventHeader intEnum: FaceCard,",
extraShapes = """ enum TheEnum {
FOO
BAR
}

intEnum FaceCard {
Copy link
Contributor

@aajtodd aajtodd Oct 9, 2025

Choose a reason for hiding this comment

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

Do we support intEnum yet? #1700

Copy link
Contributor Author

@landonxjames landonxjames Oct 9, 2025

Choose a reason for hiding this comment

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

No, but I still think this should stay in the test so that when we do support them we get an obvious test failure if they don't work as @eventHeaders instead of a false positive.

JACK = 1
QUEEN = 2
KING = 3
}""",
).asSmithyModel(),
)
}

private fun base64Encode(input: ByteArray): String {
Expand Down
Loading