ObjectId does not properly handle forward reference during deserialization

[pgelinas]

While JACKSON-792 did add some support for forward reference, it only handles the case where the object referenced are in "cascade" and where the pass or re-ordering properties (see BeanDeserializerBase#deserializeWithObjectId) ensure that the id property is always read first. However, this isn't the case most of the time, especially when alwaysAsId is used during serialization; resulting json cannot be parsed back unless under very specific conditions. For example (this is actual json generated by Jackson in a test case):

{ "employees" : [ 
     { "id" : 1, "name" : "First", "manager" : null, "reports" : [ 2 ] }, 
     { "id" : 2, "name" : "Second", "manager" : 1, "reports" : [ ] }
] }

Can't be parsed because the "Second" employee isn't inside the json object structure of the first.

I've started to look how to fix this. It's an extremely tricky case, and it gets insane when collection of ids are involved (like the above example). I have some failing test cases ready that I'll attach in a PR shortly. I might need to discuss solution I'm going for here or on dev-list, just to be sure I'm not missing any crucial point. Also, any ideas anyone can provide will help.

[cowtowncoder]

The main problem, as I recall this, was that there was no way to hold on to accessors (SettableBeanProperty) along with instance it operators on, to use once actual object is resolved. Otherwise this would be relatively easy to support. It is also worth noting that many features, such as @JsonCreator (and probably @JsonUnwrapped) may be impossible to properly support (in case of creators, there is the chicken-and-egg problem).
Nonetheless, keeping track of unresolved properties, resolving them incrementally, is the key to making deserialization work with forward references.

Maybe it would be possible to re-check this for 2.4 -- I put evaluation on hold earlier (with 2.2 I think).

[pgelinas]

Yes, there is currently a lack of context when deserializing an Object Id and thus the SettableBeanProperty cannot be used. There might be some way around this though; like with Managed and Back reference, the "link" between the two properties is done with a specific SettableBeanProperty (ManagedReferenceProperty) which is constructed during BeanDeserializerBase#resolve. So it might be that any property that needs to handle an ObjectId will need its own SettableBeanProperty implementation?

A solution to chicken-and-egg might be to create a temporary object when deserializing from an Object Id that isn't resolved, and then update that instance when the object proper is found. This is an approach that I've used in the past, but it was not done in a generic way like Jackson has to deal with, so it might not be applicable. Specifically, if the object doesn't have any setter to use for updating, then it can't be done.

As I said, I'm willing to work on this and perhaps provide an initial implementation. I need this sort of handling for the project I'm currently working on, so instead of writing custom handling for every case I encounter I'd rather fix this directly in Jackson

[cowtowncoder]

Actually 2.3.0 added general-purpose feature of "Attributes", as per:

http://www.cowtowncoder.com/blog/archives/2013/11/entry_483.html

which allows for storing state. This could -- for example -- be a mapping from Object reference to property + POJO combination. So there is now simple contextual per-call storage available.

Use of SettableBeanProperty would be problematic since they are stateless.

[pgelinas]

Thanks, I'll take a look at this.

[pgelinas]

Progress has been made! You can look at the working branch to see how things are going. I used your idea of throwing an exception, it definitely fits the problem. Next up is handling of array deserialization and a check at the end of processing to ensure are forward references are resolved and throw a proper exception if not, probably re-using info from previous exception that were thrown to handle forward reference.

[cowtowncoder]

Great to hear. This is probably the highest-priority problem with existing code.

[pgelinas]

I was able to squeeze more time to work on this. Unit tests (failing) have been added for various configuration which doesn't properly handle forward reference. I've also implemented the check at the end of processing which throw an exception about all unresolved forward reference at that point. It's currently missing information about the type which was expected, but a small modification to jackson-annotation (adding getters to IdKey) will fix that.

So, right now, current handling works for most Collection (NOT ArrayBlockingQueue), most Map (NOT EnumMap), regular POJOs and properties set through @JsonAnySetter. Also, Collection and Map properties which use a defensive copy when setting the property do not currently work (maybe they never will...). Regular array handling also isn't implemented for now. These are all show-cased by failing unit test; I'll tackle them one by one when I get some more time.

[cowtowncoder]

Sounds good so far. Just make sure to locate failing tests under com.fasterxml.jackson.failing.

Since an addition is needed in IdKey, this needs to be in 2.4, but that would most likely be the case anyway due to amount of refactoring (i.e. just to keep risk of patch regression minimal) and this being a fairly major new version.
So before merging in, we need 2.3 branch. Perhaps it'd make sense to release 2.3.1 soon (there are 6 pending fixes for databinding, couple for other components), then open up main for 2.4.