Simplify FunctionState as discussed previously

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

4 files changed, 27 insertions, 89 deletions

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 74dc7a241d..d6c073b097 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
@@ -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());
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 0c4abe566d..ce5e1230ad 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
@@ -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;
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 63b3883f5c..5bf39e1455 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
@@ -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);
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
index 48c7f71ff1..e097eeb136 100644
--- 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
@@ -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;