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

Support arbitrary primitive types and enums as discriminators #355

Merged
merged 2 commits into from Aug 2, 2019

Conversation

@kelnos
Copy link
Member

commented Jul 24, 2019

For Jackson, we assume that polymorphic POJO discriminators are of type string, which is of course a bit limiting. Ideally they can be any type, including enums.

Unfortunately, it's not possible for us to support any arbitrary type with how we handle discriminators right now, but we can at least explicitly support all primitive types, as well as enums.

Stuff I'm not happy about:

  1. To figure out which type to use, we "reverse engineer" the parsed type that the core gives us.
  2. If the type the core gives us isn't a recognized primitive type, or one of the types explicitly listed as being not supported, we just assume it's an enum type, which of course it may not be. This will result in generated code that won't compile. It would be better to present an error at generation time, but on the plus side, at least this doesn't give you runtime errors.

This already works out of the box with Circe (sorta; I think number types might get mis-JSONed as strings) because of differences in how poloymorphism are handled, as well as the fact that Scala has typeclasses, so as long as you have an Encoder in scope, you can convert anything into a JSON value. You can't really make things work that way with Java/Jackson.

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

@kelnos kelnos requested a review from blast-hardcheese Jul 24, 2019

@kelnos kelnos force-pushed the kelnos:jackson-non-string-discriminators branch 2 times, most recently from 5323417 to ddc7f20 Aug 1, 2019

@blast-hardcheese
Copy link
Collaborator

left a comment

This is wild. I would assume most of this stuff would apply to the Circe decoder as well (possibly generalizable into JVMValueHelpers or something)

@kelnos kelnos force-pushed the kelnos:jackson-non-string-discriminators branch 2 times, most recently from 9070045 to a9c2dff Aug 2, 2019

@kelnos kelnos force-pushed the kelnos:jackson-non-string-discriminators branch from a9c2dff to 83d4dd8 Aug 2, 2019

@kelnos kelnos merged commit 6b746c1 into twilio:master Aug 2, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@kelnos kelnos deleted the kelnos:jackson-non-string-discriminators branch Aug 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.