fixup! task: add support to set custom properties
Since property expansion will modify a definition's fields, Lists and
Maps need to be copied during a copy to prevent modifications on one
Task from modifying other Tasks returned from the same tasks-factory.
Also only copy a TaskBase's known field types to avoid potentially
copying a type which is not currently known, and thus may not be copied
properly. Also, add test case to validate the fix.
Change-Id: I551bf8a0532a7fb31ad21f8facc705535e69f3c5
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
index d9e8c4b..e0fec78 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.task;
+import com.google.common.primitives.Primitives;
import com.google.gerrit.common.Container;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.server.git.meta.AbstractVersionedMetaData;
@@ -90,15 +91,41 @@
}
protected TaskBase(TaskBase base) {
- for (Field field : TaskBase.class.getDeclaredFields()) {
+ copyDeclaredFields(TaskBase.class, base);
+ }
+
+ protected <T> void copyDeclaredFields(Class<T> cls, T from) {
+ for (Field field : cls.getDeclaredFields()) {
try {
field.setAccessible(true);
- field.set(this, field.get(base));
+ Class<?> fieldCls = field.getType();
+ Object val = field.get(from);
+ if (field.getType().isPrimitive()
+ || Primitives.isWrapperType(fieldCls)
+ || (val instanceof String)
+ || val == null) {
+ field.set(this, val);
+ } else if (val instanceof List) {
+ List<?> list = List.class.cast(val);
+ field.set(this, new ArrayList<>(list));
+ } else if (val instanceof Map) {
+ Map<?, ?> map = Map.class.cast(val);
+ field.set(this, new HashMap<>(map));
+ } else if (field.getName().equals("this$0")) { // Don't copy internal final field
+ } else {
+ throw new RuntimeException(
+ "Don't know how to deep copy " + fieldValueToString(field, val));
+ }
} catch (IllegalAccessException e) {
- throw new RuntimeException(e);
+ throw new RuntimeException(
+ "Cannot access field to copy it " + fieldValueToString(field, "unknown"));
}
}
}
+
+ protected String fieldValueToString(Field field, Object val) {
+ return "field:" + field.getName() + " value:" + val + " type:" + field.getType();
+ }
}
public class Task extends TaskBase {
diff --git a/src/main/resources/Documentation/test/task_states.md b/src/main/resources/Documentation/test/task_states.md
index e990856..ee0ac43 100644
--- a/src/main/resources/Documentation/test/task_states.md
+++ b/src/main/resources/Documentation/test/task_states.md
@@ -137,6 +137,19 @@
fail-hint = Name(${_name}) Change Number(${_change_number}) Change Id(${_change_id}) Change Project(${_change_project}) Change Branch(${_change_branch}) Change Status(${_change_status}) Change Topic(${_change_topic})
subtask = Subtask Properties
+[root "Root Properties Expansion"]
+ applicable = status:open
+ subtask = Subtask Property Expansion fail-hint
+
+[task "Subtask Property Expansion fail-hint"]
+ subtasks-factory = tasks-factory Property Expansion fail-hint
+
+[tasks-factory "tasks-factory Property Expansion fail-hint"]
+ set-first-property = first-property ${_name}
+ fail-hint = ${first-property}
+ fail = true
+ names-factory = names-factory static list
+
[root "Root Preload"]
preload-task = Subtask FAIL
subtask = Subtask Preload
@@ -984,6 +997,50 @@
},
{
"applicable" : true,
+ "hasPass" : false,
+ "name" : "Root Properties Expansion",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Subtask Property Expansion fail-hint",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "hint" : "first-property my a task",
+ "name" : "my a task",
+ "status" : "FAIL"
+ },
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "hint" : "first-property my b task",
+ "name" : "my b task",
+ "status" : "FAIL"
+ },
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "hint" : "first-property my c task",
+ "name" : "my c task",
+ "status" : "FAIL"
+ },
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "hint" : "first-property my d task Change Number(_change3_number) Change Id(_change3_id) Change Project(_change3_project) Change Branch(_change3_branch) Change Status(_change3_status) Change Topic(_change3_topic)",
+ "name" : "my d task Change Number(_change3_number) Change Id(_change3_id) Change Project(_change3_project) Change Branch(_change3_branch) Change Status(_change3_status) Change Topic(_change3_topic)",
+ "status" : "FAIL"
+ }
+ ]
+ }
+ ]
+ },
+ {
+ "applicable" : true,
"hasPass" : true,
"name" : "Root Preload",
"status" : "FAIL",
