Merge changes from topic "implement-cached-project-config"
* changes:
Add serializer for Project
Implement CachedProjectConfig
Make AccountsSection an AutoValue
Make AccessSection, Permission an AutoValue
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/acceptance/testsuite/project/ProjectOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java
index 89269b6..a3ddd98 100644
--- a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java
@@ -151,15 +151,19 @@
ProjectConfig projectConfig,
ImmutableList<TestProjectUpdate.TestPermissionKey> removedPermissions) {
for (TestProjectUpdate.TestPermissionKey p : removedPermissions) {
- Permission permission =
- projectConfig.getAccessSection(p.section(), true).getPermission(p.name(), true);
- if (p.group().isPresent()) {
- GroupReference group = GroupReference.create(p.group().get(), p.group().get().get());
- group = projectConfig.resolve(group);
- permission.removeRule(group);
- } else {
- permission.clearRules();
- }
+ projectConfig.upsertAccessSection(
+ p.section(),
+ as -> {
+ Permission.Builder permission = as.upsertPermission(p.name());
+ if (p.group().isPresent()) {
+ GroupReference group =
+ GroupReference.create(p.group().get(), p.group().get().get());
+ group = projectConfig.resolve(group);
+ permission.removeRule(group);
+ } else {
+ permission.clearRules();
+ }
+ });
}
}
@@ -168,10 +172,8 @@
for (TestCapability c : addedCapabilities) {
PermissionRule.Builder rule = newRule(projectConfig, c.group());
rule.setRange(c.min(), c.max());
- projectConfig
- .getAccessSection(AccessSection.GLOBAL_CAPABILITIES, true)
- .getPermission(c.name(), true)
- .add(rule.build());
+ projectConfig.upsertAccessSection(
+ AccessSection.GLOBAL_CAPABILITIES, as -> as.upsertPermission(c.name()).add(rule));
}
}
@@ -181,10 +183,7 @@
PermissionRule.Builder rule = newRule(projectConfig, p.group());
rule.setAction(p.action());
rule.setForce(p.force());
- projectConfig
- .getAccessSection(p.ref(), true)
- .getPermission(p.name(), true)
- .add(rule.build());
+ projectConfig.upsertAccessSection(p.ref(), as -> as.upsertPermission(p.name()).add(rule));
}
}
@@ -196,9 +195,8 @@
rule.setRange(p.min(), p.max());
String permissionName =
p.impersonation() ? Permission.forLabelAs(p.name()) : Permission.forLabel(p.name());
- Permission permission =
- projectConfig.getAccessSection(p.ref(), true).getPermission(permissionName, true);
- permission.add(rule.build());
+ projectConfig.upsertAccessSection(
+ p.ref(), as -> as.upsertPermission(permissionName).add(rule));
}
}
@@ -207,10 +205,9 @@
ImmutableMap<TestProjectUpdate.TestPermissionKey, Boolean> exclusiveGroupPermissions) {
exclusiveGroupPermissions.forEach(
(key, exclusive) ->
- projectConfig
- .getAccessSection(key.section(), true)
- .getPermission(key.name(), true)
- .setExclusiveGroup(exclusive));
+ projectConfig.upsertAccessSection(
+ key.section(),
+ as -> as.upsertPermission(key.name()).setExclusiveGroup(exclusive)));
}
private RevCommit headOrNull(String branch) {
diff --git a/java/com/google/gerrit/common/data/AccessSection.java b/java/com/google/gerrit/common/data/AccessSection.java
index 0c9663b..18682e3 100644
--- a/java/com/google/gerrit/common/data/AccessSection.java
+++ b/java/com/google/gerrit/common/data/AccessSection.java
@@ -14,18 +14,21 @@
package com.google.gerrit.common.data;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Objects.requireNonNull;
+import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project;
import java.util.ArrayList;
-import java.util.HashSet;
import java.util.List;
-import java.util.Set;
+import java.util.Optional;
+import java.util.function.Consumer;
/** Portion of a {@link Project} describing access rules. */
-public final class AccessSection implements Comparable<AccessSection> {
+@AutoValue
+public abstract class AccessSection implements Comparable<AccessSection> {
/** Special name given to the global capabilities; not a valid reference. */
public static final String GLOBAL_CAPABILITIES = "GLOBAL_CAPABILITIES";
/** Pattern that matches all references in a project. */
@@ -38,12 +41,16 @@
public static final String REGEX_PREFIX = "^";
/** Name of the access section. It could be a ref pattern or something else. */
- private String name;
+ public abstract String getName();
- private List<Permission> permissions;
+ public abstract ImmutableList<Permission> getPermissions();
- public AccessSection(String name) {
- this.name = name;
+ public static AccessSection create(String name) {
+ return builder(name).build();
+ }
+
+ public static Builder builder(String name) {
+ return new AutoValue_AccessSection.Builder().setName(name).setPermissions(ImmutableList.of());
}
/** @return true if the name is likely to be a valid reference section name. */
@@ -51,101 +58,19 @@
return name.startsWith("refs/") || name.startsWith("^refs/");
}
- public String getName() {
- return name;
- }
-
- public ImmutableList<Permission> getPermissions() {
- return permissions == null ? ImmutableList.of() : ImmutableList.copyOf(permissions);
- }
-
- public void setPermissions(List<Permission> list) {
- requireNonNull(list);
-
- Set<String> names = new HashSet<>();
- for (Permission p : list) {
- if (!names.add(p.getName().toLowerCase())) {
- throw new IllegalArgumentException();
- }
- }
-
- permissions = new ArrayList<>(list);
- }
-
@Nullable
public Permission getPermission(String name) {
- return getPermission(name, false);
- }
-
- @Nullable
- public Permission getPermission(String name, boolean create) {
requireNonNull(name);
-
- if (permissions != null) {
- for (Permission p : permissions) {
- if (p.getName().equalsIgnoreCase(name)) {
- return p;
- }
+ for (Permission p : getPermissions()) {
+ if (p.getName().equalsIgnoreCase(name)) {
+ return p;
}
}
-
- if (create) {
- if (permissions == null) {
- permissions = new ArrayList<>();
- }
-
- Permission p = new Permission(name);
- permissions.add(p);
- return p;
- }
-
return null;
}
- public void addPermission(Permission permission) {
- requireNonNull(permission);
-
- if (permissions == null) {
- permissions = new ArrayList<>();
- }
-
- for (Permission p : permissions) {
- if (p.getName().equalsIgnoreCase(permission.getName())) {
- throw new IllegalArgumentException();
- }
- }
-
- permissions.add(permission);
- }
-
- public void remove(Permission permission) {
- requireNonNull(permission);
- removePermission(permission.getName());
- }
-
- public void removePermission(String name) {
- requireNonNull(name);
-
- if (permissions != null) {
- permissions.removeIf(permission -> name.equalsIgnoreCase(permission.getName()));
- }
- }
-
- public void mergeFrom(AccessSection section) {
- requireNonNull(section);
-
- for (Permission src : section.getPermissions()) {
- Permission dst = getPermission(src.getName());
- if (dst != null) {
- dst.mergeFrom(src);
- } else {
- permissions.add(src);
- }
- }
- }
-
@Override
- public int compareTo(AccessSection o) {
+ public final int compareTo(AccessSection o) {
return comparePattern().compareTo(o.comparePattern());
}
@@ -157,33 +82,85 @@
}
@Override
- public String toString() {
+ public final String toString() {
return "AccessSection[" + getName() + "]";
}
- @Override
- public boolean equals(Object obj) {
- if (!(obj instanceof AccessSection)) {
- return false;
- }
-
- AccessSection other = (AccessSection) obj;
- if (!getName().equals(other.getName())) {
- return false;
- }
- return new HashSet<>(getPermissions())
- .equals(new HashSet<>(((AccessSection) obj).getPermissions()));
+ public Builder toBuilder() {
+ Builder b = autoToBuilder();
+ b.getPermissions().stream().map(Permission::toBuilder).forEach(p -> b.addPermission(p));
+ return b;
}
- @Override
- public int hashCode() {
- int hashCode = super.hashCode();
- if (permissions != null) {
- for (Permission permission : permissions) {
- hashCode += permission.hashCode();
- }
+ protected abstract Builder autoToBuilder();
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ private final List<Permission.Builder> permissionBuilders;
+
+ public Builder() {
+ permissionBuilders = new ArrayList<>();
}
- hashCode += getName().hashCode();
- return hashCode;
+
+ public abstract Builder setName(String name);
+
+ public abstract String getName();
+
+ public Builder modifyPermissions(Consumer<List<Permission.Builder>> modification) {
+ modification.accept(permissionBuilders);
+ return this;
+ }
+
+ public Builder addPermission(Permission.Builder permission) {
+ requireNonNull(permission, "permission must be non-null");
+ return modifyPermissions(p -> p.add(permission));
+ }
+
+ public Builder remove(Permission.Builder permission) {
+ requireNonNull(permission, "permission must be non-null");
+ return removePermission(permission.getName());
+ }
+
+ public Builder removePermission(String name) {
+ requireNonNull(name, "name must be non-null");
+ return modifyPermissions(
+ p -> p.removeIf(permissionBuilder -> name.equalsIgnoreCase(permissionBuilder.getName())));
+ }
+
+ public Permission.Builder upsertPermission(String permissionName) {
+ requireNonNull(permissionName, "permissionName must be non-null");
+
+ Optional<Permission.Builder> maybePermission =
+ permissionBuilders.stream()
+ .filter(p -> p.getName().equalsIgnoreCase(permissionName))
+ .findAny();
+ if (maybePermission.isPresent()) {
+ return maybePermission.get();
+ }
+
+ Permission.Builder permission = Permission.builder(permissionName);
+ modifyPermissions(p -> p.add(permission));
+ return permission;
+ }
+
+ public AccessSection build() {
+ setPermissions(
+ permissionBuilders.stream().map(Permission.Builder::build).collect(toImmutableList()));
+ if (getPermissions().size()
+ > getPermissions().stream()
+ .map(Permission::getName)
+ .map(String::toLowerCase)
+ .distinct()
+ .count()) {
+ throw new IllegalArgumentException("duplicate permissions: " + getPermissions());
+ }
+ return autoBuild();
+ }
+
+ protected abstract AccessSection autoBuild();
+
+ protected abstract ImmutableList<Permission> getPermissions();
+
+ protected abstract Builder setPermissions(ImmutableList<Permission> permissions);
}
}
diff --git a/java/com/google/gerrit/common/data/Permission.java b/java/com/google/gerrit/common/data/Permission.java
index 5aaffba..93155d5 100644
--- a/java/com/google/gerrit/common/data/Permission.java
+++ b/java/com/google/gerrit/common/data/Permission.java
@@ -14,14 +14,19 @@
package com.google.gerrit.common.data;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.common.Nullable;
import java.util.ArrayList;
-import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
+import java.util.function.Consumer;
/** A single permission within an {@link AccessSection} of a project. */
-public class Permission implements Comparable<Permission> {
+@AutoValue
+public abstract class Permission implements Comparable<Permission> {
public static final String ABANDON = "abandon";
public static final String ADD_PATCH_SET = "addPatchSet";
public static final String CREATE = "create";
@@ -133,18 +138,21 @@
return true;
}
- protected String name;
- protected boolean exclusiveGroup;
- protected List<PermissionRule> rules;
+ public abstract String getName();
- protected Permission() {}
+ protected abstract boolean isExclusiveGroup();
- public Permission(String name) {
- this.name = name;
+ public abstract ImmutableList<PermissionRule> getRules();
+
+ public static Builder builder(String name) {
+ return new AutoValue_Permission.Builder()
+ .setName(name)
+ .setExclusiveGroup(false)
+ .setRules(ImmutableList.of());
}
- public String getName() {
- return name;
+ public static Permission create(String name) {
+ return builder(name).build();
}
public String getLabel() {
@@ -155,97 +163,32 @@
// Only permit exclusive group behavior on non OWNER permissions,
// otherwise an owner might lose access to a delegated subspace.
//
- return exclusiveGroup && !OWNER.equals(getName());
+ return isExclusiveGroup() && !OWNER.equals(getName());
}
- public void setExclusiveGroup(boolean newExclusiveGroup) {
- exclusiveGroup = newExclusiveGroup;
- }
-
- public ImmutableList<PermissionRule> getRules() {
- return rules == null ? ImmutableList.of() : ImmutableList.copyOf(rules);
- }
-
- public void setRules(List<PermissionRule> list) {
- rules = new ArrayList<>(list);
- }
-
- public void add(PermissionRule rule) {
- initRules();
- rules.add(rule);
- }
-
- public void remove(PermissionRule rule) {
- if (rule != null) {
- removeRule(rule.getGroup());
- }
- }
-
- public void removeRule(GroupReference group) {
- if (rules != null) {
- rules.removeIf(permissionRule -> sameGroup(permissionRule, group));
- }
- }
-
- public void clearRules() {
- if (rules != null) {
- rules.clear();
- }
- }
-
+ @Nullable
public PermissionRule getRule(GroupReference group) {
- return getRule(group, false);
- }
-
- public PermissionRule getRule(GroupReference group, boolean create) {
- initRules();
-
- for (PermissionRule r : rules) {
+ for (PermissionRule r : getRules()) {
if (sameGroup(r, group)) {
return r;
}
}
- if (create) {
- PermissionRule r = PermissionRule.create(group);
- rules.add(r);
- return r;
- }
return null;
}
- void mergeFrom(Permission src) {
- for (PermissionRule srcRule : src.getRules()) {
- PermissionRule dstRule = getRule(srcRule.getGroup());
- if (dstRule != null) {
- rules.remove(dstRule);
- rules.add(PermissionRule.merge(srcRule, dstRule));
- } else {
- add(srcRule);
- }
- }
- }
-
private static boolean sameGroup(PermissionRule rule, GroupReference group) {
- if (group.getUUID() != null) {
+ if (group.getUUID() != null && rule.getGroup().getUUID() != null) {
return group.getUUID().equals(rule.getGroup().getUUID());
-
- } else if (group.getName() != null) {
+ } else if (group.getName() != null && rule.getGroup().getName() != null) {
return group.getName().equals(rule.getGroup().getName());
-
} else {
return false;
}
}
- private void initRules() {
- if (rules == null) {
- rules = new ArrayList<>(4);
- }
- }
-
@Override
- public int compareTo(Permission b) {
+ public final int compareTo(Permission b) {
int cmp = index(this) - index(b);
if (cmp == 0) {
cmp = getName().compareTo(b.getName());
@@ -265,28 +208,10 @@
}
@Override
- public boolean equals(Object obj) {
- if (!(obj instanceof Permission)) {
- return false;
- }
-
- final Permission other = (Permission) obj;
- if (!name.equals(other.name) || exclusiveGroup != other.exclusiveGroup) {
- return false;
- }
- return new HashSet<>(getRules()).equals(new HashSet<>(other.getRules()));
- }
-
- @Override
- public int hashCode() {
- return name.hashCode();
- }
-
- @Override
- public String toString() {
+ public final String toString() {
StringBuilder bldr = new StringBuilder();
- bldr.append(name).append(" ");
- if (exclusiveGroup) {
+ bldr.append(getName()).append(" ");
+ if (isExclusiveGroup()) {
bldr.append("[exclusive] ");
}
bldr.append("[");
@@ -300,4 +225,67 @@
bldr.append("]");
return bldr.toString();
}
+
+ protected abstract Builder autoToBuilder();
+
+ public Builder toBuilder() {
+ Builder b = autoToBuilder();
+ getRules().stream().map(PermissionRule::toBuilder).forEach(r -> b.add(r));
+ return b;
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ private final List<PermissionRule.Builder> rulesBuilders;
+
+ Builder() {
+ rulesBuilders = new ArrayList<>();
+ }
+
+ public abstract Builder setName(String value);
+
+ public abstract String getName();
+
+ public abstract Builder setExclusiveGroup(boolean value);
+
+ public Builder modifyRules(Consumer<List<PermissionRule.Builder>> modification) {
+ modification.accept(rulesBuilders);
+ return this;
+ }
+
+ public Builder add(PermissionRule.Builder rule) {
+ return modifyRules(r -> r.add(rule));
+ }
+
+ public Builder remove(PermissionRule rule) {
+ if (rule != null) {
+ return removeRule(rule.getGroup());
+ }
+ return this;
+ }
+
+ public Builder removeRule(GroupReference group) {
+ return modifyRules(rules -> rules.removeIf(rule -> sameGroup(rule.build(), group)));
+ }
+
+ public Builder clearRules() {
+ return modifyRules(r -> r.clear());
+ }
+
+ public Permission build() {
+ setRules(
+ rulesBuilders.stream().map(PermissionRule.Builder::build).collect(toImmutableList()));
+ return autoBuild();
+ }
+
+ public List<PermissionRule.Builder> getRulesBuilders() {
+ return rulesBuilders;
+ }
+
+ protected abstract ImmutableList<PermissionRule> getRules();
+
+ protected abstract Builder setRules(ImmutableList<PermissionRule> rules);
+
+ protected abstract Permission autoBuild();
+ }
}
diff --git a/java/com/google/gerrit/common/data/PermissionRule.java b/java/com/google/gerrit/common/data/PermissionRule.java
index c543769..c411652 100644
--- a/java/com/google/gerrit/common/data/PermissionRule.java
+++ b/java/com/google/gerrit/common/data/PermissionRule.java
@@ -263,6 +263,8 @@
public abstract Builder setMax(int max);
+ public abstract GroupReference getGroup();
+
public abstract PermissionRule build();
}
}
diff --git a/java/com/google/gerrit/entities/Project.java b/java/com/google/gerrit/entities/Project.java
index 759d50a..ef3cbeb 100644
--- a/java/com/google/gerrit/entities/Project.java
+++ b/java/com/google/gerrit/entities/Project.java
@@ -103,7 +103,7 @@
@Nullable
public abstract String getDescription();
- protected abstract ImmutableMap<BooleanProjectConfig, InheritableBoolean> getBooleanConfigs();
+ public abstract ImmutableMap<BooleanProjectConfig, InheritableBoolean> getBooleanConfigs();
/**
* Submit type as configured in {@code project.config}.
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 17313e4..0e593b7 100644
--- a/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
+++ b/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server;
-import static java.util.stream.Collectors.toList;
-
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.AccessSection;
@@ -34,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;
@@ -85,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());
}
@@ -101,23 +100,25 @@
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsers)) {
ProjectConfig config = projectConfigFactory.read(md);
- AccessSection createGroupAccessSection =
- config.getAccessSection(RefNames.REFS_GROUPS + "*", true);
- if (createGroupsGlobal.isEmpty()) {
- createGroupAccessSection.setPermissions(
- createGroupAccessSection.getPermissions().stream()
- .filter(p -> !Permission.CREATE.equals(p.getName()))
- .collect(toList()));
- config.replace(createGroupAccessSection);
- } else {
- // The create permission is managed by Gerrit at this point only so there is no concern of
- // overwriting user-defined permissions here.
- Permission createGroupPermission = new Permission(Permission.CREATE);
- createGroupAccessSection.remove(createGroupPermission);
- createGroupAccessSection.addPermission(createGroupPermission);
- createGroupsGlobal.forEach(createGroupPermission::add);
- config.replace(createGroupAccessSection);
- }
+ config.upsertAccessSection(
+ RefNames.REFS_GROUPS + "*",
+ refsGroupsAccessSectionBuilder -> {
+ if (createGroupsGlobal.isEmpty()) {
+ refsGroupsAccessSectionBuilder.modifyPermissions(
+ permissions -> {
+ permissions.removeIf(p -> Permission.CREATE.equals(p.getName()));
+ });
+ } else {
+ // The create permission is managed by Gerrit at this point only so there is no
+ // concern of overwriting user-defined permissions here.
+ Permission.Builder createGroupPermission = Permission.builder(Permission.CREATE);
+ refsGroupsAccessSectionBuilder.remove(createGroupPermission);
+ refsGroupsAccessSectionBuilder.addPermission(createGroupPermission);
+ createGroupsGlobal.stream()
+ .map(p -> p.toBuilder())
+ .forEach(createGroupPermission::add);
+ }
+ });
config.commit(md);
projectCache.evict(config.getProject());
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/account/CapabilityCollection.java b/java/com/google/gerrit/server/account/CapabilityCollection.java
index 157f946..de522ae 100644
--- a/java/com/google/gerrit/server/account/CapabilityCollection.java
+++ b/java/com/google/gerrit/server/account/CapabilityCollection.java
@@ -60,7 +60,7 @@
this.systemGroupBackend = systemGroupBackend;
if (section == null) {
- section = new AccessSection(AccessSection.GLOBAL_CAPABILITIES);
+ section = AccessSection.create(AccessSection.GLOBAL_CAPABILITIES);
}
Map<String, List<PermissionRule>> tmp = new HashMap<>();
diff --git a/java/com/google/gerrit/server/cache/serialize/entities/BUILD b/java/com/google/gerrit/server/cache/serialize/entities/BUILD
new file mode 100644
index 0000000..81f8f6b
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/serialize/entities/BUILD
@@ -0,0 +1,18 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+java_library(
+ name = "entities",
+ srcs = glob(["*.java"]),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//java/com/google/gerrit/common:annotations",
+ "//java/com/google/gerrit/entities",
+ "//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/git",
+ "//java/com/google/gerrit/proto",
+ "//lib:guava",
+ "//lib:jgit",
+ "//lib:protobuf",
+ "//proto:cache_java_proto",
+ ],
+)
diff --git a/java/com/google/gerrit/server/cache/serialize/entities/ProjectSerializer.java b/java/com/google/gerrit/server/cache/serialize/entities/ProjectSerializer.java
new file mode 100644
index 0000000..aa1f4ce
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/serialize/entities/ProjectSerializer.java
@@ -0,0 +1,95 @@
+// 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.cache.serialize.entities;
+
+import static com.google.common.base.Strings.emptyToNull;
+import static com.google.common.base.Strings.nullToEmpty;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
+import com.google.common.base.Converter;
+import com.google.common.base.Enums;
+import com.google.gerrit.entities.BooleanProjectConfig;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.client.InheritableBoolean;
+import com.google.gerrit.extensions.client.ProjectState;
+import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.server.cache.proto.Cache;
+import java.util.Arrays;
+import java.util.Set;
+
+/** Helper to (de)serialize values for caches. */
+public class ProjectSerializer {
+ private static final Converter<String, ProjectState> PROJECT_STATE_CONVERTER =
+ Enums.stringConverter(ProjectState.class);
+ private static final Converter<String, SubmitType> SUBMIT_TYPE_CONVERTER =
+ Enums.stringConverter(SubmitType.class);
+
+ public static Project deserialize(Cache.ProjectProto proto) {
+ Project.Builder builder =
+ Project.builder(Project.nameKey(proto.getName()))
+ .setSubmitType(SUBMIT_TYPE_CONVERTER.convert(proto.getSubmitType()))
+ .setState(PROJECT_STATE_CONVERTER.convert(proto.getState()))
+ .setDescription(emptyToNull(proto.getDescription()))
+ .setParent(emptyToNull(proto.getParent()))
+ .setMaxObjectSizeLimit(emptyToNull(proto.getMaxObjectSizeLimit()))
+ .setDefaultDashboard(emptyToNull(proto.getDefaultDashboard()))
+ .setLocalDefaultDashboard(emptyToNull(proto.getLocalDefaultDashboard()))
+ .setConfigRefState(emptyToNull(proto.getConfigRefState()));
+
+ Set<String> configs =
+ Arrays.stream(BooleanProjectConfig.values())
+ .map(BooleanProjectConfig::name)
+ .collect(toImmutableSet());
+ proto
+ .getBooleanConfigsMap()
+ .entrySet()
+ .forEach(
+ configEntry -> {
+ if (configs.contains(configEntry.getKey())) {
+ builder.setBooleanConfig(
+ BooleanProjectConfig.valueOf(configEntry.getKey()),
+ InheritableBoolean.valueOf(configEntry.getValue()));
+ }
+ });
+
+ return builder.build();
+ }
+
+ public static Cache.ProjectProto serialize(Project autoValue) {
+ Cache.ProjectProto.Builder builder =
+ Cache.ProjectProto.newBuilder()
+ .setName(autoValue.getName())
+ .setSubmitType(SUBMIT_TYPE_CONVERTER.reverse().convert(autoValue.getSubmitType()))
+ .setState(PROJECT_STATE_CONVERTER.reverse().convert(autoValue.getState()))
+ .setDescription(nullToEmpty(autoValue.getDescription()))
+ .setParent(nullToEmpty(autoValue.getParentName()))
+ .setMaxObjectSizeLimit(nullToEmpty(autoValue.getMaxObjectSizeLimit()))
+ .setDefaultDashboard(nullToEmpty(autoValue.getDefaultDashboard()))
+ .setLocalDefaultDashboard(nullToEmpty(autoValue.getLocalDefaultDashboard()))
+ .setConfigRefState(nullToEmpty(autoValue.getConfigRefState()));
+
+ autoValue
+ .getBooleanConfigs()
+ .entrySet()
+ .forEach(
+ configEntry -> {
+ builder.putBooleanConfigs(configEntry.getKey().name(), configEntry.getValue().name());
+ });
+
+ return builder.build();
+ }
+
+ private ProjectSerializer() {}
+}
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/AccountsSection.java b/java/com/google/gerrit/server/project/AccountsSection.java
index 30bd244..04f63f7 100644
--- a/java/com/google/gerrit/server/project/AccountsSection.java
+++ b/java/com/google/gerrit/server/project/AccountsSection.java
@@ -14,22 +14,16 @@
package com.google.gerrit.server.project;
+import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.data.PermissionRule;
-import java.util.ArrayList;
import java.util.List;
-public class AccountsSection {
- protected List<PermissionRule> sameGroupVisibility;
+@AutoValue
+public abstract class AccountsSection {
+ public abstract ImmutableList<PermissionRule> getSameGroupVisibility();
- public ImmutableList<PermissionRule> getSameGroupVisibility() {
- if (sameGroupVisibility == null) {
- sameGroupVisibility = ImmutableList.of();
- }
- return ImmutableList.copyOf(sameGroupVisibility);
- }
-
- public void setSameGroupVisibility(List<PermissionRule> sameGroupVisibility) {
- this.sameGroupVisibility = new ArrayList<>(sameGroupVisibility);
+ public static AccountsSection create(List<PermissionRule> sameGroupVisibility) {
+ return new AutoValue_AccountsSection(ImmutableList.copyOf(sameGroupVisibility));
}
}
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 cc13606..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);
@@ -345,25 +366,21 @@
return accountsSection;
}
- public Map<String, List<String>> getExtensionPanelSections() {
- return extensionPanelSections;
+ public void setAccountsSection(AccountsSection accountsSection) {
+ this.accountsSection = accountsSection;
}
public AccessSection getAccessSection(String name) {
- return getAccessSection(name, false);
+ return accessSections.get(name);
}
- public AccessSection getAccessSection(String name, boolean create) {
- AccessSection as = accessSections.get(name);
- if (as == null && create) {
- as = new AccessSection(name);
- accessSections.put(name, as);
- }
- return as;
- }
-
- public ImmutableSet<String> getAccessSectionNames() {
- return ImmutableSet.copyOf(accessSections.keySet());
+ public void upsertAccessSection(String name, Consumer<AccessSection.Builder> update) {
+ AccessSection.Builder accessSectionBuilder =
+ accessSections.containsKey(name)
+ ? accessSections.get(name).toBuilder()
+ : AccessSection.builder(name);
+ update.accept(accessSectionBuilder);
+ accessSections.put(name, accessSectionBuilder.build());
}
public Collection<AccessSection> getAccessSections() {
@@ -382,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);
}
@@ -400,8 +407,9 @@
if (section != null) {
String name = section.getName();
if (sectionsWithUnknownPermissions.contains(name)) {
- AccessSection a = accessSections.get(name);
- a.setPermissions(new ArrayList<>());
+ AccessSection.Builder a = accessSections.get(name).toBuilder();
+ a.modifyPermissions(p -> p.clear());
+ accessSections.put(name, a.build());
} else {
accessSections.remove(name);
}
@@ -412,8 +420,9 @@
if (permission == null) {
remove(section);
} else if (section != null) {
- AccessSection a = accessSections.get(section.getName());
- a.remove(permission);
+ AccessSection a =
+ accessSections.get(section.getName()).toBuilder().remove(permission.toBuilder()).build();
+ accessSections.put(section.getName(), a);
if (a.getPermissions().isEmpty()) {
remove(a);
}
@@ -432,28 +441,21 @@
if (p == null) {
return;
}
- p.remove(rule);
- if (p.getRules().isEmpty()) {
- a.remove(permission);
+ AccessSection.Builder accessSectionBuilder = a.toBuilder();
+ Permission.Builder permissionBuilder =
+ accessSectionBuilder.upsertPermission(permission.getName());
+ permissionBuilder.remove(rule);
+ if (permissionBuilder.build().getRules().isEmpty()) {
+ accessSectionBuilder.remove(permissionBuilder);
}
+ a = accessSectionBuilder.build();
+ accessSections.put(section.getName(), a);
if (a.getPermissions().isEmpty()) {
remove(a);
}
}
}
- public void replace(AccessSection section) {
- for (Permission permission : section.getPermissions()) {
- ImmutableList.Builder<PermissionRule> newRules = ImmutableList.builder();
- for (PermissionRule rule : permission.getRules()) {
- newRules.add(rule.toBuilder().setGroup(resolve(rule.getGroup())).build());
- }
- permission.setRules(newRules.build());
- }
-
- accessSections.put(section.getName(), section);
- }
-
public ContributorAgreement getContributorAgreement(String name) {
return contributorAgreements.get(name);
}
@@ -541,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.
*/
@@ -655,9 +652,9 @@
}
private void loadAccountsSection(Config rc) {
- accountsSection = new AccountsSection();
- accountsSection.setSameGroupVisibility(
- loadPermissionRules(rc, ACCOUNTS, null, KEY_SAME_GROUP_VISIBILITY, false));
+ accountsSection =
+ AccountsSection.create(
+ loadPermissionRules(rc, ACCOUNTS, null, KEY_SAME_GROUP_VISIBILITY, false));
}
private void loadExtensionPanelSections(Config rc) {
@@ -790,38 +787,41 @@
sectionsWithUnknownPermissions = new HashSet<>();
for (String refName : rc.getSubsections(ACCESS)) {
if (AccessSection.isValidRefSectionName(refName) && isValidRegex(refName)) {
- AccessSection as = getAccessSection(refName, true);
+ upsertAccessSection(
+ refName,
+ as -> {
+ for (String varName : rc.getStringList(ACCESS, refName, KEY_GROUP_PERMISSIONS)) {
+ for (String n : Splitter.on(EXCLUSIVE_PERMISSIONS_SPLIT_PATTERN).split(varName)) {
+ n = convertLegacyPermission(n);
+ if (isCoreOrPluginPermission(n)) {
+ as.upsertPermission(n).setExclusiveGroup(true);
+ }
+ }
+ }
- for (String varName : rc.getStringList(ACCESS, refName, KEY_GROUP_PERMISSIONS)) {
- for (String n : Splitter.on(EXCLUSIVE_PERMISSIONS_SPLIT_PATTERN).split(varName)) {
- n = convertLegacyPermission(n);
- if (isCoreOrPluginPermission(n)) {
- as.getPermission(n, true).setExclusiveGroup(true);
- }
- }
- }
-
- for (String varName : rc.getNames(ACCESS, refName)) {
- String convertedName = convertLegacyPermission(varName);
- if (isCoreOrPluginPermission(convertedName)) {
- Permission perm = as.getPermission(convertedName, true);
- loadPermissionRules(
- rc, ACCESS, refName, varName, perm, Permission.hasRange(convertedName));
- } else {
- sectionsWithUnknownPermissions.add(as.getName());
- }
- }
+ for (String varName : rc.getNames(ACCESS, refName)) {
+ String convertedName = convertLegacyPermission(varName);
+ if (isCoreOrPluginPermission(convertedName)) {
+ Permission.Builder perm = as.upsertPermission(convertedName);
+ loadPermissionRules(
+ rc, ACCESS, refName, varName, perm, Permission.hasRange(convertedName));
+ } else {
+ sectionsWithUnknownPermissions.add(as.getName());
+ }
+ }
+ });
}
}
- AccessSection capability = null;
+ AccessSection.Builder capability = null;
for (String varName : rc.getNames(CAPABILITY)) {
if (capability == null) {
- capability = new AccessSection(AccessSection.GLOBAL_CAPABILITIES);
- accessSections.put(AccessSection.GLOBAL_CAPABILITIES, capability);
+ capability = AccessSection.builder(AccessSection.GLOBAL_CAPABILITIES);
+ accessSections.put(AccessSection.GLOBAL_CAPABILITIES, capability.build());
}
- Permission perm = capability.getPermission(varName, true);
+ Permission.Builder perm = capability.upsertPermission(varName);
loadPermissionRules(rc, CAPABILITY, null, varName, perm, GlobalCapability.hasRange(varName));
+ accessSections.put(AccessSection.GLOBAL_CAPABILITIES, capability.build());
}
}
@@ -874,9 +874,9 @@
private ImmutableList<PermissionRule> loadPermissionRules(
Config rc, String section, String subsection, String varName, boolean useRange) {
- Permission perm = new Permission(varName);
+ Permission.Builder perm = Permission.builder(varName);
loadPermissionRules(rc, section, subsection, varName, perm, useRange);
- return ImmutableList.copyOf(perm.getRules());
+ return ImmutableList.copyOf(perm.build().getRules());
}
private void loadPermissionRules(
@@ -884,7 +884,7 @@
String section,
String subsection,
String varName,
- Permission perm,
+ Permission.Builder perm,
boolean useRange) {
for (String ruleString : rc.getStringList(section, subsection, varName)) {
PermissionRule rule;
@@ -916,7 +916,7 @@
PROJECT_CONFIG, "group \"" + ref.getName() + "\" not in " + GroupList.FILE_NAME));
}
- perm.add(rule.toBuilder().setGroup(ref).build());
+ perm.add(rule.toBuilder().setGroup(ref));
}
}
diff --git a/java/com/google/gerrit/server/project/ProjectCreator.java b/java/com/google/gerrit/server/project/ProjectCreator.java
index 5ec4a58..fa0a262 100644
--- a/java/com/google/gerrit/server/project/ProjectCreator.java
+++ b/java/com/google/gerrit/server/project/ProjectCreator.java
@@ -179,14 +179,17 @@
});
if (!args.ownerIds.isEmpty()) {
- AccessSection all = config.getAccessSection(AccessSection.ALL, true);
- for (AccountGroup.UUID ownerId : args.ownerIds) {
- GroupDescription.Basic g = groupBackend.get(ownerId);
- if (g != null) {
- GroupReference group = config.resolve(GroupReference.forGroup(g));
- all.getPermission(Permission.OWNER, true).add(PermissionRule.create(group));
- }
- }
+ config.upsertAccessSection(
+ AccessSection.ALL,
+ all -> {
+ for (AccountGroup.UUID ownerId : args.ownerIds) {
+ GroupDescription.Basic g = groupBackend.get(ownerId);
+ if (g != null) {
+ GroupReference group = config.resolve(GroupReference.forGroup(g));
+ all.upsertPermission(Permission.OWNER).add(PermissionRule.builder(group));
+ }
+ }
+ });
}
md.setMessage("Created project\n");
diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java
index 6353103..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;
}
@@ -280,14 +286,16 @@
sm = new ArrayList<>(fromConfig.size());
for (AccessSection section : fromConfig) {
if (isAllProjects) {
- List<Permission> copy = Lists.newArrayListWithCapacity(section.getPermissions().size());
+ List<Permission.Builder> copy = new ArrayList<>();
for (Permission p : section.getPermissions()) {
if (Permission.canBeOnAllProjects(section.getName(), p.getName())) {
- copy.add(p);
+ copy.add(p.toBuilder());
}
}
- section = new AccessSection(section.getName());
- section.setPermissions(copy);
+ section =
+ AccessSection.builder(section.getName())
+ .modifyPermissions(permissions -> permissions.addAll(copy))
+ .build();
}
SectionMatcher matcher = SectionMatcher.wrap(getNameKey(), section);
@@ -457,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);
@@ -473,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 5a3dbcd..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);
@@ -208,9 +208,9 @@
// user is a member of, as well as groups they own or that
// are visible to all users.
- AccessSection dst = null;
+ AccessSection.Builder dst = null;
for (Permission srcPerm : section.getPermissions()) {
- Permission dstPerm = null;
+ Permission.Builder dstPerm = null;
for (PermissionRule srcRule : srcPerm.getRules()) {
AccountGroup.UUID groupId = srcRule.getGroup().getUUID();
@@ -221,12 +221,12 @@
loadGroup(groups, groupId);
if (dstPerm == null) {
if (dst == null) {
- dst = new AccessSection(name);
- info.local.put(name, createAccessSection(groups, dst));
+ dst = AccessSection.builder(name);
+ info.local.put(name, createAccessSection(groups, dst.build()));
}
- dstPerm = dst.getPermission(srcPerm.getName(), true);
+ dstPerm = dst.upsertPermission(srcPerm.getName());
}
- dstPerm.add(srcRule);
+ dstPerm.add(srcRule.toBuilder());
}
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java b/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
index 0f7bc3d..d1511c1 100644
--- a/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
+++ b/java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
@@ -78,14 +78,14 @@
continue;
}
- AccessSection accessSection = new AccessSection(entry.getKey());
+ AccessSection.Builder accessSection = AccessSection.builder(entry.getKey());
for (Map.Entry<String, PermissionInfo> permissionEntry :
entry.getValue().permissions.entrySet()) {
if (permissionEntry.getValue().rules == null) {
continue;
}
- Permission p = new Permission(permissionEntry.getKey());
+ Permission.Builder p = Permission.builder(permissionEntry.getKey());
if (permissionEntry.getValue().exclusive != null) {
p.setExclusiveGroup(permissionEntry.getValue().exclusive);
}
@@ -114,11 +114,11 @@
r.setForce(pri.force);
}
}
- p.add(r.build());
+ p.add(r);
}
accessSection.addPermission(p);
}
- sections.add(accessSection);
+ sections.add(accessSection.build());
}
return sections;
}
@@ -193,25 +193,23 @@
// Apply additions
for (AccessSection section : additions) {
- AccessSection currentAccessSection = config.getAccessSection(section.getName());
-
- if (currentAccessSection == null) {
- // Add AccessSection
- config.replace(section);
- } else {
- for (Permission p : section.getPermissions()) {
- Permission currentPermission = currentAccessSection.getPermission(p.getName());
- if (currentPermission == null) {
- // Add Permission
- currentAccessSection.addPermission(p);
- } else {
- for (PermissionRule r : p.getRules()) {
- // AddPermissionRule
- currentPermission.add(r);
+ config.upsertAccessSection(
+ section.getName(),
+ existingAccessSection -> {
+ for (Permission p : section.getPermissions()) {
+ Permission currentPermission =
+ existingAccessSection.build().getPermission(p.getName());
+ if (currentPermission == null) {
+ // Add Permission
+ existingAccessSection.addPermission(p.toBuilder());
+ } else {
+ for (PermissionRule r : p.getRules()) {
+ // AddPermissionRule
+ existingAccessSection.upsertPermission(p.getName()).add(r.toBuilder());
+ }
+ }
}
- }
- }
- }
+ });
}
}
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/java/com/google/gerrit/server/schema/AclUtil.java b/java/com/google/gerrit/server/schema/AclUtil.java
index ed4a6ec..e65568f 100644
--- a/java/com/google/gerrit/server/schema/AclUtil.java
+++ b/java/com/google/gerrit/server/schema/AclUtil.java
@@ -27,13 +27,16 @@
*/
public class AclUtil {
public static void grant(
- ProjectConfig config, AccessSection section, String permission, GroupReference... groupList) {
+ ProjectConfig config,
+ AccessSection.Builder section,
+ String permission,
+ GroupReference... groupList) {
grant(config, section, permission, false, groupList);
}
public static void grant(
ProjectConfig config,
- AccessSection section,
+ AccessSection.Builder section,
String permission,
boolean force,
GroupReference... groupList) {
@@ -42,35 +45,38 @@
public static void grant(
ProjectConfig config,
- AccessSection section,
+ AccessSection.Builder section,
String permission,
boolean force,
Boolean exclusive,
GroupReference... groupList) {
- Permission p = section.getPermission(permission, true);
+ Permission.Builder p = section.upsertPermission(permission);
if (exclusive != null) {
p.setExclusiveGroup(exclusive);
}
for (GroupReference group : groupList) {
if (group != null) {
- p.add(rule(config, group).setForce(force).build());
+ p.add(rule(config, group).setForce(force));
}
}
}
public static void block(
- ProjectConfig config, AccessSection section, String permission, GroupReference... groupList) {
- Permission p = section.getPermission(permission, true);
+ ProjectConfig config,
+ AccessSection.Builder section,
+ String permission,
+ GroupReference... groupList) {
+ Permission.Builder p = section.upsertPermission(permission);
for (GroupReference group : groupList) {
if (group != null) {
- p.add(rule(config, group).setBlock().build());
+ p.add(rule(config, group).setBlock());
}
}
}
public static void grant(
ProjectConfig config,
- AccessSection section,
+ AccessSection.Builder section,
LabelType type,
int min,
int max,
@@ -80,18 +86,18 @@
public static void grant(
ProjectConfig config,
- AccessSection section,
+ AccessSection.Builder section,
LabelType type,
int min,
int max,
boolean exclusive,
GroupReference... groupList) {
String name = Permission.LABEL + type.getName();
- Permission p = section.getPermission(name, true);
+ Permission.Builder p = section.upsertPermission(name);
p.setExclusiveGroup(exclusive);
for (GroupReference group : groupList) {
if (group != null) {
- p.add(rule(config, group).setRange(min, max).build());
+ p.add(rule(config, group).setRange(min, max));
}
}
}
@@ -101,8 +107,11 @@
}
public static void remove(
- ProjectConfig config, AccessSection section, String permission, GroupReference... groupList) {
- Permission p = section.getPermission(permission, true);
+ ProjectConfig config,
+ AccessSection.Builder section,
+ String permission,
+ GroupReference... groupList) {
+ Permission.Builder p = section.upsertPermission(permission);
for (GroupReference group : groupList) {
if (group != null) {
p.remove(rule(config, group).build());
diff --git a/java/com/google/gerrit/server/schema/AllProjectsCreator.java b/java/com/google/gerrit/server/schema/AllProjectsCreator.java
index 908ce0a..0fb282e 100644
--- a/java/com/google/gerrit/server/schema/AllProjectsCreator.java
+++ b/java/com/google/gerrit/server/schema/AllProjectsCreator.java
@@ -144,79 +144,107 @@
}
private void initDefaultAcls(ProjectConfig config, AllProjectsInput input) {
- AccessSection capabilities = config.getAccessSection(AccessSection.GLOBAL_CAPABILITIES, true);
- AccessSection heads = config.getAccessSection(AccessSection.HEADS, true);
-
checkArgument(input.codeReviewLabel().isPresent());
LabelType codeReviewLabel = input.codeReviewLabel().get();
- initDefaultAclsForRegisteredUsers(heads, codeReviewLabel, config);
+ config.upsertAccessSection(
+ AccessSection.HEADS,
+ heads -> {
+ initDefaultAclsForRegisteredUsers(heads, codeReviewLabel, config);
+ });
- input
- .batchUsersGroup()
- .ifPresent(
- batchUsersGroup -> initDefaultAclsForBatchUsers(capabilities, config, batchUsersGroup));
+ config.upsertAccessSection(
+ AccessSection.GLOBAL_CAPABILITIES,
+ capabilities -> {
+ input
+ .batchUsersGroup()
+ .ifPresent(
+ batchUsersGroup ->
+ initDefaultAclsForBatchUsers(capabilities, config, batchUsersGroup));
+ });
input
.administratorsGroup()
- .ifPresent(
- adminsGroup ->
- initDefaultAclsForAdmins(
- capabilities, config, heads, codeReviewLabel, adminsGroup));
+ .ifPresent(adminsGroup -> initDefaultAclsForAdmins(config, codeReviewLabel, adminsGroup));
}
private void initDefaultAclsForRegisteredUsers(
- AccessSection heads, LabelType codeReviewLabel, ProjectConfig config) {
- AccessSection refsFor = config.getAccessSection("refs/for/*", true);
- AccessSection magic = config.getAccessSection("refs/for/" + AccessSection.ALL, true);
- AccessSection all = config.getAccessSection("refs/*", true);
+ AccessSection.Builder heads, LabelType codeReviewLabel, ProjectConfig config) {
+ config.upsertAccessSection(
+ "refs/for/*",
+ refsFor -> {
+ grant(config, refsFor, Permission.ADD_PATCH_SET, registered);
+ });
- grant(config, refsFor, Permission.ADD_PATCH_SET, registered);
grant(config, heads, codeReviewLabel, -1, 1, registered);
grant(config, heads, Permission.FORGE_AUTHOR, registered);
- grant(config, all, Permission.REVERT, registered);
- grant(config, magic, Permission.PUSH, registered);
- grant(config, magic, Permission.PUSH_MERGE, registered);
+
+ config.upsertAccessSection(
+ "refs/*",
+ all -> {
+ grant(config, all, Permission.REVERT, registered);
+ });
+
+ config.upsertAccessSection(
+ "refs/for/" + AccessSection.ALL,
+ magic -> {
+ grant(config, magic, Permission.PUSH, registered);
+ grant(config, magic, Permission.PUSH_MERGE, registered);
+ });
}
private void initDefaultAclsForBatchUsers(
- AccessSection capabilities, ProjectConfig config, GroupReference batchUsersGroup) {
- Permission priority = capabilities.getPermission(GlobalCapability.PRIORITY, true);
- priority.add(rule(config, batchUsersGroup).setAction(Action.BATCH).build());
+ AccessSection.Builder capabilities, ProjectConfig config, GroupReference batchUsersGroup) {
+ Permission.Builder priority = capabilities.upsertPermission(GlobalCapability.PRIORITY);
+ priority.add(rule(config, batchUsersGroup).setAction(Action.BATCH));
- Permission stream = capabilities.getPermission(GlobalCapability.STREAM_EVENTS, true);
- stream.add(rule(config, batchUsersGroup).build());
+ Permission.Builder stream = capabilities.upsertPermission(GlobalCapability.STREAM_EVENTS);
+ stream.add(rule(config, batchUsersGroup));
}
private void initDefaultAclsForAdmins(
- AccessSection capabilities,
- ProjectConfig config,
- AccessSection heads,
- LabelType codeReviewLabel,
- GroupReference adminsGroup) {
- AccessSection all = config.getAccessSection(AccessSection.ALL, true);
- AccessSection tags = config.getAccessSection("refs/tags/*", true);
- AccessSection meta = config.getAccessSection(RefNames.REFS_CONFIG, true);
+ ProjectConfig config, LabelType codeReviewLabel, GroupReference adminsGroup) {
+ config.upsertAccessSection(
+ AccessSection.GLOBAL_CAPABILITIES,
+ capabilities -> {
+ grant(config, capabilities, GlobalCapability.ADMINISTRATE_SERVER, adminsGroup);
+ });
- grant(config, capabilities, GlobalCapability.ADMINISTRATE_SERVER, adminsGroup);
- grant(config, all, Permission.READ, adminsGroup, anonymous);
- grant(config, heads, codeReviewLabel, -2, 2, adminsGroup, owners);
- grant(config, heads, Permission.CREATE, adminsGroup, owners);
- grant(config, heads, Permission.PUSH, adminsGroup, owners);
- grant(config, heads, Permission.SUBMIT, adminsGroup, owners);
- grant(config, heads, Permission.FORGE_COMMITTER, adminsGroup, owners);
- grant(config, heads, Permission.EDIT_TOPIC_NAME, true, adminsGroup, owners);
+ config.upsertAccessSection(
+ AccessSection.ALL,
+ all -> {
+ grant(config, all, Permission.READ, adminsGroup, anonymous);
+ });
- grant(config, tags, Permission.CREATE, adminsGroup, owners);
- grant(config, tags, Permission.CREATE_TAG, adminsGroup, owners);
- grant(config, tags, Permission.CREATE_SIGNED_TAG, adminsGroup, owners);
+ config.upsertAccessSection(
+ AccessSection.HEADS,
+ heads -> {
+ grant(config, heads, codeReviewLabel, -2, 2, adminsGroup, owners);
+ grant(config, heads, Permission.CREATE, adminsGroup, owners);
+ grant(config, heads, Permission.PUSH, adminsGroup, owners);
+ grant(config, heads, Permission.SUBMIT, adminsGroup, owners);
+ grant(config, heads, Permission.FORGE_COMMITTER, adminsGroup, owners);
+ grant(config, heads, Permission.EDIT_TOPIC_NAME, true, adminsGroup, owners);
+ });
- meta.getPermission(Permission.READ, true).setExclusiveGroup(true);
- grant(config, meta, Permission.READ, adminsGroup, owners);
- grant(config, meta, codeReviewLabel, -2, 2, adminsGroup, owners);
- grant(config, meta, Permission.CREATE, adminsGroup, owners);
- grant(config, meta, Permission.PUSH, adminsGroup, owners);
- grant(config, meta, Permission.SUBMIT, adminsGroup, owners);
+ config.upsertAccessSection(
+ "refs/tags/*",
+ tags -> {
+ grant(config, tags, Permission.CREATE, adminsGroup, owners);
+ grant(config, tags, Permission.CREATE_TAG, adminsGroup, owners);
+ grant(config, tags, Permission.CREATE_SIGNED_TAG, adminsGroup, owners);
+ });
+
+ config.upsertAccessSection(
+ RefNames.REFS_CONFIG,
+ meta -> {
+ meta.upsertPermission(Permission.READ).setExclusiveGroup(true);
+ grant(config, meta, Permission.READ, adminsGroup, owners);
+ grant(config, meta, codeReviewLabel, -2, 2, adminsGroup, owners);
+ grant(config, meta, Permission.CREATE, adminsGroup, owners);
+ grant(config, meta, Permission.PUSH, adminsGroup, owners);
+ grant(config, meta, Permission.SUBMIT, adminsGroup, owners);
+ });
}
private void initSequences(Repository git, BatchRefUpdate bru, int firstChangeId)
diff --git a/java/com/google/gerrit/server/schema/AllUsersCreator.java b/java/com/google/gerrit/server/schema/AllUsersCreator.java
index 10d7070..1ac8e69 100644
--- a/java/com/google/gerrit/server/schema/AllUsersCreator.java
+++ b/java/com/google/gerrit/server/schema/AllUsersCreator.java
@@ -22,7 +22,6 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.common.Version;
-import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
@@ -113,33 +112,39 @@
ProjectConfig config = projectConfigFactory.read(md);
config.updateProject(p -> p.setDescription("Individual user settings and preferences."));
- AccessSection users =
- config.getAccessSection(
- RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}", true);
+ config.upsertAccessSection(
+ RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}",
+ users -> {
+ grant(config, users, Permission.READ, false, true, registered);
+ grant(config, users, Permission.PUSH, false, true, registered);
+ grant(config, users, Permission.SUBMIT, false, true, registered);
+ grant(config, users, codeReviewLabel, -2, 2, true, registered);
+ });
// Initialize "Code-Review" label.
config.upsertLabelType(codeReviewLabel);
- grant(config, users, Permission.READ, false, true, registered);
- grant(config, users, Permission.PUSH, false, true, registered);
- grant(config, users, Permission.SUBMIT, false, true, registered);
- grant(config, users, codeReviewLabel, -2, 2, true, registered);
-
if (admin != null) {
- AccessSection defaults = config.getAccessSection(RefNames.REFS_USERS_DEFAULT, true);
- defaults.getPermission(Permission.READ, true).setExclusiveGroup(true);
- grant(config, defaults, Permission.READ, admin);
- defaults.getPermission(Permission.PUSH, true).setExclusiveGroup(true);
- grant(config, defaults, Permission.PUSH, admin);
- defaults.getPermission(Permission.CREATE, true).setExclusiveGroup(true);
- grant(config, defaults, Permission.CREATE, admin);
+ config.upsertAccessSection(
+ RefNames.REFS_USERS_DEFAULT,
+ defaults -> {
+ defaults.upsertPermission(Permission.READ).setExclusiveGroup(true);
+ grant(config, defaults, Permission.READ, admin);
+ defaults.upsertPermission(Permission.PUSH).setExclusiveGroup(true);
+ grant(config, defaults, Permission.PUSH, admin);
+ defaults.upsertPermission(Permission.CREATE).setExclusiveGroup(true);
+ grant(config, defaults, Permission.CREATE, admin);
+ });
}
// Grant read permissions on the group branches to all users.
// This allows group owners to see the group refs. VisibleRefFilter ensures that read
// permissions for non-group-owners are ignored.
- AccessSection groups = config.getAccessSection(RefNames.REFS_GROUPS + "*", true);
- grant(config, groups, Permission.READ, false, true, registered);
+ config.upsertAccessSection(
+ RefNames.REFS_GROUPS + "*",
+ groups -> {
+ grant(config, groups, Permission.READ, false, true, registered);
+ });
config.commit(md);
}
diff --git a/java/com/google/gerrit/server/schema/GrantRevertPermission.java b/java/com/google/gerrit/server/schema/GrantRevertPermission.java
index 2f890d5..d4ba29b 100644
--- a/java/com/google/gerrit/server/schema/GrantRevertPermission.java
+++ b/java/com/google/gerrit/server/schema/GrantRevertPermission.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.Inject;
import java.io.IOException;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
@@ -63,28 +64,41 @@
try (Repository repo = repoManager.openRepository(projectName)) {
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, projectName, repo);
ProjectConfig projectConfig = projectConfigFactory.read(md);
- AccessSection heads = projectConfig.getAccessSection(AccessSection.HEADS, true);
- Permission permissionOnRefsHeads = heads.getPermission(Permission.REVERT);
+ AtomicBoolean shouldExit = new AtomicBoolean(false);
+ projectConfig.upsertAccessSection(
+ AccessSection.HEADS,
+ heads -> {
+ Permission permissionOnRefsHeads = heads.build().getPermission(Permission.REVERT);
- if (permissionOnRefsHeads != null) {
- if (permissionOnRefsHeads.getRule(registeredUsers) == null
- || permissionOnRefsHeads.getRules().size() > 1) {
- // If admins already changed the permission, don't do anything.
- return;
- }
- // permission already exists in refs/heads/*, delete it for Registered Users.
- remove(projectConfig, heads, Permission.REVERT, registeredUsers);
- }
+ if (permissionOnRefsHeads != null) {
+ if (permissionOnRefsHeads.getRule(registeredUsers) == null
+ || permissionOnRefsHeads.getRules().size() > 1) {
+ // If admins already changed the permission, don't do anything.
+ shouldExit.set(true);
+ return;
+ }
+ // permission already exists in refs/heads/*, delete it for Registered Users.
+ remove(projectConfig, heads, Permission.REVERT, registeredUsers);
+ }
+ });
- AccessSection all = projectConfig.getAccessSection(AccessSection.ALL, true);
- Permission permissionOnRefsStar = all.getPermission(Permission.REVERT);
- if (permissionOnRefsStar != null && permissionOnRefsStar.getRule(registeredUsers) != null) {
- // permission already exists in refs/*, don't do anything.
+ if (shouldExit.get()) {
return;
}
- // If the permission doesn't exist of refs/* for Registered Users, grant it.
- grant(projectConfig, all, Permission.REVERT, registeredUsers);
+
+ projectConfig.upsertAccessSection(
+ AccessSection.ALL,
+ all -> {
+ Permission permissionOnRefsStar = all.build().getPermission(Permission.REVERT);
+ if (permissionOnRefsStar != null
+ && permissionOnRefsStar.getRule(registeredUsers) != null) {
+ // permission already exists in refs/*, don't do anything.
+ return;
+ }
+ // If the permission doesn't exist of refs/* for Registered Users, grant it.
+ grant(projectConfig, all, Permission.REVERT, registeredUsers);
+ });
md.getCommitBuilder().setAuthor(serverUser);
md.getCommitBuilder().setCommitter(serverUser);
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 3c1bc2f..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);
@@ -1653,8 +1656,11 @@
// remove default READ permissions
try (ProjectConfigUpdate u = updateProject(allUsers)) {
u.getConfig()
- .getAccessSection(RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}", true)
- .remove(new Permission(Permission.READ));
+ .upsertAccessSection(
+ RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}",
+ as -> {
+ as.remove(Permission.builder(Permission.READ));
+ });
u.save();
}
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/git/PushPermissionsIT.java b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
index 98b93a8..b1c07ad 100644
--- a/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
@@ -362,22 +362,28 @@
}
private static void removeAllBranchPermissions(ProjectConfig cfg, String... permissions) {
- cfg.getAccessSections().stream()
- .filter(
- s ->
- s.getName().startsWith("refs/heads/")
- || s.getName().startsWith("refs/for/")
- || s.getName().equals("refs/*"))
- .forEach(s -> Arrays.stream(permissions).forEach(s::removePermission));
+ for (AccessSection s : ImmutableList.copyOf(cfg.getAccessSections())) {
+ if (s.getName().startsWith("refs/heads/")
+ || s.getName().startsWith("refs/for/")
+ || s.getName().equals("refs/*")) {
+ cfg.upsertAccessSection(
+ s.getName(),
+ updatedSection -> {
+ Arrays.stream(permissions).forEach(p -> updatedSection.remove(Permission.builder(p)));
+ });
+ }
+ }
}
private static void removeAllGlobalCapabilities(ProjectConfig cfg, String... capabilities) {
Arrays.stream(capabilities)
.forEach(
c ->
- cfg.getAccessSection(AccessSection.GLOBAL_CAPABILITIES, true)
- .getPermission(c, true)
- .clearRules());
+ cfg.upsertAccessSection(
+ AccessSection.GLOBAL_CAPABILITIES,
+ as -> {
+ as.upsertPermission(c).clearRules();
+ }));
}
private PushResult push(String... refSpecs) throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index 14288b5..8b9d173 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -127,8 +127,14 @@
private void setUpPermissions() throws Exception {
// Remove read permissions for all users besides admin.
try (ProjectConfigUpdate u = updateProject(allProjects)) {
- for (AccessSection sec : u.getConfig().getAccessSections()) {
- sec.removePermission(Permission.READ);
+
+ for (AccessSection sec : ImmutableList.copyOf(u.getConfig().getAccessSections())) {
+ u.getConfig()
+ .upsertAccessSection(
+ sec.getName(),
+ updatedSec -> {
+ updatedSec.removePermission(Permission.READ);
+ });
}
u.save();
}
@@ -139,8 +145,13 @@
// Remove all read permissions on All-Users.
try (ProjectConfigUpdate u = updateProject(allUsers)) {
- for (AccessSection sec : u.getConfig().getAccessSections()) {
- sec.removePermission(Permission.READ);
+ for (AccessSection sec : ImmutableList.copyOf(u.getConfig().getAccessSections())) {
+ u.getConfig()
+ .upsertAccessSection(
+ sec.getName(),
+ updatedSec -> {
+ updatedSec.removePermission(Permission.READ);
+ });
}
u.save();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index 0514e03..46054ec 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -120,8 +120,11 @@
try (Repository repo = repoManager.openRepository(newProjectName)) {
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo);
ProjectConfig projectConfig = projectConfigFactory.read(md);
- AccessSection heads = projectConfig.getAccessSection(AccessSection.HEADS, true);
- grant(projectConfig, heads, Permission.REVERT, registeredUsers);
+ projectConfig.upsertAccessSection(
+ AccessSection.HEADS,
+ heads -> {
+ grant(projectConfig, heads, Permission.REVERT, registeredUsers);
+ });
md.getCommitBuilder().setAuthor(admin.newIdent());
md.getCommitBuilder().setCommitter(admin.newIdent());
md.setMessage("Add revert permission for all registered users\n");
@@ -155,15 +158,19 @@
try (Repository repo = repoManager.openRepository(newProjectName)) {
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo);
ProjectConfig projectConfig = projectConfigFactory.read(md);
- AccessSection heads = projectConfig.getAccessSection(AccessSection.HEADS, true);
- grant(projectConfig, heads, Permission.REVERT, registeredUsers);
- grant(projectConfig, heads, Permission.REVERT, otherGroup);
+ projectConfig.upsertAccessSection(
+ AccessSection.HEADS,
+ heads -> {
+ grant(projectConfig, heads, Permission.REVERT, registeredUsers);
+ grant(projectConfig, heads, Permission.REVERT, otherGroup);
+ });
md.getCommitBuilder().setAuthor(admin.newIdent());
md.getCommitBuilder().setCommitter(admin.newIdent());
md.setMessage("Add revert permission for all registered users\n");
projectConfig.commit(md);
}
+ projectCache.evict(newProjectName);
ProjectAccessInfo expected = pApi().access();
grantRevertPermission.execute(newProjectName);
@@ -181,7 +188,7 @@
try (Repository repo = repoManager.openRepository(newProjectName)) {
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo);
ProjectConfig projectConfig = projectConfigFactory.read(md);
- AccessSection all = projectConfig.getAccessSection(AccessSection.ALL, true);
+ AccessSection all = projectConfig.getAccessSection(AccessSection.ALL);
Permission permission = all.getPermission(Permission.REVERT);
assertThat(permission.getRules()).hasSize(1);
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/GetCommitIT.java b/javatests/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
index b18db81..1b1a36d 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
@@ -129,7 +129,12 @@
private void unblockRead() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().getAccessSection("refs/*").remove(new Permission(Permission.READ));
+ u.getConfig()
+ .upsertAccessSection(
+ "refs/*",
+ as -> {
+ as.remove(Permission.builder(Permission.READ));
+ });
u.save();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
index f5d2db4..3d3865a 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
@@ -28,6 +28,7 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.projects.ProjectApi.ListRefsRequest;
import com.google.gerrit.extensions.api.projects.TagApi;
@@ -158,6 +159,12 @@
@Test
public void listTagsOfNonVisibleBranch() throws Exception {
grantTagPermissions();
+ // Allow creating a new hidden branch
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.CREATE).group(REGISTERED_USERS).ref("refs/heads/hidden"))
+ .update();
PushOneCommit push1 = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r1 = push1.to("refs/heads/master");
@@ -169,7 +176,7 @@
assertThat(result.ref).isEqualTo(R_TAGS + tag1.ref);
assertThat(result.revision).isEqualTo(tag1.revision);
- pushTo("refs/heads/hidden");
+ pushTo("refs/heads/hidden").assertOkStatus();
PushOneCommit push2 = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r2 = push2.to("refs/heads/hidden");
r2.assertOkStatus();
@@ -470,8 +477,12 @@
}
private static void removeAllBranchPermissions(ProjectConfig cfg, String... permissions) {
- cfg.getAccessSections().stream()
- .filter(s -> s.getName().startsWith("refs/tags/"))
- .forEach(s -> Arrays.stream(permissions).forEach(s::removePermission));
+ for (AccessSection accessSection : ImmutableList.copyOf(cfg.getAccessSections())) {
+ cfg.upsertAccessSection(
+ accessSection.getName(),
+ updatedAccessSection -> {
+ Arrays.stream(permissions).forEach(updatedAccessSection::removePermission);
+ });
+ }
}
}
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/common/data/AccessSectionTest.java b/javatests/com/google/gerrit/common/data/AccessSectionTest.java
index e775cbc..f60156c 100644
--- a/javatests/com/google/gerrit/common/data/AccessSectionTest.java
+++ b/javatests/com/google/gerrit/common/data/AccessSectionTest.java
@@ -17,9 +17,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
-import com.google.common.collect.ImmutableList;
-import java.util.ArrayList;
-import java.util.List;
import java.util.Locale;
import org.junit.Before;
import org.junit.Test;
@@ -27,11 +24,11 @@
public class AccessSectionTest {
private static final String REF_PATTERN = "refs/heads/master";
- private AccessSection accessSection;
+ private AccessSection.Builder accessSection;
@Before
public void setup() {
- this.accessSection = new AccessSection(REF_PATTERN);
+ this.accessSection = AccessSection.builder(REF_PATTERN);
}
@Test
@@ -41,22 +38,36 @@
@Test
public void getEmptyPermissions() {
- assertThat(accessSection.getPermissions()).isNotNull();
- assertThat(accessSection.getPermissions()).isEmpty();
+ AccessSection builtAccessSection = accessSection.build();
+ assertThat(builtAccessSection.getPermissions()).isNotNull();
+ assertThat(builtAccessSection.getPermissions()).isEmpty();
}
@Test
public void setAndGetPermissions() {
- Permission abandonPermission = new Permission(Permission.ABANDON);
- Permission rebasePermission = new Permission(Permission.REBASE);
- accessSection.setPermissions(ImmutableList.of(abandonPermission, rebasePermission));
- assertThat(accessSection.getPermissions())
- .containsExactly(abandonPermission, rebasePermission)
- .inOrder();
+ Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
+ Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
+ accessSection.modifyPermissions(
+ permissions -> {
+ permissions.clear();
+ permissions.add(abandonPermission);
+ permissions.add(rebasePermission);
+ });
- Permission submitPermission = new Permission(Permission.SUBMIT);
- accessSection.setPermissions(ImmutableList.of(submitPermission));
- assertThat(accessSection.getPermissions()).containsExactly(submitPermission);
+ AccessSection builtAccessSection = accessSection.build();
+ assertThat(builtAccessSection.getPermissions()).hasSize(2);
+ assertThat(builtAccessSection.getPermission(abandonPermission.getName())).isNotNull();
+ assertThat(builtAccessSection.getPermission(rebasePermission.getName())).isNotNull();
+
+ Permission.Builder submitPermission = Permission.builder(Permission.SUBMIT);
+ accessSection.modifyPermissions(
+ p -> {
+ p.clear();
+ p.add(submitPermission);
+ });
+ builtAccessSection = accessSection.build();
+ assertThat(builtAccessSection.getPermissions()).hasSize(1);
+ assertThat(builtAccessSection.getPermission(submitPermission.getName())).isNotNull();
assertThrows(NullPointerException.class, () -> accessSection.setPermissions(null));
}
@@ -65,122 +76,117 @@
assertThrows(
IllegalArgumentException.class,
() ->
- accessSection.setPermissions(
- ImmutableList.of(
- new Permission(Permission.ABANDON), new Permission(Permission.ABANDON))));
+ accessSection
+ .addPermission(Permission.builder(Permission.ABANDON))
+ .addPermission(Permission.builder(Permission.ABANDON))
+ .build());
}
@Test
public void cannotSetPermissionsWithConflictingNames() {
- Permission abandonPermissionLowerCase =
- new Permission(Permission.ABANDON.toLowerCase(Locale.US));
- Permission abandonPermissionUpperCase =
- new Permission(Permission.ABANDON.toUpperCase(Locale.US));
+ Permission.Builder abandonPermissionLowerCase =
+ Permission.builder(Permission.ABANDON.toLowerCase(Locale.US));
+ Permission.Builder abandonPermissionUpperCase =
+ Permission.builder(Permission.ABANDON.toUpperCase(Locale.US));
assertThrows(
IllegalArgumentException.class,
() ->
- accessSection.setPermissions(
- ImmutableList.of(abandonPermissionLowerCase, abandonPermissionUpperCase)));
+ accessSection
+ .addPermission(abandonPermissionLowerCase)
+ .addPermission(abandonPermissionUpperCase)
+ .build());
}
@Test
public void getNonExistingPermission() {
- assertThat(accessSection.getPermission("non-existing")).isNull();
- assertThat(accessSection.getPermission("non-existing", false)).isNull();
+ assertThat(accessSection.build().getPermission("non-existing")).isNull();
+ assertThat(accessSection.build().getPermission("non-existing")).isNull();
}
@Test
public void getPermission() {
- Permission submitPermission = new Permission(Permission.SUBMIT);
- accessSection.setPermissions(ImmutableList.of(submitPermission));
- assertThat(accessSection.getPermission(Permission.SUBMIT)).isEqualTo(submitPermission);
- assertThrows(NullPointerException.class, () -> accessSection.getPermission(null));
+ Permission.Builder submitPermission = Permission.builder(Permission.SUBMIT);
+ accessSection.addPermission(submitPermission);
+ assertThat(accessSection.upsertPermission(Permission.SUBMIT)).isEqualTo(submitPermission);
+ assertThrows(NullPointerException.class, () -> accessSection.upsertPermission(null));
}
@Test
public void getPermissionWithOtherCase() {
- Permission submitPermissionLowerCase = new Permission(Permission.SUBMIT.toLowerCase(Locale.US));
- accessSection.setPermissions(ImmutableList.of(submitPermissionLowerCase));
- assertThat(accessSection.getPermission(Permission.SUBMIT.toUpperCase(Locale.US)))
+ Permission.Builder submitPermissionLowerCase =
+ Permission.builder(Permission.SUBMIT.toLowerCase(Locale.US));
+ accessSection.addPermission(submitPermissionLowerCase);
+ assertThat(accessSection.upsertPermission(Permission.SUBMIT.toUpperCase(Locale.US)))
.isEqualTo(submitPermissionLowerCase);
}
@Test
public void createMissingPermissionOnGet() {
- assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+ assertThat(accessSection.build().getPermission(Permission.SUBMIT)).isNull();
- assertThat(accessSection.getPermission(Permission.SUBMIT, true))
- .isEqualTo(new Permission(Permission.SUBMIT));
+ assertThat(accessSection.upsertPermission(Permission.SUBMIT).build())
+ .isEqualTo(Permission.create(Permission.SUBMIT));
- assertThrows(NullPointerException.class, () -> accessSection.getPermission(null, true));
+ assertThrows(NullPointerException.class, () -> accessSection.upsertPermission(null));
}
@Test
public void addPermission() {
- Permission abandonPermission = new Permission(Permission.ABANDON);
- Permission rebasePermission = new Permission(Permission.REBASE);
+ Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
+ Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
- accessSection.setPermissions(ImmutableList.of(abandonPermission, rebasePermission));
- assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
+ accessSection.addPermission(abandonPermission);
+ accessSection.addPermission(rebasePermission);
+ assertThat(accessSection.build().getPermission(Permission.SUBMIT)).isNull();
- Permission submitPermission = new Permission(Permission.SUBMIT);
+ Permission.Builder submitPermission = Permission.builder(Permission.SUBMIT);
accessSection.addPermission(submitPermission);
- assertThat(accessSection.getPermission(Permission.SUBMIT)).isEqualTo(submitPermission);
- assertThat(accessSection.getPermissions())
- .containsExactly(abandonPermission, rebasePermission, submitPermission)
+ assertThat(accessSection.build().getPermission(Permission.SUBMIT))
+ .isEqualTo(submitPermission.build());
+ assertThat(accessSection.build().getPermissions())
+ .containsExactly(
+ abandonPermission.build(), rebasePermission.build(), submitPermission.build())
.inOrder();
assertThrows(NullPointerException.class, () -> accessSection.addPermission(null));
}
@Test
- public void cannotAddPermissionByModifyingListThatWasProvidedToAccessSection() {
- Permission abandonPermission = new Permission(Permission.ABANDON);
- Permission rebasePermission = new Permission(Permission.REBASE);
-
- List<Permission> permissions = new ArrayList<>();
- permissions.add(abandonPermission);
- permissions.add(rebasePermission);
- accessSection.setPermissions(permissions);
- assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
-
- Permission submitPermission = new Permission(Permission.SUBMIT);
- permissions.add(submitPermission);
- assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
- }
-
- @Test
public void removePermission() {
- Permission abandonPermission = new Permission(Permission.ABANDON);
- Permission rebasePermission = new Permission(Permission.REBASE);
- Permission submitPermission = new Permission(Permission.SUBMIT);
+ Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
+ Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
+ Permission.Builder submitPermission = Permission.builder(Permission.SUBMIT);
- accessSection.setPermissions(
- ImmutableList.of(abandonPermission, rebasePermission, submitPermission));
- assertThat(accessSection.getPermission(Permission.SUBMIT)).isNotNull();
+ accessSection.addPermission(abandonPermission);
+ accessSection.addPermission(rebasePermission);
+ accessSection.addPermission(submitPermission);
+ assertThat(accessSection.build().getPermission(Permission.SUBMIT)).isNotNull();
accessSection.remove(submitPermission);
- assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
- assertThat(accessSection.getPermissions())
- .containsExactly(abandonPermission, rebasePermission)
+ assertThat(accessSection.build().getPermission(Permission.SUBMIT)).isNull();
+ assertThat(accessSection.build().getPermissions())
+ .containsExactly(abandonPermission.build(), rebasePermission.build())
.inOrder();
assertThrows(NullPointerException.class, () -> accessSection.remove(null));
}
@Test
public void removePermissionByName() {
- Permission abandonPermission = new Permission(Permission.ABANDON);
- Permission rebasePermission = new Permission(Permission.REBASE);
- Permission submitPermission = new Permission(Permission.SUBMIT);
+ Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
+ Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
+ Permission.Builder submitPermission = Permission.builder(Permission.SUBMIT);
- accessSection.setPermissions(
- ImmutableList.of(abandonPermission, rebasePermission, submitPermission));
- assertThat(accessSection.getPermission(Permission.SUBMIT)).isNotNull();
+ accessSection.addPermission(abandonPermission);
+ accessSection.addPermission(rebasePermission);
+ accessSection.addPermission(submitPermission);
+ AccessSection builtAccessSection = accessSection.build();
+ assertThat(builtAccessSection.getPermission(Permission.SUBMIT)).isNotNull();
accessSection.removePermission(Permission.SUBMIT);
- assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
- assertThat(accessSection.getPermissions())
- .containsExactly(abandonPermission, rebasePermission)
+ builtAccessSection = accessSection.build();
+ assertThat(builtAccessSection.getPermission(Permission.SUBMIT)).isNull();
+ assertThat(builtAccessSection.getPermissions())
+ .containsExactly(abandonPermission.build(), rebasePermission.build())
.inOrder();
assertThrows(NullPointerException.class, () -> accessSection.removePermission(null));
@@ -188,62 +194,49 @@
@Test
public void removePermissionByNameOtherCase() {
- Permission abandonPermission = new Permission(Permission.ABANDON);
- Permission rebasePermission = new Permission(Permission.REBASE);
+ Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
+ Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
String submitLowerCase = Permission.SUBMIT.toLowerCase(Locale.US);
String submitUpperCase = Permission.SUBMIT.toUpperCase(Locale.US);
- Permission submitPermissionLowerCase = new Permission(submitLowerCase);
+ Permission.Builder submitPermissionLowerCase = Permission.builder(submitLowerCase);
- accessSection.setPermissions(
- ImmutableList.of(abandonPermission, rebasePermission, submitPermissionLowerCase));
- assertThat(accessSection.getPermission(submitLowerCase)).isNotNull();
- assertThat(accessSection.getPermission(submitUpperCase)).isNotNull();
+ accessSection.addPermission(abandonPermission);
+ accessSection.addPermission(rebasePermission);
+ accessSection.addPermission(submitPermissionLowerCase);
+ AccessSection builtAccessSection = accessSection.build();
+ assertThat(builtAccessSection.getPermission(submitLowerCase)).isNotNull();
+ assertThat(builtAccessSection.getPermission(submitUpperCase)).isNotNull();
accessSection.removePermission(submitUpperCase);
- assertThat(accessSection.getPermission(submitLowerCase)).isNull();
- assertThat(accessSection.getPermission(submitUpperCase)).isNull();
- assertThat(accessSection.getPermissions())
- .containsExactly(abandonPermission, rebasePermission)
+ builtAccessSection = accessSection.build();
+ assertThat(builtAccessSection.getPermission(submitLowerCase)).isNull();
+ assertThat(builtAccessSection.getPermission(submitUpperCase)).isNull();
+ assertThat(builtAccessSection.getPermissions())
+ .containsExactly(abandonPermission.build(), rebasePermission.build())
.inOrder();
}
@Test
- public void mergeAccessSections() {
- Permission abandonPermission = new Permission(Permission.ABANDON);
- Permission rebasePermission = new Permission(Permission.REBASE);
- Permission submitPermission = new Permission(Permission.SUBMIT);
-
- AccessSection accessSection1 = new AccessSection("refs/heads/foo");
- accessSection1.setPermissions(ImmutableList.of(abandonPermission, rebasePermission));
-
- AccessSection accessSection2 = new AccessSection("refs/heads/bar");
- accessSection2.setPermissions(ImmutableList.of(rebasePermission, submitPermission));
-
- accessSection1.mergeFrom(accessSection2);
- assertThat(accessSection1.getPermissions())
- .containsExactly(abandonPermission, rebasePermission, submitPermission)
- .inOrder();
- assertThrows(NullPointerException.class, () -> accessSection.mergeFrom(null));
- }
-
- @Test
public void testEquals() {
- Permission abandonPermission = new Permission(Permission.ABANDON);
- Permission rebasePermission = new Permission(Permission.REBASE);
+ Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
+ Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
- accessSection.setPermissions(ImmutableList.of(abandonPermission, rebasePermission));
+ accessSection.addPermission(abandonPermission);
+ accessSection.addPermission(rebasePermission);
- AccessSection accessSectionSamePermissionsOtherRef = new AccessSection("refs/heads/other");
- accessSectionSamePermissionsOtherRef.setPermissions(
- ImmutableList.of(abandonPermission, rebasePermission));
- assertThat(accessSection.equals(accessSectionSamePermissionsOtherRef)).isFalse();
+ AccessSection builtAccessSection = accessSection.build();
+ AccessSection.Builder accessSectionSamePermissionsOtherRef =
+ AccessSection.builder("refs/heads/other");
+ accessSectionSamePermissionsOtherRef.addPermission(abandonPermission);
+ accessSectionSamePermissionsOtherRef.addPermission(rebasePermission);
+ assertThat(builtAccessSection.equals(accessSectionSamePermissionsOtherRef.build())).isFalse();
- AccessSection accessSectionOther = new AccessSection(REF_PATTERN);
- accessSectionOther.setPermissions(ImmutableList.of(abandonPermission));
- assertThat(accessSection.equals(accessSectionOther)).isFalse();
+ AccessSection.Builder accessSectionOther = AccessSection.builder(REF_PATTERN);
+ accessSectionOther.addPermission(abandonPermission);
+ assertThat(builtAccessSection.equals(accessSectionOther.build())).isFalse();
accessSectionOther.addPermission(rebasePermission);
- assertThat(accessSection.equals(accessSectionOther)).isTrue();
+ assertThat(builtAccessSection.equals(accessSectionOther.build())).isTrue();
}
}
diff --git a/javatests/com/google/gerrit/common/data/PermissionTest.java b/javatests/com/google/gerrit/common/data/PermissionTest.java
index a363129..9dd71ca 100644
--- a/javatests/com/google/gerrit/common/data/PermissionTest.java
+++ b/javatests/com/google/gerrit/common/data/PermissionTest.java
@@ -16,21 +16,18 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.AccountGroup;
-import java.util.ArrayList;
-import java.util.List;
import org.junit.Before;
import org.junit.Test;
public class PermissionTest {
private static final String PERMISSION_NAME = "foo";
- private Permission permission;
+ private Permission.Builder permission;
@Before
public void setup() {
- this.permission = new Permission(PERMISSION_NAME);
+ this.permission = Permission.builder(PERMISSION_NAME);
}
@Test
@@ -117,209 +114,179 @@
@Test
public void getLabel() {
- assertThat(new Permission(Permission.LABEL + "Code-Review").getLabel())
+ assertThat(Permission.create(Permission.LABEL + "Code-Review").getLabel())
.isEqualTo("Code-Review");
- assertThat(new Permission(Permission.LABEL_AS + "Code-Review").getLabel())
+ assertThat(Permission.create(Permission.LABEL_AS + "Code-Review").getLabel())
.isEqualTo("Code-Review");
- assertThat(new Permission("Code-Review").getLabel()).isNull();
- assertThat(new Permission(Permission.ABANDON).getLabel()).isNull();
+ assertThat(Permission.create("Code-Review").getLabel()).isNull();
+ assertThat(Permission.create(Permission.ABANDON).getLabel()).isNull();
}
@Test
public void exclusiveGroup() {
- assertThat(permission.getExclusiveGroup()).isFalse();
+ assertThat(permission.build().getExclusiveGroup()).isFalse();
permission.setExclusiveGroup(true);
- assertThat(permission.getExclusiveGroup()).isTrue();
+ assertThat(permission.build().getExclusiveGroup()).isTrue();
permission.setExclusiveGroup(false);
- assertThat(permission.getExclusiveGroup()).isFalse();
+ assertThat(permission.build().getExclusiveGroup()).isFalse();
}
@Test
public void noExclusiveGroupOnOwnerPermission() {
- Permission permission = new Permission(Permission.OWNER);
+ Permission permission = Permission.create(Permission.OWNER);
assertThat(permission.getExclusiveGroup()).isFalse();
- permission.setExclusiveGroup(true);
+ permission = permission.toBuilder().setExclusiveGroup(true).build();
assertThat(permission.getExclusiveGroup()).isFalse();
}
@Test
public void getEmptyRules() {
- assertThat(permission.getRules()).isNotNull();
- assertThat(permission.getRules()).isEmpty();
+ assertThat(permission.getRulesBuilders()).isNotNull();
+ assertThat(permission.getRulesBuilders()).isEmpty();
}
@Test
public void setAndGetRules() {
- PermissionRule permissionRule1 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
- PermissionRule permissionRule2 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
- permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
- assertThat(permission.getRules()).containsExactly(permissionRule1, permissionRule2).inOrder();
+ PermissionRule.Builder permissionRule1 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
+ PermissionRule.Builder permissionRule2 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
+ permission.add(permissionRule1);
+ permission.add(permissionRule2);
+ assertThat(permission.getRulesBuilders())
+ .containsExactly(permissionRule1, permissionRule2)
+ .inOrder();
- PermissionRule permissionRule3 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"));
- permission.setRules(ImmutableList.of(permissionRule3));
- assertThat(permission.getRules()).containsExactly(permissionRule3);
- }
-
- @Test
- public void cannotAddPermissionByModifyingListThatWasProvidedToAccessSection() {
- PermissionRule permissionRule1 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
- PermissionRule permissionRule2 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
- GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3");
-
- List<PermissionRule> rules = new ArrayList<>();
- rules.add(permissionRule1);
- rules.add(permissionRule2);
- permission.setRules(rules);
- assertThat(permission.getRule(groupReference3)).isNull();
-
- PermissionRule permissionRule3 = PermissionRule.create(groupReference3);
- rules.add(permissionRule3);
- assertThat(permission.getRule(groupReference3)).isNull();
+ PermissionRule.Builder permissionRule3 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"));
+ permission.modifyRules(
+ rules -> {
+ rules.clear();
+ rules.add(permissionRule3);
+ });
+ assertThat(permission.getRulesBuilders()).containsExactly(permissionRule3);
}
@Test
public void getNonExistingRule() {
GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1");
- assertThat(permission.getRule(groupReference)).isNull();
- assertThat(permission.getRule(groupReference, false)).isNull();
+ assertThat(permission.build().getRule(groupReference)).isNull();
+ assertThat(permission.build().getRule(groupReference)).isNull();
}
@Test
public void getRule() {
GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1");
- PermissionRule permissionRule = PermissionRule.create(groupReference);
- permission.setRules(ImmutableList.of(permissionRule));
- assertThat(permission.getRule(groupReference)).isEqualTo(permissionRule);
- }
-
- @Test
- public void createMissingRuleOnGet() {
- GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1");
- assertThat(permission.getRule(groupReference)).isNull();
-
- assertThat(permission.getRule(groupReference, true))
- .isEqualTo(PermissionRule.create(groupReference));
+ PermissionRule.Builder permissionRule = PermissionRule.builder(groupReference);
+ permission.add(permissionRule);
+ assertThat(permission.build().getRule(groupReference)).isEqualTo(permissionRule.build());
}
@Test
public void addRule() {
- PermissionRule permissionRule1 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
- PermissionRule permissionRule2 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
- permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
+ PermissionRule.Builder permissionRule1 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
+ PermissionRule.Builder permissionRule2 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
+ permission.add(permissionRule1);
+ permission.add(permissionRule2);
GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3");
- assertThat(permission.getRule(groupReference3)).isNull();
+ assertThat(permission.build().getRule(groupReference3)).isNull();
- PermissionRule permissionRule3 = PermissionRule.create(groupReference3);
+ PermissionRule.Builder permissionRule3 = PermissionRule.builder(groupReference3);
permission.add(permissionRule3);
- assertThat(permission.getRule(groupReference3)).isEqualTo(permissionRule3);
- assertThat(permission.getRules())
- .containsExactly(permissionRule1, permissionRule2, permissionRule3)
+ assertThat(permission.build().getRule(groupReference3)).isEqualTo(permissionRule3.build());
+ assertThat(permission.build().getRules())
+ .containsExactly(permissionRule1.build(), permissionRule2.build(), permissionRule3.build())
.inOrder();
}
@Test
public void removeRule() {
- PermissionRule permissionRule1 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
- PermissionRule permissionRule2 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
+ PermissionRule.Builder permissionRule1 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
+ PermissionRule.Builder permissionRule2 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3");
- PermissionRule permissionRule3 = PermissionRule.create(groupReference3);
+ PermissionRule.Builder permissionRule3 = PermissionRule.builder(groupReference3);
- permission.setRules(ImmutableList.of(permissionRule1, permissionRule2, permissionRule3));
- assertThat(permission.getRule(groupReference3)).isNotNull();
+ permission.add(permissionRule1);
+ permission.add(permissionRule2);
+ permission.add(permissionRule3);
+ assertThat(permission.build().getRule(groupReference3)).isNotNull();
- permission.remove(permissionRule3);
- assertThat(permission.getRule(groupReference3)).isNull();
- assertThat(permission.getRules()).containsExactly(permissionRule1, permissionRule2).inOrder();
- }
-
- @Test
- public void removeRuleByGroupReference() {
- PermissionRule permissionRule1 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
- PermissionRule permissionRule2 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
- GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3");
- PermissionRule permissionRule3 = PermissionRule.create(groupReference3);
-
- permission.setRules(ImmutableList.of(permissionRule1, permissionRule2, permissionRule3));
- assertThat(permission.getRule(groupReference3)).isNotNull();
-
- permission.removeRule(groupReference3);
- assertThat(permission.getRule(groupReference3)).isNull();
- assertThat(permission.getRules()).containsExactly(permissionRule1, permissionRule2).inOrder();
- }
-
- @Test
- public void clearRules() {
- PermissionRule permissionRule1 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
- PermissionRule permissionRule2 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
-
- permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
- assertThat(permission.getRules()).isNotEmpty();
-
- permission.clearRules();
- assertThat(permission.getRules()).isEmpty();
- }
-
- @Test
- public void mergePermissions() {
- PermissionRule permissionRule1 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
- PermissionRule permissionRule2 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
- PermissionRule permissionRule3 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"));
-
- Permission permission1 = new Permission("foo");
- permission1.setRules(ImmutableList.of(permissionRule1, permissionRule2));
-
- Permission permission2 = new Permission("bar");
- permission2.setRules(ImmutableList.of(permissionRule2, permissionRule3));
-
- permission1.mergeFrom(permission2);
- assertThat(permission1.getRules())
- .containsExactly(permissionRule1, permissionRule2, permissionRule3)
+ permission.remove(permissionRule3.build());
+ assertThat(permission.build().getRule(groupReference3)).isNull();
+ assertThat(permission.build().getRules())
+ .containsExactly(permissionRule1.build(), permissionRule2.build())
.inOrder();
}
@Test
+ public void removeRuleByGroupReference() {
+ PermissionRule.Builder permissionRule1 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
+ PermissionRule.Builder permissionRule2 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
+ GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3");
+ PermissionRule.Builder permissionRule3 = PermissionRule.builder(groupReference3);
+
+ permission.add(permissionRule1);
+ permission.add(permissionRule2);
+ permission.add(permissionRule3);
+ assertThat(permission.build().getRule(groupReference3)).isNotNull();
+
+ permission.removeRule(groupReference3);
+ assertThat(permission.build().getRule(groupReference3)).isNull();
+ assertThat(permission.build().getRules())
+ .containsExactly(permissionRule1.build(), permissionRule2.build())
+ .inOrder();
+ }
+
+ @Test
+ public void clearRules() {
+ PermissionRule.Builder permissionRule1 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
+ PermissionRule.Builder permissionRule2 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
+
+ permission.add(permissionRule1);
+ permission.add(permissionRule2);
+ assertThat(permission.build().getRules()).isNotEmpty();
+
+ permission.clearRules();
+ assertThat(permission.build().getRules()).isEmpty();
+ }
+
+ @Test
public void testEquals() {
- PermissionRule permissionRule1 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
- PermissionRule permissionRule2 =
- PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
+ PermissionRule.Builder permissionRule1 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
+ PermissionRule.Builder permissionRule2 =
+ PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
- permission.setRules(ImmutableList.of(permissionRule1, permissionRule2));
+ permission.add(permissionRule1);
+ permission.add(permissionRule2);
- Permission permissionSameRulesOtherName = new Permission("bar");
- permissionSameRulesOtherName.setRules(ImmutableList.of(permissionRule1, permissionRule2));
+ Permission.Builder permissionSameRulesOtherName = Permission.builder("bar");
+ permissionSameRulesOtherName.add(permissionRule1);
+ permissionSameRulesOtherName.add(permissionRule2);
assertThat(permission.equals(permissionSameRulesOtherName)).isFalse();
- Permission permissionSameRulesSameNameOtherExclusiveGroup = new Permission("foo");
- permissionSameRulesSameNameOtherExclusiveGroup.setRules(
- ImmutableList.of(permissionRule1, permissionRule2));
+ Permission.Builder permissionSameRulesSameNameOtherExclusiveGroup = Permission.builder("foo");
+ permissionSameRulesSameNameOtherExclusiveGroup.add(permissionRule1);
+ permissionSameRulesSameNameOtherExclusiveGroup.add(permissionRule2);
permissionSameRulesSameNameOtherExclusiveGroup.setExclusiveGroup(true);
assertThat(permission.equals(permissionSameRulesSameNameOtherExclusiveGroup)).isFalse();
- Permission permissionOther = new Permission(PERMISSION_NAME);
- permissionOther.setRules(ImmutableList.of(permissionRule1));
- assertThat(permission.equals(permissionOther)).isFalse();
+ Permission.Builder permissionOther = Permission.builder(PERMISSION_NAME);
+ permissionOther.add(permissionRule1);
+ assertThat(permission.build().equals(permissionOther.build())).isFalse();
permissionOther.add(permissionRule2);
- assertThat(permission.equals(permissionOther)).isTrue();
+ assertThat(permission.build().equals(permissionOther.build())).isTrue();
}
}
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD b/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
new file mode 100644
index 0000000..c781d86
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/BUILD
@@ -0,0 +1,25 @@
+load("//tools/bzl:junit.bzl", "junit_tests")
+
+junit_tests(
+ name = "tests",
+ srcs = glob(["*.java"]),
+ deps = [
+ "//java/com/google/gerrit/entities",
+ "//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/cache/serialize",
+ "//java/com/google/gerrit/server/cache/serialize/entities",
+ "//java/com/google/gerrit/server/cache/testing",
+ "//java/com/google/gerrit/testing:gerrit-test-util",
+ "//lib:guava",
+ "//lib:jgit",
+ "//lib:junit",
+ "//lib:protobuf",
+ "//lib/auto:auto-value",
+ "//lib/auto:auto-value-annotations",
+ "//lib/truth",
+ "//lib/truth:truth-proto-extension",
+ "//proto:cache_java_proto",
+ "//proto/testing:test_java_proto",
+ ],
+)
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/ProjectSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/ProjectSerializerTest.java
new file mode 100644
index 0000000..8d13247
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/ProjectSerializerTest.java
@@ -0,0 +1,61 @@
+// 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.cache.serialize.entities;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.cache.serialize.entities.ProjectSerializer.deserialize;
+import static com.google.gerrit.server.cache.serialize.entities.ProjectSerializer.serialize;
+
+import com.google.gerrit.entities.BooleanProjectConfig;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.client.InheritableBoolean;
+import com.google.gerrit.extensions.client.ProjectState;
+import com.google.gerrit.extensions.client.SubmitType;
+import org.junit.Test;
+
+public class ProjectSerializerTest {
+ @Test
+ public void roundTrip() {
+ Project projectAutoValue =
+ Project.builder(Project.nameKey("test"))
+ .setDescription("desc")
+ .setSubmitType(SubmitType.FAST_FORWARD_ONLY)
+ .setState(ProjectState.HIDDEN)
+ .setParent(Project.nameKey("parent"))
+ .setMaxObjectSizeLimit("11K")
+ .setDefaultDashboard("dashboard1")
+ .setLocalDefaultDashboard("dashboard2")
+ .setConfigRefState("1337")
+ .setBooleanConfig(
+ BooleanProjectConfig.ENABLE_REVIEWER_BY_EMAIL, InheritableBoolean.TRUE)
+ .setBooleanConfig(
+ BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
+ InheritableBoolean.INHERIT)
+ .build();
+
+ assertThat(deserialize(serialize(projectAutoValue))).isEqualTo(projectAutoValue);
+ }
+
+ @Test
+ public void roundTripWithMinimalValues() {
+ Project projectAutoValue =
+ Project.builder(Project.nameKey("test"))
+ .setSubmitType(SubmitType.FAST_FORWARD_ONLY)
+ .setState(ProjectState.HIDDEN)
+ .build();
+
+ assertThat(deserialize(serialize(projectAutoValue))).isEqualTo(projectAutoValue);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java b/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
index ba8485b..ea210ab 100644
--- a/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
+++ b/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
@@ -98,10 +98,15 @@
private void configureProject() throws Exception {
ProjectConfig pc = loadAllProjects();
- for (AccessSection sec : pc.getAccessSections()) {
- for (String label : pc.getLabelSections().keySet()) {
- sec.removePermission(forLabel(label));
- }
+
+ for (AccessSection sec : ImmutableList.copyOf(pc.getAccessSections())) {
+ pc.upsertAccessSection(
+ sec.getName(),
+ updatedSection -> {
+ for (String label : pc.getLabelSections().keySet()) {
+ updatedSection.removePermission(forLabel(label));
+ }
+ });
}
LabelType lt =
label("Verified", value(1, "Verified"), value(0, "No score"), value(-1, "Fails"));
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/javatests/com/google/gerrit/server/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
index b7125c3..2d31acf 100644
--- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
+++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
@@ -303,12 +303,15 @@
update(rev);
ProjectConfig cfg = read(rev);
- AccessSection section = cfg.getAccessSection("refs/heads/*");
- cfg.getAccountsSection()
- .setSameGroupVisibility(
- Collections.singletonList(PermissionRule.create(cfg.resolve(staff))));
- Permission submit = section.getPermission(Permission.SUBMIT);
- submit.add(PermissionRule.create(cfg.resolve(staff)));
+ cfg.upsertAccessSection(
+ "refs/heads/*",
+ section -> {
+ Permission.Builder submit = section.upsertPermission(Permission.SUBMIT);
+ submit.add(PermissionRule.builder(cfg.resolve(staff)));
+ });
+ cfg.setAccountsSection(
+ AccountsSection.create(
+ Collections.singletonList(PermissionRule.create(cfg.resolve(staff)))));
ContributorAgreement.Builder ca = cfg.getContributorAgreement("Individual").toBuilder();
ca.setAccepted(ImmutableList.of(PermissionRule.create(cfg.resolve(staff))));
ca.setAutoVerify(null);
@@ -423,9 +426,12 @@
update(rev);
ProjectConfig cfg = read(rev);
- AccessSection section = cfg.getAccessSection("refs/heads/*");
- Permission submit = section.getPermission(Permission.SUBMIT);
- submit.add(PermissionRule.create(cfg.resolve(staff)));
+ cfg.upsertAccessSection(
+ "refs/heads/*",
+ section -> {
+ Permission.Builder submit = section.upsertPermission(Permission.SUBMIT);
+ submit.add(PermissionRule.builder(cfg.resolve(staff)));
+ });
rev = commit(cfg);
assertThat(text(rev, "project.config"))
.isEqualTo(
@@ -692,7 +698,7 @@
update(rev);
ProjectConfig cfg = read(rev);
- cfg.getAccountsSection().setSameGroupVisibility(ImmutableList.of());
+ cfg.setAccountsSection(AccountsSection.create(ImmutableList.of()));
rev = commit(cfg);
assertThat(text(rev, "project.config"))
.isEqualTo(
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 968d4f7..2eb25da 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -44,7 +44,6 @@
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.Nullable;
-import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
@@ -1859,13 +1858,16 @@
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
md.setMessage(String.format("Grant %s on %s", permission, ref));
ProjectConfig config = projectConfigFactory.read(md);
- AccessSection s = config.getAccessSection(ref, true);
- Permission p = s.getPermission(permission, true);
- PermissionRule rule =
- PermissionRule.builder(GroupReference.create(groupUUID, groupUUID.get()))
- .setForce(force)
- .build();
- p.add(rule);
+ config.upsertAccessSection(
+ ref,
+ s -> {
+ Permission.Builder p = s.upsertPermission(permission);
+ PermissionRule.Builder rule =
+ PermissionRule.builder(GroupReference.create(groupUUID, groupUUID.get()))
+ .setForce(force);
+ p.add(rule);
+ });
+
config.commit(md);
projectCache.evict(config.getProject());
}
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
diff --git a/proto/cache.proto b/proto/cache.proto
index e157608..4d24036 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -323,3 +323,18 @@
repeated ProjectWatchProto project_watch_proto = 2;
string user_preferences = 3;
}
+
+// Serialized form of com.google.gerrit.entities.Project.
+// Next ID: 11
+message ProjectProto {
+ string name = 1;
+ string description = 2;
+ map<string, string> boolean_configs = 3;
+ string submit_type = 4; // ENUM as String
+ string state = 5; // ENUM as String
+ string parent = 6;
+ string max_object_size_limit = 7;
+ string default_dashboard = 8;
+ string local_default_dashboard = 9;
+ string config_ref_state = 10;
+}
