Refactor how permissions are matched by ProjectControl, RefControl

AccessSections are now matched onto references using pre-compiled
SectionMatcher objects. These matchers are built in the ProjectState
on demand, and cached until the ProjectState itself is discarded
from memory. This saves the Pattern.compile() costs, as well as some
basic conditionals to determine which type of reference pattern the
section uses, providing a small speed up to access rule evaluation.

ProjectControl and RefControl now stores all permissions that belong
to the project or reference, rather than only the ones relevant for
their CurrentUser. This allows the control objects to provide cached
data for other users, such as when ChangeControl needs to build a
different copy of itself for each reviewer listed on the change.

ProjectControl caches RefControls it builds, making it easier for
callers like ReceiveCommits or VisibleRefFilter to deal with a
lot of lookups for the same common reference name within a single
project access request. This comes at a cost of memory, but should
be an improvement in response time.

Project ownership checks are now handled by ProjectState, relying
on the cached localOwners data instead of looking at the OWNER
permission on "refs/*". The cached localOwners is already built up
from the "refs/*" data during ProjectState's constructor, so doing
it dynamically via RefControl inside of ProjectControl was really
quite wasteful.

Change-Id: Iaf12bab55d41217363cc05ba024f452d03bc21df

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -61,7 +61,6 @@ import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectCacheImpl;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.RefControl;
 import com.google.gerrit.server.tools.ToolsCatalog;
 import com.google.gerrit.server.util.IdGenerator;
 import com.google.gerrit.server.workflow.FunctionState;
