Move all static ref pattern related methods into a RefPattern class

The goal of this refactoring is to have a single place where a ref
pattern is validated as regular expression. So far this was done in 2
places (in RefControl and in ProjectConfig). For validating a ref
pattern as regular expression we must remove placeholder variables
such as '${username}' because it would be an invalid regular
expression otherwise. We don't want to do this in several places
especially because we want to support further placeholder variables in
future.

Change-Id: I11c76e37f22233d89b631375ac6f2ef60f97801f
Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ProjectAccessHandler.java

@@ -37,7 +37,7 @@ import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectControl;
-import com.google.gerrit.server.project.RefControl;
+import com.google.gerrit.server.project.RefPattern;
 import com.google.gerrit.server.project.SetParent;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Provider;
@@ -111,7 +111,7 @@ public abstract class ProjectAccessHandler<T> extends Handler<T> {
             continue;
           }
 
-          RefControl.validateRefPattern(name);
+          RefPattern.validate(name);
 
           replace(config, toDelete, section);
         }

gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java

@@ -40,6 +40,7 @@ import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.common.data.PermissionRule.Action;
 import com.google.gerrit.common.data.RefConfigSection;
 import com.google.gerrit.common.data.SubscribeSection;
+import com.google.gerrit.common.errors.InvalidNameException;
 import com.google.gerrit.extensions.client.InheritableBoolean;
 import com.google.gerrit.extensions.client.ProjectState;
 import com.google.gerrit.extensions.client.SubmitType;
@@ -53,6 +54,7 @@ import com.google.gerrit.server.config.ConfigUtil;
 import com.google.gerrit.server.config.PluginConfig;
 import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.server.project.CommentLinkInfo;
