Mark eclipse-collections immutable collections as collection-like for nested list handling [Fixes #71]

[yawkat]

ImmutableMap was not affected by this because it has a different code path. For Map deserializers we have a giant typextype table of deserializer handlers which have to fish the type parameter out of the JavaType directly because the position differs (eg IntObjectMap has the value type as parameter 0, while MutableMap has the value type as parameter 1).

Could maybe make the same change for map-likes as well, but not sure what exact use case it would fix (maybe custom annotations on the property would work?). Also not sure how or if it would work with primitive->ref maps, since those technically don't have a key in the type param?

[yawkat]

Looking at it further, for map-like deser with proper value type deserializer support I'd like to stick to 2.12 because that's where the templated implementation was introduced. A lot less work in that branch :)

[cowtowncoder]

Yes, 2.12 is fine, I am hoping to get first release candidate out during September; and if bigger/riskier changes are needed it makes sense too.

The main benefit of making type MapLike is wrt support for standard annotations for denoting key/value (de)serializers, and value type (de)serializer (polymorphic values). It's def bit of judgment call on primitive types and so on; it's fine to give nominal type of, say, java.lang.Object as placeholder, but then again at some point data type has so many exceptions that maybe type refinement makes no sense.

[cowtowncoder]

Also: feel free to merge; I'm happy to do that but it is usually safer for module author to do it just in case there was some small piece missing.

[yawkat]

@cowtowncoder I don't have access to merge here

[cowtowncoder]

@yawkat Oops. That is an oversight -- I invited you to team that should have access. Please let me know if it does not work. As I said, I don't mind merging but also do not want to be a blocker when my response times sometimes vary.

[yawkat]

@cowtowncoder I'm in the org now but can't merge this PR still.

Also, if I do the merging, I assume I should also port the patch to other versions (2.12, master), what's your workflow for this? Just cherry pick + run the tests?

[cowtowncoder]

Merging should simply work from older to newer as I do not cherry-pick in those cases. Cherry-picking needed for occasional reverse case (in which I add change in master, then cherry-pick into oldest, then forward [if there are intervening versions]).

I'll merge this, will see why you don't have access.

[cowtowncoder]

Looks like there is one failing test case, due to minor flaw in CollectionLikeType / MapLikeType handling: there are matching isTrueCollectionType() / isTrueMapType() methods that check contained type (is underlying erased type Collection or Map) -- which might be ok, except that then caller assumes what follows is that one can upcast to MapType / CollectionType. Normally that is fine but looks like here sometimes real Collection is only promoted as CollectionLikeType causing exception with BasicSerializerFactory.

I will patch code in jackson-databind for 2.11.3 (and further in 2.12), although not sure if there might be improvement to be made in TypeModifier for eclipse-collections: if it knows type is Collection, it should promote to CollectionType. But I don't know if this is known in type hierarchy -- either way, I can work around this issue.

[cowtowncoder]

Ok. There seems to be some issue left as SerializerTest#ref() now fails with:

com.fasterxml.jackson.databind.JsonMappingException: Size must be 1 but was 3 (through reference chain: org.eclipse.collections.impl.set.immutable.ImmutableTripletonSet["only"])
    at com.fasterxml.jackson.databind.JsonMappingException.wrapWithPath(JsonMappingException.java:397)
    at com.fasterxml.jackson.databind.JsonMappingException.wrapWithPath(JsonMappingException.java:356)
    at com.fasterxml.jackson.databind.ser.std.StdSerializer.wrapAndThrow(StdSerializer.java:316)
    at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:763)
    at com.fasterxml.jackson.databind.ser.BeanSerializer.serialize(BeanSerializer.java:178)
    at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider._serialize(DefaultSerializerProvider.java:480)
    at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:319)
    at com.fasterxml.jackson.databind.ObjectMapper._writeValueAndClose(ObjectMapper.java:4406)
    at com.fasterxml.jackson.databind.ObjectMapper.writeValueAsString(ObjectMapper.java:3660)
    at com.fasterxml.jackson.datatype.eclipsecollections.SerializerTest.ref(SerializerTest.java:34)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:89)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:542)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:770)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:464)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)
Caused by: java.lang.IllegalStateException: Size must be 1 but was 3
    at org.eclipse.collections.impl.set.immutable.ImmutableTripletonSet.getOnly(ImmutableTripletonSet.java:131)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:689)
    at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:755)
    ... 29 more

[cowtowncoder]

Merged from 2.11 -> 2.12 -> master, same test fail in all (which probably makes sense).

[cowtowncoder]

@yawkat On team: for some reason it only had "read" access (about same as all devs); changed it to give push access so merging should now be accessible.

[yawkat]

73