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

Add support for Java Record deserialization #148

Merged
merged 7 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public class BeanConstructors
protected final Class<?> _valueType;

protected Constructor<?> _noArgsCtor;
protected Constructor<?> _recordCtor;

protected Constructor<?> _intCtor;
protected Constructor<?> _longCtor;
Expand All @@ -27,6 +28,11 @@ public BeanConstructors addNoArgsConstructor(Constructor<?> ctor) {
return this;
}

public BeanConstructors addRecordConstructor(Constructor<?> ctor) {
_recordCtor = ctor;
return this;
}

public BeanConstructors addIntConstructor(Constructor<?> ctor) {
_intCtor = ctor;
return this;
Expand All @@ -46,6 +52,9 @@ public void forceAccess() {
if (_noArgsCtor != null) {
_noArgsCtor.setAccessible(true);
}
if (_recordCtor != null) {
_recordCtor.setAccessible(true);
}
if (_intCtor != null) {
_intCtor.setAccessible(true);
}
Expand All @@ -63,6 +72,13 @@ protected Object create() throws Exception {
}
return _noArgsCtor.newInstance((Object[]) null);
}

protected Object create(Object[] components) throws Exception {
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
if (_recordCtor == null) {
throw new IllegalStateException("Class "+_valueType.getName()+" does not have record constructor to use");
}
return _recordCtor.newInstance(components);
}

protected Object create(String str) throws Exception {
if (_stringCtor == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ private POJODefinition _introspectDefinition(Class<?> beanType,
} else if (argType == Long.class || argType == Long.TYPE) {
constructors.addLongConstructor(ctor);
}
} else if (RecordsHelpers.isRecordConstructor(beanType, ctor, propsByName)) {
constructors.addRecordConstructor(ctor);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,20 @@ public Object read(JSONReader r, JsonParser p) throws IOException
return _constructors.create(p.getLongValue());
case START_OBJECT:
{
if (RecordsHelpers.isRecord(_valueType)) {
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
final List<Object> values = new ArrayList<>();

String propName;
for (; (propName = p.nextFieldName()) != null;) {
BeanPropertyReader prop = findProperty(propName);
if (prop == null) {
handleUnknown(r, p, propName);
continue;
}
values.add(prop.getReader().readNext(r, p));
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
}
return _constructors.create(values.toArray());
}
Object bean = _constructors.create();
String propName;
final Object[] valueBuf = r._setterBuffer;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package com.fasterxml.jackson.jr.ob.impl;

import com.fasterxml.jackson.jr.ob.impl.POJODefinition.PropBuilder;

import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Map;

/**
* Helper class to get Java Record metadata.
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
*/
public final class RecordsHelpers {
private static boolean supportsRecords;

private static Method isRecordMethod;
private static Method getRecordComponentsMethod;
private static Method getTypeMethod;

static {
Method isRecordMethod;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can avoid this one... see note below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method getRecordComponentsMethod;
Method getTypeMethod;

try {
isRecordMethod = Class.class.getMethod("isRecord");
getRecordComponentsMethod = Class.class.getMethod("getRecordComponents");
Class<?> recordComponentClass = Class.forName("java.lang.reflect.RecordComponent");
getTypeMethod = recordComponentClass.getMethod("getType");
supportsRecords = true;
} catch (Throwable t) {
isRecordMethod = null;
getRecordComponentsMethod = null;
getTypeMethod = null;
supportsRecords = false;
}

RecordsHelpers.isRecordMethod = isRecordMethod;
RecordsHelpers.getRecordComponentsMethod = getRecordComponentsMethod;
RecordsHelpers.getTypeMethod = getTypeMethod;
}
private RecordsHelpers() {}

static boolean isRecordConstructor(Class<?> beanClass, Constructor<?> ctor, Map<String, PropBuilder> propsByName) {
if (!supportsRecords || !isRecord(beanClass)) {
return false;
}

Class<?>[] parameterTypes = ctor.getParameterTypes();
if (parameterTypes.length != propsByName.size()) {
return false;
}

try {
Object[] recordComponents = (Object[]) getRecordComponentsMethod.invoke(beanClass);
Class<?>[] componentTypes = new Class<?>[recordComponents.length];
for (int i = 0; i < recordComponents.length; i++) {
Object recordComponent = recordComponents[i];
Class<?> type = (Class<?>) getTypeMethod.invoke(recordComponent);
componentTypes[i] = type;
}

for (int i = 0; i < parameterTypes.length; i++) {
if (parameterTypes[i] != componentTypes[i]) {
return false;
}
}
} catch (IllegalAccessException | InvocationTargetException e) {
return false;
}
return true;
}

public static boolean isRecord(Class<?> clazz) {
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
if (isRecordMethod == null) {
return false;
}

try {
return (boolean) isRecordMethod.invoke(clazz);
} catch (Throwable t) {
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ protected BeanReader _resolveBeanForDeser(Class<?> raw, POJODefinition beanDef)
final Map<String, BeanPropertyReader> propMap;
Map<String, String> aliasMapping = null;

boolean isRecord = RecordsHelpers.isRecord(raw);
if (len == 0) {
propMap = Collections.emptyMap();
} else {
Expand All @@ -462,31 +463,40 @@ protected BeanReader _resolveBeanForDeser(Class<?> raw, POJODefinition beanDef)
final boolean useFields = JSON.Feature.USE_FIELDS.isEnabled(_features);
for (int i = 0; i < len; ++i) {
POJODefinition.Prop rawProp = rawProps.get(i);
Method m = rawProp.setter;
Field f = useFields ? rawProp.field : null;
Method setter = rawProp.setter;
Field field = useFields ? rawProp.field : null;

if (m != null) {
if (setter != null) {
if (forceAccess) {
m.setAccessible(true);
} else if (!Modifier.isPublic(m.getModifiers())) {
setter.setAccessible(true);
} else if (!Modifier.isPublic(setter.getModifiers())) {
// access to non-public setters must be forced to be usable:
m = null;
setter = null;
}
}
// if no setter, field would do as well
if (m == null) {
if (f == null) {
continue;
if (isRecord) {
try {
field = raw.getDeclaredField(rawProp.name);
} catch (NoSuchFieldException e) {
throw new IllegalStateException("Cannot access field " + rawProp.name
+ " of record class " + raw.getName(), e);
}
// fields should always be public, but let's just double-check
if (forceAccess) {
f.setAccessible(true);
} else if (!Modifier.isPublic(f.getModifiers())) {
continue;
} else {
// if no setter, field would do as well
if (setter == null) {
if (field == null) {
continue;
}
// fields should always be public, but let's just double-check
if (forceAccess) {
field.setAccessible(true);
} else if (!Modifier.isPublic(field.getModifiers())) {
continue;
}
}
}

propMap.put(rawProp.name, new BeanPropertyReader(rawProp.name, f, m));
propMap.put(rawProp.name, new BeanPropertyReader(rawProp.name, field, setter));

// 25-Jan-2020, tatu: Aliases are bit different because we can not tie them into
// specific reader instance, due to resolution of cyclic dependencies. Instead,
Expand Down
9 changes: 8 additions & 1 deletion jr-test-module/src/test/java/Java17RecordTest.java
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import com.fasterxml.jackson.jr.ob.JSON;
import com.fasterxml.jackson.jr.ob.JSON.Feature;
import org.junit.Assert;
import org.junit.Test;

Expand All @@ -12,9 +13,15 @@ public class Java17RecordTest {

@Test
public void testJava14RecordSupport() throws IOException {
JSON jsonParser = JSON.builder().enable(Feature.USE_FIELD_MATCHING_GETTERS).build();
var expectedString = "{\"message\":\"MOO\",\"object\":{\"Foo\":\"Bar\"}}";
var json = JSON.builder().enable(JSON.Feature.USE_FIELD_MATCHING_GETTERS).build().asString(new Cow("MOO", Map.of("Foo", "Bar")));
Cow expectedObject = new Cow("MOO", Map.of("Foo", "Bar"));

var json = jsonParser.asString(expectedObject);
Assert.assertEquals(expectedString, json);

Cow object = jsonParser.beanFrom(Cow.class, json);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is some feedback on other variants of input data.

a) different order of fields in JSON than in Java record
input: {"object":{"Foo":"Bar"}, "message":"MOO"}
result: Failed to create an instance of jr.Java17RecordTest$Cow due to (java.lang.IllegalArgumentException): argument type mismatch

This should get fixed.

b) missing field in JSON
input: {"message":"MOO"}
result: Failed to create an instance of jr.Java17RecordTest$Cow due to (java.lang.IllegalArgumentException): wrong number of arguments

I am wondering how deserialization to records should handle missing data as the all-args record constructor requires all fields and if there should also be support for Optional like provided by jackson-modules-java8.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@darxriggs Could you file a separate issue for the problem with ordering? We definitely need more testing here. It can include part about missing values as well (no need for 2 issues).
As to missing values: should pass null, although this will be problematic for many cases like primitives.
These can be addressed incrementally; Optional isn't yet supported for anything else yet so it can be deferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @darxriggs, I've included your feedback in a follow-up: #163

Assert.assertEquals(expectedObject, object);
}

record Cow(String message, Map<String, String> object) {
Expand Down
1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
<module>jr-extension-javatime</module>
<module>jr-test-module</module>
<module>jr-all</module>
<module>jr-records</module>
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
</modules>

<url>https://github.com/FasterXML/jackson-jr</url>
Expand Down
Loading