jackson-datatypes-collections icon indicating copy to clipboard operation
jackson-datatypes-collections copied to clipboard

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

Open Stephan202 opened this issue 6 years ago • 9 comments

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?

Stephan202 avatar Oct 29 '17 21:10 Stephan202

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.

cowtowncoder avatar Oct 30 '17 18:10 cowtowncoder

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();
    }
}

wykapedia avatar Apr 28 '18 16:04 wykapedia

@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.)

Stephan202 avatar Apr 28 '18 16:04 Stephan202

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?)

cowtowncoder avatar Apr 29 '18 17:04 cowtowncoder

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.

wykapedia avatar Apr 30 '18 12:04 wykapedia

@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....

cowtowncoder avatar May 01 '18 04:05 cowtowncoder

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.

vjkoskela avatar Jan 14 '19 04:01 vjkoskela

@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 avatar Jan 16 '19 19:01 cowtowncoder

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 avatar Mar 23 '21 00:03 cowtowncoder