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 for Guava's Immutable{Double,Int,Long}Array #24

Open
Stephan202 opened this issue Oct 29, 2017 · 9 comments
Open

Support for Guava's Immutable{Double,Int,Long}Array #24

Stephan202 opened this issue Oct 29, 2017 · 9 comments
Labels
3.0 For 3.x additions, enhancements (as opposed to 2.x)
Milestone

Comments

@Stephan202
Copy link

As of Guava 22.0 there are:

I'd be nice to add support for these types and I'm willing to help get this implemented. Before I start working on a proper PR however, I have a few questions:

  • jackson-datatype-guava 2.9.2 targets Guava 18.0. Is it okay to bump this dependency to version 22.0?
  • jackson-datatype-guava 2.9.2 targets Java 7. However, the JRE version of Guava 22.0 requires Java 8. As a work-around Guava 22.0-android can be targeted instead. Is that acceptable?
  • I have a local prototype which extends PrimitiveArrayDeserializers and delegates to PrimitiveArrayDeserializers.IntDeser (i.e. PrimitiveArrayDeserializers.forType(Integer.TYPE). The thinking here is that the deserializer then has feature parity with int[] support (Feature.ACCEPT_SINGLE_VALUE_AS_ARRAY etc.). But due to package-visibility of IntDeser this is pretty clunky. Any advice?
@cowtowncoder
Copy link
Member

I think that we probably can not bump dependency up for 2.9, due to timing; I'd be more willing with minor versions. Plus it's the last 2.x too... although at some point we may have to reconsider if and when Guava's compatibility constraints make it impossible to support older and newer versions.

However: for 3.0 I think we can upgrade to 22. Since master is now 3.0.0-SNAPSHOT, PR could be done against that?
I don't know what to do wrt Android, but I think we could cross that bridge when we get there. 3.0 will not be out until 2018.

@wykapedia
Copy link

I can look at implementing this perhaps. Here is an example of ImmutableLongArray deserializer:

public class ImmutableLongArrayDeserializer extends JsonDeserializer<ImmutableLongArray> {
    @Override
    public ImmutableLongArray deserialize(final JsonParser jsonParser, final DeserializationContext deserializationContext) throws IOException {
        final ImmutableLongArray.Builder builder = ImmutableLongArray.builder();
        while (!jsonParser.nextToken().equals(JsonToken.END_ARRAY)) {
            builder.add(jsonParser.getLongValue());
        }
        return builder.build();
    }
}

@Stephan202
Copy link
Author

@wykapedia, as far as I'm concerned: please go ahead :) (Although I opened the issue and indicated I'd be fine producing a PR, I won't have time for this in the near future. So we won't be in each other's way.)

@cowtowncoder cowtowncoder added the 3.0 For 3.x additions, enhancements (as opposed to 2.x) label Apr 29, 2018
@cowtowncoder
Copy link
Member

Note that no decision yet made to go to version 22.0 of Guava, although baseline will be at least 20 (for 3.0) and so deciding slightly higher baseline is not out of question.
Java 8 won't be problem with 3.x at least

I recommend suggesting version dep bump on:

https://groups.google.com/forum/#!forum/jackson-user

(or maybe jackson-dev?)

@wykapedia
Copy link

Is there harm in creating a PR with guava upgraded to version 22.0 (against master) with these features made available? Can be merged when the upgrade is done otherwise, or can be the reason for upgrade perhaps.

@cowtowncoder
Copy link
Member

@wykapedia There is pretty big harm for everyone who has a dependency to older Guava version because of aggressive deprecation policy Guava has. If we force use of a newish version I can guarantee I will get a flood of bug reports....

@vjkoskela
Copy link

There is the flip-side as well; we depend on Jackson and Guava and regularly update both. The longer it takes to achieve version compatibility across the two the more difficult it is for us to remain up to date with bug fixes and features. I've responded to the Google group linked above as well.

We've recently upgraded to Guava version 25 as version 23 is already over a year old. I assume it's not possible to guarantee compatibility across a large range due to the changes in Guava itself. Would it be possible to support two variants of this library?

For example, one for "stable" and one for "edge"? The update policy for Guava could be pegged for each; say "edge" is updated within 6 months and "stable" is only updated every couple of years.

@cowtowncoder
Copy link
Member

@vjkoskela It's a particularly tricky problem, and I don't have very good solutions.
Ideally I think we would be using versioning in some form -- perhaps similar to how Scala module works... or Hibernate? -- but whereas number of major versions for those is sort of limited / low, Guava has produced larger number of somewhat disjoint versions.

Then again, wasn't there some talk about Guava stabilizing little bit as of late?

Anyway, I am open to suggestions, proposals. Separate modules for newer Guava would be an option, but I am not sure what would be the best way to try to avoid collisions.

@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 23, 2021

Quick note: Jackson 2.12 has Guava 21.0 as baseline. This was an important increase as version compatibility after Guava 21 (that is, with 21 and all major versions up to and including) has been much improved compared to earlier versions.

No decision yet made on Jackson 2.13, which could further increase baseline.

Note that Jackson 2.13 DOES increase JDK baseline to 8 (excluding jackson-annotations and jackson-core which are still JDK 6 compatible, but has no effect here) so there are no blockers for increasing to Java 8 only guava version.

If anyone is interested in getting Guava minimum baseline increased, please bring this up jackson-dev google group (https://groups.google.com/g/jackson-dev).

PR for this feature is going in master as of now, for what that is worth.

@cowtowncoder cowtowncoder added this to the 3.0.0 milestone Mar 23, 2021
cowtowncoder added a commit that referenced this issue Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.0 For 3.x additions, enhancements (as opposed to 2.x)
Projects
None yet
Development

No branches or pull requests

4 participants