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.

Sign up for GitHub

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

Generate reflection free Jackson deserializers for classes without an empty constructor #45042

Merged
merged 1 commit into from
Dec 11, 2024
+81 −38
Merged

Generate reflection free Jackson deserializers for classes without an empty constructor #45042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 20 additions & 3 deletions .../java/io/quarkus/resteasy/reactive/jackson/deployment/processor/JacksonCodeGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@
import java.util.function.Function;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationTarget;
import org.jboss.jandex.AnnotationValue;
import org.jboss.jandex.ArrayType;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.FieldInfo;
import org.jboss.jandex.IndexView;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.MethodParameterInfo;
import org.jboss.jandex.ParameterizedType;
import org.jboss.jandex.Type;
import org.jboss.jandex.TypeVariable;
Expand Down Expand Up @@ -240,6 +242,10 @@ protected FieldSpecs fieldSpecsFromField(ClassInfo classInfo, FieldInfo fieldInf
return null;
}

protected FieldSpecs fieldSpecsFromFieldParam(MethodParameterInfo paramInfo) {
return new FieldSpecs(paramInfo);
}

protected static class FieldSpecs {

final String fieldName;
Expand All @@ -262,17 +268,28 @@ protected static class FieldSpecs {
FieldSpecs(FieldInfo fieldInfo, MethodInfo methodInfo) {
if (fieldInfo != null) {
this.fieldInfo = fieldInfo;
fieldInfo.annotations().forEach(a -> annotations.put(a.name().toString(), a));
readAnnotations(fieldInfo);
}
if (methodInfo != null) {
this.methodInfo = methodInfo;
methodInfo.annotations().forEach(a -> annotations.put(a.name().toString(), a));
readAnnotations(methodInfo);
}
this.fieldType = fieldType();
this.fieldName = fieldName();
this.jsonName = jsonName();
}

FieldSpecs(MethodParameterInfo paramInfo) {
readAnnotations(paramInfo);
this.fieldType = paramInfo.type();
this.fieldName = paramInfo.name();
this.jsonName = jsonName();
}

private void readAnnotations(AnnotationTarget target) {
target.annotations().forEach(a -> annotations.put(a.name().toString(), a));
}

public boolean isPublicField() {
return fieldInfo != null && Modifier.isPublic(fieldInfo.flags());
}
Expand All @@ -295,7 +312,7 @@ private String jsonName() {
return value.asString();
}
}
return fieldName();
return fieldName;
}

