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 #1404

Merged
merged 14 commits into from
Mar 15, 2024

Conversation

sichanyoo
Copy link
Contributor

@sichanyoo sichanyoo commented Mar 13, 2024

Issue #

#1379

Companion PR: smithy-lang/smithy-swift#680

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 smithy-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.

New/existing dependencies impact assessment, if applicable

Conventional Commits

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

Sichan Yoo added 9 commits March 8, 2024 13:29
…tObjectContent API. This test will fail until XML deserialization gets fixed to handle empty payload. The test is confirmed to pass after modifying EndEvent's readingClosure to check whether the type has any properties and returning a new instance instead of nil if the struct has no properties.
…ors & MessageUnmarshallableGenerators to it for organization.
…sageMarshallableGenerator, but uses closures for serialization & generates static closure variable named marshal instead of conformance to MessageMarshallable for the streaming union shapes.
…, replacing the call to MessageMarshallableGenerator.renderNotImplemented that printed error comment that said it's not implemented yet.
…sure as argument instead of requestEncoder during initialization.
@sichanyoo sichanyoo had a problem deploying to Codegen-Build-Test March 13, 2024 19:10 — with GitHub Actions Failure
@sichanyoo sichanyoo temporarily deployed to Integration-Test March 13, 2024 19:10 — with GitHub Actions Inactive
@sichanyoo sichanyoo temporarily deployed to Integration-Test March 13, 2024 19:10 — with GitHub Actions Inactive
@sichanyoo sichanyoo temporarily deployed to Integration-Test March 13, 2024 19:10 — with GitHub Actions Inactive
@sichanyoo sichanyoo temporarily deployed to Integration-Test March 13, 2024 19:10 — with GitHub Actions Inactive
@@ -0,0 +1,62 @@
//
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An integration test that tests the event stream output x restXml combo using S3:SelectObjectContent API.

@@ -4,6 +4,8 @@
*/
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 and package changes that follow are due to moving message marshal/unmarshal generators into a subdirectory for organization.

@@ -0,0 +1,182 @@
package software.amazon.smithy.aws.swift.codegen.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.

Virtually the same as MessageMarshallableGenerator but uses closures for serializing instead of encoder, and generates marshal as static function variable instead of as conformance to MessageMarshallable protocol. The marshallable generators & unmarshallable generators should both be refactored down the line to reduce code duplication. For now, followed the same decision that the XMLMessageUnmarshallableGenerator has taken.

@@ -62,6 +63,14 @@ class RestXmlProtocolGenerator : AWSHttpBindingProtocolGenerator() {
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the new XMLMessageMarshallableGenerator.

@@ -216,7 +216,7 @@ extension EventStreamTestClientTypes.TestStream: ClientRuntime.MessageUnmarshall
operation.buildStep.intercept(position: .before, middleware: AWSClientRuntime.UserAgentMiddleware(metadata: AWSClientRuntime.AWSUserAgentMetadata.fromConfig(serviceID: serviceName, version: "1.0.0", config: config)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updates preexisting codegen test.

@@ -0,0 +1,140 @@
package software.amazon.smithy.aws.swift.codegen.restxml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Codegen tests for the new XML event stream input code path.

@sichanyoo sichanyoo merged commit 02cc6cb into main Mar 15, 2024
17 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