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

Allow recording of immutable objects #10736

Merged
merged 1 commit into from
Jul 28, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions build-parent/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,12 @@
<artifactId>maven-resources-plugin</artifactId>
<version>3.1.0</version>
</plugin>
<plugin>
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this needed for some reason?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, so the way the immutable object recording works is that it maps field names to constructor parameter names. To actually record these names you need to use the -parameters javac option. Otherwise you would need annotations to specify the parameter names.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

I was just wondering about the placement of this configuration (I did the review in the IDE and it seemed to not have updated the comment), not the functionality itself.

<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<parameters>true</parameters>
</configuration>
</plugin>
<plugin>
<groupId>com.github.alexcojocaru</groupId>
<artifactId>elasticsearch-maven-plugin</artifactId>
Expand Down Expand Up @@ -508,6 +514,7 @@
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<release>8</release>
<parameters>true</parameters>
</configuration>
</plugin>
</plugins>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@
*
* If this is {@link ExecutionTime#RUNTIME_INIT} then it will run from a main method at application
* start.
*
* There are some limitations on what can be recorded. Only the following objects are allowed as parameters to
* recording proxies:
* <p>
* - primitives
* - String
* - Class
* - Objects returned from a previous recorder invocation
* - Objects with a no-arg constructor and getter/setters for all properties (or public fields)
* - Objects with a constructor annotated with @RecordableConstructor with parameter names that match field names
* - Any arbitrary object via the
* {@link io.quarkus.deployment.recording.RecorderContext#registerSubstitution(Class, Class, Class)} mechanism
* - arrays, lists and maps of the above
*/
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.net.URL;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -61,6 +62,7 @@
import io.quarkus.runtime.RuntimeValue;
import io.quarkus.runtime.StartupContext;
import io.quarkus.runtime.StartupTask;
import io.quarkus.runtime.annotations.RecordableConstructor;

/**
* A class that can be used to record invocations to bytecode so they can be replayed later. This is done through the
Expand All @@ -78,7 +80,9 @@
* - primitives
* - String
* - Class
* - Objects with a no-arg constructor and getter/setters for all properties
* - Objects returned from a previous recorder invocation
* - Objects with a no-arg constructor and getter/setters for all properties (or public fields)
* - Objects with a constructor annotated with @RecordableConstructor with parameter names that match field names
* - Any arbitrary object via the {@link #registerSubstitution(Class, Class, Class)} mechanism
* - arrays, lists and maps of the above
*/
Expand Down Expand Up @@ -1046,11 +1050,13 @@ ResultHandle doLoad(MethodContext context, MethodCreator method, ResultHandle ar
* @param expectedType The expected type of the object
* @return
*/
private DeferredParameter loadComplexObject(Object param, Map<Object, DeferredParameter> existing, Class<?> expectedType) {
private DeferredParameter loadComplexObject(Object param, Map<Object, DeferredParameter> existing,
Class<?> expectedType) {
//a list of steps that are performed on the object after it has been created
//we need to create all these first, to ensure the required objects have already
//been deserialized
List<SerialzationStep> setupSteps = new ArrayList<>();
List<SerialzationStep> ctorSetupSteps = new ArrayList<>();

if (param instanceof Collection) {
//if this is a collection we want to serialize every element
Expand Down Expand Up @@ -1093,10 +1099,47 @@ public void prepare(MethodContext context) {
});
}
}

//check how the object is constructed
NonDefaultConstructorHolder nonDefaultConstructorHolder = null;
DeferredParameter[] nonDefaultConstructorHandles = null;
//used to resolve the parameter position for @RecordableConstructor
Map<String, Integer> constructorParamNameMap = new HashMap<>();

if (nonDefaultConstructors.containsKey(param.getClass())) {
nonDefaultConstructorHolder = nonDefaultConstructors.get(param.getClass());
List<Object> params = nonDefaultConstructorHolder.paramGenerator.apply(param);
if (params.size() != nonDefaultConstructorHolder.constructor.getParameterCount()) {
throw new RuntimeException("Unable to serialize " + param
+ " as the wrong number of parameters were generated for "
+ nonDefaultConstructorHolder.constructor);
}
int count = 0;
nonDefaultConstructorHandles = new DeferredParameter[params.size()];
for (int i = 0; i < params.size(); i++) {
Object obj = params.get(i);
nonDefaultConstructorHandles[i] = loadObjectInstance(obj, existing,
nonDefaultConstructorHolder.constructor.getParameterTypes()[count++]);
}
} else {
for (Constructor<?> ctor : param.getClass().getConstructors()) {
if (ctor.isAnnotationPresent(RecordableConstructor.class)) {
nonDefaultConstructorHolder = new NonDefaultConstructorHolder(ctor, null);
nonDefaultConstructorHandles = new DeferredParameter[ctor.getParameterCount()];
for (int i = 0; i < ctor.getParameterCount(); ++i) {
String name = ctor.getParameters()[i].getName();
constructorParamNameMap.put(name, i);
}
break;
}
}
}

Set<String> handledProperties = new HashSet<>();
Property[] desc = PropertyUtils.getPropertyDescriptors(param);
for (Property i : desc) {
if (i.getReadMethod() != null && i.getWriteMethod() == null) {
Integer ctorParamIndex = constructorParamNameMap.remove(i.name);
if (i.getReadMethod() != null && i.getWriteMethod() == null && ctorParamIndex == null) {
try {
//read only prop, we may still be able to do stuff with it if it is a collection
if (Collection.class.isAssignableFrom(i.getPropertyType())) {
Expand Down Expand Up @@ -1180,18 +1223,19 @@ public void prepare(MethodContext context) {
} catch (Exception e) {
throw new RuntimeException(e);
}
} else if (i.getReadMethod() != null && i.getWriteMethod() != null) {
} else if (i.getReadMethod() != null && (i.getWriteMethod() != null || ctorParamIndex != null)) {
//normal javabean property
try {
handledProperties.add(i.getName());
Object propertyValue = i.read(param);
if (propertyValue == null) {
if (propertyValue == null && ctorParamIndex == null) {
//we just assume properties are null by default
//TODO: is this a valid assumption? Should we check this by creating an instance?
continue;
}
Class propertyType = i.getPropertyType();
if (i.getReadMethod().getReturnType() != i.getWriteMethod().getParameterTypes()[0]) {
if (ctorParamIndex == null
&& i.getReadMethod().getReturnType() != i.getWriteMethod().getParameterTypes()[0]) {
//this is a weird situation where the reader and writer are different types
//we iterate and try and find a valid setter method for the type we have
//OpenAPI does some weird stuff like this
Expand All @@ -1204,26 +1248,43 @@ public void prepare(MethodContext context) {
break;
}
}

}
}
DeferredParameter val = loadObjectInstance(propertyValue, existing,
i.getPropertyType());
Class finalPropertyType = propertyType;
setupSteps.add(new SerialzationStep() {
@Override
public void handle(MethodContext context, MethodCreator method, DeferredArrayStoreParameter out) {
method.invokeVirtualMethod(
ofMethod(param.getClass(), i.getWriteMethod().getName(), i.getWriteMethod().getReturnType(),
finalPropertyType),
context.loadDeferred(out),
context.loadDeferred(val));
}
if (ctorParamIndex != null) {
nonDefaultConstructorHandles[ctorParamIndex] = val;
ctorSetupSteps.add(new SerialzationStep() {
@Override
public void handle(MethodContext context, MethodCreator method, DeferredArrayStoreParameter out) {

@Override
public void prepare(MethodContext context) {
val.prepare(context);
}
});
}

@Override
public void prepare(MethodContext context) {
val.prepare(context);
}
});
} else {
Class finalPropertyType = propertyType;
setupSteps.add(new SerialzationStep() {
@Override
public void handle(MethodContext context, MethodCreator method, DeferredArrayStoreParameter out) {
method.invokeVirtualMethod(
ofMethod(param.getClass(), i.getWriteMethod().getName(),
i.getWriteMethod().getReturnType(),
finalPropertyType),
context.loadDeferred(out),
context.loadDeferred(val));
}

@Override
public void prepare(MethodContext context) {
val.prepare(context);
}
});
}
} catch (Exception e) {
throw new RuntimeException(e);
}
Expand All @@ -1232,51 +1293,56 @@ public void prepare(MethodContext context) {

//now handle accessible fields
for (Field field : param.getClass().getFields()) {
if (!Modifier.isFinal(field.getModifiers()) && !Modifier.isStatic(field.getModifiers())
&& !handledProperties.contains(field.getName())) {
if (!handledProperties.contains(field.getName())) {
Integer ctorParamIndex = constructorParamNameMap.remove(field.getName());
if ((ctorParamIndex != null || !Modifier.isFinal(field.getModifiers())) &&
!Modifier.isStatic(field.getModifiers())) {

try {
DeferredParameter val = loadObjectInstance(field.get(param), existing, field.getType());
setupSteps.add(new SerialzationStep() {
@Override
public void handle(MethodContext context, MethodCreator method, DeferredArrayStoreParameter out) {
method.writeInstanceField(
FieldDescriptor.of(param.getClass(), field.getName(), field.getType()),
context.loadDeferred(out),
context.loadDeferred(val));
}
try {
DeferredParameter val = loadObjectInstance(field.get(param), existing, field.getType());
if (ctorParamIndex != null) {
nonDefaultConstructorHandles[ctorParamIndex] = val;
ctorSetupSteps.add(new SerialzationStep() {
@Override
public void handle(MethodContext context, MethodCreator method,
DeferredArrayStoreParameter out) {

@Override
public void prepare(MethodContext context) {
val.prepare(context);
}

@Override
public void prepare(MethodContext context) {
val.prepare(context);
}
});
} else {
setupSteps.add(new SerialzationStep() {
@Override
public void handle(MethodContext context, MethodCreator method,
DeferredArrayStoreParameter out) {
method.writeInstanceField(
FieldDescriptor.of(param.getClass(), field.getName(), field.getType()),
context.loadDeferred(out),
context.loadDeferred(val));
}

@Override
public void prepare(MethodContext context) {
val.prepare(context);
}
});
}
});
} catch (Exception e) {
throw new RuntimeException(e);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
}
}

//check how the object is constructed
NonDefaultConstructorHolder nonDefaultConstructorHolder = null;
List<DeferredParameter> nonDefaultConstructorHandles = new ArrayList<>();
if (nonDefaultConstructors.containsKey(param.getClass())) {
nonDefaultConstructorHolder = nonDefaultConstructors.get(param.getClass());
List<Object> params = nonDefaultConstructorHolder.paramGenerator.apply(param);
if (params.size() != nonDefaultConstructorHolder.constructor.getParameterCount()) {
throw new RuntimeException("Unable to serialize " + param
+ " as the wrong number of parameters were generated for "
+ nonDefaultConstructorHolder.constructor);
}
int count = 0;
for (Object i : params) {
nonDefaultConstructorHandles.add(
loadObjectInstance(i, existing,
nonDefaultConstructorHolder.constructor.getParameterTypes()[count++]));
}
if (!constructorParamNameMap.isEmpty()) {
throw new RuntimeException("Could not find parameters for constructor " + nonDefaultConstructorHolder.constructor
+ " could not read field values " + constructorParamNameMap.keySet());
}

NonDefaultConstructorHolder finalNonDefaultConstructorHolder = nonDefaultConstructorHolder;
DeferredParameter[] finalCtorHandles = nonDefaultConstructorHandles;

//create a deferred value to represet the object itself. This allows the creation to be split
//over multiple methods, which is important if this is a large object
Expand All @@ -1289,7 +1355,7 @@ ResultHandle createValue(MethodContext context, MethodCreator method, ResultHand
out = method.newInstance(
ofConstructor(finalNonDefaultConstructorHolder.constructor.getDeclaringClass(),
finalNonDefaultConstructorHolder.constructor.getParameterTypes()),
nonDefaultConstructorHandles.stream().map(m -> context.loadDeferred(m))
Arrays.stream(finalCtorHandles).map(m -> context.loadDeferred(m))
.toArray(ResultHandle[]::new));
} else {
try {
Expand Down Expand Up @@ -1324,6 +1390,9 @@ ResultHandle createValue(MethodContext context, MethodCreator method, ResultHand
void doPrepare(MethodContext context) {
//this is where the object construction happens
//first create the actial object
for (SerialzationStep i : ctorSetupSteps) {
i.prepare(context);
}
objectValue.prepare(context);
for (SerialzationStep i : setupSteps) {
//then prepare the steps (i.e. creating the values to be placed into this object)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,16 @@ public void testNewInstance() throws Exception {
}, new TestJavaBean(null, 2));
}

@Test
public void testRecordableConstructor() throws Exception {
runTest(generator -> {
TestConstructorBean bean = new TestConstructorBean("John", "Citizen");
bean.setAge(30);
TestRecorder recorder = generator.getRecordingProxy(TestRecorder.class);
recorder.bean(bean);
}, new TestConstructorBean("John", "Citizen").setAge(30));
}

@Test
public void testRecordingProxyToStringNotNull() {
TestClassLoader tcl = new TestClassLoader(getClass().getClassLoader());
Expand Down
Loading