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

Add support for Guava primitives during deserialization #69

Merged

Conversation

albertlr
Copy link

<Primiteve>s.asList(..) implementation in Guava will return a private class, and in case you serialized this, you will not be able to deserialize it.

Lately we run into this problem and thought that the best way to do it is by contributing to this project.

@cowtowncoder
Copy link
Member

First of all, thank you for providing this PR!
It sounds great in general.

There are couple of practical challenges to consider that need to be thought out.
First one is practical and has to do with Jackson versioning: master branch is for upcoming Jackson 3.0 -- it is not clear when it would be released, but it will likely take longer than end of 2020 (partly depending on how much we want to develop 2.x). Most new features go for 2.x via branches; and in this case probably 2.12 (next upcoming minor version).
2.x up to 2.12 only requires Java 7 at this point and can not use Java 8 features -- there will probably be discussions on whether baseline should be switched to Java 8 for Jackson 2.13 in general, or if not, it could be done for Guava module (some components already require Java 8, including Kotlin module). Jackson 3.x requires baseline of at least Java 8 at this point
So: it might make sense to base this 2.12, but that would require avoiding use of closures. Merged to master, use of closures would be possible but that is of course more work.

Second question is the Guava version baseline module supports.
Jackson 2.10 and 2.11 have required Guava 20, but probably work for some older versions (down to either 18 or even 16). It seems that ImmutableDoubleArray was added in v23?
So it would be necessary to figure out what to do with compatibility of this module.
It may seem odd to support older versions, but at my daytime job baseline Guava version used was v16 just 1 year ago, and current baseline is v19 (due to some other dependencies requiring rather old Guava, and Guava using somewhat aggressive versioning which makes it more difficult to upgrade). So in general Jackson project needs to balance needs of various users and limit the rate at which minimum dependency versions are changed.

(side note: these dependencies are not well documented, this should be improved).

@cowtowncoder
Copy link
Member

Also: master is failing due to problems related to a different collections module, not Guava. It is a known but not yet resolved issue wrt API changes.

@cowtowncoder
Copy link
Member

Given Guava compatibility constraints, merging to master makes sense now I think (Jackson 2.12 required Guava 21 but still works with earlier versions -- I added notes to guava/README.md finally!).
Would be happy to merge this, I think; just need one thing, CLA:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

(that is, unless you sent it earlier -- apologies if I forgot, I can double check later on)

and the usual way is to print the doc, fill & sign, scan/photo, email to info at fasterxml dot com.
Only needs to be done once before the first Jackson contribution, good for all future contributions.

Once I get that I can proceed with merging. Thanks!

@albertlr
Copy link
Author

Would be happy to merge this, I think; just need one thing, CLA:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

(that is, unless you sent it earlier -- apologies if I forgot, I can double check later on)

and the usual way is to print the doc, fill & sign, scan/photo, email to info at fasterxml dot com.
Only needs to be done once before the first Jackson contribution, good for all future contributions.

Once I get that I can proceed with merging. Thanks!

@cowtowncoder just wanted to ping you that I sent the signed contribution agreement to the mentioned email

@cowtowncoder cowtowncoder added this to the 3.0.0 milestone Mar 11, 2021
@cowtowncoder cowtowncoder merged commit afdabd3 into FasterXML:master Mar 11, 2021
cowtowncoder added a commit that referenced this pull request Mar 11, 2021
@albertlr albertlr deleted the enhancement/guava-primitives-support branch March 23, 2021 08:58
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.

None yet

2 participants