opendev/gerrit
Code Issues Proposed changes

Implement CachedProjectConfig

Browse Source 
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
This commit is contained in:
Patrick Hiesel
2020-06-30 13:28:44 +02:00
parent 174e411b0d
commit 87715dd0d0
30 changed files with 293 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@@ -1213,11 +1213,15 @@ public abstract class AbstractDaemonTest {
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);

4
java/com/google/gerrit/extensions/api/projects/ConfigInfo.java
View File

@@ -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 class ConfigInfo {
public Map<String, CommentLinkInfo> commentlinks;
public Map<String, List<String>> extensionPanelNames;
public ImmutableMap<String, ImmutableList<String>> extensionPanelNames;
public static class InheritedBooleanInfo {
public Boolean value;

7
java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
View File

@@ -32,6 +32,7 @@ import com.google.inject.Provider;
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 @@ public class CreateGroupPermissionSyncer implements ChangeMergedListener {
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());
}

1
java/com/google/gerrit/server/account/AccountManager.java
View File

@@ -328,6 +328,7 @@ public class AccountManager {
.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();

5
java/com/google/gerrit/server/change/ChangeResource.java
View File

@@ -20,6 +20,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
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 @@ public class ChangeResource implements RestResource, HasETag {
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 @@ public class ChangeResource implements RestResource, HasETag {
}
}
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);
}

2
java/com/google/gerrit/server/config/PluginConfig.java
View File

@@ -52,7 +52,7 @@ public class PluginConfig {
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)) {

2
java/com/google/gerrit/server/config/PluginConfigFactory.java
View File

@@ -150,7 +150,7 @@ public class PluginConfigFactory implements ReloadPluginListener {
* @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);
}
/**

7
java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@@ -1253,12 +1253,15 @@ class ReceiveCommits {
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"));

2
java/com/google/gerrit/server/git/validators/MergeValidators.java
View File

@@ -229,7 +229,7 @@ public class MergeValidators {
String value = pluginCfg.getString(e.getExportName());
String oldValue =
destProject
.getConfig()
.getBareConfig()
.getPluginConfig(e.getPluginName())
.getString(e.getExportName());

8
java/com/google/gerrit/server/group/db/RenameGroupOp.java
View File

@@ -23,6 +23,7 @@ import com.google.gerrit.entities.Project;
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 com.google.inject.assistedinject.Assisted;
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 @@ class RenameGroupOp extends DefaultQueueOp {
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;
}

2
java/com/google/gerrit/server/mail/send/ProjectWatch.java
View File

@@ -92,7 +92,7 @@ public class ProjectWatch {
}
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);

181
java/com/google/gerrit/server/project/CachedProjectConfig.java Normal file
View File

@@ -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();
}
}

2
java/com/google/gerrit/server/project/ContributorAgreementsChecker.java
View File

@@ -90,7 +90,7 @@ public class ContributorAgreementsChecker {
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;

4
java/com/google/gerrit/server/project/GroupList.java
View File

@@ -70,6 +70,10 @@ public class GroupList extends TabFile {
return byUUID.get(uuid);
}
public Map<AccountGroup.UUID, GroupReference> byUUID() {
return byUUID;
}
@Nullable
public GroupReference byName(String name) {
return byUUID.entrySet().stream()

2
java/com/google/gerrit/server/project/ProjectCacheImpl.java
View File

@@ -297,7 +297,7 @@ public class ProjectCacheImpl implements ProjectCache {
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);
}

48
java/com/google/gerrit/server/project/ProjectConfig.java
View File

@@ -28,7 +28,7 @@ import com.google.common.base.Joiner;
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.PermissionRule.Action;
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 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
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 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
this.accountsSection = accountsSection;
}
public Map<String, List<String>> getExtensionPanelSections() {
return extensionPanelSections;
}
public AccessSection getAccessSection(String name) {
return accessSections.get(name);
}
@@ -366,10 +383,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
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 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
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 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
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.
*/

26
java/com/google/gerrit/server/project/ProjectState.java
View File

@@ -54,7 +54,6 @@ import java.util.HashSet;
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 @@ public class ProjectState {
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 @@ public class ProjectState {
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 class ProjectState {
*/
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 @@ public class ProjectState {
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 @@ public class ProjectState {
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 @@ public class ProjectState {
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) {

2
java/com/google/gerrit/server/restapi/account/GetAgreements.java
View File

@@ -96,7 +96,7 @@ public class GetAgreements implements RestReadView<AccountResource> {
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()) {

2
java/com/google/gerrit/server/restapi/account/PutAgreement.java
View File

@@ -82,7 +82,7 @@ public class PutAgreement implements RestModifyView<AccountResource, AgreementIn
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");
}

7
java/com/google/gerrit/server/restapi/change/Mergeable.java
View File

@@ -44,6 +44,7 @@ import java.util.ArrayList;
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 @@ public class Mergeable implements RestReadView<RevisionResource> {
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) {

2
java/com/google/gerrit/server/restapi/config/GetServerInfo.java
View File

@@ -174,7 +174,7 @@ public class GetServerInfo implements RestReadView<ConfigResource> {
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) {

2
java/com/google/gerrit/server/restapi/project/GetAccess.java
View File

@@ -157,7 +157,7 @@ public class GetAccess implements RestReadView<ProjectResource> {
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);

4
java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
View File

@@ -453,7 +453,7 @@ public class PrologRuleEvaluator {
} 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 @@ public class PrologRuleEvaluator {
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);
}

5
java/com/google/gerrit/server/rules/RulesCache.java
View File

@@ -55,6 +55,7 @@ import java.util.ArrayList;
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 @@ public class RulesCache {
* @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;
}

13
javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
View File

@@ -139,7 +139,6 @@ import com.google.gerrit.server.index.account.AccountIndexer;
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 @@ public class AccountIT extends AbstractDaemonTest {
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);

8
javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
View File

@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
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 @@ public class AgreementsIT extends AbstractDaemonTest {
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();

5
javatests/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
View File

@@ -47,7 +47,7 @@ import com.google.gerrit.extensions.common.ChangeInfo;
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 @@ public class CustomLabelIT extends AbstractDaemonTest {
@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");
}

4
javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
View File

@@ -102,14 +102,14 @@ public class ProjectOperationsImplTest extends AbstractDaemonTest {
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();
}

1
javatests/com/google/gerrit/server/project/CommitsCollectionTest.java
View File

@@ -228,6 +228,7 @@ public class CommitsCollectionTest {
.getAllProjects()
.getConfig()
.getAccessSection(AccessSection.GLOBAL_CAPABILITIES)
.orElseThrow(() -> new IllegalStateException("access section does not exist"))
.getPermission(GlobalCapability.ADMINISTRATE_SERVER);
return adminPermission.getRules().stream()

2
plugins/delete-project

Submodule plugins/delete-project updated: f420d06562...7cb59ecacb