Refactor TaskDefinitions
Refactor how they are named, created, and looked up.
Change-Id: Ie83011dc61ddd46ec49191edd7cb0e5958454c5d
diff --git a/src/main/java/com/google/gerrit/common/Container.java b/src/main/java/com/google/gerrit/common/Container.java
new file mode 100644
index 0000000..62b672a
--- /dev/null
+++ b/src/main/java/com/google/gerrit/common/Container.java
@@ -0,0 +1,58 @@
+// Copyright (C) 2016 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.google.gerrit.common;
+
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+/* A data container, all fields considered in equals and hash */
+public class Container {
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+
+ try {
+ for (Field field : getClass().getDeclaredFields()) {
+ field.setAccessible(true);
+ if (!Objects.deepEquals(field.get(this), field.get(o))) {
+ return false;
+ }
+ }
+ } catch (IllegalArgumentException | IllegalAccessException e) {
+ throw new RuntimeException();
+ }
+ return true;
+ }
+
+ @Override
+ public int hashCode() {
+ List values = new ArrayList();
+ try {
+ for (Field field : getClass().getDeclaredFields()) {
+ field.setAccessible(true);
+ values.add(field.get(this));
+ }
+ } catch (IllegalArgumentException | IllegalAccessException e) {
+ }
+ return Objects.hash(values);
+ }
+}
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 ed6743f..298b1d7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
@@ -23,6 +23,7 @@
import com.google.gerrit.server.query.change.ChangeQueryProcessor.ChangeAttributeFactory;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.task.TaskConfig.Task;
import java.io.IOException;
import java.util.ArrayList;
import java.util.LinkedList;
@@ -84,9 +85,9 @@
throws OrmException, QueryParseException {
TaskPluginAttribute a = new TaskPluginAttribute();
try {
- LinkedList<TaskDefinition> path = new LinkedList<>();
- for (TaskDefinition def : getRootTaskDefinitions()) {
- addApplicableTasks(a.roots, c, path, def);
+ LinkedList<Task> path = new LinkedList<>();
+ for (Task task : getRootTasks()) {
+ addApplicableTasks(a.roots, c, path, task);
}
} catch (ConfigInvalidException | IOException e) {
a.roots.add(invalid());
@@ -98,27 +99,8 @@
return a;
}
- protected List<TaskAttribute> getSubTasks(
- ChangeData c, LinkedList<TaskDefinition> path, TaskDefinition parent)
- throws OrmException, QueryParseException {
- List<TaskAttribute> subTasks = new ArrayList<>();
- for (String name : parent.subTasks) {
- try {
- TaskDefinition def = getTaskDefinition(parent, name);
- addApplicableTasks(subTasks, c, path, def);
- } catch (ConfigInvalidException | IOException e) {
- subTasks.add(invalid());
- }
- }
-
- if (subTasks.isEmpty()) {
- return null;
- }
- return subTasks;
- }
-
protected void addApplicableTasks(
- List<TaskAttribute> tasks, ChangeData c, LinkedList<TaskDefinition> path, TaskDefinition def)
+ List<TaskAttribute> tasks, ChangeData c, LinkedList<Task> path, Task def)
throws OrmException, QueryParseException {
if (path.contains(def)) { // looping definition
tasks.add(invalid());
@@ -130,7 +112,7 @@
}
protected void addApplicableTasksNoLoopCheck(
- List<TaskAttribute> tasks, ChangeData c, LinkedList<TaskDefinition> path, TaskDefinition def)
+ List<TaskAttribute> tasks, ChangeData c, LinkedList<Task> path, Task def)
throws OrmException {
try {
if (match(c, def.applicable)) {
@@ -150,6 +132,19 @@
}
}
+ protected List<TaskAttribute> getSubTasks(ChangeData c, LinkedList<Task> path, Task parent)
+ throws OrmException, QueryParseException {
+ List<TaskAttribute> subTasks = new ArrayList<>();
+ for (Task task : getSubTasks(parent)) {
+ addApplicableTasks(subTasks, c, path, task);
+ }
+
+ if (subTasks.isEmpty()) {
+ return null;
+ }
+ return subTasks;
+ }
+
protected static TaskAttribute invalid() {
// For security reasons, do not expose the task name without knowing
// the visibility which is derived from its applicability.
@@ -158,26 +153,28 @@
return a;
}
- protected List<TaskDefinition> getRootTaskDefinitions()
- throws ConfigInvalidException, IOException {
- return taskFactory.getRootConfig().getRootTaskDefinitions();
+ protected List<Task> getRootTasks() throws ConfigInvalidException, IOException {
+ return taskFactory.getRootConfig().getRootTasks();
}
- protected TaskDefinition getTaskDefinition(TaskDefinition parent, String name)
- throws ConfigInvalidException, IOException {
- return taskFactory.getTaskConfig(parent.branch, parent.fileName).getTaskDefinition(name);
+ protected List<Task> getSubTasks(Task parent) {
+ List<Task> tasks = new ArrayList<>();
+ for (String name : parent.subTasks) {
+ tasks.add(parent.config.getTask(name));
+ }
+ return tasks;
}
- protected Status getStatus(ChangeData c, TaskDefinition def, TaskAttribute a)
+ protected Status getStatus(ChangeData c, Task task, TaskAttribute a)
throws OrmException, QueryParseException {
- if (def.pass == null && a.subTasks == null) {
+ if (task.pass == null && a.subTasks == null) {
// A leaf without a PASS criteria is likely a missconfiguration.
// Either someone forgot to add subtasks, or they forgot to add
// the pass criteria.
return Status.INVALID;
}
- if (def.fail != null && match(c, def.fail)) {
+ if (task.fail != null && match(c, task.fail)) {
// A FAIL definition is meant to be a hard blocking criteria
// (like a CodeReview -2). Thus, if hard blocked, it is
// irrelevant what the subtask states, or the pass criteria are.
@@ -202,7 +199,7 @@
return Status.WAITING;
}
- if (def.pass != null && !match(c, def.pass)) {
+ if (task.pass != null && !match(c, task.pass)) {
// Non-leaf tasks with no PASS criteria are supported in order
// to support "grouping tasks" (tasks with no function aside from
// organizing tasks). A task without a PASS criteria, cannot ever
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 818b0fa..3334449 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.task;
+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;
@@ -22,6 +23,34 @@
/** Task Configuration file living in git */
public class TaskConfig extends AbstractVersionedMetaData {
+ protected class Section extends Container {
+ public TaskConfig config;
+
+ public Section() {
+ this.config = TaskConfig.this;
+ }
+ }
+
+ public class Task extends Section {
+ public String applicable;
+ public String fail;
+ public String inProgress;
+ public String name;
+ public String pass;
+ public String readyHint;
+ public List<String> subTasks;
+
+ public Task(SubSection s) {
+ applicable = getString(s, KEY_APPLICABLE, null);
+ fail = getString(s, KEY_FAIL, null);
+ inProgress = getString(s, KEY_IN_PROGRESS, null);
+ name = getString(s, KEY_NAME, s.subSection);
+ pass = getString(s, KEY_PASS, null);
+ readyHint = getString(s, KEY_READY_HINT, null);
+ subTasks = getStringList(s, KEY_SUBTASK);
+ }
+ }
+
protected static final String SECTION_ROOT = "root";
protected static final String SECTION_TASK = "task";
protected static final String KEY_APPLICABLE = "applicable";
@@ -36,49 +65,41 @@
super(branch, fileName);
}
- public List<TaskDefinition> getRootTaskDefinitions() {
- List<TaskDefinition> roots = new ArrayList<TaskDefinition>();
- // No need to get a root with no name (what would we call it?)
- for (String root : cfg.getSubsections(SECTION_ROOT)) {
- roots.add(getRootDefinition(root));
+ public List<Task> getRootTasks() {
+ return getTasks(SECTION_ROOT);
+ }
+
+ public List<Task> getTasks() {
+ return getTasks(SECTION_TASK);
+ }
+
+ 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(new SubSection(type, task)));
}
- return roots;
+ return tasks;
}
- protected TaskDefinition getRootDefinition(String name) {
- return getTaskDefinition(new Section(SECTION_ROOT, name));
+ public Task getTask(String name) {
+ return new Task(new SubSection(SECTION_TASK, name));
}
- public TaskDefinition getTaskDefinition(String name) {
- return getTaskDefinition(new Section(SECTION_TASK, name));
- }
-
- protected TaskDefinition getTaskDefinition(Section s) {
- TaskDefinition task = new TaskDefinition(branch, fileName);
- task.applicable = getString(s, KEY_APPLICABLE, null);
- task.fail = getString(s, KEY_FAIL, null);
- task.inProgress = getString(s, KEY_IN_PROGRESS, null);
- task.name = getString(s, KEY_NAME, s.subSection);
- task.pass = getString(s, KEY_PASS, null);
- task.readyHint = getString(s, KEY_READY_HINT, null);
- task.subTasks = getStringList(s, KEY_SUBTASK);
- return task;
- }
-
- protected String getString(Section s, String key, String def) {
+ protected String getString(SubSection s, String key, String def) {
String v = cfg.getString(s.section, s.subSection, key);
return v != null ? v : def;
}
- protected List<String> getStringList(Section s, String key) {
+ protected List<String> getStringList(SubSection s, String key) {
return Arrays.asList(cfg.getStringList(s.section, s.subSection, key));
}
- protected static class Section {
+ protected static class SubSection {
public final String section;
public final String subSection;
- protected Section(String section, String subSection) {
+ protected SubSection(String section, String subSection) {
this.section = section;
this.subSection = subSection;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskDefinition.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskDefinition.java
deleted file mode 100644
index 491d88b..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskDefinition.java
+++ /dev/null
@@ -1,63 +0,0 @@
-// Copyright (C) 2016 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;
-
-import com.google.gerrit.reviewdb.client.Branch;
-import java.util.List;
-import java.util.Objects;
-
-public class TaskDefinition {
- public Branch.NameKey branch;
- public String fileName;
-
- public String applicable;
- public String fail;
- public String inProgress;
- public String name;
- public String readyHint;
- public String pass;
- public List<String> subTasks;
-
- public TaskDefinition(Branch.NameKey branch, String fileName) {
- this.branch = branch;
- this.fileName = fileName;
- }
-
- @Override
- public boolean equals(Object o) {
- if (o == this) {
- return true;
- }
- if (o == null || !(o instanceof TaskDefinition)) {
- return false;
- }
- TaskDefinition t = (TaskDefinition) o;
- return Objects.equals(branch, t.branch)
- && Objects.equals(fileName, t.fileName)
- && Objects.equals(applicable, t.applicable)
- && Objects.equals(fail, t.fail)
- && Objects.equals(inProgress, t.inProgress)
- && Objects.equals(name, t.name)
- && Objects.equals(readyHint, t.readyHint)
- && Objects.equals(pass, t.pass)
- && Objects.equals(subTasks, t.subTasks);
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(
- branch, fileName, applicable, fail, inProgress, name, pass, readyHint, subTasks);
- }
-}
