Add logic to handle blank subtask and blank name in names-factory

Before the change, NPE was thrown in case the subtask or name field in
names-factory is blank. This makes the parent task invalid which is not
ideal. Add logic to handle these cases so that only the concerned task
is marked as invalid.

Change-Id: I1f8faf06f7bc55b6619a1ef40bd40ee98bd3b51b

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

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 0c3136e..be7f120 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
@@ -300,8 +300,9 @@
   }
 
   protected List<String> getStringList(SubSectionKey s, String key) {
-    return Collections.unmodifiableList(
-        Arrays.asList(cfg.getStringList(s.section(), s.subSection(), key)));
+    List<String> stringList = Arrays.asList(cfg.getStringList(s.section(), s.subSection(), key));
+    stringList.replaceAll(str -> str == null ? "" : str);
+    return Collections.unmodifiableList(stringList);
   }
 
   protected SubSectionKey subSectionKey(String section, String subSection) {

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 ac65a0f..501e9db 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -58,6 +58,7 @@
 import java.util.Optional;
 import java.util.Set;
 import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.util.StringUtils;
 
 /**
  * Add structure to access the task definitions from the config as a tree.
@@ -430,7 +431,11 @@
       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)));
+          if (StringUtils.isEmptyOrNull(name)) {
+            addInvalidNode();
+          } else {
+            addPreloaded(preloader.preload(task.config.new Task(tasksFactory, name)));
+          }
         }
       }
 

src/main/resources/Documentation/test/preview.md

diff --git a/src/main/resources/Documentation/test/preview.md b/src/main/resources/Documentation/test/preview.md
index 6aff577..510f073 100644
--- a/src/main/resources/Documentation/test/preview.md
+++ b/src/main/resources/Documentation/test/preview.md
@@ -102,6 +102,18 @@
          ]
       },
       {
+         "applicable" : true,
+         "hasPass" : true,
+         "name" : "Subtask Blank",
+         "status" : "WAITING",
+         "subTasks" : [
+            {
+               "name" : "UNKNOWN",
+               "status" : "INVALID"
+            }
+         ]
+      },
+      {
          "applicable" : false,
          "hasPass" : true,
          "name" : "NA Bad PASS query",
@@ -164,6 +176,18 @@
       {
          "applicable" : true,
          "hasPass" : false,
+         "name" : "task (names-factory name Blank)",
+         "status" : "WAITING",
+         "subTasks" : [
+            {
+               "name" : "UNKNOWN",
+               "status" : "INVALID"
+            }
+         ]
+      },
+      {
+         "applicable" : true,
+         "hasPass" : false,
          "name" : "task (names-factory duplicate)",
          "status" : "WAITING",
          "subTasks" : [
@@ -404,6 +428,18 @@
          ]
       },
       {
+         "applicable" : true,
+         "hasPass" : true,
+         "name" : "Subtask Blank",
+         "status" : "WAITING",
+         "subTasks" : [
+            {
+               "name" : "UNKNOWN",
+               "status" : "INVALID"
+            }
+         ]
+      },
+      {
          "applicable" : false,
          "hasPass" : true,
          "name" : "NA Bad PASS query",
@@ -466,6 +502,18 @@
       {
          "applicable" : true,
          "hasPass" : false,
+         "name" : "task (names-factory name Blank)",
+         "status" : "WAITING",
+         "subTasks" : [
+            {
+               "name" : "UNKNOWN",
+               "status" : "INVALID"
+            }
+         ]
+      },
+      {
+         "applicable" : true,
+         "hasPass" : false,
          "name" : "task (names-factory duplicate)",
          "status" : "WAITING",
          "subTasks" : [

src/main/resources/Documentation/test/task_states.md

diff --git a/src/main/resources/Documentation/test/task_states.md b/src/main/resources/Documentation/test/task_states.md
index f48f528..f5d4cd8 100644
--- a/src/main/resources/Documentation/test/task_states.md
+++ b/src/main/resources/Documentation/test/task_states.md
@@ -2266,6 +2266,18 @@
          ]
       },
       {
+         "applicable" : true,
+         "hasPass" : true,
+         "name" : "Subtask Blank",
+         "status" : "WAITING",
+         "subTasks" : [
+            {
+               "name" : "UNKNOWN",
+               "status" : "INVALID"
+            }
+         ]
+      },
+      {
          "applicable" : false,
          "hasPass" : true,
          "name" : "NA Bad PASS query",
@@ -2328,6 +2340,18 @@
       {
          "applicable" : true,
          "hasPass" : false,
+         "name" : "task (names-factory name Blank)",
+         "status" : "WAITING",
+         "subTasks" : [
+            {
+               "name" : "UNKNOWN",
+               "status" : "INVALID"
+            }
+         ]
+      },
+      {
+         "applicable" : true,
+         "hasPass" : false,
          "name" : "task (names-factory duplicate)",
          "status" : "WAITING",
          "subTasks" : [
@@ -2509,6 +2533,18 @@
          ]
       },
       {
+         "applicable" : true,
+         "hasPass" : true,
+         "name" : "Subtask Blank",
+         "status" : "WAITING",
+         "subTasks" : [
+            {
+               "name" : "UNKNOWN",
+               "status" : "INVALID"
+            }
+         ]
+      },
+      {
          "applicable" : false,
          "hasPass" : true,
          "name" : "NA Bad PASS query",
@@ -2571,6 +2607,18 @@
       {
          "applicable" : true,
          "hasPass" : false,
+         "name" : "task (names-factory name Blank)",
+         "status" : "WAITING",
+         "subTasks" : [
+            {
+               "name" : "UNKNOWN",
+               "status" : "INVALID"
+            }
+         ]
+      },
+      {
+         "applicable" : true,
+         "hasPass" : false,
          "name" : "task (names-factory duplicate)",
          "status" : "WAITING",
          "subTasks" : [
@@ -2669,6 +2717,10 @@
 [task "Subtask Optional"]
    subtask = MISSING | MISSING
 
+[task "Subtask Blank"]
+  pass = True
+  subtask =
+
 [task "NA Bad PASS query"]
   applicable = NOT is:open # Assumes test query is "is:open"
   fail = True
@@ -2699,6 +2751,9 @@
 [task "task (names-factory type INVALID)"]
   subtasks-factory = tasks-factory (names-factory type INVALID)
 
+[task "task (names-factory name Blank)"]
+  subtasks-factory = tasks-factory (names-factory name Blank)
+
 [task "task (names-factory duplicate)"]
   subtasks-factory = tasks-factory (names-factory duplicate)
 
@@ -2718,6 +2773,10 @@
 [tasks-factory "tasks-factory (names-factory type INVALID)"]
   names-factory = name-factory (type INVALID)
 
+[tasks-factory "tasks-factory (names-factory name Blank)"]
+  names-factory = names-factory (name Blank)
+  fail = True
+
 [tasks-factory "tasks-factory (names-factory duplicate)"]
   names-factory = names-factory duplicate
   fail = True
@@ -2744,6 +2803,10 @@
   name = invalid type test
   type = invalid
 
+[names-factory "names-factory (name Blank)"]
+  name =
+  type = static
+
 [names-factory "names-factory duplicate"]
   name = duplicate
   name = duplicate