Upgrade Option support to use ReferenceType.

[nbauernfeind]

Fixes #248, Supersedes #252.

I accidentally made the other pull request against the master branch, but I actually want this to go into the 2.7.x branch.

[cowtowncoder]

Only realized that I probably should have asked whether you wanted merge to occur (sometimes it makes sense to create PR first for review etc). But I think it's a good step, despite 3 failures for now.

[cowtowncoder]

Another question: should I merge this to master as well? I noticed that master is bit behind, and not yet upgraded to version 2.8.0-SNAPSHOT.

[nbauernfeind]

I can take care of master. I'm actually having second thoughts as to whether or not to merge this into the 2.7.x branch because the serialization format changes for Options under default typing.

I think merging was fine in this case. Of the three tests that are failing two are Either with JsonTypeInfo annotations, which I'll follow up on your questions about whether or not it should be a ReferenceType and were caused by the changes in databind, not from this pull request.

[cowtowncoder]

@nbauernfeind Ok. Yes, wrt 2.7, I did not expect these problems to surface. Would not have suggested change in patch if I had.

Question on Either being ReferenceType or not is interesting. My understanding of it is incomplete, but as it appears to have one of 2 possible values, with distinct types (only one of which exists), I think it is not an exact fit for ReferenceType. Due to dual-typing, it has some similarities with MapLikeType but... not really, at least not fundamentally as map-like things still assume String key.
So probably it should remain SimpleType (not that Simple, but that's just... "anything else" kind of type).

I hope to see how default typing works with AtomicReference and see if I can untangle that part. If so, 2.7.4 could still be an improvement, even if not perfect.

[cowtowncoder]

One more thing: I think one reason for different behavior between Option and AtomicReference, wrt default typing, is that former is abstract while latter is not. As a consequence, rules for default typing differ esp. for basic call of "enableDefaultTyping()", which is an alias for:

enableDefaultTyping(DefaultTyping.OBJECT_AND_NON_CONCRETE);

are different. This is bit unfortunate as it makes it more difficult to verify proper handling of Option and related with AtomicReference.

[nbauernfeind]

You're absolutely right; good catch. This generates the naked string:

case class User(name: String, email: Some[String])

Whereas this generates the wrapping type array:

case class User(name: String, email: Option[String])

[cowtowncoder]

Further along, specifically wrt default typing: I think there may be need to "unwrap" reference types in ObjectMapper (line 222) or so, where applicability of DefaultTyping is determined. Similar to unwrapping array types, I think if (t.isReferenceType()) { t = t.getReferredType(); } would make sense.
That, in turn, leads to re-considering what reference type serializer does with serializeWithType(); I don't think delegation is right at this point -- and this is what produces type id of Optional, instead of type id contents.

[cowtowncoder]

@nbauernfeind I checked in minor improvements (... I hope...) to reference type handling, with respect to detecting type info to include.

[nbauernfeind]

Thanks, I'll take a look later tonight.

[cowtowncoder]

Fwtw, I think I am finally done with changes at least wrt AtomicReference, for 2.7, and managed to get things working with default typing... to degree I think it's doable, and merge changes to 2.8, regarding base reference type (de)serializers. Since no wrapping level is used, one level of TypeSerializer/TypeDeserializers are effectively removed.

Anyway good thing is that I will try my best to avoid making any changes in 2.7 in this area. :)

[cowtowncoder]

@nbauernfeind One question related to this: when do you think Scala module would be ready for 2.7.3?
I can do core components at any point now I think, but don't want to rush in case something else is found wrt Option handling.

[nbauernfeind]

There is one thing I have to sort out: the Either type propagation (which started failing after your databind changes to support Option's type propagation). I have been trying to track down one of the JsonCreator issues, #231. It would be nice to fix at least one of the JsonCreator category of bugs. If I can't tackle it by this Sunday, though, I'm fine letting 2.7.3 release. Also, I doubt that I will find anything else with Option without releasing.

[cowtowncoder]

@nbauernfeind sounds reasonable.

Looking at serializer, deserializer for Either, I think it could benefit from rewrite. Deserializer uses direct reference to BasicDeserializerFactory.instance (which it shouldn't), as a small example, but I think handling of type (de)serializers is probably at level that was used earlier (2.2 or 2.3).