RefFilter: Disallow using substring and regex filters together

For the project list we disallow using the regex and substring
filters together in the same query.

Add the same restriction for listing branches and tags.

Change-Id: I76628b7a577fd385fefa33d36a3a0390a649a79e

Documentation/rest-api-projects.txt

@@ -1185,7 +1185,7 @@ Skip the given number of branches from the beginning of the list.
 Substring(m)::
 Limit the results to those branches that match the specified substring.
 +
-The match is case insensitive.
+The match is case insensitive. May not be used together with `r`.
 +
 List all branches that match substring `test`:
 
@@ -1217,7 +1217,7 @@ Boundary matchers '^' and '$' are implicit. For example: the regex 't*' will
 match any branches that start with 'test' and regex '*t' will match any
 branches that end with 'test'.
 +
-The match is case sensitive.
+The match is case sensitive. May not be used together with `m`.
 +
 List all branches that match regex `t.*1`:
 +
@@ -1875,7 +1875,7 @@ Skip the given number of tags from the beginning of the list.
 Substring(m)::
 Limit the results to those tags that match the specified substring.
 +
-The match is case insensitive.
+The match is case insensitive.  May not be used together with `r`.
 +
 List all tags that match substring `v2`:
 
@@ -1906,7 +1906,7 @@ Boundary matchers '^' and '$' are implicit. For example: the regex 't*' will
 match any tags that start with 't' and regex '*t' will match any
 tags that end with 't'.
 +
-The match is case sensitive.
+The match is case sensitive.  May not be used together with `m`.
 +
 List all tags that match regex `v.*0`:
 +

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.rest.project;
 
 import static com.google.gerrit.acceptance.rest.project.RefAssert.assertRefNames;
 import static com.google.gerrit.acceptance.rest.project.RefAssert.assertRefs;
+import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -23,6 +24,7 @@ import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.TestProjectInput;
 import com.google.gerrit.extensions.api.projects.BranchInfo;
 import com.google.gerrit.extensions.api.projects.ProjectApi.ListRefsRequest;
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.reviewdb.client.RefNames;
 import org.junit.Test;
@@ -155,6 +157,9 @@ public class ListBranchesIT extends AbstractDaemonTest {
     // Using regex.
     assertRefNames(ImmutableList.of("refs/heads/master"), list().withRegex(".*ast.*r").get());
     assertRefNames(ImmutableList.of(), list().withRegex(".*AST.*R").get());
+
+    // Conflicting options
+    assertBadRequest(list().withSubstring("somebranch").withRegex(".*ast.*r"));
   }
 
   private ListRefsRequest<BranchInfo> list() throws Exception {
@@ -168,4 +173,13 @@ public class ListBranchesIT extends AbstractDaemonTest {
     info.canDelete = canDelete ? true : null;
     return info;
   }
+
+  private void assertBadRequest(ListRefsRequest<BranchInfo> req) throws Exception {
+    try {
+      req.get();
+      fail("Expected BadRequestException");
+    } catch (BadRequestException e) {
+      // Expected
+    }
+  }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/TagsIT.java

@@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.rest.project;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static org.eclipse.jgit.lib.Constants.R_TAGS;
+import static org.junit.Assert.fail;
 
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
@@ -116,6 +117,9 @@ public class TagsIT extends AbstractDaemonTest {
     assertTagList(FluentIterable.from(testTags), result);
     result = getTags().withSubstring("ag-B").get();
     assertTagList(FluentIterable.from(ImmutableList.of("tag-B")), result);
+
+    // With conflicting options
+    assertBadRequest(getTags().withSubstring("ag-B").withRegex("^tag-[c|d]$"));
   }
 
   @Test
@@ -347,4 +351,13 @@ public class TagsIT extends AbstractDaemonTest {
   private TagApi tag(String tagname) throws Exception {
     return gApi.projects().name(project.get()).tag(tagname);
   }
+
+  private void assertBadRequest(ListRefsRequest<TagInfo> req) throws Exception {
+    try {
+      req.get();
+      fail("Expected BadRequestException");
+    } catch (BadRequestException e) {
+      // Expected
+    }
+  }
 }

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

@@ -56,6 +56,9 @@ public class RefFilter<T extends RefInfo> {
   }
 
   public List<T> filter(List<T> refs) throws BadRequestException {
+    if (!Strings.isNullOrEmpty(matchSubstring) && !Strings.isNullOrEmpty(matchRegex)) {
+      throw new BadRequestException("specify exactly one of m/r");
+    }
     FluentIterable<T> results = FluentIterable.from(refs);
     if (!Strings.isNullOrEmpty(matchSubstring)) {
       results = results.filter(new SubstringPredicate(matchSubstring));