Implement CachedProjectConfig
This commit gets us closer to only caching immutable data
structures in ProjectCache. It factors out the state we
need from ProjectConfig into its own AutoValue. Most callers
of ProjectState#getConfig only need this state. Therefore,
this commit changes the return type and offers a #getBareConfig
in addition for callers that do need the actual ProjectConfig.
In subsequent commits, we will rewrite these callers one-by-one
and eventually remove ProjectConfig from ProjectState.
Change-Id: I22a6654c5f6fc234e10b4e3b4b446ca4df49ade8
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index dfb7a55..9d79855 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -1213,11 +1213,15 @@
String ref,
boolean exclusive,
String... permissionNames) {
- ProjectConfig cfg = projectCache.get(project).orElseThrow(illegalState(project)).getConfig();
- AccessSection accessSection = cfg.getAccessSection(ref);
- assertThat(accessSection).isNotNull();
+ Optional<AccessSection> accessSection =
+ projectCache
+ .get(project)
+ .orElseThrow(illegalState(project))
+ .getConfig()
+ .getAccessSection(ref);
+ assertThat(accessSection).isPresent();
for (String permissionName : permissionNames) {
- Permission permission = accessSection.getPermission(permissionName);
+ Permission permission = accessSection.get().getPermission(permissionName);
assertPermission(permission, permissionName, exclusive, null);
assertPermissionRule(
permission.getRule(groupReference), groupReference, Action.ALLOW, false, 0, 0);
diff --git a/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java b/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
index fb2a0fe..3ba1277 100644
--- a/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
+++ b/java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
@@ -14,6 +14,8 @@
package com.google.gerrit.extensions.api.projects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ProjectState;
@@ -49,7 +51,7 @@
public Map<String, CommentLinkInfo> commentlinks;
- public Map<String, List<String>> extensionPanelNames;
+ public ImmutableMap<String, ImmutableList<String>> extensionPanelNames;
public static class InheritedBooleanInfo {
public Boolean value;
diff --git a/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java b/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
index 7fbf7ef..0e593b7 100644
--- a/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
+++ b/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
@@ -32,6 +32,7 @@
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.HashSet;
+import java.util.Optional;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -83,10 +84,10 @@
new HashSet<>(allProjectsState.getCapabilityCollection().createGroup);
Set<PermissionRule> createGroupsRef = new HashSet<>();
- AccessSection allUsersCreateGroupAccessSection =
+ Optional<AccessSection> allUsersCreateGroupAccessSection =
allUsersState.getConfig().getAccessSection(RefNames.REFS_GROUPS + "*");
- if (allUsersCreateGroupAccessSection != null) {
- Permission create = allUsersCreateGroupAccessSection.getPermission(Permission.CREATE);
+ if (allUsersCreateGroupAccessSection.isPresent()) {
+ Permission create = allUsersCreateGroupAccessSection.get().getPermission(Permission.CREATE);
if (create != null && create.getRules() != null) {
createGroupsRef.addAll(create.getRules());
}
diff --git a/java/com/google/gerrit/server/account/AccountManager.java b/java/com/google/gerrit/server/account/AccountManager.java
index 7a5b1aa..cb9412c 100644
--- a/java/com/google/gerrit/server/account/AccountManager.java
+++ b/java/com/google/gerrit/server/account/AccountManager.java
@@ -328,6 +328,7 @@
.getAllProjects()
.getConfig()
.getAccessSection(AccessSection.GLOBAL_CAPABILITIES)
+ .orElseThrow(() -> new IllegalStateException("access section does not exist"))
.getPermission(GlobalCapability.ADMINISTRATE_SERVER);
AccountGroup.UUID adminGroupUuid = admin.getRules().get(0).getGroup().getUUID();
diff --git a/java/com/google/gerrit/server/change/ChangeResource.java b/java/com/google/gerrit/server/change/ChangeResource.java
index a7fc5de..5a1798d 100644
--- a/java/com/google/gerrit/server/change/ChangeResource.java
+++ b/java/com/google/gerrit/server/change/ChangeResource.java
@@ -20,6 +20,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Change;
@@ -188,7 +189,7 @@
Iterable<ProjectState> projectStateTree =
projectCache.get(getProject()).orElseThrow(illegalState(getProject())).tree();
for (ProjectState p : projectStateTree) {
- hashObjectId(h, p.getConfig().getRevision(), buf);
+ hashObjectId(h, p.getConfig().getRevision().orElse(null), buf);
}
changeETagComputation.runEach(
@@ -218,7 +219,7 @@
}
}
- private void hashObjectId(Hasher h, ObjectId id, byte[] buf) {
+ private void hashObjectId(Hasher h, @Nullable ObjectId id, byte[] buf) {
MoreObjects.firstNonNull(id, ObjectId.zeroId()).copyRawTo(buf, 0);
h.putBytes(buf);
}
diff --git a/java/com/google/gerrit/server/config/PluginConfig.java b/java/com/google/gerrit/server/config/PluginConfig.java
index 13c5442..f41d5c2 100644
--- a/java/com/google/gerrit/server/config/PluginConfig.java
+++ b/java/com/google/gerrit/server/config/PluginConfig.java
@@ -52,7 +52,7 @@
ProjectState parent = Iterables.getFirst(state.parents(), null);
if (parent != null) {
PluginConfig parentPluginConfig =
- parent.getConfig().getPluginConfig(pluginName).withInheritance(projectStateFactory);
+ parent.getBareConfig().getPluginConfig(pluginName).withInheritance(projectStateFactory);
Set<String> allNames = cfg.getNames(PLUGIN, pluginName);
cfg = copyConfig(cfg);
for (String name : parentPluginConfig.cfg.getNames(PLUGIN, pluginName)) {
diff --git a/java/com/google/gerrit/server/config/PluginConfigFactory.java b/java/com/google/gerrit/server/config/PluginConfigFactory.java
index 483fc0a..4ba8ef6 100644
--- a/java/com/google/gerrit/server/config/PluginConfigFactory.java
+++ b/java/com/google/gerrit/server/config/PluginConfigFactory.java
@@ -150,7 +150,7 @@
* @return the plugin configuration from the 'project.config' file of the specified project
*/
public PluginConfig getFromProjectConfig(ProjectState projectState, String pluginName) {
- return projectState.getConfig().getPluginConfig(pluginName);
+ return projectState.getBareConfig().getPluginConfig(pluginName);
}
/**
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 5436db7..3dc8961 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -1253,12 +1253,15 @@
ProjectConfigEntry configEntry = e.getProvider().get();
String value = pluginCfg.getString(e.getExportName());
String oldValue =
- projectState.getConfig().getPluginConfig(e.getPluginName()).getString(e.getExportName());
+ projectState
+ .getBareConfig()
+ .getPluginConfig(e.getPluginName())
+ .getString(e.getExportName());
if (configEntry.getType() == ProjectConfigEntryType.ARRAY) {
oldValue =
Arrays.stream(
projectState
- .getConfig()
+ .getBareConfig()
.getPluginConfig(e.getPluginName())
.getStringList(e.getExportName()))
.collect(joining("\n"));
diff --git a/java/com/google/gerrit/server/git/validators/MergeValidators.java b/java/com/google/gerrit/server/git/validators/MergeValidators.java
index 04cbe36..d1c873a 100644
--- a/java/com/google/gerrit/server/git/validators/MergeValidators.java
+++ b/java/com/google/gerrit/server/git/validators/MergeValidators.java
@@ -229,7 +229,7 @@
String value = pluginCfg.getString(e.getExportName());
String oldValue =
destProject
- .getConfig()
+ .getBareConfig()
.getPluginConfig(e.getPluginName())
.getString(e.getExportName());
diff --git a/java/com/google/gerrit/server/group/db/RenameGroupOp.java b/java/com/google/gerrit/server/group/db/RenameGroupOp.java
index 4cc6138..b7564e2 100644
--- a/java/com/google/gerrit/server/group/db/RenameGroupOp.java
+++ b/java/com/google/gerrit/server/group/db/RenameGroupOp.java
@@ -23,6 +23,7 @@
import com.google.gerrit.server.git.DefaultQueueOp;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.project.CachedProjectConfig;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.Inject;
@@ -30,6 +31,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
+import java.util.Optional;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -87,10 +89,10 @@
public void run() {
Iterable<Project.NameKey> names = tryingAgain ? retryOn : projectCache.all();
for (Project.NameKey projectName : names) {
- ProjectConfig config =
+ CachedProjectConfig config =
projectCache.get(projectName).orElseThrow(illegalState(projectName)).getConfig();
- GroupReference ref = config.getGroup(uuid);
- if (ref == null || newName.equals(ref.getName())) {
+ Optional<GroupReference> ref = config.getGroup(uuid);
+ if (!ref.isPresent() || newName.equals(ref.get().getName())) {
continue;
}
diff --git a/java/com/google/gerrit/server/mail/send/ProjectWatch.java b/java/com/google/gerrit/server/mail/send/ProjectWatch.java
index ef58744..e67ff01 100644
--- a/java/com/google/gerrit/server/mail/send/ProjectWatch.java
+++ b/java/com/google/gerrit/server/mail/send/ProjectWatch.java
@@ -92,7 +92,7 @@
}
for (ProjectState state : projectState.tree()) {
- for (NotifyConfig nc : state.getConfig().getNotifyConfigs()) {
+ for (NotifyConfig nc : state.getConfig().getNotifySections().values()) {
if (nc.isNotify(type)) {
try {
add(matching, state.getNameKey(), nc);
diff --git a/java/com/google/gerrit/server/project/CachedProjectConfig.java b/java/com/google/gerrit/server/project/CachedProjectConfig.java
new file mode 100644
index 0000000..241a146
--- /dev/null
+++ b/java/com/google/gerrit/server/project/CachedProjectConfig.java
@@ -0,0 +1,181 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.project;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.ContributorAgreement;
+import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.common.data.SubscribeSection;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.git.BranchOrderSection;
+import com.google.gerrit.server.git.NotifyConfig;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import org.eclipse.jgit.lib.ObjectId;
+
+/**
+ * Cached representation of values parsed from {@link ProjectConfig}.
+ *
+ * <p>This class is immutable and thread-safe.
+ */
+@AutoValue
+public abstract class CachedProjectConfig {
+ public abstract Project getProject();
+
+ public abstract ImmutableMap<AccountGroup.UUID, GroupReference> getGroups();
+
+ /** Returns a set of all groups used by this configuration. */
+ public ImmutableSet<AccountGroup.UUID> getAllGroupUUIDs() {
+ return getGroups().keySet();
+ }
+
+ /**
+ * Returns the group reference for a {@link AccountGroup.UUID}, if the group is used by at least
+ * one rule.
+ */
+ public Optional<GroupReference> getGroup(AccountGroup.UUID uuid) {
+ return Optional.ofNullable(getGroups().get(uuid));
+ }
+
+ /** Returns the account section containing visibility information about accounts. */
+ public abstract AccountsSection getAccountsSection();
+
+ /** Returns a map of {@link AccessSection}s keyed by their name. */
+ public abstract ImmutableMap<String, AccessSection> getAccessSections();
+
+ /** Returns the {@link AccessSection} with to the given name. */
+ public Optional<AccessSection> getAccessSection(String refName) {
+ return Optional.ofNullable(getAccessSections().get(refName));
+ }
+
+ /** Returns all {@link AccessSection} names. */
+ public ImmutableSet<String> getAccessSectionNames() {
+ return ImmutableSet.copyOf(getAccessSections().keySet());
+ }
+
+ /**
+ * Returns the {@link BranchOrderSection} containing the order in which branches should be shown.
+ */
+ public abstract Optional<BranchOrderSection> getBranchOrderSection();
+
+ /** Returns the {@link ContributorAgreement}s keyed by their name. */
+ public abstract ImmutableMap<String, ContributorAgreement> getContributorAgreements();
+
+ /** Returns the {@link NotifyConfig}s keyed by their name. */
+ public abstract ImmutableMap<String, NotifyConfig> getNotifySections();
+
+ /** Returns the {@link LabelType}s keyed by their name. */
+ public abstract ImmutableMap<String, LabelType> getLabelSections();
+
+ /** Returns configured {@link ConfiguredMimeTypes}s. */
+ public abstract ConfiguredMimeTypes getMimeTypes();
+
+ /** Returns {@link SubscribeSection} keyed by the {@link Project.NameKey} they reference. */
+ public abstract ImmutableMap<Project.NameKey, SubscribeSection> getSubscribeSections();
+
+ /** Returns {@link StoredCommentLinkInfo} keyed by their name. */
+ public abstract ImmutableMap<String, StoredCommentLinkInfo> getCommentLinkSections();
+
+ /** Returns the blob ID of the {@code rules.pl} file, if present. */
+ public abstract Optional<ObjectId> getRulesId();
+
+ // TODO(hiesel): This should not have to be an Optional.
+ /** Returns the SHA1 of the {@code refs/meta/config} branch. */
+ public abstract Optional<ObjectId> getRevision();
+
+ /** Returns the maximum allowed object size. */
+ public abstract long getMaxObjectSizeLimit();
+
+ /** Returns {@code true} if received objects should be checked for validity. */
+ public abstract boolean getCheckReceivedObjects();
+
+ /** Returns a list of panel sections keyed by title. */
+ public abstract ImmutableMap<String, ImmutableList<String>> getExtensionPanelSections();
+
+ public ImmutableList<SubscribeSection> getSubscribeSections(BranchNameKey branch) {
+ return filterSubscribeSectionsByBranch(getSubscribeSections().values(), branch);
+ }
+
+ public static Builder builder() {
+ return new AutoValue_CachedProjectConfig.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder setProject(Project value);
+
+ public abstract Builder setGroups(ImmutableMap<AccountGroup.UUID, GroupReference> value);
+
+ public abstract Builder setAccountsSection(AccountsSection value);
+
+ public abstract Builder setAccessSections(ImmutableMap<String, AccessSection> value);
+
+ public abstract Builder setBranchOrderSection(Optional<BranchOrderSection> value);
+
+ public abstract Builder setContributorAgreements(
+ ImmutableMap<String, ContributorAgreement> value);
+
+ public abstract Builder setNotifySections(ImmutableMap<String, NotifyConfig> value);
+
+ public abstract Builder setLabelSections(ImmutableMap<String, LabelType> value);
+
+ public abstract Builder setMimeTypes(ConfiguredMimeTypes value);
+
+ public abstract Builder setSubscribeSections(
+ ImmutableMap<Project.NameKey, SubscribeSection> value);
+
+ public abstract Builder setCommentLinkSections(
+ ImmutableMap<String, StoredCommentLinkInfo> value);
+
+ public abstract Builder setRulesId(Optional<ObjectId> value);
+
+ public abstract Builder setRevision(Optional<ObjectId> value);
+
+ public abstract Builder setMaxObjectSizeLimit(long value);
+
+ public abstract Builder setCheckReceivedObjects(boolean value);
+
+ public abstract Builder setExtensionPanelSections(
+ ImmutableMap<String, ImmutableList<String>> value);
+
+ public Builder setExtensionPanelSections(Map<String, List<String>> value) {
+ ImmutableMap.Builder<String, ImmutableList<String>> b = ImmutableMap.builder();
+ value.entrySet().forEach(e -> b.put(e.getKey(), ImmutableList.copyOf(e.getValue())));
+ return setExtensionPanelSections(b.build());
+ }
+
+ public abstract CachedProjectConfig build();
+ }
+
+ private static ImmutableList<SubscribeSection> filterSubscribeSectionsByBranch(
+ Collection<SubscribeSection> allSubscribeSections, BranchNameKey branch) {
+ ImmutableList.Builder<SubscribeSection> ret = ImmutableList.builder();
+ for (SubscribeSection s : allSubscribeSections) {
+ if (s.appliesTo(branch)) {
+ ret.add(s);
+ }
+ }
+ return ret.build();
+ }
+}
diff --git a/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java b/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
index c2eb79d..610ee64 100644
--- a/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
+++ b/java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
@@ -90,7 +90,7 @@
IdentifiedUser iUser = user.asIdentifiedUser();
Collection<ContributorAgreement> contributorAgreements =
- projectCache.getAllProjects().getConfig().getContributorAgreements();
+ projectCache.getAllProjects().getConfig().getContributorAgreements().values();
List<UUID> okGroupIds = new ArrayList<>();
for (ContributorAgreement ca : contributorAgreements) {
List<AccountGroup.UUID> groupIds;
diff --git a/java/com/google/gerrit/server/project/GroupList.java b/java/com/google/gerrit/server/project/GroupList.java
index 9b65413..33c73e8 100644
--- a/java/com/google/gerrit/server/project/GroupList.java
+++ b/java/com/google/gerrit/server/project/GroupList.java
@@ -70,6 +70,10 @@
return byUUID.get(uuid);
}
+ public Map<AccountGroup.UUID, GroupReference> byUUID() {
+ return byUUID;
+ }
+
@Nullable
public GroupReference byName(String name) {
return byUUID.entrySet().stream()
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index a5c07e5..5f3fbeb 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -297,7 +297,7 @@
try (Repository git = mgr.openRepository(key)) {
Ref configRef = git.exactRef(RefNames.REFS_CONFIG);
if (configRef != null
- && configRef.getObjectId().equals(oldState.getConfig().getRevision())) {
+ && configRef.getObjectId().equals(oldState.getBareConfig().getRevision())) {
refreshCounter.increment(CACHE_NAME, false);
return Futures.immediateFuture(oldState);
}
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 16e7037..727bf44 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -28,7 +28,7 @@
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.common.primitives.Shorts;
import com.google.gerrit.common.Nullable;
@@ -47,7 +47,6 @@
import com.google.gerrit.common.data.SubscribeSection;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.BooleanProjectConfig;
-import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.InvalidNameException;
@@ -252,6 +251,28 @@
private boolean hasLegacyPermissions;
private Map<String, List<String>> extensionPanelSections;
+ /** Returns an immutable, thread-safe representation of this object that can be cached. */
+ public CachedProjectConfig getCacheable() {
+ return CachedProjectConfig.builder()
+ .setProject(project)
+ .setAccountsSection(accountsSection)
+ .setGroups(ImmutableMap.copyOf(groupList.byUUID()))
+ .setAccessSections(ImmutableMap.copyOf(accessSections))
+ .setBranchOrderSection(Optional.ofNullable(branchOrderSection))
+ .setContributorAgreements(ImmutableMap.copyOf(contributorAgreements))
+ .setNotifySections(ImmutableMap.copyOf(notifySections))
+ .setLabelSections(ImmutableMap.copyOf(labelSections))
+ .setMimeTypes(mimeTypes)
+ .setSubscribeSections(ImmutableMap.copyOf(subscribeSections))
+ .setCommentLinkSections(ImmutableMap.copyOf(commentLinkSections))
+ .setRulesId(Optional.ofNullable(rulesId))
+ .setRevision(Optional.ofNullable(getRevision()))
+ .setMaxObjectSizeLimit(maxObjectSizeLimit)
+ .setCheckReceivedObjects(checkReceivedObjects)
+ .setExtensionPanelSections(extensionPanelSections)
+ .build();
+ }
+
public static StoredCommentLinkInfo buildCommentLink(Config cfg, String name, boolean allowRaw)
throws IllegalArgumentException {
String match = cfg.getString(COMMENTLINK, name, KEY_MATCH);
@@ -349,10 +370,6 @@
this.accountsSection = accountsSection;
}
- public Map<String, List<String>> getExtensionPanelSections() {
- return extensionPanelSections;
- }
-
public AccessSection getAccessSection(String name) {
return accessSections.get(name);
}
@@ -366,10 +383,6 @@
accessSections.put(name, accessSectionBuilder.build());
}
- public ImmutableSet<String> getAccessSectionNames() {
- return ImmutableSet.copyOf(accessSections.keySet());
- }
-
public Collection<AccessSection> getAccessSections() {
return sort(accessSections.values());
}
@@ -386,16 +399,6 @@
return subscribeSections;
}
- public Collection<SubscribeSection> getSubscribeSections(BranchNameKey branch) {
- Collection<SubscribeSection> ret = new ArrayList<>();
- for (SubscribeSection s : subscribeSections.values()) {
- if (s.appliesTo(branch)) {
- ret.add(s);
- }
- }
- return ret;
- }
-
public void addSubscribeSection(SubscribeSection s) {
subscribeSections.put(s.project(), s);
}
@@ -540,11 +543,6 @@
return groupList.byName(groupName);
}
- /** @return set of all groups used by this configuration. */
- public Set<AccountGroup.UUID> getAllGroupUUIDs() {
- return groupList.uuids();
- }
-
/**
* @return the project's rules.pl ObjectId, if present in the branch. Null if it doesn't exist.
*/
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index 986019c..616809b 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -54,7 +54,6 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -79,6 +78,7 @@
private final List<CommentLinkInfo> commentLinks;
private final ProjectConfig config;
+ private final CachedProjectConfig cachedConfig;
private final Map<String, ProjectLevelConfig> configs;
private final Set<AccountGroup.UUID> localOwners;
private final long globalMaxObjectSizeLimit;
@@ -107,6 +107,7 @@
this.gitMgr = gitMgr;
this.commentLinks = commentLinks;
this.config = config;
+ this.cachedConfig = config.getCacheable();
this.configs = new HashMap<>();
this.capabilities =
isAllProjects
@@ -149,15 +150,15 @@
*/
public boolean hasPrologRules() {
// We check if this project has a rules.pl file
- if (getConfig().getRulesId() != null) {
+ if (getConfig().getRulesId().isPresent()) {
return true;
}
// If not, we check the parents.
return parents().stream()
.map(ProjectState::getConfig)
- .map(ProjectConfig::getRulesId)
- .anyMatch(Objects::nonNull);
+ .map(CachedProjectConfig::getRulesId)
+ .anyMatch(Optional::isPresent);
}
public Project getProject() {
@@ -172,7 +173,12 @@
return getNameKey().get();
}
- public ProjectConfig getConfig() {
+ public CachedProjectConfig getConfig() {
+ return cachedConfig;
+ }
+
+ // TODO(hiesel): Remove this method.
+ public ProjectConfig getBareConfig() {
return config;
}
@@ -459,7 +465,7 @@
cls.put(cl.name.toLowerCase(), cl);
}
for (ProjectState s : treeInOrder()) {
- for (StoredCommentLinkInfo cl : s.getConfig().getCommentLinkSections()) {
+ for (StoredCommentLinkInfo cl : s.getConfig().getCommentLinkSections().values()) {
String name = cl.getName().toLowerCase();
if (cl.getOverrideOnly()) {
CommentLinkInfo parent = cls.get(name);
@@ -475,14 +481,14 @@
return ImmutableList.copyOf(cls.values());
}
- public BranchOrderSection getBranchOrderSection() {
+ public Optional<BranchOrderSection> getBranchOrderSection() {
for (ProjectState s : tree()) {
- BranchOrderSection section = s.getConfig().getBranchOrderSection();
- if (section != null) {
+ Optional<BranchOrderSection> section = s.getConfig().getBranchOrderSection();
+ if (section.isPresent()) {
return section;
}
}
- return null;
+ return Optional.empty();
}
public Collection<SubscribeSection> getSubscribeSections(BranchNameKey branch) {
diff --git a/java/com/google/gerrit/server/restapi/account/GetAgreements.java b/java/com/google/gerrit/server/restapi/account/GetAgreements.java
index aeaeb1c..c11fc7e 100644
--- a/java/com/google/gerrit/server/restapi/account/GetAgreements.java
+++ b/java/com/google/gerrit/server/restapi/account/GetAgreements.java
@@ -96,7 +96,7 @@
List<AgreementInfo> results = new ArrayList<>();
Collection<ContributorAgreement> cas =
- projectCache.getAllProjects().getConfig().getContributorAgreements();
+ projectCache.getAllProjects().getConfig().getContributorAgreements().values();
for (ContributorAgreement ca : cas) {
List<AccountGroup.UUID> groupIds = new ArrayList<>();
for (PermissionRule rule : ca.getAccepted()) {
diff --git a/java/com/google/gerrit/server/restapi/account/PutAgreement.java b/java/com/google/gerrit/server/restapi/account/PutAgreement.java
index 42504a0..ce6f287 100644
--- a/java/com/google/gerrit/server/restapi/account/PutAgreement.java
+++ b/java/com/google/gerrit/server/restapi/account/PutAgreement.java
@@ -82,7 +82,7 @@
String agreementName = Strings.nullToEmpty(input.name);
ContributorAgreement ca =
- projectCache.getAllProjects().getConfig().getContributorAgreement(agreementName);
+ projectCache.getAllProjects().getConfig().getContributorAgreements().get(agreementName);
if (ca == null) {
throw new UnprocessableEntityException("contributor agreement not found");
}
diff --git a/java/com/google/gerrit/server/restapi/change/Mergeable.java b/java/com/google/gerrit/server/restapi/change/Mergeable.java
index 6fccdd1..3338543 100644
--- a/java/com/google/gerrit/server/restapi/change/Mergeable.java
+++ b/java/com/google/gerrit/server/restapi/change/Mergeable.java
@@ -44,6 +44,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Optional;
import java.util.concurrent.Future;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
@@ -116,10 +117,10 @@
if (otherBranches) {
result.mergeableInto = new ArrayList<>();
- BranchOrderSection branchOrder = projectState.getBranchOrderSection();
- if (branchOrder != null) {
+ Optional<BranchOrderSection> branchOrder = projectState.getBranchOrderSection();
+ if (branchOrder.isPresent()) {
int prefixLen = Constants.R_HEADS.length();
- List<String> names = branchOrder.getMoreStable(ref.getName());
+ List<String> names = branchOrder.get().getMoreStable(ref.getName());
Map<String, Ref> refs =
git.getRefDatabase().exactRef(names.toArray(new String[names.size()]));
for (String n : names) {
diff --git a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
index 0198c36..1f4b468 100644
--- a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
+++ b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
@@ -174,7 +174,7 @@
if (info.useContributorAgreements != null) {
Collection<ContributorAgreement> agreements =
- projectCache.getAllProjects().getConfig().getContributorAgreements();
+ projectCache.getAllProjects().getConfig().getContributorAgreements().values();
if (!agreements.isEmpty()) {
info.contributorAgreements = Lists.newArrayListWithCapacity(agreements.size());
for (ContributorAgreement agreement : agreements) {
diff --git a/java/com/google/gerrit/server/restapi/project/GetAccess.java b/java/com/google/gerrit/server/restapi/project/GetAccess.java
index 2416780..2d1191f 100644
--- a/java/com/google/gerrit/server/restapi/project/GetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/GetAccess.java
@@ -157,7 +157,7 @@
projectState = projectCache.get(projectName).orElseThrow(illegalState(projectName));
perm = permissionBackend.currentUser().project(projectName);
} else if (config.getRevision() != null
- && !config.getRevision().equals(projectState.getConfig().getRevision())) {
+ && !config.getRevision().equals(projectState.getConfig().getRevision().orElse(null))) {
projectCache.evict(config.getProject());
projectState = projectCache.get(projectName).orElseThrow(illegalState(projectName));
perm = permissionBackend.currentUser().project(projectName);
diff --git a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
index 5f1268b..87f5758 100644
--- a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
+++ b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
@@ -453,7 +453,7 @@
} else {
pmc =
rulesCache.loadMachine(
- projectState.getNameKey(), projectState.getConfig().getRulesId());
+ projectState.getNameKey(), projectState.getConfig().getRulesId().orElse(null));
}
env = envFactory.create(pmc);
} catch (CompileException err) {
@@ -490,7 +490,7 @@
parentEnv =
envFactory.create(
rulesCache.loadMachine(
- parentState.getNameKey(), parentState.getConfig().getRulesId()));
+ parentState.getNameKey(), parentState.getConfig().getRulesId().orElse(null)));
} catch (CompileException err) {
throw new RuleEvalException("Cannot consult rules.pl for " + parentState.getName(), err);
}
diff --git a/java/com/google/gerrit/server/rules/RulesCache.java b/java/com/google/gerrit/server/rules/RulesCache.java
index ed67e68..8b72714 100644
--- a/java/com/google/gerrit/server/rules/RulesCache.java
+++ b/java/com/google/gerrit/server/rules/RulesCache.java
@@ -55,6 +55,7 @@
import java.util.EnumSet;
import java.util.List;
import java.util.concurrent.ExecutionException;
+import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.errors.LargeObjectException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -128,8 +129,8 @@
* @return a Prolog machine, after loading the specified rules.
* @throws CompileException the machine cannot be created.
*/
- public synchronized PrologMachineCopy loadMachine(Project.NameKey project, ObjectId rulesId)
- throws CompileException {
+ public synchronized PrologMachineCopy loadMachine(
+ @Nullable Project.NameKey project, @Nullable ObjectId rulesId) throws CompileException {
if (!enableProjectRules || project == null || rulesId == null) {
return defaultMachine;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 43cc1f4..aaa1eaa 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -139,7 +139,6 @@
import com.google.gerrit.server.index.account.StalenessChecker;
import com.google.gerrit.server.notedb.Sequences;
import com.google.gerrit.server.plugincontext.PluginSetContext;
-import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.RefPattern;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gerrit.server.update.RetryHelper;
@@ -284,12 +283,16 @@
String labelName,
int min,
int max) {
- ProjectConfig cfg = projectCache.get(project).orElseThrow(illegalState(project)).getConfig();
- AccessSection accessSection = cfg.getAccessSection(ref);
- assertThat(accessSection).isNotNull();
+ Optional<AccessSection> accessSection =
+ projectCache
+ .get(project)
+ .orElseThrow(illegalState(project))
+ .getConfig()
+ .getAccessSection(ref);
+ assertThat(accessSection).isPresent();
String permissionName = Permission.LABEL + labelName;
- Permission permission = accessSection.getPermission(permissionName);
+ Permission permission = accessSection.get().getPermission(permissionName);
assertPermission(permission, permissionName, exclusive, labelName);
assertPermissionRule(
permission.getRule(groupReference), groupReference, Action.ALLOW, false, min, max);
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
index 23fdc0b..62a1ad2 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
@@ -18,6 +18,7 @@
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Comparator.comparing;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -125,8 +126,11 @@
if (isContributorAgreementsEnabled()) {
assertThat(info.auth.useContributorAgreements).isTrue();
assertThat(info.auth.contributorAgreements).hasSize(2);
- assertAgreement(info.auth.contributorAgreements.get(0), caAutoVerify);
- assertAgreement(info.auth.contributorAgreements.get(1), caNoAutoVerify);
+ // Sort to get a stable assertion as the API does not guarantee ordering.
+ List<AgreementInfo> agreements =
+ ImmutableList.sortedCopyOf(comparing(a -> a.name), info.auth.contributorAgreements);
+ assertAgreement(agreements.get(0), caAutoVerify);
+ assertAgreement(agreements.get(1), caNoAutoVerify);
} else {
assertThat(info.auth.useContributorAgreements).isNull();
assertThat(info.auth.contributorAgreements).isNull();
diff --git a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
index 813a715..1991e79 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
@@ -47,7 +47,7 @@
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.events.CommentAddedListener;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.project.CachedProjectConfig;
import com.google.inject.Inject;
import org.junit.Before;
import org.junit.Test;
@@ -322,7 +322,8 @@
@Test
public void customLabel_withBranch() throws Exception {
saveLabelConfig(LABEL.toBuilder().setRefPatterns(ImmutableList.of("master")));
- ProjectConfig cfg = projectCache.get(project).orElseThrow(illegalState(project)).getConfig();
+ CachedProjectConfig cfg =
+ projectCache.get(project).orElseThrow(illegalState(project)).getConfig();
assertThat(cfg.getLabelSections().get(LABEL_NAME).getRefPatterns()).contains("master");
}
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
index c8899b9..3e8c017 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
@@ -102,14 +102,14 @@
Project.NameKey key = projectOperations.newProject().create();
ProjectConfig projectConfig = projectOperations.project(key).getProjectConfig();
ProjectState cachedProjectState1 = projectCache.get(key).orElseThrow(illegalState(project));
- ProjectConfig cachedProjectConfig1 = cachedProjectState1.getConfig();
+ ProjectConfig cachedProjectConfig1 = cachedProjectState1.getBareConfig();
assertThat(cachedProjectConfig1).isNotSameInstanceAs(projectConfig);
assertThat(cachedProjectConfig1.getProject().getDescription()).isEmpty();
assertThat(projectConfig.getProject().getDescription()).isEmpty();
projectConfig.updateProject(p -> p.setDescription("my fancy project"));
ProjectConfig cachedProjectConfig2 =
- projectCache.get(key).orElseThrow(illegalState(project)).getConfig();
+ projectCache.get(key).orElseThrow(illegalState(project)).getBareConfig();
assertThat(cachedProjectConfig2).isNotSameInstanceAs(projectConfig);
assertThat(cachedProjectConfig2.getProject().getDescription()).isEmpty();
}
diff --git a/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java b/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java
index 5e94daa..5e57551 100644
--- a/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java
+++ b/javatests/com/google/gerrit/server/project/CommitsCollectionTest.java
@@ -228,6 +228,7 @@
.getAllProjects()
.getConfig()
.getAccessSection(AccessSection.GLOBAL_CAPABILITIES)
+ .orElseThrow(() -> new IllegalStateException("access section does not exist"))
.getPermission(GlobalCapability.ADMINISTRATE_SERVER);
return adminPermission.getRules().stream()
diff --git a/plugins/delete-project b/plugins/delete-project
index f420d06..7cb59ec 160000
--- a/plugins/delete-project
+++ b/plugins/delete-project
@@ -1 +1 @@
-Subproject commit f420d06562b97eab26a627baa7722c7f84d95763
+Subproject commit 7cb59ecacbbe7bc995873ae112e48cf0ff521d2a
