Support optional preload-task
Optionally preloading a task can be used to customize certain tasks
created dynamically with a task-factory.
Change-Id: I0c64735aaf47ff8364d6af6a6e0392257d4006d7
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/Preloader.java b/src/main/java/com/googlesource/gerrit/plugins/task/Preloader.java
index e7b91a8..e0b76c1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Preloader.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Preloader.java
@@ -21,18 +21,18 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import org.eclipse.jgit.errors.ConfigInvalidException;
/** Use to pre-load a task definition with values from its preload-task definition. */
public class Preloader {
- public static void preload(Task definition) {
+ public static void preload(Task definition) throws ConfigInvalidException {
String name = definition.preloadTask;
if (name != null) {
- Task task = definition.config.getTask(name);
- if (task == null) {
- throw new RuntimeException("Unknown preload-task.");
+ Task task = definition.config.getTaskOptional(name);
+ if (task != null) {
+ preload(task);
+ preloadFrom(definition, task);
}
- preload(task);
- preloadFrom(definition, task);
}
}
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 0eac473..5fcc8f1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
@@ -24,6 +24,9 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.eclipse.jgit.errors.ConfigInvalidException;
/** Task Configuration file living in git */
public class TaskConfig extends AbstractVersionedMetaData {
@@ -84,6 +87,9 @@
}
}
+ protected static final Pattern OPTIONAL_TASK_PATTERN =
+ Pattern.compile("([^ |]*( *[^ |])*) *\\| *");
+
protected static final String SECTION_EXTERNAL = "external";
protected static final String SECTION_ROOT = "root";
protected static final String SECTION_TASK = "task";
@@ -138,7 +144,31 @@
return externals;
}
- public Task getTask(String name) {
+ /* returs null only if optional and not found */
+ public Task getTaskOptional(String name) throws ConfigInvalidException {
+ int end = 0;
+ Matcher m = OPTIONAL_TASK_PATTERN.matcher(name);
+ while (m.find()) {
+ end = m.end();
+ Task task = getTaskOrNull(m.group(1));
+ if (task != null) {
+ return task;
+ }
+ }
+
+ String last = name.substring(end);
+ if (!"".equals(last)) { // Last entry was not optional
+ Task task = getTaskOrNull(last);
+ if (task != null) {
+ return task;
+ }
+ throw new ConfigInvalidException("task not defined");
+ }
+ return null;
+ }
+
+ /* returns null if not found */
+ protected Task getTaskOrNull(String name) {
SubSection subSection = new SubSection(SECTION_TASK, name);
return getNames(subSection).isEmpty() ? null : new Task(subSection, isVisible, isTrusted);
}
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 b1fe1a1..263043c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -36,8 +36,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
/**
@@ -48,8 +46,6 @@
*/
public class TaskTree {
protected static final String TASK_DIR = "task";
- protected static final Pattern OPTIONAL_TASK_PATTERN =
- Pattern.compile("([^ |]*( *[^ |])*) *\\| *");
protected final AccountResolver accountResolver;
protected final AllUsersNameProvider allUsers;
@@ -116,7 +112,8 @@
public class Node extends NodeList {
public final Task definition;
- public Node(Task definition, List<String> path, Map<String, String> parentProperties) {
+ public Node(Task definition, List<String> path, Map<String, String> parentProperties)
+ throws ConfigInvalidException {
this.definition = definition;
this.path.addAll(path);
this.path.add(definition.name);
@@ -133,7 +130,7 @@
}
protected void addSubDefinitions() throws OrmException {
- addSubDefinitions(getSubTasks());
+ addSubDefinitions(getSubDefinitions());
addSubFileDefinitions();
addExternalDefinitions();
}
@@ -159,7 +156,7 @@
if (ext == null) {
nodes.add(null);
} else {
- addSubDefinitions(getTasks(ext));
+ addSubDefinitions(getTaskDefinitions(ext));
}
} catch (ConfigInvalidException | IOException e) {
nodes.add(null);
@@ -167,32 +164,22 @@
}
}
- protected List<Task> getSubTasks() {
- List<Task> tasks = new ArrayList<>();
- for (String subTask : definition.subTasks) {
- addSubTaskTo(subTask, tasks);
- }
- return tasks;
- }
-
- protected void addSubTaskTo(String subTaskEntry, List<Task> tasks) {
- int end = 0;
- Matcher m = OPTIONAL_TASK_PATTERN.matcher(subTaskEntry);
- while (m.find()) {
- end = m.end();
- Task subTask = definition.config.getTask(m.group(1));
- if (subTask != null) {
- tasks.add(subTask);
- return;
+ protected List<Task> getSubDefinitions() {
+ List<Task> defs = new ArrayList<>();
+ for (String name : definition.subTasks) {
+ try {
+ Task def = definition.config.getTaskOptional(name);
+ if (def != null) {
+ defs.add(def);
+ }
+ } catch (ConfigInvalidException e) {
+ defs.add(null);
}
}
- String last = subTaskEntry.substring(end);
- if (!"".equals(last)) { // Last entry was not optional
- tasks.add(definition.config.getTask(subTaskEntry.substring(end)));
- }
+ return defs;
}
- protected List<Task> getTasks(External external)
+ protected List<Task> getTaskDefinitions(External external)
throws ConfigInvalidException, IOException, OrmException {
return getTasks(resolveUserBranch(external.user), external.file);
}
diff --git a/src/main/resources/Documentation/task.md b/src/main/resources/Documentation/task.md
index 326c550..a642426 100644
--- a/src/main/resources/Documentation/task.md
+++ b/src/main/resources/Documentation/task.md
@@ -154,7 +154,8 @@
from the current task if they redefined in the current task. Attributes
which are lists (such as subtasks) or maps (such as properties), will be
preloaded by the preload-task and then extended with the attributes from the
-current task.
+current task. See [Optional Tasks](#optional_tasks) for how to define optional
+preload-tasks.
Example:
```
@@ -165,7 +166,8 @@
: This key lists the name of a subtask of the current task. This key may be
used several times in a task section to define more than one subtask for a
-particular task.
+particular task. See [Optional Tasks](#optional_tasks) for how to define
+optional subtasks.
Example:
@@ -179,24 +181,6 @@
...
```
-To define a subtask that may not exist and that will not cause the parent task
-to be INVALID, follow the subtask name with pipe (`|`) character. This feature
-is particularly useful when a property is used in the subtask name.
-
-```
- subtask = Optional Subtask {$_name} |
-```
-
-To define an alternate subtask to load when an optional subtask does not exist,
-list the alterante subtask name after the pipe (`|`) character. This feature
-may be chained together as many times as needed.
-
-```
- subtask = Optional Subtask {$_name} |
- Backup Optional Subtask {$_name} Backup |
- Default Subtask # Must exist if the above two don't!
-```
-
`subtasks-external`
: This key defines a file containing subtasks of the current task. This
@@ -270,6 +254,27 @@
fail = label:code-review-2
```
+<a id="optional_tasks"/>
+Optional Tasks
+--------------
+To define a task that may not exist and that will not cause the task referencing
+it to be INVALID, follow the task name with pipe (`|`) character. This feature
+is particularly useful when a property is used in the task name.
+
+```
+ preload-task = Optional Subtask {$_name} |
+```
+
+To define an alternate task to load when an optional task does not exist,
+list the alterante task name after the pipe (`|`) character. This feature
+may be chained together as many times as needed.
+
+```
+ subtask = Optional Subtask {$_name} |
+ Backup Optional Subtask {$_name} Backup |
+ Default Subtask # Must exist if the above two don't!
+```
+
External Entries
----------------
A name for external task files on other projects and branches may be given
diff --git a/src/main/resources/Documentation/task_states.md b/src/main/resources/Documentation/task_states.md
index 5f41202..721032f 100644
--- a/src/main/resources/Documentation/task_states.md
+++ b/src/main/resources/Documentation/task_states.md
@@ -185,6 +185,7 @@
subtask = Subtask Preload Override Pass
subtask = Subtask Preload Override Fail
subtask = Subtask Preload Extend Subtasks
+ subtask = Subtask Preload Optional
subtask = Subtask Preload Properties
[task "Subtask Preload Preload"]
@@ -213,6 +214,9 @@
preload-task = Subtask READY
subtask = Subtask APPLICABLE
+[task "Subtask Preload Optional"]
+ preload-task = Missing | Subtask PASS
+
[task "Subtask Preload Properties"]
preload-task = Subtask Properties Hints
set-fourth-property = fourth-value
@@ -709,6 +713,11 @@
},
{
"hasPass" : true,
+ "name" : "Subtask Preload Optional",
+ "status" : "PASS"
+ },
+ {
+ "hasPass" : true,
"hint" : "second-property(first-value second-extra third-value) fourth-property(fourth-value)",
"name" : "Subtask Preload Properties",
"status" : "FAIL"
diff --git a/test/all b/test/all
index c99aa48..82347ab 100644
--- a/test/all
+++ b/test/all
@@ -472,6 +472,12 @@
{
"applicable" : true,
"hasPass" : true,
+ "name" : "Subtask Preload Optional",
+ "status" : "PASS"
+ },
+ {
+ "applicable" : true,
+ "hasPass" : true,
"hint" : "second-property(first-value second-extra third-value) fourth-property(fourth-value)",
"name" : "Subtask Preload Properties",
"status" : "FAIL"
