commit 171ff5f238b37d781364d66af7dd24fbb20a2ebf
author Patrick Hiesel <hiesel@google.com> Mon May 06 16:46:41 2019 +0200
committer Antonio Barone <syntonyze@gmail.com> Mon Aug 26 13:50:17 2019 +0200
tree 896c30b3b1b9b82b4b357943222f116cce83917b
parent 8e8bd2b6693e5455bfd4ee4c702cab516b2b8b8d

Don't store LabelTypes in ProjectState

ProjectState is only invalidated if the underlying config changes, but
not when any of its parent's config changes. Therefore it must not
store any data locally that it derived from any parent.

Bug: Issue 11248
Change-Id: I3f8d74e829274701c54d5898f6348971a59f866f

gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
index 08c3d9f..b5dfa95 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
@@ -75,7 +75,10 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/** Cached information on a project. */
+/**
+ * Cached information on a project. Must not contain any data derived from parents other than it's
+ * immediate parent's {@link Project.NameKey}.
+ */
 public class ProjectState {
   private static final Logger log = LoggerFactory.getLogger(ProjectState.class);
 
@@ -115,9 +118,6 @@
   /** If this is all projects, the capabilities used by the server. */
   private final CapabilityCollection capabilities;
 
-  /** All label types applicable to changes in this project. */
-  private LabelTypes labelTypes;
-
   @Inject
   public ProjectState(
       SitePaths sitePaths,
@@ -482,10 +482,23 @@
 
   /** All available label types. */
   public LabelTypes getLabelTypes() {
-    if (labelTypes == null) {
-      labelTypes = loadLabelTypes();
+    Map<String, LabelType> types = new LinkedHashMap<>();
+    for (ProjectState s : treeInOrder()) {
+      for (LabelType type : s.getConfig().getLabelSections().values()) {
+        String lower = type.getName().toLowerCase();
+        LabelType old = types.get(lower);
+        if (old == null || old.canOverride()) {
+          types.put(lower, type);
+        }
+      }
     }
-    return labelTypes;
+    List<LabelType> all = Lists.newArrayListWithCapacity(types.size());
+    for (LabelType type : types.values()) {
+      if (!type.getValues().isEmpty()) {
+        all.add(type);
+      }
+    }
+    return new LabelTypes(Collections.unmodifiableList(all));
   }
 
   /** All available label types for this change and user. */
@@ -633,26 +646,6 @@
     return false;
   }
 
-  private LabelTypes loadLabelTypes() {
-    Map<String, LabelType> types = new LinkedHashMap<>();
-    for (ProjectState s : treeInOrder()) {
-      for (LabelType type : s.getConfig().getLabelSections().values()) {
-        String lower = type.getName().toLowerCase();
-        LabelType old = types.get(lower);
-        if (old == null || old.canOverride()) {
-          types.put(lower, type);
-        }
-      }
-    }
-    List<LabelType> all = Lists.newArrayListWithCapacity(types.size());
-    for (LabelType type : types.values()) {
-      if (!type.getValues().isEmpty()) {
-        all.add(type);
-      }
-    }
-    return new LabelTypes(Collections.unmodifiableList(all));
-  }
-
   private boolean match(Branch.NameKey destination, String refPattern, CurrentUser user) {
     return RefPatternMatcher.getMatcher(refPattern).match(destination.get(), user);
   }