From 6a765190df238f60674804e8c541ae210757c407 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Wed, 5 Jan 2011 12:46:21 -0800 Subject: [PATCH] Move "ref_rights" table into Git Permissions are stored in the project.config file within the refs/meta/config branch of each project. This makes the rules more flexible in the future, as well as adds version control. For example: [access "refs/*"] owner = group tools-owners [access "refs/heads/*"] label-Verified = -1..+1 group tools-dev label-Verified = -1..+1 group tools-owners label-Code-Review = -2..+2 group tools-owners submit = group tools-dev submit = group tools-owners [access "refs/heads/stable"] exclusiveGroupPermissions = read create push read = group Anonymous Users push = group tools-repo-maintainer To enable easy remote editing of the configuration rules, the following access block is added by default to -- All Projects -- and is thus inherited throughout the entire site: [access "refs/meta/config"] read = group Project Owners push = group Project Owners This configuration section permits any project owner or site administrator (as they are indirectly always a project owner of any project) to push changes to the project.config file within the refs/meta/config branch, updating access (and other project information) remotely without using the web UI. Change-Id: Idb56f657a4bf88108ad40bbb19d831e6806b68c5 Signed-off-by: Shawn O. Pearce --- .../gerrit/common/data/AccessSection.java | 129 ++++ .../gerrit/common/data/ApprovalType.java | 16 + .../gerrit/common/data/ApprovalTypes.java | 49 +- .../gerrit/common/data/ChangeDetail.java | 18 +- .../gerrit/common/data/GroupReference.java | 76 +++ .../common/data/PatchSetPublishDetail.java | 53 +- .../google/gerrit/common/data/Permission.java | 190 ++++++ .../gerrit/common/data/PermissionRange.java | 95 +++ .../gerrit/common/data/PermissionRule.java | 182 ++++++ .../PatchSetComplexDisclosurePanel.java | 11 +- .../client/changes/PublishCommentScreen.java | 27 +- .../rpc/changedetail/ChangeDetailFactory.java | 13 +- .../PatchSetPublishDetailFactory.java | 77 +-- .../rpc/patch/PatchDetailServiceImpl.java | 4 +- .../gerrit/reviewdb/ApprovalCategory.java | 78 +-- .../com/google/gerrit/reviewdb/RefRight.java | 246 -------- .../gerrit/reviewdb/RefRightAccess.java | 33 - .../com/google/gerrit/reviewdb/ReviewDb.java | 3 - .../google/gerrit/reviewdb/index_generic.sql | 8 - .../google/gerrit/reviewdb/index_postgres.sql | 8 - .../gerrit/common/ChangeHookRunner.java | 2 +- .../server/config/ApprovalTypesProvider.java | 14 +- .../gerrit/server/events/EventFactory.java | 2 +- .../server/git/CreateCodeReviewNotes.java | 11 +- .../com/google/gerrit/server/git/MergeOp.java | 2 +- .../gerrit/server/git/ProjectConfig.java | 249 ++++++++ .../gerrit/server/git/ReceiveCommits.java | 2 +- .../server/git/ReviewNoteHeaderFormatter.java | 3 +- .../gerrit/server/git/VersionedMetaData.java | 12 + .../gerrit/server/patch/PublishComments.java | 2 +- .../gerrit/server/project/ChangeControl.java | 46 +- .../gerrit/server/project/ProjectCache.java | 3 - .../server/project/ProjectCacheImpl.java | 36 +- .../gerrit/server/project/ProjectControl.java | 80 ++- .../gerrit/server/project/ProjectState.java | 209 ++----- .../gerrit/server/project/RefControl.java | 586 ++++++++---------- .../server/query/change/LabelPredicate.java | 52 +- .../gerrit/server/schema/SchemaCreator.java | 192 ++---- .../gerrit/server/schema/Schema_53.java | 309 ++++++++- .../server/workflow/CategoryFunction.java | 28 +- .../gerrit/server/workflow/FunctionState.java | 32 +- .../server/workflow/SubmitFunction.java | 68 -- .../gerrit/server/git/ProjectConfigTest.java | 191 ++++++ .../gerrit/server/project/RefControlTest.java | 165 ++--- .../server/schema/SchemaCreatorTest.java | 150 ----- .../gerrit/sshd/commands/AdminSetParent.java | 4 - .../gerrit/sshd/commands/CreateProject.java | 41 +- .../gerrit/sshd/commands/ReviewCommand.java | 2 +- 48 files changed, 2181 insertions(+), 1628 deletions(-) create mode 100644 gerrit-common/src/main/java/com/google/gerrit/common/data/AccessSection.java create mode 100644 gerrit-common/src/main/java/com/google/gerrit/common/data/GroupReference.java create mode 100644 gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java create mode 100644 gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRange.java create mode 100644 gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java delete mode 100644 gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRight.java delete mode 100644 gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRightAccess.java delete mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java create mode 100644 gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/AccessSection.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/AccessSection.java new file mode 100644 index 0000000000..dc87753910 --- /dev/null +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/AccessSection.java @@ -0,0 +1,129 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.common.data; + +import com.google.gerrit.reviewdb.Project; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Set; + +/** Portion of a {@link Project} describing access rules. */ +public class AccessSection implements Comparable { + /** Pattern that matches all references in a project. */ + public static final String ALL = "refs/*"; + + /** Pattern that matches all branches in a project. */ + public static final String HEADS = "refs/heads/*"; + + /** Prefix that triggers a regular expression pattern. */ + public static final String REGEX_PREFIX = "^"; + + /** @return true if the name is likely to be a valid access section name. */ + public static boolean isAccessSection(String name) { + return name.startsWith("refs/") || name.startsWith("^refs/"); + } + + protected String refPattern; + protected List permissions; + + protected AccessSection() { + } + + public AccessSection(String refPattern) { + setRefPattern(refPattern); + } + + public String getRefPattern() { + return refPattern; + } + + public void setRefPattern(String refPattern) { + this.refPattern = refPattern; + } + + public List getPermissions() { + if (permissions == null) { + permissions = new ArrayList(); + } + return permissions; + } + + public void setPermissions(List list) { + Set names = new HashSet(); + for (Permission p : list) { + if (!names.add(p.getName().toLowerCase())) { + throw new IllegalArgumentException(); + } + } + + permissions = list; + } + + public Permission getPermission(String name) { + return getPermission(name, false); + } + + public Permission getPermission(String name, boolean create) { + for (Permission p : getPermissions()) { + if (p.getName().equalsIgnoreCase(name)) { + return p; + } + } + + if (create) { + Permission p = new Permission(name); + permissions.add(p); + return p; + } else { + return null; + } + } + + public void remove(Permission permission) { + if (permission != null) { + removePermission(permission.getName()); + } + } + + public void removePermission(String name) { + if (permissions != null) { + for (Iterator itr = permissions.iterator(); itr.hasNext();) { + if (name.equalsIgnoreCase(itr.next().getName())) { + itr.remove(); + } + } + } + } + + @Override + public int compareTo(AccessSection o) { + return comparePattern().compareTo(o.comparePattern()); + } + + private String comparePattern() { + if (getRefPattern().startsWith(REGEX_PREFIX)) { + return getRefPattern().substring(REGEX_PREFIX.length()); + } + return getRefPattern(); + } + + @Override + public String toString() { + return "AccessSection[" + getRefPattern() + "]"; + } +} diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalType.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalType.java index ea9aedb750..7d03457278 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalType.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalType.java @@ -31,6 +31,7 @@ public class ApprovalType { protected short maxNegative; protected short maxPositive; + private transient List intList; private transient Map byValue; protected ApprovalType() { @@ -56,6 +57,9 @@ public class ApprovalType { maxPositive = values.get(values.size() - 1).getValue(); } } + + // Force the label name to pre-compute so we don't have data race conditions. + getCategory().getLabelName(); } public ApprovalCategory getCategory() { @@ -107,4 +111,16 @@ public class ApprovalType { } } } + + public List getValuesAsList() { + if (intList == null) { + intList = new ArrayList(values.size()); + for (ApprovalCategoryValue acv : values) { + intList.add(Integer.valueOf(acv.getValue())); + } + Collections.sort(intList); + Collections.reverse(intList); + } + return intList; + } } diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalTypes.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalTypes.java index 1b6d4a3289..0518010ba1 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalTypes.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalTypes.java @@ -19,20 +19,17 @@ import com.google.gerrit.reviewdb.ApprovalCategory; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; public class ApprovalTypes { protected List approvalTypes; - protected List actionTypes; - private transient Map byCategoryId; + private transient Map byId; + private transient Map byLabel; protected ApprovalTypes() { } - public ApprovalTypes(final List approvals, - final List actions) { + public ApprovalTypes(final List approvals) { approvalTypes = approvals; - actionTypes = actions; byCategory(); } @@ -40,33 +37,35 @@ public class ApprovalTypes { return approvalTypes; } - public List getActionTypes() { - return actionTypes; - } - - public ApprovalType getApprovalType(final ApprovalCategory.Id id) { + public ApprovalType byId(final ApprovalCategory.Id id) { return byCategory().get(id); } - public Set getApprovalCategories() { - return byCategory().keySet(); - } - private Map byCategory() { - if (byCategoryId == null) { - byCategoryId = new HashMap(); - if (actionTypes != null) { - for (final ApprovalType t : actionTypes) { - byCategoryId.put(t.getCategory().getId(), t); - } - } - + if (byId == null) { + byId = new HashMap(); if (approvalTypes != null) { for (final ApprovalType t : approvalTypes) { - byCategoryId.put(t.getCategory().getId(), t); + byId.put(t.getCategory().getId(), t); } } } - return byCategoryId; + return byId; + } + + public ApprovalType byLabel(String labelName) { + return byLabel().get(labelName.toLowerCase()); + } + + private Map byLabel() { + if (byLabel == null) { + byLabel = new HashMap(); + if (approvalTypes != null) { + for (ApprovalType t : approvalTypes) { + byLabel.put(t.getCategory().getLabelName().toLowerCase(), t); + } + } + } + return byLabel; } } diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java index 572fd7aad7..f196c058f8 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java @@ -39,10 +39,10 @@ public class ChangeDetail { protected List patchSets; protected List approvals; protected Set missingApprovals; + protected boolean canSubmit; protected List messages; protected PatchSet.Id currentPatchSetId; protected PatchSetDetail currentDetail; - protected Set currentActions; public ChangeDetail() { } @@ -87,6 +87,14 @@ public class ChangeDetail { canRevert = a; } + public boolean canSubmit() { + return canSubmit; + } + + public void setCanSubmit(boolean a) { + canSubmit = a; + } + public Change getChange() { return change; } @@ -153,14 +161,6 @@ public class ChangeDetail { missingApprovals = a; } - public Set getCurrentActions() { - return currentActions; - } - - public void setCurrentActions(Set a) { - currentActions = a; - } - public boolean isCurrentPatchSet(final PatchSetDetail detail) { return currentPatchSetId != null && detail.getPatchSet().getId().equals(currentPatchSetId); diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupReference.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupReference.java new file mode 100644 index 0000000000..206be0ed0d --- /dev/null +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupReference.java @@ -0,0 +1,76 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.common.data; + +import com.google.gerrit.reviewdb.AccountGroup; + +/** Describes a group within a projects {@link AccessSection}s. */ +public class GroupReference implements Comparable { + /** @return a new reference to the given group description. */ + public static GroupReference forGroup(AccountGroup group) { + return new GroupReference(group.getGroupUUID(), group.getName()); + } + + protected String uuid; + protected String name; + + protected GroupReference() { + } + + public GroupReference(AccountGroup.UUID uuid, String name) { + setUUID(uuid); + setName(name); + } + + public AccountGroup.UUID getUUID() { + return uuid != null ? new AccountGroup.UUID(uuid) : null; + } + + public void setUUID(AccountGroup.UUID newUUID) { + uuid = newUUID.get(); + } + + public String getName() { + return name; + } + + public void setName(String newName) { + this.name = newName; + } + + @Override + public int compareTo(GroupReference o) { + return uuid(this).compareTo(uuid(o)); + } + + private static String uuid(GroupReference a) { + return a.getUUID() != null ? a.getUUID().get() : "?"; + } + + @Override + public int hashCode() { + return uuid(this).hashCode(); + } + + @Override + public boolean equals(Object o) { + return o instanceof GroupReference && compareTo((GroupReference) o) == 0; + } + + @Override + public String toString() { + return "Group[" + getName() + " / " + getUUID() + "]"; + } +} diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java index 7a0ada3029..273f18d582 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java @@ -15,39 +15,35 @@ package com.google.gerrit.common.data; import com.google.gerrit.reviewdb.ApprovalCategory; -import com.google.gerrit.reviewdb.ApprovalCategoryValue; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.PatchLineComment; import com.google.gerrit.reviewdb.PatchSetApproval; import com.google.gerrit.reviewdb.PatchSetInfo; import java.util.List; -import java.util.Map; -import java.util.Set; public class PatchSetPublishDetail { protected AccountInfoCache accounts; protected PatchSetInfo patchSetInfo; protected Change change; protected List drafts; - protected Map> allowed; - protected Map given; - protected boolean isSubmitAllowed; + protected List labels; + protected List given; + protected boolean canSubmit; - public Map> getAllowed() { - return allowed; + public List getLabels() { + return labels; } - public void setAllowed( - Map> allowed) { - this.allowed = allowed; + public void setLabels(List labels) { + this.labels = labels; } - public Map getGiven() { + public List getGiven() { return given; } - public void setGiven(Map given) { + public void setGiven(List given) { this.given = given; } @@ -67,8 +63,8 @@ public class PatchSetPublishDetail { this.drafts = drafts; } - public void setSubmitAllowed(boolean allowed) { - isSubmitAllowed = allowed; + public void setCanSubmit(boolean allowed) { + canSubmit = allowed; } public AccountInfoCache getAccounts() { @@ -87,20 +83,25 @@ public class PatchSetPublishDetail { return drafts; } - public boolean isAllowed(final ApprovalCategory.Id id) { - final Set s = getAllowed(id); - return s != null && !s.isEmpty(); + public PermissionRange getRange(final String permissionName) { + for (PermissionRange s : labels) { + if (s.getName().equals(permissionName)) { + return s; + } + } + return null; } - public Set getAllowed(final ApprovalCategory.Id id) { - return allowed.get(id); + public PatchSetApproval getChangeApproval(ApprovalCategory.Id id) { + for (PatchSetApproval a : given) { + if (a.getCategoryId().equals(id)) { + return a; + } + } + return null; } - public PatchSetApproval getChangeApproval(final ApprovalCategory.Id id) { - return given.get(id); - } - - public boolean isSubmitAllowed() { - return isSubmitAllowed; + public boolean canSubmit() { + return canSubmit; } } diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java new file mode 100644 index 0000000000..19a8737a38 --- /dev/null +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/Permission.java @@ -0,0 +1,190 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.common.data; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +/** A single permission within an {@link AccessSection} of a project. */ +public class Permission implements Comparable { + public static final String CREATE = "create"; + public static final String FORGE_AUTHOR = "forgeAuthor"; + public static final String FORGE_COMMITTER = "forgeCommitter"; + public static final String FORGE_SERVER = "forgeServerAsCommitter"; + public static final String LABEL = "label-"; + public static final String OWNER = "owner"; + public static final String PUSH = "push"; + public static final String PUSH_MERGE = "pushMerge"; + public static final String PUSH_TAG = "pushTag"; + public static final String READ = "read"; + public static final String SUBMIT = "submit"; + + private static final List NAMES_LC; + + static { + NAMES_LC = new ArrayList(); + NAMES_LC.add(OWNER.toLowerCase()); + NAMES_LC.add(READ.toLowerCase()); + NAMES_LC.add(CREATE.toLowerCase()); + NAMES_LC.add(FORGE_AUTHOR.toLowerCase()); + NAMES_LC.add(FORGE_COMMITTER.toLowerCase()); + NAMES_LC.add(FORGE_SERVER.toLowerCase()); + NAMES_LC.add(PUSH.toLowerCase()); + NAMES_LC.add(PUSH_MERGE.toLowerCase()); + NAMES_LC.add(PUSH_TAG.toLowerCase()); + NAMES_LC.add(LABEL.toLowerCase()); + NAMES_LC.add(SUBMIT.toLowerCase()); + } + + /** @return true if the name is recognized as a permission name. */ + public static boolean isPermission(String varName) { + String lc = varName.toLowerCase(); + if (lc.startsWith(LABEL)) { + return LABEL.length() < lc.length(); + } + return NAMES_LC.contains(lc); + } + + /** @return true if the permission name is actually for a review label. */ + public static boolean isLabel(String varName) { + return varName.startsWith(LABEL) && LABEL.length() < varName.length(); + } + + /** @return permission name for the given review label. */ + public static String forLabel(String labelName) { + return LABEL + labelName; + } + + protected String name; + protected boolean exclusiveGroup; + protected List rules; + + protected Permission() { + } + + public Permission(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public boolean isLabel() { + return isLabel(getName()); + } + + public String getLabel() { + if (isLabel()) { + return getName().substring(LABEL.length()); + } + return null; + } + + public boolean getExclusiveGroup() { + // Only permit exclusive group behavior on non OWNER permissions, + // otherwise an owner might lose access to a delegated subspace. + // + return exclusiveGroup && !OWNER.equals(getName()); + } + + public void setExclusiveGroup(boolean newExclusiveGroup) { + exclusiveGroup = newExclusiveGroup; + } + + public List getRules() { + initRules(); + return rules; + } + + public void setRules(List list) { + rules = 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) { + for (Iterator itr = rules.iterator(); itr.hasNext();) { + if (sameGroup(itr.next(), group)) { + itr.remove(); + } + } + } + } + + public PermissionRule getRule(GroupReference group) { + return getRule(group, false); + } + + public PermissionRule getRule(GroupReference group, boolean create) { + initRules(); + + for (PermissionRule r : rules) { + if (sameGroup(r, group)) { + return r; + } + } + + if (create) { + PermissionRule r = new PermissionRule(group); + rules.add(r); + return r; + } else { + return null; + } + } + + private static boolean sameGroup(PermissionRule rule, GroupReference group) { + if (group.getUUID() != null) { + return group.getUUID().equals(rule.getGroup().getUUID()); + + } else if (group.getName() != null) { + return group.getName().equals(rule.getGroup().getName()); + + } else { + return false; + } + } + + private void initRules() { + if (rules == null) { + rules = new ArrayList(4); + } + } + + @Override + public int compareTo(Permission b) { + int cmp = index(this) - index(b); + if (cmp == 0) getName().compareTo(b.getName()); + return cmp; + } + + private static int index(Permission a) { + String lc = a.isLabel() ? Permission.LABEL : a.getName().toLowerCase(); + int index = NAMES_LC.indexOf(lc); + return 0 <= index ? index : NAMES_LC.size(); + } +} diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRange.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRange.java new file mode 100644 index 0000000000..bc2dadde76 --- /dev/null +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRange.java @@ -0,0 +1,95 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.common.data; + +public class PermissionRange implements Comparable { + protected String name; + protected int min; + protected int max; + + protected PermissionRange() { + } + + public PermissionRange(String name, int min, int max) { + this.name = name; + + if (min <= max) { + this.min = min; + this.max = max; + } else { + this.min = max; + this.max = min; + } + } + + public String getName() { + return name; + } + + public boolean isLabel() { + return Permission.isLabel(getName()); + } + + public String getLabel() { + return isLabel() ? getName().substring(Permission.LABEL.length()) : null; + } + + public int getMin() { + return min; + } + + public int getMax() { + return max; + } + + /** True if the value is within the range. */ + public boolean contains(int value) { + return getMin() <= value && value <= getMax(); + } + + /** Normalize the value to fit within the bounds of the range. */ + public int squash(int value) { + return Math.min(Math.max(getMin(), value), getMax()); + } + + /** True both {@link #getMin()} and {@link #getMax()} are 0. */ + public boolean isEmpty() { + return getMin() == 0 && getMax() == 0; + } + + @Override + public int compareTo(PermissionRange o) { + return getName().compareTo(o.getName()); + } + + @Override + public String toString() { + StringBuilder r = new StringBuilder(); + if (getMin() < 0 && getMax() == 0) { + r.append(getMin()); + r.append(' '); + } else { + if (getMin() != getMax()) { + if (0 <= getMin()) r.append('+'); + r.append(getMin()); + r.append(".."); + } + if (0 <= getMax()) r.append('+'); + r.append(getMax()); + r.append(' '); + } + return r.toString(); + } +} diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java new file mode 100644 index 0000000000..dc07ace911 --- /dev/null +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java @@ -0,0 +1,182 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.common.data; + +public class PermissionRule implements Comparable { + protected boolean deny; + protected boolean force; + protected int min; + protected int max; + protected GroupReference group; + + public PermissionRule() { + } + + public PermissionRule(GroupReference group) { + this.group = group; + } + + public boolean getDeny() { + return deny; + } + + public void setDeny(boolean newDeny) { + deny = newDeny; + } + + public boolean getForce() { + return force; + } + + public void setForce(boolean newForce) { + force = newForce; + } + + public Integer getMin() { + return min; + } + + public void setMin(Integer min) { + this.min = min; + } + + public void setMax(Integer max) { + this.max = max; + } + + public Integer getMax() { + return max; + } + + public void setRange(int newMin, int newMax) { + if (newMax < newMin) { + min = newMax; + max = newMin; + } else { + min = newMin; + max = newMax; + } + } + + public GroupReference getGroup() { + return group; + } + + public void setGroup(GroupReference newGroup) { + group = newGroup; + } + + @Override + public int compareTo(PermissionRule o) { + int cmp = deny(this) - deny(o); + if (cmp == 0) cmp = range(o) - range(this); + if (cmp == 0) cmp = group(this).compareTo(group(o)); + return cmp; + } + + private static int deny(PermissionRule a) { + return a.getDeny() ? 1 : 0; + } + + private static int range(PermissionRule a) { + return Math.abs(a.getMin()) + Math.abs(a.getMax()); + } + + private static String group(PermissionRule a) { + return a.getGroup().getName() != null ? a.getGroup().getName() : ""; + } + + @Override + public String toString() { + return asString(true); + } + + public String asString(boolean canUseRange) { + StringBuilder r = new StringBuilder(); + + if (getDeny()) { + r.append("deny "); + } + + if (getForce()) { + r.append("+force "); + } + + if (canUseRange && (getMin() != 0 || getMax() != 0)) { + if (0 <= getMin()) r.append('+'); + r.append(getMin()); + r.append(".."); + if (0 <= getMax()) r.append('+'); + r.append(getMax()); + r.append(' '); + } + + r.append("group "); + r.append(getGroup().getName()); + + return r.toString(); + } + + public static PermissionRule fromString(String src, boolean mightUseRange) { + final String orig = src; + final PermissionRule rule = new PermissionRule(); + + src = src.trim(); + + if (src.startsWith("deny ")) { + rule.setDeny(true); + src = src.substring(5).trim(); + } + + if (src.startsWith("+force ")) { + rule.setForce(true); + src = src.substring("+force ".length()).trim(); + } + + if (mightUseRange && !src.startsWith("group ")) { + int sp = src.indexOf(' '); + String range = src.substring(0, sp); + + if (range.matches("^([+-]\\d+)\\.\\.([+-]\\d)$")) { + int dotdot = range.indexOf(".."); + int min = parseInt(range.substring(0, dotdot)); + int max = parseInt(range.substring(dotdot + 2)); + rule.setRange(min, max); + } else { + throw new IllegalArgumentException("Invalid range in rule: " + orig); + } + + src = src.substring(sp + 1).trim(); + } + + if (src.startsWith("group ")) { + src = src.substring(6).trim(); + GroupReference group = new GroupReference(); + group.setName(src); + rule.setGroup(group); + } else { + throw new IllegalArgumentException("Rule must include group: " + orig); + } + + return rule; + } + + private static int parseInt(String value) { + if (value.startsWith("+")) { + value = value.substring(1); + } + return Integer.parseInt(value); + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java index efa971b124..cba3168314 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java @@ -28,6 +28,8 @@ import com.google.gerrit.reviewdb.Account; import com.google.gerrit.reviewdb.AccountDiffPreference; import com.google.gerrit.reviewdb.AccountGeneralPreferences; import com.google.gerrit.reviewdb.ApprovalCategory; +import com.google.gerrit.reviewdb.AccountGeneralPreferences.DownloadCommand; +import com.google.gerrit.reviewdb.AccountGeneralPreferences.DownloadScheme; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.ChangeMessage; import com.google.gerrit.reviewdb.Patch; @@ -35,8 +37,6 @@ import com.google.gerrit.reviewdb.PatchSet; import com.google.gerrit.reviewdb.PatchSetInfo; import com.google.gerrit.reviewdb.Project; import com.google.gerrit.reviewdb.UserIdentity; -import com.google.gerrit.reviewdb.AccountGeneralPreferences.DownloadCommand; -import com.google.gerrit.reviewdb.AccountGeneralPreferences.DownloadScheme; import com.google.gwt.core.client.GWT; import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; @@ -54,7 +54,6 @@ import com.google.gwt.user.client.ui.Panel; import com.google.gwt.user.client.ui.HTMLTable.CellFormatter; import com.google.gwtexpui.clippy.client.CopyableLabel; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -401,12 +400,8 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O private void populateActions(final PatchSetDetail detail) { final boolean isOpen = changeDetail.getChange().getStatus().isOpen(); - Set allowed = changeDetail.getCurrentActions(); - if (allowed == null) { - allowed = Collections.emptySet(); - } - if (isOpen && allowed.contains(ApprovalCategory.SUBMIT)) { + if (isOpen && changeDetail.canSubmit()) { final Button b = new Button(Util.M .submitPatchSet(detail.getPatchSet().getPatchSetId())); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java index 0ed58b41d0..127009fb74 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java @@ -25,8 +25,11 @@ import com.google.gerrit.client.ui.PatchLink; import com.google.gerrit.client.ui.SmallHeading; import com.google.gerrit.common.PageLinks; import com.google.gerrit.common.data.ApprovalType; +import com.google.gerrit.common.data.ApprovalTypes; import com.google.gerrit.common.data.ChangeDetail; import com.google.gerrit.common.data.PatchSetPublishDetail; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.ApprovalCategoryValue; import com.google.gerrit.reviewdb.Change; @@ -218,16 +221,25 @@ public class PublishCommentScreen extends AccountScreen implements } private void initApprovals(final PatchSetPublishDetail r, final Panel body) { - for (final ApprovalType ct : Gerrit.getConfig().getApprovalTypes() - .getApprovalTypes()) { - if (r.isAllowed(ct.getCategory().getId())) { - initApprovalType(r, body, ct); + ApprovalTypes types = Gerrit.getConfig().getApprovalTypes(); + + for (ApprovalType type : types.getApprovalTypes()) { + String permission = Permission.forLabel(type.getCategory().getLabelName()); + PermissionRange range = r.getRange(permission); + if (range != null && !range.isEmpty()) { + initApprovalType(r, body, type, range); + } + } + + for (PermissionRange range : r.getLabels()) { + if (!range.isEmpty() && types.byLabel(range.getLabel()) == null) { + // TODO: this is a non-standard label. Offer it without the type. } } } private void initApprovalType(final PatchSetPublishDetail r, - final Panel body, final ApprovalType ct) { + final Panel body, final ApprovalType ct, final PermissionRange range) { body.add(new SmallHeading(ct.getCategory().getName() + ":")); final VerticalPanel vp = new VerticalPanel(); @@ -236,11 +248,10 @@ public class PublishCommentScreen extends AccountScreen implements new ArrayList(ct.getValues()); Collections.reverse(lst); final ApprovalCategory.Id catId = ct.getCategory().getId(); - final Set allowed = r.getAllowed(catId); final PatchSetApproval prior = r.getChangeApproval(catId); for (final ApprovalCategoryValue buttonValue : lst) { - if (!allowed.contains(buttonValue.getId())) { + if (!range.contains(buttonValue.getValue())) { continue; } @@ -306,7 +317,7 @@ public class PublishCommentScreen extends AccountScreen implements } } - submit.setVisible(r.isSubmitAllowed()); + submit.setVisible(r.canSubmit()); } private void onSend(final boolean submit) { diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java index 560b1552af..d2d4ce0ed4 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java @@ -33,6 +33,7 @@ import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountInfoCacheFactory; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; +import com.google.gerrit.server.project.CanSubmitResult; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.workflow.CategoryFunction; @@ -94,6 +95,7 @@ public class ChangeDetailFactory extends Handler { if (patch == null) { throw new NoSuchEntityException(); } + final CanSubmitResult canSubmitResult = control.canSubmit(patch.getId()); aic.want(change.getOwner()); @@ -103,6 +105,7 @@ public class ChangeDetailFactory extends Handler { detail.setCanAbandon(change.getStatus().isOpen() && control.canAbandon()); detail.setCanRestore(change.getStatus() == Change.Status.ABANDONED && control.canRestore()); + detail.setCanSubmit(canSubmitResult == CanSubmitResult.OK); detail.setStarred(control.getCurrentUser().getStarredChanges().contains( changeId)); @@ -141,23 +144,13 @@ public class ChangeDetailFactory extends Handler { final Set missingApprovals = new HashSet(); - final Set currentActions = - new HashSet(); - for (final ApprovalType at : approvalTypes.getApprovalTypes()) { CategoryFunction.forCategory(at.getCategory()).run(at, fs); if (!fs.isValid(at)) { missingApprovals.add(at.getCategory().getId()); } } - for (final ApprovalType at : approvalTypes.getActionTypes()) { - if (CategoryFunction.forCategory(at.getCategory()).isValid( - control.getCurrentUser(), at, fs)) { - currentActions.add(at.getCategory().getId()); - } - } detail.setMissingApprovals(missingApprovals); - detail.setCurrentActions(currentActions); } final boolean canRemoveReviewers = detail.getChange().getStatus().isOpen() // diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java index 9b03b70451..58daa09cd5 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java @@ -15,19 +15,14 @@ package com.google.gerrit.httpd.rpc.changedetail; import com.google.gerrit.common.data.AccountInfoCache; -import com.google.gerrit.common.data.ApprovalType; -import com.google.gerrit.common.data.ApprovalTypes; import com.google.gerrit.common.data.PatchSetPublishDetail; +import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.httpd.rpc.Handler; -import com.google.gerrit.reviewdb.AccountGroup; -import com.google.gerrit.reviewdb.ApprovalCategory; -import com.google.gerrit.reviewdb.ApprovalCategoryValue; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.PatchLineComment; import com.google.gerrit.reviewdb.PatchSet; import com.google.gerrit.reviewdb.PatchSetApproval; import com.google.gerrit.reviewdb.PatchSetInfo; -import com.google.gerrit.reviewdb.RefRight; import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountInfoCacheFactory; @@ -36,27 +31,20 @@ import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.project.CanSubmitResult; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gerrit.server.project.ProjectCache; -import com.google.gerrit.server.project.ProjectState; -import com.google.gerrit.server.project.RefControl; import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; -import java.util.HashMap; -import java.util.HashSet; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; -import java.util.Map; -import java.util.Set; final class PatchSetPublishDetailFactory extends Handler { interface Factory { PatchSetPublishDetailFactory create(PatchSet.Id patchSetId); } - private final ProjectCache projectCache; private final PatchSetInfoFactory infoFactory; - private final ApprovalTypes approvalTypes; private final ReviewDb db; private final ChangeControl.Factory changeControlFactory; private final AccountInfoCacheFactory aic; @@ -68,19 +56,14 @@ final class PatchSetPublishDetailFactory extends Handler private PatchSetInfo patchSetInfo; private Change change; private List drafts; - private Map> allowed; - private Map given; @Inject PatchSetPublishDetailFactory(final PatchSetInfoFactory infoFactory, - final ProjectCache projectCache, final ApprovalTypes approvalTypes, final ReviewDb db, final AccountInfoCacheFactory.Factory accountInfoCacheFactory, final ChangeControl.Factory changeControlFactory, final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) { - this.projectCache = projectCache; this.infoFactory = infoFactory; - this.approvalTypes = approvalTypes; this.db = db; this.changeControlFactory = changeControlFactory; this.aic = accountInfoCacheFactory.create(); @@ -98,15 +81,17 @@ final class PatchSetPublishDetailFactory extends Handler patchSetInfo = infoFactory.get(patchSetId); drafts = db.patchComments().draft(patchSetId, user.getAccountId()).toList(); - allowed = new HashMap>(); - given = new HashMap(); + List allowed = Collections.emptyList(); + List given = Collections.emptyList(); + if (change.getStatus().isOpen() && patchSetId.equals(change.currentPatchSetId())) { - computeAllowed(); - for (final PatchSetApproval a : db.patchSetApprovals().byPatchSetUser( - patchSetId, user.getAccountId())) { - given.put(a.getCategoryId(), a); - } + allowed = new ArrayList(control.getLabelRanges()); + Collections.sort(allowed); + + given = db.patchSetApprovals() // + .byPatchSetUser(patchSetId, user.getAccountId()) // + .toList(); } aic.want(change.getOwner()); @@ -117,46 +102,12 @@ final class PatchSetPublishDetailFactory extends Handler detail.setPatchSetInfo(patchSetInfo); detail.setChange(change); detail.setDrafts(drafts); - detail.setAllowed(allowed); + detail.setLabels(allowed); detail.setGiven(given); final CanSubmitResult canSubmitResult = control.canSubmit(patchSetId); - detail.setSubmitAllowed(canSubmitResult == CanSubmitResult.OK); + detail.setCanSubmit(canSubmitResult == CanSubmitResult.OK); return detail; } - - private void computeAllowed() { - final Set am = user.getEffectiveGroups(); - final ProjectState pe = projectCache.get(change.getProject()); - for (ApprovalCategory.Id category : approvalTypes.getApprovalCategories()) { - RefControl rc = pe.controlFor(user).controlForRef(change.getDest()); - List categoryRights = rc.getApplicableRights(category); - computeAllowed(am, categoryRights, category); - } - } - - private void computeAllowed(final Set am, - final List list, ApprovalCategory.Id category) { - - Set s = allowed.get(category); - if (s == null) { - s = new HashSet(); - allowed.put(category, s); - } - - for (final RefRight r : list) { - if (!am.contains(r.getAccountGroupUUID())) { - continue; - } - final ApprovalType at = - approvalTypes.getApprovalType(r.getApprovalCategoryId()); - for (short m = r.getMinValue(); m <= r.getMaxValue(); m++) { - final ApprovalCategoryValue v = at.getValue(m); - if (v != null) { - s.add(v.getId()); - } - } - } - } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java index da52596903..a76d0dfa3d 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java @@ -188,7 +188,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements .byPatchSetUser(ps_id, aid)) { final ApprovalCategory.Id category = ca.getCategoryId(); if (change.getStatus().isOpen()) { - fs.normalize(approvalTypes.getApprovalType(category), ca); + fs.normalize(approvalTypes.byId(category), ca); } if (ca.getValue() == 0 || ApprovalCategory.SUBMIT.equals(category)) { @@ -232,7 +232,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements for (PatchSetApproval ca : db.patchSetApprovals().byPatchSet(ps_id)) { final ApprovalCategory.Id category = ca.getCategoryId(); if (change.getStatus().isOpen()) { - fs.normalize(approvalTypes.getApprovalType(category), ca); + fs.normalize(approvalTypes.byId(category), ca); } if (ca.getValue() == 0 || ApprovalCategory.SUBMIT.equals(category)) { diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ApprovalCategory.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ApprovalCategory.java index 29ea97ee99..d7e5024dbd 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ApprovalCategory.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ApprovalCategory.java @@ -24,33 +24,6 @@ public final class ApprovalCategory { public static final ApprovalCategory.Id SUBMIT = new ApprovalCategory.Id("SUBM"); - /** Id of the special "Read" action (and category). */ - public static final ApprovalCategory.Id READ = - new ApprovalCategory.Id("READ"); - - /** Id of the special "Own" category; manages a project. */ - public static final ApprovalCategory.Id OWN = new ApprovalCategory.Id("OWN"); - - /** Id of the special "Push Annotated Tag" action (and category). */ - public static final ApprovalCategory.Id PUSH_TAG = - new ApprovalCategory.Id("pTAG"); - public static final short PUSH_TAG_SIGNED = 1; - public static final short PUSH_TAG_ANNOTATED = 2; - - /** Id of the special "Push Branch" action (and category). */ - public static final ApprovalCategory.Id PUSH_HEAD = - new ApprovalCategory.Id("pHD"); - public static final short PUSH_HEAD_UPDATE = 1; - public static final short PUSH_HEAD_CREATE = 2; - public static final short PUSH_HEAD_REPLACE = 3; - - /** Id of the special "Forge Identity" category. */ - public static final ApprovalCategory.Id FORGE_IDENTITY = - new ApprovalCategory.Id("FORG"); - public static final short FORGE_AUTHOR = 1; - public static final short FORGE_COMMITTER = 2; - public static final short FORGE_SERVER = 3; - public static class Id extends StringKey> { private static final long serialVersionUID = 1L; @@ -73,15 +46,6 @@ public final class ApprovalCategory { protected void set(String newValue) { id = newValue; } - - /** True if the right can be assigned on the wild project. */ - public boolean canBeOnWildProject() { - if (OWN.equals(this)) { - return false; - } else { - return true; - } - } } /** Internal short unique identifier for this category. */ @@ -96,16 +60,7 @@ public final class ApprovalCategory { @Column(id = 3, length = 4, notNull = false) protected String abbreviatedName; - /** - * Order of this category within the Approvals table when presented. - *

- * If < 0 (e.g. -1) this category is not shown in the Approvals table but is - * instead considered to be an action that the user might be able to perform, - * e.g. "Submit". - *

- * If >= 0 this category is shown in the Approvals table, sorted along with - * its siblings by position, name. - */ + /** Order of this category within the Approvals table when presented. */ @Column(id = 4) protected short position; @@ -117,6 +72,9 @@ public final class ApprovalCategory { @Column(id = 6) protected boolean copyMinScore; + /** Computed name derived from {@link #name}. */ + protected String labelName; + protected ApprovalCategory() { } @@ -136,6 +94,26 @@ public final class ApprovalCategory { public void setName(final String n) { name = n; + labelName = null; + } + + /** Clean version of {@link #getName()}, e.g. "Code Review" is "Code-Review". */ + public String getLabelName() { + if (labelName == null) { + StringBuilder r = new StringBuilder(); + for (int i = 0; i < name.length(); i++) { + char c = name.charAt(i); + if (('0' <= c && c <= '9') // + || ('a' <= c && c <= 'z') // + || ('A' <= c && c <= 'Z')) { + r.append(c); + } else if (c == ' ') { + r.append('-'); + } + } + labelName = r.toString(); + } + return labelName; } public String getAbbreviatedName() { @@ -154,14 +132,6 @@ public final class ApprovalCategory { position = p; } - public boolean isAction() { - return position < 0; - } - - public boolean isRange() { - return !isAction(); - } - public String getFunctionName() { return functionName; } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRight.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRight.java deleted file mode 100644 index 112922ca2c..0000000000 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRight.java +++ /dev/null @@ -1,246 +0,0 @@ -// Copyright (C) 2010 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.reviewdb; - -import com.google.gwtorm.client.Column; -import com.google.gwtorm.client.CompoundKey; -import com.google.gwtorm.client.StringKey; - -import java.util.Comparator; - -/** Grant to use an {@link ApprovalCategory} in the scope of a git ref. */ -public final class RefRight { - /** Pattern that matches all references in a project. */ - public static final String ALL = "refs/*"; - - /** Prefix that triggers a regular expression pattern. */ - public static final String REGEX_PREFIX = "^"; - - public static class RefPattern extends - StringKey> { - private static final long serialVersionUID = 1L; - - @Column(id = 1) - protected String pattern; - - protected RefPattern() { - } - - public RefPattern(final String pattern) { - this.pattern = pattern; - } - - @Override - public String get() { - return pattern; - } - - @Override - protected void set(String pattern) { - this.pattern = pattern; - } - } - - public static class Key extends CompoundKey { - private static final long serialVersionUID = 1L; - - @Column(id = 1) - protected Project.NameKey projectName; - - @Column(id = 2) - protected RefPattern refPattern; - - @Column(id = 3) - protected ApprovalCategory.Id categoryId; - - @Column(id = 4) - protected AccountGroup.Id groupId; - - protected Key() { - projectName = new Project.NameKey(); - refPattern = new RefPattern(); - categoryId = new ApprovalCategory.Id(); - groupId = new AccountGroup.Id(); - } - - public Key(final Project.NameKey projectName, final RefPattern refPattern, - final ApprovalCategory.Id categoryId, final AccountGroup.Id groupId) { - this.projectName = projectName; - this.refPattern = refPattern; - this.categoryId = categoryId; - this.groupId = groupId; - } - - @Override - public Project.NameKey getParentKey() { - return projectName; - } - - public Project.NameKey getProjectNameKey() { - return projectName; - } - - public String getRefPattern() { - return refPattern.get(); - } - - public void setGroupId(AccountGroup.Id groupId) { - this.groupId = groupId; - } - - @Override - public com.google.gwtorm.client.Key[] members() { - return new com.google.gwtorm.client.Key[] {refPattern, categoryId, - groupId}; - } - } - - @Column(id = 1, name = Column.NONE) - protected Key key; - - @Column(id = 2) - protected short minValue; - - @Column(id = 3) - protected short maxValue; - - protected transient AccountGroup.UUID groupUUID; - - protected RefRight() { - } - - public RefRight(RefRight.Key key) { - this.key = key; - } - - public RefRight(final RefRight refRight, final AccountGroup.UUID groupId) { - this(new RefRight.Key(refRight.getKey().projectName, - refRight.getKey().refPattern, refRight.getKey().categoryId, null)); - setMinValue(refRight.getMinValue()); - setMaxValue(refRight.getMaxValue()); - setAccountGroupUUID(groupId); - } - - public RefRight.Key getKey() { - return key; - } - - public String getRefPattern() { - if (isExclusive()) { - return key.refPattern.get().substring(1); - } - return key.refPattern.get(); - } - - public String getRefPatternForDisplay() { - return key.refPattern.get(); - } - - public Project.NameKey getProjectNameKey() { - return getKey().getProjectNameKey(); - } - - public boolean isExclusive() { - return key.refPattern.get().startsWith("-"); - } - - public ApprovalCategory.Id getApprovalCategoryId() { - return key.categoryId; - } - - public AccountGroup.Id getAccountGroupId() { - return key.groupId; - } - - public AccountGroup.UUID getAccountGroupUUID() { - return groupUUID; - } - - public void setAccountGroupUUID(AccountGroup.UUID uuid) { - groupUUID = uuid; - } - - public short getMinValue() { - return minValue; - } - - public void setMinValue(final short m) { - minValue = m; - } - - public short getMaxValue() { - return maxValue; - } - - public void setMaxValue(final short m) { - maxValue = m; - } - - @Override - public String toString() { - StringBuilder s = new StringBuilder(); - s.append("{group :"); - s.append(getAccountGroupId().get()); - s.append(", proj :"); - s.append(getProjectNameKey().get()); - s.append(", cat :"); - s.append(getApprovalCategoryId().get()); - s.append(", pattern :"); - s.append(getRefPatternForDisplay()); - s.append(", min :"); - s.append(getMinValue()); - s.append(", max :"); - s.append(getMaxValue()); - s.append("}"); - return s.toString(); - } - - @Override - public int hashCode() { - return getKey().hashCode(); - } - - @Override - public boolean equals(Object o) { - if (o instanceof RefRight) { - RefRight a = this; - RefRight b = (RefRight) o; - return a.getKey().equals(b.getKey()) - && a.getMinValue() == b.getMinValue() - && a.getMaxValue() == b.getMaxValue(); - } - return false; - } - - public static final Comparator REF_PATTERN_ORDER = - new Comparator() { - - @Override - public int compare(RefRight a, RefRight b) { - int aLength = a.getRefPattern().length(); - int bLength = b.getRefPattern().length(); - if (bLength == aLength) { - ApprovalCategory.Id aCat = a.getApprovalCategoryId(); - ApprovalCategory.Id bCat = b.getApprovalCategoryId(); - if (aCat.get().equals(bCat.get())) { - return a.getRefPattern().compareTo(b.getRefPattern()); - } - return a.getApprovalCategoryId().get() - .compareTo(b.getApprovalCategoryId().get()); - } - return bLength - aLength; - } - }; -} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRightAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRightAccess.java deleted file mode 100644 index a42ff2cd2c..0000000000 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/RefRightAccess.java +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (C) 2010 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.reviewdb; - -import com.google.gwtorm.client.Access; -import com.google.gwtorm.client.OrmException; -import com.google.gwtorm.client.PrimaryKey; -import com.google.gwtorm.client.Query; -import com.google.gwtorm.client.ResultSet; - -public interface RefRightAccess extends Access { - @PrimaryKey("key") - RefRight get(RefRight.Key refRight) throws OrmException; - - @Query("WHERE key.projectName = ?") - ResultSet byProject(Project.NameKey project) throws OrmException; - - @Query("WHERE key.categoryId = ? AND key.groupId = ?") - ResultSet byCategoryGroup(ApprovalCategory.Id cat, - AccountGroup.Id group) throws OrmException; -} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java index c92694c257..b75b91bb1f 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java @@ -110,9 +110,6 @@ public interface ReviewDb extends Schema { @Relation PatchLineCommentAccess patchComments(); - @Relation - RefRightAccess refRights(); - @Relation TrackingIdAccess trackingIds(); diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql index d33d24d210..9784a4dcd4 100644 --- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql +++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql @@ -157,14 +157,6 @@ ON patch_set_ancestors (ancestor_revision); -- @PrimaryKey covers: all, suggestByName --- ********************************************************************* --- RefRightAccess --- @PrimaryKey covers: byProject --- covers: byCategoryGroup -CREATE INDEX ref_rights_byCatGroup -ON ref_rights (category_id, group_id); - - -- ********************************************************************* -- TrackingIdAccess -- diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql index 8e3cead5d5..db6894defa 100644 --- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql +++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql @@ -239,14 +239,6 @@ ON patch_set_ancestors (ancestor_revision); -- covers: ownedByGroup --- ********************************************************************* --- RefRightAccess --- @PrimaryKey covers: byProject --- covers: byCategoryGroup -CREATE INDEX ref_rights_byCatGroup -ON ref_rights (category_id, group_id); - - -- ********************************************************************* -- TrackingIdAccess -- diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java index 1a4a863f55..c46a114c72 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java +++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java @@ -433,7 +433,7 @@ public class ChangeHookRunner { Entry approval) { ApprovalAttribute a = new ApprovalAttribute(); a.type = approval.getKey().get(); - final ApprovalType at = approvalTypes.getApprovalType(approval.getKey()); + final ApprovalType at = approvalTypes.byId(approval.getKey()); a.description = at.getCategory().getName(); a.value = Short.toString(approval.getValue().get()); return a; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/ApprovalTypesProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/ApprovalTypesProvider.java index 25ab239fe9..4552be78bf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/ApprovalTypesProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/ApprovalTypesProvider.java @@ -39,8 +39,7 @@ class ApprovalTypesProvider implements Provider { @Override public ApprovalTypes get() { - List approvalTypes = new ArrayList(2); - List actionTypes = new ArrayList(2); + List types = new ArrayList(2); try { final ReviewDb db = schema.open(); @@ -48,12 +47,7 @@ class ApprovalTypesProvider implements Provider { for (final ApprovalCategory c : db.approvalCategories().all()) { final List values = db.approvalCategoryValues().byCategory(c.getId()).toList(); - final ApprovalType type = new ApprovalType(c, values); - if (type.getCategory().isAction()) { - actionTypes.add(type); - } else { - approvalTypes.add(type); - } + types.add(new ApprovalType(c, values)); } } finally { db.close(); @@ -62,8 +56,6 @@ class ApprovalTypesProvider implements Provider { throw new ProvisionException("Cannot query approval categories", e); } - approvalTypes = Collections.unmodifiableList(approvalTypes); - actionTypes = Collections.unmodifiableList(actionTypes); - return new ApprovalTypes(approvalTypes, actionTypes); + return new ApprovalTypes(Collections.unmodifiableList(types)); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java index 573e6bb250..beeaa599a8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java @@ -212,7 +212,7 @@ public class EventFactory { a.by = asAccountAttribute(approval.getAccountId()); a.grantedOn = approval.getGranted().getTime() / 1000L; - ApprovalType at = approvalTypes.getApprovalType(approval.getCategoryId()); + ApprovalType at = approvalTypes.byId(approval.getCategoryId()); if (at != null) { a.description = at.getCategory().getName(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java index 67705e407d..112ed4d9fb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CreateCodeReviewNotes.java @@ -212,10 +212,13 @@ public class CreateCodeReviewNotes { if (ApprovalCategory.SUBMIT.equals(a.getCategoryId())) { submit = a; } else { - formatter.appendApproval( - approvalTypes.getApprovalType(a.getCategoryId()).getCategory(), - a.getValue(), - accountCache.get(a.getAccountId()).getAccount()); + ApprovalCategory type = approvalTypes.byId(a.getCategoryId()).getCategory(); + if (type != null) { + formatter.appendApproval( + type, + a.getValue(), + accountCache.get(a.getAccountId()).getAccount()); + } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index caeb71cf3d..5e3f0fad9e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -805,7 +805,7 @@ public class MergeOp { tag = "Tested-by"; } else { final ApprovalType at = - approvalTypes.getApprovalType(a.getCategoryId()); + approvalTypes.byId(a.getCategoryId()); if (at == null) { // A deprecated/deleted approval type, ignore it. continue; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java index b69514cb3f..7ad82de9bb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java @@ -14,24 +14,45 @@ package com.google.gerrit.server.git; +import static com.google.gerrit.common.data.AccessSection.isAccessSection; +import static com.google.gerrit.common.data.Permission.isPermission; + +import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.GroupReference; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRule; +import com.google.gerrit.reviewdb.AccountGroup; import com.google.gerrit.reviewdb.Project; import com.google.gerrit.reviewdb.Project.SubmitType; +import com.google.gerrit.server.account.GroupCache; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.ObjectId; +import java.io.BufferedReader; import java.io.IOException; +import java.io.StringReader; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; public class ProjectConfig extends VersionedMetaData { private static final String PROJECT_CONFIG = "project.config"; + private static final String GROUP_LIST = "groups"; private static final String PROJECT = "project"; private static final String KEY_DESCRIPTION = "description"; private static final String ACCESS = "access"; private static final String KEY_INHERIT_FROM = "inheritFrom"; + private static final String KEY_GROUP_PERMISSIONS = "exclusiveGroupPermissions"; private static final String RECEIVE = "receive"; private static final String KEY_REQUIRE_SIGNED_OFF_BY = "requireSignedOffBy"; @@ -48,6 +69,8 @@ public class ProjectConfig extends VersionedMetaData { private Project.NameKey projectName; private Project project; + private Map groupsByUUID; + private Map accessSections; public static ProjectConfig read(MetaDataUpdate update) throws IOException, ConfigInvalidException { @@ -71,6 +94,60 @@ public class ProjectConfig extends VersionedMetaData { return project; } + public AccessSection getAccessSection(String name) { + return getAccessSection(name, false); + } + + public AccessSection getAccessSection(String name, boolean create) { + AccessSection as = accessSections.get(name); + if (as == null && create) { + as = new AccessSection(name); + accessSections.put(name, as); + } + return as; + } + + public Collection getAccessSections() { + return accessSections.values(); + } + + public void remove(AccessSection section) { + accessSections.remove(section.getRefPattern()); + } + + public GroupReference resolve(AccountGroup group) { + return resolve(GroupReference.forGroup(group)); + } + + public GroupReference resolve(GroupReference group) { + if (group != null) { + GroupReference ref = groupsByUUID.get(group.getUUID()); + if (ref != null) { + return ref; + } + groupsByUUID.put(group.getUUID(), group); + } + return group; + } + + /** + * Check all GroupReferences use current group name, repairing stale ones. + * + * @param groupCache cache to use when looking up group information by UUID. + * @return true if one or more group names was stale. + */ + public boolean updateGroupNames(GroupCache groupCache) { + boolean dirty = false; + for (GroupReference ref : groupsByUUID.values()) { + AccountGroup g = groupCache.get(ref.getUUID()); + if (g != null && !g.getName().equals(ref.getName())) { + dirty = true; + ref.setName(g.getName()); + } + } + return dirty; + } + @Override protected String getRefName() { return GitRepositoryManager.REF_CONFIG; @@ -78,6 +155,8 @@ public class ProjectConfig extends VersionedMetaData { @Override protected void onLoad() throws IOException, ConfigInvalidException { + Map groupsByName = readGroupList(); + Config rc = readConfig(PROJECT_CONFIG); project = new Project(projectName); @@ -94,6 +173,80 @@ public class ProjectConfig extends VersionedMetaData { p.setSubmitType(rc.getEnum(SUBMIT, null, KEY_ACTION, defaultSubmitAction)); p.setUseContentMerge(rc.getBoolean(SUBMIT, null, KEY_MERGE_CONTENT, false)); + + accessSections = new HashMap(); + for (String refName : rc.getSubsections(ACCESS)) { + if (isAccessSection(refName)) { + AccessSection as = getAccessSection(refName, true); + + for (String varName : rc.getStringList(ACCESS, refName, KEY_GROUP_PERMISSIONS)) { + for (String n : varName.split("[, \t]{1,}")) { + if (isPermission(n)) { + as.getPermission(n, true).setExclusiveGroup(true); + } + } + } + + for (String varName : rc.getNames(ACCESS, refName)) { + if (isPermission(varName)) { + Permission perm = as.getPermission(varName, true); + + boolean useRange = perm.isLabel(); + for (String ruleString : rc.getStringList(ACCESS, refName, varName)) { + PermissionRule rule; + try { + rule = PermissionRule.fromString(ruleString, useRange); + } catch (IllegalArgumentException notRule) { + throw new ConfigInvalidException("Invalid rule in " + ACCESS + + "." + refName + "." + varName + ": " + + notRule.getMessage(), notRule); + } + + GroupReference ref = groupsByName.get(rule.getGroup().getName()); + if (ref == null) { + // The group wasn't mentioned in the groups table, so there is + // no valid UUID for it. Pool the reference anyway so at least + // all rules in the same file share the same GroupReference. + // + ref = rule.getGroup(); + groupsByName.put(ref.getName(), ref); + } + + rule.setGroup(ref); + perm.add(rule); + } + } + } + } + } + } + + private Map readGroupList() throws IOException, + ConfigInvalidException { + groupsByUUID = new HashMap(); + Map groupsByName = + new HashMap(); + + BufferedReader br = new BufferedReader(new StringReader(readUTF8(GROUP_LIST))); + String s; + while ((s = br.readLine()) != null) { + if (s.isEmpty() || s.startsWith("#")) { + continue; + } + + int tab = s.indexOf('\t'); + if (tab < 0) { + throw new ConfigInvalidException("Invalid group line: " + s); + } + + AccountGroup.UUID uuid = new AccountGroup.UUID(s.substring(0, tab).trim()); + String name = s.substring(tab + 1).trim(); + GroupReference ref = new GroupReference(uuid, name); + + groupsByUUID.put(uuid, ref); + groupsByName.put(name, ref); + } + return groupsByName; } @Override @@ -120,6 +273,102 @@ public class ProjectConfig extends VersionedMetaData { set(rc, SUBMIT, null, KEY_ACTION, p.getSubmitType(), defaultSubmitAction); set(rc, SUBMIT, null, KEY_MERGE_CONTENT, p.isUseContentMerge()); + Set keepGroups = new HashSet(); + for (AccessSection as : sort(accessSections.values())) { + String refName = as.getRefPattern(); + + StringBuilder doNotInherit = new StringBuilder(); + for (Permission perm : sort(as.getPermissions())) { + if (perm.getExclusiveGroup()) { + if (0 < doNotInherit.length()) { + doNotInherit.append(' '); + } + doNotInherit.append(perm.getName()); + } + } + if (0 < doNotInherit.length()) { + rc.setString(ACCESS, refName, KEY_GROUP_PERMISSIONS, doNotInherit.toString()); + } else { + rc.unset(ACCESS, refName, KEY_GROUP_PERMISSIONS); + } + + Set have = new HashSet(); + for (Permission permission : sort(as.getPermissions())) { + have.add(permission.getName().toLowerCase()); + + boolean needRange = permission.isLabel(); + List rules = new ArrayList(); + for (PermissionRule rule : sort(permission.getRules())) { + GroupReference group = rule.getGroup(); + if (group.getUUID() != null) { + keepGroups.add(group.getUUID()); + } + rules.add(rule.asString(needRange)); + } + rc.setStringList(ACCESS, refName, permission.getName(), rules); + } + + for (String varName : rc.getNames(ACCESS, refName)) { + if (isPermission(varName) && !have.contains(varName.toLowerCase())) { + rc.unset(ACCESS, refName, varName); + } + } + } + + for (String name : rc.getSubsections(ACCESS)) { + if (isAccessSection(name) && !accessSections.containsKey(name)) { + rc.unsetSection(ACCESS, name); + } + } + groupsByUUID.keySet().retainAll(keepGroups); + saveConfig(PROJECT_CONFIG, rc); + saveGroupList(); + } + + private void saveGroupList() throws IOException { + if (groupsByUUID.isEmpty()) { + saveFile(GROUP_LIST, null); + return; + } + + final int uuidLen = 40; + StringBuilder buf = new StringBuilder(); + buf.append(pad(uuidLen, "# UUID")); + buf.append('\t'); + buf.append("Group Name"); + buf.append('\n'); + + buf.append('#'); + buf.append('\n'); + + for (GroupReference g : sort(groupsByUUID.values())) { + if (g.getUUID() != null && g.getName() != null) { + buf.append(pad(uuidLen, g.getUUID().get())); + buf.append('\t'); + buf.append(g.getName()); + buf.append('\n'); + } + } + saveUTF8(GROUP_LIST, buf.toString()); + } + + private static String pad(int len, String src) { + if (len <= src.length()) { + return src; + } + + StringBuilder r = new StringBuilder(len); + r.append(src); + while (r.length() < len) { + r.append(' '); + } + return r.toString(); + } + + private static > List sort(Collection m) { + ArrayList r = new ArrayList(m); + Collections.sort(r); + return r; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 13ac1736e4..84f3422223 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -1359,7 +1359,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook { } final ApprovalType type = - approvalTypes.getApprovalType(a.getCategoryId()); + approvalTypes.byId(a.getCategoryId()); if (a.getPatchSetId().equals(priorPatchSet) && type.getCategory().isCopyMinScore() && type.isMaxNegative(a)) { // If there was a negative vote on the prior patch set, carry it diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java index d54dceced1..8997e2b767 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReviewNoteHeaderFormatter.java @@ -51,8 +51,7 @@ class ReviewNoteHeaderFormatter { void appendApproval(ApprovalCategory category, short value, Account user) { - // TODO: use category.getLabel() when available - sb.append(category.getName().replace(' ', '-')); + sb.append(category.getLabelName()); sb.append(value < 0 ? "-" : "+").append(Math.abs(value)).append(": "); appendUserData(user); sb.append("\n"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java index 27ebe293db..bb910443f9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java @@ -69,6 +69,18 @@ public abstract class VersionedMetaData { return revision.copy(); } + /** Initialize in-memory as though the repository branch doesn't exist. */ + public void createInMemory() { + try { + revision = null; + onLoad(); + } catch (IOException err) { + throw new RuntimeException("Unexpected IOException", err); + } catch (ConfigInvalidException err) { + throw new RuntimeException("Unexpected ConfigInvalidException", err); + } + } + /** * Load the current version from the branch. *

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java index f3e28902e1..7d25ee8d3b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java @@ -171,7 +171,7 @@ public class PublishComments implements Callable { final short o = a.getValue(); a.setValue(want.get()); a.cache(change); - functionState.normalize(types.getApprovalType(a.getCategoryId()), a); + functionState.normalize(types.byId(a.getCategoryId()), a); if (o != a.getValue()) { // Value changed, ensure we update the database. // diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java index 3291f1ac3b..b387e704aa 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java @@ -16,13 +16,12 @@ package com.google.gerrit.server.project; import com.google.gerrit.common.data.ApprovalType; import com.google.gerrit.common.data.ApprovalTypes; -import com.google.gerrit.reviewdb.ApprovalCategory; +import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.PatchSet; import com.google.gerrit.reviewdb.PatchSetApproval; import com.google.gerrit.reviewdb.Project; import com.google.gerrit.reviewdb.ReviewDb; -import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.workflow.CategoryFunction; @@ -31,7 +30,6 @@ import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; -import java.util.ArrayList; import java.util.List; @@ -164,8 +162,14 @@ public class ChangeControl { return canAbandon(); // Anyone who can abandon the change can restore it back } - public short normalize(ApprovalCategory.Id category, short score) { - return getRefControl().normalize(category, score); + /** All value ranges of any allowed label permission. */ + public List getLabelRanges() { + return getRefControl().getLabelRanges(); + } + + /** The range of permitted values associated with a label permission. */ + public PermissionRange getRange(String permission) { + return getRefControl().getRange(permission); } /** Can this user add a patch set to this change? */ @@ -240,34 +244,22 @@ public class ChangeControl { return result; } - final List allApprovals = - new ArrayList(db.patchSetApprovals().byPatchSet( - patchSetId).toList()); - final PatchSetApproval myAction = - ChangeUtil.createSubmitApproval(patchSetId, - (IdentifiedUser) getCurrentUser(), db); - - final ApprovalType actionType = - approvalTypes.getApprovalType(myAction.getCategoryId()); - if (actionType == null || !actionType.getCategory().isAction()) { - return new CanSubmitResult("Invalid action " + myAction.getCategoryId()); - } + final List all = + db.patchSetApprovals().byPatchSet(patchSetId).toList(); final FunctionState fs = - functionStateFactory.create(change, patchSetId, allApprovals); + functionStateFactory.create(change, patchSetId, all); + for (ApprovalType c : approvalTypes.getApprovalTypes()) { CategoryFunction.forCategory(c.getCategory()).run(c, fs); } - if (!CategoryFunction.forCategory(actionType.getCategory()).isValid( - getCurrentUser(), actionType, fs)) { - return new CanSubmitResult(actionType.getCategory().getName() - + " not permitted"); - } - fs.normalize(actionType, myAction); - if (myAction.getValue() <= 0) { - return new CanSubmitResult(actionType.getCategory().getName() - + " not permitted"); + + for (ApprovalType type : approvalTypes.getApprovalTypes()) { + if (!fs.isValid(type)) { + return new CanSubmitResult("Requires " + type.getCategory().getName()); + } } + return CanSubmitResult.OK; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java index 7359f1713f..4202a6254d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCache.java @@ -29,9 +29,6 @@ public interface ProjectCache { /** Invalidate the cached information about the given project. */ public void evict(Project p); - /** Invalidate the cached information about all projects. */ - public void evictAll(); - /** @return sorted iteration of projects. */ public abstract Iterable all(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java index c8a1f7f48d..6459e3c2ff 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectCacheImpl.java @@ -14,9 +14,7 @@ package com.google.gerrit.server.project; -import com.google.gerrit.reviewdb.AccountGroup; import com.google.gerrit.reviewdb.Project; -import com.google.gerrit.reviewdb.RefRight; import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.cache.Cache; import com.google.gerrit.server.cache.CacheModule; @@ -33,13 +31,9 @@ import com.google.inject.name.Named; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.Repository; -import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.Iterator; -import java.util.Map; import java.util.NoSuchElementException; -import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; import java.util.concurrent.locks.Lock; @@ -99,11 +93,6 @@ public class ProjectCacheImpl implements ProjectCache { } } - /** Invalidate the cached information about all projects. */ - public void evictAll() { - byName.removeAll(); - } - @Override public void onCreateProject(Project.NameKey newProjectName) { listLock.lock(); @@ -193,30 +182,7 @@ public class ProjectCacheImpl implements ProjectCache { try { final ProjectConfig cfg = new ProjectConfig(key); cfg.load(git); - - final Project p = cfg.getProject(); - - Collection rights = db.refRights().byProject(key).toList(); - - Set groupIds = new HashSet(); - for (RefRight r : rights) { - groupIds.add(r.getAccountGroupId()); - } - Map groupsById = - db.accountGroups().toMap(db.accountGroups().get(groupIds)); - - for (RefRight r : rights) { - AccountGroup group = groupsById.get(r.getAccountGroupId()); - if (group != null) { - r.setAccountGroupUUID(group.getGroupUUID()); - } else { - r.setAccountGroupUUID(new AccountGroup.UUID("DELETED_GROUP_" - + r.getAccountGroupId().get())); - } - } - rights = Collections.unmodifiableCollection(rights); - - return projectStateFactory.create(p, rights); + return projectStateFactory.create(cfg); } finally { git.close(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java index c84b1f5bf4..4b11849af4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java @@ -14,13 +14,15 @@ package com.google.gerrit.server.project; -import static com.google.gerrit.common.CollectionsUtil.*; +import static com.google.gerrit.common.CollectionsUtil.isAnyIncludedIn; + +import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.AccountGroup; -import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.Branch; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.Project; -import com.google.gerrit.reviewdb.RefRight; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.ReplicationUser; import com.google.gerrit.server.config.GitReceivePackGroups; @@ -29,6 +31,7 @@ import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; +import java.util.Collection; import java.util.HashSet; import java.util.Set; @@ -108,6 +111,8 @@ public class ProjectControl { private final CurrentUser user; private final ProjectState state; + private Collection access; + @Inject ProjectControl(@GitUploadPackGroups Set uploadGroups, @GitReceivePackGroups Set receiveGroups, @@ -155,18 +160,18 @@ public class ProjectControl { /** Can this user see this project exists? */ public boolean isVisible() { return visibleForReplication() - || canPerformOnAnyRef(ApprovalCategory.READ, (short) 1); + || canPerformOnAnyRef(Permission.READ); } public boolean canAddRefs() { - return (canPerformOnAnyRef(ApprovalCategory.PUSH_HEAD, ApprovalCategory.PUSH_HEAD_CREATE) + return (canPerformOnAnyRef(Permission.CREATE) || isOwnerAnyRef()); } /** Can this user see all the refs in this projects? */ public boolean allRefsAreVisible() { return visibleForReplication() - || canPerformOnAllRefs(ApprovalCategory.READ, (short) 1); + || canPerformOnAllRefs(Permission.READ); } /** Is this project completely visible for replication? */ @@ -177,49 +182,60 @@ public class ProjectControl { /** Is this user a project owner? Ownership does not imply {@link #isVisible()} */ public boolean isOwner() { - return controlForRef(RefRight.ALL).isOwner() + return controlForRef(AccessSection.ALL).isOwner() || getCurrentUser().isAdministrator(); } /** Does this user have ownership on at least one reference name? */ public boolean isOwnerAnyRef() { - return canPerformOnAnyRef(ApprovalCategory.OWN, (short) 1) + return canPerformOnAnyRef(Permission.OWNER) || getCurrentUser().isAdministrator(); } /** @return true if the user can upload to at least one reference */ public boolean canPushToAtLeastOneRef() { - return canPerformOnAnyRef(ApprovalCategory.READ, (short) 2) - || canPerformOnAnyRef(ApprovalCategory.PUSH_HEAD, (short) 1) - || canPerformOnAnyRef(ApprovalCategory.PUSH_TAG, (short) 1); + return canPerformOnAnyRef(Permission.PUSH) + || canPerformOnAnyRef(Permission.PUSH_TAG); } - // TODO (anatol.pomazau): Try to merge this method with similar RefRightsForPattern#canPerform - private boolean canPerformOnAnyRef(ApprovalCategory.Id actionId, - short requireValue) { + private boolean canPerformOnAnyRef(String permissionName) { final Set groups = user.getEffectiveGroups(); - for (final RefRight pr : state.getAllRights(actionId, true)) { - if (groups.contains(pr.getAccountGroupUUID()) - && pr.getMaxValue() >= requireValue) { - return true; + for (AccessSection section : access()) { + Permission permission = section.getPermission(permissionName); + if (permission == null) { + continue; + } + + for (PermissionRule rule : permission.getRules()) { + if (rule.getDeny()) { + continue; + } + + // Being in a group that was granted this permission is only an + // approximation. There might be overrides and doNotInherit + // that would render this to be false. + // + if (groups.contains(rule.getGroup().getUUID()) + && controlForRef(section.getRefPattern()).canPerform(permissionName)) { + return true; + } } } return false; } - private boolean canPerformOnAllRefs(ApprovalCategory.Id actionId, - short requireValue) { + private boolean canPerformOnAllRefs(String permission) { boolean canPerform = false; - final Set patterns = allRefPatterns(actionId); - if (patterns.contains(RefRight.ALL)) { + Set patterns = allRefPatterns(permission); + if (patterns.contains(AccessSection.ALL)) { // Only possible if granted on the pattern that // matches every possible reference. Check all // patterns also have the permission. // for (final String pattern : patterns) { - if (controlForRef(pattern).canPerform(actionId, requireValue)) { + if (controlForRef(pattern).canPerform(permission)) { canPerform = true; } else { return false; @@ -229,14 +245,24 @@ public class ProjectControl { return canPerform; } - private Set allRefPatterns(ApprovalCategory.Id actionId) { - final Set all = new HashSet(); - for (final RefRight pr : state.getAllRights(actionId, true)) { - all.add(pr.getRefPattern()); + private Set allRefPatterns(String permissionName) { + Set all = new HashSet(); + for (AccessSection section : access()) { + Permission permission = section.getPermission(permissionName); + if (permission != null) { + all.add(section.getRefPattern()); + } } return all; } + Collection access() { + if (access == null) { + access = state.getAllAccessSections(); + } + return access; + } + public boolean canRunUploadPack() { return isAnyIncludedIn(uploadGroups, user.getEffectiveGroups()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java index 5456769ae7..73765e8756 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java @@ -14,13 +14,16 @@ package com.google.gerrit.server.project; +import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.GroupReference; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.AccountGroup; -import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.Project; -import com.google.gerrit.reviewdb.RefRight; import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.config.WildProjectName; +import com.google.gerrit.server.git.ProjectConfig; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -28,15 +31,13 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedList; import java.util.List; import java.util.Set; /** Cached information on a project. */ public class ProjectState { public interface Factory { - ProjectState create(Project project, Collection localRights); + ProjectState create(ProjectConfig config); } private final AnonymousUser anonymousUser; @@ -44,92 +45,64 @@ public class ProjectState { private final ProjectCache projectCache; private final ProjectControl.AssistedFactory projectControlFactory; - private final Project project; - private final Collection localRights; + private final ProjectConfig config; private final Set localOwners; - private volatile Collection inheritedRights; - @Inject protected ProjectState(final AnonymousUser anonymousUser, final ProjectCache projectCache, @WildProjectName final Project.NameKey wildProject, final ProjectControl.AssistedFactory projectControlFactory, - @Assisted final Project project, - @Assisted Collection rights) { + @Assisted final ProjectConfig config) { this.anonymousUser = anonymousUser; this.projectCache = projectCache; this.wildProject = wildProject; this.projectControlFactory = projectControlFactory; + this.config = config; - if (wildProject.equals(project.getNameKey())) { - rights = new ArrayList(rights); - for (Iterator itr = rights.iterator(); itr.hasNext();) { - if (!itr.next().getApprovalCategoryId().canBeOnWildProject()) { - itr.remove(); + HashSet groups = new HashSet(); + AccessSection all = config.getAccessSection(AccessSection.ALL); + if (all != null) { + Permission owner = all.getPermission(Permission.OWNER); + if (owner != null) { + for (PermissionRule rule : owner.getRules()) { + GroupReference ref = rule.getGroup(); + if (ref.getUUID() != null) { + groups.add(ref.getUUID()); + } } } - rights = Collections.unmodifiableCollection(rights); - } - - this.project = project; - this.localRights = rights; - - final HashSet groups = new HashSet(); - for (final RefRight right : rights) { - if (ApprovalCategory.OWN.equals(right.getApprovalCategoryId()) - && right.getMaxValue() > 0 - && right.getRefPattern().equals(RefRight.ALL)) { - groups.add(right.getAccountGroupUUID()); - } } localOwners = Collections.unmodifiableSet(groups); } public Project getProject() { - return project; + return getConfig().getProject(); + } + + public ProjectConfig getConfig() { + return config; } /** Get the rights that pertain only to this project. */ - public Collection getLocalRights() { - return localRights; + public Collection getLocalAccessSections() { + return getConfig().getAccessSections(); } - /** - * Get the rights that pertain only to this project. - * - * @param action the category requested. - * @return immutable collection of rights for the requested category. - */ - public Collection getLocalRights(ApprovalCategory.Id action) { - return filter(getLocalRights(), action); - } - - /** Get the rights this project inherits from the wild project. */ - public Collection getInheritedRights() { - if (inheritedRights == null) { - inheritedRights = computeInheritedRights(); - } - return inheritedRights; - } - - void setInheritedRights(Collection all) { - inheritedRights = all; - } - - private Collection computeInheritedRights() { - if (isSpecialWildProject()) { + /** Get the rights this project inherits. */ + public Collection getInheritedAccessSections() { + if (isWildProject()) { return Collections.emptyList(); } - List inherited = new ArrayList(); + List inherited = new ArrayList(); Set seen = new HashSet(); - Project.NameKey parent = project.getParent(); + Project.NameKey parent = getProject().getParent(); while (parent != null && seen.add(parent)) { ProjectState s = projectCache.get(parent); if (s != null) { - inherited.addAll(s.getLocalRights()); + inherited.addAll(s.getLocalAccessSections()); parent = s.getProject().getParent(); } else { break; @@ -138,76 +111,21 @@ public class ProjectState { // Wild project is the parent, or the root of the tree if (parent == null) { - inherited.addAll(getWildProjectRights()); - } - - return Collections.unmodifiableCollection(inherited); - } - - private Collection getWildProjectRights() { - final ProjectState s = projectCache.get(wildProject); - return s != null ? s.getLocalRights() : Collections. emptyList(); - } - - /** - * Utility class that is needed to filter overridden refrights - */ - private static class Grant { - final AccountGroup.Id group; - final String pattern; - - private Grant(AccountGroup.Id group, String pattern) { - this.group = group; - this.pattern = pattern; - } - - @Override - public boolean equals(Object o) { - if (o == null) - return false; - Grant grant = (Grant) o; - return group.equals(grant.group) && pattern.equals(grant.pattern); - } - - @Override - public int hashCode() { - int result = group.hashCode(); - result = 31 * result + pattern.hashCode(); - return result; - } - } - - /** - * Get the rights this project has and inherits from the wild project. - * - * @param action the category requested. - * @param dropOverridden whether to remove inherited permissions in case if we have a - * local one that matches (action,group,ref) - * @return immutable collection of rights for the requested category. - */ - public Collection getAllRights(ApprovalCategory.Id action, boolean dropOverridden) { - Collection rights = new LinkedList(getLocalRights(action)); - rights.addAll(filter(getInheritedRights(), action)); - if (dropOverridden) { - Set grants = new HashSet(); - Iterator iter = rights.iterator(); - while (iter.hasNext()) { - RefRight right = iter.next(); - - Grant grant = new Grant(right.getAccountGroupId(), right.getRefPattern()); - if (grants.contains(grant)) { - iter.remove(); - } else { - grants.add(grant); - } + ProjectState s = projectCache.get(wildProject); + if (s != null) { + inherited.addAll(s.getLocalAccessSections()); } } - return Collections.unmodifiableCollection(rights); + + return inherited; } - /** Is this the special wild project which manages inherited rights? */ - public boolean isSpecialWildProject() { - return project.getNameKey().equals(wildProject); + /** Get both local and inherited access sections. */ + public Collection getAllAccessSections() { + List all = new ArrayList(); + all.addAll(getLocalAccessSections()); + all.addAll(getInheritedAccessSections()); + return all; } /** @@ -217,12 +135,12 @@ public class ProjectState { * that has local owners are returned */ public Set getOwners() { - if (!localOwners.isEmpty() || isSpecialWildProject() - || project.getParent() == null) { + Project.NameKey parentName = getProject().getParent(); + if (!localOwners.isEmpty() || parentName == null || isWildProject()) { return localOwners; } - final ProjectState parent = projectCache.get(project.getParent()); + ProjectState parent = projectCache.get(parentName); if (parent != null) { return parent.getOwners(); } @@ -238,12 +156,22 @@ public class ProjectState { * assigned for one of the parent projects (the inherited owners). */ public Set getAllOwners() { - final HashSet owners = new HashSet(); - for (final RefRight right : getAllRights(ApprovalCategory.OWN, true)) { - if (right.getMaxValue() > 0 && right.getRefPattern().equals(RefRight.ALL)) { - owners.add(right.getAccountGroupUUID()); + HashSet owners = new HashSet(); + owners.addAll(localOwners); + + Set seen = new HashSet(); + Project.NameKey parent = getProject().getParent(); + + while (parent != null && seen.add(parent)) { + ProjectState s = projectCache.get(parent); + if (s != null) { + owners.addAll(s.localOwners); + parent = s.getProject().getParent(); + } else { + break; } } + return Collections.unmodifiableSet(owners); } @@ -255,20 +183,7 @@ public class ProjectState { return projectControlFactory.create(user, this); } - private static Collection filter(Collection all, - ApprovalCategory.Id actionId) { - if (all.isEmpty()) { - return Collections.emptyList(); - } - final Collection mine = new ArrayList(all.size()); - for (final RefRight right : all) { - if (right.getApprovalCategoryId().equals(actionId)) { - mine.add(right); - } - } - if (mine.isEmpty()) { - return Collections.emptyList(); - } - return Collections.unmodifiableCollection(mine); + private boolean isWildProject() { + return wildProject.equals(getProject().getNameKey()); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java index 617ff2a95b..36674b1156 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java @@ -14,25 +14,13 @@ package com.google.gerrit.server.project; -import static com.google.gerrit.reviewdb.ApprovalCategory.FORGE_AUTHOR; -import static com.google.gerrit.reviewdb.ApprovalCategory.FORGE_COMMITTER; -import static com.google.gerrit.reviewdb.ApprovalCategory.FORGE_IDENTITY; -import static com.google.gerrit.reviewdb.ApprovalCategory.FORGE_SERVER; -import static com.google.gerrit.reviewdb.ApprovalCategory.OWN; -import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_HEAD; -import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_HEAD_CREATE; -import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_HEAD_REPLACE; -import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_HEAD_UPDATE; -import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_TAG; -import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_TAG_ANNOTATED; -import static com.google.gerrit.reviewdb.ApprovalCategory.PUSH_TAG_SIGNED; -import static com.google.gerrit.reviewdb.ApprovalCategory.READ; - +import com.google.gerrit.common.CollectionsUtil; +import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.ParamertizedString; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRange; +import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.AccountGroup; -import com.google.gerrit.reviewdb.ApprovalCategory; -import com.google.gerrit.reviewdb.RefRight; -import com.google.gerrit.reviewdb.SystemConfig; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.inject.Inject; @@ -49,7 +37,6 @@ import org.eclipse.jgit.revwalk.RevWalk; import java.io.IOException; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -57,8 +44,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.SortedMap; -import java.util.TreeMap; import java.util.regex.Pattern; @@ -71,6 +56,9 @@ public class RefControl { private final ProjectControl projectControl; private final String refName; + private Map> permissions; + + private Boolean owner; private Boolean canForgeAuthor; private Boolean canForgeCommitter; @@ -111,26 +99,29 @@ public class RefControl { /** Is this user a ref owner? */ public boolean isOwner() { - if (canPerform(OWN, (short) 1)) { - return true; - } + if (owner == null) { + if (canPerform(Permission.OWNER)) { + owner = true; - // We have to prevent infinite recursion here, the project control - // calls us to find out if there is ownership of all references in - // order to determine project level ownership. - // - if (getRefName().equals( - RefRight.ALL.substring(0, RefRight.ALL.length() - 1))) { - return getCurrentUser().isAdministrator(); - } else { - return getProjectControl().isOwner(); + } else if (getRefName().equals( + AccessSection.ALL.substring(0, AccessSection.ALL.length() - 1))) { + // We have to prevent infinite recursion here, the project control + // calls us to find out if there is ownership of all references in + // order to determine project level ownership. + // + owner = getCurrentUser().isAdministrator(); + + } else { + owner = getProjectControl().isOwner(); + } } + return owner; } /** Can this user see this reference exists? */ public boolean isVisible() { return getProjectControl().visibleForReplication() - || canPerform(READ, (short) 1); + || canPerform(Permission.READ); } /** @@ -141,27 +132,40 @@ public class RefControl { * ref */ public boolean canUpload() { - return canPerform(READ, (short) 2); + return getProjectControl() + .controlForRef("refs/for/" + getRefName()) + .canPerform(Permission.PUSH); } /** @return true if this user can submit merge patch sets to this ref */ public boolean canUploadMerges() { - return canPerform(READ, (short) 3); + return getProjectControl() + .controlForRef("refs/for/" + getRefName()) + .canPerform(Permission.PUSH_MERGE); } /** @return true if this user can submit patch sets to this ref */ public boolean canSubmit() { - return canPerform(ApprovalCategory.SUBMIT, (short) 1); + return canPerform(Permission.SUBMIT); } /** @return true if the user can update the reference as a fast-forward. */ public boolean canUpdate() { - return canPerform(PUSH_HEAD, PUSH_HEAD_UPDATE); + return canPerform(Permission.PUSH); } /** @return true if the user can rewind (force push) the reference. */ public boolean canForceUpdate() { - return canPerform(PUSH_HEAD, PUSH_HEAD_REPLACE) || canDelete(); + return canPushWithForce() || canDelete(); + } + + private boolean canPushWithForce() { + for (PermissionRule rule : access(Permission.PUSH)) { + if (rule.getForce()) { + return true; + } + } + return false; } /** @@ -183,7 +187,7 @@ public class RefControl { } if (object instanceof RevCommit) { - return owner || canPerform(PUSH_HEAD, PUSH_HEAD_CREATE); + return owner || canPerform(Permission.CREATE); } else if (object instanceof RevTag) { final RevTag tag = (RevTag) object; @@ -205,7 +209,7 @@ public class RefControl { } else { valid = false; } - if (!valid && !owner && !canPerform(FORGE_IDENTITY, FORGE_COMMITTER)) { + if (!valid && !owner && !canForgeCommitter()) { return false; } } @@ -214,9 +218,9 @@ public class RefControl { // than if it doesn't have a PGP signature. // if (tag.getFullMessage().contains("-----BEGIN PGP SIGNATURE-----\n")) { - return owner || canPerform(PUSH_TAG, PUSH_TAG_SIGNED); + return owner || canPerform(Permission.PUSH_TAG); } else { - return owner || canPerform(PUSH_TAG, PUSH_TAG_ANNOTATED); + return owner || canPerform(Permission.PUSH_TAG); } } else { @@ -233,10 +237,10 @@ public class RefControl { public boolean canDelete() { switch (getCurrentUser().getAccessPath()) { case WEB_UI: - return isOwner() || canPerform(PUSH_HEAD, PUSH_HEAD_REPLACE); + return isOwner() || canPushWithForce(); case GIT: - return canPerform(PUSH_HEAD, PUSH_HEAD_REPLACE); + return canPushWithForce(); default: return false; @@ -246,7 +250,7 @@ public class RefControl { /** @return true if this user can forge the author line in a commit. */ public boolean canForgeAuthor() { if (canForgeAuthor == null) { - canForgeAuthor = canPerform(FORGE_IDENTITY, FORGE_AUTHOR); + canForgeAuthor = canPerform(Permission.FORGE_AUTHOR); } return canForgeAuthor; } @@ -254,314 +258,103 @@ public class RefControl { /** @return true if this user can forge the committer line in a commit. */ public boolean canForgeCommitter() { if (canForgeCommitter == null) { - canForgeCommitter = canPerform(FORGE_IDENTITY, FORGE_COMMITTER); + canForgeCommitter = canPerform(Permission.FORGE_COMMITTER); } return canForgeCommitter; } /** @return true if this user can forge the server on the committer line. */ public boolean canForgeGerritServerIdentity() { - return canPerform(FORGE_IDENTITY, FORGE_SERVER); + return canPerform(Permission.FORGE_SERVER); } - public short normalize(ApprovalCategory.Id category, short score) { - short minAllowed = 0, maxAllowed = 0; - for (RefRight r : getApplicableRights(category)) { - if (getCurrentUser().getEffectiveGroups().contains(r.getAccountGroupUUID())) { - minAllowed = (short) Math.min(minAllowed, r.getMinValue()); - maxAllowed = (short) Math.max(maxAllowed, r.getMaxValue()); + /** All value ranges of any allowed label permission. */ + public List getLabelRanges() { + List r = new ArrayList(); + for (Map.Entry> e : permissions().entrySet()) { + if (Permission.isLabel(e.getKey())) { + r.add(toRange(e.getKey(), e.getValue())); } } - - if (score < minAllowed) { - score = minAllowed; - } - if (score > maxAllowed) { - score = maxAllowed; - } - return score; + return r; } - /** - * Convenience holder class used to map a ref pattern to the list of - * {@code RefRight}s that use it in the database. - */ - public final static class RefRightsForPattern { - private final List rights; - private boolean containsExclusive; - - public RefRightsForPattern() { - rights = new ArrayList(); - containsExclusive = false; + /** The range of permitted values associated with a label permission. */ + public PermissionRange getRange(String permission) { + if (Permission.isLabel(permission)) { + return toRange(permission, access(permission)); } + return null; + } - public void addRight(RefRight right) { - rights.add(right); - if (right.isExclusive()) { - containsExclusive = true; - } + private static PermissionRange toRange(String permissionName, List ruleList) { + int min = 0; + int max = 0; + for (PermissionRule rule : ruleList) { + min = Math.min(min, rule.getMin()); + max = Math.max(max, rule.getMax()); } + return new PermissionRange(permissionName, min, max); + } - public List getRights() { - return Collections.unmodifiableList(rights); - } + /** True if the user has this permission. Works only for non labels. */ + boolean canPerform(String permissionName) { + return !access(permissionName).isEmpty(); + } - public boolean containsExclusive() { - return containsExclusive; - } + /** Rules for the given permission, or the empty list. */ + private List access(String permissionName) { + List r = permissions().get(permissionName); + return r != null ? r : Collections. emptyList(); + } - /** - * Returns The max allowed value for this ref pattern for all specified - * groups. - * - * @param groups The groups of the user - * @return The allowed value for this ref for all the specified groups - */ - private boolean allowedValueForRef(Set groups, short level) { - for (RefRight right : rights) { - if (groups.contains(right.getAccountGroupUUID()) - && right.getMaxValue() >= level) { - return true; + /** All rules that pertain to this user, on this reference. */ + private Map> permissions() { + if (permissions == null) { + List sections = new ArrayList(); + for (AccessSection section : projectControl.access()) { + if (appliesToRef(section)) { + sections.add(section); } } - return false; - } - } + Collections.sort(sections, new MostSpecificComparator(getRefName())); - boolean canPerform(ApprovalCategory.Id actionId, short level) { - final Set groups = getCurrentUser().getEffectiveGroups(); + Set seen = new HashSet(); + Set exclusiveGroupPermissions = new HashSet(); - List allRights = new ArrayList(); - allRights.addAll(getAllRights(actionId)); + permissions = new HashMap>(); + for (AccessSection section : sections) { + for (Permission permission : section.getPermissions()) { + if (exclusiveGroupPermissions.contains(permission.getName())) { + continue; + } - SortedMap perPatternRights = - sortedRightsByPattern(allRights); - - for (RefRightsForPattern right : perPatternRights.values()) { - if (right.allowedValueForRef(groups, level)) { - return true; - } - if (right.containsExclusive() && !actionId.equals(OWN)) { - break; - } - } - return false; - } - - /** - * Order the Ref Pattern by the most specific. This sort is done by: - *

    - *
  • 1 - The minor value of Levenshtein string distance between the branch - * name and the regex string shortest example. A shorter distance is a more - * specific match. - *
  • 2 - Finites first, infinities after. - *
  • 3 - Number of transitions. - *
  • 4 - Length of the expression text. - *
- * - * Levenshtein distance is a measure of the similarity between two strings. - * The distance is the number of deletions, insertions, or substitutions - * required to transform one string into another. - * - * For example, if given refs/heads/m* and refs/heads/*, the distances are 5 - * and 6. It means that refs/heads/m* is more specific because it's closer to - * refs/heads/master than refs/heads/*. - * - * Another example could be refs/heads/* and refs/heads/[a-zA-Z]*, the - * distances are both 6. Both are infinite, but refs/heads/[a-zA-Z]* has more - * transitions, which after all turns it more specific. - */ - private final Comparator BY_MOST_SPECIFIC_SORT = - new Comparator() { - public int compare(final String pattern1, final String pattern2) { - int cmp = distance(pattern1) - distance(pattern2); - if (cmp == 0) { - boolean p1_finite = finite(pattern1); - boolean p2_finite = finite(pattern2); - - if (p1_finite && !p2_finite) { - cmp = -1; - } else if (!p1_finite && p2_finite) { - cmp = 1; - } else /* if (f1 == f2) */{ - cmp = 0; + for (PermissionRule rule : permission.getRules()) { + if (matchGroup(rule.getGroup().getUUID())) { + SeenRule s = new SeenRule(section, permission, rule); + if (seen.add(s) && !rule.getDeny()) { + List r = permissions.get(permission.getName()); + if (r == null) { + r = new ArrayList(2); + permissions.put(permission.getName(), r); + } + r.add(rule); + } } } - if (cmp == 0) { - cmp = transitions(pattern1) - transitions(pattern2); - } - if (cmp == 0) { - cmp = pattern2.length() - pattern1.length(); - } - return cmp; - } - private int distance(String pattern) { - String example; - if (isRE(pattern)) { - example = shortestExample(pattern); - - } else if (pattern.endsWith("/*")) { - example = pattern.substring(0, pattern.length() - 1) + '1'; - - } else if (pattern.equals(getRefName())) { - return 0; - - } else { - return Math.max(pattern.length(), getRefName().length()); - } - return StringUtils.getLevenshteinDistance(example, getRefName()); - } - - private boolean finite(String pattern) { - if (isRE(pattern)) { - return toRegExp(pattern).toAutomaton().isFinite(); - - } else if (pattern.endsWith("/*")) { - return false; - - } else { - return true; + if (permission.getExclusiveGroup()) { + exclusiveGroupPermissions.add(permission.getName()); } } - - private int transitions(String pattern) { - if (isRE(pattern)) { - return toRegExp(pattern).toAutomaton().getNumberOfTransitions(); - - } else if (pattern.endsWith("/*")) { - return pattern.length(); - - } else { - return pattern.length(); - } - } - }; - - /** - * Sorts all given rights into a map, ordered by descending length of - * ref pattern. - * - * For example, if given the following rights in argument: - * - * ["refs/heads/master", group1, -1, +1], - * ["refs/heads/master", group2, -2, +2], - * ["refs/heads/*", group3, -1, +1] - * ["refs/heads/stable", group2, -1, +1] - * - * Then the following map is returned: - * "refs/heads/master" => { - * ["refs/heads/master", group1, -1, +1], - * ["refs/heads/master", group2, -2, +2] - * } - * "refs/heads/stable" => {["refs/heads/stable", group2, -1, +1]} - * "refs/heads/*" => {["refs/heads/*", group3, -1, +1]} - * - * @param actionRights - * @return A sorted map keyed off the ref pattern of all rights. - */ - private SortedMap sortedRightsByPattern( - List actionRights) { - SortedMap rights = - new TreeMap(BY_MOST_SPECIFIC_SORT); - for (RefRight actionRight : actionRights) { - RefRightsForPattern patternRights = - rights.get(actionRight.getRefPattern()); - if (patternRights == null) { - patternRights = new RefRightsForPattern(); - rights.put(actionRight.getRefPattern(), patternRights); - } - patternRights.addRight(actionRight); - } - return rights; - } - - private List getAllRights(ApprovalCategory.Id actionId) { - final List allRefRights = filter(getProjectState().getAllRights(actionId, true)); - return resolveOwnerGroups(allRefRights); - } - - /** - * Returns all applicable rights for a given approval category. - * - * Applicable rights are defined as the list of {@code RefRight}s which match - * the ref for which this object was created, stopping the ref wildcard - * matching when an exclusive ref right was encountered, for the given - * approval category. - * @param id The {@link ApprovalCategory.Id}. - * @return All applicable rights. - */ - public List getApplicableRights(final ApprovalCategory.Id id) { - List l = new ArrayList(); - l.addAll(getAllRights(id)); - SortedMap perPatternRights = - sortedRightsByPattern(l); - List applicable = new ArrayList(); - for (RefRightsForPattern patternRights : perPatternRights.values()) { - applicable.addAll(patternRights.getRights()); - if (patternRights.containsExclusive()) { - break; } } - return Collections.unmodifiableList(applicable); + return permissions; } - /** - * Resolves all refRights which assign privileges to the 'Project Owners' - * group. All other refRights stay unchanged. - * - * @param refRights refRights to be resolved - * @return the resolved refRights - */ - private List resolveOwnerGroups(final List refRights) { - final List resolvedRefRights = - new ArrayList(refRights.size()); - for (final RefRight refRight : refRights) { - resolvedRefRights.addAll(resolveOwnerGroups(refRight)); - } - return resolvedRefRights; - } + private boolean appliesToRef(AccessSection section) { + String refPattern = section.getRefPattern(); - /** - * Checks if the given refRight assigns privileges to the 'Project Owners' - * group. - * If yes, resolves the 'Project Owners' group to the concrete groups that - * own the project and creates new refRights for the concrete owner groups - * which are returned. - * If no, the given refRight is returned unchanged. - * - * @param refRight refRight - * @return the resolved refRights - */ - private Set resolveOwnerGroups(final RefRight refRight) { - final Set resolvedRefRights = new HashSet(); - if (AccountGroup.PROJECT_OWNERS.equals(refRight.getAccountGroupUUID())) { - for (final AccountGroup.UUID ownerGroup : getProjectState().getAllOwners()) { - if (!AccountGroup.PROJECT_OWNERS.equals(ownerGroup)) { - resolvedRefRights.add(new RefRight(refRight, ownerGroup)); - } - } - } else { - resolvedRefRights.add(refRight); - } - return resolvedRefRights; - } - - private List filter(Collection all) { - List mine = new ArrayList(all.size()); - for (RefRight right : all) { - if (matches(right.getRefPattern())) { - mine.add(right); - } - } - return mine; - } - - private ProjectState getProjectState() { - return projectControl.getProjectState(); - } - - private boolean matches(String refPattern) { if (isTemplate(refPattern)) { ParamertizedString template = new ParamertizedString(refPattern); HashMap p = new HashMap(); @@ -596,6 +389,18 @@ public class RefControl { } } + private boolean matchGroup(AccountGroup.UUID uuid) { + Set userGroups = getCurrentUser().getEffectiveGroups(); + + if (AccountGroup.PROJECT_OWNERS.equals(uuid)) { + ProjectState state = projectControl.getProjectState(); + return CollectionsUtil.isAnyIncludedIn(state.getAllOwners(), userGroups); + + } else { + return userGroups.contains(uuid); + } + } + private static boolean isTemplate(String refPattern) { return 0 <= refPattern.indexOf("${"); } @@ -608,7 +413,7 @@ public class RefControl { } private static boolean isRE(String refPattern) { - return refPattern.startsWith(RefRight.REGEX_PREFIX); + return refPattern.startsWith(AccessSection.REGEX_PREFIX); } public static String shortestExample(String pattern) { @@ -627,4 +432,143 @@ public class RefControl { } return new RegExp(refPattern, RegExp.NONE); } + + /** Tracks whether or not a permission has been overridden. */ + private static class SeenRule { + final String refPattern; + final String permissionName; + final AccountGroup.UUID group; + + SeenRule(AccessSection section, Permission permission, PermissionRule rule) { + refPattern = section.getRefPattern(); + permissionName = permission.getName(); + group = rule.getGroup().getUUID(); + } + + @Override + public int hashCode() { + int hc = refPattern.hashCode(); + hc = hc * 31 + permissionName.hashCode(); + if (group != null) { + hc = hc * 31 + group.hashCode(); + } + return hc; + } + + @Override + public boolean equals(Object other) { + if (other instanceof SeenRule) { + SeenRule a = this; + SeenRule b = (SeenRule) other; + return a.refPattern.equals(b.refPattern) // + && a.permissionName.equals(b.permissionName) // + && eq(a.group, b.group); + } + return false; + } + + private boolean eq(AccountGroup.UUID a, AccountGroup.UUID b) { + return a != null && b != null && a.equals(b); + } + } + + /** + * Order the Ref Pattern by the most specific. This sort is done by: + *
    + *
  • 1 - The minor value of Levenshtein string distance between the branch + * name and the regex string shortest example. A shorter distance is a more + * specific match. + *
  • 2 - Finites first, infinities after. + *
  • 3 - Number of transitions. + *
  • 4 - Length of the expression text. + *
+ * + * Levenshtein distance is a measure of the similarity between two strings. + * The distance is the number of deletions, insertions, or substitutions + * required to transform one string into another. + * + * For example, if given refs/heads/m* and refs/heads/*, the distances are 5 + * and 6. It means that refs/heads/m* is more specific because it's closer to + * refs/heads/master than refs/heads/*. + * + * Another example could be refs/heads/* and refs/heads/[a-zA-Z]*, the + * distances are both 6. Both are infinite, but refs/heads/[a-zA-Z]* has more + * transitions, which after all turns it more specific. + */ + private static final class MostSpecificComparator implements + Comparator { + private final String refName; + + MostSpecificComparator(String refName) { + this.refName = refName; + } + + public int compare(AccessSection a, AccessSection b) { + return compare(a.getRefPattern(), b.getRefPattern()); + } + + private int compare(final String pattern1, final String pattern2) { + int cmp = distance(pattern1) - distance(pattern2); + if (cmp == 0) { + boolean p1_finite = finite(pattern1); + boolean p2_finite = finite(pattern2); + + if (p1_finite && !p2_finite) { + cmp = -1; + } else if (!p1_finite && p2_finite) { + cmp = 1; + } else /* if (f1 == f2) */{ + cmp = 0; + } + } + if (cmp == 0) { + cmp = transitions(pattern1) - transitions(pattern2); + } + if (cmp == 0) { + cmp = pattern2.length() - pattern1.length(); + } + return cmp; + } + + private int distance(String pattern) { + String example; + if (isRE(pattern)) { + example = shortestExample(pattern); + + } else if (pattern.endsWith("/*")) { + example = pattern.substring(0, pattern.length() - 1) + '1'; + + } else if (pattern.equals(refName)) { + return 0; + + } else { + return Math.max(pattern.length(), refName.length()); + } + return StringUtils.getLevenshteinDistance(example, refName); + } + + private boolean finite(String pattern) { + if (isRE(pattern)) { + return toRegExp(pattern).toAutomaton().isFinite(); + + } else if (pattern.endsWith("/*")) { + return false; + + } else { + return true; + } + } + + private int transitions(String pattern) { + if (isRE(pattern)) { + return toRegExp(pattern).toAutomaton().getNumberOfTransitions(); + + } else if (pattern.endsWith("/*")) { + return pattern.length(); + + } else { + return pattern.length(); + } + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java index e76c2782f0..c5c974d2be 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.query.change; import com.google.gerrit.common.data.ApprovalType; import com.google.gerrit.common.data.ApprovalTypes; +import com.google.gerrit.common.data.Permission; import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.PatchSetApproval; import com.google.gerrit.reviewdb.ReviewDb; @@ -33,48 +34,54 @@ class LabelPredicate extends OperatorPredicate { private static enum Test { EQ { @Override - public boolean match(short psValue, short expValue) { + public boolean match(int psValue, int expValue) { return psValue == expValue; } }, GT_EQ { @Override - public boolean match(short psValue, short expValue) { + public boolean match(int psValue, int expValue) { return psValue >= expValue; } }, LT_EQ { @Override - public boolean match(short psValue, short expValue) { + public boolean match(int psValue, int expValue) { return psValue <= expValue; } }; - abstract boolean match(short psValue, short expValue); + abstract boolean match(int psValue, int expValue); } - private static ApprovalCategory.Id category(ApprovalTypes types, String toFind) { - if (types.getApprovalType(new ApprovalCategory.Id(toFind)) != null) { - return new ApprovalCategory.Id(toFind); + private static ApprovalCategory category(ApprovalTypes types, String toFind) { + if (types.byLabel(toFind) != null) { + return types.byLabel(toFind).getCategory(); + } + + if (types.byId(new ApprovalCategory.Id(toFind)) != null) { + return types.byId(new ApprovalCategory.Id(toFind)).getCategory(); } for (ApprovalType at : types.getApprovalTypes()) { - String name = at.getCategory().getName(); - if (toFind.equalsIgnoreCase(name)) { - return at.getCategory().getId(); + ApprovalCategory category = at.getCategory(); - } else if (toFind.equalsIgnoreCase(name.replace(" ", ""))) { - return at.getCategory().getId(); + if (toFind.equalsIgnoreCase(category.getName())) { + return category; + + } else if (toFind.equalsIgnoreCase(category.getName().replace(" ", ""))) { + return category; } } for (ApprovalType at : types.getApprovalTypes()) { - if (toFind.equalsIgnoreCase(at.getCategory().getAbbreviatedName())) { - return at.getCategory().getId(); + ApprovalCategory category = at.getCategory(); + if (toFind.equalsIgnoreCase(category.getAbbreviatedName())) { + return category; } } - return new ApprovalCategory.Id(toFind); + return new ApprovalCategory(new ApprovalCategory.Id(toFind), toFind); } private static Test op(String op) { @@ -92,19 +99,20 @@ class LabelPredicate extends OperatorPredicate { } } - private static short value(String value) { + private static int value(String value) { if (value.startsWith("+")) { value = value.substring(1); } - return Short.parseShort(value); + return Integer.parseInt(value); } private final ChangeControl.GenericFactory ccFactory; private final IdentifiedUser.GenericFactory userFactory; private final Provider dbProvider; private final Test test; - private final ApprovalCategory.Id category; - private final short expVal; + private final ApprovalCategory category; + private final String permissionName; + private final int expVal; LabelPredicate(ChangeControl.GenericFactory ccFactory, IdentifiedUser.GenericFactory userFactory, Provider dbProvider, @@ -131,13 +139,15 @@ class LabelPredicate extends OperatorPredicate { test = Test.EQ; expVal = 1; } + + this.permissionName = Permission.forLabel(category.getLabelName()); } @Override public boolean match(final ChangeData object) throws OrmException { for (PatchSetApproval p : object.currentApprovals(dbProvider)) { if (p.getCategoryId().equals(category)) { - short psVal = p.getValue(); + int psVal = p.getValue(); if (test.match(psVal, expVal)) { // Double check the value is still permitted for the user. // @@ -149,7 +159,7 @@ class LabelPredicate extends OperatorPredicate { // continue; } - psVal = cc.normalize(category, psVal); + psVal = cc.getRange(permissionName).squash(psVal); } catch (NoSuchChangeException e) { // The project has disappeared. // diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java index 295e5b7a54..c5ca3085a2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java @@ -14,13 +14,16 @@ package com.google.gerrit.server.schema; +import com.google.gerrit.common.Version; +import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.AccountGroup; import com.google.gerrit.reviewdb.AccountGroupName; import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.ApprovalCategoryValue; import com.google.gerrit.reviewdb.CurrentSchemaVersion; import com.google.gerrit.reviewdb.Project; -import com.google.gerrit.reviewdb.RefRight; import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.reviewdb.SystemConfig; import com.google.gerrit.server.GerritPersonIdent; @@ -31,8 +34,6 @@ import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.NoReplication; import com.google.gerrit.server.git.ProjectConfig; -import com.google.gerrit.server.workflow.NoOpFunction; -import com.google.gerrit.server.workflow.SubmitFunction; import com.google.gwtjsonrpc.server.SignedToken; import com.google.gwtorm.client.OrmException; import com.google.gwtorm.jdbc.JdbcExecutor; @@ -69,6 +70,11 @@ public class SchemaCreator { private final ScriptRunner index_postgres; private final ScriptRunner mysql_nextval; + private AccountGroup admin; + private AccountGroup anonymous; + private AccountGroup registered; + private AccountGroup owners; + @Inject public SchemaCreator(final SitePaths site, @Current final SchemaVersion version, final GitRepositoryManager mgr, @@ -103,14 +109,8 @@ public class SchemaCreator { db.schemaVersion().insert(Collections.singleton(sVer)); final SystemConfig sConfig = initSystemConfig(db); - initOwnerCategory(db); - initReadCategory(db, sConfig); initVerifiedCategory(db); initCodeReviewCategory(db, sConfig); - initSubmitCategory(db); - initPushTagCategory(db); - initPushUpdateBranchCategory(db); - initForgeIdentityCategory(db, sConfig); if (mgr != null) { // TODO This should never be null when initializing a site. @@ -145,14 +145,14 @@ public class SchemaCreator { } private SystemConfig initSystemConfig(final ReviewDb c) throws OrmException { - final AccountGroup admin = newGroup(c, "Administrators", null); + admin = newGroup(c, "Administrators", null); admin.setDescription("Gerrit Site Administrators"); admin.setType(AccountGroup.Type.INTERNAL); c.accountGroups().insert(Collections.singleton(admin)); c.accountGroupNames().insert( Collections.singleton(new AccountGroupName(admin))); - final AccountGroup anonymous = + anonymous = newGroup(c, "Anonymous Users", AccountGroup.ANONYMOUS_USERS); anonymous.setDescription("Any user, signed-in or not"); anonymous.setOwnerGroupId(admin.getId()); @@ -161,7 +161,7 @@ public class SchemaCreator { c.accountGroupNames().insert( Collections.singleton(new AccountGroupName(anonymous))); - final AccountGroup registered = + registered = newGroup(c, "Registered Users", AccountGroup.REGISTERED_USERS); registered.setDescription("Any signed-in user"); registered.setOwnerGroupId(admin.getId()); @@ -178,8 +178,7 @@ public class SchemaCreator { c.accountGroupNames().insert( Collections.singleton(new AccountGroupName(batchUsers))); - final AccountGroup owners = - newGroup(c, "Project Owners", AccountGroup.PROJECT_OWNERS); + owners = newGroup(c, "Project Owners", AccountGroup.PROJECT_OWNERS); owners.setDescription("Any owner of the project"); owners.setOwnerGroupId(admin.getId()); owners.setType(AccountGroup.Type.SYSTEM); @@ -236,7 +235,29 @@ public class SchemaCreator { p.setDescription("Rights inherited by all other projects"); p.setUseContributorAgreements(false); - md.setMessage("Created project\n"); + AccessSection all = config.getAccessSection(AccessSection.ALL, true); + AccessSection heads = config.getAccessSection(AccessSection.HEADS, true); + AccessSection meta = config.getAccessSection(GitRepositoryManager.REF_CONFIG, true); + + PermissionRule review = rule(config, registered); + review.setRange(-1, 1); + heads.getPermission(Permission.LABEL + "Code-Review", true).add(review); + + all.getPermission(Permission.READ, true) // + .add(rule(config, admin)); + all.getPermission(Permission.READ, true) // + .add(rule(config, anonymous)); + + config.getAccessSection("refs/for/" + AccessSection.ALL, true) // + .getPermission(Permission.PUSH, true) // + .add(rule(config, registered)); + all.getPermission(Permission.FORGE_AUTHOR, true) // + .add(rule(config, registered)); + + meta.getPermission(Permission.READ, true) // + .add(rule(config, owners)); + + md.setMessage("Initialized Gerrit Code Review " + Version.getVersion()); if (!config.commit(md)) { throw new IOException("Cannot create " + DEFAULT_WILD_NAME.get()); } @@ -245,6 +266,10 @@ public class SchemaCreator { } } + private PermissionRule rule(ProjectConfig config, AccountGroup group) { + return new PermissionRule(config.resolve(group)); + } + private void initVerifiedCategory(final ReviewDb c) throws OrmException { final ApprovalCategory cat; final ArrayList vals; @@ -277,143 +302,6 @@ public class SchemaCreator { vals.add(value(cat, -2, "Do not submit")); c.approvalCategories().insert(Collections.singleton(cat)); c.approvalCategoryValues().insert(vals); - - final RefRight approve = - new RefRight(new RefRight.Key(DEFAULT_WILD_NAME, - new RefRight.RefPattern("refs/heads/*"), cat.getId(), - sConfig.registeredGroupId)); - approve.setMaxValue((short) 1); - approve.setMinValue((short) -1); - c.refRights().insert(Collections.singleton(approve)); - } - - private void initOwnerCategory(final ReviewDb c) throws OrmException { - final ApprovalCategory cat; - final ArrayList vals; - - cat = new ApprovalCategory(ApprovalCategory.OWN, "Owner"); - cat.setPosition((short) -1); - cat.setFunctionName(NoOpFunction.NAME); - vals = new ArrayList(); - vals.add(value(cat, 1, "Administer All Settings")); - c.approvalCategories().insert(Collections.singleton(cat)); - c.approvalCategoryValues().insert(vals); - } - - private void initReadCategory(final ReviewDb c, final SystemConfig sConfig) - throws OrmException { - final ApprovalCategory cat; - final ArrayList vals; - - cat = new ApprovalCategory(ApprovalCategory.READ, "Read Access"); - cat.setPosition((short) -1); - cat.setFunctionName(NoOpFunction.NAME); - vals = new ArrayList(); - vals.add(value(cat, 3, "Upload merges permission")); - vals.add(value(cat, 2, "Upload permission")); - vals.add(value(cat, 1, "Read access")); - vals.add(value(cat, -1, "No access")); - c.approvalCategories().insert(Collections.singleton(cat)); - c.approvalCategoryValues().insert(vals); - - final RefRight.RefPattern pattern = new RefRight.RefPattern(RefRight.ALL); - { - final RefRight read = - new RefRight(new RefRight.Key(DEFAULT_WILD_NAME, pattern, - cat.getId(), sConfig.anonymousGroupId)); - read.setMaxValue((short) 1); - read.setMinValue((short) 1); - c.refRights().insert(Collections.singleton(read)); - } - { - final RefRight read = - new RefRight(new RefRight.Key(DEFAULT_WILD_NAME, pattern, - cat.getId(), sConfig.registeredGroupId)); - read.setMaxValue((short) 2); - read.setMinValue((short) 1); - c.refRights().insert(Collections.singleton(read)); - } - { - final RefRight read = - new RefRight(new RefRight.Key(DEFAULT_WILD_NAME, pattern, - cat.getId(), sConfig.adminGroupId)); - read.setMaxValue((short) 1); - read.setMinValue((short) 1); - c.refRights().insert(Collections.singleton(read)); - } - } - - private void initSubmitCategory(final ReviewDb c) throws OrmException { - final ApprovalCategory cat; - final ArrayList vals; - - cat = new ApprovalCategory(ApprovalCategory.SUBMIT, "Submit"); - cat.setPosition((short) -1); - cat.setFunctionName(SubmitFunction.NAME); - vals = new ArrayList(); - vals.add(value(cat, 1, "Submit")); - c.approvalCategories().insert(Collections.singleton(cat)); - c.approvalCategoryValues().insert(vals); - } - - private void initPushTagCategory(final ReviewDb c) throws OrmException { - final ApprovalCategory cat; - final ArrayList vals; - - cat = new ApprovalCategory(ApprovalCategory.PUSH_TAG, "Push Tag"); - cat.setPosition((short) -1); - cat.setFunctionName(NoOpFunction.NAME); - vals = new ArrayList(); - vals.add(value(cat, ApprovalCategory.PUSH_TAG_SIGNED, "Create Signed Tag")); - vals.add(value(cat, ApprovalCategory.PUSH_TAG_ANNOTATED, - "Create Annotated Tag")); - c.approvalCategories().insert(Collections.singleton(cat)); - c.approvalCategoryValues().insert(vals); - } - - private void initPushUpdateBranchCategory(final ReviewDb c) - throws OrmException { - final ApprovalCategory cat; - final ArrayList vals; - - cat = new ApprovalCategory(ApprovalCategory.PUSH_HEAD, "Push Branch"); - cat.setPosition((short) -1); - cat.setFunctionName(NoOpFunction.NAME); - vals = new ArrayList(); - vals.add(value(cat, ApprovalCategory.PUSH_HEAD_UPDATE, "Update Branch")); - vals.add(value(cat, ApprovalCategory.PUSH_HEAD_CREATE, "Create Branch")); - vals.add(value(cat, ApprovalCategory.PUSH_HEAD_REPLACE, - "Force Push Branch; Delete Branch")); - c.approvalCategories().insert(Collections.singleton(cat)); - c.approvalCategoryValues().insert(vals); - } - - private void initForgeIdentityCategory(final ReviewDb c, - final SystemConfig sConfig) throws OrmException { - final ApprovalCategory cat; - final ArrayList values; - - cat = - new ApprovalCategory(ApprovalCategory.FORGE_IDENTITY, "Forge Identity"); - cat.setPosition((short) -1); - cat.setFunctionName(NoOpFunction.NAME); - values = new ArrayList(); - values.add(value(cat, ApprovalCategory.FORGE_AUTHOR, - "Forge Author Identity")); - values.add(value(cat, ApprovalCategory.FORGE_COMMITTER, - "Forge Committer or Tagger Identity")); - values.add(value(cat, ApprovalCategory.FORGE_SERVER, - "Forge Gerrit Code Review Server Identity")); - c.approvalCategories().insert(Collections.singleton(cat)); - c.approvalCategoryValues().insert(values); - - RefRight right = - new RefRight(new RefRight.Key(sConfig.wildProjectName, - new RefRight.RefPattern(RefRight.ALL), - ApprovalCategory.FORGE_IDENTITY, sConfig.registeredGroupId)); - right.setMinValue(ApprovalCategory.FORGE_AUTHOR); - right.setMaxValue(ApprovalCategory.FORGE_AUTHOR); - c.refRights().insert(Collections.singleton(right)); } private static ApprovalCategoryValue value(final ApprovalCategory cat, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_53.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_53.java index 87f881ceb3..8a04071ac9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_53.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_53.java @@ -14,7 +14,24 @@ package com.google.gerrit.server.schema; +import static com.google.gerrit.common.data.Permission.CREATE; +import static com.google.gerrit.common.data.Permission.FORGE_AUTHOR; +import static com.google.gerrit.common.data.Permission.FORGE_COMMITTER; +import static com.google.gerrit.common.data.Permission.FORGE_SERVER; +import static com.google.gerrit.common.data.Permission.LABEL; +import static com.google.gerrit.common.data.Permission.OWNER; +import static com.google.gerrit.common.data.Permission.PUSH; +import static com.google.gerrit.common.data.Permission.PUSH_MERGE; +import static com.google.gerrit.common.data.Permission.PUSH_TAG; +import static com.google.gerrit.common.data.Permission.READ; +import static com.google.gerrit.common.data.Permission.SUBMIT; + +import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.GroupReference; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.Project; import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.reviewdb.SystemConfig; @@ -38,6 +55,7 @@ import java.io.IOException; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -48,7 +66,19 @@ class Schema_53 extends SchemaVersion { private final PersonIdent serverUser; private SystemConfig systemConfig; - private Map groupMap; + private Map groupMap; + private Map categoryMap; + private GroupReference projectOwners; + + private Map parentsByProject; + private Map> rightsByProject; + + private final String OLD_SUBMIT = "SUBM"; + private final String OLD_READ = "READ"; + private final String OLD_OWN = "OWN"; + private final String OLD_PUSH_TAG = "pTAG"; + private final String OLD_PUSH_HEAD = "pHD"; + private final String OLD_FORGE_IDENTITY = "FORG"; @Inject Schema_53(Provider prior, GitRepositoryManager mgr, @@ -62,16 +92,33 @@ class Schema_53 extends SchemaVersion { protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException { systemConfig = db.systemConfig().get(new SystemConfig.Key()); + categoryMap = db.approvalCategories().toMap(db.approvalCategories().all()); + assignGroupUUIDs(db); + readOldRefRights(db); + readProjectParents(db); exportProjectConfig(db); + + deleteActionCategories(db); + } + + private void deleteActionCategories(ReviewDb db) throws OrmException { + List delete = new ArrayList(); + for (ApprovalCategory category : categoryMap.values()) { + if (category.getPosition() < 0) { + delete.add(category); + } + } + db.approvalCategories().delete(delete); } private void assignGroupUUIDs(ReviewDb db) throws OrmException { - groupMap = new HashMap(); + groupMap = new HashMap(); List groups = db.accountGroups().all().toList(); for (AccountGroup g : groups) { if (g.getId().equals(systemConfig.ownerGroupId)) { g.setGroupUUID(AccountGroup.PROJECT_OWNERS); + projectOwners = GroupReference.forGroup(g); } else if (g.getId().equals(systemConfig.anonymousGroupId)) { g.setGroupUUID(AccountGroup.ANONYMOUS_USERS); @@ -82,7 +129,7 @@ class Schema_53 extends SchemaVersion { } else { g.setGroupUUID(GroupUUID.make(g.getName(), serverUser)); } - groupMap.put(g.getId(), g); + groupMap.put(g.getId(), GroupReference.forGroup(g)); } db.accountGroups().update(groups); @@ -92,7 +139,7 @@ class Schema_53 extends SchemaVersion { } private AccountGroup.UUID toUUID(AccountGroup.Id id) { - return groupMap.get(id).getGroupUUID(); + return groupMap.get(id).getUUID(); } private void exportProjectConfig(ReviewDb db) throws OrmException, @@ -123,6 +170,16 @@ class Schema_53 extends SchemaVersion { ProjectConfig config = ProjectConfig.read(md); loadProject(rs, config.getProject()); + config.getAccessSections().clear(); + convertRights(config); + + // Grant out read on the config branch by default. + // + if (config.getProject().getNameKey().equals(systemConfig.wildProjectName)) { + AccessSection meta = config.getAccessSection(GitRepositoryManager.REF_CONFIG, true); + Permission read = meta.getPermission(READ, true); + read.getRule(config.resolve(projectOwners), true); + } md.setMessage("Import project configuration from SQL\n"); if (!config.commit(md)) { @@ -169,4 +226,248 @@ class Schema_53 extends SchemaVersion { project.setUseContentMerge("Y".equals(rs.getString("use_content_merge"))); project.setParentName(rs.getString("parent_name")); } + + private void readOldRefRights(ReviewDb db) throws SQLException { + rightsByProject = new HashMap>(); + + Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); + ResultSet rs = stmt.executeQuery("SELECT * FROM ref_rights"); + while (rs.next()) { + OldRefRight right = new OldRefRight(rs); + if (right.group == null || right.category == null) { + continue; + } + + List list; + + list = rightsByProject.get(right.project); + if (list == null) { + list = new ArrayList(); + rightsByProject.put(right.project, list); + } + list.add(right); + } + rs.close(); + stmt.close(); + } + + private void readProjectParents(ReviewDb db) throws SQLException { + parentsByProject = new HashMap(); + Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); + ResultSet rs = stmt.executeQuery("SELECT * FROM projects"); + while (rs.next()) { + String name = rs.getString("name"); + String parent_name = rs.getString("parent_name"); + if (parent_name == null) { + parent_name = systemConfig.wildProjectName.get(); + } + parentsByProject.put(new Project.NameKey(name), // + new Project.NameKey(parent_name)); + } + rs.close(); + stmt.close(); + } + + private void convertRights(ProjectConfig config) { + List myRights = + rightsByProject.get(config.getProject().getNameKey()); + if (myRights == null) { + return; + } + + for (OldRefRight old : myRights) { + AccessSection section = config.getAccessSection(old.ref_pattern, true); + GroupReference group = config.resolve(old.group); + + if (OLD_SUBMIT.equals(old.category)) { + PermissionRule submit = rule(group); + submit.setDeny(old.max_value <= 0); + add(section, SUBMIT, old.exclusive, submit); + + } else if (OLD_READ.equals(old.category)) { + if (old.exclusive) { + section.getPermission(READ, true).setExclusiveGroup(true); + newChangePermission(config, old.ref_pattern).setExclusiveGroup(true); + } + + PermissionRule read = rule(group); + read.setDeny(old.max_value <= 0); + add(section, READ, old.exclusive, read); + + if (3 <= old.max_value) { + newMergePermission(config, old.ref_pattern).add(rule(group)); + } else if (3 <= inheritedMax(config, old)) { + newMergePermission(config, old.ref_pattern).add(deny(group)); + } + + if (2 <= old.max_value) { + newChangePermission(config, old.ref_pattern).add(rule(group)); + } else if (2 <= inheritedMax(config, old)) { + newChangePermission(config, old.ref_pattern).add(deny(group)); + } + + } else if (OLD_OWN.equals(old.category)) { + add(section, OWNER, false, rule(group)); + + } else if (OLD_PUSH_TAG.equals(old.category)) { + PermissionRule push = rule(group); + push.setDeny(old.max_value <= 0); + add(section, PUSH_TAG, old.exclusive, push); + + } else if (OLD_PUSH_HEAD.equals(old.category)) { + if (old.exclusive) { + section.getPermission(PUSH, true).setExclusiveGroup(true); + section.getPermission(CREATE, true).setExclusiveGroup(true); + } + + PermissionRule push = rule(group); + push.setDeny(old.max_value <= 0); + push.setForce(3 <= old.max_value); + add(section, PUSH, old.exclusive, push); + + if (2 <= old.max_value) { + add(section, CREATE, old.exclusive, rule(group)); + } else if (2 <= inheritedMax(config, old)) { + add(section, CREATE, old.exclusive, deny(group)); + } + + } else if (OLD_FORGE_IDENTITY.equals(old.category)) { + if (old.exclusive) { + section.getPermission(FORGE_AUTHOR, true).setExclusiveGroup(true); + section.getPermission(FORGE_COMMITTER, true).setExclusiveGroup(true); + section.getPermission(FORGE_SERVER, true).setExclusiveGroup(true); + } + + if (1 <= old.max_value) { + add(section, FORGE_AUTHOR, old.exclusive, rule(group)); + } + + if (2 <= old.max_value) { + add(section, FORGE_COMMITTER, old.exclusive, rule(group)); + } else if (2 <= inheritedMax(config, old)) { + add(section, FORGE_COMMITTER, old.exclusive, deny(group)); + } + + if (3 <= old.max_value) { + add(section, FORGE_SERVER, old.exclusive, rule(group)); + } else if (3 <= inheritedMax(config, old)) { + add(section, FORGE_SERVER, old.exclusive, deny(group)); + } + + } else { + PermissionRule rule = rule(group); + rule.setRange(old.min_value, old.max_value); + if (old.min_value == 0 && old.max_value == 0) { + rule.setDeny(true); + } + add(section, LABEL + varNameOf(old.category), old.exclusive, rule); + } + } + } + + private static Permission newChangePermission(ProjectConfig config, + String name) { + if (name.startsWith(AccessSection.REGEX_PREFIX)) { + name = AccessSection.REGEX_PREFIX + + "refs/for/" + + name.substring(AccessSection.REGEX_PREFIX.length()); + } else { + name = "refs/for/" + name; + } + return config.getAccessSection(name, true).getPermission(PUSH, true); + } + + private static Permission newMergePermission(ProjectConfig config, + String name) { + if (name.startsWith(AccessSection.REGEX_PREFIX)) { + name = AccessSection.REGEX_PREFIX + + "refs/for/" + + name.substring(AccessSection.REGEX_PREFIX.length()); + } else { + name = "refs/for/" + name; + } + return config.getAccessSection(name, true).getPermission(PUSH_MERGE, true); + } + + private static PermissionRule rule(GroupReference group) { + return new PermissionRule(group); + } + + private static PermissionRule deny(GroupReference group) { + PermissionRule rule = rule(group); + rule.setDeny(true); + return rule; + } + + private int inheritedMax(ProjectConfig config, OldRefRight old) { + int max = 0; + + String ref = old.ref_pattern; + String category = old.category; + AccountGroup.UUID group = old.group.getUUID(); + + Project.NameKey project = config.getProject().getParent(); + if (project == null) { + project = systemConfig.wildProjectName; + } + do { + List rights = rightsByProject.get(project); + if (rights != null) { + for (OldRefRight r : rights) { + if (r.ref_pattern.equals(ref) // + && r.group.getUUID().equals(group) // + && r.category.equals(category)) { + max = Math.max(max, r.max_value); + break; + } + } + } + project = parentsByProject.get(project); + } while (!project.equals(systemConfig.wildProjectName)); + + return max; + } + + private String varNameOf(String id) { + ApprovalCategory category = categoryMap.get(new ApprovalCategory.Id(id)); + if (category == null) { + category = new ApprovalCategory(new ApprovalCategory.Id(id), id); + } + return category.getLabelName(); + } + + private static void add(AccessSection section, String name, + boolean exclusive, PermissionRule rule) { + Permission p = section.getPermission(name, true); + if (exclusive) { + p.setExclusiveGroup(true); + } + p.add(rule); + } + + private class OldRefRight { + final int min_value; + final int max_value; + final String ref_pattern; + final boolean exclusive; + final GroupReference group; + final String category; + final Project.NameKey project; + + OldRefRight(ResultSet rs) throws SQLException { + min_value = rs.getInt("min_value"); + max_value = rs.getInt("max_value"); + project = new Project.NameKey(rs.getString("project_name")); + + String r = rs.getString("ref_pattern"); + exclusive = r.startsWith("-"); + if (exclusive) { + r = r.substring(1); + } + ref_pattern = r; + + category = rs.getString("category_id"); + group = groupMap.get(new AccountGroup.Id(rs.getInt("group_id"))); + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java index 232974afc8..ffed95a1de 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java @@ -15,11 +15,10 @@ package com.google.gerrit.server.workflow; import com.google.gerrit.common.data.ApprovalType; +import com.google.gerrit.common.data.Permission; import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.PatchSetApproval; -import com.google.gerrit.reviewdb.RefRight; import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.project.RefControl; import java.util.HashMap; import java.util.Map; @@ -29,7 +28,6 @@ public abstract class CategoryFunction { private static Map all = new HashMap(); static { - all.put(SubmitFunction.NAME, new SubmitFunction()); all.put(MaxWithBlock.NAME, new MaxWithBlock()); all.put(MaxNoBlock.NAME, new MaxNoBlock()); all.put(NoOpFunction.NAME, new NoOpFunction()); @@ -44,21 +42,10 @@ public abstract class CategoryFunction { * is not known to Gerrit and thus cannot be executed. */ public static CategoryFunction forCategory(final ApprovalCategory category) { - final CategoryFunction r = forName(category.getFunctionName()); + final CategoryFunction r = all.get(category.getFunctionName()); return r != null ? r : new NoOpFunction(); } - /** - * Locate a function by name. - * - * @param functionName the function's unique name. - * @return the function implementation; null if the function is not known to - * Gerrit and thus cannot be executed. - */ - public static CategoryFunction forName(final String functionName) { - return all.get(functionName); - } - /** * Normalize ChangeApprovals and set the valid flag for this category. *

@@ -92,13 +79,8 @@ public abstract class CategoryFunction { public boolean isValid(final CurrentUser user, final ApprovalType at, final FunctionState state) { - RefControl rc = state.controlFor(user); - for (final RefRight pr : rc.getApplicableRights(at.getCategory().getId())) { - if (user.getEffectiveGroups().contains(pr.getAccountGroupUUID()) - && (pr.getMinValue() < 0 || pr.getMaxValue() > 0)) { - return true; - } - } - return false; + return !state.controlFor(user) // + .getRange(Permission.forLabel(at.getCategory().getLabelName())) // + .isEmpty(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java index 30ff6a3d8b..2cb3e8142d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java @@ -16,13 +16,13 @@ package com.google.gerrit.server.workflow; import com.google.gerrit.common.data.ApprovalType; import com.google.gerrit.common.data.ApprovalTypes; -import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.ApprovalCategoryValue; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.PatchSet; import com.google.gerrit.reviewdb.PatchSetApproval; -import com.google.gerrit.reviewdb.RefRight; import com.google.gerrit.reviewdb.ApprovalCategory.Id; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; @@ -139,28 +139,12 @@ public class FunctionState { * of them is used. *

*/ - private void applyRightFloor(final PatchSetApproval a) { + private void applyRightFloor(final ApprovalType at, final PatchSetApproval a) { + final ApprovalCategory category = at.getCategory(); + final String permission = Permission.forLabel(category.getLabelName()); final IdentifiedUser user = userFactory.create(a.getAccountId()); - RefControl rc = controlFor(user); - - // Find the maximal range actually granted to the user. - // - short minAllowed = 0, maxAllowed = 0; - for (final RefRight r : rc.getApplicableRights(a.getCategoryId())) { - final AccountGroup.UUID grp = r.getAccountGroupUUID(); - if (user.getEffectiveGroups().contains(grp)) { - minAllowed = (short) Math.min(minAllowed, r.getMinValue()); - maxAllowed = (short) Math.max(maxAllowed, r.getMaxValue()); - } - } - - // Normalize the value into that range. - // - if (a.getValue() < minAllowed) { - a.setValue(minAllowed); - } else if (a.getValue() > maxAllowed) { - a.setValue(maxAllowed); - } + final PermissionRange range = controlFor(user).getRange(permission); + a.setValue((short) range.squash(a.getValue())); } RefControl controlFor(final CurrentUser user) { @@ -172,7 +156,7 @@ public class FunctionState { /** Run applyTypeFloor, applyRightFloor. */ public void normalize(final ApprovalType at, final PatchSetApproval ca) { applyTypeFloor(at, ca); - applyRightFloor(ca); + applyRightFloor(at, ca); } private static Id id(final ApprovalType at) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java b/gerrit-server/src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java deleted file mode 100644 index 7e2f42bf76..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright (C) 2008 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.server.workflow; - -import com.google.gerrit.common.data.ApprovalType; -import com.google.gerrit.reviewdb.ApprovalCategory; -import com.google.gerrit.reviewdb.Change; -import com.google.gerrit.reviewdb.RefRight; -import com.google.gerrit.server.CurrentUser; -import com.google.gerrit.server.project.RefControl; - -/** - * Computes if the submit function can be used. - *

- * In order to be considered "approved" this function requires that all approval - * categories with a position >= 0 (that is any whose - * {@link ApprovalCategory#isAction()} method returns false) is valid and that - * the change state be {@link Change.Status#NEW}. - *

- * This is mostly useful for actions, like {@link ApprovalCategory#SUBMIT}. - */ -public class SubmitFunction extends CategoryFunction { - public static String NAME = "Submit"; - - @Override - public void run(final ApprovalType at, final FunctionState state) { - state.valid(at, valid(at, state)); - } - - @Override - public boolean isValid(final CurrentUser user, final ApprovalType at, - final FunctionState state) { - if (valid(at, state)) { - RefControl rc = state.controlFor(user); - for (final RefRight pr : rc.getApplicableRights(at.getCategory().getId())) { - if (user.getEffectiveGroups().contains(pr.getAccountGroupUUID()) - && pr.getMaxValue() > 0) { - return true; - } - } - } - return false; - } - - private static boolean valid(final ApprovalType at, final FunctionState state) { - if (state.getChange().getStatus() != Change.Status.NEW) { - return false; - } - for (final ApprovalType t : state.getApprovalTypes()) { - if (!state.isValid(t)) { - return false; - } - } - return true; - } -} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java new file mode 100644 index 0000000000..e8a235f62d --- /dev/null +++ b/gerrit-server/src/test/java/com/google/gerrit/server/git/ProjectConfigTest.java @@ -0,0 +1,191 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.git; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.GroupReference; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRule; +import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.Project; + +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.errors.IncorrectObjectTypeException; +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.RefUpdate; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevObject; +import org.eclipse.jgit.util.RawParseUtils; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; + +public class ProjectConfigTest extends LocalDiskRepositoryTestCase { + private final GroupReference developers = new GroupReference( + new AccountGroup.UUID("X"), "Developers"); + private final GroupReference staff = new GroupReference( + new AccountGroup.UUID("Y"), "Staff"); + + private Repository db; + private TestRepository util; + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + db = createBareRepository(); + util = new TestRepository(db); + } + + @Test + public void testReadConfig() throws Exception { + RevCommit rev = util.commit(util.tree( // + util.file("groups", util.blob(group(developers))), // + util.file("project.config", util.blob(""// + + "[access \"refs/heads/*\"]\n" // + + " exclusiveGroupPermissions = read submit create\n" // + + " submit = group Developers\n" // + + " push = group Developers\n" // + + " read = group Developers\n")) // + )); + + ProjectConfig cfg = read(rev); + AccessSection section = cfg.getAccessSection("refs/heads/*"); + assertNotNull("has refs/heads/*", section); + assertNull("no refs/*", cfg.getAccessSection("refs/*")); + + Permission create = section.getPermission(Permission.CREATE); + Permission submit = section.getPermission(Permission.SUBMIT); + Permission read = section.getPermission(Permission.READ); + Permission push = section.getPermission(Permission.PUSH); + + assertTrue(create.getExclusiveGroup()); + assertTrue(submit.getExclusiveGroup()); + assertTrue(read.getExclusiveGroup()); + assertFalse(push.getExclusiveGroup()); + } + + @Test + public void testEditConfig() throws Exception { + RevCommit rev = util.commit(util.tree( // + util.file("groups", util.blob(group(developers))), // + util.file("project.config", util.blob(""// + + "[access \"refs/heads/*\"]\n" // + + " exclusiveGroupPermissions = read submit\n" // + + " submit = group Developers\n" // + + " upload = group Developers\n" // + + " read = group Developers\n")) // + )); + update(rev); + + ProjectConfig cfg = read(rev); + AccessSection section = cfg.getAccessSection("refs/heads/*"); + Permission submit = section.getPermission(Permission.SUBMIT); + submit.add(new PermissionRule(cfg.resolve(staff))); + rev = commit(cfg); + assertEquals(""// + + "[access \"refs/heads/*\"]\n" // + + " exclusiveGroupPermissions = read submit\n" // + + " submit = group Developers\n" // + + "\tsubmit = group Staff\n" // + + " upload = group Developers\n" // + + " read = group Developers\n", text(rev, "project.config")); + } + + @Test + public void testEditConfigMissingGroupTableEntry() throws Exception { + RevCommit rev = util.commit(util.tree( // + util.file("groups", util.blob(group(developers))), // + util.file("project.config", util.blob(""// + + "[access \"refs/heads/*\"]\n" // + + " exclusiveGroupPermissions = read submit\n" // + + " submit = group People Who Can Submit\n" // + + " upload = group Developers\n" // + + " read = group Developers\n")) // + )); + update(rev); + + ProjectConfig cfg = read(rev); + AccessSection section = cfg.getAccessSection("refs/heads/*"); + Permission submit = section.getPermission(Permission.SUBMIT); + submit.add(new PermissionRule(cfg.resolve(staff))); + rev = commit(cfg); + assertEquals(""// + + "[access \"refs/heads/*\"]\n" // + + " exclusiveGroupPermissions = read submit\n" // + + " submit = group People Who Can Submit\n" // + + "\tsubmit = group Staff\n" // + + " upload = group Developers\n" // + + " read = group Developers\n", text(rev, "project.config")); + } + + private ProjectConfig read(RevCommit rev) throws IOException, + ConfigInvalidException { + ProjectConfig cfg = new ProjectConfig(new Project.NameKey("test")); + cfg.load(db, rev); + return cfg; + } + + private RevCommit commit(ProjectConfig cfg) throws IOException, + MissingObjectException, IncorrectObjectTypeException { + MetaDataUpdate md = new MetaDataUpdate(new NoReplication(), // + cfg.getProject().getNameKey(), // + db); + util.tick(5); + util.setAuthorAndCommitter(md.getCommitBuilder()); + md.setMessage("Edit\n"); + assertTrue("commit finished", cfg.commit(md)); + + Ref ref = db.getRef(GitRepositoryManager.REF_CONFIG); + return util.getRevWalk().parseCommit(ref.getObjectId()); + } + + private void update(RevCommit rev) throws Exception { + RefUpdate u = db.updateRef(GitRepositoryManager.REF_CONFIG); + u.disableRefLog(); + u.setNewObjectId(rev); + switch (u.forceUpdate()) { + case FAST_FORWARD: + case FORCED: + case NEW: + case NO_CHANGE: + break; + default: + fail("Cannot update ref for test: " + u.getResult()); + } + } + + private String text(RevCommit rev, String path) throws Exception { + RevObject blob = util.get(rev.getTree(), path); + byte[] data = db.open(blob).getCachedBytes(Integer.MAX_VALUE); + return RawParseUtils.decode(data); + } + + private static String group(GroupReference g) { + return g.getUUID().get() + "\t" + g.getName() + "\n"; + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index a2d849f3b0..a037e30f7b 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -14,23 +14,24 @@ package com.google.gerrit.server.project; -import static com.google.gerrit.reviewdb.ApprovalCategory.OWN; -import static com.google.gerrit.reviewdb.ApprovalCategory.READ; -import static com.google.gerrit.reviewdb.ApprovalCategory.SUBMIT; +import static com.google.gerrit.common.data.Permission.OWNER; +import static com.google.gerrit.common.data.Permission.PUSH; +import static com.google.gerrit.common.data.Permission.READ; +import static com.google.gerrit.common.data.Permission.SUBMIT; +import com.google.gerrit.common.data.GroupReference; +import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.AccountGroup; import com.google.gerrit.reviewdb.AccountProjectWatch; -import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.Project; -import com.google.gerrit.reviewdb.RefRight; import com.google.gerrit.reviewdb.SystemConfig; -import com.google.gerrit.reviewdb.RefRight.RefPattern; import com.google.gerrit.server.AccessPath; import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.git.ProjectConfig; import com.google.inject.AbstractModule; import com.google.inject.Guice; import com.google.inject.Injector; @@ -41,19 +42,17 @@ import org.apache.commons.codec.binary.Base64; import org.eclipse.jgit.lib.Config; import java.io.UnsupportedEncodingException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Set; public class RefControlTest extends TestCase { public void testOwnerProject() { - grant(local, OWN, admin, "refs/*", 1); + grant(local, OWNER, admin, "refs/*"); ProjectControl uBlah = user(devs); ProjectControl uAdmin = user(devs, admin); @@ -63,8 +62,8 @@ public class RefControlTest extends TestCase { } public void testBranchDelegation1() { - grant(local, OWN, admin, "refs/*", 1); - grant(local, OWN, devs, "refs/heads/x/*", 1); + grant(local, OWNER, admin, "refs/*"); + grant(local, OWNER, devs, "refs/heads/x/*"); ProjectControl uDev = user(devs); assertFalse("not owner", uDev.isOwner()); @@ -79,9 +78,10 @@ public class RefControlTest extends TestCase { } public void testBranchDelegation2() { - grant(local, OWN, admin, "refs/*", 1); - grant(local, OWN, devs, "refs/heads/x/*", 1); - grant(local, OWN, fixers, "-refs/heads/x/y/*", 1); + grant(local, OWNER, admin, "refs/*"); + grant(local, OWNER, devs, "refs/heads/x/*"); + grant(local, OWNER, fixers, "refs/heads/x/y/*"); + doNotInherit(local, OWNER, "refs/heads/x/y/*"); ProjectControl uDev = user(devs); assertFalse("not owner", uDev.isOwner()); @@ -106,8 +106,11 @@ public class RefControlTest extends TestCase { } public void testInheritRead_SingleBranchDeniesUpload() { - grant(parent, READ, registered, "refs/*", 1, 2); - grant(local, READ, registered, "-refs/heads/foobar", 1); + grant(parent, READ, registered, "refs/*"); + grant(parent, PUSH, registered, "refs/for/refs/*"); + grant(local, READ, registered, "refs/heads/foobar"); + doNotInherit(local, READ, "refs/heads/foobar"); + doNotInherit(local, PUSH, "refs/for/refs/heads/foobar"); ProjectControl u = user(); assertTrue("can upload", u.canPushToAtLeastOneRef()); @@ -120,8 +123,9 @@ public class RefControlTest extends TestCase { } public void testInheritRead_SingleBranchDoesNotOverrideInherited() { - grant(parent, READ, registered, "refs/*", 1, 2); - grant(local, READ, registered, "refs/heads/foobar", 1); + grant(parent, READ, registered, "refs/*"); + grant(parent, PUSH, registered, "refs/for/refs/*"); + grant(local, READ, registered, "refs/heads/foobar"); ProjectControl u = user(); assertTrue("can upload", u.canPushToAtLeastOneRef()); @@ -134,16 +138,16 @@ public class RefControlTest extends TestCase { } public void testInheritRead_OverrideWithDeny() { - grant(parent, READ, registered, "refs/*", 1); - grant(local, READ, registered, "refs/*", 0); + grant(parent, READ, registered, "refs/*"); + grant(local, READ, registered, "refs/*").setDeny(true); ProjectControl u = user(); assertFalse("can't read", u.isVisible()); } public void testInheritRead_AppendWithDenyOfRef() { - grant(parent, READ, registered, "refs/*", 1); - grant(local, READ, registered, "refs/heads/*", 0); + grant(parent, READ, registered, "refs/*"); + grant(local, READ, registered, "refs/heads/*").setDeny(true); ProjectControl u = user(); assertTrue("can read", u.isVisible()); @@ -153,9 +157,9 @@ public class RefControlTest extends TestCase { } public void testInheritRead_OverridesAndDeniesOfRef() { - grant(parent, READ, registered, "refs/*", 1); - grant(local, READ, registered, "refs/*", 0); - grant(local, READ, registered, "refs/heads/*", -1, 1); + grant(parent, READ, registered, "refs/*"); + grant(local, READ, registered, "refs/*").setDeny(true); + grant(local, READ, registered, "refs/heads/*"); ProjectControl u = user(); assertTrue("can read", u.isVisible()); @@ -165,9 +169,9 @@ public class RefControlTest extends TestCase { } public void testInheritSubmit_OverridesAndDeniesOfRef() { - grant(parent, SUBMIT, registered, "refs/*", 1); - grant(local, SUBMIT, registered, "refs/*", 0); - grant(local, SUBMIT, registered, "refs/heads/*", -1, 1); + grant(parent, SUBMIT, registered, "refs/*"); + grant(local, SUBMIT, registered, "refs/*").setDeny(true); + grant(local, SUBMIT, registered, "refs/heads/*"); ProjectControl u = user(); assertFalse("can't submit", u.controlForRef("refs/foobar").canSubmit()); @@ -176,8 +180,9 @@ public class RefControlTest extends TestCase { } public void testCannotUploadToAnyRef() { - grant(parent, READ, registered, "refs/*", 1); - grant(local, READ, devs, "refs/heads/*", 1, 2); + grant(parent, READ, registered, "refs/*"); + grant(local, READ, devs, "refs/heads/*"); + grant(local, PUSH, devs, "refs/for/refs/heads/*"); ProjectControl u = user(); assertFalse("cannot upload", u.canPushToAtLeastOneRef()); @@ -188,8 +193,8 @@ public class RefControlTest extends TestCase { // ----------------------------------------------------------------------- - private final Project.NameKey local = new Project.NameKey("test"); - private final Project.NameKey parent = new Project.NameKey("parent"); + private ProjectConfig local; + private ProjectConfig parent; private final AccountGroup.UUID admin = new AccountGroup.UUID("test.admin"); private final AccountGroup.UUID anonymous = AccountGroup.ANONYMOUS_USERS; private final AccountGroup.UUID registered = AccountGroup.REGISTERED_USERS; @@ -201,9 +206,6 @@ public class RefControlTest extends TestCase { private final AuthConfig authConfig; private final AnonymousUser anonymousUser; - private final Map groupIds = - new HashMap(); - public RefControlTest() { systemConfig = SystemConfig.create(); systemConfig.adminGroupUUID = admin; @@ -231,14 +233,16 @@ public class RefControlTest extends TestCase { anonymousUser = injector.getInstance(AnonymousUser.class); } - private List localRights; - private List inheritedRights; - @Override - protected void setUp() throws Exception { + public void setUp() throws Exception { super.setUp(); - localRights = new ArrayList(); - inheritedRights = new ArrayList(); + + parent = new ProjectConfig(new Project.NameKey("parent")); + parent.createInMemory(); + + local = new ProjectConfig(new Project.NameKey("local")); + local.createInMemory(); + local.getProject().setParentName(parent.getProject().getName()); } private static void assertOwner(String ref, ProjectControl u) { @@ -249,33 +253,27 @@ public class RefControlTest extends TestCase { assertFalse("NOT OWN " + ref, u.controlForRef(ref).isOwner()); } - private void grant(Project.NameKey project, ApprovalCategory.Id categoryId, - AccountGroup.UUID group, String ref, int maxValue) { - grant(project, categoryId, group, ref, maxValue, maxValue); + private PermissionRule grant(ProjectConfig project, String permissionName, + AccountGroup.UUID group, String ref) { + PermissionRule rule = newRule(project, group); + project.getAccessSection(ref, true) // + .getPermission(permissionName, true) // + .add(rule); + return rule; } - private void grant(Project.NameKey project, ApprovalCategory.Id categoryId, - AccountGroup.UUID groupUUID, String ref, int minValue, int maxValue) { - AccountGroup.Id groupId = groupIds.get(groupUUID); - if (groupId == null) { - groupId = new AccountGroup.Id(groupIds.size() + 1); - groupIds.put(groupUUID, groupId); - } + private void doNotInherit(ProjectConfig project, String permissionName, + String ref) { + project.getAccessSection(ref, true) // + .getPermission(permissionName, true) // + .setExclusiveGroup(true); + } - RefRight right = - new RefRight(new RefRight.Key(project, new RefPattern(ref), - categoryId, groupId)); - right.setAccountGroupUUID(groupUUID); - right.setMinValue((short) minValue); - right.setMaxValue((short) maxValue); + private PermissionRule newRule(ProjectConfig project, AccountGroup.UUID groupUUID) { + GroupReference group = new GroupReference(groupUUID, groupUUID.get()); + group = project.resolve(group); - if (project == parent) { - inheritedRights.add(right); - } else if (project == local) { - localRights.add(right); - } else { - fail("Unknown project key: " + project); - } + return new PermissionRule(group); } private ProjectControl user(AccountGroup.UUID... memberOf) { @@ -291,15 +289,40 @@ public class RefControlTest extends TestCase { } private ProjectState newProjectState() { - ProjectCache projectCache = null; + final Map all = + new HashMap(); + final ProjectCache projectCache = new ProjectCache() { + @Override + public ProjectState get(Project.NameKey projectName) { + return all.get(projectName); + } + + @Override + public void evict(Project p) { + } + + @Override + public Iterable all() { + return Collections.emptySet(); + } + + @Override + public Iterable byName(String prefix) { + return Collections.emptySet(); + } + + @Override + public void onCreateProject(Project.NameKey newProjectName) { + } + }; + Project.NameKey wildProject = new Project.NameKey("-- All Projects --"); ProjectControl.AssistedFactory projectControlFactory = null; - Project project = new Project(parent); - ProjectState ps = - new ProjectState(anonymousUser, projectCache, wildProject, - projectControlFactory, project, localRights); - ps.setInheritedRights(inheritedRights); - return ps; + all.put(local.getProject().getNameKey(), new ProjectState(anonymousUser, + projectCache, wildProject, projectControlFactory, local)); + all.put(parent.getProject().getNameKey(), new ProjectState(anonymousUser, + projectCache, wildProject, projectControlFactory, parent)); + return all.get(local.getProject().getNameKey()); } private class MockUser extends CurrentUser { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java index 3b847bfb4a..1d70d4efb6 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/schema/SchemaCreatorTest.java @@ -17,11 +17,8 @@ package com.google.gerrit.server.schema; import com.google.gerrit.reviewdb.AccountGroup; import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.ApprovalCategoryValue; -import com.google.gerrit.reviewdb.RefRight; import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.reviewdb.SystemConfig; -import com.google.gerrit.server.workflow.NoOpFunction; -import com.google.gerrit.server.workflow.SubmitFunction; import com.google.gerrit.testutil.InMemoryDatabase; import com.google.gwtorm.client.OrmException; import com.google.gwtorm.jdbc.JdbcSchema; @@ -163,7 +160,6 @@ public class SchemaCreatorTest extends TestCase { assertEquals("R", cat.getAbbreviatedName()); assertEquals("MaxWithBlock", cat.getFunctionName()); assertTrue(cat.isCopyMinScore()); - assertFalse(cat.isAction()); assertTrue(0 <= cat.getPosition()); } finally { c.close(); @@ -171,101 +167,6 @@ public class SchemaCreatorTest extends TestCase { assertValueRange(codeReview, -2, -1, 0, 1, 2); } - public void testCreateSchema_ApprovalCategory_Read() throws OrmException { - final ReviewDb c = db.create().open(); - try { - final ApprovalCategory cat; - - cat = c.approvalCategories().get(ApprovalCategory.READ); - assertNotNull(cat); - assertEquals(ApprovalCategory.READ, cat.getId()); - assertEquals("Read Access", cat.getName()); - assertNull(cat.getAbbreviatedName()); - assertEquals(NoOpFunction.NAME, cat.getFunctionName()); - assertTrue(cat.isAction()); - } finally { - c.close(); - } - assertValueRange(ApprovalCategory.READ, -1, 1, 2, 3); - } - - public void testCreateSchema_ApprovalCategory_Submit() throws OrmException { - final ReviewDb c = db.create().open(); - try { - final ApprovalCategory cat; - - cat = c.approvalCategories().get(ApprovalCategory.SUBMIT); - assertNotNull(cat); - assertEquals(ApprovalCategory.SUBMIT, cat.getId()); - assertEquals("Submit", cat.getName()); - assertNull(cat.getAbbreviatedName()); - assertEquals(SubmitFunction.NAME, cat.getFunctionName()); - assertTrue(cat.isAction()); - } finally { - c.close(); - } - assertValueRange(ApprovalCategory.SUBMIT, 1); - } - - public void testCreateSchema_ApprovalCategory_PushTag() throws OrmException { - final ReviewDb c = db.create().open(); - try { - final ApprovalCategory cat; - - cat = c.approvalCategories().get(ApprovalCategory.PUSH_TAG); - assertNotNull(cat); - assertEquals(ApprovalCategory.PUSH_TAG, cat.getId()); - assertEquals("Push Tag", cat.getName()); - assertNull(cat.getAbbreviatedName()); - assertEquals(NoOpFunction.NAME, cat.getFunctionName()); - assertTrue(cat.isAction()); - } finally { - c.close(); - } - assertValueRange(ApprovalCategory.PUSH_TAG, // - ApprovalCategory.PUSH_TAG_SIGNED, // - ApprovalCategory.PUSH_TAG_ANNOTATED); - } - - public void testCreateSchema_ApprovalCategory_PushHead() throws OrmException { - final ReviewDb c = db.create().open(); - try { - final ApprovalCategory cat; - - cat = c.approvalCategories().get(ApprovalCategory.PUSH_HEAD); - assertNotNull(cat); - assertEquals(ApprovalCategory.PUSH_HEAD, cat.getId()); - assertEquals("Push Branch", cat.getName()); - assertNull(cat.getAbbreviatedName()); - assertEquals(NoOpFunction.NAME, cat.getFunctionName()); - assertTrue(cat.isAction()); - } finally { - c.close(); - } - assertValueRange(ApprovalCategory.PUSH_HEAD, // - ApprovalCategory.PUSH_HEAD_UPDATE, // - ApprovalCategory.PUSH_HEAD_CREATE, // - ApprovalCategory.PUSH_HEAD_REPLACE); - } - - public void testCreateSchema_ApprovalCategory_Owner() throws OrmException { - final ReviewDb c = db.create().open(); - try { - final ApprovalCategory cat; - - cat = c.approvalCategories().get(ApprovalCategory.OWN); - assertNotNull(cat); - assertEquals(ApprovalCategory.OWN, cat.getId()); - assertEquals("Owner", cat.getName()); - assertNull(cat.getAbbreviatedName()); - assertEquals(NoOpFunction.NAME, cat.getFunctionName()); - assertTrue(cat.isAction()); - } finally { - c.close(); - } - assertValueRange(ApprovalCategory.OWN, 1); - } - private void assertValueRange(ApprovalCategory.Id cat, int... range) throws OrmException { final HashSet act = @@ -295,55 +196,4 @@ public class SchemaCreatorTest extends TestCase { fail("Category " + cat + " has additional values: " + act); } } - - public void testCreateSchema_DefaultAccess_AnonymousUsers() - throws OrmException { - db.create(); - final SystemConfig config = db.getSystemConfig(); - assertDefaultRight("refs/*", config.anonymousGroupId, - ApprovalCategory.READ, 1, 1); - } - - public void testCreateSchema_DefaultAccess_RegisteredUsers() - throws OrmException { - db.create(); - final SystemConfig config = db.getSystemConfig(); - assertDefaultRight("refs/*", config.registeredGroupId, - ApprovalCategory.READ, 1, 2); - assertDefaultRight("refs/heads/*", config.registeredGroupId, codeReview, - -1, 1); - } - - public void testCreateSchema_DefaultAccess_Administrators() - throws OrmException { - db.create(); - final SystemConfig config = db.getSystemConfig(); - assertDefaultRight("refs/*", config.adminGroupId, ApprovalCategory.READ, 1, - 1); - } - - private void assertDefaultRight(final String pattern, - final AccountGroup.Id group, final ApprovalCategory.Id category, int min, - int max) throws OrmException { - final ReviewDb c = db.open(); - try { - final SystemConfig cfg; - final RefRight right; - - cfg = c.systemConfig().get(new SystemConfig.Key()); - right = - c.refRights().get( - new RefRight.Key(cfg.wildProjectName, new RefRight.RefPattern( - pattern), category, group)); - - assertNotNull(right); - assertEquals(cfg.wildProjectName, right.getProjectNameKey()); - assertEquals(group, right.getAccountGroupId()); - assertEquals(category, right.getApprovalCategoryId()); - assertEquals(min, right.getMinValue()); - assertEquals(max, right.getMaxValue()); - } finally { - c.close(); - } - } } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminSetParent.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminSetParent.java index 2f955b5aae..feb48913ca 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminSetParent.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminSetParent.java @@ -135,10 +135,6 @@ final class AdminSetParent extends BaseCommand { } } - // Invalidate all projects in cache since inherited rights were changed. - // - projectCache.evictAll(); - if (err.length() > 0) { while (err.charAt(err.length() - 1) == '\n') { err.setLength(err.length() - 1); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProject.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProject.java index 117445f181..77dc8f12d4 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProject.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/CreateProject.java @@ -15,11 +15,12 @@ package com.google.gerrit.sshd.commands; import com.google.gerrit.common.CollectionsUtil; +import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.GroupReference; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.reviewdb.AccountGroup; -import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.Project; -import com.google.gerrit.reviewdb.RefRight; -import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.reviewdb.Project.SubmitType; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; @@ -33,7 +34,6 @@ import com.google.gerrit.server.git.ReplicationQueue; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.sshd.BaseCommand; -import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import org.apache.sshd.server.Environment; @@ -96,9 +96,6 @@ final class CreateProject extends BaseCommand { @Option(name = "--empty-commit", usage = "to create initial empty commit") private boolean createEmptyCommit; - @Inject - private ReviewDb db; - @Inject private GitRepositoryManager repoManager; @@ -202,27 +199,11 @@ final class CreateProject extends BaseCommand { } } - private void createProject() throws OrmException, IOException, - ConfigInvalidException { - - List access = new ArrayList(); - for (AccountGroup.UUID ownerId : ownerIds) { - AccountGroup group = groupCache.get(ownerId); - - final RefRight.Key prk = - new RefRight.Key(nameKey, new RefRight.RefPattern( - RefRight.ALL), ApprovalCategory.OWN, group.getId()); - final RefRight pr = new RefRight(prk); - pr.setMaxValue((short) 1); - pr.setMinValue((short) 1); - access.add(pr); - } - db.refRights().insert(access); - - Project.NameKey nameKey = new Project.NameKey(projectName); + private void createProject() throws IOException, ConfigInvalidException { MetaDataUpdate md = metaDataUpdateFactory.create(nameKey); try { ProjectConfig config = ProjectConfig.read(md); + config.load(md); Project newProject = config.getProject(); newProject.setDescription(projectDescription); @@ -235,6 +216,16 @@ final class CreateProject extends BaseCommand { newProject.setParentName(newParent.getProject().getName()); } + if (!ownerIds.isEmpty()) { + AccessSection all = config.getAccessSection(AccessSection.ALL, true); + for (AccountGroup.UUID ownerId : ownerIds) { + AccountGroup accountGroup = groupCache.get(ownerId); + GroupReference group = config.resolve(accountGroup); + all.getPermission(Permission.OWNER, true).add( + new PermissionRule(group)); + } + } + md.setMessage("Created project\n"); if (!config.commit(md)) { throw new IOException("Cannot create " + projectName); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java index 47bae4f32e..0a6033bc0f 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java @@ -331,7 +331,7 @@ public class ReviewCommand extends BaseCommand { functionStateFactory.create(changeControl.getChange(), patchSetId, Collections. emptyList()); psa.setValue(v); - fs.normalize(approvalTypes.getApprovalType(psa.getCategoryId()), psa); + fs.normalize(approvalTypes.byId(psa.getCategoryId()), psa); if (v != psa.getValue()) { throw error(ao.name() + "=" + ao.value() + " not permitted"); }