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

Nested generics support for Structs #103

Merged
merged 1 commit into from
Dec 27, 2018

Conversation

Praveen2112
Copy link

@Praveen2112 Praveen2112 commented Dec 17, 2018

For a class Bean<T>(as Thrift struct) when we try to serialize/de-serialize for Bean<String> drift codec generates the code for the type String and maintains it the generated DriftCodec in a cache. But now if we try to do the same operation for Bean<Long> it tries to use the code generated for Bean<String>. This patch makes sure that we generate a separate DriftCodec for different type of T in Bean<T>.

@electrum
Copy link
Member

I'm curious, how did you run into this issue? I didn't realize we supported this feature, and it seems like it would be hard to use in RPC which requires concrete types.

@Praveen2112
Copy link
Author

Praveen2112 commented Dec 18, 2018

@electrum Thanks for the review. Resolved the above issues. We currently use drift-codec for only for serialization/de-serialization (as a replacement for JSON serialization/de-serialization). Like List<T> we have a custom object Bean<T> which has to be serialized/de-serialized. First we created a ThriftCodec for Bean<String> and it was working fine , when we try to create a ThriftCodec for Bean<Long> and try to serialize a instance of Bean<Long> we faced this issue.

@Praveen2112
Copy link
Author

Praveen2112 commented Dec 19, 2018

@electrum Any other changes that I need to do ? Ideally it might resolve the issue discussed here (#70 (comment)) ?

@electrum electrum merged commit 98c4b3e into airlift:master Dec 27, 2018
@Praveen2112 Praveen2112 deleted the nested_generics_for_structs branch January 10, 2019 07:39
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.

2 participants