Skip to content

Smithy XML gem#318

Merged
mullermp merged 8 commits into
decaffrom
xml
Jul 29, 2025
Merged

Smithy XML gem#318
mullermp merged 8 commits into
decaffrom
xml

Conversation

@mullermp
Copy link
Copy Markdown
Contributor

@mullermp mullermp commented Jul 25, 2025

Adds the Smithy XML gem for generic XML parsing. It has the same 5 engines as SDK v3. It currently passes AwsQuery protocol tests.

Copy link
Copy Markdown
Contributor

@richardwang1124 richardwang1124 left a comment

Choose a reason for hiding this comment

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

Nice work, left a few questions. Most changes are very similar to V3.

Comment thread gems/smithy-client/sig/smithy-client/interfaces.rbs
def consume_child_frame(child) # rubocop:disable Metrics/AbcSize
case child
when MapEntryFrame
@result[@member[:name]] ||= {}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I noticed that the V3 code does not have this. Should we fix this in V3? Not sure how possible/likely that @result[@member[:name]] would be nil.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In the v3 code we were using DefaultList and DefaultMap. I think this was wrong because even when there weren't any values, we would parse empty lists and maps. There is a patch in the json parser in v3 that has to maintain this behavior from query to json compatibility. I'm not 100% but this should have prevented this, I will double check.

Comment thread gems/smithy/spec/spec_helper.rb
Copy link
Copy Markdown
Contributor

@jterapin jterapin left a comment

Choose a reason for hiding this comment

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

Nice!

Comment thread gems/smithy-xml/lib/smithy-xml/codec.rb
Comment thread gems/smithy-xml/lib/smithy-xml/builder.rb
Comment thread gems/smithy-xml/lib/smithy-xml/parser/frame.rb
Comment thread gems/smithy/spec/spec_helper.rb
Comment thread gems/smithy-xml/spec/spec_helper.rb
Comment thread gems/smithy-xml/lib/smithy-xml/parser.rb Outdated
Comment thread gems/smithy-xml/lib/smithy-xml/parser.rb
@mullermp mullermp merged commit 66b3ab2 into decaf Jul 29, 2025
15 checks passed
@mullermp mullermp deleted the xml branch July 29, 2025 23:05
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.

3 participants