From d7417dfa72c486ca3b6f7b9f1c958a7e65bc8ad6 Mon Sep 17 00:00:00 2001
From: Patrick Hiesel <hiesel@google.com>
Date: Mon, 6 May 2019 16:46:41 +0200
Subject: [PATCH] Don't store LabelTypes in ProjectState

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

Change-Id: I3f8d74e829274701c54d5898f6348971a59f866f
---
 .../gerrit/server/project/ProjectState.java   | 47 ++++++++-----------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index acfbf408b0..535779b0ce 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -67,7 +67,10 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 
-/** 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 FluentLogger logger = FluentLogger.forEnclosingClass();
 
@@ -100,9 +103,6 @@ public class ProjectState {
   /** 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(
       ProjectCache projectCache,
@@ -451,10 +451,23 @@ public class ProjectState {
 
   /** 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. */
@@ -571,26 +584,6 @@ public class ProjectState {
     return project;
   }
 
-  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(BranchNameKey destination, String refPattern) {
     return RefPatternMatcher.getMatcher(refPattern).match(destination.branch(), null);
   }