commit 2e2f27e7ff6870a98031d8edf3f8137039754d77
author Martin Fick <martin.fick@linaro.org> Mon Jul 25 16:43:20 2022 -0600
committer Martin Fick <martin.fick@linaro.org> Wed Sep 07 15:39:45 2022 -0600
tree dca1507277f79fd56179c3b3fb466965c80e8b6d
parent 0d65e85e93ac4181fc19d816d600dd4da7c18393

Moving caching logic out of loadSubNodes()

Since loadSubNodes() was no longer following a loader pattern, and it
was getting involved in caching policies which violates normal loader
expectations, move the caching logic back to getSubNodes() and reduce
the complexity of the loaders. Since Node was not previously overriding
getSubNodes(), overriding this now results in a nice place for the
caching logic again and it helps emphasize that it is in fact different
from its superclass' caching policy.

Change-Id: Ibb6dedaf4fc4de4ffb9b763c8bd68c59b66b1974

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 058bb2a..d551137 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -113,14 +113,14 @@
     protected List<Node> cachedNodes;
 
     public List<Node> getSubNodes() throws ConfigInvalidException, IOException, OrmException {
-      if (cachedNodes == null) {
-        return loadSubNodes();
+      if (cachedNodes != null) {
+        return refresh(cachedNodes);
       }
-      return refresh(cachedNodes);
+      return cachedNodes = loadSubNodes();
     }
 
     protected List<Node> loadSubNodes() throws ConfigInvalidException, IOException, OrmException {
-      return cachedNodes = new SubNodeAdder().getSubNodes();
+      return new SubNodeAdder().getSubNodes();
     }
 
     public ChangeData getChangeData() {
@@ -214,6 +214,22 @@
       return String.valueOf(getChangeData().getId().get()) + TaskConfig.SEP + taskKey;
     }
 
+    public List<Node> getSubNodes() throws ConfigInvalidException, IOException, OrmException {
+      if (cachedNodes != null) {
+        return refresh(cachedNodes);
+      }
+      List<Node> nodes = loadSubNodes();
+      if (!properties.isSubNodeReloadRequired()) {
+        return cachedNodes = nodes;
+      }
+      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, OrmException {
       if (hasUnfilterableSubNodes) {
@@ -226,15 +242,6 @@
     protected List<Node> loadSubNodes() throws ConfigInvalidException, IOException, OrmException {
       List<Node> nodes = new SubNodeAdder().getSubNodes();
       properties.expansionComplete();
-      if (!properties.isSubNodeReloadRequired()) {
-        cachedNodes = nodes;
-      } else {
-        hasUnfilterableSubNodes = true;
-        cachedNodeByTask.clear();
-        nodes.stream()
-            .filter(n -> !(n instanceof Invalid) && !n.isChange())
-            .forEach(n -> cachedNodeByTask.put(n.task.key(), n));
-      }
       return nodes;
     }