private String fieldName() {
Expand Down
94 changes: 60 additions & 34 deletions ...io/quarkus/resteasy/reactive/jackson/deployment/processor/JacksonDeserializerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,20 +201,20 @@ protected String[] getInterfacesNames(ClassInfo classInfo) {

@Override
protected boolean createSerializationMethod(ClassInfo classInfo, ClassCreator classCreator, String beanClassName) {
if (!classInfo.hasNoArgsConstructor()) {
return false;
}

MethodCreator deserialize = classCreator
.getMethodCreator("deserialize", Object.class, JsonParser.class, DeserializationContext.class)
.setModifiers(ACC_PUBLIC)
.addException(IOException.class)
.addException(JacksonException.class);

ResultHandle deserializedHandle = deserialize
.newInstance(MethodDescriptor.ofConstructor(classInfo.name().toString()));
DeserializationData deserData = new DeserializationData(classInfo, classCreator, deserialize,
getJsonNode(deserialize), parseTypeParameters(classInfo, classCreator), new HashSet<>());
ResultHandle deserializedHandle = createDeserializedObject(deserData);
if (deserializedHandle == null) {
return false;
}

boolean valid = deserializeObject(classInfo, deserializedHandle, classCreator, deserialize);
boolean valid = deserializeObjectFields(deserData, deserializedHandle);
deserialize.returnValue(deserializedHandle);
return valid;
}
Expand All @@ -229,13 +229,35 @@ private static ResultHandle getJsonNode(MethodCreator deserialize) {
return deserialize.checkCast(treeNode, JsonNode.class);
}

private boolean deserializeObject(ClassInfo classInfo, ResultHandle objHandle, ClassCreator classCreator,
MethodCreator deserialize) {
ResultHandle jsonNode = getJsonNode(deserialize);
private ResultHandle createDeserializedObject(DeserializationData deserData) {
if (deserData.classInfo.hasNoArgsConstructor()) {
return deserData.methodCreator.newInstance(MethodDescriptor.ofConstructor(deserData.classInfo.name().toString()));
}

ResultHandle fieldsIterator = deserialize
.invokeVirtualMethod(ofMethod(JsonNode.class, "fields", Iterator.class), jsonNode);
BytecodeCreator loopCreator = deserialize.whileLoop(c -> iteratorHasNext(c, fieldsIterator)).block();
var ctorOpt = deserData.classInfo.constructors().stream().filter(ctor -> Modifier.isPublic(ctor.flags())).findFirst();
if (!ctorOpt.isPresent()) {
return null;
}
MethodInfo ctor = ctorOpt.get();
ResultHandle[] params = new ResultHandle[ctor.parameters().size()];
int i = 0;
for (MethodParameterInfo paramInfo : ctor.parameters()) {
FieldSpecs fieldSpecs = fieldSpecsFromFieldParam(paramInfo);
deserData.constructorFields.add(fieldSpecs.jsonName);
ResultHandle fieldValue = deserData.methodCreator.invokeVirtualMethod(
ofMethod(JsonNode.class, "get", JsonNode.class, String.class), deserData.jsonNode,
deserData.methodCreator.load(fieldSpecs.jsonName));
params[i++] = readValueFromJson(deserData.classCreator, deserData.methodCreator,
deserData.methodCreator.getMethodParam(1), fieldSpecs, deserData.typeParametersIndex, fieldValue);
}
return deserData.methodCreator.newInstance(ctor, params);
}

private boolean deserializeObjectFields(DeserializationData deserData, ResultHandle objHandle) {

ResultHandle fieldsIterator = deserData.methodCreator
.invokeVirtualMethod(ofMethod(JsonNode.class, "fields", Iterator.class), deserData.jsonNode);
BytecodeCreator loopCreator = deserData.methodCreator.whileLoop(c -> iteratorHasNext(c, fieldsIterator)).block();
ResultHandle nextField = loopCreator
.invokeInterfaceMethod(ofMethod(Iterator.class, "next", Object.class), fieldsIterator);
ResultHandle mapEntry = loopCreator.checkCast(nextField, Map.Entry.class);
Expand All @@ -250,8 +272,8 @@ private boolean deserializeObject(ClassInfo classInfo, ResultHandle objHandle, C
.invokeInterfaceMethod(ofMethod(Map.Entry.class, "getKey", Object.class), mapEntry);
Switch.StringSwitch strSwitch = fieldReader.stringSwitch(fieldName);

return deserializeFields(classCreator, classInfo, deserialize.getMethodParam(1), objHandle, fieldValue, new HashSet<>(),
strSwitch, parseTypeParameters(classInfo, classCreator));
return deserializeFields(deserData, deserData.methodCreator.getMethodParam(1), objHandle, fieldValue,
deserData.constructorFields, strSwitch);
}

private BranchResult iteratorHasNext(BytecodeCreator creator, ResultHandle iterator) {
Expand Down Expand Up @@ -294,50 +316,50 @@ private static void createContextualMethod(ClassCreator classCreator) {
createContextual.returnValue(deserializer);
}

private boolean deserializeFields(ClassCreator classCreator, ClassInfo classInfo, ResultHandle deserializationContext,
ResultHandle objHandle, ResultHandle fieldValue, Set<String> deserializedFields, Switch.StringSwitch strSwitch,
Map<String, Integer> typeParametersIndex) {
private boolean deserializeFields(DeserializationData deserData, ResultHandle deserializationContext,
ResultHandle objHandle, ResultHandle fieldValue, Set<String> deserializedFields, Switch.StringSwitch strSwitch) {

AtomicBoolean valid = new AtomicBoolean(true);

for (FieldInfo fieldInfo : classFields(classInfo)) {
if (!deserializeFieldSpecs(classCreator, classInfo, deserializationContext, objHandle, fieldValue,
deserializedFields, strSwitch, typeParametersIndex, fieldSpecsFromField(classInfo, fieldInfo), valid))
for (FieldInfo fieldInfo : classFields(deserData.classInfo)) {
if (!deserializeFieldSpecs(deserData, deserializationContext, objHandle, fieldValue,
deserializedFields, strSwitch, fieldSpecsFromField(deserData.classInfo, fieldInfo), valid))
return false;
}

for (MethodInfo methodInfo : classMethods(classInfo)) {
if (!deserializeFieldSpecs(classCreator, classInfo, deserializationContext, objHandle, fieldValue,
deserializedFields, strSwitch, typeParametersIndex, fieldSpecsFromMethod(methodInfo), valid))
for (MethodInfo methodInfo : classMethods(deserData.classInfo)) {
if (!deserializeFieldSpecs(deserData, deserializationContext, objHandle, fieldValue,
deserializedFields, strSwitch, fieldSpecsFromMethod(methodInfo), valid))
return false;
}

return valid.get();
}

private boolean deserializeFieldSpecs(ClassCreator classCreator, ClassInfo classInfo, ResultHandle deserializationContext,
private boolean deserializeFieldSpecs(DeserializationData deserData, ResultHandle deserializationContext,
ResultHandle objHandle, ResultHandle fieldValue, Set<String> deserializedFields, Switch.StringSwitch strSwitch,
Map<String, Integer> typeParametersIndex, FieldSpecs fieldSpecs, AtomicBoolean valid) {
if (fieldSpecs != null && deserializedFields.add(fieldSpecs.fieldName)) {
FieldSpecs fieldSpecs, AtomicBoolean valid) {
if (fieldSpecs != null && deserializedFields.add(fieldSpecs.jsonName)) {
if (fieldSpecs.hasUnknownAnnotation()) {
return false;
}
strSwitch.caseOf(fieldSpecs.jsonName,
bytecode -> valid.compareAndSet(true, deserializeField(classCreator, classInfo, bytecode, objHandle,
fieldValue, typeParametersIndex, fieldSpecs, deserializationContext)));
bytecode -> valid.compareAndSet(true, deserializeField(deserData, bytecode, objHandle,
fieldValue, fieldSpecs, deserializationContext)));
}
return true;
}

private boolean deserializeField(ClassCreator classCreator, ClassInfo classInfo, BytecodeCreator bytecode,
ResultHandle objHandle, ResultHandle fieldValue, Map<String, Integer> typeParametersIndex, FieldSpecs fieldSpecs,
private boolean deserializeField(DeserializationData deserData, BytecodeCreator bytecode,
ResultHandle objHandle, ResultHandle fieldValue, FieldSpecs fieldSpecs,
ResultHandle deserializationContext) {
ResultHandle valueHandle = readValueFromJson(classCreator, bytecode, deserializationContext, fieldSpecs,
typeParametersIndex, fieldValue);
ResultHandle valueHandle = readValueFromJson(deserData.classCreator, bytecode, deserializationContext, fieldSpecs,
deserData.typeParametersIndex, fieldValue);
if (valueHandle == null) {
return false;
}
writeValueToObject(classInfo, objHandle, fieldSpecs, bytecode, fieldSpecs.toValueWriterHandle(bytecode, valueHandle));
writeValueToObject(deserData.classInfo, objHandle, fieldSpecs, bytecode,
fieldSpecs.toValueWriterHandle(bytecode, valueHandle));
return true;
}

Expand Down Expand Up @@ -445,4 +467,8 @@ private MethodDescriptor readMethodForPrimitiveFields(String typeName) {
protected boolean shouldGenerateCodeFor(ClassInfo classInfo) {
return super.shouldGenerateCodeFor(classInfo) && classInfo.hasNoArgsConstructor();
}

private record DeserializationData(ClassInfo classInfo, ClassCreator classCreator, MethodCreator methodCreator,
ResultHandle jsonNode, Map<String, Integer> typeParametersIndex, Set<String> constructorFields) {
}
}
2 changes: 1 addition & 1 deletion ...rc/test/java/io/quarkus/resteasy/reactive/jackson/deployment/test/SimpleJsonResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public Dog echoDog(Dog dog) {
@POST
@Path("/record-echo")
@Consumes(MediaType.APPLICATION_JSON)
public StateRecord echoDog(StateRecord stateRecord) {
public StateRecord echoRecord(StateRecord stateRecord) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you perhaps missing something from the commit?
Asking because I don't see how this exercises the scenario this change is supposed to address

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method renaming is unrelated, it is just to fix the formerly wrong name that came from an excess of copy/paste :)

This endpoint and the test invoking it are actually exercising this improvement in the sense that returned StateRecord wasn't deserialized through the generated reflection-free deserializer before, since the record structurally doesn't have an no-args constructor. Now it generates the deserializers also for records. The final effect is of course the same (of course in a reflection-free way this time), the only issue is that I don't know how to check that the deserializers is actually generated and it is using it, since the net behaviour is the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, thanks.

I don't have any suggestions for better testing at the moment

return stateRecord;
}

Expand Down
Loading