Merge "For the 'conflicts' operator return only changes that actually conflict"
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index cb3a3b6..85c0711 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -77,10 +77,9 @@
[[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'::
diff --git 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
index 2b9cb2e..87f5271 100644
--- 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.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 @@
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());
diff --git 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
index dd96d50..d1d2cae 100644
--- 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 @@
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;
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 37890a5..86c0d38 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
@@ -32,6 +32,7 @@
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 @@
final ProjectCache projectCache;
final Provider<ListChildProjects> listChildProjects;
final IndexCollection indexes;
+ final SubmitStrategyFactory submitStrategyFactory;
+ final ConflictsCache conflictsCache;
@Inject
@VisibleForTesting
@@ -169,7 +172,9 @@
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 @@
this.projectCache = projectCache;
this.listChildProjects = listChildProjects;
this.indexes = indexes;
+ this.submitStrategyFactory = submitStrategyFactory;
+ this.conflictsCache = conflictsCache;
}
}
@@ -317,8 +324,10 @@
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
diff --git 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
new file mode 100644
index 0000000..e177e37
--- /dev/null
+++ 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);
+ }
+}
diff --git 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
new file mode 100644
index 0000000..bf7a5dd
--- /dev/null
+++ 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);
+}
diff --git 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
new file mode 100644
index 0000000..1b3473e
--- /dev/null
+++ 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);
+ }
+}
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
index e398bb3..cd94930 100644
--- 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 @@
}
List<Predicate<ChangeData>> predicatesForOneChange =
- Lists.newArrayListWithCapacity(4);
+ Lists.newArrayListWithCapacity(5);
predicatesForOneChange.add(
not(new LegacyChangeIdPredicate(db, c.getId())));
predicatesForOneChange.add(
@@ -55,7 +96,115 @@
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 @@
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;
+ }
+ }
}
diff --git 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
index 77633c8..c94186c 100644
--- 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 @@
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);
}
