Merge "For the 'conflicts' operator return only changes that actually conflict"

2013-11-15 08:33:08 +00:00
parent cb48541b99 86e8362313
commit 8610d49e2c
9 changed files with 387 additions and 18 deletions
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -77,10 +77,9 @@ 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.
+Changes that conflict with 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'::
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -111,6 +111,7 @@ import com.google.gerrit.server.project.ProjectNode;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.SectionSortCache;
 import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.gerrit.server.query.change.ConflictsCacheImpl;
 import com.google.gerrit.server.ssh.SshAddressesModule;
 import com.google.gerrit.server.tools.ToolsCatalog;
 import com.google.gerrit.server.util.IdGenerator;
@@ -143,13 +144,14 @@ public class GerritGlobalModule extends FactoryModule {
    install(authModule);
    install(AccountByEmailCacheImpl.module());
    install(AccountCacheImpl.module());
+    install(ChangeCache.module());
+    install(ConflictsCacheImpl.module());
    install(GroupCacheImpl.module());
    install(GroupIncludeCacheImpl.module());
    install(PatchListCacheImpl.module());
    install(ProjectCacheImpl.module());
    install(SectionSortCache.module());
    install(TagCache.module());
-    install(ChangeCache.module());

    install(new AccessControlModule());
    install(new CmdLineParserModule());
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/BasicChangeRewrites.java
@@ -33,8 +33,8 @@ public abstract class BasicChangeRewrites extends QueryRewriter<ChangeData> {
      new ChangeQueryBuilder.Arguments( //
          new InvalidProvider<ReviewDb>(), //
          new InvalidProvider<ChangeQueryRewriter>(), //
-          null, null, null, null, null, null, //
-          null, null, null, null, null, null), null);
+          null, null, null, null, null, null, null, //
+          null, null, null, null, null, null, null), null);

  static Schema<ChangeData> schema(@Nullable IndexCollection indexes) {
    ChangeIndex index = indexes != null ? indexes.getSearchIndex() : null;
--- 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
@@ -32,6 +32,7 @@ import com.google.gerrit.server.account.GroupBackend;
 import com.google.gerrit.server.account.GroupBackends;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.SubmitStrategyFactory;
 import com.google.gerrit.server.index.ChangeIndex;
 import com.google.gerrit.server.index.IndexCollection;
 import com.google.gerrit.server.index.Schema;
@@ -153,6 +154,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
    final ProjectCache projectCache;
    final Provider<ListChildProjects> listChildProjects;
    final IndexCollection indexes;
+    final SubmitStrategyFactory submitStrategyFactory;
+    final ConflictsCache conflictsCache;

    @Inject
    @VisibleForTesting
@@ -169,7 +172,9 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
        GitRepositoryManager repoManager,
        ProjectCache projectCache,
        Provider<ListChildProjects> listChildProjects,
-        IndexCollection indexes) {
+        IndexCollection indexes,
+        SubmitStrategyFactory submitStrategyFactory,
+        ConflictsCache conflictsCache) {
      this.dbProvider = dbProvider;
      this.rewriter = rewriter;
      this.userFactory = userFactory;
@@ -184,6 +189,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
      this.projectCache = projectCache;
      this.listChildProjects = listChildProjects;
      this.indexes = indexes;
+      this.submitStrategyFactory = submitStrategyFactory;
+      this.conflictsCache = conflictsCache;
    }
  }