+import com.google.gerrit.server.project.RefPattern;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.CommitBuilder;
@@ -645,8 +647,8 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
 
   private boolean isValidRegex(String refPattern) {
     try {
-      Pattern.compile(refPattern.replace("${username}/", ""));
+      RefPattern.validateRegExp(refPattern);
-    } catch (PatternSyntaxException e) {
+    } catch (InvalidNameException e) {
       error(new ValidationError(PROJECT_CONFIG, "Invalid ref name: "
           + e.getMessage()));
       return false;

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

@@ -15,7 +15,7 @@
 package com.google.gerrit.server.project;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
-import static com.google.gerrit.server.project.RefControl.isRE;
+import static com.google.gerrit.server.project.RefPattern.isRE;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.Lists;
@@ -74,7 +74,7 @@ public class PermissionCollection {
     PermissionCollection filter(Iterable<SectionMatcher> matcherList,
         String ref, CurrentUser user) {
       if (isRE(ref)) {
-        ref = RefControl.shortestExample(ref);
+        ref = RefPattern.shortestExample(ref);
       } else if (ref.endsWith("/*")) {
         ref = ref.substring(0, ref.length() - 1);
       }

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

@@ -14,20 +14,15 @@
 
 package com.google.gerrit.server.project;
 
-import com.google.gerrit.common.data.AccessSection;
 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.common.data.RefConfigSection;
-import com.google.gerrit.common.errors.InvalidNameException;
 import com.google.gerrit.extensions.client.ProjectState;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.group.SystemGroupBackend;
 
-import dk.brics.automaton.RegExp;
-
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
@@ -47,8 +42,6 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.regex.Pattern;
-import java.util.regex.PatternSyntaxException;
 
 
 /** Manages access control for Git references (aka branches, tags). */
@@ -646,53 +639,4 @@ public class RefControl {
     effective.put(permissionName, mine);
     return mine;
   }
-
-  public static boolean isRE(String refPattern) {
-    return refPattern.startsWith(AccessSection.REGEX_PREFIX);
-  }
-
-  public static String shortestExample(String pattern) {
-    if (isRE(pattern)) {
-      // Since Brics will substitute dot [.] with \0 when generating
-      // shortest example, any usage of dot will fail in
-      // Repository.isValidRefName() if not combined with star [*].
-      // To get around this, we substitute the \0 with an arbitrary
-      // accepted character.
-      return toRegExp(pattern).toAutomaton().getShortestExample(true).replace('\0', '-');
-    } else if (pattern.endsWith("/*")) {
-      return pattern.substring(0, pattern.length() - 1) + '1';
-    } else {
-      return pattern;
-    }
-  }
-
-  public static RegExp toRegExp(String refPattern) {
-    if (isRE(refPattern)) {
-      refPattern = refPattern.substring(1);
-    }
-    return new RegExp(refPattern, RegExp.NONE);
-  }
-
-  public static void validateRefPattern(String refPattern)
-      throws InvalidNameException {
-    if (refPattern.startsWith(RefConfigSection.REGEX_PREFIX)) {
-      if (!Repository.isValidRefName(RefControl.shortestExample(refPattern))) {
-        throw new InvalidNameException(refPattern);
-      }
-    } else if (refPattern.equals(RefConfigSection.ALL)) {
-      // This is a special case we have to allow, it fails below.
-    } else if (refPattern.endsWith("/*")) {
-      String prefix = refPattern.substring(0, refPattern.length() - 2);
-      if (!Repository.isValidRefName(prefix)) {
-        throw new InvalidNameException(refPattern);
-      }
-    } else if (!Repository.isValidRefName(refPattern)) {
-      throw new InvalidNameException(refPattern);
-    }
-    try {
-      Pattern.compile(refPattern.replace("${username}/", ""));
-    } catch (PatternSyntaxException e) {
-      throw new InvalidNameException(refPattern + " " + e.getMessage());
-    }
-  }
 }

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

@@ -0,0 +1,85 @@
+// Copyright (C) 2016 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 com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.RefConfigSection;
+import com.google.gerrit.common.errors.InvalidNameException;
+
+import dk.brics.automaton.RegExp;
+
+import org.eclipse.jgit.lib.Repository;
+
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+
+public class RefPattern {
+  public static final String USERNAME = "username";
+
+  public static String shortestExample(String refPattern) {
+    if (isRE(refPattern)) {
+      // Since Brics will substitute dot [.] with \0 when generating
+      // shortest example, any usage of dot will fail in
+      // Repository.isValidRefName() if not combined with star [*].
+      // To get around this, we substitute the \0 with an arbitrary
+      // accepted character.
+      return toRegExp(refPattern).toAutomaton().getShortestExample(true)
+          .replace('\0', '-');
+    } else if (refPattern.endsWith("/*")) {
+      return refPattern.substring(0, refPattern.length() - 1) + '1';
+    } else {
+      return refPattern;
+    }
+  }
+
+  public static boolean isRE(String refPattern) {
+    return refPattern.startsWith(AccessSection.REGEX_PREFIX);
+  }
+
+  public static RegExp toRegExp(String refPattern) {
+    if (isRE(refPattern)) {
+      refPattern = refPattern.substring(1);
+    }
+    return new RegExp(refPattern, RegExp.NONE);
+  }
+
+  public static void validate(String refPattern)
+      throws InvalidNameException {
+    if (refPattern.startsWith(RefConfigSection.REGEX_PREFIX)) {
+      if (!Repository.isValidRefName(shortestExample(refPattern))) {
+        throw new InvalidNameException(refPattern);
+      }
+    } else if (refPattern.equals(RefConfigSection.ALL)) {
+      // This is a special case we have to allow, it fails below.
+    } else if (refPattern.endsWith("/*")) {
+      String prefix = refPattern.substring(0, refPattern.length() - 2);
+      if (!Repository.isValidRefName(prefix)) {
+        throw new InvalidNameException(refPattern);
+      }
+    } else if (!Repository.isValidRefName(refPattern)) {
+      throw new InvalidNameException(refPattern);
+    }
+    validateRegExp(refPattern);
+  }
+
+  public static void validateRegExp(String refPattern)
+      throws InvalidNameException {
+    try {
+      Pattern.compile(refPattern.replace("${" + USERNAME + "}/", ""));
+    } catch (PatternSyntaxException e) {
+      throw new InvalidNameException(refPattern + " " + e.getMessage());
+    }
+  }
+}

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

@@ -14,7 +14,7 @@
 
 package com.google.gerrit.server.project;
 
-import static com.google.gerrit.server.project.RefControl.isRE;
+import static com.google.gerrit.server.project.RefPattern.isRE;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
@@ -93,12 +93,13 @@ public abstract class RefPatternMatcher {
         // 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.
+        String replacement = ":" + RefPattern.USERNAME.toUpperCase() + ":";
         Automaton am =
-            RefControl.toRegExp(
+            RefPattern.toRegExp(
-                template.replace(Collections.singletonMap("username",
+                template.replace(Collections.singletonMap(RefPattern.USERNAME,
-                    ":USERNAME:"))).toAutomaton();
+                    replacement))).toAutomaton();
         String rePrefix = am.getCommonPrefix();
-        prefix = rePrefix.substring(0, rePrefix.indexOf(":USERNAME:"));
+        prefix = rePrefix.substring(0, rePrefix.indexOf(replacement));
       } else {
         prefix = pattern.substring(0, pattern.indexOf("${"));
       }
@@ -154,8 +155,8 @@ public abstract class RefPatternMatcher {
     }
 
     private String expand(ParameterizedString parameterizedRef, String userName) {
-      return parameterizedRef.replace(Collections.singletonMap("username",
+      return parameterizedRef
-          userName));
+          .replace(Collections.singletonMap(RefPattern.USERNAME, userName));
     }
   }
 }

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

@@ -146,7 +146,7 @@ public class SetAccess implements
             throw new AuthException("You are not allowed to edit permissions"
                 + "for ref: " + name);
           }
