(datatypes) Support of JavaFX collections

[cowtowncoder]

(moved from existing issue reported by @agonist)

Hi, refering to this issue FasterXML/jackson-databind#718 I'd like to suggest to add support for JavaFX collections since it included in JDK8.

http://docs.oracle.com/javase/8/javafx/api/javafx/beans/property/ListProperty.html http://docs.oracle.com/javase/8/javafx/api/javafx/collections/ObservableList.html http://docs.oracle.com/javase/8/javafx/api/javafx/beans/property/MapProperty.html etc...

[thiagogcm]

Any update on this ?

[Nava2]

After starting on this a few days ago, I would like to keep working on this to make Jackson with JavaFX's properties.

However, there's many hoops to jump through and some design decisions.

• Adding this puts a requirement on using Oracle's JDK on Linux or installing JavaFX separately on most installations (e.g. Ubuntu)
• For properties:
  • Binding to a "bean" requires a lot of back references that need to be dealt with in one of two ways:
    1. Let the constructor of the "bean" handle associating the bean value (i.e. ignore it on properties). This follows the best practices of properties.
    2. Associate beans in such a way as to force them to be bound via a ManagedReference
  • Properties have a "value" but the "value" can be either:
    1. Stored
    2. Bound to another property and read from there This leads to the problem of how to deal with serializing and deserializing. Deserializing is likely best solved via ManagedReference mechanics, but it also forces all Property values to be bound this way (probably doable)
  • The general pattern when working with properties is to create a class like:

    public static class BoundToOther {
    
        private final ObjectProperty<OtherClass> toBind;
    
        public BoundToOther() {
            this.toBind = new SimpleObjectProperty<>(this, "toBind");
        }
    
        public OtherClass getToBind() {
            return toBind.get();
        }
    
        public ObjectProperty<OtherClass> toBindProperty() {
            return toBind;
        }
    
        public void setToBind(OtherClass toBind) {
            this.toBind.set(toBind);
        }
    }
    

    When Jackson tries to read the property information, it will skip toBindProperty() and treat the Property as a OtherClass when it should be specially treated to be a Property<OtherClass> which checks for whether it is bound or stores a value itself (possibly null..). I'm unsure how to handle this in any capacity, I expect an AnnotationIntrospector is required, but documentation on "getting started" is kind of lacking (and example would be great!).
  • ReadOnlyProperty values also need to be special cased in that they can not be "set" but can be read and bound to other Property<> values

I've started some work in my own branch here. Please be aware, it is horribly unpolished and is missing tonnes of documentation. I'll get there. 👍

[cowtowncoder]

@Nava2 Thank you for prototyping here! I am not familiar enough with JavaFX to be of much help, but I can offer one high-level suggestion:

• Due to additional dependencies, this definitely needs to be a separate module, and not part of java8 datatypes. In theory we could try to make it dynamically loadable, but... seems like unnecessary hassle. Users can include this module whenever they need it (and/or frameworks can provide it)

Trying to make it all work does sound complicated, so good luck! I can try to help with specific aspects, but I think regarding AnnotationIntrospector maybe the best way to think of it is that anything done via actual annotations can be achieved by overriding AI methods.

Perhaps one helpful thing could be to document an example case that you would hope to support initially. Simple use case. That might help others (well, me and others) understand specific challenges here.

[Nava2]

• Due to additional dependencies, this definitely needs to be a separate module, and not part of java8 datatypes. In theory we could try to make it dynamically loadable, but... seems like unnecessary hassle. Users can include this module whenever they need it (and/or frameworks can provide it)

It's in a strange place because it is technically part of Java 8 (and OpenJDK 8), but it doesn't install with it by default. Odd, but I think the ideal case would be dynamic loading, though likely not worth the hassle as you said.

but I think regarding AnnotationIntrospector maybe the best way to think of it is that anything done via actual annotations can be achieved by overriding AI methods.

From the docs for AnnotationIntrospector it appears as though implementing an extension of a BeanPropertyDefinition might be the way to property build up the additional information required. Exposing it as a sub-class. From there, installing [de]serializers when a property is found is somewhat trivial. How to auto inject the ID information is another story.

As for a simple use case: https://gist.github.com/Nava2/83a6667a99018c56e506482da04ce509. This shows the assumed Id info that would have to be present and adjusts the serialization (and by proxy deserialization).

[Nava2]

In working on this some more, I wanted to outline what my current plan of attack is for you @cowtowncoder to make sure I'm not off in left field.

1. Extend an AnnotationIntrospector to add some functionality like forcing stored property values to use an ID if one is not specified. For example:

    // in JavaFXAnnotationInspector
    @Override @SuppressWarnings("unchecked")
    protected <A extends Annotation> A _findAnnotation(Annotated annotated, Class<A> annoClass) {
        if (annotated.getType().isTypeOrSubTypeOf(ReadOnlyProperty.class)) { // read-only property
            if (annoClass == JsonIdentityInfo.class) { // someone is asking for ID info
                JsonIdentityInfo fromChildren = super._findAnnotation(annotated, JsonIdentityInfo.class);
   
                if (fromChildren != null) {
                    return (A)fromChildren;
                } else {
                    return (A)new JsonIdentityInfo() {
                        @Override
                        public Class<? extends ObjectIdGenerator<?>> generator() {
                            // Use UUIDs since Integers may clash later ??
                            return ObjectIdGenerators.UUIDGenerator.class;
                        }
   
                        //... snip for brevity, default values for the rest
                    };
                }
            }
        }
        return super._findAnnotation(annotated, annoClass);
    }
   

2. Create an extended ClassIntrospector to add functionality when building the POJO types (e.g. POJOPropertiesCollector and POJOPropertyBuilder). Doing this allows us to:

• Find the xxxProperty() methods and replacing the "getter" reference with the "property version." This should provide the correct semantics if the MergeInfo is correctly specified to extract the type.
• Whether or not this is serialized as a Property<> should depend on the set Visibility and not try to default unless there is an associated ${prop}Property() method that returns a Property<>. This follows nearly the same semantics as a normal property. The naming strategy should be able to be configured, but ${prop}Property() is the standard the JDK uses, so seems a good start.

3. With these new values saved in the POJO types, it should behave as expected when the information is extracted assuming an appropriate ReadOnlyProperty<> [de]serializer is associated and a TypeModifier is installed to correctly discern that it is in-fact a ReferenceType.

I am unaware if this approach will work for the setter in the same way. Ideally, if there is no setter specified, but there is a ${prop}Property method specified, then a setter could be synthetic as Property#setValue(V). However, it appears as though the setter must be an actual Method that is invoked later.

I've also realized that when trying to override behaviour in the POJOPropertiesCollector and the POJOPropertyBuilder much of the "overridable" methods aren't actually very flexible, lacking things like POJOPropertiesCollector#_constructPOJOPropertyBuilder() which would allow a subclass to override the produced value -- instead I have to override the _property(...) method, which is not a big deal, just seems unnecessary. Additionally, there is direct property access in POJOPropertiesCollector where it would be beneficial to use virtual methods: _annotationInspector, _config and _forSerialization. It also creates a lot of down-casting, but I'm not sure using CRTP saves much in terms of thinking here. I was going to open a PR to add these into the databind project. They have ultimately no affect on other code, but allows for an extension point (and is handily convenient for me -- no bias here!).

Lastly, given the situation with separation of javafx from the JDK, should I create a new issue somewhere else to continue this discussion?

[cowtowncoder]

To be honest, I don't really like the complexity of the approach, and that suggest to me that this is not the way to go -- either as approach, or even supporting JavaFX.

My specific concerns are:

• I don't think BeanPropertyDefinition should be sub-classed by anything outside core jackson-databind: it is not designed to be an extension point. Adding some support for additional properties might make sense but mostly if it's more general need, not just by one extension
• Overloading of _findAnnotation is complicated and although method does exist for possible case of re-routing some lookups I think it's risky to traverse inheritance hierarchy

I wish I had something more constructive to offer, but the whole concept of JavaFX seems quite alien to me. It would almost seem better to use simpler JSON that maps to (over?)complicate JavaFX object hierarchy. I'll try reading more about raison d'etre for JavaFX: I think I may be missing a lot about its design philosophy and usage.

[Nava2]

To be honest, I don't really like the complexity of the approach, and that suggest to me that this is not the way to go -- either as approach, or even supporting JavaFX.

It feels "wrong." I've been working on the approach, and it's ugly. There needs to be a way to handle the "binding" of properties. The worst part of that being that the jfx libraries don't actually expose the binding information except if it is "isBound()." This issue has actually been raised to the JavaFX guys here. Nothing moved since 2013, and the "proposed solution" doesn't answer the exact same issue of binding.

Extremely uncomfortably, there is an "observable" field on most of the types (e.g. IntegerProperty, StringProperty, ObjectProperty). This would let us get the binding information, but it is 100% relying on an internal property. I can not think of another way to get that information.

• I don't think BeanPropertyDefinition should be sub-classed by anything outside core jackson-databind: it is not designed to be an extension point. Adding some support for additional properties might make sense but mostly if it's more general need, not just by one extension

I agree, it feels gross, I would rather use "official" means to expose it. But I'm also not sure how to add the extra information via official means. Could the "property" information be added any other way without forcing the use of adding annotations to each component? I was thinking of a TypeResolver the reason being that they're really just reference types wrapping the underlying value. The actual serial/deserialization can be custom implemented via annotation processing.

Edit: The only strange part is that there are two "getter" methods so often times the type will be incorrect as internally, databind uses the normal getter's return type as the type of the field as a priority when in reality it is likely not that value, but an underlying type that appears as that value.

Sorry, I've realized this is a much simpler approach than my initial one -- the joys of learning and over-thinking. 😊 Does this approach sound more sane, @cowtowncoder?

[Nava2]

I did some more on this today and my previous assertions that I was overcomplicating it were definitely true. I've managed to get serialization working as I would expect using only an AnnotationIntrospector and a custom Deserializer for the basic ObjectProperty. Here is an example: https://gist.github.com/Nava2/c166524b17a51a617da90263d53924f0.

This is progress! This will hopefully branch over to the non-object properties (e.g. Integer, Double). I have come across a new problem though, when I serialize the values, I "force" the type to be serialized as the field property (i.e.ObjectProperty<>). This is needed to make sure the value isn't serialized as a value by default. I will enable some annotations to have this turned on/off, but for now it works as is. This leads into an issue in deserializing though, how can I modify the behaviour of the @JsonCreator constructors to extract the value from the ReferenceType if it comes across it?

[cowtowncoder]

No activity in years, doubt this will happen; closing.