Merge "Chip 'passed' and 'duplicate' navigate to 'All' task in primary tab" into stable-3.5
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskPluginDefinedInfoFactory.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskPluginDefinedInfoFactory.java
index 2e3dfa8..7cb083f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskPluginDefinedInfoFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskPluginDefinedInfoFactory.java
@@ -175,17 +175,24 @@
public Node node;
protected Task task;
protected TaskAttribute attribute;
+ protected boolean isDuplicate;
protected AttributeFactory(Node node) {
this.node = node;
this.task = node.task;
attribute = new TaskAttribute(task.name());
+
+ isDuplicate =
+ node.isDuplicate
+ || (node.isChange()
+ && node.getNodeSetByBaseTasksFactory().get(task.subSection).contains(node.key()));
+
if (options.includeStatistics) {
statistics.numberOfNodes++;
if (node.isChange()) {
statistics.numberOfChangeNodes++;
}
- if (node.isDuplicate) {
+ if (isDuplicate) {
statistics.numberOfDuplicates++;
}
attribute.statistics = new TaskAttribute.Statistics();
@@ -215,8 +222,8 @@
if (node.isChange()) {
attribute.change = node.getChangeData().getId().get();
}
- attribute.hasPass = !node.isDuplicate && (task.pass != null || task.fail != null);
- if (!node.isDuplicate) {
+ attribute.hasPass = !(isDuplicate || isAllNull(task.pass, task.fail));
+ if (!isDuplicate) {
attribute.subTasks = getSubTasks();
}
attribute.status = getStatus();
@@ -239,7 +246,7 @@
if (!options.onlyApplicable) {
attribute.applicable = applicable;
}
- if (!node.isDuplicate) {
+ if (!isDuplicate) {
if (task.inProgress != null) {
attribute.inProgress = node.matchOrNull(task.inProgress);
}
@@ -251,6 +258,9 @@
attribute.evaluationMilliSeconds = millis() - attribute.evaluationMilliSeconds;
}
addStatistics(attribute.statistics);
+ if (node.isChange()) {
+ node.getNodeSetByBaseTasksFactory().get(task.subSection).add(node.key());
+ }
return Optional.of(attribute);
}
}
@@ -284,7 +294,7 @@
}
protected Status getStatusWithExceptions() throws StorageException, QueryParseException {
- if (node.isDuplicate) {
+ if (isDuplicate) {
return Status.DUPLICATE;
}
if (isAllNull(task.pass, task.fail, attribute.subTasks)) {
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 d7e4b7d..4785885 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -89,6 +89,32 @@
public transient int summaryCount;
}
+ public class NodeSet {
+ protected ChangeData changeData;
+ protected Set<String> nodeKeys = new HashSet<>();
+
+ public NodeSet() {
+ changeData = TaskTree.this.changeData;
+ }
+
+ public boolean contains(String nodeKey) {
+ clearIfNeeded();
+ return nodeKeys.contains(nodeKey);
+ }
+
+ public boolean add(String nodeKey) {
+ clearIfNeeded();
+ return nodeKeys.add(nodeKey);
+ }
+
+ protected void clearIfNeeded() {
+ if (changeData != TaskTree.this.changeData) {
+ nodeKeys.clear();
+ changeData = TaskTree.this.changeData;
+ }
+ }
+ }
+
protected static final String TASK_DIR = "task";
protected final AccountResolver accountResolver;
@@ -105,6 +131,7 @@
protected final Provider<ChangeQueryProcessor> changeQueryProcessorProvider;
protected final StatisticsMap<String, List<ChangeData>> changesByNamesFactoryQuery =
new HitHashMap<>();
+ protected final Map<SubSectionKey, NodeSet> nodeSetByBaseTasksFactory = new HashMap<>();
protected final StatisticsMap<SubSectionKey, List<Task>> definitionsBySubSection =
new HitHashMapOfCollection<>();
protected final StatisticsMap<SubSectionKey, Map<BranchNameKey, List<Task>>>
@@ -144,6 +171,7 @@
this.changeData = changeData;
root.path = Collections.emptyList();
root.duplicateKeys = Collections.emptyList();
+ nodeSetByBaseTasksFactory.clear();
return root.getSubNodes();
}
@@ -171,6 +199,10 @@
return TaskTree.this.changeData;
}
+ protected Map<SubSectionKey, NodeSet> getNodeSetByBaseTasksFactory() {
+ return TaskTree.this.nodeSetByBaseTasksFactory;
+ }
+
protected boolean isTrusted() {
return true;
}
@@ -190,7 +222,8 @@
return createFromPreloaded(def, (parent, definition) -> new Node(parent, definition));
}
- public Node createFromPreloaded(Task def, ChangeData changeData) {
+ public Node createFromPreloaded(
+ Task def, ChangeData changeData, Map<SubSectionKey, NodeSet> nodeSetByBaseTasksFactory) {
return createFromPreloaded(
def,
(parent, definition) ->
@@ -204,6 +237,11 @@
public boolean isChange() {
return true;
}
+
+ @Override
+ protected Map<SubSectionKey, NodeSet> getNodeSetByBaseTasksFactory() {
+ return nodeSetByBaseTasksFactory;
+ }
});
}
@@ -338,6 +376,20 @@
}
@Override
+ protected Map<SubSectionKey, NodeSet> getNodeSetByBaseTasksFactory() {
+ return parent.getNodeSetByBaseTasksFactory();
+ }
+
+ protected Map<SubSectionKey, NodeSet> getNodeSetByBaseTasksFactory(SubSectionKey subSection) {
+ Map<SubSectionKey, NodeSet> nodeSetByBaseTasksFactory = getNodeSetByBaseTasksFactory();
+ if (!nodeSetByBaseTasksFactory.containsKey(subSection)) {
+ nodeSetByBaseTasksFactory = new HashMap<>(nodeSetByBaseTasksFactory);
+ nodeSetByBaseTasksFactory.put(subSection, new NodeSet());
+ }
+ return nodeSetByBaseTasksFactory;
+ }
+
+ @Override
protected boolean isTrusted() {
return parent.isTrusted() && !task.isMasqueraded;
}
@@ -473,11 +525,14 @@
throws IOException {
try {
if (namesFactory.changes != null) {
+ Map<SubSectionKey, NodeSet> nodeSetByBaseTasksFactory =
+ getNodeSetByBaseTasksFactory(tasksFactory.subSection);
for (ChangeData changeData : query(namesFactory.changes, task.isVisible)) {
addPreloaded(
preloader.preload(
task.config.new Task(tasksFactory, changeData.getId().toString())),
- changeData);
+ changeData,
+ nodeSetByBaseTasksFactory);
}
return;
}
@@ -519,8 +574,9 @@
nodes.addAll(factory.createFromPreloaded(defs));
}
- public void addPreloaded(Task def, ChangeData changeData) {
- nodes.add(factory.createFromPreloaded(def, changeData));
+ public void addPreloaded(
+ Task def, ChangeData changeData, Map<SubSectionKey, NodeSet> nodeSetByBaseTasksFactory) {
+ nodes.add(factory.createFromPreloaded(def, changeData, nodeSetByBaseTasksFactory));
}
public void addPreloaded(Task def) {
diff --git a/src/main/resources/Documentation/task.md b/src/main/resources/Documentation/task.md
index 4cf8c71..931b3f9 100644
--- a/src/main/resources/Documentation/task.md
+++ b/src/main/resources/Documentation/task.md
@@ -57,10 +57,17 @@
A task with a `READY` status is ready to be executed. All of its subtasks are
in the `PASS` state.
-A task with a `DUPLICATE` status has the same task key as one of its ancestors.
+A task can have `DUPLICATE` status in one of the following scenarios:
+
+- It has the same task key as one of its ancestors
+- If the task is generated using a task-factory with change type names-factory,
+ and it has the same task key as another change task descendant of the same
+ ancestor task-factory
+
Task keys are generally made up of the canonical task name and the change to
-which it applies. To avoid infinite loops, subtasks are ignored on duplicate
-tasks.
+which it applies. To avoid infinite loops, and to potentially reduce needless
+duplication, subtasks are ignored on duplicate tasks.
+Also see [duplicate-key](#duplicate-key).
A task with a `PASS` status meets all the criteria for `READY`, and has
executed and was successful.
@@ -238,6 +245,8 @@
subtasks-file = common.config # references the file named task/common.config
```
+<a id="duplicate-key"></a>
+
`duplicate-key`
: This key defines an identifier to help identify tasks which should be
diff --git a/src/main/resources/Documentation/test/task_states.md b/src/main/resources/Documentation/test/task_states.md
index 2e21fa8..aea074e 100644
--- a/src/main/resources/Documentation/test/task_states.md
+++ b/src/main/resources/Documentation/test/task_states.md
@@ -2284,60 +2284,10 @@
{
"applicable" : true,
"change" : _change2_number,
- "hasPass" : true,
+ "hasPass" : false,
+ "hint" : "Duplicate task is non blocking and empty to break the loop",
"name" : "_change2_number",
- "status" : "FAIL",
- "subTasks" : [
- {
- "applicable" : true,
- "hasPass" : false,
- "name" : "task (tasks-factory changes loop)",
- "status" : "WAITING",
- "subTasks" : [
- {
- "applicable" : true,
- "change" : _change1_number,
- "hasPass" : true,
- "name" : "_change1_number",
- "status" : "FAIL",
- "subTasks" : [
- {
- "applicable" : true,
- "hasPass" : false,
- "name" : "task (tasks-factory changes loop)",
- "status" : "PASS",
- "subTasks" : [
- {
- "applicable" : true,
- "change" : _change1_number,
- "hasPass" : false,
- "hint" : "Duplicate task is non blocking and empty to break the loop",
- "name" : "_change1_number",
- "status" : "DUPLICATE"
- },
- {
- "applicable" : true,
- "change" : _change2_number,
- "hasPass" : false,
- "hint" : "Duplicate task is non blocking and empty to break the loop",
- "name" : "_change2_number",
- "status" : "DUPLICATE"
- }
- ]
- }
- ]
- },
- {
- "applicable" : true,
- "change" : _change2_number,
- "hasPass" : false,
- "hint" : "Duplicate task is non blocking and empty to break the loop",
- "name" : "_change2_number",
- "status" : "DUPLICATE"
- }
- ]
- }
- ]
+ "status" : "DUPLICATE"
}
]
}
