Merge "Support 'conflicts:<change>' operator to find changes with same files"
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 058be89..87ea1cd 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -74,6 +74,14 @@
Either a legacy numerical 'ID' such as 15183, or a newer style
Change-Id that was scraped out of the commit message.
+[[conflicts]]
+conflicts:'ID'::
++
+Changes that potentially conflict with change 'ID' because they touch
+at least one file that was also touched by change 'ID'. Change 'ID' can
+be specified as a legacy numerical 'ID' such as 15183, or a newer style
+Change-Id that was scraped out of the commit message.
+
[[owner]]
owner:'USER'::
+
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 784b461..b24ec47 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -55,10 +55,14 @@
}
private void beforeTest(Config cfg, boolean memory) throws Exception {
- server = GerritServer.start(cfg, memory);
+ server = startServer(cfg, memory);
server.getTestInjector().injectMembers(this);
}
+ protected GerritServer startServer(Config cfg, boolean memory) throws Exception {
+ return GerritServer.start(cfg, memory);
+ }
+
private void afterTest() throws Exception {
server.stop();
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTestWithSecondaryIndex.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTestWithSecondaryIndex.java
new file mode 100644
index 0000000..7ab5911
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTestWithSecondaryIndex.java
@@ -0,0 +1,26 @@
+// Copyright (C) 2013 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.acceptance;
+
+import org.eclipse.jgit.lib.Config;
+
+public class AbstractDaemonTestWithSecondaryIndex extends AbstractDaemonTest {
+
+ @Override
+ protected GerritServer startServer(Config cfg, boolean memory)
+ throws Exception {
+ return GerritServer.start(cfg, memory, true);
+ }
+}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java
index daf9d38..6fec519 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GerritServer.java
@@ -15,10 +15,13 @@
package com.google.gerrit.acceptance;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.lucene.LuceneIndexModule;
import com.google.gerrit.pgm.Daemon;
import com.google.gerrit.pgm.Init;
+import com.google.gerrit.pgm.Reindex;
import com.google.gerrit.server.config.FactoryModule;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.index.ChangeSchemas;
import com.google.gerrit.server.util.SocketUtil;
import com.google.inject.Injector;
import com.google.inject.Key;
@@ -48,6 +51,12 @@
/** Returns fully started Gerrit server */
static GerritServer start(Config base, boolean memory) throws Exception {
+ return start(base, memory, false);
+ }
+
+ /** Returns fully started Gerrit server */
+ static GerritServer start(Config base, boolean memory, boolean index)
+ throws Exception {
final CyclicBarrier serverStarted = new CyclicBarrier(2);
final Daemon daemon = new Daemon(new Runnable() {
public void run() {
@@ -69,11 +78,25 @@
mergeTestConfig(cfg);
cfg.setBoolean("httpd", null, "requestLog", false);
cfg.setBoolean("sshd", null, "requestLog", false);
+ if (index) {
+ cfg.setString("index", null, "type", "lucene");
+ cfg.setBoolean("index", "lucene", "testInmemory", true);
+ daemon.setLuceneModule(new LuceneIndexModule(
+ ChangeSchemas.getLatest().getVersion(),
+ Runtime.getRuntime().availableProcessors(), null));
+ }
daemon.setDatabaseForTesting(ImmutableList.<Module>of(
new InMemoryTestingDatabaseModule(cfg)));
daemon.start();
} else {
- site = initSite(base);
+ Config cfg = base != null ? base : new Config();
+ if (index) {
+ cfg.setString("index", null, "type", "lucene");
+ }
+ site = initSite(cfg);
+ if (index) {
+ reindex(site);
+ }
daemonService = Executors.newSingleThreadExecutor();
daemonService.submit(new Callable<Void>() {
public Void call() throws Exception {
@@ -114,6 +137,15 @@
return tmp;
}
+ /** Runs the reindex command. Works only if the site is not currently running. */
+ private static void reindex(File site) throws Exception {
+ Reindex reindex = new Reindex();
+ int rc = reindex.main(new String[] {"-d", site.getPath()});
+ if (rc != 0) {
+ throw new RuntimeException("Reindex failed");
+ }
+ }
+
private static void mergeTestConfig(Config cfg)
throws IOException {
InetSocketAddress http = newPort();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ConflictsOperatorIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ConflictsOperatorIT.java
new file mode 100644
index 0000000..1472f38
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ConflictsOperatorIT.java
@@ -0,0 +1,152 @@
+// Copyright (C) 2013 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.acceptance.rest.change;
+
+import static com.google.gerrit.acceptance.git.GitUtil.checkout;
+import static com.google.gerrit.acceptance.git.GitUtil.cloneProject;
+import static com.google.gerrit.acceptance.git.GitUtil.initSsh;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.gerrit.acceptance.AbstractDaemonTestWithSecondaryIndex;
+import com.google.gerrit.acceptance.AccountCreator;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.RestSession;
+import com.google.gerrit.acceptance.SshSession;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.git.GitUtil;
+import com.google.gerrit.acceptance.git.PushOneCommit;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gson.Gson;
+import com.google.gson.reflect.TypeToken;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Inject;
+
+import com.jcraft.jsch.JSchException;
+
+import org.apache.http.HttpStatus;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.Set;
+
+public class ConflictsOperatorIT extends AbstractDaemonTestWithSecondaryIndex {
+
+ @Inject
+ private AccountCreator accounts;
+
+ @Inject
+ private SchemaFactory<ReviewDb> reviewDbProvider;
+
+ private TestAccount admin;
+ private RestSession session;
+ private Project.NameKey project;
+ private ReviewDb db;
+ private int count;
+
+ @Before
+ public void setUp() throws Exception {
+ admin = accounts.admin();
+ session = new RestSession(server, admin);
+ initSsh(admin);
+
+ project = new Project.NameKey("p");
+
+ db = reviewDbProvider.open();
+ }
+
+ @Test
+ public void noConflictingChanges() throws JSchException, IOException,
+ GitAPIException {
+ Git git = createProject();
+ PushOneCommit.Result change = createChange(git, true);
+ createChange(git, false);
+
+ Set<String> changes = queryConflictingChanges(change);
+ assertEquals(0, changes.size());
+ }
+
+ @Test
+ public void conflictingChanges() throws JSchException, IOException,
+ GitAPIException {
+ Git git = createProject();
+ PushOneCommit.Result change = createChange(git, true);
+ PushOneCommit.Result conflictingChange1 = createChange(git, true);
+ PushOneCommit.Result conflictingChange2 = createChange(git, true);
+ createChange(git, false);
+
+ Set<String> changes = queryConflictingChanges(change);
+ assertChanges(changes, conflictingChange1, conflictingChange2);
+ }
+
+ private Git createProject() throws JSchException, IOException,
+ GitAPIException {
+ SshSession sshSession = new SshSession(server, admin);
+ try {
+ GitUtil.createProject(sshSession, project.get(), null, true);
+ return cloneProject(sshSession.getUrl() + "/" + project.get());
+ } finally {
+ sshSession.close();
+ }
+ }
+
+ private PushOneCommit.Result createChange(Git git, boolean conflicting)
+ throws GitAPIException, IOException {
+ checkout(git, "origin/master");
+ String file = conflicting ? "test.txt" : "test-" + count + ".txt";
+ PushOneCommit push =
+ new PushOneCommit(db, admin.getIdent(), "Change " + count, file,
+ "content " + count);
+ count++;
+ return push.to(git, "refs/for/master");
+ }
+
+ private Set<String> queryConflictingChanges(PushOneCommit.Result change)
+ throws IOException {
+ RestResponse r =
+ session.get("/changes/?q=conflicts:" + change.getChangeId());
+ assertEquals(HttpStatus.SC_OK, r.getStatusCode());
+ Set<ChangeInfo> changes =
+ (new Gson()).fromJson(r.getReader(),
+ new TypeToken<Set<ChangeInfo>>() {}.getType());
+ r.consume();
+ return ImmutableSet.copyOf(Iterables.transform(changes,
+ new Function<ChangeInfo, String>() {
+ @Override
+ public String apply(ChangeInfo input) {
+ return input.id;
+ }
+ }));
+ }
+
+ private void assertChanges(Set<String> actualChanges,
+ PushOneCommit.Result... expectedChanges) {
+ assertEquals(expectedChanges.length, actualChanges.size());
+ for (PushOneCommit.Result c : expectedChanges) {
+ assertTrue(actualChanges.contains(id(c)));
+ }
+ }
+
+ private String id(PushOneCommit.Result change) {
+ return project.get() + "~master~" + change.getChangeId();
+ }
+}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
index da9f1c6..a526c59 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java
@@ -80,6 +80,7 @@
suggestions.add("commit:");
suggestions.add("comment:");
+ suggestions.add("conflicts:");
suggestions.add("project:");
suggestions.add("branch:");
suggestions.add("topic:");
@@ -89,7 +90,6 @@
suggestions.add("label:");
suggestions.add("message:");
suggestions.add("file:");
-
suggestions.add("has:");
suggestions.add("has:draft");
suggestions.add("has:star");
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
index 35d8e76..9c28135 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java
@@ -138,6 +138,7 @@
private Injector httpdInjector;
private File runFile;
private boolean test;
+ private AbstractModule luceneModule;
private Runnable serverStarted;
@@ -250,6 +251,12 @@
}
@VisibleForTesting
+ public void setLuceneModule(LuceneIndexModule m) {
+ luceneModule = m;
+ test = true;
+ }
+
+ @VisibleForTesting
public void start() {
if (dbInjector == null) {
dbInjector = createDbInjector(MULTI_USER);
@@ -303,7 +310,7 @@
AbstractModule changeIndexModule;
switch (IndexModule.getIndexType(cfgInjector)) {
case LUCENE:
- changeIndexModule = new LuceneIndexModule();
+ changeIndexModule = luceneModule != null ? luceneModule : new LuceneIndexModule();
break;
case SOLR:
changeIndexModule = new SolrIndexModule();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 9fc3dbf..85459b7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -84,6 +84,7 @@
public static final String FIELD_CHANGE = "change";
public static final String FIELD_COMMENT = "comment";
public static final String FIELD_COMMIT = "commit";
+ public static final String FIELD_CONFLICTS = "conflicts";
public static final String FIELD_DRAFTBY = "draftby";
public static final String FIELD_FILE = "file";
public static final String FIELD_IS = "is";
@@ -218,10 +219,7 @@
.parse(query));
} else if (PAT_CHANGE_ID.matcher(query).matches()) {
- if (query.charAt(0) == 'i') {
- query = "I" + query.substring(1);
- }
- return new ChangeIdPredicate(args.dbProvider, query);
+ return new ChangeIdPredicate(args.dbProvider, parseChangeId(query));
}
throw new IllegalArgumentException();
@@ -308,6 +306,14 @@
}
@Operator
+ public Predicate<ChangeData> conflicts(String value) throws OrmException,
+ QueryParseException {
+ requireIndex(FIELD_CONFLICTS, value);
+ return new ConflictsPredicate(args.dbProvider, args.patchListCache, value,
+ parseChange(value));
+ }
+
+ @Operator
public Predicate<ChangeData> project(String name) {
if (name.startsWith("^"))
return new RegexProjectPredicate(args.dbProvider, name);
@@ -665,6 +671,31 @@
return g;
}
+ private List<Change> parseChange(String value) throws OrmException,
+ QueryParseException {
+ if (PAT_LEGACY_ID.matcher(value).matches()) {
+ return Collections.singletonList(args.dbProvider.get().changes()
+ .get(Change.Id.parse(value)));
+ } else if (PAT_CHANGE_ID.matcher(value).matches()) {
+ Change.Key a = new Change.Key(parseChangeId(value));
+ List<Change> changes =
+ args.dbProvider.get().changes().byKeyRange(a, a.max()).toList();
+ if (changes.isEmpty()) {
+ throw error("Change " + value + " not found");
+ }
+ return changes;
+ }
+
+ throw error("Change " + value + " not found");
+ }
+
+ private static String parseChangeId(String value) {
+ if (value.charAt(0) == 'i') {
+ value = "I" + value.substring(1);
+ }
+ return value;
+ }
+
private Account.Id self() {
if (currentUser.isIdentifiedUser()) {
return ((IdentifiedUser) currentUser).getAccountId();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
new file mode 100644
index 0000000..9cb2d94
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2013 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.query.change;
+
+import com.google.common.collect.Lists;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.query.OrPredicate;
+import com.google.gerrit.server.query.Predicate;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Provider;
+
+import java.util.List;
+
+class ConflictsPredicate extends OrPredicate<ChangeData> {
+ private final String value;
+
+ ConflictsPredicate(Provider<ReviewDb> db, PatchListCache plc, String value,
+ List<Change> changes) throws OrmException {
+ super(predicates(db, plc, changes));
+ this.value = value;
+ }
+
+ private static List<Predicate<ChangeData>> predicates(Provider<ReviewDb> db,
+ PatchListCache plc, List<Change> changes) throws OrmException {
+ List<Predicate<ChangeData>> changePredicates =
+ Lists.newArrayListWithCapacity(changes.size());
+ for (Change c : changes) {
+ List<String> files = new ChangeData(c).currentFilePaths(db, plc);
+ List<Predicate<ChangeData>> filePredicates =
+ Lists.newArrayListWithCapacity(files.size());
+ for (String file : files) {
+ filePredicates.add(new EqualsFilePredicate(db, plc, file));
+ }
+
+ List<Predicate<ChangeData>> predicatesForOneChange =
+ Lists.newArrayListWithCapacity(2);
+ predicatesForOneChange.add(
+ not(new LegacyChangeIdPredicate(db, c.getId())));
+ predicatesForOneChange.add(or(filePredicates));
+
+ changePredicates.add(and(predicatesForOneChange));
+ }
+ return changePredicates;
+ }
+
+ @Override
+ public String toString() {
+ return ChangeQueryBuilder.FIELD_CONFLICTS + ":" + value;
+ }
+}
