opendev/gerrit
Code Issues Proposed changes

Simplify FunctionState as discussed previously

Browse Source 
This change removes some of the caches that were in FunctionState,
favoring the use of RefControl instead. It should make it easier to
get rid of FunctionState in subsequent changes.

Also removed a bunch of unused methods in FunctionState, and lowered
access level in a few others to simplify its interface.

Change-Id: I8de2118c61a0c242d7748f74eff80c6e1daca3ba
This commit is contained in:
Nico Sallembien
2010-01-28 15:31:24 -08:00
parent 4ef9b4660f
commit 6f3b9ee614
4 changed files with 27 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
View File

@@ -249,6 +249,14 @@ public class RefControl {
return filter(projectControl.getProjectState().getInheritedRights(), actionId);
}
public List<RefRight> getAllRights(final ApprovalCategory.Id id) {
List<RefRight> l = new ArrayList<RefRight>();
l.addAll(getLocalRights(id));
l.addAll(getInheritedRights(id));
Collections.sort(l, RefRight.REF_PATTERN_ORDER);
return Collections.unmodifiableList(RefControl.filterMostSpecific(l));
}
private List<RefRight> filter(Collection<RefRight> all,
ApprovalCategory.Id actionId) {
List<RefRight> mine = new ArrayList<RefRight>(all.size());

4
gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java
View File

@@ -19,6 +19,7 @@ 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;
@@ -89,7 +90,8 @@ public abstract class CategoryFunction {
public boolean isValid(final CurrentUser user, final ApprovalType at,
final FunctionState state) {
for (final RefRight pr : state.getAllRights(at)) {
RefControl rc = state.controlFor(user);
for (final RefRight pr : rc.getAllRights(at.getCategory().getId())) {
if (user.getEffectiveGroups().contains(pr.getAccountGroupId())
&& (pr.getMinValue() < 0 || pr.getMaxValue() > 0)) {
return true;

100
gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java
View File

@@ -22,12 +22,13 @@ 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.Project;
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;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.RefControl;
import com.google.inject.Inject;
@@ -37,10 +38,8 @@ 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;
/** State passed through to a {@link CategoryFunction}. */
public class FunctionState {
@@ -58,11 +57,6 @@ public class FunctionState {
new HashMap<ApprovalCategory.Id, Boolean>();
private final Change change;
private final ProjectState project;
private final Map<ApprovalCategory.Id, List<RefRight>> allRights =
new HashMap<ApprovalCategory.Id, List<RefRight>>();
private Map<ApprovalCategory.Id, List<RefRight>> refRights;
private Map<ApprovalCategory.Id, List<RefRight>> inheritedRights;
private Set<PatchSetApproval> modified;
@Inject
FunctionState(final ApprovalTypes approvalTypes,
@@ -92,14 +86,10 @@ public class FunctionState {
return approvalTypes.getApprovalTypes();
}
public Change getChange() {
Change getChange() {
return change;
}
public Project getProject() {
return project.getProject();
}
public void valid(final ApprovalType at, final boolean v) {
valid.put(id(at), v);
}
@@ -122,88 +112,21 @@ public class FunctionState {
return l != null ? l : Collections.<PatchSetApproval> emptySet();
}
public void dirty(final PatchSetApproval ap) {
if (modified == null) {
modified = new HashSet<PatchSetApproval>();
}
modified.add(ap);
}
public Collection<PatchSetApproval> getDirtyChangeApprovals() {
if (modified != null) {
return modified;
}
return Collections.emptySet();
}
private List<RefRight> getRefRights(final ApprovalCategory.Id id) {
if (refRights == null) {
refRights = index(project.getLocalRights());
}
final List<RefRight> l = refRights.get(id);
return l != null ? l : Collections.<RefRight> emptyList();
}
private List<RefRight> getWildcardRights(final ApprovalCategory.Id id) {
if (inheritedRights == null) {
inheritedRights = index(project.getInheritedRights());
}
final List<RefRight> l = inheritedRights.get(id);
return l != null ? l : Collections.<RefRight> emptyList();
}
public Collection<RefRight> getAllRights(final ApprovalType at) {
return getAllRights(id(at));
}
public List<RefRight> getAllRights(final ApprovalCategory.Id id) {
List<RefRight> l = allRights.get(id);
if (l == null) {
l = new ArrayList<RefRight>();
l.addAll(getRefRights(id));
l.addAll(getWildcardRights(id));
Collections.sort(l, RefRight.REF_PATTERN_ORDER);
l = Collections.unmodifiableList(RefControl.filterMostSpecific(l));
allRights.put(id, l);
}
return l;
}
private Map<Id, List<RefRight>> index(final Collection<RefRight> rights) {
final HashMap<ApprovalCategory.Id, List<RefRight>> r;
r = new HashMap<ApprovalCategory.Id, List<RefRight>>();
for (final RefRight pr : rights) {
if (RefControl.matches(change.getDest().get(), pr.getRefPattern())) {
List<RefRight> l = r.get(pr.getApprovalCategoryId());
if (l == null) {
l = new ArrayList<RefRight>();
r.put(pr.getApprovalCategoryId(), l);
}
l.add(pr);
}
}
return r;
}
/**
* Normalize the approval record down to the range permitted by the type, in
* case the type was modified since the approval was originally granted.
* <p>
* If the record's value was modified, its automatically marked as dirty.
*/
public void applyTypeFloor(final ApprovalType at, final PatchSetApproval a) {
private void applyTypeFloor(final ApprovalType at, final PatchSetApproval a) {
final ApprovalCategoryValue atMin = at.getMin();
if (atMin != null && a.getValue() < atMin.getValue()) {
a.setValue(atMin.getValue());
dirty(a);
}
final ApprovalCategoryValue atMax = at.getMax();
if (atMax != null && a.getValue() > atMax.getValue()) {
a.setValue(atMax.getValue());
dirty(a);
}
}
@@ -215,15 +138,15 @@ public class FunctionState {
* is a member of) the lowest minValue and the highest maxValue of the union
* of them is used.
* <p>
* If the record's value was modified, its automatically marked as dirty.
*/
public void applyRightFloor(final PatchSetApproval a) {
private void applyRightFloor(final PatchSetApproval a) {
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 : getAllRights(a.getCategoryId())) {
for (final RefRight r : rc.getAllRights(a.getCategoryId())) {
final AccountGroup.Id grp = r.getAccountGroupId();
if (user.getEffectiveGroups().contains(grp)) {
minAllowed = (short) Math.min(minAllowed, r.getMinValue());
@@ -236,14 +159,17 @@ public class FunctionState {
//
if (a.getValue() < minAllowed) {
a.setValue(minAllowed);
dirty(a);
} else if (a.getValue() > maxAllowed) {
a.setValue(maxAllowed);
dirty(a);
}
}
RefControl controlFor(final CurrentUser user) {
ProjectControl pc = project.controlFor(user);
RefControl rc = pc.controlForRef(change.getDest().get());
return rc;
}
/** Run <code>applyTypeFloor</code>, <code>applyRightFloor</code>. */
public void normalize(final ApprovalType at, final PatchSetApproval ca) {
applyTypeFloor(at, ca);

4
gerrit-server/src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java
View File

@@ -19,6 +19,7 @@ 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.
@@ -42,7 +43,8 @@ public class SubmitFunction extends CategoryFunction {
public boolean isValid(final CurrentUser user, final ApprovalType at,
final FunctionState state) {
if (valid(at, state)) {
for (final RefRight pr : state.getAllRights(at)) {
RefControl rc = state.controlFor(user);
for (final RefRight pr : rc.getAllRights(at.getCategory().getId())) {
if (user.getEffectiveGroups().contains(pr.getAccountGroupId())
&& pr.getMaxValue() > 0) {
return true;