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

Nested xml map with xml name #2236

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

peterrsongg
Copy link
Contributor

Background

  • What do these changes do?
    Adds a protocol test for the restXml protocol for unmarshalling a response that contains a nested xml map with the @xmlName trait applied to both key and value pairs

  • Why are they important?
    While implementing other protocol tests we realized that this was a gap in our generator, but it was hard to fix without an existing protocol test. This drives TDD for us.

Testing

  • How did you test these changes?
./gradlew :smithy-aws-protocol-tests:build 
PS C:\Dev\Repos\smithy-fork> ./gradlew :smithy-aws-protocol-tests:build
Picked up JAVA_TOOL_OPTIONS: -Dlog4j2.formatMsgNoLookups=true

> Configure project :
Smithy version: '1.47.0'

> Task :smithy-validation-model:smithyFormat
Running smithy format

> Task :smithy-aws-protocol-tests:smithyBuild
Running smithy build
SUCCESS: Validated 2831 shapes

Validated model, now starting projections...

ΓöÇΓöÇ  source  ΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇΓöÇ
Completed projection source (2831): C:\Dev\Repos\smithy-fork\smithy-aws-protocol-tests\build\smithyprojections\smithy-aws-protocol-tests\source

Summary: Smithy built 1 projection(s), 3 plugin(s), and 134 artifacts

> Task :smithy-aws-protocol-tests:smithyJarValidate
Running smithy validate
SUCCESS: Validated 2831 shapes


Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.6/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD SUCCESSFUL in 4s
36 actionable tasks: 8 executed, 28 up-to-date

Links

  • Links to additional context, if necessary
  • Issue #, if applicable (see here for a list of keywords to use for linking issues)

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

@peterrsongg peterrsongg requested a review from a team as a code owner April 9, 2024 18:35
@peterrsongg peterrsongg requested a review from kstich April 9, 2024 18:35
Copy link
Contributor

@kstich kstich left a comment

Choose a reason for hiding this comment

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

Will run tests in an additional client after the request test is added.

value: String
}

apply NestedXmlMapWithXmlName @httpResponseTests([
Copy link
Contributor

Choose a reason for hiding this comment

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

Should have an @httpRequestTests application as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the request test

@peterrsongg peterrsongg requested a review from kstich April 15, 2024 16:16
@kstich kstich merged commit 0941511 into smithy-lang:main Apr 16, 2024
13 checks passed
@peterrsongg peterrsongg deleted the nested-xml-map-with-xml-name branch September 10, 2024 21:21
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