Fix to not mark a valid preloaded subtask as INVALID
Before this change, a subtask was marked as INVALID when a task had a
preload-task pointing to a task from external file which had a
subtask [1]. While preloading a task, all the subtasks are copied to
the current task. Since the subtask property was a String, it does
not maintain the information regarding the location of the subtask.
Due to which, the plugin fails to lookup the subtask.
Thus, fix this issue by using TaskKey as the type for subtask property,
which helps in tracking the location of the subtask. Add tests for the
same.
[1]
file: `All-Projects:refs/meta/config:task.config`
```
[root "root task"]
applicable = status:open
preload-task = //common.config^simple task with subtask
```
file: `All-Projects:refs/meta/config:task/common.config`
```
[task "simple task with subtask"]
applicable = is:open
subtask = passing task
[task "passing task"]
applicable = is:open
pass = True
```
Change-Id: Iacf5345c575209267cf6ad0489327ee10feed55c
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 f5f0bd4..56cdd88 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
@@ -27,6 +27,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.stream.Collectors;
/** Task Configuration file living in git */
public class TaskConfig extends AbstractVersionedMetaData {
@@ -63,7 +64,7 @@
public String preloadTask;
public Map<String, String> properties;
public String readyHint;
- public List<String> subTasks;
+ public List<TaskKey> subTasks;
public List<String> subTasksExternals;
public List<String> subTasksFactories;
public List<String> subTasksFiles;
@@ -85,7 +86,10 @@
preloadTask = getString(s, KEY_PRELOAD_TASK, null);
properties = getProperties(s, KEY_PROPERTIES_PREFIX);
readyHint = getString(s, KEY_READY_HINT, null);
- subTasks = getStringList(s, KEY_SUBTASK);
+ subTasks =
+ getStringList(s, KEY_SUBTASK).stream()
+ .map(subtask -> TaskKey.create(s.file(), subtask))
+ .collect(Collectors.toList());
subTasksExternals = getStringList(s, KEY_SUBTASKS_EXTERNAL);
subTasksFactories = getStringList(s, KEY_SUBTASKS_FACTORY);
subTasksFiles = getStringList(s, KEY_SUBTASKS_FILE);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskKey.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskKey.java
index bd0b683..6b8f486 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskKey.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskKey.java
@@ -52,6 +52,10 @@
return subSection().file().branch();
}
+ public FileKey file() {
+ return subSection().file();
+ }
+
public abstract SubSectionKey subSection();
public abstract String task();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
index 44fcadc..d5ab48e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -365,10 +365,11 @@
}
protected void addSubTasks() throws IOException, StorageException {
- for (String expression : task.subTasks) {
+ for (TaskKey taskKey : task.subTasks) {
try {
Optional<Task> def =
- preloader.getOptionalTask(taskExpressionFactory.create(task.file(), expression));
+ preloader.getOptionalTask(
+ taskExpressionFactory.create(taskKey.file(), taskKey.task()));
if (def.isPresent()) {
addPreloaded(def.get());
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/properties/AbstractExpander.java b/src/main/java/com/googlesource/gerrit/plugins/task/properties/AbstractExpander.java
index c970cec..1c9ef43 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/properties/AbstractExpander.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/properties/AbstractExpander.java
@@ -14,7 +14,9 @@
package com.googlesource.gerrit.plugins.task.properties;
+import com.googlesource.gerrit.plugins.task.TaskKey;
import java.lang.reflect.Field;
+import java.lang.reflect.ParameterizedType;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -101,11 +103,24 @@
field.set(cow.getForWrite(), expanded);
}
} else if (o instanceof List) {
- @SuppressWarnings("unchecked")
- List<String> forceCheck = List.class.cast(o);
- List<String> expanded = expand(forceCheck);
- if (expanded != o) {
- field.set(cow.getForWrite(), expanded);
+ ParameterizedType parameterizedType = (ParameterizedType) field.getGenericType();
+ Class<?> classType = (Class<?>) parameterizedType.getActualTypeArguments()[0];
+ if (classType == String.class) {
+ @SuppressWarnings("unchecked")
+ List<String> forceCheck = List.class.cast(o);
+ List<String> expanded = expand(forceCheck);
+ if (expanded != o) {
+ field.set(cow.getForWrite(), expanded);
+ }
+ } else if (classType == TaskKey.class) {
+ @SuppressWarnings("unchecked")
+ List<TaskKey> forceCheck = List.class.cast(o);
+ List<TaskKey> expanded = expandTaskKey(forceCheck);
+ if (expanded != o) {
+ field.set(cow.getForWrite(), expanded);
+ }
+ } else {
+ throw new RuntimeException(String.format("Unknown list type: %s", classType));
}
}
} catch (IllegalAccessException e) {
@@ -118,6 +133,29 @@
* Returns expanded unmodifiable List if property found. Returns same object if no expansions
* occurred.
*/
+ public List<TaskKey> expandTaskKey(List<TaskKey> list) {
+ if (list != null) {
+ boolean hasProperty = false;
+ List<TaskKey> expandedList = new ArrayList<>(list.size());
+ for (TaskKey value : list) {
+ String expanded = expandText(value.task());
+ boolean hasExpanded = (value.task() != expanded);
+ hasProperty = hasProperty || hasExpanded;
+ if (hasExpanded) {
+ expandedList.add(TaskKey.create(value.file(), expanded));
+ } else {
+ expandedList.add(value);
+ }
+ }
+ return hasProperty ? Collections.unmodifiableList(expandedList) : list;
+ }
+ return null;
+ }
+
+ /**
+ * Returns expanded unmodifiable List if property found. Returns same object if no expansions
+ * occurred.
+ */
public List<String> expand(List<String> list) {
if (list != null) {
boolean hasProperty = false;
diff --git a/src/main/resources/Documentation/test/task_states.md b/src/main/resources/Documentation/test/task_states.md
index 2df1263..dbe6fac 100644
--- a/src/main/resources/Documentation/test/task_states.md
+++ b/src/main/resources/Documentation/test/task_states.md
@@ -2451,6 +2451,144 @@
]
}
+[root "Root Preload from all-projects sub-dir"]
+ preload-task = //dir/common.config^Sample relative task in sub dir
+
+{
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Root Preload from all-projects sub-dir",
+ "status" : "PASS"
+}
+
+[root "Root Preload from all-projects sub-dir which has subtask in same file"]
+ preload-task = //dir/common.config^Sample relative task in sub dir with subtask from same file
+
+{
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Root Preload from all-projects sub-dir which has subtask in same file",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Sample relative task in sub dir",
+ "status" : "PASS"
+ }
+ ]
+}
+
+[root "Root Preload from all-projects sub-dir which has subtask in different file"]
+ preload-task = //dir/common.config^Sample relative task in sub dir with subtask from different file
+
+{
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Root Preload from all-projects sub-dir which has subtask in different file",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Passing task",
+ "status" : "PASS"
+ }
+ ]
+}
+
+[root "Root Preload from user ref"]
+ preload-task = @testuser/dir/relative.config^Relative Task
+
+{
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Root Preload from user ref",
+ "status" : "PASS"
+}
+
+[root "Root Preload from user ref which has subtask in same file"]
+ preload-task = @testuser/dir/relative.config^Relative Task with subtask
+
+{
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Root Preload from user ref which has subtask in same file",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Passing task",
+ "status" : "PASS"
+ }
+ ]
+}
+
+[root "Root Preload from user ref which has subtask in different file"]
+ preload-task = @testuser/dir/relative.config^Import All-Projects root task
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root Preload from user ref which has subtask in different file",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Subtask PASS",
+ "status" : "PASS"
+ }
+ ]
+}
+
+[root "Root Preload from group ref"]
+ preload-task = %{non_secret_group_name_without_space}^task in group root config file 1
+
+{
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Root Preload from group ref",
+ "status" : "PASS"
+}
+
+[root "Root Preload from group ref which has subtask in same file"]
+ preload-task = %{non_secret_group_name_without_space}^task in group root with subtask
+
+{
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Root Preload from group ref which has subtask in same file",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Passing task",
+ "status" : "PASS"
+ }
+ ]
+}
+
+[root "Root Preload from group ref which has subtask in different file"]
+ preload-task = %{non_secret_group_name_without_space}^task in group root with subtask from all-projects
+
+{
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Root Preload from group ref which has subtask in different file",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Subtask PASS",
+ "status" : "PASS"
+ }
+ ]
+}
+
[root "Root INVALID Preload"]
preload-task = missing
@@ -3045,6 +3183,10 @@
```
[task "Root Import task from subdir using relative syntax"]
subtask = dir/common.config^Sample relative task in sub dir
+
+[task "Passing task"]
+ applicable = is:open
+ pass = True
```
file: `All-Projects:refs/meta/config:task/dir/common.config`
@@ -3053,6 +3195,16 @@
applicable = is:open
pass = is:open
+[task "Sample relative task in sub dir with subtask from same file"]
+ applicable = is:open
+ pass = is:open
+ subtask = Sample relative task in sub dir
+
+[task "Sample relative task in sub dir with subtask from different file"]
+ applicable = is:open
+ pass = is:open
+ subtask = //relative.config^Passing task
+
[task "Root Import task from root task.config"]
applicable = is:open
subtask = ^Subtask PASS
@@ -3268,6 +3420,15 @@
applicable = is:open
pass = is:open
+[task "Relative Task with subtask"]
+ applicable = is:open
+ pass = is:open
+ subtask = Passing task
+
+[task "Passing task"]
+ applicable = is:open
+ pass = True
+
[task "Import All-Projects root task"]
applicable = is:open
subtask = //^Subtask PASS
@@ -3319,6 +3480,20 @@
applicable = is:open
pass = is:open
+[task "task in group root with subtask"]
+ applicable = is:open
+ pass = is:open
+ subtask = Passing task
+
+[task "Passing task"]
+ applicable = is:open
+ pass = True
+
+[task "task in group root with subtask from all-projects"]
+ applicable = is:open
+ pass = is:open
+ subtask = //^Subtask PASS
+
[task "task in group root config file 2"]
applicable = is:open
pass = is:open
