Store Properties object instead of Map in TaskTree.Node
Pass around a Properties object to make variables named 'properties'
less confusing than Maps named properties, and also enable better type
checking. This additionally paves the way for storing and passing around
more information besides just the Map. To pass around the correct
Properties types, split the Properties class into two classes, one for
Tasks, and one for NamesFactories. Enable accessing the valueByName map
for descendants and NamesFactories by extending the Expander class now
that the main types can do this (the Properties class could not extend
an inner class of itself).
Change-Id: Ia3eee75b2ae0e5a16c8918c662c85c8a3cb2824d
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 ff8dfad..ed38fb3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java
@@ -18,8 +18,6 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
-import com.googlesource.gerrit.plugins.task.TaskConfig.NamesFactory;
-import com.googlesource.gerrit.plugins.task.TaskConfig.Task;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Collections;
@@ -32,25 +30,42 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-/** Use to expand properties like ${_name} for a Task Definition. */
+/** Use to expand properties like ${_name} in the text of various definitions. */
public class Properties {
- public Properties(ChangeData changeData, Task definition, Map<String, String> parentProperties)
- throws OrmException {
- Map<String, String> expanded = new HashMap<>(parentProperties);
- expanded.putAll(getInternalProperties(definition, changeData));
- new RecursiveExpander(expanded).expand(definition.getAllProperties());
+ /** Use to expand properties specifically for Tasks. */
+ public static class Task extends Expander {
+ public static final Task EMPTY_PARENT = new Task();
- definition.setExpandedProperties(expanded);
+ public Task() {
+ super(Collections.emptyMap());
+ }
- new Expander(expanded).expandFieldValues(definition, Collections.emptySet());
+ public Task(ChangeData changeData, TaskConfig.Task definition, Task parentProperties)
+ throws OrmException {
+ super(parentProperties.forDescendants());
+ valueByName.putAll(getInternalProperties(definition, changeData));
+ new RecursiveExpander(valueByName).expand(definition.getAllProperties());
+
+ definition.setExpandedProperties(valueByName);
+
+ expandFieldValues(definition, Collections.emptySet());
+ }
+
+ protected Map<String, String> forDescendants() {
+ return new HashMap<>(valueByName);
+ }
}
- public Properties(NamesFactory namesFactory, Map<String, String> properties) {
- new Expander(properties).expandFieldValues(namesFactory, Sets.newHashSet(TaskConfig.KEY_TYPE));
+ /** Use to expand properties specifically for NamesFactories. */
+ public static class NamesFactory extends Expander {
+ public NamesFactory(TaskConfig.NamesFactory namesFactory, Task properties) {
+ super(properties.valueByName);
+ expandFieldValues(namesFactory, Sets.newHashSet(TaskConfig.KEY_TYPE));
+ }
}
- protected static Map<String, String> getInternalProperties(Task definition, ChangeData changeData)
- throws OrmException {
+ protected static Map<String, String> getInternalProperties(
+ TaskConfig.Task definition, ChangeData changeData) throws OrmException {
Map<String, String> properties = new HashMap<>();
properties.put("_name", definition.name);
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 b1e686b..f1979a3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -39,11 +39,9 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
-import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -97,7 +95,6 @@
protected LinkedList<String> path = new LinkedList<>();
protected List<Node> nodes;
protected Set<String> names = new HashSet<>();
- protected Map<String, String> properties;
protected void addSubDefinitions(List<Task> defs) {
for (Task def : defs) {
@@ -111,7 +108,7 @@
// path check above detects looping definitions
// names check above detects duplicate subtasks
try {
- node = new Node(def, path, properties);
+ node = new Node(def, path, getProperties());
} catch (Exception e) {
} // bad definition, handled with null
}
@@ -121,13 +118,13 @@
public ChangeData getChangeData() {
return TaskTree.this.changeData;
}
+
+ protected Properties.Task getProperties() {
+ return Properties.Task.EMPTY_PARENT;
+ }
}
protected class Root extends NodeList {
- protected Root() {
- properties = new HashMap<String, String>();
- }
-
public List<Node> getRootNodes() throws ConfigInvalidException, IOException {
if (nodes == null) {
nodes = new ArrayList<>();
@@ -143,15 +140,15 @@
public class Node extends NodeList {
public final Task task;
+ protected final Properties.Task properties;
- public Node(Task definition, List<String> path, Map<String, String> parentProperties)
+ public Node(Task definition, List<String> path, Properties.Task parentProperties)
throws ConfigInvalidException, OrmException {
this.task = definition;
this.path.addAll(path);
this.path.add(definition.name);
Preloader.preload(definition);
- new Properties(getChangeData(), definition, parentProperties);
- properties = definition.properties;
+ properties = new Properties.Task(getChangeData(), definition, parentProperties);
}
public List<Node> getSubNodes() throws OrmException {
@@ -214,7 +211,7 @@
if (tasksFactory != null) {
NamesFactory namesFactory = task.config.getNamesFactory(tasksFactory.namesFactory);
if (namesFactory != null && namesFactory.type != null) {
- new Properties(namesFactory, task.properties);
+ new Properties.NamesFactory(namesFactory, getProperties());
switch (NamesFactoryType.getNamesFactoryType(namesFactory.type)) {
case STATIC:
addStaticTypeTasksDefinitions(tasksFactory, namesFactory);
@@ -269,6 +266,11 @@
.getTasks();
}
+ @Override
+ protected Properties.Task getProperties() {
+ return properties;
+ }
+
protected String resolveTaskFileName(String file) throws ConfigInvalidException {
if (file == null) {
throw new ConfigInvalidException("External file not defined");
