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

feat: Add eventstream input support for protocols that use XML for requests #680

Merged
merged 10 commits into from
Mar 15, 2024

Conversation

sichanyoo
Copy link
Contributor

@sichanyoo sichanyoo commented Mar 13, 2024

Issue #

awslabs/aws-sdk-swift#1379

Companion PR: awslabs/aws-sdk-swift#1404

Description of changes

Currently the Swift SDK does not support event stream input for protocols that use XML as request wire protocol (e.g., restXml).
Fortunately, currently there is no AWS service that uses restXml AND uses event stream input for an operation in their API either.
However, if any service that uses restXml adds an operation that takes in an event stream input in the future, our SDK would break the customers.

This PR and the companion PR in aws-sdk-swift linked above do the groundwork for supporting event stream input for protocols that use XML as request wire protocol (e.g., restXml).

Detailed descriptions of each change will be put as comments on the Github diff.

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.

Sichan Yoo added 7 commits March 12, 2024 10:46
…to handle empty structs with zero properties.
…and returns the marshalled EventStream.Message. Also add jsonMarshalClosure utility method that wraps the old marshall() methods code-generated for each streaming union shape in JSON protocols within the returned MarshalClosure.
…odyMiddleware with marshalClosure. Add marshalClosure to constructor of the middleware, and pass it along to DefaultMessageEncoderStream instead of requestEncoder.
…uctor call-site codegen. If request wire protocol is XML, uses static marshal closure (codegen for this is upcoming). If request wire protocol is JSON, calls the runtime utility function jsonMarshalClosure to get MarshalClosure.
@@ -13,42 +13,42 @@ extension EventStream {
let stream: AsyncThrowingStream<Event, Error>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, the usage of requestEncoder is completely replaced by marshalClosure. This has to be done because XML serialization does not rely on Swift's Codable interface. For JSON serialization, the closure passed here will use JSONEncoder behind the scenes. For XML serialization, the closure passed here will use XMLWriter behind the scenes.

@@ -16,3 +16,16 @@ public protocol MessageMarshallable {
/// - Returns: The marshalled `Message`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Defines a new typealias for closure that takes a stream struct, determines the specific instance of Event, and returns an EventStream.Message that has values "serialized" into it from the original Event for generic handling and encoding into Data.

A utility method jsonMarshalClosure is defined here that returns a closure which simply uses the old marshall method - that gets code-generated for each streaming union shape within the MessageMarshallable protocol conformance - to return an instance of EventStream.Message.

@@ -17,13 +17,16 @@ public struct EventStreamBodyMiddleware<OperationStackInput,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of getting the requestEncoder from middleware context and passing it along to DefaultMessageEncoderStream, the middleware now takes in a marshalClosure at initialization and passes it along to DefaultMessageEncoderStream.

@@ -21,7 +21,9 @@ import software.amazon.smithy.swift.codegen.SwiftWriter
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Depending on the request wire protocol, either pass in the utility method defined for JSON, or pass in the newly-added code-generated marshal static function variable of the streaming shape.

@@ -38,7 +38,7 @@ open class StructDecodeXMLGenerator(
SmithyXMLTypes.Reader
Copy link
Contributor Author

@sichanyoo sichanyoo Mar 13, 2024

Choose a reason for hiding this comment

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

This additional condition fixes the error thrown when handling EndEvent that's returned as part of the event stream output of the call to S3:SelectObjectContent. The S3 docs say the payload of this event is empty (not an empty XML object like <EndEvent></EndEvent>, just literally empty), which causes an issue with deserializing in SDK side. The additional condition checks if the S3 struct being deserialized has any properties, and if it doesn't, returns a new instance rather than returning nil.

Sichan Yoo added 2 commits March 13, 2024 12:05
@jbelkins jbelkins self-requested a review March 15, 2024 20:23
@sichanyoo sichanyoo merged commit a822119 into main Mar 15, 2024
12 checks passed
@sichanyoo sichanyoo deleted the feat/eventstream-xml branch March 15, 2024 22:48
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