Explicitly signal end of Task Properties expansion
Previously knowing whether a node's subnodes needed to be reloaded was
only determined at the time a node was refreshed. This was a bit clunky
as it required looking at data from the previous cycle to determine
this. Instead, explicitly signal the completion of node loading so that
this can be evaluated right away without waiting until the next cycle.
This makes the code a bit easier to reason about and less fragile, and
it paves the way for some smarter caching policies which can release
more memory after processing a node instead of the next time the node is
(potentially) loaded.
Change-Id: I875a68f1e988fc75e41a7d6872edec76ef618474
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 57c12ee..3568eb8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java
@@ -58,17 +58,8 @@
/** Use to expand properties specifically for Tasks. */
public Task getTask(ChangeData changeData) throws OrmException {
- if (loader != null && loader.isNonTaskDefinedPropertyLoaded()) {
- // To detect NamesFactories dependent on non task defined properties, the checking must be
- // done after subnodes are fully loaded, which unfortunately happens after getTask() is
- // called. However, these non task property uses from the last change are still detectable
- // here before we replace the old Loader with a new one.
- isSubNodeReloadRequired = true;
- }
-
loader = new Loader(changeData);
expander = new Expander(n -> loader.load(n));
-
if (isTaskRefreshNeeded || init) {
Map<String, String> exported = expander.expand(origTask.exported);
if (exported != origTask.exported) {
@@ -85,6 +76,13 @@
return task.getForRead();
}
+ // To detect NamesFactories dependent on non task defined properties, the checking must be
+ // done after subnodes are fully loaded, which unfortunately happens after getTask() is
+ // called, therefore this must be called after all subnodes have been loaded.
+ public void expansionComplete() {
+ isSubNodeReloadRequired = loader.isNonTaskDefinedPropertyLoaded();
+ }
+
public boolean isSubNodeReloadRequired() {
return isSubNodeReloadRequired;
}
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 d51f5bb..6df4b9d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -114,15 +114,14 @@
public List<Node> getSubNodes() throws ConfigInvalidException, IOException, OrmException {
if (cachedNodes == null) {
- cachedNodes = createAdder().getSubNodes();
- } else {
- refreshSubNodes();
+ return loadSubNodes();
}
+ refreshSubNodes();
return cachedNodes;
}
- protected SubNodeAdder createAdder() {
- return new SubNodeAdder();
+ protected List<Node> loadSubNodes() throws ConfigInvalidException, IOException, OrmException {
+ return cachedNodes = new SubNodeAdder().getSubNodes();
}
public void refreshSubNodes() throws ConfigInvalidException, OrmException {
@@ -219,8 +218,10 @@
}
@Override
- protected SubNodeAdder createAdder() {
- return new SubNodeAdder();
+ protected List<Node> loadSubNodes() throws ConfigInvalidException, IOException, OrmException {
+ cachedNodes = new SubNodeAdder().getSubNodes();
+ properties.expansionComplete();
+ return cachedNodes;
}
/* The task needs to be refreshed before a node is used, however
