{"action":"created","issue":{"url":"https://api.github.com/repos/stripe/stripe-dotnet/issues/1370","repository_url":"https://api.github.com/repos/stripe/stripe-dotnet","labels_url":"https://api.github.com/repos/stripe/stripe-dotnet/issues/1370/labels{/name}","comments_url":"https://api.github.com/repos/stripe/stripe-dotnet/issues/1370/comments","events_url":"https://api.github.com/repos/stripe/stripe-dotnet/issues/1370/events","html_url":"https://github.com/stripe/stripe-dotnet/pull/1370","id":377984974,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI4Nzk3MjY1","number":1370,"title":"[wip] [experimental] New type and converter for expandable fields","user":{"login":"ob-stripe","id":23086931,"node_id":"MDQ6VXNlcjIzMDg2OTMx","avatar_url":"https://avatars0.githubusercontent.com/u/23086931?v=4","gravatar_id":"","url":"https://api.github.com/users/ob-stripe","html_url":"https://github.com/ob-stripe","followers_url":"https://api.github.com/users/ob-stripe/followers","following_url":"https://api.github.com/users/ob-stripe/following{/other_user}","gists_url":"https://api.github.com/users/ob-stripe/gists{/gist_id}","starred_url":"https://api.github.com/users/ob-stripe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ob-stripe/subscriptions","organizations_url":"https://api.github.com/users/ob-stripe/orgs","repos_url":"https://api.github.com/users/ob-stripe/repos","events_url":"https://api.github.com/users/ob-stripe/events{/privacy}","received_events_url":"https://api.github.com/users/ob-stripe/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-11-06T18:50:19Z","updated_at":"2018-11-07T14:45:44Z","closed_at":null,"author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/stripe/stripe-dotnet/pulls/1370","html_url":"https://github.com/stripe/stripe-dotnet/pull/1370","diff_url":"https://github.com/stripe/stripe-dotnet/pull/1370.diff","patch_url":"https://github.com/stripe/stripe-dotnet/pull/1370.patch"},"body":"Still very experimental, and the code is downright awful at the moment, but here's the gist of it:\r\n- adds a new `ExpandableField<>` generic class used to replace the previous `StringOrObject<>.Map` method. The class is simply a container for two properties:\r\n    - a string (`Id`)\r\n    - an instance of the type param (`ExpandedObject`)\r\n- adds an `ExpandableFieldConverter` custom converter class to deserialize `ExpandableField`s. The converter:\r\n    - creates an instance of `ExpandableField`\r\n    - if the JSON is a string, fills `Id` with the string and sets `ExpandedObject` to `null`\r\n    - if the JSON is an object:\r\n        - if the param type is an interface (i.e. if the type is a polymorphic resource), deserializes the JSON with the proper custom converter\r\n        - if the param type is _not_ an interface (i.e. it's a concrete class), directly deserializes the JSON into an instance of the type\r\n        - sets `ExpandableField` to the deserialized instance and `Id` to the value of the `Id` property of the deserializes instance\r\n        - if the deserialization fails for some reason (e.g. because of a new polymorphic type that the library doesn't know about), make a last attempt at manually extracting the `id` property of the raw JSON object\r\n    - returns the filled `ExpandableField` instance\r\n\r\nSort of works, but we'll also need to add a custom deserializer for `StripeList` to handle polymorphic types there as well.\r\n"},"comment":{"url":"https://api.github.com/repos/stripe/stripe-dotnet/issues/comments/436646518","html_url":"https://github.com/stripe/stripe-dotnet/pull/1370#issuecomment-436646518","issue_url":"https://api.github.com/repos/stripe/stripe-dotnet/issues/1370","id":436646518,"node_id":"MDEyOklzc3VlQ29tbWVudDQzNjY0NjUxOA==","user":{"login":"ob-stripe","id":23086931,"node_id":"MDQ6VXNlcjIzMDg2OTMx","avatar_url":"https://avatars0.githubusercontent.com/u/23086931?v=4","gravatar_id":"","url":"https://api.github.com/users/ob-stripe","html_url":"https://github.com/ob-stripe","followers_url":"https://api.github.com/users/ob-stripe/followers","following_url":"https://api.github.com/users/ob-stripe/following{/other_user}","gists_url":"https://api.github.com/users/ob-stripe/gists{/gist_id}","starred_url":"https://api.github.com/users/ob-stripe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ob-stripe/subscriptions","organizations_url":"https://api.github.com/users/ob-stripe/orgs","repos_url":"https://api.github.com/users/ob-stripe/repos","events_url":"https://api.github.com/users/ob-stripe/events{/privacy}","received_events_url":"https://api.github.com/users/ob-stripe/received_events","type":"User","site_admin":false},"created_at":"2018-11-07T14:45:44Z","updated_at":"2018-11-07T14:45:44Z","author_association":"COLLABORATOR","body":"> I would love to see a test for serialization/deserialization to ensure this continues to work as devs expect\r\n\r\nYep, definitely on my TODO list.\r\n\r\n> I wonder if there's value in only ever supporting the Expanded object and not have the customer get/setter for the id/object. Basically do what we did in Go and less what we did in Java if that makes sense\r\n\r\nI'm on the fence about this, I kind of like that those custom getters help keep a ~1:1 mapping between the library's class structure and the API's structure. If we only exposed the `ExpandableField` attribute that would add a virtual layer that only exists in the library. Not sure it's that big a deal though.\r\n\r\n> I'm curious how hard it will be to remember to changer the converter for future interfaces. Is it easy to add a test for this? If not, I worry that we really on tribal knowledge.\r\n\r\nNot sure this is easily (or even hardly 😛) testable. I think it's on us to keep in mind that handling polymorphic resource is one of the more fragile parts of the library and we should write thorough tests when such resources are added. I'll see if I can come up with something automated though.\r\n"}}
{"actor":{"display_login":"ob-stripe"}}