Merge "Support 'conflicts:<change>' operator to find changes with same files"

Documentation/user-search.txt

@@ -74,6 +74,14 @@ change:'ID'::
 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'::
 +

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -55,10 +55,14 @@ public abstract class AbstractDaemonTest {
   }
 
   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();
   }

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);
+  }
+}

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 @@ public class GerritServer {
 
   /** 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 @@ public class GerritServer {
       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 @@ public class GerritServer {
     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();

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();
+  }
+}

gerrit-gwtui/src/main/java/com/google/gerrit/client/SearchSuggestOracle.java

@@ -80,6 +80,7 @@ public class SearchSuggestOracle extends HighlightSuggestOracle {
 
     suggestions.add("commit:");
     suggestions.add("comment:");
+    suggestions.add("conflicts:");
     suggestions.add("project:");
     suggestions.add("branch:");
     suggestions.add("topic:");
@@ -89,7 +90,6 @@ public class SearchSuggestOracle extends HighlightSuggestOracle {
     suggestions.add("label:");
     suggestions.add("message:");
     suggestions.add("file:");
-
     suggestions.add("has:");
     suggestions.add("has:draft");
     suggestions.add("has:star");

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java

@@ -138,6 +138,7 @@ public class Daemon extends SiteProgram {
   private Injector httpdInjector;
   private File runFile;
   private boolean test;
+  private AbstractModule luceneModule;
 
   private Runnable serverStarted;
 
@@ -249,6 +250,12 @@ public class Daemon extends SiteProgram {
     headless = true;
   }
 
+  @VisibleForTesting
+  public void setLuceneModule(LuceneIndexModule m) {
+    luceneModule = m;
+    test = true;
+  }
+
   @VisibleForTesting
   public void start() {
     if (dbInjector == null) {
@@ -303,7 +310,7 @@ public class Daemon extends SiteProgram {
     AbstractModule changeIndexModule;
     switch (IndexModule.getIndexType(cfgInjector)) {
       case LUCENE:
-        changeIndexModule = new LuceneIndexModule();
+        changeIndexModule = luceneModule != null ? luceneModule : new LuceneIndexModule();
         break;
       case SOLR:
         changeIndexModule = new SolrIndexModule();

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java

@@ -84,6 +84,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
   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 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
           .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();
@@ -307,6 +305,14 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
         .fromString(id));
   }
 
+  @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("^"))
@@ -665,6 +671,31 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
     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();

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;
+  }
+}