commit 871238cb8ca35930b187b6dbe3b95cb93a389f80
author Martin Fick <mfick@codeaurora.org> Tue Apr 13 15:52:45 2021 -0600
committer Aditya Gupta <agaditya@codeaurora.org> Tue May 25 11:58:57 2021 +0530
tree df89df16deecb3276d48fcb246abfdab1e719f42
parent b4748b32c3600e5831919fe0794c788f9019c275

Add TaskTree definitions more directly

Instead of putting all the task configs into a List and then adding the
List, add the task configs to the nodelist directly. This eliminates a
bunch of boiler plate "collection" code.

Change-Id: I431b32dcd683dd94a6f5b730fe0c6ad90942816c

src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java

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 efb4a39..5f29f00 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -163,18 +163,31 @@
     }
 
     protected void addSubDefinitions() throws OrmException {
-      addSubDefinitions(getSubDefinitions());
-      addSubDefinitions(getTasksFactoryDefinitions());
+      addSubTaskDefinitions();
+      addSubTasksFactoryDefinitions();
       addSubFileDefinitions();
       addExternalDefinitions();
     }
 
+    protected void addSubTaskDefinitions() {
+      for (String name : task.subTasks) {
+        try {
+          Task def = task.config.getTaskOptional(name);
+          if (def != null) {
+            addSubDefinition(def);
+          }
+        } catch (ConfigInvalidException e) {
+          addSubDefinition(null);
+        }
+      }
+    }
+
     protected void addSubFileDefinitions() {
       for (String file : task.subTasksFiles) {
         try {
           addSubDefinitions(getTaskDefinitions(task.config.getBranch(), file));
         } catch (ConfigInvalidException | IOException e) {
-          nodes.add(null);
+          addSubDefinition(null);
         }
       }
     }
@@ -184,32 +197,17 @@
         try {
           External ext = task.config.getExternal(external);
           if (ext == null) {
-            nodes.add(null);
+            addSubDefinition(null);
           } else {
             addSubDefinitions(getTaskDefinitions(ext));
           }
         } catch (ConfigInvalidException | IOException e) {
-          nodes.add(null);
+          addSubDefinition(null);
         }
       }
     }
 
-    protected List<Task> getSubDefinitions() {
-      List<Task> defs = new ArrayList<>();
-      for (String name : task.subTasks) {
-        try {
-          Task def = task.config.getTaskOptional(name);
-          if (def != null) {
-            defs.add(def);
-          }
-        } catch (ConfigInvalidException e) {
-          defs.add(null);
-        }
-      }
-      return defs;
-    }
-
-    protected List<Task> getTasksFactoryDefinitions() throws OrmException {
+    protected void addSubTasksFactoryDefinitions() throws OrmException {
       List<Task> taskList = new ArrayList<>();
       for (String taskFactoryName : task.subTasksFactories) {
         TasksFactory tasksFactory = task.config.getTasksFactory(taskFactoryName);
@@ -219,28 +217,27 @@
             new Properties(namesFactory, task.properties);
             switch (NamesFactoryType.getNamesFactoryType(namesFactory.type)) {
               case STATIC:
-                getStaticTypeTasksDefinitions(tasksFactory, namesFactory, taskList);
+                addStaticTypeTasksDefinitions(tasksFactory, namesFactory);
                 continue;
               case CHANGE:
-                getChangesTypeTaskDefinitions(tasksFactory, namesFactory, taskList);
+                addChangesTypeTaskDefinitions(tasksFactory, namesFactory);
                 continue;
             }
           }
         }
-        taskList.add(null);
+        addSubDefinition(null);
       }
-      return taskList;
     }
 
-    protected void getStaticTypeTasksDefinitions(
-        TasksFactory tasksFactory, NamesFactory namesFactory, List<Task> taskList) {
+    protected void addStaticTypeTasksDefinitions(
+        TasksFactory tasksFactory, NamesFactory namesFactory) {
       for (String name : namesFactory.names) {
-        taskList.add(task.config.createTask(tasksFactory, name));
+        addSubDefinition(task.config.createTask(tasksFactory, name));
       }
     }
 
-    protected void getChangesTypeTaskDefinitions(
-        TasksFactory tasksFactory, NamesFactory namesFactory, List<Task> taskList) {
+    protected void addChangesTypeTaskDefinitions(
+        TasksFactory tasksFactory, NamesFactory namesFactory) {
       try {
         if (namesFactory.changes != null) {
           List<ChangeData> changeDataList =
@@ -248,7 +245,7 @@
                   .get()
                   .query(changeQueryBuilderProvider.get().parse(namesFactory.changes)).entities();
           for (ChangeData changeData : changeDataList) {
-            taskList.add(task.config.createTask(tasksFactory, changeData.getId().toString()));
+            addSubDefinition(task.config.createTask(tasksFactory, changeData.getId().toString()));
           }
           return;
         }
@@ -256,7 +253,7 @@
         log.atSevere().withCause(e).log("ERROR: running changes query: " + namesFactory.changes);
       } catch (QueryParseException e) {
       }
-      taskList.add(null);
+      addSubDefinition(null);
     }
 
     protected List<Task> getTaskDefinitions(External external)