Skip to content

Conversation

gregggreg
Copy link

DynamoDBEvent.Decoder isn't currently handling the case where .binary attributes are base64encoded strings and throws an error

Motivation:

If you try to use DynamoDBEvent.Decoder on binary values, it throws an error saying it is expecting the value to be a string

Modifications:

Decoding string values now handles the case where they are .binary as well

Result:

If you have a DynamoDB record with binary values, you will be able to use DynamoDBEvent.Decoder to properly decode it without an error being thrown

@sebsto
Copy link
Collaborator

sebsto commented Nov 7, 2024

Thank you @gregggreg
Can you add a unit test to cover this change ?

Copy link
Collaborator

@sebsto sebsto left a comment

Choose a reason for hiding this comment

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

Looks good but I would like to add a unit test to cover this change

@sebsto sebsto added the 🆕 semver/minor Adds new public API. label Nov 26, 2024
@sebsto
Copy link
Collaborator

sebsto commented Jun 27, 2025

Hello @gregggreg
DO you think you can find a bit of time to add a unit test for this PR ?

@sebsto
Copy link
Collaborator

sebsto commented Oct 22, 2025

Closing in favor of #103

@sebsto sebsto closed this Oct 22, 2025
sebsto pushed a commit that referenced this pull request Oct 22, 2025
…Event.Decoder (#103)

Cleaned up version of
[#61](#61) that
also has tests.

This PR makes it so that in lists, the decoder will not try to access
the 0th index of an empty array.

This PR will also make it so that `AttributeValue`s of type `.binary`
will properly decode to base64 Strings when String is put as the type in
the the `Decodable`. To avoid including `Data` in the code, I included
code from the original source of the base 64 decoding methods
(https://github.com/fabianfett/swift-base64-kit) to add in the
corresponding encoding methods. However, I saw lots of Foundation is
imported throughout so I reverted it. What are your thoughts on this
matter?

I also added tests that aim to make these failure points more resilient
in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants