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

Implement pair deserialization #37

Merged
merged 1 commit into from
Oct 31, 2018

Conversation

yawkat
Copy link
Member

@yawkat yawkat commented Oct 24, 2018

By request of the eclipse-collections authors, I've implemented deserialization support for pairs (and the twin class).

Jackson can already do the serialization to json fine, since pairs are normal beans, but the deserialization fails because the API consists of interfaces. In this PR, I take a shortcut by implementing ValueInstantiators for the tuple classes that use the PrimitiveTuples.pair factory methods. For this purpose, I use CreatorPropertys - I'm not sure if this is the best approach, but it appears to work very well. If there's a better solution, please suggest it.

Tested are all pair classes and the twin class, including both the primitive and the object variants. Additionally, there is one test (for ObjectIntPair) that tests subtype serialization to confirm that I didn't screw up type deserializers.

The patch was developed and tested against 2.9, but since only the POM changed, I don't see any merge issues against 2.10 - is 2.10 the current branch to develop on?

@cowtowncoder
Copy link
Member

I think that works, so I will just go ahead and merge this. Thank you again for all your work here!

@cowtowncoder cowtowncoder merged commit e002ec5 into FasterXML:2.10 Oct 31, 2018
@cowtowncoder cowtowncoder changed the title Implement pair deserialization for eclipse-collections Implement pair deserialization Oct 31, 2018
@cowtowncoder
Copy link
Member

Ok.... so, the good news is that it works as expected for 2.10.
Bad news is that it will not work for 3.0, due to refactoring which makes TypeDeserializer only accessible via DeserializationContext. I will see if I can resolve that.

@cowtowncoder
Copy link
Member

Was able to solve the problem relatively simply, so master (3.0.0-SNAPSHOT) works too.

@cowtowncoder cowtowncoder added this to the 2.10.0 milestone Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants