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

New FromJson method #1484

Merged
merged 1 commit into from
Jan 24, 2019
Merged

New FromJson method #1484

merged 1 commit into from
Jan 24, 2019

Conversation

ob-stripe
Copy link
Contributor

@ob-stripe ob-stripe commented Jan 23, 2019

r? @remi-stripe
cc @stripe/api-libraries

Adds a new FromJson() utility method to deserialize JSON strings into Stripe objects.

The method can be used in two ways:

  • var charge = StripeEntity.FromJson<Charge>(json);
  • var charge = Charge.FromJson(json);

(The latter is made possible by using the curiously recurring template pattern.)

EDIT: Also possible:

  • var charge = StripeEntity.FromJson(json). At compile time, charge will be an IHasObject, and will be instantiated to the correct concrete type at runtime depending on the contents of json. Of course this only works if the JSON string has an object key with a known value, otherwise this returns null.

@ob-stripe ob-stripe mentioned this pull request Jan 23, 2019
52 tasks
@ob-stripe ob-stripe force-pushed the ob-from-json branch 3 times, most recently from 413ef53 to de68dc1 Compare January 23, 2019 16:55
Copy link
Contributor

@remi-stripe remi-stripe left a comment

Choose a reason for hiding this comment

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

This looks good to me, I think at least! A bit sad we have to explicitly pass the class name to StripeEntity though

@ob-stripe
Copy link
Contributor Author

A bit sad we have to explicitly pass the class name to StripeEntity though

I could add a method that instantiates the correct type based on the object key in the JSON, but it would have to live on IHasObject, not StripeEntity, since it would only work for models that do have an object key.

@remi-stripe
Copy link
Contributor

Do you think it'd be better? It would prevent deserializing some hashes that are not real API resources right?

@ob-stripe
Copy link
Contributor Author

Do you think it'd be better? It would prevent deserializing some hashes that are not real API resources right?

Such a method could only deserialize "real" API resources, i.e. resources with an object key, yes. Otherwise we simply don't know which type to instantiate.

To be clear, all of these are just convenience helper methods. We won't be relying on them internally, we're just exposing them to users to make it a bit simpler to manually deserialize resources.

@ob-stripe ob-stripe assigned ob-stripe and remi-stripe and unassigned ob-stripe Jan 23, 2019
@ob-stripe ob-stripe merged commit 79c4187 into integration-v23 Jan 24, 2019
@ob-stripe ob-stripe deleted the ob-from-json branch January 24, 2019 09:26
@bugejakurt
Copy link

Can we have an async equivalent of this method?

@ob-stripe
Copy link
Contributor Author

ob-stripe commented Jul 29, 2019

@bugejakurt Unfortunately, I don't think that'd be possible. The library uses Newtonsoft.Json to deserialize JSON, and Newtonsoft.Json's DeserializeObject method is synchronous.

EDIT: unless you meant adding overload methods that accept a Stream instead of a String? We could have async variants for those, for the stream-to-string conversion part (not the actual string-to-object deserialization).

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