Merge changes Ibe956014,I4222b894,Iac4ea144,I803717e8,I39e97dcd, ...
* changes:
Support updating multiple VersionedMetaDatas in a BatchRefUpdate
Refactor byChange() into two methods: drafts and published
Add draft comments to PatchLineCommentsUtil
Fix bug for comments with no range
Resolve issue with naming of drafts ref
Add method to parse an AccountId out of a RefName
Improve PatchSet.Id.fromRef performance and avoid double-parsing
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetCommitIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
index 004b7cf..d4a5b3d 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
@@ -14,85 +14,147 @@
package com.google.gerrit.acceptance.rest.project;
-import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNull;
+import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.common.CommitInfo;
-import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ProjectConfig;
-import com.google.gerrit.server.project.ListBranches.BranchInfo;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import org.apache.http.HttpStatus;
+import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
-import java.io.IOException;
-
public class GetCommitIT extends AbstractDaemonTest {
@Inject
private AllProjectsName allProjects;
- @Test
- public void getCommit() throws Exception {
- RestResponse r =
- adminSession.get("/projects/" + project.get() + "/branches/"
- + IdString.fromDecoded(RefNames.REFS_CONFIG).encoded());
- assertEquals(HttpStatus.SC_OK, r.getStatusCode());
- BranchInfo branchInfo =
- newGson().fromJson(r.getReader(), BranchInfo.class);
- r.consume();
+ @Inject
+ private GitRepositoryManager repoManager;
- allow(Permission.READ, ANONYMOUS_USERS, branchInfo.ref);
+ @Inject
+ private ProjectCache projectCache;
- r = adminSession.get("/projects/" + project.get() + "/commits/"
- + branchInfo.revision);
- assertEquals(HttpStatus.SC_OK, r.getStatusCode());
- CommitInfo commitInfo =
- newGson().fromJson(r.getReader(), CommitInfo.class);
- assertEquals(branchInfo.revision, commitInfo.commit);
- assertEquals("Created project", commitInfo.subject);
- assertEquals("Created project\n", commitInfo.message);
- assertNotNull(commitInfo.author);
- assertEquals("Administrator", commitInfo.author.name);
- assertNotNull(commitInfo.committer);
- assertEquals("Gerrit Code Review", commitInfo.committer.name);
- assertTrue(commitInfo.parents.isEmpty());
- }
+ private TestRepository<Repository> repo;
- @Test
- public void getNonExistingCommit_NotFound() throws IOException {
- RestResponse r = adminSession.get("/projects/" + project.get() + "/commits/"
- + ObjectId.zeroId().name());
- assertEquals(HttpStatus.SC_NOT_FOUND, r.getStatusCode());
- }
+ @Before
+ public void setUp() throws Exception {
+ repo = new TestRepository<>(repoManager.openRepository(project));
- @Test
- public void getNonVisibleCommit_NotFound() throws Exception {
- RestResponse r =
- adminSession.get("/projects/" + project.get() + "/branches/"
- + IdString.fromDecoded(RefNames.REFS_CONFIG).encoded());
- assertEquals(HttpStatus.SC_OK, r.getStatusCode());
- BranchInfo branchInfo =
- newGson().fromJson(r.getReader(), BranchInfo.class);
- r.consume();
-
- ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
- for (AccessSection sec : cfg.getAccessSections()) {
+ ProjectConfig pc = projectCache.checkedGet(allProjects).getConfig();
+ for (AccessSection sec : pc.getAccessSections()) {
sec.removePermission(Permission.READ);
}
- saveProjectConfig(allProjects, cfg);
+ saveProjectConfig(allProjects, pc);
+ }
- r = adminSession.get("/projects/" + project.get() + "/commits/"
- + branchInfo.revision);
+ @After
+ public void tearDown() throws Exception {
+ if (repo != null) {
+ repo.getRepository().close();
+ }
+ }
+
+ @Test
+ public void getNonExistingCommit_NotFound() throws Exception {
+ assertNotFound(
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
+ }
+
+ @Test
+ public void getMergedCommit_Found() throws Exception {
+ allow(Permission.READ, REGISTERED_USERS, "refs/heads/*");
+ RevCommit commit = repo.parseBody(repo.branch("master")
+ .commit()
+ .message("Create\n\nNew commit\n")
+ .create());
+
+ CommitInfo info = getCommit(commit);
+ assertEquals(commit.name(), info.commit);
+ assertEquals("Create", info.subject);
+ assertEquals("Create\n\nNew commit\n", info.message);
+ assertEquals("J. Author", info.author.name);
+ assertEquals("jauthor@example.com", info.author.email);
+ assertEquals("J. Committer", info.committer.name);
+ assertEquals("jcommitter@example.com", info.committer.email);
+
+ CommitInfo parent = Iterables.getOnlyElement(info.parents);
+ assertEquals(commit.getParent(0).name(), parent.commit);
+ assertEquals("Initial empty repository", parent.subject);
+ assertNull(parent.message);
+ assertNull(parent.author);
+ assertNull(parent.committer);
+ }
+
+ @Test
+ public void getMergedCommit_NotFound() throws Exception {
+ RevCommit commit = repo.parseBody(repo.branch("master")
+ .commit()
+ .message("Create\n\nNew commit\n")
+ .create());
+ assertNotFound(commit);
+ }
+
+ @Test
+ public void getOpenChange_Found() throws Exception {
+ allow(Permission.READ, REGISTERED_USERS, "refs/heads/*");
+ PushOneCommit.Result r = pushFactory.create(db, admin.getIdent())
+ .to(git, "refs/for/master");
+ r.assertOkStatus();
+
+ CommitInfo info = getCommit(r.getCommitId());
+ assertEquals(r.getCommitId().name(), info.commit);
+ assertEquals("test commit", info.subject);
+ assertEquals("test commit\n\nChange-Id: " + r.getChangeId() + "\n",
+ info.message);
+ assertEquals("admin", info.author.name);
+ assertEquals("admin@example.com", info.author.email);
+ assertEquals("admin", info.committer.name);
+ assertEquals("admin@example.com", info.committer.email);
+
+ CommitInfo parent = Iterables.getOnlyElement(info.parents);
+ assertEquals(r.getCommit().getParent(0).name(), parent.commit);
+ assertEquals("Initial empty repository", parent.subject);
+ assertNull(parent.message);
+ assertNull(parent.author);
+ assertNull(parent.committer);
+ }
+
+ @Test
+ public void getOpenChange_NotFound() throws Exception {
+ PushOneCommit.Result r = pushFactory.create(db, admin.getIdent())
+ .to(git, "refs/for/master");
+ r.assertOkStatus();
+ assertNotFound(r.getCommitId());
+ }
+
+ private void assertNotFound(ObjectId id) throws Exception {
+ RestResponse r = userSession.get(
+ "/projects/" + project.get() + "/commits/" + id.name());
assertEquals(HttpStatus.SC_NOT_FOUND, r.getStatusCode());
}
+
+ private CommitInfo getCommit(ObjectId id) throws Exception {
+ RestResponse r = userSession.get(
+ "/projects/" + project.get() + "/commits/" + id.name());
+ assertEquals(HttpStatus.SC_OK, r.getStatusCode());
+ CommitInfo result = newGson().fromJson(r.getReader(), CommitInfo.class);
+ r.consume();
+ return result;
+ }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
index 923d752..bb3d2e7 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
@@ -121,6 +121,11 @@
Project.NameKey projectAwesome = new Project.NameKey("project-awesome");
createProject(sshSession, projectAwesome.get());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?p=some&r=.*").getStatusCode());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?p=some&m=some").getStatusCode());
+
RestResponse r = GET("/projects/?p=some");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
Map<String, ProjectInfo> result = toProjectInfoMap(r);
@@ -138,13 +143,22 @@
Project.NameKey projectAwesome = new Project.NameKey("project-awesome");
createProject(sshSession, projectAwesome.get());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?r=[.*some").getStatusCode());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?r=.*&p=s").getStatusCode());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?r=.*&m=s").getStatusCode());
+
RestResponse r = GET("/projects/?r=.*some");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
Map<String, ProjectInfo> result = toProjectInfoMap(r);
assertProjects(Arrays.asList(projectAwesome), result.values());
- r = GET("/projects/?r=[.*some");
- assertEquals(HttpStatus.SC_BAD_REQUEST, r.getStatusCode());
+ r = GET("/projects/?r=some-project$");
+ assertEquals(HttpStatus.SC_OK, r.getStatusCode());
+ result = toProjectInfoMap(r);
+ assertProjects(Arrays.asList(someProject), result.values());
r = GET("/projects/?r=.*");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
@@ -181,6 +195,11 @@
Project.NameKey projectAwesome = new Project.NameKey("project-awesome");
createProject(sshSession, projectAwesome.get());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?m=some&r=.*").getStatusCode());
+ assertEquals(HttpStatus.SC_BAD_REQUEST,
+ GET("/projects/?m=some&p=some").getStatusCode());
+
RestResponse r = GET("/projects/?m=some");
assertEquals(HttpStatus.SC_OK, r.getStatusCode());
Map<String, ProjectInfo> result = toProjectInfoMap(r);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/CommonConverters.java b/gerrit-server/src/main/java/com/google/gerrit/server/CommonConverters.java
new file mode 100644
index 0000000..09c2ea6
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/CommonConverters.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2014 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;
+
+import com.google.gerrit.extensions.common.GitPerson;
+
+import org.eclipse.jgit.lib.PersonIdent;
+
+import java.sql.Timestamp;
+
+/**
+ * Converters to classes in {@link com.google.gerrit.extensions.common}.
+ * <p>
+ * The server frequently needs to convert internal types to types exposed in the
+ * extension API, but the converters themselves are not part of this API. This
+ * class contains such converters as static utility methods.
+ */
+public class CommonConverters {
+ public static GitPerson toGitPerson(PersonIdent ident) {
+ GitPerson result = new GitPerson();
+ result.name = ident.getName();
+ result.email = ident.getEmailAddress();
+ result.date = new Timestamp(ident.getWhen().getTime());
+ result.tz = ident.getTimeZoneOffset();
+ return result;
+ }
+
+ private CommonConverters() {
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
index 0ccb15c..fd727c4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
@@ -21,12 +21,12 @@
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.common.CommitInfo;
-import com.google.gerrit.extensions.common.GitPerson;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetAncestor;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectControl;
@@ -39,7 +39,6 @@
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -50,7 +49,6 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
-import java.sql.Timestamp;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedList;
@@ -272,15 +270,6 @@
return r;
}
- private static GitPerson toGitPerson(PersonIdent id) {
- GitPerson p = new GitPerson();
- p.name = id.getName();
- p.email = id.getEmailAddress();
- p.date = new Timestamp(id.getWhen().getTime());
- p.tz = id.getTimeZoneOffset();
- return p;
- }
-
public static class RelatedInfo {
public List<ChangeAndCommit> changes;
}
@@ -309,7 +298,7 @@
p.commit = c.getParent(i).name();
commit.parents.add(p);
}
- commit.author = toGitPerson(c.getAuthorIdent());
+ commit.author = CommonConverters.toGitPerson(c.getAuthorIdent());
commit.subject = c.getShortMessage();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetCommit.java
index 0d07ce8..a857914 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetCommit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetCommit.java
@@ -17,6 +17,7 @@
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.GitPerson;
import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.CommonConverters;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.PersonIdent;
@@ -36,8 +37,8 @@
private static CommitInfo toCommitInfo(RevCommit commit) {
CommitInfo info = new CommitInfo();
info.commit = commit.getName();
- info.author = toGitPerson(commit.getAuthorIdent());
- info.committer = toGitPerson(commit.getCommitterIdent());
+ info.author = CommonConverters.toGitPerson(commit.getAuthorIdent());
+ info.committer = CommonConverters.toGitPerson(commit.getCommitterIdent());
info.subject = commit.getShortMessage();
info.message = commit.getFullMessage();
info.parents = new ArrayList<>(commit.getParentCount());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetReflog.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetReflog.java
index 1440200..7e52381 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetReflog.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetReflog.java
@@ -20,12 +20,12 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.args4j.TimestampHandler;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.ReflogEntry;
import org.eclipse.jgit.lib.ReflogReader;
import org.eclipse.jgit.lib.Repository;
@@ -122,14 +122,7 @@
public ReflogEntryInfo(ReflogEntry e) {
oldId = e.getOldId().getName();
newId = e.getNewId().getName();
-
- PersonIdent ident = e.getWho();
- who = new GitPerson();
- who.name = ident.getName();
- who.email = ident.getEmailAddress();
- who.date = new Timestamp(ident.getWhen().getTime());
- who.tz = ident.getTimeZoneOffset();
-
+ who = CommonConverters.toGitPerson(e.getWho());
comment = e.getComment();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
index a28ee40..8b895f2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
@@ -16,6 +16,7 @@
import com.google.common.base.Predicate;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
@@ -38,14 +39,12 @@
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.util.RegexListSearcher;
import com.google.gerrit.server.util.TreeFormatter;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import dk.brics.automaton.RegExp;
-import dk.brics.automaton.RunAutomaton;
-
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
@@ -449,8 +448,10 @@
private Iterable<Project.NameKey> scan() throws BadRequestException {
if (matchPrefix != null) {
+ checkMatchOptions(matchSubstring == null && matchRegex == null);
return projectCache.byName(matchPrefix);
} else if (matchSubstring != null) {
+ checkMatchOptions(matchPrefix == null && matchRegex == null);
return Iterables.filter(projectCache.all(),
new Predicate<Project.NameKey>() {
public boolean apply(Project.NameKey in) {
@@ -459,32 +460,31 @@
}
});
} else if (matchRegex != null) {
- if (matchRegex.startsWith("^")) {
- matchRegex = matchRegex.substring(1);
- if (matchRegex.endsWith("$") && !matchRegex.endsWith("\\$")) {
- matchRegex = matchRegex.substring(0, matchRegex.length() - 1);
- }
- }
- if (matchRegex.equals(".*")) {
- return projectCache.all();
- }
+ checkMatchOptions(matchPrefix == null && matchSubstring == null);
+ RegexListSearcher<Project.NameKey> searcher;
try {
- final RunAutomaton a =
- new RunAutomaton(new RegExp(matchRegex).toAutomaton());
- return Iterables.filter(projectCache.all(),
- new Predicate<Project.NameKey>() {
- public boolean apply(Project.NameKey in) {
- return a.run(in.get());
- }
- });
+ searcher = new RegexListSearcher<Project.NameKey>(matchRegex) {
+ @Override
+ public String apply(Project.NameKey in) {
+ return in.get();
+ }
+ };
} catch (IllegalArgumentException e) {
throw new BadRequestException(e.getMessage());
}
+ return searcher.search(ImmutableList.copyOf(projectCache.all()));
} else {
return projectCache.all();
}
}
+ private static void checkMatchOptions(boolean cond)
+ throws BadRequestException {
+ if (!cond) {
+ throw new BadRequestException("specify exactly one of p/m/r");
+ }
+ }
+
private void printProjectTree(final PrintWriter stdout,
final TreeMap<Project.NameKey, ProjectNode> treeMap) {
final SortedSet<ProjectNode> sortedNodes = new TreeSet<>();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
index d073002..3d6f8b4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
@@ -16,76 +16,21 @@
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.RegexPredicate;
+import com.google.gerrit.server.util.RegexListSearcher;
import com.google.gwtorm.server.OrmException;
-import dk.brics.automaton.Automaton;
-import dk.brics.automaton.RegExp;
-import dk.brics.automaton.RunAutomaton;
-
-import java.util.Collections;
import java.util.List;
class RegexPathPredicate extends RegexPredicate<ChangeData> {
- private final RunAutomaton pattern;
-
- private final String prefixBegin;
- private final String prefixEnd;
- private final int prefixLen;
- private final boolean prefixOnly;
-
RegexPathPredicate(String fieldName, String re) {
super(ChangeField.PATH, re);
-
- if (re.startsWith("^")) {
- re = re.substring(1);
- }
-
- if (re.endsWith("$") && !re.endsWith("\\$")) {
- re = re.substring(0, re.length() - 1);
- }
-
- Automaton automaton = new RegExp(re).toAutomaton();
- prefixBegin = automaton.getCommonPrefix();
- prefixLen = prefixBegin.length();
-
- if (0 < prefixLen) {
- char max = (char) (prefixBegin.charAt(prefixLen - 1) + 1);
- prefixEnd = prefixBegin.substring(0, prefixLen - 1) + max;
- prefixOnly = re.equals(prefixBegin + ".*");
- } else {
- prefixEnd = "";
- prefixOnly = false;
- }
-
- pattern = prefixOnly ? null : new RunAutomaton(automaton);
}
@Override
public boolean match(ChangeData object) throws OrmException {
List<String> files = object.currentFilePaths();
if (files != null) {
- int begin, end;
-
- if (0 < prefixLen) {
- begin = find(files, prefixBegin);
- end = find(files, prefixEnd);
- } else {
- begin = 0;
- end = files.size();
- }
-
- if (prefixOnly) {
- return begin < end;
- }
-
- while (begin < end) {
- if (pattern.run(files.get(begin++))) {
- return true;
- }
- }
-
- return false;
-
+ return RegexListSearcher.ofStrings(getValue()).hasMatch(files);
} else {
// The ChangeData can't do expensive lookups right now. Bypass
// them and include the result anyway. We might be able to do
@@ -95,11 +40,6 @@
}
}
- private static int find(List<String> files, String p) {
- int r = Collections.binarySearch(files, p);
- return r < 0 ? -(r + 1) : r;
- }
-
@Override
public int getCost() {
return 1;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/RegexListSearcher.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/RegexListSearcher.java
new file mode 100644
index 0000000..4b0fd35
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/RegexListSearcher.java
@@ -0,0 +1,112 @@
+// Copyright (C) 2014 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.util;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.primitives.Chars;
+
+import dk.brics.automaton.Automaton;
+import dk.brics.automaton.RegExp;
+import dk.brics.automaton.RunAutomaton;
+
+import java.util.Collections;
+import java.util.List;
+
+/** Helper to search sorted lists for elements matching a regex. */
+public abstract class RegexListSearcher<T> implements Function<T, String> {
+ public static RegexListSearcher<String> ofStrings(String re) {
+ return new RegexListSearcher<String>(re) {
+ @Override
+ public String apply(String in) {
+ return in;
+ }
+ };
+ }
+
+ private final RunAutomaton pattern;
+
+ private final String prefixBegin;
+ private final String prefixEnd;
+ private final int prefixLen;
+ private final boolean prefixOnly;
+
+ public RegexListSearcher(String re) {
+ if (re.startsWith("^")) {
+ re = re.substring(1);
+ }
+
+ if (re.endsWith("$") && !re.endsWith("\\$")) {
+ re = re.substring(0, re.length() - 1);
+ }
+
+ Automaton automaton = new RegExp(re).toAutomaton();
+ prefixBegin = automaton.getCommonPrefix();
+ prefixLen = prefixBegin.length();
+
+ if (0 < prefixLen) {
+ char max = Chars.checkedCast(prefixBegin.charAt(prefixLen - 1) + 1);
+ prefixEnd = prefixBegin.substring(0, prefixLen - 1) + max;
+ prefixOnly = re.equals(prefixBegin + ".*");
+ } else {
+ prefixEnd = "";
+ prefixOnly = false;
+ }
+
+ pattern = prefixOnly ? null : new RunAutomaton(automaton);
+ }
+
+ public Iterable<T> search(List<T> list) {
+ checkNotNull(list);
+ int begin, end;
+
+ if (0 < prefixLen) {
+ // Assumes many consecutive elements may have the same prefix, so the cost
+ // of two binary searches is less than iterating to find the endpoints.
+ begin = find(list, prefixBegin);
+ end = find(list, prefixEnd);
+ } else {
+ begin = 0;
+ end = list.size();
+ }
+
+ if (prefixOnly) {
+ return begin < end ? list.subList(begin, end) : ImmutableList.<T> of();
+ }
+
+ return Iterables.filter(
+ list.subList(begin, end),
+ new Predicate<T>() {
+ @Override
+ public boolean apply(T in) {
+ return pattern.run(RegexListSearcher.this.apply(in));
+ }
+ });
+ }
+
+ public boolean hasMatch(List<T> list) {
+ return !Iterables.isEmpty(search(list));
+ }
+
+ private int find(List<T> list, String p) {
+ int r = Collections.binarySearch(Lists.transform(list, this), p);
+ return r < 0 ? -(r + 1) : r;
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/util/RegexListSearcherTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/util/RegexListSearcherTest.java
new file mode 100644
index 0000000..8f73005
--- /dev/null
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/util/RegexListSearcherTest.java
@@ -0,0 +1,84 @@
+// Copyright (C) 2014 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.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Ordering;
+
+import org.junit.Test;
+
+import java.util.List;
+
+public class RegexListSearcherTest {
+ private static final List<String> EMPTY = ImmutableList.of();
+
+ @Test
+ public void emptyList() {
+ assertSearchReturns(EMPTY, "pat", EMPTY);
+ }
+
+ @Test
+ public void hasMatch() {
+ List<String> list = ImmutableList.of("bar", "foo", "quux");
+ assertTrue(RegexListSearcher.ofStrings("foo").hasMatch(list));
+ assertFalse(RegexListSearcher.ofStrings("xyz").hasMatch(list));
+ }
+
+ @Test
+ public void anchors() {
+ List<String> list = ImmutableList.of("foo");
+ assertSearchReturns(list, "^f.*", list);
+ assertSearchReturns(list, "^f.*o$", list);
+ assertSearchReturns(list, "f.*o$", list);
+ assertSearchReturns(list, "f.*o$", list);
+ assertSearchReturns(EMPTY, "^.*\\$", list);
+ }
+
+ @Test
+ public void noCommonPrefix() {
+ List<String> list = ImmutableList.of("bar", "foo", "quux");
+ assertSearchReturns(ImmutableList.of("foo"), "f.*", list);
+ assertSearchReturns(ImmutableList.of("foo"), ".*o.*", list);
+ assertSearchReturns(ImmutableList.of("bar", "foo", "quux"), ".*[aou].*",
+ list);
+ }
+
+ @Test
+ public void commonPrefix() {
+ List<String> list = ImmutableList.of(
+ "bar",
+ "baz",
+ "foo1",
+ "foo2",
+ "foo3",
+ "quux");
+ assertSearchReturns(ImmutableList.of("bar", "baz"), "b.*", list);
+ assertSearchReturns(ImmutableList.of("foo1", "foo2"), "foo[12]", list);
+ assertSearchReturns(ImmutableList.of("foo1", "foo2", "foo3"), "foo.*",
+ list);
+ assertSearchReturns(ImmutableList.of("quux"), "q.*", list);
+ }
+
+ private void assertSearchReturns(List<?> expected, String re,
+ List<String> inputs) {
+ assertTrue(Ordering.natural().isOrdered(inputs));
+ assertEquals(expected,
+ ImmutableList.copyOf(RegexListSearcher.ofStrings(re).search(inputs)));
+ }
+}
