Merge branch 'stable-2.16' into stable-3.0
* stable-2.16: (27 commits)
Use shallow clone while expanding definitions
Move match() and matchOrNull() to MatchCache
fixup! Cache Task definition lists for ChangeNodes
Cache Task definition lists for ChangeNodes
Split SubNodeFactory out of SubNodeAdder
Moving caching logic out of loadSubNodes()
Use a TaskTree.Node.Invalid instead of nulls
Use overloading for special cases instead of nulls
Add a TaskTree ApplicableNodeFilter
Add an isCacheableByBranch() to the PredicateCache
Track whether Task.applicable needs to be refreshed
Allow Expander to expand a single Task's field
Minor cleanup of task Properties.expandText()
Remove cached Change subNodes on Node completion
Explicitly signal end of Task Properties expansion
Use a SubNodeAdder to add TaskTree SubNodes
Avoid hard coding the refs/meta/config ref
Inject All-Projects in TaskConfigFactory
Change TaskExpressions to iterate over TaskKeys
Avoid passing isTrusted in TaskConfigFactory.getTaskConfig()
...
Change-Id: I1d65c0fbd5f5326867526977d1ee82652737eab1
diff --git a/gr-task-plugin/gr-task-plugin.js b/gr-task-plugin/gr-task-plugin.js
index e49f493..8579c93 100644
--- a/gr-task-plugin/gr-task-plugin.js
+++ b/gr-task-plugin/gr-task-plugin.js
@@ -107,7 +107,7 @@
icon.tooltip = 'Failed';
break;
case 'READY':
- icon.id = 'gr-icons:rebase';
+ icon.id = 'gr-icons:playArrow';
icon.color = 'green';
icon.tooltip = 'Ready';
break;
@@ -117,13 +117,18 @@
icon.tooltip = 'Invalid';
break;
case 'WAITING':
- icon.id = 'gr-icons:side-by-side';
+ icon.id = 'gr-icons:pause';
icon.color = 'red';
icon.tooltip = 'Waiting';
break;
- case 'PASS':
+ case 'DUPLICATE':
icon.id = 'gr-icons:check';
icon.color = 'green';
+ icon.tooltip = 'Duplicate';
+ break;
+ case 'PASS':
+ icon.id = 'gr-icons:check-circle';
+ icon.color = 'green';
icon.tooltip = 'Passed';
break;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/Copier.java b/src/main/java/com/googlesource/gerrit/plugins/task/Copier.java
index 2c0ccad..10f4048 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Copier.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Copier.java
@@ -14,44 +14,19 @@
package com.googlesource.gerrit.plugins.task;
-import com.google.common.primitives.Primitives;
import java.lang.reflect.Field;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
public class Copier {
- protected static <T> void deepCopyDeclaredFields(
- Class<T> cls,
- T from,
- T to,
- boolean includeInaccessible,
- Collection<Class<?>> copyReferenceOnly) {
+ protected static <T> void shallowCopyDeclaredFields(
+ Class<T> cls, T from, T to, boolean includeInaccessible) {
for (Field field : cls.getDeclaredFields()) {
try {
if (includeInaccessible) {
field.setAccessible(true);
}
- Class<?> fieldCls = field.getType();
Object val = field.get(from);
- if (field.getType().isPrimitive()
- || Primitives.isWrapperType(fieldCls)
- || (val instanceof String)
- || val == null
- || copyReferenceOnly.contains(fieldCls)) {
+ if (!field.getName().equals("this$0")) { // Can't copy internal final field
field.set(to, val);
- } else if (val instanceof List) {
- List<?> list = List.class.cast(val);
- field.set(to, new ArrayList<>(list));
- } else if (val instanceof Map) {
- Map<?, ?> map = Map.class.cast(val);
- field.set(to, new HashMap<>(map));
- } else if (field.getName().equals("this$0")) { // Can't copy internal final field
- } else {
- throw new RuntimeException(
- "Don't know how to deep copy " + fieldValueToString(field, val));
}
} catch (IllegalAccessException e) {
if (includeInaccessible) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/CopyOnWrite.java b/src/main/java/com/googlesource/gerrit/plugins/task/CopyOnWrite.java
index efafd57..8369d67 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/CopyOnWrite.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/CopyOnWrite.java
@@ -14,9 +14,58 @@
package com.googlesource.gerrit.plugins.task;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.Optional;
import java.util.function.Function;
public class CopyOnWrite<T> {
+ public static class CloneOnWrite<C extends Cloneable> extends CopyOnWrite<C> {
+ public CloneOnWrite(C cloneable) {
+ super(cloneable, copier(cloneable));
+ }
+ }
+
+ public static <C extends Cloneable> Function<C, C> copier(C cloneable) {
+ return c -> clone(c);
+ }
+
+ @SuppressWarnings("unchecked")
+ public static <C extends Cloneable> C clone(C cloneable) {
+ try {
+ for (Class<?> cls = cloneable.getClass(); cls != null; cls = cls.getSuperclass()) {
+ Optional<Method> optional = getOptionalDeclaredMethod(cls, "clone");
+ if (optional.isPresent()) {
+ Method clone = optional.get();
+ clone.setAccessible(true);
+ return (C) cloneable.getClass().cast(clone.invoke(cloneable));
+ }
+ }
+ throw new RuntimeException("Cannot find clone() method");
+ } catch (SecurityException
+ | IllegalAccessException
+ | IllegalArgumentException
+ | InvocationTargetException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ /**
+ * A faster getDeclaredMethod() without exceptions. The original apparently does a linear search
+ * anyway, and it is significantly slower when it throws NoSuchMethodExceptions.
+ */
+ public static Optional<Method> getOptionalDeclaredMethod(
+ Class<?> cls, String name, Class<?>... parameterTypes) {
+ for (Method method : cls.getDeclaredMethods()) {
+ if (method.getName().equals(name)
+ && Arrays.equals(method.getParameterTypes(), parameterTypes)) {
+ return Optional.of(method);
+ }
+ }
+ return Optional.empty();
+ }
+
protected Function<T, T> copier;
protected T original;
protected T copy;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/MatchCache.java b/src/main/java/com/googlesource/gerrit/plugins/task/MatchCache.java
index 45fe46d..ef5af15 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/MatchCache.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/MatchCache.java
@@ -31,25 +31,28 @@
this.changeData = changeData;
}
- protected boolean match(String query) throws StorageException, QueryParseException {
+ public boolean match(String query) throws StorageException, QueryParseException {
if (query == null) {
return true;
}
Boolean isMatched = matchResultByQuery.get(query);
if (isMatched == null) {
- isMatched = predicateCache.match(changeData, query);
+ isMatched = predicateCache.matchWithExceptions(changeData, query);
matchResultByQuery.put(query, isMatched);
}
return isMatched;
}
- protected Boolean matchOrNull(String query) {
+ public Boolean matchOrNull(String query) {
if (query == null) {
return null;
}
Boolean isMatched = matchResultByQuery.get(query);
if (isMatched == null) {
- isMatched = predicateCache.matchOrNull(changeData, query);
+ try {
+ isMatched = predicateCache.matchWithExceptions(changeData, query);
+ } catch (QueryParseException | RuntimeException e) {
+ }
matchResultByQuery.put(query, isMatched);
}
return isMatched;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/PredicateCache.java b/src/main/java/com/googlesource/gerrit/plugins/task/PredicateCache.java
index 7896417..6540ad2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/PredicateCache.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/PredicateCache.java
@@ -15,46 +15,52 @@
package com.googlesource.gerrit.plugins.task;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.index.query.AndPredicate;
+import com.google.gerrit.index.query.NotPredicate;
+import com.google.gerrit.index.query.OrPredicate;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.gerrit.server.query.change.DestinationPredicate;
+import com.google.gerrit.server.query.change.ProjectPredicate;
+import com.google.gerrit.server.query.change.RefPredicate;
+import com.google.gerrit.server.query.change.RegexProjectPredicate;
+import com.google.gerrit.server.query.change.RegexRefPredicate;
import com.google.inject.Inject;
+import java.util.Arrays;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
+import org.eclipse.jgit.lib.Config;
public class PredicateCache {
protected final ChangeQueryBuilder cqb;
+ protected final Set<String> cacheableByBranchPredicateClassNames;
protected final CurrentUser user;
protected final Map<String, ThrowingProvider<Predicate<ChangeData>, QueryParseException>>
predicatesByQuery = new HashMap<>();
@Inject
- public PredicateCache(CurrentUser user, ChangeQueryBuilder cqb) {
+ public PredicateCache(
+ @GerritServerConfig Config config,
+ @PluginName String pluginName,
+ CurrentUser user,
+ ChangeQueryBuilder cqb) {
this.user = user;
this.cqb = cqb;
+ cacheableByBranchPredicateClassNames =
+ new HashSet<>(
+ Arrays.asList(
+ config.getStringList(pluginName, "cacheable-predicates", "byBranch-className")));
}
- public boolean match(ChangeData c, String query) throws StorageException, QueryParseException {
- if (query == null) {
- return true;
- }
- return matchWithExceptions(c, query);
- }
-
- public Boolean matchOrNull(ChangeData c, String query) {
- if (query != null) {
- try {
- return matchWithExceptions(c, query);
- } catch (QueryParseException | RuntimeException e) {
- }
- }
- return null;
- }
-
- protected boolean matchWithExceptions(ChangeData c, String query)
+ public boolean matchWithExceptions(ChangeData c, String query)
throws QueryParseException, StorageException {
if ("true".equalsIgnoreCase(query)) {
return true;
@@ -78,4 +84,39 @@
throw e;
}
}
+
+ /**
+ * Can this query's output be assumed to be constant given any Change destined for the same
+ * Branch.NameKey?
+ */
+ public boolean isCacheableByBranch(String query) throws QueryParseException {
+ if (query == null
+ || "".equals(query)
+ || "false".equalsIgnoreCase(query)
+ || "true".equalsIgnoreCase(query)) {
+ return true;
+ }
+ return isCacheableByBranch(getPredicate(query));
+ }
+
+ protected boolean isCacheableByBranch(Predicate<ChangeData> predicate) {
+ if (predicate instanceof AndPredicate
+ || predicate instanceof NotPredicate
+ || predicate instanceof OrPredicate) {
+ for (Predicate<ChangeData> subPred : predicate.getChildren()) {
+ if (!isCacheableByBranch(subPred)) {
+ return false;
+ }
+ }
+ return true;
+ }
+ if (predicate instanceof DestinationPredicate
+ || predicate instanceof ProjectPredicate
+ || predicate instanceof RefPredicate
+ || predicate instanceof RegexProjectPredicate
+ || predicate instanceof RegexRefPredicate) {
+ return true;
+ }
+ return cacheableByBranchPredicateClassNames.contains(predicate.getClass().getName());
+ }
}
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 c48ed6e..b240c3b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Preloader.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Preloader.java
@@ -14,7 +14,10 @@
package com.googlesource.gerrit.plugins.task;
+import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.task.TaskConfig.Task;
+import com.googlesource.gerrit.plugins.task.cli.PatchSetArgument;
+import java.io.IOException;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.HashMap;
@@ -26,17 +29,23 @@
/** Use to pre-load a task definition with values from its preload-task definition. */
public class Preloader {
+ protected final TaskConfigFactory taskConfigFactory;
protected final Map<TaskExpressionKey, Optional<Task>> optionalTaskByExpression = new HashMap<>();
- public List<Task> getRootTasks(TaskConfig cfg) {
- return getTasks(cfg, TaskConfig.SECTION_ROOT);
+ @Inject
+ public Preloader(TaskConfigFactory taskConfigFactory) {
+ this.taskConfigFactory = taskConfigFactory;
}
- public List<Task> getTasks(TaskConfig cfg) {
- return getTasks(cfg, TaskConfig.SECTION_TASK);
+ public List<Task> getRootTasks() throws IOException, ConfigInvalidException {
+ return getTasks(taskConfigFactory.getRootConfig(), TaskConfig.SECTION_ROOT);
}
- protected List<Task> getTasks(TaskConfig cfg, String type) {
+ public List<Task> getTasks(FileKey file) throws IOException, ConfigInvalidException {
+ return getTasks(taskConfigFactory.getTaskConfig(file), TaskConfig.SECTION_TASK);
+ }
+
+ protected List<Task> getTasks(TaskConfig cfg, String type) throws IOException {
List<Task> preloaded = new ArrayList<>();
for (Task task : cfg.getTasks(type)) {
try {
@@ -55,27 +64,27 @@
* @return Optional<Task> which is empty if the expression is optional and no tasks are resolved
* @throws ConfigInvalidException if the expression requires a task and no tasks are resolved
*/
- public Optional<Task> getOptionalTask(TaskConfig cfg, TaskExpression expression)
- throws ConfigInvalidException {
+ public Optional<Task> getOptionalTask(TaskExpression expression)
+ throws ConfigInvalidException, IOException {
Optional<Task> task = optionalTaskByExpression.get(expression.key);
if (task == null) {
- task = preloadOptionalTask(cfg, expression);
+ task = preloadOptionalTask(expression);
optionalTaskByExpression.put(expression.key, task);
}
return task;
}
- protected Optional<Task> preloadOptionalTask(TaskConfig cfg, TaskExpression expression)
- throws ConfigInvalidException {
- Optional<Task> definition = loadOptionalTask(cfg, expression);
+ protected Optional<Task> preloadOptionalTask(TaskExpression expression)
+ throws ConfigInvalidException, IOException {
+ Optional<Task> definition = loadOptionalTask(expression);
return definition.isPresent() ? Optional.of(preload(definition.get())) : definition;
}
- public Task preload(Task definition) throws ConfigInvalidException {
+ public Task preload(Task definition) throws ConfigInvalidException, IOException {
String expression = definition.preloadTask;
if (expression != null) {
Optional<Task> preloadFrom =
- getOptionalTask(definition.config, new TaskExpression(definition.file(), expression));
+ getOptionalTask(new TaskExpression(definition.file(), expression));
if (preloadFrom.isPresent()) {
return preloadFrom(definition, preloadFrom.get());
}
@@ -83,11 +92,11 @@
return definition;
}
- protected Optional<Task> loadOptionalTask(TaskConfig cfg, TaskExpression expression)
- throws ConfigInvalidException {
+ protected Optional<Task> loadOptionalTask(TaskExpression expression)
+ throws ConfigInvalidException, IOException {
try {
- for (String name : expression) {
- Optional<Task> task = cfg.getOptionalTask(name);
+ for (TaskKey key : expression) {
+ Optional<Task> task = getOptionalTask(key);
if (task.isPresent()) {
return task;
}
@@ -117,6 +126,14 @@
return preloadTo;
}
+ protected Optional<Task> getOptionalTask(TaskKey key) throws IOException, ConfigInvalidException {
+ return taskConfigFactory.getTaskConfig(key.subSection().file()).getOptionalTask(key.task());
+ }
+
+ public void masquerade(PatchSetArgument psa) {
+ taskConfigFactory.masquerade(psa);
+ }
+
protected static <S, K, V> void preloadField(
Field field, Task definition, Task preloadFrom, Task preloadTo)
throws IllegalArgumentException, IllegalAccessException {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java b/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java
index 5f9a8d5..609511b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java
@@ -14,7 +14,7 @@
package com.googlesource.gerrit.plugins.task;
-import com.google.common.collect.Sets;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.query.change.ChangeData;
@@ -34,15 +34,22 @@
/** Use to expand properties like ${_name} in the text of various definitions. */
public class Properties {
- public static final Properties EMPTY_PARENT = new Properties();
+ public static final Properties EMPTY =
+ new Properties() {
+ @Override
+ protected Function<String, String> getParentMapper() {
+ return n -> "";
+ }
+ };
- protected final Properties parentProperties;
+ protected final TaskTree.Node node;
protected final Task origTask;
protected final CopyOnWrite<Task> task;
protected Expander expander;
protected Loader loader;
protected boolean init = true;
protected boolean isTaskRefreshNeeded;
+ protected boolean isApplicableRefreshRequired;
protected boolean isSubNodeReloadRequired;
public Properties() {
@@ -50,33 +57,27 @@
expander = new Expander(n -> "");
}
- public Properties(Task origTask, Properties parentProperties) {
+ public Properties(TaskTree.Node node, Task origTask) {
+ this.node = node;
this.origTask = origTask;
- this.parentProperties = parentProperties;
- task = new CopyOnWrite<>(origTask, t -> origTask.config.new Task(t));
+ task = new CopyOnWrite.CloneOnWrite<>(origTask);
}
/** Use to expand properties specifically for Tasks. */
public Task getTask(ChangeData changeData) throws StorageException {
- if (loader != null && loader.isNonTaskDefinedPropertyLoaded()) {
- // To detect NamesFactories dependent on non task defined properties, the checking must be
- // done after subnodes are fully loaded, which unfortunately happens after getTask() is
- // called. However, these non task property uses from the last change are still detectable
- // here before we replace the old Loader with a new one.
- isSubNodeReloadRequired = true;
- }
-
loader = new Loader(changeData);
expander = new Expander(n -> loader.load(n));
-
if (isTaskRefreshNeeded || init) {
+ expander.expand(task, TaskConfig.KEY_APPLICABLE);
+ isApplicableRefreshRequired = loader.isNonTaskDefinedPropertyLoaded();
+
+ expander.expand(task, ImmutableSet.of(TaskConfig.KEY_APPLICABLE, TaskConfig.KEY_NAME));
+
Map<String, String> exported = expander.expand(origTask.exported);
if (exported != origTask.exported) {
task.getForWrite().exported = exported;
}
- expander.expand(task, Collections.emptySet());
-
if (init) {
init = false;
isTaskRefreshNeeded = loader.isNonTaskDefinedPropertyLoaded();
@@ -85,31 +86,37 @@
return task.getForRead();
}
+ // To detect NamesFactories dependent on non task defined properties, the checking must be
+ // done after subnodes are fully loaded, which unfortunately happens after getTask() is
+ // called, therefore this must be called after all subnodes have been loaded.
+ public void expansionComplete() {
+ isSubNodeReloadRequired = loader.isNonTaskDefinedPropertyLoaded();
+ }
+
+ public boolean isApplicableRefreshRequired() {
+ return isApplicableRefreshRequired;
+ }
+
public boolean isSubNodeReloadRequired() {
return isSubNodeReloadRequired;
}
/** Use to expand properties specifically for NamesFactories. */
public NamesFactory getNamesFactory(NamesFactory namesFactory) {
- return expander.expand(
- namesFactory,
- nf -> namesFactory.config.new NamesFactory(nf),
- Sets.newHashSet(TaskConfig.KEY_TYPE));
+ return expander.expand(namesFactory, ImmutableSet.of(TaskConfig.KEY_TYPE));
+ }
+
+ protected Function<String, String> getParentMapper() {
+ return n -> node.getParentProperties().expander.getValueForName(n);
}
protected class Loader {
protected final ChangeData changeData;
- protected final Function<String, String> inheritedMapper;
protected Change change;
protected boolean isInheritedPropertyLoaded;
public Loader(ChangeData changeData) {
this.changeData = changeData;
- if (parentProperties == null || parentProperties.expander == null) {
- inheritedMapper = n -> "";
- } else {
- inheritedMapper = n -> parentProperties.expander.getValueForName(n);
- }
}
public boolean isNonTaskDefinedPropertyLoaded() {
@@ -124,7 +131,7 @@
if (value == null) {
value = origTask.properties.get(name);
if (value == null) {
- value = inheritedMapper.apply(name);
+ value = getParentMapper().apply(name);
if (!value.isEmpty()) {
isInheritedPropertyLoaded = true;
}
@@ -207,7 +214,7 @@
/**
* Expand all properties (${property_name} -> property_value) in the given text. Returns same
- * object if no expansions occured.
+ * object if no expansions occurred.
*/
public Map<String, String> expand(Map<String, String> map) {
if (map != null) {
@@ -269,7 +276,15 @@
/**
* Returns expanded object if property found in the Strings in the object's Fields (except the
- * excluded ones). Returns same object if no expansions occured.
+ * excluded ones). Returns same object if no expansions occurred.
+ */
+ public <C extends Cloneable> C expand(C object, Set<String> excludedFieldNames) {
+ return expand(new CopyOnWrite.CloneOnWrite<>(object), excludedFieldNames);
+ }
+
+ /**
+ * Returns expanded object if property found in the Strings in the object's Fields (except the
+ * excluded ones). Returns same object if no expansions occurred.
*/
public <T> T expand(T object, Function<T, T> copier, Set<String> excludedFieldNames) {
return expand(new CopyOnWrite<>(object, copier), excludedFieldNames);
@@ -277,38 +292,59 @@
/**
* Returns expanded object if property found in the Strings in the object's Fields (except the
- * excluded ones). Returns same object if no expansions occured.
+ * excluded ones). Returns same object if no expansions occurred.
*/
public <T> T expand(CopyOnWrite<T> cow, Set<String> excludedFieldNames) {
for (Field field : cow.getOriginal().getClass().getFields()) {
- try {
- if (!excludedFieldNames.contains(field.getName())) {
- field.setAccessible(true);
- Object o = field.get(cow.getOriginal());
- if (o instanceof String) {
- String expanded = expandText((String) o);
- if (expanded != o) {
- 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);
- }
- }
- }
- } catch (IllegalAccessException e) {
- throw new RuntimeException(e);
+ if (!excludedFieldNames.contains(field.getName())) {
+ expand(cow, field);
}
}
return cow.getForRead();
}
/**
+ * Returns expanded object if property found in the fieldName Field if it is a String, or in the
+ * List's Strings if it is a List. Returns same object if no expansions occurred.
+ */
+ public <T> T expand(CopyOnWrite<T> cow, String fieldName) {
+ try {
+ return expand(cow, cow.getOriginal().getClass().getField(fieldName));
+ } catch (NoSuchFieldException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ /**
+ * Returns expanded object if property found in the Field if it is a String, or in the List's
+ * Strings if it is a List. Returns same object if no expansions occurred.
+ */
+ public <T> T expand(CopyOnWrite<T> cow, Field field) {
+ try {
+ field.setAccessible(true);
+ Object o = field.get(cow.getOriginal());
+ if (o instanceof String) {
+ String expanded = expandText((String) o);
+ if (expanded != o) {
+ 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);
+ }
+ }
+ } catch (IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ return cow.getForRead();
+ }
+
+ /**
* Returns expanded unmodifiable List if property found. Returns same object if no expansions
- * occured.
+ * occurred.
*/
public List<String> expand(List<String> list) {
if (list != null) {
@@ -325,18 +361,18 @@
}
/**
- * Expand all properties (${property_name} -> property_value) in the given text . Returns same
- * object if no expansions occured.
+ * Expand all properties (${property_name} -> property_value) in the given text. Returns same
+ * object if no expansions occurred.
*/
public String expandText(String text) {
if (text == null) {
return null;
}
- StringBuffer out = new StringBuffer();
Matcher m = PATTERN.matcher(text);
if (!m.find()) {
return text;
}
+ StringBuffer out = new StringBuffer();
do {
m.appendReplacement(out, Matcher.quoteReplacement(getValueForName(m.group(1))));
} while (m.find());
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
index 08add9f..9367d23 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
@@ -27,9 +27,11 @@
import com.googlesource.gerrit.plugins.task.cli.PatchSetArgument;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
public class TaskAttributeFactory implements ChangeAttributeFactory {
@@ -38,6 +40,7 @@
public enum Status {
INVALID,
UNKNOWN,
+ DUPLICATE,
WAITING,
READY,
PASS,
@@ -89,13 +92,14 @@
}
protected PluginDefinedInfo createWithExceptions(ChangeData c) {
+ MatchCache matchCache = new MatchCache(predicateCache, c);
TaskPluginAttribute a = new TaskPluginAttribute();
try {
for (Node node : definitions.getRootNodes(c)) {
- if (node == null) {
+ if (node instanceof Node.Invalid) {
a.roots.add(invalid());
} else {
- new AttributeFactory(node).create().ifPresent(t -> a.roots.add(t));
+ new AttributeFactory(node, matchCache).create().ifPresent(t -> a.roots.add(t));
}
}
} catch (ConfigInvalidException | IOException | StorageException e) {
@@ -114,10 +118,6 @@
protected Task task;
protected TaskAttribute attribute;
- protected AttributeFactory(Node node) {
- this(node, new MatchCache(predicateCache, node.getChangeData()));
- }
-
protected AttributeFactory(Node node, MatchCache matchCache) {
this.node = node;
this.matchCache = matchCache;
@@ -133,7 +133,7 @@
boolean applicable = matchCache.match(task.applicable);
if (!task.isVisible) {
- if (!task.isTrusted || (!applicable && !options.onlyApplicable)) {
+ if (!node.isTrusted() || (!applicable && !options.onlyApplicable)) {
return Optional.of(unknown());
}
}
@@ -142,8 +142,10 @@
if (node.isChange()) {
attribute.change = node.getChangeData().getId().get();
}
- attribute.hasPass = task.pass != null || task.fail != null;
- attribute.subTasks = getSubTasks();
+ attribute.hasPass = !node.isDuplicate && (task.pass != null || task.fail != null);
+ if (!node.isDuplicate) {
+ attribute.subTasks = getSubTasks();
+ }
attribute.status = getStatus();
if (options.onlyInvalid && !isValidQueries()) {
attribute.status = Status.INVALID;
@@ -157,11 +159,13 @@
if (!options.onlyApplicable) {
attribute.applicable = applicable;
}
- if (task.inProgress != null) {
- attribute.inProgress = matchCache.matchOrNull(task.inProgress);
+ if (!node.isDuplicate) {
+ if (task.inProgress != null) {
+ attribute.inProgress = matchCache.matchOrNull(task.inProgress);
+ }
+ attribute.exported = task.exported.isEmpty() ? null : task.exported;
}
attribute.hint = getHint(attribute.status, task);
- attribute.exported = task.exported.isEmpty() ? null : task.exported;
if (options.evaluationTime) {
attribute.evaluationMilliSeconds = millis() - attribute.evaluationMilliSeconds;
@@ -170,16 +174,16 @@
}
}
}
- } catch (ConfigInvalidException
- | IOException
- | QueryParseException
- | RuntimeException e) {
+ } catch (ConfigInvalidException | IOException | QueryParseException | RuntimeException e) {
return Optional.of(invalid()); // bad applicability query
}
return Optional.empty();
}
protected Status getStatusWithExceptions() throws StorageException, QueryParseException {
+ if (node.isDuplicate) {
+ return Status.DUPLICATE;
+ }
if (isAllNull(task.pass, task.fail, attribute.subTasks)) {
// A leaf def has no defined subdefs.
boolean hasDefinedSubtasks =
@@ -215,7 +219,8 @@
}
}
- if (attribute.subTasks != null && !isAll(attribute.subTasks, Status.PASS)) {
+ if (attribute.subTasks != null
+ && !isAll(attribute.subTasks, EnumSet.of(Status.PASS, Status.DUPLICATE))) {
// It is possible for a subtask's PASS criteria to change while
// a parent task is executing, or even after the parent task
// completes. This can result in the parent PASS criteria being
@@ -251,8 +256,9 @@
protected List<TaskAttribute> getSubTasks()
throws ConfigInvalidException, IOException, StorageException {
List<TaskAttribute> subTasks = new ArrayList<>();
- for (Node subNode : node.getSubNodes()) {
- if (subNode == null) {
+ for (Node subNode :
+ options.onlyApplicable ? node.getSubNodes(matchCache) : node.getSubNodes()) {
+ if (subNode instanceof Node.Invalid) {
subTasks.add(invalid());
} else {
MatchCache subMatchCache = matchCache;
@@ -299,10 +305,16 @@
}
protected String getHint(Status status, Task def) {
- if (status == Status.READY) {
- return def.readyHint;
- } else if (status == Status.FAIL) {
- return def.failHint;
+ if (status != null) {
+ switch (status) {
+ case READY:
+ return def.readyHint;
+ case FAIL:
+ return def.failHint;
+ case DUPLICATE:
+ return "Duplicate task is non blocking and empty to break the loop";
+ default:
+ }
}
return null;
}
@@ -316,9 +328,9 @@
return true;
}
- protected static boolean isAll(Iterable<TaskAttribute> atts, Status state) {
+ protected static boolean isAll(Iterable<TaskAttribute> atts, Set<Status> states) {
for (TaskAttribute att : atts) {
- if (att.status != state) {
+ if (!states.contains(att.status)) {
return false;
}
}
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 698b33a..b4e79e0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
@@ -14,13 +14,11 @@
package com.googlesource.gerrit.plugins.task;
-import com.google.common.collect.Sets;
import com.google.gerrit.common.Container;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.server.git.meta.AbstractVersionedMetaData;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@@ -55,6 +53,7 @@
public class TaskBase extends SubSection {
public String applicable;
+ public String duplicateKey;
public Map<String, String> exported;
public String fail;
public String failHint;
@@ -69,13 +68,14 @@
public List<String> subTasksFiles;
public boolean isVisible;
- public boolean isTrusted;
+ public boolean isMasqueraded;
- public TaskBase(SubSectionKey s, boolean isVisible, boolean isTrusted) {
+ public TaskBase(SubSectionKey s, boolean isVisible, boolean isMasqueraded) {
super(s);
this.isVisible = isVisible;
- this.isTrusted = isTrusted;
+ this.isMasqueraded = isMasqueraded;
applicable = getString(s, KEY_APPLICABLE, null);
+ duplicateKey = getString(s, KEY_DUPLICATE_KEY, null);
exported = getProperties(s, KEY_EXPORT_PREFIX);
fail = getString(s, KEY_FAIL, null);
failHint = getString(s, KEY_FAIL_HINT, null);
@@ -92,7 +92,7 @@
protected TaskBase(TaskBase base) {
this(base.subSection);
- Copier.deepCopyDeclaredFields(TaskBase.class, base, this, false, copyOnlyReferencesFor());
+ Copier.shallowCopyDeclaredFields(TaskBase.class, base, this, false);
}
protected TaskBase(SubSectionKey s) {
@@ -100,22 +100,14 @@
}
}
- public class Task extends TaskBase {
+ public class Task extends TaskBase implements Cloneable {
public final TaskKey key;
- public Task(SubSectionKey s, boolean isVisible, boolean isTrusted) {
- super(s, isVisible, isTrusted);
+ public Task(SubSectionKey s, boolean isVisible, boolean isMasqueraded) {
+ super(s, isVisible, isMasqueraded);
key = TaskKey.create(s);
}
- public Task(Task task) {
- super(task);
- // Despite being copied in Copier.deepCopyDeclaredFields this
- // is needed to avoid the final variable initialization warning.
- this.key = task.key;
- Copier.deepCopyDeclaredFields(Task.class, task, this, false, copyOnlyReferencesFor());
- }
-
public Task(TasksFactory tasks, String name) {
super(tasks);
key = TaskKey.create(tasks.subSection, name);
@@ -148,13 +140,13 @@
public class TasksFactory extends TaskBase {
public String namesFactory;
- public TasksFactory(SubSectionKey s, boolean isVisible, boolean isTrusted) {
- super(s, isVisible, isTrusted);
+ public TasksFactory(SubSectionKey s, boolean isVisible, boolean isMasqueraded) {
+ super(s, isVisible, isMasqueraded);
namesFactory = getString(s, KEY_NAMES_FACTORY, null);
}
}
- public class NamesFactory extends SubSection {
+ public class NamesFactory extends SubSection implements Cloneable {
public String changes;
public List<String> names;
public String type;
@@ -165,11 +157,6 @@
names = getStringList(s, KEY_NAME);
type = getString(s, KEY_TYPE, null);
}
-
- public NamesFactory(NamesFactory n) {
- super(n.subSection);
- Copier.deepCopyDeclaredFields(NamesFactory.class, n, this, false, copyOnlyReferencesFor());
- }
}
public class External extends SubSection {
@@ -190,10 +177,11 @@
protected static final String SECTION_EXTERNAL = "external";
protected static final String SECTION_NAMES_FACTORY = "names-factory";
protected static final String SECTION_ROOT = "root";
- protected static final String SECTION_TASK = "task";
+ protected static final String SECTION_TASK = TaskKey.CONFIG_SECTION;
protected static final String SECTION_TASKS_FACTORY = "tasks-factory";
protected static final String KEY_APPLICABLE = "applicable";
protected static final String KEY_CHANGES = "changes";
+ protected static final String KEY_DUPLICATE_KEY = "duplicate-key";
protected static final String KEY_EXPORT_PREFIX = "export-";
protected static final String KEY_FAIL = "fail";
protected static final String KEY_FAIL_HINT = "fail-hint";
@@ -214,25 +202,25 @@
protected final FileKey file;
public boolean isVisible;
- public boolean isTrusted;
+ public boolean isMasqueraded;
- public TaskConfig(FileKey file, boolean isVisible, boolean isTrusted) {
- this(file.branch(), file, isVisible, isTrusted);
+ public TaskConfig(FileKey file, boolean isVisible, boolean isMasqueraded) {
+ this(file.branch(), file, isVisible, isMasqueraded);
}
public TaskConfig(
- Branch.NameKey masqueraded, FileKey file, boolean isVisible, boolean isTrusted) {
+ Branch.NameKey masqueraded, FileKey file, boolean isVisible, boolean isMasqueraded) {
super(masqueraded, file.file());
this.file = file;
this.isVisible = isVisible;
- this.isTrusted = isTrusted;
+ this.isMasqueraded = isMasqueraded;
}
protected List<Task> getTasks(String type) {
List<Task> tasks = new ArrayList<>();
// No need to get a task with no name (what would we call it?)
for (String task : cfg.getSubsections(type)) {
- tasks.add(new Task(subSectionKey(type, task), isVisible, isTrusted));
+ tasks.add(new Task(subSectionKey(type, task), isVisible, isMasqueraded));
}
return tasks;
}
@@ -250,11 +238,11 @@
SubSectionKey subSection = subSectionKey(SECTION_TASK, name);
return getNames(subSection).isEmpty()
? Optional.empty()
- : Optional.of(new Task(subSection, isVisible, isTrusted));
+ : Optional.of(new Task(subSection, isVisible, isMasqueraded));
}
public TasksFactory getTasksFactory(String name) {
- return new TasksFactory(subSectionKey(SECTION_TASKS_FACTORY, name), isVisible, isTrusted);
+ return new TasksFactory(subSectionKey(SECTION_TASKS_FACTORY, name), isVisible, isMasqueraded);
}
public NamesFactory getNamesFactory(String name) {
@@ -284,7 +272,7 @@
for (String name : names) {
valueByName.put(name, getString(s, name));
}
- return valueByName;
+ return Collections.unmodifiableMap(valueByName);
}
protected Set<String> getMatchingNames(SubSectionKey s, String match) {
@@ -294,7 +282,7 @@
matched.add(name);
}
}
- return matched;
+ return Collections.unmodifiableSet(matched);
}
protected Set<String> getNames(SubSectionKey s) {
@@ -318,8 +306,4 @@
protected SubSectionKey subSectionKey(String section, String subSection) {
return SubSectionKey.create(file, section, subSection);
}
-
- protected Collection<Class<?>> copyOnlyReferencesFor() {
- return Sets.newHashSet(TaskKey.class, SubSectionKey.class);
- }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
index bbf424b..b309dd4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
@@ -18,8 +18,10 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.AllProjectsNameProvider;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -45,21 +47,22 @@
protected final AllProjectsName allProjects;
protected final Map<Branch.NameKey, PatchSetArgument> psaMasquerades = new HashMap<>();
+ protected final Map<FileKey, TaskConfig> taskCfgByFile = new HashMap<>();
@Inject
protected TaskConfigFactory(
- AllProjectsName allProjects,
+ AllProjectsNameProvider allProjectsNameProvider,
GitRepositoryManager gitMgr,
PermissionBackend permissionBackend,
CurrentUser user) {
- this.allProjects = allProjects;
+ this.allProjects = allProjectsNameProvider.get();
this.gitMgr = gitMgr;
this.permissionBackend = permissionBackend;
this.user = user;
}
public TaskConfig getRootConfig() throws ConfigInvalidException, IOException {
- return getTaskConfig(FileKey.create(getRootBranch(), DEFAULT), true);
+ return getTaskConfig(FileKey.create(getRootBranch(), DEFAULT));
}
public void masquerade(PatchSetArgument psa) {
@@ -67,26 +70,35 @@
}
protected Branch.NameKey getRootBranch() {
- return new Branch.NameKey(allProjects, "refs/meta/config");
+ return new Branch.NameKey(allProjects, RefNames.REFS_CONFIG);
}
- public TaskConfig getTaskConfig(FileKey file, boolean isTrusted)
- throws ConfigInvalidException, IOException {
+ public TaskConfig getTaskConfig(FileKey key) throws ConfigInvalidException, IOException {
+ TaskConfig cfg = taskCfgByFile.get(key);
+ if (cfg == null) {
+ cfg = loadTaskConfig(key);
+ taskCfgByFile.put(key, cfg);
+ }
+ return cfg;
+ }
+
+ private TaskConfig loadTaskConfig(FileKey file) throws ConfigInvalidException, IOException {
Branch.NameKey branch = file.branch();
PatchSetArgument psa = psaMasquerades.get(branch);
boolean visible = true; // invisible psas are filtered out by commandline
+ boolean isMasqueraded = false;
if (psa == null) {
visible = canRead(branch);
} else {
- isTrusted = false;
+ isMasqueraded = true;
branch = new Branch.NameKey(psa.change.getProject(), psa.patchSet.getId().toRefName());
}
Project.NameKey project = file.branch().getParentKey();
TaskConfig cfg =
- psa == null
- ? new TaskConfig(file, visible, isTrusted)
- : new TaskConfig(branch, file, visible, isTrusted);
+ isMasqueraded
+ ? new TaskConfig(branch, file, visible, isMasqueraded)
+ : new TaskConfig(file, visible, isMasqueraded);
try (Repository git = gitMgr.openRepository(project)) {
cfg.load(project, git);
} catch (IOException e) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskExpression.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskExpression.java
index 90dffff..5a61d29 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskExpression.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskExpression.java
@@ -35,7 +35,7 @@
* <li><code> "foo | bar |" -> ("foo", "bar") optional</code>
* </ul>
*/
-public class TaskExpression implements Iterable<String> {
+public class TaskExpression implements Iterable<TaskKey> {
protected static final Pattern EXPRESSION_PATTERN = Pattern.compile("([^ |]+[^|]*)(\\|)?");
protected final TaskExpressionKey key;
@@ -44,8 +44,8 @@
}
@Override
- public Iterator<String> iterator() {
- return new Iterator<String>() {
+ public Iterator<TaskKey> iterator() {
+ return new Iterator<TaskKey>() {
Matcher m = EXPRESSION_PATTERN.matcher(key.expression());
Boolean hasNext;
boolean optional;
@@ -65,13 +65,14 @@
}
@Override
- public String next() {
- hasNext(); // in case next() was (re)called w/o calling hasNext()
+ public TaskKey next() {
+ // Can't use @SuppressWarnings("ReturnValueIgnored") on method call
+ boolean ignored = hasNext(); // in case next() was (re)called w/o calling hasNext()
if (!hasNext) {
throw new NoSuchElementException("No more names, yet expression was not optional");
}
hasNext = null;
- return m.group(1).trim();
+ return TaskKey.create(key.file(), m.group(1).trim());
}
};
}
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 0af75c8..481cbd5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskKey.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskKey.java
@@ -20,11 +20,18 @@
/** An immutable reference to a task in task config file. */
@AutoValue
public abstract class TaskKey {
+ protected static final String CONFIG_SECTION = "task";
+
/** Creates a TaskKey with task name as the name of sub section. */
public static TaskKey create(SubSectionKey section) {
return create(section, section.subSection());
}
+ /** Creates a TaskKey with given FileKey and task name and sub section's name as 'task'. */
+ public static TaskKey create(FileKey file, String task) {
+ return create(SubSectionKey.create(file, CONFIG_SECTION, task));
+ }
+
/** Creates a TaskKey from a sub section and task name, generally used by TasksFactory. */
public static TaskKey create(SubSectionKey section, String task) {
return new AutoValue_TaskKey(section, task);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskStatus.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskStatus.java
new file mode 100644
index 0000000..5dc7afd
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskStatus.java
@@ -0,0 +1,27 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.task;
+
+public class TaskStatus {
+ public enum Status {
+ INVALID,
+ UNKNOWN,
+ DUPLICATE,
+ WAITING,
+ READY,
+ PASS,
+ FAIL;
+ }
+}
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 2bd8cf6..c99b6a2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -14,6 +14,8 @@
package com.googlesource.gerrit.plugins.task;
+import static java.util.stream.Collectors.toList;
+
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -70,11 +72,11 @@
protected final AccountResolver accountResolver;
protected final AllUsersNameProvider allUsers;
protected final CurrentUser user;
- protected final TaskConfigFactory taskFactory;
protected final Preloader preloader;
protected final NodeList root = new NodeList();
protected final Provider<ChangeQueryBuilder> changeQueryBuilderProvider;
protected final Provider<ChangeQueryProcessor> changeQueryProcessorProvider;
+ protected final Map<SubSectionKey, List<Task>> definitionsBySubSection = new HashMap<>();
protected ChangeData changeData;
@@ -84,119 +86,148 @@
AllUsersNameProvider allUsers,
AnonymousUser anonymousUser,
CurrentUser user,
- TaskConfigFactory taskFactory,
Provider<ChangeQueryBuilder> changeQueryBuilderProvider,
Provider<ChangeQueryProcessor> changeQueryProcessorProvider,
Preloader preloader) {
this.accountResolver = accountResolver;
this.allUsers = allUsers;
this.user = user != null ? user : anonymousUser;
- this.taskFactory = taskFactory;
this.changeQueryProcessorProvider = changeQueryProcessorProvider;
this.changeQueryBuilderProvider = changeQueryBuilderProvider;
this.preloader = preloader;
}
public void masquerade(PatchSetArgument psa) {
- taskFactory.masquerade(psa);
+ preloader.masquerade(psa);
}
public List<Node> getRootNodes(ChangeData changeData)
throws ConfigInvalidException, IOException, StorageException {
this.changeData = changeData;
root.path = Collections.emptyList();
+ root.duplicateKeys = Collections.emptyList();
return root.getSubNodes();
}
protected class NodeList {
protected NodeList parent = null;
protected Collection<String> path;
+ protected Collection<String> duplicateKeys;
protected Map<TaskKey, Node> cachedNodeByTask = new HashMap<>();
- protected List<Node> nodes;
- protected Set<String> names = new HashSet<>();
+ protected List<Node> cachedNodes;
- protected void addSubNodes() throws ConfigInvalidException, IOException, StorageException {
- addPreloaded(preloader.getRootTasks(taskFactory.getRootConfig()));
- }
-
- protected void addPreloaded(List<Task> defs) throws ConfigInvalidException, StorageException {
- for (Task def : defs) {
- addPreloaded(def);
+ protected List<Node> getSubNodes()
+ throws ConfigInvalidException, IOException, StorageException {
+ if (cachedNodes != null) {
+ return refresh(cachedNodes);
}
+ return cachedNodes = loadSubNodes();
}
- protected void addPreloaded(Task def) throws ConfigInvalidException, StorageException {
- addPreloaded(def, (parent, definition) -> new Node(parent, definition));
- }
-
- protected void addPreloaded(Task def, NodeFactory nodeFactory)
- throws ConfigInvalidException, StorageException {
- if (def != null) {
- try {
- Node node = cachedNodeByTask.get(def.key());
- boolean isRefreshNeeded = node != null;
- if (node == null) {
- node = nodeFactory.create(this, def);
- }
-
- if (!path.contains(node.key()) && names.add(def.name())) {
- // path check above detects looping definitions
- // names check above detects duplicate subtasks
- if (isRefreshNeeded) {
- node.refreshTask();
- }
- nodes.add(node);
- return;
- }
- } catch (Exception e) {
- }
- }
- addInvalidNode();
- }
-
- protected void addInvalidNode() {
- nodes.add(null); // null node indicates invalid
- }
-
- protected List<Node> getSubNodes() throws ConfigInvalidException, IOException, StorageException {
- if (nodes == null) {
- nodes = new ArrayList<>();
- addSubNodes();
- } else {
- refreshSubNodes();
- }
- return nodes;
- }
-
- public void refreshSubNodes() throws ConfigInvalidException, StorageException {
- if (nodes != null) {
- for (Node node : nodes) {
- if (node != null) {
- node.refreshTask();
- }
- }
- }
+ protected List<Node> loadSubNodes()
+ throws ConfigInvalidException, IOException, StorageException {
+ return new SubNodeFactory().createFromPreloaded(preloader.getRootTasks());
}
public ChangeData getChangeData() {
- return parent == null ? TaskTree.this.changeData : parent.getChangeData();
+ return TaskTree.this.changeData;
}
- protected Properties getProperties() {
- return Properties.EMPTY_PARENT;
+ protected boolean isTrusted() {
+ return true;
+ }
+
+ protected class SubNodeFactory {
+ protected Set<String> names = new HashSet<>();
+
+ public List<Node> createFromPreloaded(List<Task> defs)
+ throws ConfigInvalidException, StorageException {
+ List<Node> nodes = new ArrayList<>();
+ for (Task def : defs) {
+ nodes.add(createFromPreloaded(def));
+ }
+ return nodes;
+ }
+
+ public Node createFromPreloaded(Task def) throws ConfigInvalidException, StorageException {
+ return createFromPreloaded(def, (parent, definition) -> new Node(parent, definition));
+ }
+
+ public Node createFromPreloaded(Task def, ChangeData changeData)
+ throws ConfigInvalidException, StorageException {
+ return createFromPreloaded(
+ def,
+ (parent, definition) ->
+ new Node(parent, definition) {
+ @Override
+ public ChangeData getChangeData() {
+ return changeData;
+ }
+
+ @Override
+ public boolean isChange() {
+ return true;
+ }
+ });
+ }
+
+ protected Node createFromPreloaded(Task def, NodeFactory nodeFactory)
+ throws ConfigInvalidException, StorageException {
+ if (def != null) {
+ try {
+ Node node = cachedNodeByTask.get(def.key());
+ boolean isRefreshNeeded = node != null;
+ if (node == null) {
+ node = nodeFactory.create(NodeList.this, def);
+ }
+
+ if (names.add(def.name())) {
+ // names check above detects duplicate subtasks
+ if (isRefreshNeeded) {
+ node.refreshTask();
+ }
+ return node;
+ }
+ } catch (Exception e) {
+ }
+ }
+ return createInvalid();
+ }
+
+ protected Node createInvalid() {
+ return new Node().new Invalid();
+ }
}
}
public class Node extends NodeList {
+ public class Invalid extends Node {
+ @Override
+ public void refreshTask() throws ConfigInvalidException, StorageException {}
+
+ @Override
+ public Task getDefinition() {
+ return null;
+ }
+ }
+
public Task task;
+ public boolean isDuplicate;
protected final Properties properties;
protected final TaskKey taskKey;
+ protected Map<Branch.NameKey, List<Node>> nodesByBranch;
+ protected boolean hasUnfilterableSubNodes = false;
+
+ protected Node() { // Only for Invalid
+ taskKey = null;
+ properties = null;
+ }
public Node(NodeList parent, Task task) throws ConfigInvalidException, StorageException {
this.parent = parent;
taskKey = task.key();
- properties = new Properties(task, parent.getProperties());
+ properties = new Properties(this, task);
refreshTask();
}
@@ -204,153 +235,291 @@
return String.valueOf(getChangeData().getId().get()) + TaskConfig.SEP + taskKey;
}
+ public List<Node> getSubNodes() throws ConfigInvalidException, IOException, StorageException {
+ if (cachedNodes != null) {
+ return refresh(cachedNodes);
+ }
+ List<Node> nodes = loadSubNodes();
+ if (!properties.isSubNodeReloadRequired()) {
+ if (!isChange()) {
+ return cachedNodes = nodes;
+ }
+ definitionsBySubSection.computeIfAbsent(
+ task.key().subSection(),
+ k -> nodes.stream().map(n -> n.getDefinition()).collect(toList()));
+ } else {
+ hasUnfilterableSubNodes = true;
+ cachedNodeByTask.clear();
+ nodes.stream()
+ .filter(n -> !(n instanceof Invalid) && !n.isChange())
+ .forEach(n -> cachedNodeByTask.put(n.task.key(), n));
+ }
+ return nodes;
+ }
+
+ public List<Node> getSubNodes(MatchCache matchCache)
+ throws ConfigInvalidException, IOException, StorageException {
+ if (hasUnfilterableSubNodes) {
+ return getSubNodes();
+ }
+ return new ApplicableNodeFilter(matchCache).getSubNodes();
+ }
+
+ @Override
+ protected List<Node> loadSubNodes()
+ throws ConfigInvalidException, IOException, StorageException {
+ List<Task> cachedDefinitions = definitionsBySubSection.get(task.key().subSection());
+ if (cachedDefinitions != null) {
+ return new SubNodeFactory().createFromPreloaded(cachedDefinitions);
+ }
+ List<Node> nodes = new SubNodeAdder().getSubNodes();
+ properties.expansionComplete();
+ return nodes;
+ }
+
/* The task needs to be refreshed before a node is used, however
subNode refreshing can wait until they are fetched since they may
not be needed. */
public void refreshTask() throws ConfigInvalidException, StorageException {
this.path = new LinkedList<>(parent.path);
- this.path.add(key());
+ String key = key();
+ isDuplicate = path.contains(key);
+ path.add(key);
this.task = properties.getTask(getChangeData());
- if (nodes != null && properties.isSubNodeReloadRequired()) {
- cachedNodeByTask.clear();
- nodes.stream().filter(n -> n != null).forEach(n -> cachedNodeByTask.put(n.task.key(), n));
- names.clear();
- nodes = null;
+ this.duplicateKeys = new LinkedList<>(parent.duplicateKeys);
+ if (task.duplicateKey != null) {
+ isDuplicate |= duplicateKeys.contains(task.duplicateKey);
+ duplicateKeys.add(task.duplicateKey);
}
}
+ protected Properties getParentProperties() {
+ return (parent instanceof Node) ? ((Node) parent).properties : Properties.EMPTY;
+ }
+
@Override
- protected void addSubNodes() throws ConfigInvalidException, StorageException {
- addSubTasks();
- addSubTasksFactoryTasks();
- addSubTasksFiles();
- addSubTasksExternals();
- }
-
- protected void addSubTasks() throws ConfigInvalidException, StorageException {
- for (String expression : task.subTasks) {
- try {
- Optional<Task> def =
- preloader.getOptionalTask(task.config, new TaskExpression(task.file(), expression));
- if (def.isPresent()) {
- addPreloaded(def.get());
- }
- } catch (ConfigInvalidException e) {
- addInvalidNode();
- }
- }
- }
-
- protected void addSubTasksFiles() throws ConfigInvalidException, StorageException {
- for (String file : task.subTasksFiles) {
- try {
- addPreloaded(
- getPreloadedTasks(FileKey.create(task.key().branch(), resolveTaskFileName(file))));
- } catch (ConfigInvalidException | IOException e) {
- addInvalidNode();
- }
- }
- }
-
- protected void addSubTasksExternals() throws ConfigInvalidException, StorageException {
- for (String external : task.subTasksExternals) {
- try {
- External ext = task.config.getExternal(external);
- if (ext == null) {
- addInvalidNode();
- } else {
- addPreloaded(getPreloadedTasks(ext));
- }
- } catch (ConfigInvalidException | IOException e) {
- addInvalidNode();
- }
- }
- }
-
- protected void addSubTasksFactoryTasks() throws ConfigInvalidException, StorageException {
- for (String tasksFactoryName : task.subTasksFactories) {
- TasksFactory tasksFactory = task.config.getTasksFactory(tasksFactoryName);
- if (tasksFactory != null) {
- NamesFactory namesFactory = task.config.getNamesFactory(tasksFactory.namesFactory);
- if (namesFactory != null && namesFactory.type != null) {
- namesFactory = getProperties().getNamesFactory(namesFactory);
- switch (NamesFactoryType.getNamesFactoryType(namesFactory.type)) {
- case STATIC:
- addStaticTypeTasks(tasksFactory, namesFactory);
- continue;
- case CHANGE:
- addChangeTypeTasks(tasksFactory, namesFactory);
- continue;
- }
- }
- }
- addInvalidNode();
- }
- }
-
- protected void addStaticTypeTasks(TasksFactory tasksFactory, NamesFactory namesFactory)
- throws ConfigInvalidException, StorageException {
- for (String name : namesFactory.names) {
- addPreloaded(preloader.preload(task.config.new Task(tasksFactory, name)));
- }
- }
-
- protected void addChangeTypeTasks(TasksFactory tasksFactory, NamesFactory namesFactory)
- throws ConfigInvalidException, StorageException {
- try {
- if (namesFactory.changes != null) {
- List<ChangeData> changeDataList =
- changeQueryProcessorProvider
- .get()
- .query(changeQueryBuilderProvider.get().parse(namesFactory.changes))
- .entities();
- for (ChangeData changeData : changeDataList) {
- addPreloaded(
- preloader.preload(
- task.config.new Task(tasksFactory, changeData.getId().toString())),
- (parent, definition) ->
- new Node(parent, definition) {
- @Override
- public ChangeData getChangeData() {
- return changeData;
- }
-
- @Override
- public boolean isChange() {
- return true;
- }
- });
- }
- return;
- }
- } catch (StorageException e) {
- log.atSevere().withCause(e).log("ERROR: running changes query: " + namesFactory.changes);
- } catch (QueryParseException e) {
- }
- addInvalidNode();
- }
-
- protected List<Task> getPreloadedTasks(External external)
- throws ConfigInvalidException, IOException, StorageException {
- return getPreloadedTasks(
- FileKey.create(resolveUserBranch(external.user), resolveTaskFileName(external.file)));
- }
-
- protected List<Task> getPreloadedTasks(FileKey file)
- throws ConfigInvalidException, IOException {
- return preloader.getTasks(taskFactory.getTaskConfig(file, task.isTrusted));
+ protected boolean isTrusted() {
+ return parent.isTrusted() && !task.isMasqueraded;
}
@Override
- protected Properties getProperties() {
- return properties;
+ public ChangeData getChangeData() {
+ return parent.getChangeData();
+ }
+
+ public Task getDefinition() {
+ return properties.origTask;
}
public boolean isChange() {
return false;
}
+
+ protected class SubNodeAdder {
+ protected List<Node> nodes = new ArrayList<>();
+ protected SubNodeFactory factory = new SubNodeFactory();
+
+ public List<Node> getSubNodes() throws ConfigInvalidException, IOException, StorageException {
+ addSubTasks();
+ addSubTasksFactoryTasks();
+ addSubTasksFiles();
+ addSubTasksExternals();
+ return nodes;
+ }
+
+ protected void addSubTasks() throws ConfigInvalidException, IOException, StorageException {
+ for (String expression : task.subTasks) {
+ try {
+ Optional<Task> def =
+ preloader.getOptionalTask(new TaskExpression(task.file(), expression));
+ if (def.isPresent()) {
+ addPreloaded(def.get());
+ }
+ } catch (ConfigInvalidException e) {
+ addInvalidNode();
+ }
+ }
+ }
+
+ protected void addSubTasksFiles() throws ConfigInvalidException, StorageException {
+ for (String file : task.subTasksFiles) {
+ try {
+ addPreloaded(
+ preloader.getTasks(FileKey.create(task.key().branch(), resolveTaskFileName(file))));
+ } catch (ConfigInvalidException | IOException e) {
+ addInvalidNode();
+ }
+ }
+ }
+
+ protected void addSubTasksExternals() throws ConfigInvalidException, StorageException {
+ for (String external : task.subTasksExternals) {
+ try {
+ External ext = task.config.getExternal(external);
+ if (ext == null) {
+ addInvalidNode();
+ } else {
+ addPreloaded(getPreloadedTasks(ext));
+ }
+ } catch (ConfigInvalidException | IOException e) {
+ addInvalidNode();
+ }
+ }
+ }
+
+ protected void addSubTasksFactoryTasks()
+ throws ConfigInvalidException, IOException, StorageException {
+ for (String tasksFactoryName : task.subTasksFactories) {
+ TasksFactory tasksFactory = task.config.getTasksFactory(tasksFactoryName);
+ if (tasksFactory != null) {
+ NamesFactory namesFactory = task.config.getNamesFactory(tasksFactory.namesFactory);
+ if (namesFactory != null && namesFactory.type != null) {
+ namesFactory = properties.getNamesFactory(namesFactory);
+ switch (NamesFactoryType.getNamesFactoryType(namesFactory.type)) {
+ case STATIC:
+ addStaticTypeTasks(tasksFactory, namesFactory);
+ continue;
+ case CHANGE:
+ addChangeTypeTasks(tasksFactory, namesFactory);
+ continue;
+ }
+ }
+ }
+ addInvalidNode();
+ }
+ }
+
+ protected void addStaticTypeTasks(TasksFactory tasksFactory, NamesFactory namesFactory)
+ throws ConfigInvalidException, IOException, StorageException {
+ for (String name : namesFactory.names) {
+ addPreloaded(preloader.preload(task.config.new Task(tasksFactory, name)));
+ }
+ }
+
+ protected void addChangeTypeTasks(TasksFactory tasksFactory, NamesFactory namesFactory)
+ throws ConfigInvalidException, IOException, StorageException {
+ try {
+ if (namesFactory.changes != null) {
+ List<ChangeData> changeDataList =
+ changeQueryProcessorProvider
+ .get()
+ .query(changeQueryBuilderProvider.get().parse(namesFactory.changes))
+ .entities();
+ for (ChangeData changeData : changeDataList) {
+ addPreloaded(
+ preloader.preload(
+ task.config.new Task(tasksFactory, changeData.getId().toString())),
+ changeData);
+ }
+ return;
+ }
+ } catch (StorageException e) {
+ log.atSevere().withCause(e).log("ERROR: running changes query: " + namesFactory.changes);
+ } catch (QueryParseException e) {
+ }
+ addInvalidNode();
+ }
+
+ public void addPreloaded(List<Task> defs) throws ConfigInvalidException, StorageException {
+ nodes.addAll(factory.createFromPreloaded(defs));
+ }
+
+ public void addPreloaded(Task def, ChangeData changeData)
+ throws ConfigInvalidException, StorageException {
+ nodes.add(factory.createFromPreloaded(def, changeData));
+ }
+
+ public void addPreloaded(Task def) throws ConfigInvalidException, StorageException {
+ nodes.add(factory.createFromPreloaded(def));
+ }
+
+ public void addInvalidNode() {
+ nodes.add(factory.createInvalid());
+ }
+
+ protected List<Task> getPreloadedTasks(External external)
+ throws ConfigInvalidException, IOException, StorageException {
+ return preloader.getTasks(
+ FileKey.create(resolveUserBranch(external.user), resolveTaskFileName(external.file)));
+ }
+ }
+
+ public class ApplicableNodeFilter {
+ protected MatchCache matchCache;
+ protected PredicateCache pcache;
+ protected Branch.NameKey branch = getChangeData().change().getDest();
+
+ public ApplicableNodeFilter(MatchCache matchCache)
+ throws ConfigInvalidException, IOException, StorageException {
+ this.matchCache = matchCache;
+ this.pcache = matchCache.predicateCache;
+ }
+
+ public List<Node> getSubNodes() throws ConfigInvalidException, IOException, StorageException {
+ if (nodesByBranch != null) {
+ List<Node> nodes = nodesByBranch.get(branch);
+ if (nodes != null) {
+ return refresh(nodes);
+ }
+ }
+
+ List<Node> nodes = Node.this.getSubNodes();
+ if (!hasUnfilterableSubNodes && !nodes.isEmpty()) {
+ Optional<List<Node>> filterable = getOptionalApplicableForBranch(nodes);
+ if (filterable.isPresent()) {
+ if (nodesByBranch == null) {
+ nodesByBranch = new HashMap<>();
+ }
+ nodesByBranch.put(branch, filterable.get());
+ return filterable.get();
+ }
+ hasUnfilterableSubNodes = true;
+ }
+ return nodes;
+ }
+
+ protected Optional<List<Node>> getOptionalApplicableForBranch(List<Node> nodes)
+ throws ConfigInvalidException, IOException, StorageException {
+ int filterable = 0;
+ List<Node> applicableNodes = new ArrayList<>();
+ for (Node node : nodes) {
+ if (node instanceof Invalid) {
+ filterable++;
+ } else if (isApplicableCacheableByBranch(node)) {
+ filterable++;
+ try {
+ if (!matchCache.match(node.task.applicable)) {
+ // Correctness will not be affected if more nodes are added than necessary
+ // (i.e. if isApplicableCacheableByBranch() does not realize a Node is cacheable
+ // based on its Branch), but it is incorrect to filter out a Node now that could
+ // later be applicable when a property, other than its Change's destination, is
+ // altered.
+ continue;
+ }
+ } catch (QueryParseException e) {
+ }
+ }
+ applicableNodes.add(node);
+ }
+ // Simple heuristic to determine whether storing the filtered nodes is worth it. There
+ // is minor evidence to suggest that storing a large list actually hurts performance.
+ return (filterable > nodes.size() / 2) ? Optional.of(applicableNodes) : Optional.empty();
+ }
+
+ protected boolean isApplicableCacheableByBranch(Node node) {
+ String applicable = node.task.applicable;
+ if (node.properties.isApplicableRefreshRequired()) {
+ return false;
+ }
+ try {
+ return pcache.isCacheableByBranch(applicable);
+ } catch (QueryParseException e) {
+ return false;
+ }
+ }
+ }
}
protected String resolveTaskFileName(String file) throws ConfigInvalidException {
@@ -377,4 +546,12 @@
}
return new Branch.NameKey(allUsers.get(), RefNames.refsUsers(acct));
}
+
+ protected static List<Node> refresh(List<Node> nodes)
+ throws ConfigInvalidException, StorageException {
+ for (Node node : nodes) {
+ node.refreshTask();
+ }
+ return nodes;
+ }
}
diff --git a/src/main/resources/Documentation/config-gerrit.md b/src/main/resources/Documentation/config-gerrit.md
new file mode 100644
index 0000000..6f0b0a5
--- /dev/null
+++ b/src/main/resources/Documentation/config-gerrit.md
@@ -0,0 +1,25 @@
+# Admin User Guide - Configuration
+
+## File `etc/gerrit.config`
+
+The file `'$site_path'/etc/gerrit.config` is a Git-style config file
+that controls many host specific settings for Gerrit.
+
+### Section @PLUGIN@ "cacheable-predicates"
+
+The @PLUGIN@.cacheable-predicates section configures Change Predicate
+optimizations which the @PLUGIN@ plugin may use when evaluating tasks.
+
+#### @PLUGIN@.cacheable-predicates.byBranch-className
+
+The value set with this key specifies a fully qualified class name
+of a Predicate which can be assumed to always return the same match
+result to all Changes destined for the same project/branch
+combinations. This key may be specified more than once.
+
+Example:
+
+```
+[@PLUGIN@ "cacheable-predicates"]
+ byBranch-className = com.google.gerrit.server.query.change.BranchSetPredicate
+```
\ No newline at end of file
diff --git a/src/main/resources/Documentation/task.md b/src/main/resources/Documentation/task.md
index fa5e834..ce2dd40 100644
--- a/src/main/resources/Documentation/task.md
+++ b/src/main/resources/Documentation/task.md
@@ -51,11 +51,17 @@
completes.
A task with a `WAITING` status is not yet ready to execute. A task in this
-state is blocked by its subtasks which are not yet in the `PASS` state.
+state is blocked by its subtasks which are not yet in the `PASS` or `DUPLICATE`
+state.
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.
+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.
+
A task with a `PASS` status meets all the criteria for `READY`, and has
executed and was successful.
@@ -231,6 +237,48 @@
subtasks-file = common.config # references the file named task/common.config
```
+`duplicate-key`
+
+: This key defines an identifier to help identify tasks which should be
+considered duplicates even if they are not exact duplicates. When the task
+plugin encounters a task with the same duplicate-key as one of its
+ancestors, it will be considered a duplicate of that ancestor. Tasks such as
+a starting task and a looping tasks-factory that preload the same base task
+are not exact duplicates, yet they may logically represent duplicates. In
+this case, defining a `duplicate-key` on the base task which is preloaded
+from two different places (usually a root and a change tasks-factory), will
+ensure that any loops are halted once the original change is reached. Without
+a duplicate-key, the walking would generally walk one task further than
+desired.
+
+Outlined below is a simple way to walk a change's git dependencies in the
+task plugin. While Git does not allow loops in commit histories, sometimes
+in Gerrit when changes get rebased, it can cause loops (because Gerrit
+sometimes tracks outdated dependencies). The use of the duplicate-key
+below results in the loop being detected when you would expect it to be.
+
+Example:
+
+```
+[root "git dependencies"]
+ applicable = status:new
+ preload-task = git dependencies
+
+[task "git dependencies"]
+ fail = -status:new
+ fail-hint = [${_change_status}] dependency needs to be OPEN
+ subtasks-factory = git dependencies
+ duplicate-key = git dependencies ${_change_number}
+
+[tasks-factory "git dependencies"]
+ names-factory = git dependencies
+ preload-task = git dependencies
+
+[names-factory "git dependencies"]
+ type = change
+ changes = -status:merged parentof:${_change_number} project:${_change_project} branch:${_change_branch}
+```
+
Root Tasks
----------
Root tasks typically define the "final verification" tasks for changes. Each
diff --git a/src/main/resources/Documentation/test/preview.md b/src/main/resources/Documentation/test/preview.md
index 1aeb649..6aff577 100644
--- a/src/main/resources/Documentation/test/preview.md
+++ b/src/main/resources/Documentation/test/preview.md
@@ -122,18 +122,6 @@
"status" : "INVALID" # Only Test Suite: invalid
},
{
- "applicable" : true,
- "hasPass" : false,
- "name" : "Looping",
- "status" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- },
- {
"name" : "UNKNOWN",
"status" : "INVALID"
},
@@ -226,106 +214,6 @@
"status" : "INVALID"
}
]
- },
- {
- "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" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "applicable" : true,
- "change" : _change2_number,
- "hasPass" : true,
- "name" : "_change2_number",
- "status" : "FAIL",
- "subTasks" : [
- {
- "applicable" : true,
- "hasPass" : false,
- "name" : "task (tasks-factory changes loop)",
- "status" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- }
- ]
- }
- ]
- },
- {
- "applicable" : true,
- "change" : _change2_number,
- "hasPass" : true,
- "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" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- }
- ]
}
]
}
@@ -536,18 +424,6 @@
"status" : "INVALID" # Only Test Suite: invalid
},
{
- "applicable" : true,
- "hasPass" : false,
- "name" : "Looping",
- "status" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- },
- {
"name" : "UNKNOWN",
"status" : "INVALID"
},
@@ -640,106 +516,6 @@
"status" : "INVALID"
}
]
- },
- {
- "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" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "applicable" : true,
- "change" : _change2_number,
- "hasPass" : true,
- "name" : "_change2_number",
- "status" : "FAIL",
- "subTasks" : [
- {
- "applicable" : true,
- "hasPass" : false,
- "name" : "task (tasks-factory changes loop)",
- "status" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- }
- ]
- }
- ]
- },
- {
- "applicable" : true,
- "change" : _change2_number,
- "hasPass" : true,
- "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" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- }
- ]
}
]
}
diff --git a/src/main/resources/Documentation/test/task_states.md b/src/main/resources/Documentation/test/task_states.md
index e9cc8b6..f48f528 100644
--- a/src/main/resources/Documentation/test/task_states.md
+++ b/src/main/resources/Documentation/test/task_states.md
@@ -1183,6 +1183,99 @@
]
}
+[root "Root applicable Property"]
+ subtask = Subtask applicable Property
+ subtasks-factory = tasks-factory branch NOT applicable Property
+
+[tasks-factory "tasks-factory branch NOT applicable Property"]
+ names-factory = names-factory branch NOT applicable Property
+ applicable = branch:dev
+ fail = True
+
+[names-factory "names-factory branch NOT applicable Property"]
+ type = static
+ name = NOT Applicable 1
+ name = NOT Applicable 2
+ name = NOT Applicable 3
+
+[task "Subtask applicable Property"]
+ applicable = change:${_change_number}
+ fail = True
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root applicable Property",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Subtask applicable Property",
+ "status" : "FAIL"
+ }, # Only Test Suite: all
+ { # Only Test Suite: all
+ "applicable" : false, # Only Test Suite: all
+ "hasPass" : true, # Only Test Suite: all
+ "name" : "NOT Applicable 1", # Only Test Suite: all
+ "status" : "FAIL" # Only Test Suite: all
+ }, # Only Test Suite: all
+ { # Only Test Suite: all
+ "applicable" : false, # Only Test Suite: all
+ "hasPass" : true, # Only Test Suite: all
+ "name" : "NOT Applicable 2", # Only Test Suite: all
+ "status" : "FAIL" # Only Test Suite: all
+ }, # Only Test Suite: all
+ { # Only Test Suite: all
+ "applicable" : false, # Only Test Suite: all
+ "hasPass" : true, # Only Test Suite: all
+ "name" : "NOT Applicable 3", # Only Test Suite: all
+ "status" : "FAIL" # Only Test Suite: all
+ }
+ ]
+}
+
+[root "Root branch applicable Property"]
+ subtasks-factory = tasks-factory branch applicable Property
+
+[tasks-factory "tasks-factory branch applicable Property"]
+ names-factory = names-factory branch applicable Property
+ applicable = branch:master
+ fail = True
+
+[names-factory "names-factory branch applicable Property"]
+ type = static
+ name = Applicable 1
+ name = Applicable 2
+ name = Applicable 3
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root branch applicable Property",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Applicable 1",
+ "status" : "FAIL"
+ },
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Applicable 2",
+ "status" : "FAIL"
+ },
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Applicable 3",
+ "status" : "FAIL"
+ }
+ ]
+}
+
[root "Root Properties tasks-factory STATIC"]
subtasks-factory = tasks-factory STATIC Properties
@@ -1393,6 +1486,46 @@
]
}
+[root "Root CHANGE constant subtask list CHANGE Properties"]
+ subtasks-factory = tasks-factory Properties CHANGE names-factory CHANGE
+
+[tasks-factory "tasks-factory Properties CHANGE names-factory CHANGE"]
+ names-factory = Properties names-factory current CHANGE
+ subtask = Current CHANGE Property
+
+[task "Current CHANGE Property"]
+ fail = True
+ fail-hint = Current Change: ${_change_number}
+
+[names-factory "Properties names-factory current CHANGE"]
+ type = change
+ changes = change:${_change_number}
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root CHANGE constant subtask list CHANGE Properties",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "change" : _change_number,
+ "hasPass" : false,
+ "name" : "_change_number",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "hint" : "Current Change: _change_number",
+ "name" : "Current CHANGE Property",
+ "status" : "FAIL"
+ }
+ ]
+ }
+ ]
+}
+
[root "Root Preload"]
preload-task = Subtask FAIL
subtask = Subtask Preload
@@ -1814,6 +1947,213 @@
]
}
+[root "Root Looping"]
+ subtask = Looping
+
+[task "Looping"]
+ subtask = Looping
+ pass = True
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root Looping",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : true,
+ "name" : "Looping",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "hint" : "Duplicate task is non blocking and empty to break the loop",
+ "name" : "Looping",
+ "status" : "DUPLICATE"
+ }
+ ]
+ }
+ ]
+}
+
+[root "Root Looping DuplicateKey"]
+ preload-task = DuplicateKey
+
+[task "Looping DuplicateKey"]
+ preload-task = DuplicateKey
+ pass = True
+
+[task "DuplicateKey"]
+ duplicate-key = 1234
+ subtask = Looping DuplicateKey
+
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root Looping DuplicateKey",
+ "status" : "PASS",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "hint" : "Duplicate task is non blocking and empty to break the loop",
+ "name" : "Looping DuplicateKey",
+ "status" : "DUPLICATE"
+ }
+ ]
+}
+
+[root "Root changes loop"]
+ subtask = task (tasks-factory changes loop)
+
+[task "task (tasks-factory changes loop)"]
+ subtasks-factory = tasks-factory change loop
+
+[tasks-factory "tasks-factory change loop"]
+ names-factory = names-factory change constant
+ subtask = task (tasks-factory changes loop)
+ fail = True
+
+[names-factory "names-factory change constant"]
+ changes = change:_change1_number OR change:_change2_number
+ type = change
+
+{
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Root changes loop",
+ "status" : "WAITING",
+ "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" : "WAITING",
+ "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" : true,
+ "name" : "_change2_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" : true,
+ "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"
+ }
+ ]
+ }
+ ]
+ }
+ ]
+ }
+ ]
+}
+
[root "Root INVALID Preload"]
preload-task = missing
@@ -1946,18 +2286,6 @@
"status" : "INVALID" # Only Test Suite: !all
},
{
- "applicable" : true,
- "hasPass" : false,
- "name" : "Looping",
- "status" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- },
- {
"name" : "UNKNOWN",
"status" : "INVALID"
},
@@ -2050,106 +2378,6 @@
"status" : "INVALID"
}
]
- },
- {
- "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" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "applicable" : true,
- "change" : _change2_number,
- "hasPass" : true,
- "name" : "_change2_number",
- "status" : "FAIL",
- "subTasks" : [
- {
- "applicable" : true,
- "hasPass" : false,
- "name" : "task (tasks-factory changes loop)",
- "status" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- }
- ]
- }
- ]
- },
- {
- "applicable" : true,
- "change" : _change2_number,
- "hasPass" : true,
- "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" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- }
- ]
}
]
}
@@ -2301,18 +2529,6 @@
"status" : "INVALID" # Only Test Suite: !all
},
{
- "applicable" : true,
- "hasPass" : false,
- "name" : "Looping",
- "status" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- },
- {
"name" : "UNKNOWN",
"status" : "INVALID"
},
@@ -2405,106 +2621,6 @@
"status" : "INVALID"
}
]
- },
- {
- "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" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "applicable" : true,
- "change" : _change2_number,
- "hasPass" : true,
- "name" : "_change2_number",
- "status" : "FAIL",
- "subTasks" : [
- {
- "applicable" : true,
- "hasPass" : false,
- "name" : "task (tasks-factory changes loop)",
- "status" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- }
- ]
- }
- ]
- },
- {
- "applicable" : true,
- "change" : _change2_number,
- "hasPass" : true,
- "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" : "WAITING",
- "subTasks" : [
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- },
- {
- "name" : "UNKNOWN",
- "status" : "INVALID"
- }
- ]
- }
- ]
- }
- ]
}
]
}
@@ -2568,9 +2684,6 @@
fail = True
in-progress = has:bad
-[task "Looping"]
- subtask = Looping
-
[task "Looping Properties"]
set-A = ${B}
set-B = ${A}
@@ -2598,9 +2711,6 @@
[task "task (names-factory changes invalid)"]
subtasks-factory = tasks-factory change (names-factory changes invalid)
-[task "task (tasks-factory changes loop)"]
- subtasks-factory = tasks-factory change loop
-
[tasks-factory "tasks-factory (names-factory type missing)"]
names-factory = names-factory (type missing)
fail = True
@@ -2624,11 +2734,6 @@
names-factory = names-factory change list (changes invalid)
fail = True
-[tasks-factory "tasks-factory change loop"]
- names-factory = names-factory change constant
- subtask = task (tasks-factory changes loop)
- fail = True
-
[names-factory "names-factory (type missing)"]
name = no type test
@@ -2648,11 +2753,7 @@
type = change
[names-factory "names-factory change list (changes invalid)"]
- change = change:invalidChange
- type = change
-
-[names-factory "names-factory change constant"]
- changes = change:_change1_number OR change:_change2_number
+ changes = change:invalidChange
type = change
```
diff --git a/src/test/java/com/googlesource/gerrit/plugins/task/TaskExpressionTest.java b/src/test/java/com/googlesource/gerrit/plugins/task/TaskExpressionTest.java
index ac4ee88..1f7f529 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/task/TaskExpressionTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/task/TaskExpressionTest.java
@@ -34,107 +34,111 @@
public static String PEACE = "peace";
public static FileKey file = createFileKey("foo", "bar", "baz");
+ public static TaskKey SIMPLE_TASK = TaskKey.create(file, SIMPLE);
+ public static TaskKey WORLD_TASK = TaskKey.create(file, WORLD);
+ public static TaskKey PEACE_TASK = TaskKey.create(file, PEACE);
+
public void testBlank() {
TaskExpression exp = getTaskExpression("");
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
assertNoSuchElementException(it);
}
public void testRequiredSingleName() {
TaskExpression exp = getTaskExpression(SIMPLE);
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
- assertEquals(it.next(), SIMPLE);
+ assertEquals(it.next(), SIMPLE_TASK);
assertTrue(it.hasNext());
assertNoSuchElementException(it);
}
public void testOptionalSingleName() {
TaskExpression exp = getTaskExpression(SIMPLE + "|");
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
- assertEquals(it.next(), SIMPLE);
+ assertEquals(it.next(), SIMPLE_TASK);
assertFalse(it.hasNext());
}
public void testRequiredTwoNames() {
TaskExpression exp = getTaskExpression(WORLD + "|" + PEACE);
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
- assertEquals(it.next(), WORLD);
+ assertEquals(it.next(), WORLD_TASK);
assertTrue(it.hasNext());
- assertEquals(it.next(), PEACE);
+ assertEquals(it.next(), PEACE_TASK);
assertTrue(it.hasNext());
assertNoSuchElementException(it);
}
public void testOptionalTwoNames() {
TaskExpression exp = getTaskExpression(WORLD + "|" + PEACE + "|");
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
- assertEquals(it.next(), WORLD);
+ assertEquals(it.next(), WORLD_TASK);
assertTrue(it.hasNext());
- assertEquals(it.next(), PEACE);
+ assertEquals(it.next(), PEACE_TASK);
assertFalse(it.hasNext());
}
public void testBlankSpaces() {
TaskExpression exp = getTaskExpression(" ");
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
assertNoSuchElementException(it);
}
public void testRequiredSingleNameLeadingSpaces() {
TaskExpression exp = getTaskExpression(" " + SIMPLE);
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
- assertEquals(it.next(), SIMPLE);
+ assertEquals(it.next(), SIMPLE_TASK);
assertTrue(it.hasNext());
assertNoSuchElementException(it);
}
public void testRequiredSingleNameTrailingSpaces() {
TaskExpression exp = getTaskExpression(SIMPLE + " ");
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
- assertEquals(it.next(), SIMPLE);
+ assertEquals(it.next(), SIMPLE_TASK);
assertTrue(it.hasNext());
assertNoSuchElementException(it);
}
public void testOptionalSingleNameLeadingSpaces() {
TaskExpression exp = getTaskExpression(" " + SIMPLE + "|");
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
- assertEquals(it.next(), SIMPLE);
+ assertEquals(it.next(), SIMPLE_TASK);
assertFalse(it.hasNext());
}
public void testOptionalSingleNameTrailingSpaces() {
TaskExpression exp = getTaskExpression(SIMPLE + "| ");
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
- assertEquals(it.next(), SIMPLE);
+ assertEquals(it.next(), SIMPLE_TASK);
assertFalse(it.hasNext());
}
public void testOptionalSingleNameMiddleSpaces() {
TaskExpression exp = getTaskExpression(SIMPLE + " |");
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
- assertEquals(it.next(), SIMPLE);
+ assertEquals(it.next(), SIMPLE_TASK);
assertFalse(it.hasNext());
}
public void testRequiredTwoNamesMiddleSpaces() {
TaskExpression exp = getTaskExpression(WORLD + " | " + PEACE);
- Iterator<String> it = exp.iterator();
+ Iterator<TaskKey> it = exp.iterator();
assertTrue(it.hasNext());
- assertEquals(it.next(), WORLD);
+ assertEquals(it.next(), WORLD_TASK);
assertTrue(it.hasNext());
- assertEquals(it.next(), PEACE);
+ assertEquals(it.next(), PEACE_TASK);
assertTrue(it.hasNext());
assertNoSuchElementException(it);
}
@@ -163,7 +167,7 @@
assertFalse(exp.key.equals(otherExp.key));
}
- protected static void assertNoSuchElementException(Iterator<String> it) {
+ protected static void assertNoSuchElementException(Iterator<TaskKey> it) {
try {
it.next();
assertTrue(false);
diff --git a/test/strip_non_applicable.py b/test/strip_non_applicable.py
index 1ff097a..41c21fa 100755
--- a/test/strip_non_applicable.py
+++ b/test/strip_non_applicable.py
@@ -43,7 +43,7 @@
status=''
if STATUS in task.keys():
status = task[STATUS]
- if status != 'INVALID':
+ if status != 'INVALID' and status != 'DUPLICATE':
del tasks[i]
nexti = i