@@ -317,8 +324,10 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
  public Predicate<ChangeData> conflicts(String value) throws OrmException,
      QueryParseException {
    requireIndex(FIELD_CONFLICTS, value);
-    return new ConflictsPredicate(args.dbProvider, args.patchListCache, value,
-        parseChange(value));
+    return new ConflictsPredicate(args.dbProvider, args.patchListCache,
+        args.submitStrategyFactory, args.changeControlGenericFactory,
+        args.userFactory, args.repoManager, args.projectCache,
+        args.conflictsCache, value, parseChange(value));
  }

  @Operator
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictKey.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictKey.java
@@ -0,0 +1,78 @@
+// 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.base.Objects;
+import com.google.gerrit.reviewdb.client.Project.SubmitType;
+
+import org.eclipse.jgit.lib.ObjectId;
+
+import java.io.Serializable;
+
+public class ConflictKey implements Serializable {
+  private static final long serialVersionUID = 1L;
+
+  private final ObjectId commit;
+  private final ObjectId otherCommit;
+  private final SubmitType submitType;
+  private final boolean contentMerge;
+
+  public ConflictKey(ObjectId commit, ObjectId otherCommit,
+      SubmitType submitType, boolean contentMerge) {
+    if (SubmitType.FAST_FORWARD_ONLY.equals(submitType)
+        || commit.compareTo(otherCommit) < 0) {
+      this.commit = commit;
+      this.otherCommit = otherCommit;
+    } else {
+      this.commit = otherCommit;
+      this.otherCommit = commit;
+    }
+    this.submitType = submitType;
+    this.contentMerge = contentMerge;
+  }
+
+  public ObjectId getCommit() {
+    return commit;
+  }
+
+  public ObjectId getOtherCommit() {
+    return otherCommit;
+  }
+
+  public SubmitType getSubmitType() {
+    return submitType;
+  }
+
+  public boolean isContentMerge() {
+    return contentMerge;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (!(o instanceof ConflictKey)) {
+      return false;
+    }
+    ConflictKey other = (ConflictKey)o;
+    return commit.equals(other.commit)
+        && otherCommit.equals(other.otherCommit)
+        && submitType.equals(other.submitType)
+        && contentMerge == other.contentMerge;
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hashCode(commit, otherCommit, submitType, contentMerge);
+  }
+}
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsCache.java
@@ -0,0 +1,25 @@
+// 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.gerrit.common.Nullable;
+
+public interface ConflictsCache {
+
+  public void put(ConflictKey key, Boolean value);
+
+  @Nullable
+  public Boolean getIfPresent(ConflictKey key);
+}
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsCacheImpl.java
@@ -0,0 +1,56 @@
+// 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.cache.Cache;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Singleton;
+import com.google.inject.name.Named;
+
+@Singleton
+public class ConflictsCacheImpl implements ConflictsCache {
+  public final static String NAME = "conflicts";
+
+  public static Module module() {
+    return new CacheModule() {
+      @Override
+      protected void configure() {
+        persist(NAME, ConflictKey.class, Boolean.class)
+            .maximumWeight(37400);
+        bind(ConflictsCache.class).to(ConflictsCacheImpl.class);
+      }
+    };
+  }
+
+  private final Cache<ConflictKey, Boolean> conflictsCache;
+
+  @Inject
+  public ConflictsCacheImpl(
+      @Named(NAME) Cache<ConflictKey, Boolean> conflictsCache) {
+    this.conflictsCache = conflictsCache;
+  }
+
+  @Override
+  public void put(ConflictKey key, Boolean value) {
+    conflictsCache.put(key, value);
+  }
+
+  @Override
+  public Boolean getIfPresent(ConflictKey key) {
+    return conflictsCache.getIfPresent(key);
+  }
+}
--- 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
@@ -15,30 +15,71 @@
 package com.google.gerrit.server.query.change;

 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import com.google.gerrit.common.data.SubmitTypeRecord;
 import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project.SubmitType;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.git.CodeReviewCommit;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MergeException;
+import com.google.gerrit.server.git.SubmitStrategy;
+import com.google.gerrit.server.git.SubmitStrategyFactory;
 import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.query.OperatorPredicate;
 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 org.eclipse.jgit.errors.IncorrectObjectTypeException;
+import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevFlag;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+import java.io.IOException;
 import java.util.List;
+import java.util.Set;

 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));
+  ConflictsPredicate(Provider<ReviewDb> db, PatchListCache plc,
+      SubmitStrategyFactory submitStrategyFactory,
+      ChangeControl.GenericFactory changeControlFactory,
+      IdentifiedUser.GenericFactory identifiedUserFactory,
+      GitRepositoryManager repoManager, ProjectCache projectCache,
+      ConflictsCache conflictsCache, String value, List<Change> changes)
+      throws OrmException {
+    super(predicates(db, plc, submitStrategyFactory, changeControlFactory,
+        identifiedUserFactory, repoManager, projectCache, conflictsCache,
+        value, changes));
    this.value = value;
  }

-  private static List<Predicate<ChangeData>> predicates(Provider<ReviewDb> db,
-      PatchListCache plc, List<Change> changes) throws OrmException {
+  private static List<Predicate<ChangeData>> predicates(
+      final Provider<ReviewDb> db, final PatchListCache plc,
+      final SubmitStrategyFactory submitStrategyFactory,
+      final ChangeControl.GenericFactory changeControlFactory,
+      final IdentifiedUser.GenericFactory identifiedUserFactory,
+      final GitRepositoryManager repoManager, final ProjectCache projectCache,
+      final ConflictsCache conflictsCache, final String value,
+      List<Change> changes) throws OrmException {
    List<Predicate<ChangeData>> changePredicates =
        Lists.newArrayListWithCapacity(changes.size());
-    for (Change c : changes) {
+    for (final Change c : changes) {
+      final ChangeDataCache changeDataCache = new ChangeDataCache(c, db, projectCache);
      List<String> files = new ChangeData(c).currentFilePaths(db, plc);
      List<Predicate<ChangeData>> filePredicates =
          Lists.newArrayListWithCapacity(files.size());
@@ -47,7 +88,7 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
      }

      List<Predicate<ChangeData>> predicatesForOneChange =
-          Lists.newArrayListWithCapacity(4);
+          Lists.newArrayListWithCapacity(5);
      predicatesForOneChange.add(
          not(new LegacyChangeIdPredicate(db, c.getId())));
      predicatesForOneChange.add(
@@ -55,7 +96,115 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
      predicatesForOneChange.add(
          new RefPredicate(db, c.getDest().get()));
      predicatesForOneChange.add(or(filePredicates));
+      predicatesForOneChange.add(new OperatorPredicate<ChangeData>(
+          ChangeQueryBuilder.FIELD_CONFLICTS, value) {

+        @Override
+        public boolean match(ChangeData object) throws OrmException {
+          Change otherChange = object.change(db);
+          if (otherChange == null) {
+            return false;
+          }
+          if (!otherChange.getDest().equals(c.getDest())) {
+            return false;
+          }
+          SubmitType submitType = getSubmitType(otherChange, object);
+          if (submitType == null) {
+            return false;
+          }
+          ObjectId other = ObjectId.fromString(
+              object.currentPatchSet(db).getRevision().get());
+          ConflictKey conflictsKey =
+              new ConflictKey(changeDataCache.getTestAgainst(), other, submitType,
+                  changeDataCache.getProjectState().isUseContentMerge());
+          Boolean conflicts = conflictsCache.getIfPresent(conflictsKey);
+          if (conflicts != null) {
+            return conflicts;
+          }
+          try {
+            Repository repo =
+                repoManager.openRepository(otherChange.getProject());
+            try {
+              RevWalk rw = new RevWalk(repo) {
+                @Override
+                protected RevCommit createCommit(AnyObjectId id) {
+                  return new CodeReviewCommit(id);
+                }
+              };
+              try {
+                RevFlag canMergeFlag = rw.newFlag("CAN_MERGE");
+                CodeReviewCommit commit =
+                    (CodeReviewCommit) rw.parseCommit(changeDataCache.getTestAgainst());
+                SubmitStrategy strategy =
+                    submitStrategyFactory.create(submitType,
+                        db.get(), repo, rw, null, canMergeFlag,
+                        getAlreadyAccepted(repo, rw, commit),
+                        otherChange.getDest());
+                CodeReviewCommit otherCommit =
+                    (CodeReviewCommit) rw.parseCommit(other);
+                otherCommit.add(canMergeFlag);
+                conflicts = !strategy.dryRun(commit, otherCommit);
+                conflictsCache.put(conflictsKey, conflicts);
+                return conflicts;
+              } catch (MergeException e) {
+                throw new IllegalStateException(e);
+              } catch (NoSuchProjectException e) {
+                throw new IllegalStateException(e);
+              } finally {
+                rw.release();
+              }
+            } finally {
+              repo.close();
+            }
+          } catch (IOException e) {
+            throw new IllegalStateException(e);
+          }
+        }
+
+        @Override
+        public int getCost() {
+          return 5;
+        }
+
+        private SubmitType getSubmitType(Change change, ChangeData cd) throws OrmException {
+          try {
+            final SubmitTypeRecord r =
+                changeControlFactory.controlFor(change,
+                    identifiedUserFactory.create(change.getOwner()))
+                    .getSubmitTypeRecord(db.get(), cd.currentPatchSet(db), cd);
+            if (r.status != SubmitTypeRecord.Status.OK) {
+              return null;
+            }
+            return r.type;
+          } catch (NoSuchChangeException e) {
+            return null;
+          }
+        }
+
+        private Set<RevCommit> getAlreadyAccepted(Repository repo, RevWalk rw,
+            CodeReviewCommit tip) throws MergeException {
+          Set<RevCommit> alreadyAccepted = Sets.newHashSet();
+
+          if (tip != null) {
+            alreadyAccepted.add(tip);
+          }
+
+          try {
+            for (ObjectId id : changeDataCache.getAlreadyAccepted(repo)) {
+              try {
+                alreadyAccepted.add(rw.parseCommit(id));
+              } catch (IncorrectObjectTypeException iote) {
+                // Not a commit? Skip over it.
+              }
+            }
+          } catch (IOException e) {
+            throw new MergeException(
+                "Failed to determine already accepted commits.", e);
+          }
+
+          return alreadyAccepted;
+        }
+      });
      changePredicates.add(and(predicatesForOneChange));
    }
    return changePredicates;
@@ -65,4 +214,55 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
  public String toString() {
    return ChangeQueryBuilder.FIELD_CONFLICTS + ":" + value;
  }
+
+  private static class ChangeDataCache {
+    private final Change change;
+    private final Provider<ReviewDb> db;
+    private final ProjectCache projectCache;
+
+    private ObjectId testAgainst;
+    private ProjectState projectState;
+    private Set<ObjectId> alreadyAccepted;
+
+    ChangeDataCache(Change change, Provider<ReviewDb> db, ProjectCache projectCache) {
+      this.change = change;
+      this.db = db;
+      this.projectCache = projectCache;
+    }
+
+    ObjectId getTestAgainst()
+        throws OrmException {
+      if (testAgainst == null) {
+        testAgainst = ObjectId.fromString(
+          new ChangeData(change).currentPatchSet(db).getRevision().get());
+      }
+      return testAgainst;
+    }
+
+    ProjectState getProjectState() {
+      if (projectState == null) {
+        projectState = projectCache.get(change.getProject());
+        if (projectState == null) {
+          throw new IllegalStateException(
+              new NoSuchProjectException(change.getProject()));
+        }
+      }
+      return projectState;
+    }
+
+    Set<ObjectId> getAlreadyAccepted(Repository repo) {
+      if (alreadyAccepted == null) {
+        alreadyAccepted = Sets.newHashSet();
+        for (Ref r : repo.getAllRefs().values()) {
+          if (r.getName().startsWith(Constants.R_HEADS)
+              || r.getName().startsWith(Constants.R_TAGS)) {
+            if (r.getObjectId() != null) {
+              alreadyAccepted.add(r.getObjectId());
+            }
+          }
+        }
+      }
+      return alreadyAccepted;
+    }
+  }
 }
--- a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java
@@ -26,7 +26,7 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
        new FakeQueryBuilder.Definition<ChangeData, FakeQueryBuilder>(
          FakeQueryBuilder.class),
        new ChangeQueryBuilder.Arguments(null, null, null, null, null, null,
-          null, null, null, null, null, null, null, indexes),
+          null, null, null, null, null, null, null, indexes, null, null),
        null);
  }