-
Notifications
You must be signed in to change notification settings - Fork 6
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 record types in JDK 14 #46
Comments
Since core jackson-databind 2.x only requires Java 7, it is likely it can not support this natively (I could be wrong as it all depends, but seems likely). But it could perhaps be part of Java 14 support module. I would be interested in knowing bit more about actual implementation of Records, to know what markers there might be to indicate a Record type, and if those can only be detected with JDK 14, or perhaps earlier (similar to how Enums and Annotations, for example, use existing facilities of bytecode to some degree, but JDK only added support in 1.5). |
Hi @cowtowncoder,
I would say it's safe to assume that record type can be detected pre-JDK14 with the given method. |
I wanted to check if records could be deserialized out of the box using Jackson.
This works because annotations are propagated in the generated constructor parameters and Jackson will use it for deserialization. However putting the annotations feels kind of redundant so I looked for a way to not do it. Given my knowledge of Jackson this is probably not covering all cases and features of the library. |
@youribonnaffe Thank you for sharing your findings! At least it is good to first know what workarounds are needed currently. And yes, this is one case where inheritance of constructor property arguments comes in pretty handy. Glad I implemented it in the first place (mostly for mix-in annotation use case), despite it being quite a bit of extra work back in the day. :-) |
@Data
public class Person {
@JsonProperty("id")
private int id;
private String name;
private List<String> alias;
@JsonSetter("alias")
public void setAlias(String aliasStr) {
alias = Arrays.asList(aliasStr.split(","));
}
public static void main(String[] args) throws IOException {
ObjectMapper objectMapper = new ObjectMapper();
String jsonStr = """
{
"id":1,
"name":"name1",
"alias": "alias1,alias2"
}
""";
Person person = objectMapper.readValue(jsonStr, Person.class);
System.out.println(person);
}
}
But with @JsonIgnoreProperties(ignoreUnknown = true)
public record Person(
Integer id,
String name,
// if this field have custom converter, JsonProperty is required
@JsonProperty(value = "alias", access = JsonProperty.Access.WRITE_ONLY)
@JsonDeserialize(converter = AliasConverter.class)
List<String>alias,
@JsonProperty(value = "password", access = JsonProperty.Access.WRITE_ONLY)
String password
) {
@JsonGetter("alias")
public String jsonGetAlias() {
return String.join(",", alias);
}
public static void main(String[] args) throws IOException {
ObjectMapper objectMapper = new ObjectMapper();
// get default propertyName from record
// thanks youribonnaffe
JacksonAnnotationIntrospector implicitRecordAI = new JacksonAnnotationIntrospector() {
@Override
public String findImplicitPropertyName(AnnotatedMember m) {
if (m.getDeclaringClass().isRecord()) {
if (m instanceof AnnotatedParameter parameter) {
return m.getDeclaringClass().getRecordComponents()[parameter.getIndex()].getName();
}
if (m instanceof AnnotatedMember member) {
for (RecordComponent recordComponent : m.getDeclaringClass().getRecordComponents()) {
if (recordComponent.getName().equals(member.getName())) {
return member.getName();
}
}
}
}
return super.findImplicitPropertyName(m);
}
};
objectMapper.setAnnotationIntrospector(implicitRecordAI);
String jsonStr = """
{
"id":1,
"name":"name1",
"alias": "alias1,alias2",
"password": "password1"
}
""";
Person person = objectMapper.readValue(jsonStr, Person.class);
System.out.println(person);
// Person[id=1, name=name1, alias=[alias1, alias2], password=password1]
String jsonOut = objectMapper.writeValueAsString(person);
System.out.println(jsonOut);
// {"alias":"alias1,alias2","id":1,"name":"name1"}
}
/**
* convert String to List, just for field alias
* <p>
* this is class-bound logic, so keep it internal
*/
static class AliasConverter extends StdConverter<String, List<String>> {
@Override
public List<String> convert(String value) {
return Arrays.stream(value.split(",")).collect(Collectors.toList());
}
}
} |
Thanks for the feedback. I tried it and it seems that the change you did in the JacksonAnnotationIntrospector is not required for it work. Using @JsonProperty probably makes Jackson use the private fields instead of the record's constructor. I debugged a little and found this approach : https://gist.github.com/youribonnaffe/03176be516c0ed06828ccc7d6c1724ce (same Gist, updated). It feels more hackish than the first version. |
also, just wanted to mention that this lack of support for new format of getter methods makes using record types with springboot fail by default during json serialization with an obscure error like below (which can be tracked down to fasterxml/jackson) -
the error goes away after adding annotation mentioned in the description
|
Hi, Caused by: java.lang.RuntimeException: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `com.suspendfun.CsvRecord` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('name')
at [Source: (com.fasterxml.jackson.dataformat.csv.impl.UTF8Reader); line: 1, column: 1]
|
Hi, There is one difference between the CsvService and the unit test when the CsvMapper is created :
Adding those in CsvService fixes the context initialization exception, then adding Defining the columns in the schema probably helps Jackson to use the constructor (to be checked). |
Thanks @youribonnaffe for the quick code fix, explanation of why it (possibly) works (and sorry about the missing code part that adds the columns). About the last part, are you aware if the support for Records is being tracked somewhere else? |
I don't think there is an existing issue yet, so I will go ahead and create one for |
Created: FasterXML/jackson-databind#2709 |
@cowtowncoder I'll be happy to help on that. Not sure how to get started though as the code base of Jackson is quite big :). |
@youribonnaffe Yes, understandably it is not easy to know where to get started. One of first things would probably just be that of figuring out if a class is Record, similar to how Enums are detected; and in particular if it was possible to do so with JDK 8 only accessors of |
@cowtowncoder I think you can assume that any class that subclasses In fact, I just checked the source-code for
I don't know what |
@cowwoc That would be simply enough then, from JDK8: just try to load If |
In terms of features, what should be offered out of the box? |
@youribonnaffe For deserialization it pretty much works (when you have more than one field in record, if you do then I need to add @JsonProperty, pretty annoying but workable) when you use I use jackson with records like that and I'm pretty happy with it, it is basically just like using a immutable class with all fields initialized with a single constructor. |
@youribonnaffe That sounds reasonable to me (although I haven't tried using Records so I don't know if there are some caveats). Another thing this might be related to are various similar types like Scala case classes, Kotlin data classes, where it seems that definition of properties is driven directly by field definitions. And Jackson databind could perhaps offer little more core-support for modules, to help with some issues regarding property naming (basically, start by naming convention derived from field, match accessors, if any; this results in better mapping than Bean-style attempt to start from accessor). |
Another workaround is to use com.fasterxml.jackson.annotation.JsonCreator |
Another update on records: https://openjdk.java.net/jeps/384 |
Hi, I work on the JDK, and implemented the Serializable record support in Java 14. I would love to help out with this effort, in any way that I can. I can provide as much details as you would like, but essentially Java serialization can use a record's accessors to extract the values of its components for serializing. On the deserializing side, the record's canonical constructor is invoked to instantiate the record object - the constructor is passed the appropriate component values. From a runtime reflection point of view, Java 14 added two specific methods to support records:
It is with these two primitives that the whole Java Serializable record support is built upon. It is worth noting that starting in Java 15, core reflection will not be able to mutate the component fields of a record object. Construction should proceed through the canonical constructor. Again, I would like to help here in any capacity that I can, so that Jackson works out-of-the-box with records. Yes, even local records, maybe ;-) |
The safest corse of action would be to reflectively determine if the Class::isRecord method is present, and invoke it reflectively if it is. If the method is present and returns true then the class is a record, otherwise it is not - you cannot have records on JDK's where this method is not present. |
@ChrisHegarty excellent, help would be much appreciated! One thing that I would really like to do but don't have time to right now is to create simple test repo(s) to test handling of new JDK features, regardless of whether a separate module is needed or not.
but in this case could have JDK 14 compatibility tests (I assume for the most part LTS would benefit). Alternatively if it's easy enough to do, adding optional (only run for certain profile) Maven tests for existing repos could also work. Now as to support: I suspect that since detection of But what if.... what if there was new |
If the compilation, execution, command line args, of such tests can be conditional on the JDK version or some such, then that would be easiest. The tests could then be merged into the regular non-optional profile later. But a separate repo would be ok too, just maybe a little more awkward to manage?
A record can have more than one constructor, but there is always a single canonical constructor, that all other overloaded constructors will eventually delegate to. To find the canonical constructor one can do:
For example, in Java Serialization:
I'm not quite sure what would be best: 1) JavaType::isRecord, or 2) a subtype of JavaType. I guess it depends where is best to add the serializer and deserializer specific logic for records. Maybe start with the least invasive change and see how far we can get with that. I think it might be most straight forward to just get things working on JDK 14 / 15, and then see how/where the Java 14+ specific code would need to live and how best to make it optional ( so as to not require JDK 14+ for compilation or runtime ). |
In this PR FasterXML/jackson-databind#2714 I used a Maven profile activated with Java 14 and a separate test source folder to use records. The changes in Jackson don't use any API from Java 14 and we can still write tests with records. The code you suggested @ChrisHegarty is very similar to https:/FasterXML/jackson-databind/blob/a1b9a76491ddba2994c65457ba4b6e244a146b40/src/main/java/com/fasterxml/jackson/databind/util/RecordUtil.java#L52 |
Thanks @youribonnaffe, I'll take a look at that PR, and post comments there. |
I think this comes down to the amount of customisation that you want to allow for records, and then how that surfaces in the API. For example, Java Serialization of records allows no customisation of the serial-form of the record, i.e. the serial-form ( the serializable fields ) is that of the record components, nothing more and nothing less. This could be one choice, or alternatively records could be viewed as just another shape of bean; getters without the get prefix, no setters, and a single findable canonical constructor, and allowing much (all?) of the customisations of any other ordinary class. |
Update to my previous comment. I see @youribonnaffe has a test that uses |
At this point there is probably not too much need to expose Record-ness, I agree. I added some minor comments to PR, but my main suggestion is just to separate introspection of the canonical constructor into bit separate code path (I can help with this), so that while introspection of annotations user can use (for renaming, custom (de)serializers) can and should shared code, all the logic for figuring out set of Creators can be avoided as logic for finding one should be straight-forward -- find components, then find constructor that matches it. I hope to find time to follow up on this with less delay, now that I think I understand the constraints. |
This is part of 2.12 now! I guess the issue can be closed @cowtowncoder. |
Thanks! Yes indeed! Anyone seeing the update, feedback on implementation (as in, testing, verifying it works) would be super helpful before 2.12.0 release (first release candidate hopefully out during September 2020). |
Add support for record classes (data classes) from JDK 14 Feature Preview. https://cr.openjdk.java.net/~briangoetz/amber/datum.html
Right now the out-of-the-box experience while using records in jackson is the following:
Serializing a simple record with no extra methods:
results in a "no properties" exception:
com.fasterxml.jackson.databind.exc.InvalidDefinitionException: No serializer found for class com.company.TestRecord and no properties discovered to create BeanSerializer
A
@JsonAutoDetect
annotation can be use to solve the problem:results in a correct
{"x":1,"y":2,"z":3}
outputSo right now it works with a little bit of work, but it would be great if there would be a correct out-of-the-box experience with record classes.
The text was updated successfully, but these errors were encountered: