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

Add JSON protocol tests for document-valued maps #2125

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

ianbotsf
Copy link
Contributor

@ianbotsf ianbotsf commented Feb 5, 2024

Issue #, if available:

(none)

Description of changes:

This change adds tests for serializing/deserializing maps which use the document shape as a value. The AWS SDK for Kotlin previously did not handle these correctly, leading to a customer contact.

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

@ianbotsf ianbotsf requested a review from a team as a code owner February 5, 2024 18:48
/// This example serializes documents as the value of maps.
@idempotent
@http(uri: "/DocumentTypeAsMapKey", method: "PUT")
operation DocumentTypeAsMapKey {
Copy link
Contributor

Choose a reason for hiding this comment

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

Document is the map value in these tests, all the names should be updated accordingly.

apply DocumentTypeAsMapKey @httpResponseTests([
{
id: "DocumentTypeAsMapKeyOutput",
documentation: "Deserializes a map that uses documents as the value.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
documentation: "Deserializes a map that uses documents as the value.",
documentation: "Serializes a map that uses documents as the value.",

nit: Since this isn't client specific

…n' instead of 'deserialization' in documentation
@kstich kstich merged commit 8dc0769 into smithy-lang:main Feb 8, 2024
10 checks passed
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