-          RefControl.validateRefPattern(name);
+          RefPattern.validate(name);
         }
 
         // Check all permissions for soundness

gerrit-server/src/main/java/com/google/gerrit/server/util/MostSpecificComparator.java

@@ -15,7 +15,7 @@
 package com.google.gerrit.server.util;
 
 import com.google.gerrit.common.data.RefConfigSection;
-import com.google.gerrit.server.project.RefControl;
+import com.google.gerrit.server.project.RefPattern;
 
 import org.apache.commons.lang.StringUtils;
 
@@ -82,8 +82,8 @@ public final class MostSpecificComparator implements
 
   private int distance(String pattern) {
     String example;
-    if (RefControl.isRE(pattern)) {
+    if (RefPattern.isRE(pattern)) {
-      example = RefControl.shortestExample(pattern);
+      example = RefPattern.shortestExample(pattern);
 
     } else if (pattern.endsWith("/*")) {
       example = pattern;
@@ -98,8 +98,8 @@ public final class MostSpecificComparator implements
   }
 
   private boolean finite(String pattern) {
-    if (RefControl.isRE(pattern)) {
+    if (RefPattern.isRE(pattern)) {
-      return RefControl.toRegExp(pattern).toAutomaton().isFinite();
+      return RefPattern.toRegExp(pattern).toAutomaton().isFinite();
 
     } else if (pattern.endsWith("/*")) {
       return false;
@@ -110,8 +110,8 @@ public final class MostSpecificComparator implements
   }
 
   private int transitions(String pattern) {
-    if (RefControl.isRE(pattern)) {
+    if (RefPattern.isRE(pattern)) {
-      return RefControl.toRegExp(pattern).toAutomaton()
+      return RefPattern.toRegExp(pattern).toAutomaton()
           .getNumberOfTransitions();
 
     } else if (pattern.endsWith("/*")) {

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

@@ -823,27 +823,26 @@ public class RefControlTest {
 
   @Test
   public void testValidateRefPatternsOK() throws Exception {
-    RefControl.validateRefPattern("refs/*");
+    RefPattern.validate("refs/*");
-    RefControl.validateRefPattern("^refs/heads/*");
+    RefPattern.validate("^refs/heads/*");
-    RefControl.validateRefPattern("^refs/tags/[0-9a-zA-Z-_.]+");
+    RefPattern.validate("^refs/tags/[0-9a-zA-Z-_.]+");
-    RefControl.validateRefPattern("refs/heads/review/${username}/*");
+    RefPattern.validate("refs/heads/review/${username}/*");
   }
 
   @Test(expected = InvalidNameException.class)
   public void testValidateBadRefPatternDoubleCaret() throws Exception {
-    RefControl.validateRefPattern("^^refs/*");
+    RefPattern.validate("^^refs/*");
   }
 
   @Test(expected = InvalidNameException.class)
   public void testValidateBadRefPatternDanglingCharacter() throws Exception {
-    RefControl
+    RefPattern
-        .validateRefPattern("^refs/heads/tmp/sdk/[0-9]{3,3}_R[1-9][A-Z][0-9]{3,3}*");
+        .validate("^refs/heads/tmp/sdk/[0-9]{3,3}_R[1-9][A-Z][0-9]{3,3}*");
   }
 
   @Test
   public void testValidateRefPatternNoDanglingCharacter() throws Exception {
-    RefControl
+    RefPattern.validate("^refs/heads/tmp/sdk/[0-9]{3,3}_R[1-9][A-Z][0-9]{3,3}");
-        .validateRefPattern("^refs/heads/tmp/sdk/[0-9]{3,3}_R[1-9][A-Z][0-9]{3,3}");
   }
 
   private InMemoryRepository add(ProjectConfig pc) {