Skip to content

Commit

Permalink
Fix the non-default constructor mechanism of bytecode recording
Browse files Browse the repository at this point in the history
Bytecode recorders attempt to match constructor parameters to properties
and skip setting properties that were initialized by the constructor.
The assumption is that if the constructor has a parameter with the same
name as the property, the property is set by the constructor. Otherwise
it needs to be set using a setter.

This matching was not implemented for the non-default constructor mechanism.
This commit fixes that.
  • Loading branch information
Ladicek authored and gastaldi committed Feb 16, 2023
1 parent 4196737 commit c94ae9a
Showing 1 changed file with 45 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public class BytecodeRecorderImpl implements RecorderContext {

private final List<ObjectLoader> loaders = new ArrayList<>();
private final Map<Class<?>, ConstantHolder<?>> constants = new HashMap<>();
private final Set<Class> classesToUseRecorableConstructor = new HashSet<>();
private final Set<Class> classesToUseRecordableConstructor = new HashSet<>();
private final boolean useIdentityComparison;

/**
Expand Down Expand Up @@ -394,7 +394,7 @@ public void run() {
}

public void markClassAsConstructorRecordable(Class<?> clazz) {
classesToUseRecorableConstructor.add(clazz);
classesToUseRecordableConstructor.add(clazz);
}

private ProxyInstance getProxyInstance(Class<?> returnType) throws InstantiationException, IllegalAccessException {
Expand Down Expand Up @@ -1173,7 +1173,14 @@ public void prepare(MethodContext context) {
nonDefaultConstructorHandles[i] = loadObjectInstance(obj, existing,
parameterTypes[count++], relaxedValidation);
}
} else if (classesToUseRecorableConstructor.contains(param.getClass())) {
if (nonDefaultConstructorHolder.constructor.getParameterCount() > 0) {
Parameter[] parameters = nonDefaultConstructorHolder.constructor.getParameters();
for (int i = 0; i < parameters.length; ++i) {
String name = parameters[i].getName();
constructorParamNameMap.put(name, i);
}
}
} else if (classesToUseRecordableConstructor.contains(param.getClass())) {
Constructor<?> current = null;
int count = 0;
for (var c : param.getClass().getConstructors()) {
Expand All @@ -1191,27 +1198,36 @@ public void prepare(MethodContext context) {
nonDefaultConstructorHandles = new DeferredParameter[current.getParameterCount()];
if (current.getParameterCount() > 0) {
Parameter[] parameters = current.getParameters();
for (int i = 0; i < current.getParameterCount(); ++i) {
for (int i = 0; i < parameters.length; ++i) {
String name = parameters[i].getName();
constructorParamNameMap.put(name, i);
}
}
} else {
for (Constructor<?> ctor : param.getClass().getConstructors()) {
Constructor<?>[] ctors = param.getClass().getConstructors();
Constructor<?> selectedCtor = null;
if (ctors.length == 1) {
// if there is a single constructor we use it regardless of the presence of @RecordableConstructor annotation
selectedCtor = ctors[0];
}
for (Constructor<?> ctor : ctors) {
if (ctor.isAnnotationPresent(RecordableConstructor.class)) {
nonDefaultConstructorHolder = new NonDefaultConstructorHolder(ctor, null);
nonDefaultConstructorHandles = new DeferredParameter[ctor.getParameterCount()];

if (ctor.getParameterCount() > 0) {
Parameter[] ctorParameters = ctor.getParameters();
for (int i = 0; i < ctor.getParameterCount(); ++i) {
String name = ctorParameters[i].getName();
constructorParamNameMap.put(name, i);
}
}
selectedCtor = ctor;
break;
}
}
if (selectedCtor != null) {
nonDefaultConstructorHolder = new NonDefaultConstructorHolder(selectedCtor, null);
nonDefaultConstructorHandles = new DeferredParameter[selectedCtor.getParameterCount()];

if (selectedCtor.getParameterCount() > 0) {
Parameter[] ctorParameters = selectedCtor.getParameters();
for (int i = 0; i < ctorParameters.length; ++i) {
String name = ctorParameters[i].getName();
constructorParamNameMap.put(name, i);
}
}
}
}

Set<String> handledProperties = new HashSet<>();
Expand All @@ -1224,7 +1240,8 @@ public void prepare(MethodContext context) {
}
// check if the matching field is ignored
try {
if (param.getClass().getDeclaredField(i.getName()).getAnnotation(IgnoreProperty.class) != null) {
Field field = param.getClass().getDeclaredField(i.getName());
if (ignoreField(field)) {
continue;
}
} catch (NoSuchFieldException ignored) {
Expand Down Expand Up @@ -1414,7 +1431,7 @@ public void prepare(MethodContext context) {
//now handle accessible fields
for (Field field : param.getClass().getFields()) {
// check if the field is ignored
if (field.getAnnotation(IgnoreProperty.class) != null) {
if (ignoreField(field)) {
continue;
}
if (!handledProperties.contains(field.getName())) {
Expand Down Expand Up @@ -1551,6 +1568,16 @@ ResultHandle createValue(MethodContext context, MethodCreator method, ResultHand
};
}

/**
* Returns {@code true} iff the field is annotated {@link IgnoreProperty} or the field is marked as {@code transient}
*/
private static boolean ignoreField(Field field) {
if (Modifier.isTransient(field.getModifiers())) {
return true;
}
return field.getAnnotation(IgnoreProperty.class) != null;
}

private DeferredParameter findLoaded(final Object param) {
for (ObjectLoader loader : loaders) {
if (loader.canHandleObject(param, staticInit)) {
Expand Down Expand Up @@ -1817,7 +1844,7 @@ ResultHandle doLoad(MethodContext context, MethodCreator valueMethod, ResultHand
retValue = valueMethod.load(value.asChar());
break;
case CLASS:
retValue = valueMethod.loadClassFromTCCL(value.asClass().toString());
retValue = valueMethod.loadClassFromTCCL(value.asClass().name().toString());
break;
case ARRAY:
retValue = arrayValue(value, valueMethod, method, annotationClass);
Expand Down

0 comments on commit c94ae9a

Please sign in to comment.