@@ -156,7 +155,6 @@ public class GerritGlobalModule extends FactoryModule {
     factory(CapabilityControl.Factory.class);
     factory(GroupInfoCacheFactory.Factory.class);
     factory(ProjectState.Factory.class);
-    factory(RefControl.Factory.class);
 
     bind(FileTypeRegistry.class).to(MimeUtilFileTypeRegistry.class);
     bind(WorkQueue.class);

gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java

@@ -0,0 +1,311 @@
+// 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.project;
+
+import static com.google.gerrit.server.project.RefControl.isRE;
+import static com.google.gerrit.server.project.RefControl.shortestExample;
+import static com.google.gerrit.server.project.RefControl.toRegExp;
+
+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 org.apache.commons.lang.StringUtils;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Effective permissions applied to a reference in a project.
+ * <p>
+ * A collection may be user specific if a matching {@link AccessSection} uses
+ * "${username}" in its name. The permissions granted in that section may only
+ * be granted to the username that appears in the reference name, and also only
+ * if the user is a member of the relevant group.
+ */
+public class PermissionCollection {
+  /**
+   * Get all permissions that apply to a reference.
+   *
+   * @param matcherList collection of sections that should be considered, in
+   *        priority order (project specific definitions must appear before
+   *        inherited ones).
+   * @param ref reference being accessed.
+   * @param username if the reference is a per-user reference, access sections
+   *        using the parameter variable "${username}" will first have {@code
+   *        username} inserted into them before seeing if they apply to the
+   *        reference named by {@code ref}. If null, per-user references are
+   *        ignored.
+   * @return map of permissions that apply to this reference, keyed by
+   *         permission name.
+   */
+  static PermissionCollection filter(Iterable<SectionMatcher> matcherList,
+      String ref, String username) {
+    if (isRE(ref)) {
+      ref = RefControl.shortestExample(ref);
+    } else if (ref.endsWith("/*")) {
+      ref = ref.substring(0, ref.length() - 1);
+    }
+
+    boolean perUser = false;
+    List<AccessSection> sections = new ArrayList<AccessSection>();
+    for (SectionMatcher matcher : matcherList) {
+      // If the matcher has to expand parameters and its prefix matches the
+      // reference there is a very good chance the reference is actually user
+      // specific, even if the matcher does not match the reference. Since its
+      // difficult to prove this is true all of the time, use an approximation
+      // to prevent reuse of collections across users accessing the same
+      // reference at the same time.
+      //
+      // This check usually gets caching right, as most per-user references
+      // use a common prefix like "refs/sandbox/" or "refs/heads/users/"
+      // that will never be shared with non-user references, and the per-user
+      // references are usually less frequent than the non-user references.
+      //
+      if (username != null && !perUser
+          && matcher instanceof SectionMatcher.ExpandParameters) {
+        perUser = ((SectionMatcher.ExpandParameters) matcher).matchPrefix(ref);
+      }
+
+      if (matcher.match(ref, username)) {
+        sections.add(matcher.section);
+      }
+    }
+    Collections.sort(sections, new MostSpecificComparator(ref));
+
+    Set<SeenRule> seen = new HashSet<SeenRule>();
+    Set<String> exclusiveGroupPermissions = new HashSet<String>();
+
+    HashMap<String, List<PermissionRule>> permissions =
+        new HashMap<String, List<PermissionRule>>();
+    for (AccessSection section : sections) {
+      for (Permission permission : section.getPermissions()) {
+        if (exclusiveGroupPermissions.contains(permission.getName())) {
+          continue;
+        }
+
+        for (PermissionRule rule : permission.getRules()) {
+          SeenRule s = new SeenRule(section, permission, rule);
+          if (seen.add(s) && !rule.getDeny()) {
+            List<PermissionRule> r = permissions.get(permission.getName());
+            if (r == null) {
+              r = new ArrayList<PermissionRule>(2);
+              permissions.put(permission.getName(), r);
+            }
+            r.add(rule);
+          }
+        }
+
+        if (permission.getExclusiveGroup()) {
+          exclusiveGroupPermissions.add(permission.getName());
+        }
+      }
+    }
+
+    return new PermissionCollection(ref, permissions, perUser ? username : null);
+  }
+
+  private final String ref;
+  private final Map<String, List<PermissionRule>> rules;
+  private final String username;
+
+  private PermissionCollection(String ref,
+      Map<String, List<PermissionRule>> rules, String username) {
+    this.ref = ref;
+    this.rules = rules;
+    this.username = username;
+  }
+
+  /**
+   * @return true if a "${username}" pattern might need to be expanded to build
+   *         this collection, making the results user specific.
+   */
+  public boolean isUserSpecific() {
+    return username != null;
+  }
+
+  /**
+   * Obtain all permission rules for a given type of permission.
+   *
+   * @param permissionName type of permission.
+   * @return all rules that apply to this reference, for any group. Never null;
+   *         the empty list is returned when there are no rules for the requested
+   *         permission name.
+   */
+  public List<PermissionRule> getPermission(String permissionName) {
+    List<PermissionRule> r = rules.get(permissionName);
+    return r != null ? r : Collections.<PermissionRule> emptyList();
+  }
+
+  /**
+   * Obtain all declared permission rules that match the reference.
+   *
+   * @return all rules. The collection will iterate a permission if it was
+   *         declared in the project configuration, either directly or
+   *         inherited. If the project owner did not use a known permission (for
+   *         example {@link Permission#FORGE_SERVER}, then it will not be
+   *         represented in the result even if {@link #getPermission(String)}
+   *         returns an empty list for the same permission.
+   */
+  public Iterable<Map.Entry<String, List<PermissionRule>>> getDeclaredPermissions() {
+    return rules.entrySet();
+  }
+
+  /** 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.getName();
+      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:
+   * <ul>
+   * <li>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.
+   * <li>2 - Finites first, infinities after.
+   * <li>3 - Number of transitions.
+   * <li>4 - Length of the expression text.
+   * </ul>
+   *
+   * 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<AccessSection> {
+    private final String refName;
+
+    MostSpecificComparator(String refName) {
+      this.refName = refName;
+    }
+
+    public int compare(AccessSection a, AccessSection b) {
+      return compare(a.getName(), b.getName());
+    }
+
+    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();
+      }
+    }
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java

@@ -14,8 +14,6 @@
 
 package com.google.gerrit.server.project;
 
-import static com.google.gerrit.common.CollectionsUtil.isAnyIncludedIn;
-
 import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.Capable;
@@ -46,10 +44,11 @@ import com.google.inject.assistedinject.Assisted;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-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;
 
 import javax.annotation.Nullable;
@@ -130,34 +129,35 @@ public class ProjectControl {
   private final Set<AccountGroup.UUID> receiveGroups;
 
   private final String canonicalWebUrl;
-  private final RefControl.Factory refControlFactory;
   private final SchemaFactory<ReviewDb> schema;
   private final CurrentUser user;
   private final ProjectState state;
   private final GroupCache groupCache;
 
-
+  private List<SectionMatcher> allSections;
-  private Collection<AccessSection> access;
+  private Map<String, RefControl> refControls;
+  private Boolean declaredOwner;
 
   @Inject
   ProjectControl(@GitUploadPackGroups Set<AccountGroup.UUID> uploadGroups,
       @GitReceivePackGroups Set<AccountGroup.UUID> receiveGroups,
       final SchemaFactory<ReviewDb> schema, final GroupCache groupCache,
       @CanonicalWebUrl @Nullable final String canonicalWebUrl,
-      final RefControl.Factory refControlFactory,
       @Assisted CurrentUser who, @Assisted ProjectState ps) {
     this.uploadGroups = uploadGroups;
     this.receiveGroups = receiveGroups;
     this.schema = schema;
     this.groupCache = groupCache;
     this.canonicalWebUrl = canonicalWebUrl;
-    this.refControlFactory = refControlFactory;
     user = who;
     state = ps;
   }
 
-  public ProjectControl forUser(final CurrentUser who) {
+  public ProjectControl forUser(CurrentUser who) {
-    return state.controlFor(who);
+    ProjectControl r = state.controlFor(who);
+    // Not per-user, and reusing saves lookup time.
+    r.allSections = allSections;
+    return r;
   }
 
   public ChangeControl controlFor(final Change change) {
@@ -169,7 +169,17 @@ public class ProjectControl {
   }
 
   public RefControl controlForRef(String refName) {
-    return refControlFactory.create(this, refName);
+    if (refControls == null) {
+      refControls = new HashMap<String, RefControl>();
+    }
+    RefControl ctl = refControls.get(refName);
+    if (ctl == null) {
+      PermissionCollection relevant =
+          PermissionCollection.filter(access(), refName, user.getUserName());
+      ctl = new RefControl(this, refName, relevant);
+      refControls.put(refName, ctl);
+    }
+    return ctl;
   }
 
   public CurrentUser getCurrentUser() {
@@ -181,7 +191,7 @@ public class ProjectControl {
   }
 
   public Project getProject() {
-    return getProjectState().getProject();
+    return state.getProject();
   }
 
   /** Can this user see this project exists? */
@@ -203,20 +213,27 @@ public class ProjectControl {
 
   /** Is this project completely visible for replication? */
   boolean visibleForReplication() {
-    return getCurrentUser() instanceof ReplicationUser
+    return user instanceof ReplicationUser
-        && ((ReplicationUser) getCurrentUser()).isEverythingVisible();
+        && ((ReplicationUser) user).isEverythingVisible();
   }
 
   /** Is this user a project owner? Ownership does not imply {@link #isVisible()} */
   public boolean isOwner() {
-    return controlForRef(AccessSection.ALL).isOwner()
+    return isDeclaredOwner()
-        || getCurrentUser().getCapabilities().canAdministrateServer();
+      || user.getCapabilities().canAdministrateServer();
+  }
+
+  private boolean isDeclaredOwner() {
+    if (declaredOwner == null) {
+      declaredOwner = state.isOwner(user.getEffectiveGroups());
+    }
+    return declaredOwner;
   }
 
   /** Does this user have ownership on at least one reference name? */
   public boolean isOwnerAnyRef() {
     return canPerformOnAnyRef(Permission.OWNER)
-        || getCurrentUser().getCapabilities().canAdministrateServer();
+        || user.getCapabilities().canAdministrateServer();
   }
 
   /** @return true if the user can upload to at least one reference */
@@ -370,33 +387,16 @@ public class ProjectControl {
     return value == null || value.trim().equals("");
   }
 
-  /**
-   * @return the effective groups of the current user for this project
-   */
-  private Set<AccountGroup.UUID> getEffectiveUserGroups() {
-    final Set<AccountGroup.UUID> userGroups = user.getEffectiveGroups();
-    if (isOwner()) {
-      final Set<AccountGroup.UUID> userGroupsOnProject =
-          new HashSet<AccountGroup.UUID>(userGroups.size() + 1);
-      userGroupsOnProject.addAll(userGroups);
-      userGroupsOnProject.add(AccountGroup.PROJECT_OWNERS);
-      return Collections.unmodifiableSet(userGroupsOnProject);
-    } else {
-      return userGroups;
-    }
-  }
-
   private boolean canPerformOnAnyRef(String permissionName) {
-    final Set<AccountGroup.UUID> groups = getEffectiveUserGroups();
+    for (SectionMatcher matcher : access()) {
-
+      AccessSection section = matcher.section;
-    for (AccessSection section : access()) {
       Permission permission = section.getPermission(permissionName);
       if (permission == null) {
         continue;
       }
 
       for (PermissionRule rule : permission.getRules()) {
-        if (rule.getDeny()) {
+        if (rule.getDeny() || !match(rule)) {
           continue;
         }
 
@@ -404,9 +404,10 @@ public class ProjectControl {
         // approximation.  There might be overrides and doNotInherit
         // that would render this to be false.
         //
-        if (groups.contains(rule.getGroup().getUUID())
+        if (controlForRef(section.getName()).canPerform(permissionName)) {
-            && controlForRef(section.getName()).canPerform(permissionName)) {
           return true;
+        } else {
+          break;
         }
       }
     }
@@ -435,7 +436,8 @@ public class ProjectControl {
 
   private Set<String> allRefPatterns(String permissionName) {
     Set<String> all = new HashSet<String>();
-    for (AccessSection section : access()) {
+    for (SectionMatcher matcher : access()) {
+      AccessSection section = matcher.section;
       Permission permission = section.getPermission(permissionName);
       if (permission != null) {
         all.add(section.getName());
@@ -444,18 +446,40 @@ public class ProjectControl {
     return all;
   }
 
-  Collection<AccessSection> access() {
+  private List<SectionMatcher> access() {
-    if (access == null) {
+    if (allSections == null) {
-      access = state.getAllAccessSections();
+      allSections = state.getAllSections();
+    }
+    return allSections;
+  }
+
+  boolean match(PermissionRule rule) {
+    return match(rule.getGroup().getUUID());
+  }
+
+  boolean match(AccountGroup.UUID uuid) {
+    if (AccountGroup.PROJECT_OWNERS.equals(uuid)) {
+      return isDeclaredOwner();
+    } else {
+      return user.getEffectiveGroups().contains(uuid);
     }
-    return access;
   }
 
   public boolean canRunUploadPack() {
-    return isAnyIncludedIn(uploadGroups, getEffectiveUserGroups());
+    for (AccountGroup.UUID group : uploadGroups) {
+      if (match(group)) {
+        return true;
+      }
+    }
+    return false;
   }
 
   public boolean canRunReceivePack() {
-    return isAnyIncludedIn(receiveGroups, getEffectiveUserGroups());
+    for (AccountGroup.UUID group : receiveGroups) {
+      if (match(group)) {
+        return true;
+      }
+    }
+    return false;
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.project;
 
+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;
@@ -66,6 +67,9 @@ public class ProjectState {
   /** Last system time the configuration's revision was examined. */
   private volatile long lastCheckTime;
 
+  /** Local access sections, wrapped in SectionMatchers for faster evaluation. */
+  private volatile List<SectionMatcher> localAccessSections;
+
   /** If this is all projects, the capabilities used by the server. */
   private final CapabilityCollection capabilities;
 
@@ -148,58 +152,62 @@ public class ProjectState {
     if (pmc == null) {
       pmc = rulesCache.loadMachine(
           getProject().getNameKey(),
-          getConfig().getRulesId());
+          config.getRulesId());
       rulesMachine = pmc;
     }
     return envFactory.create(pmc);
   }
 
   public Project getProject() {
-    return getConfig().getProject();
+    return config.getProject();
   }
 
   public ProjectConfig getConfig() {
     return config;
   }
 
-  /** Get the rights that pertain only to this project. */
+  /** Get the sections that pertain only to this project. */
-  public Collection<AccessSection> getLocalAccessSections() {
+  private List<SectionMatcher> getLocalAccessSections() {
-    return getConfig().getAccessSections();
+    List<SectionMatcher> sm = localAccessSections;
+    if (sm == null) {
+      Collection<AccessSection> fromConfig = config.getAccessSections();
+      sm = new ArrayList<SectionMatcher>(fromConfig.size());
+      for (AccessSection section : fromConfig) {
+        SectionMatcher matcher = SectionMatcher.wrap(section);
+        if (matcher != null) {
+          sm.add(matcher);
+        }
+      }
+      localAccessSections = sm;
+    }
+    return sm;
   }
 
-  /** Get the rights this project inherits. */
+  /**
-  public Collection<AccessSection> getInheritedAccessSections() {
+   * Obtain all local and inherited sections. This collection is looked up
+   * dynamically and is not cached. Callers should try to cache this result
+   * per-request as much as possible.
+   */
+  List<SectionMatcher> getAllSections() {
     if (isAllProjects) {
-      return Collections.emptyList();
+      return getLocalAccessSections();
     }
 
-    List<AccessSection> inherited = new ArrayList<AccessSection>();
+    List<SectionMatcher> all = new ArrayList<SectionMatcher>();
     Set<Project.NameKey> seen = new HashSet<Project.NameKey>();
-    Project.NameKey parent = getProject().getParent();
+    seen.add(getProject().getNameKey());
 
-    while (parent != null && seen.add(parent)) {
+    ProjectState s = this;
-      ProjectState s = projectCache.get(parent);
+    do {
-      if (s != null) {
+      all.addAll(s.getLocalAccessSections());
-        inherited.addAll(s.getLocalAccessSections());
+
-        parent = s.getProject().getParent();
+      Project.NameKey parent = s.getProject().getParent();
-      } else {
+      if (parent == null || !seen.add(parent)) {
         break;
       }
-    }
+      s = projectCache.get(parent);
-
+    } while (s != null);
-    // The root of the tree is the special "All-Projects" case.
+    all.addAll(projectCache.getAllProjects().getLocalAccessSections());
-    if (parent == null) {
-      inherited.addAll(projectCache.getAllProjects().getLocalAccessSections());
-    }
-
-    return inherited;
-  }
-
-  /** Get both local and inherited access sections. */
-  public Collection<AccessSection> getAllAccessSections() {
-    List<AccessSection> all = new ArrayList<AccessSection>();
-    all.addAll(getLocalAccessSections());
-    all.addAll(getInheritedAccessSections());
     return all;
   }
 
@@ -224,30 +232,26 @@ public class ProjectState {
   }
 
   /**
-   * @return all {@link AccountGroup}'s that are allowed to administrate the
+   * @return true if any of the groups listed in {@code groups} was declared to
-   *         complete project. This includes all groups to which the owner
+   *         be an owner of this project, or one of its parent projects..
-   *         privilege for 'refs/*' is assigned for this project (the local
-   *         owners) and all groups to which the owner privilege for 'refs/*' is
-   *         assigned for one of the parent projects (the inherited owners).
    */
-  public Set<AccountGroup.UUID> getAllOwners() {
+  boolean isOwner(Set<AccountGroup.UUID> groups) {
-    HashSet<AccountGroup.UUID> owners = new HashSet<AccountGroup.UUID>();
-    owners.addAll(localOwners);
-
     Set<Project.NameKey> seen = new HashSet<Project.NameKey>();
-    Project.NameKey parent = getProject().getParent();
+    seen.add(getProject().getNameKey());
 
-    while (parent != null && seen.add(parent)) {
+    ProjectState s = this;
-      ProjectState s = projectCache.get(parent);
+    do {
-      if (s != null) {
+      if (CollectionsUtil.isAnyIncludedIn(s.localOwners, groups)) {
-        owners.addAll(s.localOwners);
+        return true;
-        parent = s.getProject().getParent();
+      }
-      } else {
+
+      Project.NameKey parent = s.getProject().getParent();
+      if (parent == null || !seen.add(parent)) {
         break;
       }
-    }
+      s = projectCache.get(parent);
-
+    } while (s != null);
-    return Collections.unmodifiableSet(owners);
+    return false;
   }
 
   public ProjectControl controlFor(final CurrentUser user) {

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

@@ -14,22 +14,16 @@
 
 package com.google.gerrit.server.project;
 
-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.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
 
 import dk.brics.automaton.RegExp;
 
-import org.apache.commons.lang.StringUtils;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevObject;
@@ -39,43 +33,32 @@ import org.eclipse.jgit.revwalk.RevWalk;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import java.util.regex.Pattern;
 
 
 /** Manages access control for Git references (aka branches, tags). */
 public class RefControl {
-  public interface Factory {
-    RefControl create(ProjectControl projectControl, String ref);
-  }
-
   private final ProjectControl projectControl;
   private final String refName;
 
-  private Map<String, List<PermissionRule>> permissions;
+  /** All permissions that apply to this reference. */
+  private final PermissionCollection relevant;
+
+  /** Cached set of permissions matching this user. */
+  private final Map<String, List<PermissionRule>> effective;
 
   private Boolean owner;
   private Boolean canForgeAuthor;
   private Boolean canForgeCommitter;
 
-  @Inject
+  RefControl(ProjectControl projectControl, String ref,
-  protected RefControl(@Assisted final ProjectControl projectControl,
+      PermissionCollection relevant) {
-      @Assisted String ref) {
-    if (isRE(ref)) {
-      ref = shortestExample(ref);
-
-    } else if (ref.endsWith("/*")) {
-      ref = ref.substring(0, ref.length() - 1);
-
-    }
-
     this.projectControl = projectControl;
     this.refName = ref;
+    this.relevant = relevant;
+    this.effective = new HashMap<String, List<PermissionRule>>();
   }
 
   public String getRefName() {
@@ -87,11 +70,16 @@ public class RefControl {
   }
 
   public CurrentUser getCurrentUser() {
-    return getProjectControl().getCurrentUser();
+    return projectControl.getCurrentUser();
   }
 
-  public RefControl forUser(final CurrentUser who) {
+  public RefControl forUser(CurrentUser who) {
-    return getProjectControl().forUser(who).controlForRef(getRefName());
+    ProjectControl newCtl = projectControl.forUser(who);
+    if (relevant.isUserSpecific()) {
+      return newCtl.controlForRef(getRefName());
+    } else {
+      return new RefControl(newCtl, getRefName(), relevant);
+    }
   }
 
   /** Is this user a ref owner? */
@@ -100,16 +88,8 @@ public class RefControl {
       if (canPerform(Permission.OWNER)) {
         owner = true;
 
-      } 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().getCapabilities().canAdministrateServer();
-
       } else {
-        owner = getProjectControl().isOwner();
+        owner = projectControl.isOwner();
       }
     }
     return owner;
@@ -117,7 +97,7 @@ public class RefControl {
 
   /** Can this user see this reference exists? */
   public boolean isVisible() {
-    return getProjectControl().visibleForReplication()
+    return projectControl.visibleForReplication()
         || canPerform(Permission.READ);
   }
 
@@ -129,14 +109,14 @@ public class RefControl {
    *         ref
    */
   public boolean canUpload() {
-    return getProjectControl()
+    return projectControl
-        .controlForRef("refs/for/" + getRefName())
+      .controlForRef("refs/for/" + getRefName())
-        .canPerform(Permission.PUSH);
+      .canPerform(Permission.PUSH);
   }
 
   /** @return true if this user can submit merge patch sets to this ref */
   public boolean canUploadMerges() {
-    return getProjectControl()
+    return projectControl
       .controlForRef("refs/for/" + getRefName())
       .canPerform(Permission.PUSH_MERGE);
   }
@@ -149,7 +129,7 @@ public class RefControl {
       // rules. Allowing this to be done by a non-project-owner opens
       // a security hole enabling editing of access rules, and thus
       // granting of powers beyond submitting to the configuration.
-      return getProjectControl().isOwner();
+      return projectControl.isOwner();
     }
     return canPerform(Permission.SUBMIT);
   }
@@ -157,7 +137,7 @@ public class RefControl {
   /** @return true if the user can update the reference as a fast-forward. */
   public boolean canUpdate() {
     if (GitRepositoryManager.REF_CONFIG.equals(refName)
-        && !getProjectControl().isOwner()) {
+        && !projectControl.isOwner()) {
       // Pushing requires being at least project owner, in addition to push.
       // Pushing configuration changes modifies the access control
       // rules. Allowing this to be done by a non-project-owner opens
@@ -175,7 +155,7 @@ public class RefControl {
 
   private boolean canPushWithForce() {
     if (GitRepositoryManager.REF_CONFIG.equals(refName)
-        && !getProjectControl().isOwner()) {
+        && !projectControl.isOwner()) {
       // Pushing requires being at least project owner, in addition to push.
       // Pushing configuration changes modifies the access control
       // rules. Allowing this to be done by a non-project-owner opens
@@ -303,9 +283,19 @@ public class RefControl {
   /** All value ranges of any allowed label permission. */
   public List<PermissionRange> getLabelRanges() {
     List<PermissionRange> r = new ArrayList<PermissionRange>();
-    for (Map.Entry<String, List<PermissionRule>> e : permissions().entrySet()) {
+    for (Map.Entry<String, List<PermissionRule>> e : relevant.getDeclaredPermissions()) {
       if (Permission.isLabel(e.getKey())) {
-        r.add(toRange(e.getKey(), e.getValue()));
+        int min = 0;
+        int max = 0;
+        for (PermissionRule rule : e.getValue()) {
+          if (projectControl.match(rule)) {
+            min = Math.min(min, rule.getMin());
+            max = Math.max(max, rule.getMax());
+          }
+        }
+        if (min != 0 || max != 0) {
+          r.add(new PermissionRange(e.getKey(), min, max));
+        }
       }
     }
     return r;
@@ -319,7 +309,8 @@ public class RefControl {
     return null;
   }
 
-  private static PermissionRange toRange(String permissionName, List<PermissionRule> ruleList) {
+  private static PermissionRange toRange(String permissionName,
+      List<PermissionRule> ruleList) {
     int min = 0;
     int max = 0;
     for (PermissionRule rule : ruleList) {
@@ -336,115 +327,41 @@ public class RefControl {
 
   /** Rules for the given permission, or the empty list. */
   private List<PermissionRule> access(String permissionName) {
-    List<PermissionRule> r = permissions().get(permissionName);
+    List<PermissionRule> rules = effective.get(permissionName);
-    return r != null ? r : Collections.<PermissionRule> emptyList();
+    if (rules != null) {
-  }
+      return rules;
-
-  /** All rules that pertain to this user, on this reference. */
-  private Map<String, List<PermissionRule>> permissions() {
-    if (permissions == null) {
-      List<AccessSection> sections = new ArrayList<AccessSection>();
-      for (AccessSection section : projectControl.access()) {
-        if (appliesToRef(section)) {
-          sections.add(section);
-        }
-      }
-      Collections.sort(sections, new MostSpecificComparator(getRefName()));
-
-      Set<SeenRule> seen = new HashSet<SeenRule>();
-      Set<String> exclusiveGroupPermissions = new HashSet<String>();
-
-      permissions = new HashMap<String, List<PermissionRule>>();
-      for (AccessSection section : sections) {
-        for (Permission permission : section.getPermissions()) {
-          if (exclusiveGroupPermissions.contains(permission.getName())) {
-            continue;
-          }
-
-          for (PermissionRule rule : permission.getRules()) {
-            if (matchGroup(rule.getGroup().getUUID())) {
-              SeenRule s = new SeenRule(section, permission, rule);
-              if (seen.add(s) && !rule.getDeny()) {
-                List<PermissionRule> r = permissions.get(permission.getName());
-                if (r == null) {
-                  r = new ArrayList<PermissionRule>(2);
-                  permissions.put(permission.getName(), r);
-                }
-                r.add(rule);
-              }
-            }
-          }
-
-          if (permission.getExclusiveGroup()) {
-            exclusiveGroupPermissions.add(permission.getName());
-          }
-        }
-      }
-    }
-    return permissions;
-  }
-
-  private boolean appliesToRef(AccessSection section) {
-    String refPattern = section.getName();
-
-    if (isTemplate(refPattern)) {
-      ParamertizedString template = new ParamertizedString(refPattern);
-      HashMap<String, String> p = new HashMap<String, String>();
-
-      if (getCurrentUser() instanceof IdentifiedUser) {
-        p.put("username", ((IdentifiedUser) getCurrentUser()).getUserName());
-      } else {
-        // Right now we only template the username. If not available
-        // this rule cannot be matched at all.
-        //
-        return false;
-      }
-
-      if (isRE(refPattern)) {
-        for (Map.Entry<String, String> ent : p.entrySet()) {
-          ent.setValue(escape(ent.getValue()));
-        }
-      }
-
-      refPattern = template.replace(p);
     }
 
-    if (isRE(refPattern)) {
+    rules = relevant.getPermission(permissionName);
-      return Pattern.matches(refPattern, getRefName());
 
-    } else if (refPattern.endsWith("/*")) {
+    if (rules.isEmpty()) {
-      String prefix = refPattern.substring(0, refPattern.length() - 1);
+      effective.put(permissionName, rules);
-      return getRefName().startsWith(prefix);
+      return rules;
-
-    } else {
-      return getRefName().equals(refPattern);
     }
-  }
 
-  private boolean matchGroup(AccountGroup.UUID uuid) {
+    if (rules.size() == 1) {
-    Set<AccountGroup.UUID> userGroups = getCurrentUser().getEffectiveGroups();
+      if (!projectControl.match(rules.get(0))) {
-
+        rules = Collections.emptyList();
-    if (AccountGroup.PROJECT_OWNERS.equals(uuid)) {
+      }
-      ProjectState state = projectControl.getProjectState();
+      effective.put(permissionName, rules);
-      return CollectionsUtil.isAnyIncludedIn(state.getAllOwners(), userGroups);
+      return rules;
-
-    } else {
-      return userGroups.contains(uuid);
     }
+
+    List<PermissionRule> mine = new ArrayList<PermissionRule>(rules.size());
+    for (PermissionRule rule : rules) {
+      if (projectControl.match(rule)) {
+        mine.add(rule);
+      }
+    }
+
+    if (mine.isEmpty()) {
+      mine = Collections.emptyList();
+    }
+    effective.put(permissionName, mine);
+    return mine;
   }
 
-  private static boolean isTemplate(String refPattern) {
+  static boolean isRE(String refPattern) {
-    return 0 <= refPattern.indexOf("${");
-  }
-
-  private static String escape(String value) {
-    // Right now the only special character allowed in a
-    // variable value is a . in the username.
-    //
-    return value.replace(".", "\\.");
-  }
-
-  private static boolean isRE(String refPattern) {
     return refPattern.startsWith(AccessSection.REGEX_PREFIX);
   }
 
@@ -458,149 +375,10 @@ public class RefControl {
     }
   }
 
-  private static RegExp toRegExp(String refPattern) {
+  static RegExp toRegExp(String refPattern) {
     if (isRE(refPattern)) {
       refPattern = refPattern.substring(1);
     }
     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.getName();
-      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:
-   * <ul>
-   * <li>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.
-   * <li>2 - Finites first, infinities after.
-   * <li>3 - Number of transitions.
-   * <li>4 - Length of the expression text.
-   * </ul>
-   *
-   * 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<AccessSection> {
-    private final String refName;
-
-    MostSpecificComparator(String refName) {
-      this.refName = refName;
-    }
-
-    public int compare(AccessSection a, AccessSection b) {
-      return compare(a.getName(), b.getName());
-    }
-
-    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();
-      }
-    }
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/project/SectionMatcher.java

@@ -0,0 +1,154 @@
+// 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.project;
+
+import static com.google.gerrit.server.project.RefControl.isRE;
+
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.ParamertizedString;
+
+import dk.brics.automaton.Automaton;
+
+import java.util.Collections;
+import java.util.regex.Pattern;
+
+/**
+ * Matches an AccessSection against a reference name.
+ * <p>
+ * These matchers are "compiled" versions of the AccessSection name, supporting
+ * faster selection of which sections are relevant to any given input reference.
+ */
+abstract class SectionMatcher {
+  static SectionMatcher wrap(AccessSection section) {
+    String ref = section.getName();
+    if (AccessSection.isAccessSection(ref)) {
+      return wrap(ref, section);
+    } else {
+      return null;
+    }
+  }
+
+  static SectionMatcher wrap(String pattern, AccessSection section) {
+    if (pattern.contains("${")) {
+      return new ExpandParameters(pattern, section);
+
+    } else if (isRE(pattern)) {
+      return new Regexp(pattern, section);
+
+    } else if (pattern.endsWith("/*")) {
+      return new Prefix(pattern.substring(0, pattern.length() - 1), section);
+
+    } else {
+      return new Exact(pattern, section);
+    }
+  }
+
+  final AccessSection section;
+
+  SectionMatcher(AccessSection section) {
+    this.section = section;
+  }
+
+  abstract boolean match(String ref, String username);
+
+  private static class Exact extends SectionMatcher {
+    private final String expect;
+
+    Exact(String name, AccessSection section) {
+      super(section);
+      expect = name;
+    }
+
+    @Override
+    boolean match(String ref, String username) {
+      return expect.equals(ref);
+    }
+  }
+
+  private static class Prefix extends SectionMatcher {
+    private final String prefix;
+
+    Prefix(String pfx, AccessSection section) {
+      super(section);
+      prefix = pfx;
+    }
+
+    @Override
+    boolean match(String ref, String username) {
+      return ref.startsWith(prefix);
+    }
+  }
+
+  private static class Regexp extends SectionMatcher {
+    private final Pattern pattern;
+
+    Regexp(String re, AccessSection section) {
+      super(section);
+      pattern = Pattern.compile(re);
+    }
+
+    @Override
+    boolean match(String ref, String username) {
+      return pattern.matcher(ref).matches();
+    }
+  }
+
+  static class ExpandParameters extends SectionMatcher {
+    private final ParamertizedString template;
+    private final String prefix;
+
+    ExpandParameters(String pattern, AccessSection section) {
+      super(section);
+      template = new ParamertizedString(pattern);
+
+      if (isRE(pattern)) {
+        // Replace ${username} with ":USERNAME:" as : is not legal
+        // in a reference and the string :USERNAME: is not likely to
+        // be a valid part of the regex. This later allows the pattern
+        // prefix to be clipped, saving time on evaluation.
+        Automaton am = RefControl.toRegExp(
+            template.replace(Collections.singletonMap("username", ":USERNAME:")))
+            .toAutomaton();
+        String rePrefix = am.getCommonPrefix();
+        prefix = rePrefix.substring(0, rePrefix.indexOf(":USERNAME:"));
+      } else {
+        prefix = pattern.substring(0, pattern.indexOf("${"));
+      }
+    }
+
+    @Override
+    boolean match(String ref, String username) {
+      if (!ref.startsWith(prefix) || username == null) {
+        return false;
+      }
+
+      String u;
+      if (isRE(template.getPattern())) {
+        u = username.replace(".", "\\.");
+      } else {
+        u = username;
+      }
+
+      SectionMatcher next = wrap(
+          template.replace(Collections.singletonMap("username", u)),
+          section);
+      return next != null ? next.match(ref, username) : false;
+    }
+
+   boolean matchPrefix(String ref) {
+     return ref.startsWith(prefix);
+    }
+  }
+}

gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java

@@ -195,6 +195,21 @@ public class RefControlTest extends TestCase {
         u.controlForRef("refs/heads/master").canUpload());
   }
 
+  public void testUsernamePatternNonRegex() {
+    grant(local, READ, devs, "refs/sb/${username}/heads/*");
+
+    ProjectControl u = user("u", devs), d = user("d", devs);
+    assertFalse("u can't read", u.controlForRef("refs/sb/d/heads/foobar").isVisible());
+    assertTrue("d can read", d.controlForRef("refs/sb/d/heads/foobar").isVisible());
+  }
+
+  public void testUsernamePatternWithRegex() {
+    grant(local, READ, devs, "^refs/sb/${username}/heads/.*");
+
+    ProjectControl u = user("d.v", devs), d = user("dev", devs);
+    assertFalse("u can't read", u.controlForRef("refs/sb/dev/heads/foobar").isVisible());
+    assertTrue("d can read", d.controlForRef("refs/sb/dev/heads/foobar").isVisible());
+  }
 
   // -----------------------------------------------------------------------
 
@@ -307,19 +322,17 @@ public class RefControlTest extends TestCase {
   }
 
   private ProjectControl user(AccountGroup.UUID... memberOf) {
+    return user(null, memberOf);
+  }
+
+  private ProjectControl user(String name, AccountGroup.UUID... memberOf) {
     SchemaFactory<ReviewDb> schema = null;
     GroupCache groupCache = null;
     String canonicalWebUrl = "http://localhost";
 
-    RefControl.Factory refControlFactory = new RefControl.Factory() {
-      @Override
-      public RefControl create(final ProjectControl projectControl, final String ref) {
-        return new RefControl(projectControl, ref);
-      }
-    };
     return new ProjectControl(Collections.<AccountGroup.UUID> emptySet(),
         Collections.<AccountGroup.UUID> emptySet(), schema, groupCache,
-        canonicalWebUrl, refControlFactory, new MockUser(memberOf),
+        canonicalWebUrl, new MockUser(name, memberOf),
         newProjectState());
   }
 
@@ -338,10 +351,12 @@ public class RefControlTest extends TestCase {
   }
 
   private class MockUser extends CurrentUser {
+    private final String username;
     private final Set<AccountGroup.UUID> groups;
 
-    MockUser(AccountGroup.UUID[] groupId) {
+    MockUser(String name, AccountGroup.UUID[] groupId) {
       super(RefControlTest.this.capabilityControlFactory, AccessPath.UNKNOWN);
+      username = name;
       groups = new HashSet<AccountGroup.UUID>(Arrays.asList(groupId));
       groups.add(registered);
       groups.add(anonymous);
@@ -352,6 +367,11 @@ public class RefControlTest extends TestCase {
       return groups;
     }
 
+    @Override
+    public String getUserName() {
+      return username;
+    }
+
     @Override
     public Set<Change.Id> getStarredChanges() {
       return Collections.emptySet();