fix 3.0.0-SNAPSHOT build issues #431
Comments
|
Yeah, this will require quite a bit of work, likely. Let me know if I can help: Kotlin module has some challenges too, but I did help with some changes there. |
|
@pjfanning One question/suggestion, more related to 2.12 (but eventually to 3.0/master too): I created this project: https://github.com/FasterXML/jackson-integration-tests to allow basic sanity/smoke tests; something that would run simple tests to ensure that a set (like 2.12.0-SNAPSHOT, 3.0.0-SNAPSHOT) of components in their latest publish snapshots versions still works. I have added tests for dataformats, some of datatypes (joda, guava), as well as certain combinations (Joda + XML/CSV/Properties wrt timestamp values). I am not sure exactly how, but I wonder if there was a way to invoke some Scala mapper sanity tests from Java, built by Maven, to do simplest of validations that a scala-supporting object mapper can be constructed and used on something that exercises its functionality? At any rate, I would really like to quickly find out if changes I am making break Scala module: I just integrated PR for FasterXML/jackson-databind#1296 and doing refactoring for it. I will try to keep things backwards compatible for 2.12 obviously, but sometimes there are dependencies I do not see (Scala and Kotlin typically, although of course there are many 3rd party extensions beyond this). And with 3.x I will simply remove deprecated methods so there is more work anyway. |
|
I'm not sure - all that can be done is to copy lots of the code and tests from jackson-module-scala into jackson-integration-tests - I'd be reluctant to maintain 2 copies of the code. I have nightly builds running jackson-module-scala on travis but recently my travis account has had a problem that doesn't let me set up new builds for github orgs that I am member of. I need to either follow up with travis or try something else like github actions. I'm afraid master version of jackson-module-scala was well broken before I even had a chance to set up builds. I have it compiling and tests running but many are broken - many tests are probably broken for similar reasons. |
|
@cowtowncoder a lot of the broken tests appear to be because the DeserializerResolvers in master do not appear to have calls made to findReferenceDeserializer - only to findBeanDeserializer. Tests that pass in v2.12 and that fail in master have lots of calls to findReferenceDeserializer in 2.12 but none in master. |
|
@pjfanning Quick clarification wrt |
|
@pjfanning On I would guess 2 likely sources of issues:
Would it be possible to test to see if calls expected to go to |
|
I'm not sure when I'll next have chance to spend time on this. With reference types, wouldn't java.util.Optional be a reference type (akin to scala.Option and the master tests are failing for scala.Option)? |
|
Yes, |
|
In 2.12, the reference type deserializer is created in this stack - this does not happen in master, despite the same ReferenceType being in scope. |
|
I found what was breaking the scala Either tests on master but quite a lot more stuff to fix still. @cowtowncoder could you look at FasterXML/jackson-module-jsonSchema#140 ? |
|
@pjfanning Ok good wrt Either tests. Added a note on json-schema module: at this point I have no plans to support it for 3.x, will be end-of-life'd (not much work on 2.x either). |
|
@cowtowncoder another set of tests that have stopped working in master relate to the scala BigDecimal and BigInteger classes which are different from the Java equivalents. The deserializer is registered as it was for Jackson 2.x but Jackson no longer seems to try to use the deserializer. I added an implementation of hasDeserializerFor - which wasn't needed for Jackson 2.x - but this method is not called. There are some unit tests and when I run them, I can't find any calls being made to the ScalaNumberDeserializersModule. Any ideas? |
|
Ok, first |
|
@pjfanning Odd. Registration code looks perfectly fine... one thing to check might be to see if for some reason other Does the test result in a I assume other custom deserializers work ok, as Scala module has a few, right? I wonder if registration code differs somehow. |
|
A high fraction of the custom scala deserializers are broken. I'm just working through 1 at a time. |
|
In 2.12, this code finds the right deserlializer (BasicDeserializerFactory) But in master, the factoryConfig.deserializers() appears to be empty - same test run in 2.12 and the factoryConfig.deserializers() has loads of deserializers, including all the custom scala ones. |
|
It appears to be something in the ScalaObjectMapper class - a standard JsonMapper works ok in the scala tests but the ScalaObjectMapper mixin doesn't work properly |
|
I've fixed the issue in ScalaObjectMapper - that fixes 50 tests but still about 30 broken or yet to be rewritten. |
|
Well that is good progress! |
|
@cowtowncoder one of the issues that is happening in master branch is that deserialization is running invalid JSON in one case - jackson-module-scala does not actually create the json result and is just trying the help the core jackson code by providing class metadata. There are other issues affecting this test case on the Scala module side but could you check to see why the core code produces invalid JSON? |
|
@pjfanning If I can figure out how to run Scala module tests on Eclipse, can try to help. But which tests in particular has this issue? |
|
This issue appears in com.fasterxml.jackson.module.scala.deser.ListMapTest |
|
Ok at least I can run |
|
If you just run You can run |
|
@pjfanning Ok so So do you know if calls to specific method are not being made, or more generally? There have been some changes to signatures of methods too (to always pass |
|
Jackson 2 logging (with additional stacktaces) - the 2nd call never happens in Jackson 3. So for some reason, jackson-databind neglects to get the scala class information for the MetricPath class. And then, jackson 3 is unable to deserialize any json for Metrics because it doesn't know how deserialize MetricPaths. For some reason, Jackson 3 does not properly analyse classes that are the types of object members. |
|
Jackson 3 seems to do value instantiation first |
|
So the "missing" method would be |
|
findImplicitPropertyName is implemented in the ScalaAnnotationIntrospector and gets call correctly for Metric but not for the MetricPath class. I've been trying to step though jackson-databind but the jackson-databind code is not something I really understand. I can see the jackson-databind code does look at the properties of Metric class including the property with MetricPath type but gives up and never calls findImplicitPropertyName for the MetricPath class. Is it possible that jackson 3 requires ScalaAnnotationIntrospector to implement more methods? - right now it is only overrides a small number of methods on NopAnnotationIntrospector |
|
Ok. It is possible other things might be needed. But another thing is that introspection may also be little bit asynchronous, in that order in which things go may be different. |
|
POJOPropertiesCollector never gets instantiated for MetricPath (in jackson 3). It does for Metric class. I'm debugging code in IntelliJ - it is easy to get IntelliJ to debug the jackson-module-scala tests - it's just that I simply do not understand the code in jackson-databind, so I do not know what to look for in order to explain why jackson 3 and not jackson 2 refuses to introspect the MetricPath class. jackson-module-scala is unusable if it cannot handle classes like Metric. |
|
@pjfanning Is there something special about Too bad |
|
There is nothing special about Metric - it is as plain a class as you could possibly have in Scala. _createAndCache2 in DeserializerCache - this code in Jackson2 works perfectly - The resolve on BeanDeserializerBase walks the object graph - picking up all the type information for the class and the classes of its members. The new code in jackson 3 does not even try to read the class information for the classes of the members. |
|
@pjfanning Right I would be surprised if resolution did not proceed. What I am trying to think of is where between locating a deserialization and introspection properties things get derailed. What is the fail message again? Presumable a deserializer (but not functioning one) gets created or... ? |
|
in jackson 3 BeanDeserializerFactory addBeanProps, the code just simply fails to get the creator property for the 'path' property that had MetricPath type |
|
@cowtowncoder test fails with #431 (comment) |
|
Ok, perhaps it'd be simpler to troubleshoot direct deserialization of |
|
Oh, wait, perhaps not. If the inner type ( If so it would be due to missing linkage when resolving pieces of At least if I am understanding the situation better now. |
|
MetricPath deserializes ok if it is deserialized by itself - the issue is when it is the type of a member of another class. |
|
Ok that makes sense. The problem seems to be that introspection does not find creator property for some reason. Not At this point, would it make sense to create a new issue for this one problem. Not sure if there are other ones, but discussion here has gotten bit long and covers a few different issues, most resolved? |
|
I found some code in the Scala annotation classes that is not behaving as it does in Jackson 2 - let me investigate this - it could be the cause of the issue here |
|
@cowtowncoder a possible/probable cause is that the jackson-module-scala was modified in jackson 3 because the interface in jackson-databind changed. ValueInstantiators findValueInstantiator(DeserializationConfig var1, BeanDescription var2, ValueInstantiator var3) has dropped the final param - the defaultValueInstantiator The scala code now struggles to replace that default. Any suggestions as to how to get a default instantiator in jackson 3? |
|
@pjfanning Let me see, I think this was just split in two parts or so. Ok, yes; so, there is new |
|
I have no value instantiator so can't modify it. Is there any way to create a new default instantiator that has all the creator properties calculated - that's what I get in jackson 2 - automatically but now I am left with no instantiator and need to create from scratch . findValueInstantiator(config: DeserializationConfig, ...) -- because this method does not give access to a full DeserializationContext, it is hard to try BeanDeserializerBuilder to try to get a value instantiator from it |
|
Please look at signature: you are given the default instantiator UNLESS earlier call to already returned a So basically earlier single call was split in two parts: one to ask for instantiator, and if none given, a default POJO instantiator is created and passed to give another chance for custom variants. So if you want to use that default instantiator it is given and you may return it as-is, or create different one based on it (or just ignore). |
I have no defaultInstantiator to pass in |
|
Ok now I am confused. This is a callback method of If Scala module does need to call it, where is the code? Maybe I can help with it. |
|
In jackson 2, the perfect value instantiator is created in BasicDeserializer.findValueInstantiator -- and specifically when this is called This instantiator is then used when the call to the old |
|
I think I fixed it - I made the findValueInstantiator return null so now the modifyValueInstantiator is called with the instantiator created in the BasicDeserializer |
|
We're now down to 2 or 3 commented out tests in master that pass in jackson v2 and that fail in v3 - today's fix fixed about 5 broken tests |
|
@pjfanning Ok good, yes, Getting very close to parity which is good! |
|
created #536 for follow on work |
Compilation fails due to major changes in jackson-databind
https://travis-ci.org/FasterXML/jackson-module-scala/jobs/588055923
The text was updated successfully, but these errors were encountered: