Move permission filtering for All-Projects from ProjectState to ProjectConfig

Certain permissions cannot be used in All-Projects and are filtered
out before we evaluate them. This used to be done in ProjectState
and was cached when that entity was cached. Now, we cache immutable
data objects (mostly AutoValues) for better thread safety.

When the ProjectCache was refactored, this logic was left in
ProjectState. This means it has to be recomputed quite often when
permissions are evaluated. A recent profile showed that we can
save CPU time (hence request time) as well as heap if we cache
the outcome of this filtering.

This is what this commit does. Existing tests check that permissions
that are not allowed on All-Projects (e.g. OWNERS on refs/*) are
indeed not evaluated.

Since this filtering depends on if a project is considered to
be All-Projects or not, we have to invalidate the cache when
the name of All-Projects changes. We document this fact.

Release-Notes: Cache permission filtering for All-Projects
Change-Id: I01e08e70bf36aaf4be522564286ab4a86e7681a3

3 files changed, 47 insertions, 30 deletions

diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index ab0657a183..868d32cbc1 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2214,6 +2214,9 @@ access controls and settings. These are inherited into every
 other project managed by the running server. The name is
 relative to `gerrit.basePath`.
 +
+The link:#cache_names[persisted_projects cache] must be
+flushed after this setting is changed.
++
 Defaults to `All-Projects` if not set.
 
 [[gerrit.defaultBranch]]gerrit.defaultBranch::
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 8f0b5357a4..c101ddf4e3 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -214,7 +214,8 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
           projectName,
           projectName.equals(allProjectsName)
               ? allProjectsConfigProvider.get(allProjectsName)
-              : Optional.empty());
+              : Optional.empty(),
+          allProjectsName);
     }
 
     public ProjectConfig read(MetaDataUpdate update) throws IOException, ConfigInvalidException {
@@ -240,6 +241,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
   }
 
   private final Optional<StoredConfig> baseConfig;
+  private final AllProjectsName allProjectsName;
 
   private Project project;
   private AccountsSection accountsSection;
@@ -277,7 +279,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
             .setCheckReceivedObjects(checkReceivedObjects)
             .setExtensionPanelSections(extensionPanelSections);
     groupList.byUUID().values().forEach(g -> builder.addGroup(g));
-    accessSections.values().forEach(a -> builder.addAccessSection(a));
     contributorAgreements.values().forEach(c -> builder.addContributorAgreement(c));
     notifySections.values().forEach(n -> builder.addNotifySection(n));
     subscribeSections.values().forEach(s -> builder.addSubscribeSection(s));
@@ -290,6 +291,28 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
     projectLevelConfigs
         .entrySet()
         .forEach(c -> builder.addProjectLevelConfig(c.getKey(), c.getValue().toText()));
+
+    if (projectName.equals(allProjectsName)) {
+      // Filter out permissions that aren't allowed to be set on All-Projects
+      accessSections
+          .values()
+          .forEach(
+              a -> {
+                List<Permission.Builder> copy = new ArrayList<>();
+                for (Permission p : a.getPermissions()) {
+                  if (Permission.canBeOnAllProjects(a.getName(), p.getName())) {
+                    copy.add(p.toBuilder());
+                  }
+                }
+                AccessSection section =
+                    AccessSection.builder(a.getName())
+                        .modifyPermissions(permissions -> permissions.addAll(copy))
+                        .build();
+                builder.addAccessSection(section);
+              });
+    } else {
+      accessSections.values().forEach(a -> builder.addAccessSection(a));
+    }
     return builder.build();
   }
 
@@ -345,9 +368,13 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
     requireNonNull(commentLinkSections.remove(name));
   }
 
-  private ProjectConfig(Project.NameKey projectName, Optional<StoredConfig> baseConfig) {
+  private ProjectConfig(
+      Project.NameKey projectName,
+      Optional<StoredConfig> baseConfig,
+      AllProjectsName allProjectsName) {
     this.projectName = projectName;
     this.baseConfig = baseConfig;
+    this.allProjectsName = allProjectsName;
   }
 
   public void load(Repository repo) throws IOException, ConfigInvalidException {
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index 69e6036d76..03a216d149 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -268,35 +268,22 @@ public class ProjectState {
 
   /** Get the sections that pertain only to this project. */
   List<SectionMatcher> getLocalAccessSections() {
-    List<SectionMatcher> sm = localAccessSections;
-    if (sm == null) {
-      ImmutableList<AccessSection> fromConfig =
-          cachedConfig.getAccessSections().values().stream()
-              .sorted(comparing(AccessSection::getName))
-              .collect(toImmutableList());
-      sm = new ArrayList<>(fromConfig.size());
-      for (AccessSection section : fromConfig) {
-        if (isAllProjects) {
-          List<Permission.Builder> copy = new ArrayList<>();
-          for (Permission p : section.getPermissions()) {
-            if (Permission.canBeOnAllProjects(section.getName(), p.getName())) {
-              copy.add(p.toBuilder());
-            }
-          }
-          section =
-              AccessSection.builder(section.getName())
-                  .modifyPermissions(permissions -> permissions.addAll(copy))
-                  .build();
-        }
-
-        SectionMatcher matcher = SectionMatcher.wrap(getNameKey(), section);
-        if (matcher != null) {
-          sm.add(matcher);
-        }
+    if (localAccessSections != null) {
+      return localAccessSections;
+    }
+    ImmutableList<AccessSection> fromConfig =
+        cachedConfig.getAccessSections().values().stream()
+            .sorted(comparing(AccessSection::getName))
+            .collect(toImmutableList());
+    List<SectionMatcher> sm = new ArrayList<>(fromConfig.size());
+    for (AccessSection section : fromConfig) {
+      SectionMatcher matcher = SectionMatcher.wrap(getNameKey(), section);
+      if (matcher != null) {
+        sm.add(matcher);
       }
-      localAccessSections = sm;
     }
-    return sm;
+    localAccessSections = sm;
+    return localAccessSections;
   }
 
   /**