Skip to content
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.

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

chore: Move event marshal/unmarshal code to smithy-swift #728

Merged
merged 5 commits into from
May 16, 2024

Conversation

jbelkins
Copy link
Contributor

@jbelkins jbelkins commented May 15, 2024

Description of changes

  • Move MessageMarshallableGenerator and MessageUnmarshallableGenerator from aws-sdk-swift to smithy-swift to support event streaming in non-AWS SDKs.
  • Move message marshal/unmarshal support in AWSHTTPBindingProtocolGenerator up to smithy-swift superclass HTTPBindingProtocolGenerator.
  • Moved event stream codegen tests from aws-sdk-swift to smithy-swift.

Scope

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -0,0 +1,204 @@
package software.amazon.smithy.swift.codegen.events
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other than this package statement, this file is unchanged from how it was in aws-sdk-swift.


class MessageUnmarshallableGenerator(
val ctx: ProtocolGenerator.GenerationContext,
val customizations: HTTPProtocolCustomizable,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class now takes customizations as a parameter, so that the unknown error type can be passed in by the calling protocol.

writer.write("let httpResponse = HttpResponse(body: .data(message.payload), statusCode: .ok)")
writer.write(
"return \$L(httpResponse: httpResponse, message: \"error processing event stream, unrecognized ':exceptionType': \\(params.exceptionType); contentType: \\(params.contentType ?? \"nil\")\", requestID: nil, typeName: nil)",
customizations.unknownServiceErrorSymbol,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The unknownServiceErrorSymbol provided by customizations replaces the hard-coded reference to an AWS type so that this file can be moved to smithy-swift.

(Same change in one other place in this file)

@@ -484,9 +488,50 @@ abstract class HTTPBindingProtocolGenerator(
return containedOperations
}

abstract override fun generateMessageMarshallable(ctx: ProtocolGenerator.GenerationContext)
Copy link
Contributor Author

@jbelkins jbelkins May 15, 2024

Choose a reason for hiding this comment

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

The 4 functions added in below are moved in unchanged from AWSHTTPBindingProtocolGenerator.

contents.shouldContainOnlyOnce(expected)
}

private fun setupTests(smithyFile: String, serviceShapeId: String): TestContext {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test setup was changed to use the smithy-swift test utils. The test expectations were adjusted because some of the generated types are now non-AWS types, and some non-AWS middlewares & other config are omitted.


override fun generateMessageUnmarshallable(ctx: ProtocolGenerator.GenerationContext) {
TODO("Not yet implemented")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These dummy implementations are removed so this protocol generator will actually render marshal/unmarshal code.


use aws.protocols#restJson1
use aws.api#service
use aws.auth#sigv4
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test Smithy file is entirely unchanged from SDK.

@jbelkins jbelkins requested review from dayaffe and sichanyoo May 15, 2024 20:54
@jbelkins jbelkins merged commit 2b46ad1 into main May 16, 2024
17 checks passed
@jbelkins jbelkins deleted the jbe/marshal_unmarshal_to_smithy_swift branch May 16, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants