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

Support for JDK 14 record types #2709

Closed
cowtowncoder opened this issue May 3, 2020 · 15 comments
Closed

Support for JDK 14 record types #2709

cowtowncoder opened this issue May 3, 2020 · 15 comments
Labels
most-wanted Tag to indicate that there is heavy user +1'ing action
Milestone

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented May 3, 2020

(note: created based on FasterXML/jackson-future-ideas#46)

With JDK 14 Java will get Record type of classes. While it appears that they can already be used with right set of configuration options, it would probably make sense to add explicit support to maybe use different auto-detect defaults.
Support should be done in a way that does not require JDK 14 dependency by jackson-databind (that is a leap too far even for 3.0 at least as of May 2020): either so that

  1. If possible, detection simply uses existing JDK functionality (assuming enough features are translated into bytecode accessible using JDK 8 methods), OR
  2. If support does require JDK 14 API, create another module (jackson-module-jdk14 or jackson-datatype-jdk14, depending).

Marking tentatively as 3.0, but may be moved to a 2.x version if and when someone actual works on this.

@cowtowncoder cowtowncoder added the most-wanted Tag to indicate that there is heavy user +1'ing action label May 3, 2020
@gunnarmorling
Copy link

Can Jackson make use of the actual (parameterized) record constructor? That'd definitely be preferable over injecting values into private record fields.

@cowtowncoder
Copy link
Member Author

@gunnarmorling yes that would be optimal: Kotlin module does that for data classes at least, I think Scala module similarly for case classes. That's where detecting Record types would help, to forcible enable use of constructor (and possibly figure out main one, if class definition has multiple alternatives), without changing behavior of POJO types (where there are backwards-compatibility issues, possible security concerns).

@gunnarmorling
Copy link

Re identifying the canonical constructor, this requires a small amout of bespoke logic to collect the component types: https://twitter.com/sormuras/status/1258657911263428608. The biggest challenge I see (without knowing internals of Jackson at all) is that object graphs (records referencing other records referencing other records...) need to be built starting at leaf nodes, so that the inner nodes can be passed to constructors of outer nodes. Is there some logic for that already?

youribonnaffe added a commit to youribonnaffe/jackson-databind that referenced this issue May 9, 2020
First attempt at supporting Java 14 records (JEP 359).

Records are simple DTO/POJO objects with final fields (components) and accessors.
Record's components are automatically serialized and the canonical constructor is
used for deserialization.

Implementation is still compatible with Java 8 and uses a bit of reflection to
access record's components. However the unit tests now require a JDK 14 to run.

The basic idea is to make record's components discovered as properties (similar to
beans having getters) and to make the canonical constructor accessible via implicit
parameter names.
youribonnaffe added a commit to youribonnaffe/jackson-databind that referenced this issue May 9, 2020
First attempt at supporting Java 14 records (JEP 359).

Records are simple DTO/POJO objects with final fields (components) and accessors.
Record's components are automatically serialized and the canonical constructor is
used for deserialization.

Implementation is still compatible with Java 8 and uses a bit of reflection to
access record's components. However the unit tests now require a JDK 14 to run.

The basic idea is to make record's components discovered as properties (similar to
beans having getters) and to make the canonical constructor accessible via implicit
parameter names.
@cowtowncoder
Copy link
Member Author

@gunnarmorling I am not sure I (yet) understand the specific issue, but I suspect answer is no: Jackson simply iterates over constructors it sees and has to determine it in "shallow" manner (deserializers for delegated type are resolved in separate passes to allow for cyclic type dependencies), without much coupling between levels.

Then again, traversing constructors could perhaps be encapsulated without most of databinding being aware of the logic. Still, if there are multiple non-zero-arg constructors that gets tricky and is not a supported use case yet.

@gunnarmorling
Copy link

It might well be there's no problem at all. What I was thinking of was this case:

{
    "name" : "foo",
    "bar" : { "name" : "bar" }
}

As records are immutable, the nested Bar must be instantiated first, so it can be passed when invoking the Foo(String, Bar) constructor.

@youribonnaffe
Copy link
Contributor

Hi @gunnarmorling, is it

record RecordOfRecord(SimpleRecord record) {
the use case you had in mind?

youribonnaffe added a commit to youribonnaffe/jackson-databind that referenced this issue May 10, 2020
First attempt at supporting Java 14 records (JEP 359).

Records are simple DTO/POJO objects with final fields (components) and accessors.
Record's components are automatically serialized and the canonical constructor is
used for deserialization.

Implementation is still compatible with Java 8 and uses a bit of reflection to
access record's components.
A new Maven profile has been introduced to build JDK14 tests using records, those
tests have their own source folder, this way it is still possible to build with
JDK < 14.

The basic idea is to make record's components discovered as properties (similar to
beans having getters) and to make the canonical constructor accessible via implicit
parameter names.
@gunnarmorling
Copy link

@youribonnaffe, yes it is. Nice, seems records are nicely supported than in Jackson already 👍

@jedvardsson
Copy link

Would it be possible to support method local records as well? Currently in BeanDeserializerFactory::isPotentialBeanType there is a check preventing that:

        /* also: can't deserialize some local classes: static are ok; in-method not;
         * other non-static inner classes are ok
         */
        typeStr = ClassUtil.isLocalType(type, true);
        if (typeStr != null) {
            throw new IllegalArgumentException("Cannot deserialize Class "+type.getName()+" (of type "+typeStr+") as a Bean");
        }

@jedvardsson
Copy link

Until official record support here is an annotation introspection that can be used as workaround:

public class JavaRecordAnnotationIntrospector extends NopAnnotationIntrospector {
    public JavaRecordAnnotationIntrospector() {
    }

    @Override
    public String findImplicitPropertyName(AnnotatedMember member) {
        Class<?> declaringClass = member.getDeclaringClass();
        if (declaringClass.isRecord()) {
            if (member instanceof AnnotatedMethod am) {
                RecordComponent[] recordComponents = declaringClass.getRecordComponents();
                for (RecordComponent recordComponent : recordComponents) {
                    if (recordComponent.getAccessor().equals(am.getAnnotated())) {
                        return recordComponent.getName();
                    }
                }
            }
        }
        return null;
    }
}

@youribonnaffe
Copy link
Contributor

Hi @jedvardsson, it looks similar to https://gist.github.com/youribonnaffe/03176be516c0ed06828ccc7d6c1724ce which was one of my first attempt to support it

@cowtowncoder
Copy link
Member Author

Some progress here: was able to make serialization work as expected by introduction of new handler (see #2800) and Record-specific branching with default one.
Now working on deserialization side, trying to figure out a clean way to plug that in -- existing Creator introspection code is horrible and in dire need of rewriting, but I'll have to find a compromise wherein I'll branch Record-specific logic into clean(er) new implementation but will not change non-Record POJO handling. This after digging into code for a while and realizing that neither well-placed hack nor full-scale rewrite will be practical. :)
Hoping to get it wrapped this weekend, either way.

cowtowncoder added a commit that referenced this issue Aug 21, 2020
@cowtowncoder cowtowncoder changed the title Support for record types in JDK 14 Support for JDK 14 record types Aug 21, 2020
@cowtowncoder cowtowncoder added 2.12 and removed 3.x labels Aug 21, 2020
@cowtowncoder cowtowncoder added this to the 2.12.0 milestone Aug 21, 2020
@cowtowncoder
Copy link
Member Author

And there! It is done -- initial implementation at any rate.

If anyone wants to check it out that would be highly appreciated. The highest value contribution would probably be addition of tests in RecordTest under src/test-jdk14/java (same test class or new ones whatever) -- I suspect that there may be edge cases wrt handling of annotations, mostly on deserialization side.

youribonnaffe added a commit to youribonnaffe/jackson-databind that referenced this issue Aug 23, 2020
With the support of Java records, a Maven build profile has been introduced to
Java 14 language and preview features.

This was done in 2.12 branch, just porting it back to master as records are
supported there too.
cowtowncoder pushed a commit that referenced this issue Aug 23, 2020
With the support of Java records, a Maven build profile has been introduced to
Java 14 language and preview features.

This was done in 2.12 branch, just porting it back to master as records are
supported there too.
@cowtowncoder
Copy link
Member Author

Released as part of Jackson 2.12.0.

@vogella
Copy link

vogella commented Dec 1, 2020

Very nice. Is the usage documented somewhere?

@youribonnaffe
Copy link
Contributor

Hello @vogella,
The usage is pretty much what you can expect from a POJO, take a record it will serialize/deserialize out of the box.
There are a few examples in the unit tests: https://github.com/FasterXML/jackson-databind/blob/master/src/test-jdk14/java/com/fasterxml/jackson/databind/RecordTest.java

Do you think it would be worth its own documentation?
I would be ok to help on that. Where would such documentation live? In the README or in https://github.com/FasterXML/jackson-docs?

Also @cowtowncoder published a blog post on the latest release where the feature is mentioned: https://cowtowncoder.medium.com/jackson-2-12-features-eee9456fec75

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
most-wanted Tag to indicate that there is heavy user +1'ing action
Projects
None yet
Development

No branches or pull requests

5 participants