Make AccessSection, Permission an AutoValue
Change-Id: If0a679bb1d461c5d6534dc030ff6d25a55ebb2dd
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/server/CreateGroupPermissionSyncer.java b/java/com/google/gerrit/server/CreateGroupPermissionSyncer.java
index 17313e4..7fbf7ef 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;
@@ -101,23 +99,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/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/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index cc13606..5ff1d83 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -350,16 +350,16 @@
}
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 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 ImmutableSet<String> getAccessSectionNames() {
@@ -400,8 +400,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 +413,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 +434,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);
}
@@ -790,38 +785,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 +872,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 +882,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 +914,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..986019c 100644
--- a/java/com/google/gerrit/server/project/ProjectState.java
+++ b/java/com/google/gerrit/server/project/ProjectState.java
@@ -280,14 +280,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);
diff --git a/java/com/google/gerrit/server/restapi/project/GetAccess.java b/java/com/google/gerrit/server/restapi/project/GetAccess.java
index 5a3dbcd..2416780 100644
--- a/java/com/google/gerrit/server/restapi/project/GetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/GetAccess.java
@@ -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/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..43cc1f4 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -1653,8 +1653,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/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/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/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/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
index b7125c3..3686e4e 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.upsertAccessSection(
+ "refs/heads/*",
+ section -> {
+ Permission.Builder submit = section.upsertPermission(Permission.SUBMIT);
+ submit.add(PermissionRule.builder(cfg.resolve(staff)));
+ });
cfg.getAccountsSection()
.setSameGroupVisibility(
Collections.singletonList(PermissionRule.create(cfg.resolve(staff))));
- Permission submit = section.getPermission(Permission.SUBMIT);
- submit.add(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(
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());
}
