Skip to content

Serialize null values in all maps#903

Merged
ob-stripe merged 1 commit intomasterfrom
ob-serialize-nulls-in-maps
Nov 16, 2019
Merged

Serialize null values in all maps#903
ob-stripe merged 1 commit intomasterfrom
ob-serialize-nulls-in-maps

Conversation

@ob-stripe
Copy link
Contributor

r? @brandur-stripe @richardm-stripe
cc @stripe/api-libraries

This PR serializes null values in all Maps when converting typed parameters classes to untyped maps.

Previously, we were only serializing null values in Map<String, String> maps. The intention was that only metadata fields had this type and that's where we wanted to serialize values.

However, after #849, some metadata fields (such as TransactionUpdateParams.metadata) were no longer typed as Map<String, String> but rather as Object because they became polymorphic (either a HashMap<String, String> or an EmptyParam).

Due to type erasure, the type parameters are no longer present at runtime and the type adapter that was supposed to serialize null values failed to do so because this check failed.

This PR simplifies the type adapter by simply checking if the value is a Map (or any subclass such as HashMap). When that's the case, the type adapter sets serializeNulls(true) and then delegates to the normal adapter for the type. As a result, all null values are now serialized in all Maps. I think that's what we want anyway: if an entry is present in a map, it was necessarily added by the user and should be serialized.

Fixes #893.

Copy link
Contributor

@brandur-stripe brandur-stripe left a comment

Choose a reason for hiding this comment

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

if an entry is present in a map, it was necessarily added by the user and should be serialized.

Yeah, I would've said the same thing.

Thanks for the detailed PR description and for tracking all of this down. LGTM.

@ob-stripe ob-stripe force-pushed the ob-serialize-nulls-in-maps branch from 77908d4 to ba268df Compare November 15, 2019 19:45
@ob-stripe ob-stripe merged commit 611078f into master Nov 16, 2019
@ob-stripe ob-stripe deleted the ob-serialize-nulls-in-maps branch November 16, 2019 03:57
@ob-stripe
Copy link
Contributor Author

Released as v15.3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot remove metadata entries with typed / builder update params

4 participants