-
Notifications
You must be signed in to change notification settings - Fork 299
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
Handle Composite's name serialization properly #503
Conversation
It looks like this issue was closed without the fix being merged into the product? I ran into this same problem in release 1.1-2, the symptom being a NullPointerException. The CompositeSerializer defaults to ByteType which is not in the type-to-comparator mapping in AbstractComposite resulting in an NPE on deserialization. I coded my own fix which looks a lot like vaidhy's. So I'm wondering why this issue was closed. |
The merge above is in master currently. Can you compare your change with the following and send me a patch: If something is amiss, we'll get it sorted out. Thanks. |
I'm not referring to the change in AbstractComposite here which is also in release 1.1-2. I don't believe that that change actually addresses the original issue (#503) unless I am missing something. I am referring to vaidhy's pull request which is a change to CompositeSerializer that takes a list of serializers at construction time. I am a bit confused actually, because I don't see how deserialization of a Composite could possibly work without the serializers being properly set for each component. I looked into the NPE I was getting, and this does not occur in master due to the use of a different version of the google guava library. Nate's suggestion to use a factory to create a proper CompositeSerializer is fine, but it doesn't fix the problem with the singleton CompositeSerializer which is broken for deserialization. I have just submitted a pull request that contains a modified CompositeSerializer and also has two unit tests that demonstrate the problem it fixes. The test that uses the singleton CompositeSerializer fails, and the test that constructs a CompositeSerializer with the proper serializers succeeds. |
I'm still getting used to how GitHub works. Seems pull requests and issues are essentially the same thing (?), so apparently my pull request, #585, sort of does already re-open this issue. |
Hi Eric, I decided to build my own CompositeSerializer due to the exact same On Mon, Feb 4, 2013 at 4:31 PM, Eric Zoerner [email protected]:
|
When a composite is serialized and then deserialized, the components of composite are typed as ByteBuffer. This patch enables the use of right kind of serializers for composite.
This patch contains the following: