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

Jackson 2.8.11 release for Scala module #359

Closed
cowtowncoder opened this issue Dec 26, 2017 · 7 comments
Closed

Jackson 2.8.11 release for Scala module #359

cowtowncoder opened this issue Dec 26, 2017 · 7 comments
Assignees

Comments

@cowtowncoder
Copy link
Member

I hope this should be easier than 2.9. So, 2.8.11 is out:

https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.8.11

and it'd be good to get Scala module out too. This is planned to be the last full 2.8.x release, closing the branch. So should perhaps simplify things a bit (I forget if there's any scala version deprecation involved here, perhaps not).

@mbknor
Copy link
Member

mbknor commented Jan 3, 2018

hmm.. This one also fails with the same error as #357

Is it something wrong with my machine?

....

We need to figure this out..

@cowtowncoder
Copy link
Member Author

Hmmh. It could be one of backported fixes. While nothing looks particularly suspicious, it could be either one of:

One thing you could try locally is to build against 2.8.10 and see if that'd work.
I would guess it does, which wouldn't help a lot. If it is easy enough to build against 2.8.11-SNAPSHOT could also try checking out various points in 2.8 to rule out PRs.
But I guess fundamentally, assuming it is side-effect of one of the fixes, something would probably have to change in the test. Or, in worst case, scala module... (worst just because it's not very easy to see how things work, sometimes).

@mbknor
Copy link
Member

mbknor commented Jan 14, 2018

I have not tracked the issue down yet, but this is what I've found out so far:

I've been using the 2.8-branch for testing - I guess there are less changes between 2.8.10 and 2.8.11.

The problem is this line:

In 2.8.10, _mapDeser ends up with SortedMapDeserializer
In 2.8.11, it ends up with SeqDeserializer.

I have not figured out why yet, but I write it here for info/starting point

@cowtowncoder
Copy link
Member Author

Ok. I don't know if change is problematic (it may well be), but for context that "untyped" deserializer is for use with java.lang.Object as nominal type (most often due to raw types / missing generic declaration). And override is needed, I think, to change mapping from JSON Object into Scala Map, not JDK Map (and perhaps ditto for Java List to Scala List).

@mbknor
Copy link
Member

mbknor commented Jan 14, 2018

I found it the change causing the problem:

FasterXML/jackson-databind@4675896#diff-71da9745111f01bc93519d574c2f9fabL416

if (baseType.isInterface()) {
    // The scala-test was relying on this line.
    newType = baseType.refine(subclass, tb, null, new JavaType[] { baseType });
} else {
    newType = baseType.refine(subclass, tb, baseType, NO_TYPES);
}

For 2.8.10, we end up with isMapLike == true, but for 2.8.11 we end up with isCollectionLike == true which breaks down the line..

I'm not sure if I have found a regression or not.. What do you think?

@cowtowncoder
Copy link
Member Author

Hmmh. This could also be related to the fact that Scala module is not necessarily supporting type refinement (to support collection- / map-like) in the right place: things have changed a little.
So another place to look might be TypeModifier, to ensure it recognizes the need.

New code in this specific place is, I think, (more) correct. But I don't quite understand negative ramifications on Scala side.

Getting CollectionLikeType and MapLikeType right is important, since code in other areas relies on that.

brharrington added a commit to brharrington/jackson-module-scala that referenced this issue Jan 28, 2018
brharrington added a commit to brharrington/jackson-module-scala that referenced this issue Jan 28, 2018
@mbknor
Copy link
Member

mbknor commented Jan 28, 2018

2.8.11 released

@mbknor mbknor closed this as completed Jan 28, 2018
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

No branches or pull requests

3 participants