opendev/gerrit
Code Issues Proposed changes

Make AccessSection, Permission an AutoValue

Browse Source 
Change-Id: If0a679bb1d461c5d6534dc030ff6d25a55ebb2dd
This commit is contained in:
Patrick Hiesel
2020-06-29 14:26:43 +02:00
parent d2cececdf5
commit a8e8c47de8
26 changed files with 802 additions and 765 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java
View File

@@ -151,15 +151,19 @@ public class ProjectOperationsImpl implements ProjectOperations {
ProjectConfig projectConfig, ProjectConfig projectConfig,
ImmutableList<TestProjectUpdate.TestPermissionKey> removedPermissions) { ImmutableList<TestProjectUpdate.TestPermissionKey> removedPermissions) {
for (TestProjectUpdate.TestPermissionKey p : removedPermissions) { for (TestProjectUpdate.TestPermissionKey p : removedPermissions) {
Permission permission = projectConfig.upsertAccessSection(
projectConfig.getAccessSection(p.section(), true).getPermission(p.name(), true); p.section(),
if (p.group().isPresent()) { as -> {
GroupReference group = GroupReference.create(p.group().get(), p.group().get().get()); Permission.Builder permission = as.upsertPermission(p.name());
group = projectConfig.resolve(group); if (p.group().isPresent()) {
permission.removeRule(group); GroupReference group =
} else { GroupReference.create(p.group().get(), p.group().get().get());
permission.clearRules(); group = projectConfig.resolve(group);
} permission.removeRule(group);
} else {
permission.clearRules();
}
});
} }
} }
@@ -168,10 +172,8 @@ public class ProjectOperationsImpl implements ProjectOperations {
for (TestCapability c : addedCapabilities) { for (TestCapability c : addedCapabilities) {
PermissionRule.Builder rule = newRule(projectConfig, c.group()); PermissionRule.Builder rule = newRule(projectConfig, c.group());
rule.setRange(c.min(), c.max()); rule.setRange(c.min(), c.max());
projectConfig projectConfig.upsertAccessSection(
.getAccessSection(AccessSection.GLOBAL_CAPABILITIES, true) AccessSection.GLOBAL_CAPABILITIES, as -> as.upsertPermission(c.name()).add(rule));
.getPermission(c.name(), true)
.add(rule.build());
} }
} }
@@ -181,10 +183,7 @@ public class ProjectOperationsImpl implements ProjectOperations {
PermissionRule.Builder rule = newRule(projectConfig, p.group()); PermissionRule.Builder rule = newRule(projectConfig, p.group());
rule.setAction(p.action()); rule.setAction(p.action());
rule.setForce(p.force()); rule.setForce(p.force());
projectConfig projectConfig.upsertAccessSection(p.ref(), as -> as.upsertPermission(p.name()).add(rule));
.getAccessSection(p.ref(), true)
.getPermission(p.name(), true)
.add(rule.build());
} }
} }
@@ -196,9 +195,8 @@ public class ProjectOperationsImpl implements ProjectOperations {
rule.setRange(p.min(), p.max()); rule.setRange(p.min(), p.max());
String permissionName = String permissionName =
p.impersonation() ? Permission.forLabelAs(p.name()) : Permission.forLabel(p.name()); p.impersonation() ? Permission.forLabelAs(p.name()) : Permission.forLabel(p.name());
Permission permission = projectConfig.upsertAccessSection(
projectConfig.getAccessSection(p.ref(), true).getPermission(permissionName, true); p.ref(), as -> as.upsertPermission(permissionName).add(rule));
permission.add(rule.build());
} }
} }
@@ -207,10 +205,9 @@ public class ProjectOperationsImpl implements ProjectOperations {
ImmutableMap<TestProjectUpdate.TestPermissionKey, Boolean> exclusiveGroupPermissions) { ImmutableMap<TestProjectUpdate.TestPermissionKey, Boolean> exclusiveGroupPermissions) {
exclusiveGroupPermissions.forEach( exclusiveGroupPermissions.forEach(
(key, exclusive) -> (key, exclusive) ->
projectConfig projectConfig.upsertAccessSection(
.getAccessSection(key.section(), true) key.section(),
.getPermission(key.name(), true) as -> as.upsertPermission(key.name()).setExclusiveGroup(exclusive)));
.setExclusiveGroup(exclusive));
} }
private RevCommit headOrNull(String branch) { private RevCommit headOrNull(String branch) {

207
java/com/google/gerrit/common/data/AccessSection.java
View File

@@ -14,18 +14,21 @@
package com.google.gerrit.common.data; package com.google.gerrit.common.data;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.Objects.requireNonNull; import static java.util.Objects.requireNonNull;
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project; import com.google.gerrit.entities.Project;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashSet;
import java.util.List; 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. */ /** 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. */ /** Special name given to the global capabilities; not a valid reference. */
public static final String GLOBAL_CAPABILITIES = "GLOBAL_CAPABILITIES"; public static final String GLOBAL_CAPABILITIES = "GLOBAL_CAPABILITIES";
/** Pattern that matches all references in a project. */ /** Pattern that matches all references in a project. */
@@ -38,12 +41,16 @@ public final class AccessSection implements Comparable<AccessSection> {
public static final String REGEX_PREFIX = "^"; public static final String REGEX_PREFIX = "^";
/** Name of the access section. It could be a ref pattern or something else. */ /** 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) { public static AccessSection create(String name) {
this.name = 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. */ /** @return true if the name is likely to be a valid reference section name. */
@@ -51,101 +58,19 @@ public final class AccessSection implements Comparable<AccessSection> {
return name.startsWith("refs/") || name.startsWith("^refs/"); 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 @Nullable
public Permission getPermission(String name) { public Permission getPermission(String name) {
return getPermission(name, false);
}
@Nullable
public Permission getPermission(String name, boolean create) {
requireNonNull(name); requireNonNull(name);
for (Permission p : getPermissions()) {
if (permissions != null) { if (p.getName().equalsIgnoreCase(name)) {
for (Permission p : permissions) { return p;
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; 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 @Override
public int compareTo(AccessSection o) { public final int compareTo(AccessSection o) {
return comparePattern().compareTo(o.comparePattern()); return comparePattern().compareTo(o.comparePattern());
} }
@@ -157,33 +82,85 @@ public final class AccessSection implements Comparable<AccessSection> {
} }
@Override @Override
public String toString() { public final String toString() {
return "AccessSection[" + getName() + "]"; return "AccessSection[" + getName() + "]";
} }
@Override public Builder toBuilder() {
public boolean equals(Object obj) { Builder b = autoToBuilder();
if (!(obj instanceof AccessSection)) { b.getPermissions().stream().map(Permission::toBuilder).forEach(p -> b.addPermission(p));
return false; return b;
}
AccessSection other = (AccessSection) obj;
if (!getName().equals(other.getName())) {
return false;
}
return new HashSet<>(getPermissions())
.equals(new HashSet<>(((AccessSection) obj).getPermissions()));
} }
@Override protected abstract Builder autoToBuilder();
public int hashCode() {
int hashCode = super.hashCode(); @AutoValue.Builder
if (permissions != null) { public abstract static class Builder {
for (Permission permission : permissions) { private final List<Permission.Builder> permissionBuilders;
hashCode += permission.hashCode();
} 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);
} }
} }

192
java/com/google/gerrit/common/data/Permission.java
View File

@@ -14,14 +14,19 @@
package com.google.gerrit.common.data; 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.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.function.Consumer;
/** A single permission within an {@link AccessSection} of a project. */ /** 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 ABANDON = "abandon";
public static final String ADD_PATCH_SET = "addPatchSet"; public static final String ADD_PATCH_SET = "addPatchSet";
public static final String CREATE = "create"; public static final String CREATE = "create";
@@ -133,18 +138,21 @@ public class Permission implements Comparable<Permission> {
return true; return true;
} }
protected String name; public abstract String getName();
protected boolean exclusiveGroup;
protected List<PermissionRule> rules;
protected Permission() {} protected abstract boolean isExclusiveGroup();
public Permission(String name) { public abstract ImmutableList<PermissionRule> getRules();
this.name = name;
public static Builder builder(String name) {
return new AutoValue_Permission.Builder()
.setName(name)
.setExclusiveGroup(false)
.setRules(ImmutableList.of());
} }
public String getName() { public static Permission create(String name) {
return name; return builder(name).build();
} }
public String getLabel() { public String getLabel() {
@@ -155,97 +163,32 @@ public class Permission implements Comparable<Permission> {
// Only permit exclusive group behavior on non OWNER permissions, // Only permit exclusive group behavior on non OWNER permissions,
// otherwise an owner might lose access to a delegated subspace. // 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) { public PermissionRule getRule(GroupReference group) {
return getRule(group, false); for (PermissionRule r : getRules()) {
}
public PermissionRule getRule(GroupReference group, boolean create) {
initRules();
for (PermissionRule r : rules) {
if (sameGroup(r, group)) { if (sameGroup(r, group)) {
return r; return r;
} }
} }
if (create) {
PermissionRule r = PermissionRule.create(group);
rules.add(r);
return r;
}
return null; 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) { 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()); return group.getUUID().equals(rule.getGroup().getUUID());
} else if (group.getName() != null && rule.getGroup().getName() != null) {
} else if (group.getName() != null) {
return group.getName().equals(rule.getGroup().getName()); return group.getName().equals(rule.getGroup().getName());
} else { } else {
return false; return false;
} }
} }
private void initRules() {
if (rules == null) {
rules = new ArrayList<>(4);
}
}
@Override @Override
public int compareTo(Permission b) { public final int compareTo(Permission b) {
int cmp = index(this) - index(b); int cmp = index(this) - index(b);
if (cmp == 0) { if (cmp == 0) {
cmp = getName().compareTo(b.getName()); cmp = getName().compareTo(b.getName());
@@ -265,28 +208,10 @@ public class Permission implements Comparable<Permission> {
} }
@Override @Override
public boolean equals(Object obj) { public final String toString() {
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() {
StringBuilder bldr = new StringBuilder(); StringBuilder bldr = new StringBuilder();
bldr.append(name).append(" "); bldr.append(getName()).append(" ");
if (exclusiveGroup) { if (isExclusiveGroup()) {
bldr.append("[exclusive] "); bldr.append("[exclusive] ");
} }
bldr.append("["); bldr.append("[");
@@ -300,4 +225,67 @@ public class Permission implements Comparable<Permission> {
bldr.append("]"); bldr.append("]");
return bldr.toString(); 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();
}
} }

2
java/com/google/gerrit/common/data/PermissionRule.java
View File

@@ -263,6 +263,8 @@ public abstract class PermissionRule implements Comparable<PermissionRule> {
public abstract Builder setMax(int max); public abstract Builder setMax(int max);
public abstract GroupReference getGroup();
public abstract PermissionRule build(); public abstract PermissionRule build();
} }
} }

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

@@ -14,8 +14,6 @@
package com.google.gerrit.server; package com.google.gerrit.server;
import static java.util.stream.Collectors.toList;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger; import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.AccessSection;
@@ -101,23 +99,25 @@ public class CreateGroupPermissionSyncer implements ChangeMergedListener {
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsers)) { try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsers)) {
ProjectConfig config = projectConfigFactory.read(md); ProjectConfig config = projectConfigFactory.read(md);
AccessSection createGroupAccessSection = config.upsertAccessSection(
config.getAccessSection(RefNames.REFS_GROUPS + "*", true); RefNames.REFS_GROUPS + "*",
if (createGroupsGlobal.isEmpty()) { refsGroupsAccessSectionBuilder -> {
createGroupAccessSection.setPermissions( if (createGroupsGlobal.isEmpty()) {
createGroupAccessSection.getPermissions().stream() refsGroupsAccessSectionBuilder.modifyPermissions(
.filter(p -> !Permission.CREATE.equals(p.getName())) permissions -> {
.collect(toList())); permissions.removeIf(p -> Permission.CREATE.equals(p.getName()));
config.replace(createGroupAccessSection); });
} else { } else {
// The create permission is managed by Gerrit at this point only so there is no concern of // The create permission is managed by Gerrit at this point only so there is no
// overwriting user-defined permissions here. // concern of overwriting user-defined permissions here.
Permission createGroupPermission = new Permission(Permission.CREATE); Permission.Builder createGroupPermission = Permission.builder(Permission.CREATE);
createGroupAccessSection.remove(createGroupPermission); refsGroupsAccessSectionBuilder.remove(createGroupPermission);
createGroupAccessSection.addPermission(createGroupPermission); refsGroupsAccessSectionBuilder.addPermission(createGroupPermission);
createGroupsGlobal.forEach(createGroupPermission::add); createGroupsGlobal.stream()
config.replace(createGroupAccessSection); .map(p -> p.toBuilder())
} .forEach(createGroupPermission::add);
}
});
config.commit(md); config.commit(md);
projectCache.evict(config.getProject()); projectCache.evict(config.getProject());

2
java/com/google/gerrit/server/account/CapabilityCollection.java
View File

@@ -60,7 +60,7 @@ public class CapabilityCollection {
this.systemGroupBackend = systemGroupBackend; this.systemGroupBackend = systemGroupBackend;
if (section == null) { if (section == null) {
section = new AccessSection(AccessSection.GLOBAL_CAPABILITIES); section = AccessSection.create(AccessSection.GLOBAL_CAPABILITIES);
} }
Map<String, List<PermissionRule>> tmp = new HashMap<>(); Map<String, List<PermissionRule>> tmp = new HashMap<>();

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

@@ -350,16 +350,16 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
} }
public AccessSection getAccessSection(String name) { public AccessSection getAccessSection(String name) {
return getAccessSection(name, false); return accessSections.get(name);
} }
public AccessSection getAccessSection(String name, boolean create) { public void upsertAccessSection(String name, Consumer<AccessSection.Builder> update) {
AccessSection as = accessSections.get(name); AccessSection.Builder accessSectionBuilder =
if (as == null && create) { accessSections.containsKey(name)
as = new AccessSection(name); ? accessSections.get(name).toBuilder()
accessSections.put(name, as); : AccessSection.builder(name);
} update.accept(accessSectionBuilder);
return as; accessSections.put(name, accessSectionBuilder.build());
} }
public ImmutableSet<String> getAccessSectionNames() { public ImmutableSet<String> getAccessSectionNames() {
@@ -400,8 +400,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
if (section != null) { if (section != null) {
String name = section.getName(); String name = section.getName();
if (sectionsWithUnknownPermissions.contains(name)) { if (sectionsWithUnknownPermissions.contains(name)) {
AccessSection a = accessSections.get(name); AccessSection.Builder a = accessSections.get(name).toBuilder();
a.setPermissions(new ArrayList<>()); a.modifyPermissions(p -> p.clear());
accessSections.put(name, a.build());
} else { } else {
accessSections.remove(name); accessSections.remove(name);
} }
@@ -412,8 +413,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
if (permission == null) { if (permission == null) {
remove(section); remove(section);
} else if (section != null) { } else if (section != null) {
AccessSection a = accessSections.get(section.getName()); AccessSection a =
a.remove(permission); accessSections.get(section.getName()).toBuilder().remove(permission.toBuilder()).build();
accessSections.put(section.getName(), a);
if (a.getPermissions().isEmpty()) { if (a.getPermissions().isEmpty()) {
remove(a); remove(a);
} }
@@ -432,28 +434,21 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
if (p == null) { if (p == null) {
return; return;
} }
p.remove(rule); AccessSection.Builder accessSectionBuilder = a.toBuilder();
if (p.getRules().isEmpty()) { Permission.Builder permissionBuilder =
a.remove(permission); 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()) { if (a.getPermissions().isEmpty()) {
remove(a); 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) { public ContributorAgreement getContributorAgreement(String name) {
return contributorAgreements.get(name); return contributorAgreements.get(name);
} }
@@ -790,38 +785,41 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
sectionsWithUnknownPermissions = new HashSet<>(); sectionsWithUnknownPermissions = new HashSet<>();
for (String refName : rc.getSubsections(ACCESS)) { for (String refName : rc.getSubsections(ACCESS)) {
if (AccessSection.isValidRefSectionName(refName) && isValidRegex(refName)) { 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 varName : rc.getNames(ACCESS, refName)) {
for (String n : Splitter.on(EXCLUSIVE_PERMISSIONS_SPLIT_PATTERN).split(varName)) { String convertedName = convertLegacyPermission(varName);
n = convertLegacyPermission(n); if (isCoreOrPluginPermission(convertedName)) {
if (isCoreOrPluginPermission(n)) { Permission.Builder perm = as.upsertPermission(convertedName);
as.getPermission(n, true).setExclusiveGroup(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 perm = as.getPermission(convertedName, true);
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)) { for (String varName : rc.getNames(CAPABILITY)) {
if (capability == null) { if (capability == null) {
capability = new AccessSection(AccessSection.GLOBAL_CAPABILITIES); capability = AccessSection.builder(AccessSection.GLOBAL_CAPABILITIES);
accessSections.put(AccessSection.GLOBAL_CAPABILITIES, capability); 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)); loadPermissionRules(rc, CAPABILITY, null, varName, perm, GlobalCapability.hasRange(varName));
accessSections.put(AccessSection.GLOBAL_CAPABILITIES, capability.build());
} }
} }
@@ -874,9 +872,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
private ImmutableList<PermissionRule> loadPermissionRules( private ImmutableList<PermissionRule> loadPermissionRules(
Config rc, String section, String subsection, String varName, boolean useRange) { 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); loadPermissionRules(rc, section, subsection, varName, perm, useRange);
return ImmutableList.copyOf(perm.getRules()); return ImmutableList.copyOf(perm.build().getRules());
} }
private void loadPermissionRules( private void loadPermissionRules(
@@ -884,7 +882,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
String section, String section,
String subsection, String subsection,
String varName, String varName,
Permission perm, Permission.Builder perm,
boolean useRange) { boolean useRange) {
for (String ruleString : rc.getStringList(section, subsection, varName)) { for (String ruleString : rc.getStringList(section, subsection, varName)) {
PermissionRule rule; PermissionRule rule;
@@ -916,7 +914,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
PROJECT_CONFIG, "group \"" + ref.getName() + "\" not in " + GroupList.FILE_NAME)); PROJECT_CONFIG, "group \"" + ref.getName() + "\" not in " + GroupList.FILE_NAME));
} }
perm.add(rule.toBuilder().setGroup(ref).build()); perm.add(rule.toBuilder().setGroup(ref));
} }
} }

19
java/com/google/gerrit/server/project/ProjectCreator.java
View File

@@ -179,14 +179,17 @@ public class ProjectCreator {
}); });
if (!args.ownerIds.isEmpty()) { if (!args.ownerIds.isEmpty()) {
AccessSection all = config.getAccessSection(AccessSection.ALL, true); config.upsertAccessSection(
for (AccountGroup.UUID ownerId : args.ownerIds) { AccessSection.ALL,
GroupDescription.Basic g = groupBackend.get(ownerId); all -> {
if (g != null) { for (AccountGroup.UUID ownerId : args.ownerIds) {
GroupReference group = config.resolve(GroupReference.forGroup(g)); GroupDescription.Basic g = groupBackend.get(ownerId);
all.getPermission(Permission.OWNER, true).add(PermissionRule.create(group)); if (g != null) {
} GroupReference group = config.resolve(GroupReference.forGroup(g));
} all.upsertPermission(Permission.OWNER).add(PermissionRule.builder(group));
}
}
});
} }
md.setMessage("Created project\n"); md.setMessage("Created project\n");

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

@@ -280,14 +280,16 @@ public class ProjectState {
sm = new ArrayList<>(fromConfig.size()); sm = new ArrayList<>(fromConfig.size());
for (AccessSection section : fromConfig) { for (AccessSection section : fromConfig) {
if (isAllProjects) { if (isAllProjects) {
List<Permission> copy = Lists.newArrayListWithCapacity(section.getPermissions().size()); List<Permission.Builder> copy = new ArrayList<>();
for (Permission p : section.getPermissions()) { for (Permission p : section.getPermissions()) {
if (Permission.canBeOnAllProjects(section.getName(), p.getName())) { if (Permission.canBeOnAllProjects(section.getName(), p.getName())) {
copy.add(p); copy.add(p.toBuilder());
} }
} }
section = new AccessSection(section.getName()); section =
section.setPermissions(copy); AccessSection.builder(section.getName())
.modifyPermissions(permissions -> permissions.addAll(copy))
.build();
} }
SectionMatcher matcher = SectionMatcher.wrap(getNameKey(), section); SectionMatcher matcher = SectionMatcher.wrap(getNameKey(), section);

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

@@ -208,9 +208,9 @@ public class GetAccess implements RestReadView<ProjectResource> {
// user is a member of, as well as groups they own or that // user is a member of, as well as groups they own or that
// are visible to all users. // are visible to all users.
AccessSection dst = null; AccessSection.Builder dst = null;
for (Permission srcPerm : section.getPermissions()) { for (Permission srcPerm : section.getPermissions()) {
Permission dstPerm = null; Permission.Builder dstPerm = null;
for (PermissionRule srcRule : srcPerm.getRules()) { for (PermissionRule srcRule : srcPerm.getRules()) {
AccountGroup.UUID groupId = srcRule.getGroup().getUUID(); AccountGroup.UUID groupId = srcRule.getGroup().getUUID();
@@ -221,12 +221,12 @@ public class GetAccess implements RestReadView<ProjectResource> {
loadGroup(groups, groupId); loadGroup(groups, groupId);
if (dstPerm == null) { if (dstPerm == null) {
if (dst == null) { if (dst == null) {
dst = new AccessSection(name); dst = AccessSection.builder(name);
info.local.put(name, createAccessSection(groups, dst)); 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());
} }
} }
} }

42
java/com/google/gerrit/server/restapi/project/SetAccessUtil.java
View File

@@ -78,14 +78,14 @@ public class SetAccessUtil {
continue; continue;
} }
AccessSection accessSection = new AccessSection(entry.getKey()); AccessSection.Builder accessSection = AccessSection.builder(entry.getKey());
for (Map.Entry<String, PermissionInfo> permissionEntry : for (Map.Entry<String, PermissionInfo> permissionEntry :
entry.getValue().permissions.entrySet()) { entry.getValue().permissions.entrySet()) {
if (permissionEntry.getValue().rules == null) { if (permissionEntry.getValue().rules == null) {
continue; continue;
} }
Permission p = new Permission(permissionEntry.getKey()); Permission.Builder p = Permission.builder(permissionEntry.getKey());
if (permissionEntry.getValue().exclusive != null) { if (permissionEntry.getValue().exclusive != null) {
p.setExclusiveGroup(permissionEntry.getValue().exclusive); p.setExclusiveGroup(permissionEntry.getValue().exclusive);
} }
@@ -114,11 +114,11 @@ public class SetAccessUtil {
r.setForce(pri.force); r.setForce(pri.force);
} }
} }
p.add(r.build()); p.add(r);
} }
accessSection.addPermission(p); accessSection.addPermission(p);
} }
sections.add(accessSection); sections.add(accessSection.build());
} }
return sections; return sections;
} }
@@ -193,25 +193,23 @@ public class SetAccessUtil {
// Apply additions // Apply additions
for (AccessSection section : additions) { for (AccessSection section : additions) {
AccessSection currentAccessSection = config.getAccessSection(section.getName()); config.upsertAccessSection(
section.getName(),
if (currentAccessSection == null) { existingAccessSection -> {
// Add AccessSection for (Permission p : section.getPermissions()) {
config.replace(section); Permission currentPermission =
} else { existingAccessSection.build().getPermission(p.getName());
for (Permission p : section.getPermissions()) { if (currentPermission == null) {
Permission currentPermission = currentAccessSection.getPermission(p.getName()); // Add Permission
if (currentPermission == null) { existingAccessSection.addPermission(p.toBuilder());
// Add Permission } else {
currentAccessSection.addPermission(p); for (PermissionRule r : p.getRules()) {
} else { // AddPermissionRule
for (PermissionRule r : p.getRules()) { existingAccessSection.upsertPermission(p.getName()).add(r.toBuilder());
// AddPermissionRule }
currentPermission.add(r); }
} }
} });
}
}
} }
} }

37
java/com/google/gerrit/server/schema/AclUtil.java
View File

@@ -27,13 +27,16 @@ import com.google.gerrit.server.project.ProjectConfig;
*/ */
public class AclUtil { public class AclUtil {
public static void grant( 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); grant(config, section, permission, false, groupList);
} }
public static void grant( public static void grant(
ProjectConfig config, ProjectConfig config,
AccessSection section, AccessSection.Builder section,
String permission, String permission,
boolean force, boolean force,
GroupReference... groupList) { GroupReference... groupList) {
@@ -42,35 +45,38 @@ public class AclUtil {
public static void grant( public static void grant(
ProjectConfig config, ProjectConfig config,
AccessSection section, AccessSection.Builder section,
String permission, String permission,
boolean force, boolean force,
Boolean exclusive, Boolean exclusive,
GroupReference... groupList) { GroupReference... groupList) {
Permission p = section.getPermission(permission, true); Permission.Builder p = section.upsertPermission(permission);
if (exclusive != null) { if (exclusive != null) {
p.setExclusiveGroup(exclusive); p.setExclusiveGroup(exclusive);
} }
for (GroupReference group : groupList) { for (GroupReference group : groupList) {
if (group != null) { if (group != null) {
p.add(rule(config, group).setForce(force).build()); p.add(rule(config, group).setForce(force));
} }
} }
} }
public static void block( public static void block(
ProjectConfig config, AccessSection section, String permission, GroupReference... groupList) { ProjectConfig config,
Permission p = section.getPermission(permission, true); AccessSection.Builder section,
String permission,
GroupReference... groupList) {
Permission.Builder p = section.upsertPermission(permission);
for (GroupReference group : groupList) { for (GroupReference group : groupList) {
if (group != null) { if (group != null) {
p.add(rule(config, group).setBlock().build()); p.add(rule(config, group).setBlock());
} }
} }
} }
public static void grant( public static void grant(
ProjectConfig config, ProjectConfig config,
AccessSection section, AccessSection.Builder section,
LabelType type, LabelType type,
int min, int min,
int max, int max,
@@ -80,18 +86,18 @@ public class AclUtil {
public static void grant( public static void grant(
ProjectConfig config, ProjectConfig config,
AccessSection section, AccessSection.Builder section,
LabelType type, LabelType type,
int min, int min,
int max, int max,
boolean exclusive, boolean exclusive,
GroupReference... groupList) { GroupReference... groupList) {
String name = Permission.LABEL + type.getName(); String name = Permission.LABEL + type.getName();
Permission p = section.getPermission(name, true); Permission.Builder p = section.upsertPermission(name);
p.setExclusiveGroup(exclusive); p.setExclusiveGroup(exclusive);
for (GroupReference group : groupList) { for (GroupReference group : groupList) {
if (group != null) { 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 class AclUtil {
} }
public static void remove( public static void remove(
ProjectConfig config, AccessSection section, String permission, GroupReference... groupList) { ProjectConfig config,
Permission p = section.getPermission(permission, true); AccessSection.Builder section,
String permission,
GroupReference... groupList) {
Permission.Builder p = section.upsertPermission(permission);
for (GroupReference group : groupList) { for (GroupReference group : groupList) {
if (group != null) { if (group != null) {
p.remove(rule(config, group).build()); p.remove(rule(config, group).build());

128
java/com/google/gerrit/server/schema/AllProjectsCreator.java
View File

@@ -144,79 +144,107 @@ public class AllProjectsCreator {
} }
private void initDefaultAcls(ProjectConfig config, AllProjectsInput input) { 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()); checkArgument(input.codeReviewLabel().isPresent());
LabelType codeReviewLabel = input.codeReviewLabel().get(); LabelType codeReviewLabel = input.codeReviewLabel().get();
initDefaultAclsForRegisteredUsers(heads, codeReviewLabel, config); config.upsertAccessSection(
AccessSection.HEADS,
heads -> {
initDefaultAclsForRegisteredUsers(heads, codeReviewLabel, config);
});
input config.upsertAccessSection(
.batchUsersGroup() AccessSection.GLOBAL_CAPABILITIES,
.ifPresent( capabilities -> {
batchUsersGroup -> initDefaultAclsForBatchUsers(capabilities, config, batchUsersGroup)); input
.batchUsersGroup()
.ifPresent(
batchUsersGroup ->
initDefaultAclsForBatchUsers(capabilities, config, batchUsersGroup));
});
input input
.administratorsGroup() .administratorsGroup()
.ifPresent( .ifPresent(adminsGroup -> initDefaultAclsForAdmins(config, codeReviewLabel, adminsGroup));
adminsGroup ->
initDefaultAclsForAdmins(
capabilities, config, heads, codeReviewLabel, adminsGroup));
} }
private void initDefaultAclsForRegisteredUsers( private void initDefaultAclsForRegisteredUsers(
AccessSection heads, LabelType codeReviewLabel, ProjectConfig config) { AccessSection.Builder heads, LabelType codeReviewLabel, ProjectConfig config) {
AccessSection refsFor = config.getAccessSection("refs/for/*", true); config.upsertAccessSection(
AccessSection magic = config.getAccessSection("refs/for/" + AccessSection.ALL, true); "refs/for/*",
AccessSection all = config.getAccessSection("refs/*", true); 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, codeReviewLabel, -1, 1, registered);
grant(config, heads, Permission.FORGE_AUTHOR, registered); grant(config, heads, Permission.FORGE_AUTHOR, registered);
grant(config, all, Permission.REVERT, registered);
grant(config, magic, Permission.PUSH, registered); config.upsertAccessSection(
grant(config, magic, Permission.PUSH_MERGE, registered); "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( private void initDefaultAclsForBatchUsers(
AccessSection capabilities, ProjectConfig config, GroupReference batchUsersGroup) { AccessSection.Builder capabilities, ProjectConfig config, GroupReference batchUsersGroup) {
Permission priority = capabilities.getPermission(GlobalCapability.PRIORITY, true); Permission.Builder priority = capabilities.upsertPermission(GlobalCapability.PRIORITY);
priority.add(rule(config, batchUsersGroup).setAction(Action.BATCH).build()); priority.add(rule(config, batchUsersGroup).setAction(Action.BATCH));
Permission stream = capabilities.getPermission(GlobalCapability.STREAM_EVENTS, true); Permission.Builder stream = capabilities.upsertPermission(GlobalCapability.STREAM_EVENTS);
stream.add(rule(config, batchUsersGroup).build()); stream.add(rule(config, batchUsersGroup));
} }
private void initDefaultAclsForAdmins( private void initDefaultAclsForAdmins(
AccessSection capabilities, ProjectConfig config, LabelType codeReviewLabel, GroupReference adminsGroup) {
ProjectConfig config, config.upsertAccessSection(
AccessSection heads, AccessSection.GLOBAL_CAPABILITIES,
LabelType codeReviewLabel, capabilities -> {
GroupReference adminsGroup) { grant(config, capabilities, GlobalCapability.ADMINISTRATE_SERVER, adminsGroup);
AccessSection all = config.getAccessSection(AccessSection.ALL, true); });
AccessSection tags = config.getAccessSection("refs/tags/*", true);
AccessSection meta = config.getAccessSection(RefNames.REFS_CONFIG, true);
grant(config, capabilities, GlobalCapability.ADMINISTRATE_SERVER, adminsGroup); config.upsertAccessSection(
grant(config, all, Permission.READ, adminsGroup, anonymous); AccessSection.ALL,
grant(config, heads, codeReviewLabel, -2, 2, adminsGroup, owners); all -> {
grant(config, heads, Permission.CREATE, adminsGroup, owners); grant(config, all, Permission.READ, adminsGroup, anonymous);
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);
grant(config, tags, Permission.CREATE, adminsGroup, owners); config.upsertAccessSection(
grant(config, tags, Permission.CREATE_TAG, adminsGroup, owners); AccessSection.HEADS,
grant(config, tags, Permission.CREATE_SIGNED_TAG, adminsGroup, owners); 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); config.upsertAccessSection(
grant(config, meta, Permission.READ, adminsGroup, owners); "refs/tags/*",
grant(config, meta, codeReviewLabel, -2, 2, adminsGroup, owners); tags -> {
grant(config, meta, Permission.CREATE, adminsGroup, owners); grant(config, tags, Permission.CREATE, adminsGroup, owners);
grant(config, meta, Permission.PUSH, adminsGroup, owners); grant(config, tags, Permission.CREATE_TAG, adminsGroup, owners);
grant(config, meta, Permission.SUBMIT, 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) private void initSequences(Repository git, BatchRefUpdate bru, int firstChangeId)

41
java/com/google/gerrit/server/schema/AllUsersCreator.java
View File

@@ -22,7 +22,6 @@ import static com.google.gerrit.server.schema.AllProjectsInput.getDefaultCodeRev
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt; import com.google.gerrit.common.UsedAt;
import com.google.gerrit.common.Version; 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.GroupReference;
import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.Permission;
@@ -113,33 +112,39 @@ public class AllUsersCreator {
ProjectConfig config = projectConfigFactory.read(md); ProjectConfig config = projectConfigFactory.read(md);
config.updateProject(p -> p.setDescription("Individual user settings and preferences.")); config.updateProject(p -> p.setDescription("Individual user settings and preferences."));
AccessSection users = config.upsertAccessSection(
config.getAccessSection( RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}",
RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}", true); 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. // Initialize "Code-Review" label.
config.upsertLabelType(codeReviewLabel); 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) { if (admin != null) {
AccessSection defaults = config.getAccessSection(RefNames.REFS_USERS_DEFAULT, true); config.upsertAccessSection(
defaults.getPermission(Permission.READ, true).setExclusiveGroup(true); RefNames.REFS_USERS_DEFAULT,
grant(config, defaults, Permission.READ, admin); defaults -> {
defaults.getPermission(Permission.PUSH, true).setExclusiveGroup(true); defaults.upsertPermission(Permission.READ).setExclusiveGroup(true);
grant(config, defaults, Permission.PUSH, admin); grant(config, defaults, Permission.READ, admin);
defaults.getPermission(Permission.CREATE, true).setExclusiveGroup(true); defaults.upsertPermission(Permission.PUSH).setExclusiveGroup(true);
grant(config, defaults, Permission.CREATE, admin); 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. // Grant read permissions on the group branches to all users.
// This allows group owners to see the group refs. VisibleRefFilter ensures that read // This allows group owners to see the group refs. VisibleRefFilter ensures that read
// permissions for non-group-owners are ignored. // permissions for non-group-owners are ignored.
AccessSection groups = config.getAccessSection(RefNames.REFS_GROUPS + "*", true); config.upsertAccessSection(
grant(config, groups, Permission.READ, false, true, registered); RefNames.REFS_GROUPS + "*",
groups -> {
grant(config, groups, Permission.READ, false, true, registered);
});
config.commit(md); config.commit(md);
} }

48
java/com/google/gerrit/server/schema/GrantRevertPermission.java
View File

@@ -30,6 +30,7 @@ import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ProjectConfig; import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.io.IOException; import java.io.IOException;
import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
@@ -63,28 +64,41 @@ public class GrantRevertPermission {
try (Repository repo = repoManager.openRepository(projectName)) { try (Repository repo = repoManager.openRepository(projectName)) {
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, projectName, repo); MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, projectName, repo);
ProjectConfig projectConfig = projectConfigFactory.read(md); 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 != null) {
if (permissionOnRefsHeads.getRule(registeredUsers) == null if (permissionOnRefsHeads.getRule(registeredUsers) == null
|| permissionOnRefsHeads.getRules().size() > 1) { || permissionOnRefsHeads.getRules().size() > 1) {
// If admins already changed the permission, don't do anything. // If admins already changed the permission, don't do anything.
return; shouldExit.set(true);
} return;
// permission already exists in refs/heads/*, delete it for Registered Users. }
remove(projectConfig, heads, Permission.REVERT, registeredUsers); // permission already exists in refs/heads/*, delete it for Registered Users.
} remove(projectConfig, heads, Permission.REVERT, registeredUsers);
}
});
AccessSection all = projectConfig.getAccessSection(AccessSection.ALL, true); if (shouldExit.get()) {
Permission permissionOnRefsStar = all.getPermission(Permission.REVERT);
if (permissionOnRefsStar != null && permissionOnRefsStar.getRule(registeredUsers) != null) {
// permission already exists in refs/*, don't do anything.
return; 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().setAuthor(serverUser);
md.getCommitBuilder().setCommitter(serverUser); md.getCommitBuilder().setCommitter(serverUser);

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

@@ -1653,8 +1653,11 @@ public class AccountIT extends AbstractDaemonTest {
// remove default READ permissions // remove default READ permissions
try (ProjectConfigUpdate u = updateProject(allUsers)) { try (ProjectConfigUpdate u = updateProject(allUsers)) {
u.getConfig() u.getConfig()
.getAccessSection(RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}", true) .upsertAccessSection(
.remove(new Permission(Permission.READ)); RefNames.REFS_USERS + "${" + RefPattern.USERID_SHARDED + "}",
as -> {
as.remove(Permission.builder(Permission.READ));
});
u.save(); u.save();
} }

26
javatests/com/google/gerrit/acceptance/git/PushPermissionsIT.java
View File

@@ -362,22 +362,28 @@ public class PushPermissionsIT extends AbstractDaemonTest {
} }
private static void removeAllBranchPermissions(ProjectConfig cfg, String... permissions) { private static void removeAllBranchPermissions(ProjectConfig cfg, String... permissions) {
cfg.getAccessSections().stream() for (AccessSection s : ImmutableList.copyOf(cfg.getAccessSections())) {
.filter( if (s.getName().startsWith("refs/heads/")
s -> || s.getName().startsWith("refs/for/")
s.getName().startsWith("refs/heads/") || s.getName().equals("refs/*")) {
|| s.getName().startsWith("refs/for/") cfg.upsertAccessSection(
|| s.getName().equals("refs/*")) s.getName(),
.forEach(s -> Arrays.stream(permissions).forEach(s::removePermission)); updatedSection -> {
Arrays.stream(permissions).forEach(p -> updatedSection.remove(Permission.builder(p)));
});
}
}
} }
private static void removeAllGlobalCapabilities(ProjectConfig cfg, String... capabilities) { private static void removeAllGlobalCapabilities(ProjectConfig cfg, String... capabilities) {
Arrays.stream(capabilities) Arrays.stream(capabilities)
.forEach( .forEach(
c -> c ->
cfg.getAccessSection(AccessSection.GLOBAL_CAPABILITIES, true) cfg.upsertAccessSection(
.getPermission(c, true) AccessSection.GLOBAL_CAPABILITIES,
.clearRules()); as -> {
as.upsertPermission(c).clearRules();
}));
} }
private PushResult push(String... refSpecs) throws Exception { private PushResult push(String... refSpecs) throws Exception {

19
javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
View File

@@ -127,8 +127,14 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
private void setUpPermissions() throws Exception { private void setUpPermissions() throws Exception {
// Remove read permissions for all users besides admin. // Remove read permissions for all users besides admin.
try (ProjectConfigUpdate u = updateProject(allProjects)) { 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(); u.save();
} }
@@ -139,8 +145,13 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
// Remove all read permissions on All-Users. // Remove all read permissions on All-Users.
try (ProjectConfigUpdate u = updateProject(allUsers)) { try (ProjectConfigUpdate u = updateProject(allUsers)) {
for (AccessSection sec : u.getConfig().getAccessSections()) { for (AccessSection sec : ImmutableList.copyOf(u.getConfig().getAccessSections())) {
sec.removePermission(Permission.READ); u.getConfig()
.upsertAccessSection(
sec.getName(),
updatedSec -> {
updatedSec.removePermission(Permission.READ);
});
} }
u.save(); u.save();
} }

19
javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
View File

@@ -120,8 +120,11 @@ public class AccessIT extends AbstractDaemonTest {
try (Repository repo = repoManager.openRepository(newProjectName)) { try (Repository repo = repoManager.openRepository(newProjectName)) {
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo); MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo);
ProjectConfig projectConfig = projectConfigFactory.read(md); ProjectConfig projectConfig = projectConfigFactory.read(md);
AccessSection heads = projectConfig.getAccessSection(AccessSection.HEADS, true); projectConfig.upsertAccessSection(
grant(projectConfig, heads, Permission.REVERT, registeredUsers); AccessSection.HEADS,
heads -> {
grant(projectConfig, heads, Permission.REVERT, registeredUsers);
});
md.getCommitBuilder().setAuthor(admin.newIdent()); md.getCommitBuilder().setAuthor(admin.newIdent());
md.getCommitBuilder().setCommitter(admin.newIdent()); md.getCommitBuilder().setCommitter(admin.newIdent());
md.setMessage("Add revert permission for all registered users\n"); md.setMessage("Add revert permission for all registered users\n");
@@ -155,15 +158,19 @@ public class AccessIT extends AbstractDaemonTest {
try (Repository repo = repoManager.openRepository(newProjectName)) { try (Repository repo = repoManager.openRepository(newProjectName)) {
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo); MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo);
ProjectConfig projectConfig = projectConfigFactory.read(md); ProjectConfig projectConfig = projectConfigFactory.read(md);
AccessSection heads = projectConfig.getAccessSection(AccessSection.HEADS, true); projectConfig.upsertAccessSection(
grant(projectConfig, heads, Permission.REVERT, registeredUsers); AccessSection.HEADS,
grant(projectConfig, heads, Permission.REVERT, otherGroup); heads -> {
grant(projectConfig, heads, Permission.REVERT, registeredUsers);
grant(projectConfig, heads, Permission.REVERT, otherGroup);
});
md.getCommitBuilder().setAuthor(admin.newIdent()); md.getCommitBuilder().setAuthor(admin.newIdent());
md.getCommitBuilder().setCommitter(admin.newIdent()); md.getCommitBuilder().setCommitter(admin.newIdent());
md.setMessage("Add revert permission for all registered users\n"); md.setMessage("Add revert permission for all registered users\n");
projectConfig.commit(md); projectConfig.commit(md);
} }
projectCache.evict(newProjectName);
ProjectAccessInfo expected = pApi().access(); ProjectAccessInfo expected = pApi().access();
grantRevertPermission.execute(newProjectName); grantRevertPermission.execute(newProjectName);
@@ -181,7 +188,7 @@ public class AccessIT extends AbstractDaemonTest {
try (Repository repo = repoManager.openRepository(newProjectName)) { try (Repository repo = repoManager.openRepository(newProjectName)) {
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo); MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, newProjectName, repo);
ProjectConfig projectConfig = projectConfigFactory.read(md); ProjectConfig projectConfig = projectConfigFactory.read(md);
AccessSection all = projectConfig.getAccessSection(AccessSection.ALL, true); AccessSection all = projectConfig.getAccessSection(AccessSection.ALL);
Permission permission = all.getPermission(Permission.REVERT); Permission permission = all.getPermission(Permission.REVERT);
assertThat(permission.getRules()).hasSize(1); assertThat(permission.getRules()).hasSize(1);

7
javatests/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
View File

@@ -129,7 +129,12 @@ public class GetCommitIT extends AbstractDaemonTest {
private void unblockRead() throws Exception { private void unblockRead() throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) { 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(); u.save();
} }
} }

19
javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
View File

@@ -28,6 +28,7 @@ import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations; 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.common.data.Permission;
import com.google.gerrit.extensions.api.projects.ProjectApi.ListRefsRequest; import com.google.gerrit.extensions.api.projects.ProjectApi.ListRefsRequest;
import com.google.gerrit.extensions.api.projects.TagApi; import com.google.gerrit.extensions.api.projects.TagApi;
@@ -158,6 +159,12 @@ public class TagsIT extends AbstractDaemonTest {
@Test @Test
public void listTagsOfNonVisibleBranch() throws Exception { public void listTagsOfNonVisibleBranch() throws Exception {
grantTagPermissions(); 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 push1 = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r1 = push1.to("refs/heads/master"); PushOneCommit.Result r1 = push1.to("refs/heads/master");
@@ -169,7 +176,7 @@ public class TagsIT extends AbstractDaemonTest {
assertThat(result.ref).isEqualTo(R_TAGS + tag1.ref); assertThat(result.ref).isEqualTo(R_TAGS + tag1.ref);
assertThat(result.revision).isEqualTo(tag1.revision); assertThat(result.revision).isEqualTo(tag1.revision);
pushTo("refs/heads/hidden"); pushTo("refs/heads/hidden").assertOkStatus();
PushOneCommit push2 = pushFactory.create(admin.newIdent(), testRepo); PushOneCommit push2 = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r2 = push2.to("refs/heads/hidden"); PushOneCommit.Result r2 = push2.to("refs/heads/hidden");
r2.assertOkStatus(); r2.assertOkStatus();
@@ -470,8 +477,12 @@ public class TagsIT extends AbstractDaemonTest {
} }
private static void removeAllBranchPermissions(ProjectConfig cfg, String... permissions) { private static void removeAllBranchPermissions(ProjectConfig cfg, String... permissions) {
cfg.getAccessSections().stream() for (AccessSection accessSection : ImmutableList.copyOf(cfg.getAccessSections())) {
.filter(s -> s.getName().startsWith("refs/tags/")) cfg.upsertAccessSection(
.forEach(s -> Arrays.stream(permissions).forEach(s::removePermission)); accessSection.getName(),
updatedAccessSection -> {
Arrays.stream(permissions).forEach(updatedAccessSection::removePermission);
});
}
} }
} }

235
javatests/com/google/gerrit/common/data/AccessSectionTest.java
View File

@@ -17,9 +17,6 @@ package com.google.gerrit.common.data;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows; 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 java.util.Locale;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
@@ -27,11 +24,11 @@ import org.junit.Test;
public class AccessSectionTest { public class AccessSectionTest {
private static final String REF_PATTERN = "refs/heads/master"; private static final String REF_PATTERN = "refs/heads/master";
private AccessSection accessSection; private AccessSection.Builder accessSection;
@Before @Before
public void setup() { public void setup() {
this.accessSection = new AccessSection(REF_PATTERN); this.accessSection = AccessSection.builder(REF_PATTERN);
} }
@Test @Test
@@ -41,22 +38,36 @@ public class AccessSectionTest {
@Test @Test
public void getEmptyPermissions() { public void getEmptyPermissions() {
assertThat(accessSection.getPermissions()).isNotNull(); AccessSection builtAccessSection = accessSection.build();
assertThat(accessSection.getPermissions()).isEmpty(); assertThat(builtAccessSection.getPermissions()).isNotNull();
assertThat(builtAccessSection.getPermissions()).isEmpty();
} }
@Test @Test
public void setAndGetPermissions() { public void setAndGetPermissions() {
Permission abandonPermission = new Permission(Permission.ABANDON); Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
Permission rebasePermission = new Permission(Permission.REBASE); Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
accessSection.setPermissions(ImmutableList.of(abandonPermission, rebasePermission)); accessSection.modifyPermissions(
assertThat(accessSection.getPermissions()) permissions -> {
.containsExactly(abandonPermission, rebasePermission) permissions.clear();
.inOrder(); permissions.add(abandonPermission);
permissions.add(rebasePermission);
});
Permission submitPermission = new Permission(Permission.SUBMIT); AccessSection builtAccessSection = accessSection.build();
accessSection.setPermissions(ImmutableList.of(submitPermission)); assertThat(builtAccessSection.getPermissions()).hasSize(2);
assertThat(accessSection.getPermissions()).containsExactly(submitPermission); 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)); assertThrows(NullPointerException.class, () -> accessSection.setPermissions(null));
} }
@@ -65,122 +76,117 @@ public class AccessSectionTest {
assertThrows( assertThrows(
IllegalArgumentException.class, IllegalArgumentException.class,
() -> () ->
accessSection.setPermissions( accessSection
ImmutableList.of( .addPermission(Permission.builder(Permission.ABANDON))
new Permission(Permission.ABANDON), new Permission(Permission.ABANDON)))); .addPermission(Permission.builder(Permission.ABANDON))
.build());
} }
@Test @Test
public void cannotSetPermissionsWithConflictingNames() { public void cannotSetPermissionsWithConflictingNames() {
Permission abandonPermissionLowerCase = Permission.Builder abandonPermissionLowerCase =
new Permission(Permission.ABANDON.toLowerCase(Locale.US)); Permission.builder(Permission.ABANDON.toLowerCase(Locale.US));
Permission abandonPermissionUpperCase = Permission.Builder abandonPermissionUpperCase =
new Permission(Permission.ABANDON.toUpperCase(Locale.US)); Permission.builder(Permission.ABANDON.toUpperCase(Locale.US));
assertThrows( assertThrows(
IllegalArgumentException.class, IllegalArgumentException.class,
() -> () ->
accessSection.setPermissions( accessSection
ImmutableList.of(abandonPermissionLowerCase, abandonPermissionUpperCase))); .addPermission(abandonPermissionLowerCase)
.addPermission(abandonPermissionUpperCase)
.build());
} }
@Test @Test
public void getNonExistingPermission() { public void getNonExistingPermission() {
assertThat(accessSection.getPermission("non-existing")).isNull(); assertThat(accessSection.build().getPermission("non-existing")).isNull();
assertThat(accessSection.getPermission("non-existing", false)).isNull(); assertThat(accessSection.build().getPermission("non-existing")).isNull();
} }
@Test @Test
public void getPermission() { public void getPermission() {
Permission submitPermission = new Permission(Permission.SUBMIT); Permission.Builder submitPermission = Permission.builder(Permission.SUBMIT);
accessSection.setPermissions(ImmutableList.of(submitPermission)); accessSection.addPermission(submitPermission);
assertThat(accessSection.getPermission(Permission.SUBMIT)).isEqualTo(submitPermission); assertThat(accessSection.upsertPermission(Permission.SUBMIT)).isEqualTo(submitPermission);
assertThrows(NullPointerException.class, () -> accessSection.getPermission(null)); assertThrows(NullPointerException.class, () -> accessSection.upsertPermission(null));
} }
@Test @Test
public void getPermissionWithOtherCase() { public void getPermissionWithOtherCase() {
Permission submitPermissionLowerCase = new Permission(Permission.SUBMIT.toLowerCase(Locale.US)); Permission.Builder submitPermissionLowerCase =
accessSection.setPermissions(ImmutableList.of(submitPermissionLowerCase)); Permission.builder(Permission.SUBMIT.toLowerCase(Locale.US));
assertThat(accessSection.getPermission(Permission.SUBMIT.toUpperCase(Locale.US))) accessSection.addPermission(submitPermissionLowerCase);
assertThat(accessSection.upsertPermission(Permission.SUBMIT.toUpperCase(Locale.US)))
.isEqualTo(submitPermissionLowerCase); .isEqualTo(submitPermissionLowerCase);
} }
@Test @Test
public void createMissingPermissionOnGet() { public void createMissingPermissionOnGet() {
assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull(); assertThat(accessSection.build().getPermission(Permission.SUBMIT)).isNull();
assertThat(accessSection.getPermission(Permission.SUBMIT, true)) assertThat(accessSection.upsertPermission(Permission.SUBMIT).build())
.isEqualTo(new Permission(Permission.SUBMIT)); .isEqualTo(Permission.create(Permission.SUBMIT));
assertThrows(NullPointerException.class, () -> accessSection.getPermission(null, true)); assertThrows(NullPointerException.class, () -> accessSection.upsertPermission(null));
} }
@Test @Test
public void addPermission() { public void addPermission() {
Permission abandonPermission = new Permission(Permission.ABANDON); Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
Permission rebasePermission = new Permission(Permission.REBASE); Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
accessSection.setPermissions(ImmutableList.of(abandonPermission, rebasePermission)); accessSection.addPermission(abandonPermission);
assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull(); 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); accessSection.addPermission(submitPermission);
assertThat(accessSection.getPermission(Permission.SUBMIT)).isEqualTo(submitPermission); assertThat(accessSection.build().getPermission(Permission.SUBMIT))
assertThat(accessSection.getPermissions()) .isEqualTo(submitPermission.build());
.containsExactly(abandonPermission, rebasePermission, submitPermission) assertThat(accessSection.build().getPermissions())
.containsExactly(
abandonPermission.build(), rebasePermission.build(), submitPermission.build())
.inOrder(); .inOrder();
assertThrows(NullPointerException.class, () -> accessSection.addPermission(null)); 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 @Test
public void removePermission() { public void removePermission() {
Permission abandonPermission = new Permission(Permission.ABANDON); Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
Permission rebasePermission = new Permission(Permission.REBASE); Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
Permission submitPermission = new Permission(Permission.SUBMIT); Permission.Builder submitPermission = Permission.builder(Permission.SUBMIT);
accessSection.setPermissions( accessSection.addPermission(abandonPermission);
ImmutableList.of(abandonPermission, rebasePermission, submitPermission)); accessSection.addPermission(rebasePermission);
assertThat(accessSection.getPermission(Permission.SUBMIT)).isNotNull(); accessSection.addPermission(submitPermission);
assertThat(accessSection.build().getPermission(Permission.SUBMIT)).isNotNull();
accessSection.remove(submitPermission); accessSection.remove(submitPermission);
assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull(); assertThat(accessSection.build().getPermission(Permission.SUBMIT)).isNull();
assertThat(accessSection.getPermissions()) assertThat(accessSection.build().getPermissions())
.containsExactly(abandonPermission, rebasePermission) .containsExactly(abandonPermission.build(), rebasePermission.build())
.inOrder(); .inOrder();
assertThrows(NullPointerException.class, () -> accessSection.remove(null)); assertThrows(NullPointerException.class, () -> accessSection.remove(null));
} }
@Test @Test
public void removePermissionByName() { public void removePermissionByName() {
Permission abandonPermission = new Permission(Permission.ABANDON); Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
Permission rebasePermission = new Permission(Permission.REBASE); Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
Permission submitPermission = new Permission(Permission.SUBMIT); Permission.Builder submitPermission = Permission.builder(Permission.SUBMIT);
accessSection.setPermissions( accessSection.addPermission(abandonPermission);
ImmutableList.of(abandonPermission, rebasePermission, submitPermission)); accessSection.addPermission(rebasePermission);
assertThat(accessSection.getPermission(Permission.SUBMIT)).isNotNull(); accessSection.addPermission(submitPermission);
AccessSection builtAccessSection = accessSection.build();
assertThat(builtAccessSection.getPermission(Permission.SUBMIT)).isNotNull();
accessSection.removePermission(Permission.SUBMIT); accessSection.removePermission(Permission.SUBMIT);
assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull(); builtAccessSection = accessSection.build();
assertThat(accessSection.getPermissions()) assertThat(builtAccessSection.getPermission(Permission.SUBMIT)).isNull();
.containsExactly(abandonPermission, rebasePermission) assertThat(builtAccessSection.getPermissions())
.containsExactly(abandonPermission.build(), rebasePermission.build())
.inOrder(); .inOrder();
assertThrows(NullPointerException.class, () -> accessSection.removePermission(null)); assertThrows(NullPointerException.class, () -> accessSection.removePermission(null));
@@ -188,62 +194,49 @@ public class AccessSectionTest {
@Test @Test
public void removePermissionByNameOtherCase() { public void removePermissionByNameOtherCase() {
Permission abandonPermission = new Permission(Permission.ABANDON); Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
Permission rebasePermission = new Permission(Permission.REBASE); Permission.Builder rebasePermission = Permission.builder(Permission.REBASE);
String submitLowerCase = Permission.SUBMIT.toLowerCase(Locale.US); String submitLowerCase = Permission.SUBMIT.toLowerCase(Locale.US);
String submitUpperCase = Permission.SUBMIT.toUpperCase(Locale.US); String submitUpperCase = Permission.SUBMIT.toUpperCase(Locale.US);
Permission submitPermissionLowerCase = new Permission(submitLowerCase); Permission.Builder submitPermissionLowerCase = Permission.builder(submitLowerCase);
accessSection.setPermissions( accessSection.addPermission(abandonPermission);
ImmutableList.of(abandonPermission, rebasePermission, submitPermissionLowerCase)); accessSection.addPermission(rebasePermission);
assertThat(accessSection.getPermission(submitLowerCase)).isNotNull(); accessSection.addPermission(submitPermissionLowerCase);
assertThat(accessSection.getPermission(submitUpperCase)).isNotNull(); AccessSection builtAccessSection = accessSection.build();
assertThat(builtAccessSection.getPermission(submitLowerCase)).isNotNull();
assertThat(builtAccessSection.getPermission(submitUpperCase)).isNotNull();
accessSection.removePermission(submitUpperCase); accessSection.removePermission(submitUpperCase);
assertThat(accessSection.getPermission(submitLowerCase)).isNull(); builtAccessSection = accessSection.build();
assertThat(accessSection.getPermission(submitUpperCase)).isNull(); assertThat(builtAccessSection.getPermission(submitLowerCase)).isNull();
assertThat(accessSection.getPermissions()) assertThat(builtAccessSection.getPermission(submitUpperCase)).isNull();
.containsExactly(abandonPermission, rebasePermission) assertThat(builtAccessSection.getPermissions())
.containsExactly(abandonPermission.build(), rebasePermission.build())
.inOrder(); .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 @Test
public void testEquals() { public void testEquals() {
Permission abandonPermission = new Permission(Permission.ABANDON); Permission.Builder abandonPermission = Permission.builder(Permission.ABANDON);
Permission rebasePermission = new Permission(Permission.REBASE); 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"); AccessSection builtAccessSection = accessSection.build();
accessSectionSamePermissionsOtherRef.setPermissions( AccessSection.Builder accessSectionSamePermissionsOtherRef =
ImmutableList.of(abandonPermission, rebasePermission)); AccessSection.builder("refs/heads/other");
assertThat(accessSection.equals(accessSectionSamePermissionsOtherRef)).isFalse(); accessSectionSamePermissionsOtherRef.addPermission(abandonPermission);
accessSectionSamePermissionsOtherRef.addPermission(rebasePermission);
assertThat(builtAccessSection.equals(accessSectionSamePermissionsOtherRef.build())).isFalse();
AccessSection accessSectionOther = new AccessSection(REF_PATTERN); AccessSection.Builder accessSectionOther = AccessSection.builder(REF_PATTERN);
accessSectionOther.setPermissions(ImmutableList.of(abandonPermission)); accessSectionOther.addPermission(abandonPermission);
assertThat(accessSection.equals(accessSectionOther)).isFalse(); assertThat(builtAccessSection.equals(accessSectionOther.build())).isFalse();
accessSectionOther.addPermission(rebasePermission); accessSectionOther.addPermission(rebasePermission);
assertThat(accessSection.equals(accessSectionOther)).isTrue(); assertThat(builtAccessSection.equals(accessSectionOther.build())).isTrue();
} }
} }

255
javatests/com/google/gerrit/common/data/PermissionTest.java
View File

@@ -16,21 +16,18 @@ package com.google.gerrit.common.data;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.entities.AccountGroup;
import java.util.ArrayList;
import java.util.List;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
public class PermissionTest { public class PermissionTest {
private static final String PERMISSION_NAME = "foo"; private static final String PERMISSION_NAME = "foo";
private Permission permission; private Permission.Builder permission;
@Before @Before
public void setup() { public void setup() {
this.permission = new Permission(PERMISSION_NAME); this.permission = Permission.builder(PERMISSION_NAME);
} }
@Test @Test
@@ -117,209 +114,179 @@ public class PermissionTest {
@Test @Test
public void getLabel() { public void getLabel() {
assertThat(new Permission(Permission.LABEL + "Code-Review").getLabel()) assertThat(Permission.create(Permission.LABEL + "Code-Review").getLabel())
.isEqualTo("Code-Review"); .isEqualTo("Code-Review");
assertThat(new Permission(Permission.LABEL_AS + "Code-Review").getLabel()) assertThat(Permission.create(Permission.LABEL_AS + "Code-Review").getLabel())
.isEqualTo("Code-Review"); .isEqualTo("Code-Review");
assertThat(new Permission("Code-Review").getLabel()).isNull(); assertThat(Permission.create("Code-Review").getLabel()).isNull();
assertThat(new Permission(Permission.ABANDON).getLabel()).isNull(); assertThat(Permission.create(Permission.ABANDON).getLabel()).isNull();
} }
@Test @Test
public void exclusiveGroup() { public void exclusiveGroup() {
assertThat(permission.getExclusiveGroup()).isFalse(); assertThat(permission.build().getExclusiveGroup()).isFalse();
permission.setExclusiveGroup(true); permission.setExclusiveGroup(true);
assertThat(permission.getExclusiveGroup()).isTrue(); assertThat(permission.build().getExclusiveGroup()).isTrue();
permission.setExclusiveGroup(false); permission.setExclusiveGroup(false);
assertThat(permission.getExclusiveGroup()).isFalse(); assertThat(permission.build().getExclusiveGroup()).isFalse();
} }
@Test @Test
public void noExclusiveGroupOnOwnerPermission() { public void noExclusiveGroupOnOwnerPermission() {
Permission permission = new Permission(Permission.OWNER); Permission permission = Permission.create(Permission.OWNER);
assertThat(permission.getExclusiveGroup()).isFalse(); assertThat(permission.getExclusiveGroup()).isFalse();
permission.setExclusiveGroup(true); permission = permission.toBuilder().setExclusiveGroup(true).build();
assertThat(permission.getExclusiveGroup()).isFalse(); assertThat(permission.getExclusiveGroup()).isFalse();
} }
@Test @Test
public void getEmptyRules() { public void getEmptyRules() {
assertThat(permission.getRules()).isNotNull(); assertThat(permission.getRulesBuilders()).isNotNull();
assertThat(permission.getRules()).isEmpty(); assertThat(permission.getRulesBuilders()).isEmpty();
} }
@Test @Test
public void setAndGetRules() { public void setAndGetRules() {
PermissionRule permissionRule1 = PermissionRule.Builder permissionRule1 =
PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
PermissionRule permissionRule2 = PermissionRule.Builder permissionRule2 =
PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
permission.setRules(ImmutableList.of(permissionRule1, permissionRule2)); permission.add(permissionRule1);
assertThat(permission.getRules()).containsExactly(permissionRule1, permissionRule2).inOrder(); permission.add(permissionRule2);
assertThat(permission.getRulesBuilders())
.containsExactly(permissionRule1, permissionRule2)
.inOrder();
PermissionRule permissionRule3 = PermissionRule.Builder permissionRule3 =
PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-3"), "group3")); PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"));
permission.setRules(ImmutableList.of(permissionRule3)); permission.modifyRules(
assertThat(permission.getRules()).containsExactly(permissionRule3); rules -> {
} rules.clear();
rules.add(permissionRule3);
@Test });
public void cannotAddPermissionByModifyingListThatWasProvidedToAccessSection() { assertThat(permission.getRulesBuilders()).containsExactly(permissionRule3);
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();
} }
@Test @Test
public void getNonExistingRule() { public void getNonExistingRule() {
GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"); GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1");
assertThat(permission.getRule(groupReference)).isNull(); assertThat(permission.build().getRule(groupReference)).isNull();
assertThat(permission.getRule(groupReference, false)).isNull(); assertThat(permission.build().getRule(groupReference)).isNull();
} }
@Test @Test
public void getRule() { public void getRule() {
GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"); GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1");
PermissionRule permissionRule = PermissionRule.create(groupReference); PermissionRule.Builder permissionRule = PermissionRule.builder(groupReference);
permission.setRules(ImmutableList.of(permissionRule)); permission.add(permissionRule);
assertThat(permission.getRule(groupReference)).isEqualTo(permissionRule); assertThat(permission.build().getRule(groupReference)).isEqualTo(permissionRule.build());
}
@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));
} }
@Test @Test
public void addRule() { public void addRule() {
PermissionRule permissionRule1 = PermissionRule.Builder permissionRule1 =
PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
PermissionRule permissionRule2 = PermissionRule.Builder permissionRule2 =
PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
permission.setRules(ImmutableList.of(permissionRule1, permissionRule2)); permission.add(permissionRule1);
permission.add(permissionRule2);
GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"); 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); permission.add(permissionRule3);
assertThat(permission.getRule(groupReference3)).isEqualTo(permissionRule3); assertThat(permission.build().getRule(groupReference3)).isEqualTo(permissionRule3.build());
assertThat(permission.getRules()) assertThat(permission.build().getRules())
.containsExactly(permissionRule1, permissionRule2, permissionRule3) .containsExactly(permissionRule1.build(), permissionRule2.build(), permissionRule3.build())
.inOrder(); .inOrder();
} }
@Test @Test
public void removeRule() { public void removeRule() {
PermissionRule permissionRule1 = PermissionRule.Builder permissionRule1 =
PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
PermissionRule permissionRule2 = PermissionRule.Builder permissionRule2 =
PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2"));
GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"); 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)); permission.add(permissionRule1);
assertThat(permission.getRule(groupReference3)).isNotNull(); permission.add(permissionRule2);
permission.add(permissionRule3);
assertThat(permission.build().getRule(groupReference3)).isNotNull();
permission.remove(permissionRule3); permission.remove(permissionRule3.build());
assertThat(permission.getRule(groupReference3)).isNull(); assertThat(permission.build().getRule(groupReference3)).isNull();
assertThat(permission.getRules()).containsExactly(permissionRule1, permissionRule2).inOrder(); assertThat(permission.build().getRules())
} .containsExactly(permissionRule1.build(), permissionRule2.build())
@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)
.inOrder(); .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 @Test
public void testEquals() { public void testEquals() {
PermissionRule permissionRule1 = PermissionRule.Builder permissionRule1 =
PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule.builder(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"));
PermissionRule permissionRule2 = PermissionRule.Builder permissionRule2 =
PermissionRule.create(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); 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"); Permission.Builder permissionSameRulesOtherName = Permission.builder("bar");
permissionSameRulesOtherName.setRules(ImmutableList.of(permissionRule1, permissionRule2)); permissionSameRulesOtherName.add(permissionRule1);
permissionSameRulesOtherName.add(permissionRule2);
assertThat(permission.equals(permissionSameRulesOtherName)).isFalse(); assertThat(permission.equals(permissionSameRulesOtherName)).isFalse();
Permission permissionSameRulesSameNameOtherExclusiveGroup = new Permission("foo"); Permission.Builder permissionSameRulesSameNameOtherExclusiveGroup = Permission.builder("foo");
permissionSameRulesSameNameOtherExclusiveGroup.setRules( permissionSameRulesSameNameOtherExclusiveGroup.add(permissionRule1);
ImmutableList.of(permissionRule1, permissionRule2)); permissionSameRulesSameNameOtherExclusiveGroup.add(permissionRule2);
permissionSameRulesSameNameOtherExclusiveGroup.setExclusiveGroup(true); permissionSameRulesSameNameOtherExclusiveGroup.setExclusiveGroup(true);
assertThat(permission.equals(permissionSameRulesSameNameOtherExclusiveGroup)).isFalse(); assertThat(permission.equals(permissionSameRulesSameNameOtherExclusiveGroup)).isFalse();
Permission permissionOther = new Permission(PERMISSION_NAME); Permission.Builder permissionOther = Permission.builder(PERMISSION_NAME);
permissionOther.setRules(ImmutableList.of(permissionRule1)); permissionOther.add(permissionRule1);
assertThat(permission.equals(permissionOther)).isFalse(); assertThat(permission.build().equals(permissionOther.build())).isFalse();
permissionOther.add(permissionRule2); permissionOther.add(permissionRule2);
assertThat(permission.equals(permissionOther)).isTrue(); assertThat(permission.build().equals(permissionOther.build())).isTrue();
} }
} }

13
javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
View File

@@ -98,10 +98,15 @@ public class LabelNormalizerTest {
private void configureProject() throws Exception { private void configureProject() throws Exception {
ProjectConfig pc = loadAllProjects(); ProjectConfig pc = loadAllProjects();
for (AccessSection sec : pc.getAccessSections()) {
for (String label : pc.getLabelSections().keySet()) { for (AccessSection sec : ImmutableList.copyOf(pc.getAccessSections())) {
sec.removePermission(forLabel(label)); pc.upsertAccessSection(
} sec.getName(),
updatedSection -> {
for (String label : pc.getLabelSections().keySet()) {
updatedSection.removePermission(forLabel(label));
}
});
} }
LabelType lt = LabelType lt =
label("Verified", value(1, "Verified"), value(0, "No score"), value(-1, "Fails")); label("Verified", value(1, "Verified"), value(0, "No score"), value(-1, "Fails"));

18
javatests/com/google/gerrit/server/project/ProjectConfigTest.java
View File

@@ -303,12 +303,15 @@ public class ProjectConfigTest {
update(rev); update(rev);
ProjectConfig cfg = read(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() cfg.getAccountsSection()
.setSameGroupVisibility( .setSameGroupVisibility(
Collections.singletonList(PermissionRule.create(cfg.resolve(staff)))); 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(); ContributorAgreement.Builder ca = cfg.getContributorAgreement("Individual").toBuilder();
ca.setAccepted(ImmutableList.of(PermissionRule.create(cfg.resolve(staff)))); ca.setAccepted(ImmutableList.of(PermissionRule.create(cfg.resolve(staff))));
ca.setAutoVerify(null); ca.setAutoVerify(null);
@@ -423,9 +426,12 @@ public class ProjectConfigTest {
update(rev); update(rev);
ProjectConfig cfg = read(rev); ProjectConfig cfg = read(rev);
AccessSection section = cfg.getAccessSection("refs/heads/*"); cfg.upsertAccessSection(
Permission submit = section.getPermission(Permission.SUBMIT); "refs/heads/*",
submit.add(PermissionRule.create(cfg.resolve(staff))); section -> {
Permission.Builder submit = section.upsertPermission(Permission.SUBMIT);
submit.add(PermissionRule.builder(cfg.resolve(staff)));
});
rev = commit(cfg); rev = commit(cfg);
assertThat(text(rev, "project.config")) assertThat(text(rev, "project.config"))
.isEqualTo( .isEqualTo(

18
javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -44,7 +44,6 @@ import com.google.common.truth.ThrowableSubject;
import com.google.gerrit.acceptance.config.GerritConfig; import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.Nullable; 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.GroupReference;
import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.Permission;
@@ -1859,13 +1858,16 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) { try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
md.setMessage(String.format("Grant %s on %s", permission, ref)); md.setMessage(String.format("Grant %s on %s", permission, ref));
ProjectConfig config = projectConfigFactory.read(md); ProjectConfig config = projectConfigFactory.read(md);
AccessSection s = config.getAccessSection(ref, true); config.upsertAccessSection(
Permission p = s.getPermission(permission, true); ref,
PermissionRule rule = s -> {
PermissionRule.builder(GroupReference.create(groupUUID, groupUUID.get())) Permission.Builder p = s.upsertPermission(permission);
.setForce(force) PermissionRule.Builder rule =
.build(); PermissionRule.builder(GroupReference.create(groupUUID, groupUUID.get()))
p.add(rule); .setForce(force);
p.add(rule);
});
config.commit(md); config.commit(md);
projectCache.evict(config.getProject()); projectCache.evict(config.getProject());
} }