Merge branch 'stable-2.11'

* stable-2.11:
  Fix flaky test that relied on non-stable order of approvals
  Update 2.11.4 release notes
  Use uploader for approvals specified on push, not the committer
  Fix "Conflicts With" when current change is a merge commit
  Fix "Conflicts With" when other change is a merge-commit
  UploadValidationListener: Fix minor typo in javadoc
  Provide better detection of requiring sign-in
  Update replication plugin
  Submit: Fix ClassCastException for anonymous user

Note:
The testPushForMasterWithApprovalsForgeCommitterButNoForgeVote test is
broken due to refactoring that was done on the master branch since the
stable-2.11 branch was created. The breakage is not fixed in this merge,
but in the follow-up commit.

Change-Id: I52bdc55ecdf817c2d4ccf3a17e74cff6f53dfa60
This commit is contained in:
David Pursehouse
2015-10-16 14:28:05 +09:00
14 changed files with 170 additions and 122 deletions

View File

@@ -35,10 +35,10 @@ Note: This was previously fixed in Gerrit version 2.11.1, but was inadvertently
reverted in 2.11.2 and 2.11.3. reverted in 2.11.2 and 2.11.3.
* link:https://code.google.com/p/gerrit/issues/detail?id=2817[Issue 2817]: * link:https://code.google.com/p/gerrit/issues/detail?id=2817[Issue 2817]:
Insert Change-Id into access right changes. Insert `Change-Id` footer into access right changes.
+ +
When modifications of access rights were saved for review, the change When modifications of access rights were saved for review, the change
did not have a Change-Id in the commit message. did not have a `Change-Id` footer in the commit message.
* Fix duplicated log lines after reloading a plugin. * Fix duplicated log lines after reloading a plugin.
+ +
@@ -72,22 +72,17 @@ reviewers were added.
The tag name from the header was used, rather than the ref name. In some cases The tag name from the header was used, rather than the ref name. In some cases
this resulted in the wrong tag ref being listed. this resulted in the wrong tag ref being listed.
* singleusergroup plugin: enable to add a user within a project's ACL using `user/username`. * Prevent user from bypassing `ref-update` hook through gerrit-created commits.
+
A user could not be added to a project's ACL unless the user already had READ
permission in the project's ACL.
* Prevent user from bypassing ref-update hook through gerrit-created commits.
+ +
If the user used the cherry-pick ability in the UI or via the REST API, they If the user used the cherry-pick ability in the UI or via the REST API, they
could put a commit on a branch that bypassed the requirements of the ref-update could put a commit on a branch that bypassed the requirements of the `ref-update`
hook (such as that certain branches require QA-tickets to be referenced in the hook (such as that certain branches require QA-tickets to be referenced in the
commit message). commit message).
* Allow InternalUsers to see drafts. * Allow `InternalUsers` to see drafts.
+ +
According to the documentation, InternalUsers should have full read access. According to the documentation, `InternalUsers` should have full read access.
This was not true, since InternalUsers could not see drafts. This was not true, since `InternalUsers` could not see drafts.
* link:https://code.google.com/p/gerrit/issues/detail?id=2683[Issue 2683]: * link:https://code.google.com/p/gerrit/issues/detail?id=2683[Issue 2683]:
Fix non-ASCII password authentication failure under tomcat (LDAP). Fix non-ASCII password authentication failure under tomcat (LDAP).
@@ -100,12 +95,52 @@ characters such as ä, ö, Ä, and Ö.
The login URL token used to redirect from the login servlet to the target page The login URL token used to redirect from the login servlet to the target page
is already decoded and should not be decoded again. is already decoded and should not be decoded again.
* Include approvals from magic branch in change message. * link:https://code.google.com/p/gerrit/issues/detail?id=3020[Issue 3020]:
Include approvals specified on push in change message.
+ +
When using the `%l` option to apply a review label on uploaded changes or When using the `%l` option to apply a review label on uploaded changes or
patch sets, the applied label was not mentioned in the change message. patch sets, the applied label was not mentioned in the change message.
* Fire the `comment-added` hook for approvals from the magic branch. * Fire the `comment-added` hook for approvals specified on push.
+ +
When using the `%l` option to apply a review label on uploaded changes or When using the `%l` option to apply a review label on uploaded changes or
patch sets, the `comment-added` hook was not being fired. patch sets, the `comment-added` hook was not being fired.
* link:https://code.google.com/p/gerrit/issues/detail?id=3602[Issue 3602]:
Use uploader for approvals specified on push, not the committer.
+
When using the `%l` option to apply a review label on uploaded changes or
patch sets, the review label was in some cases applied as the committer rather
than the uploader.
* link:https://code.google.com/p/gerrit/issues/detail?id=3531[Issue 3531]:
Fix internal server error on unified diff screen for anonymous users.
* link:https://code.google.com/p/gerrit/issues/detail?id=2414[Issue 2414]:
Improve detection of requiring sign-in.
+
Some queries, such as the `has:*` operators, require the user to be signed in.
+
Also, when handling a REST API failure, detect 'Invalid authentication' responses
as also requiring a new session.
* link:https://code.google.com/p/gerrit/issues/detail?id=3052[Issue 3052]:
Fix 'Conflicts With' list for merge commits.
+
The 'Conflicts List' was not being populated correctly if the change being viewed
was a merge commit, or if the change being viewed conflicted with an open merge
commit.
Plugin Bugfixes
---------------
* singleusergroup: Allow to add a user to a project's ACL using `user/username`.
+
A user could not be added to a project's ACL unless the user already had READ
permission in the project's ACL.
* replication: Add waiting time and number of retries to replication log.
+
Only the replication execution time was printed in the 'replication completed'
log statement. The waiting time and retry count is added, to help debug
replication delays.

View File

@@ -16,6 +16,9 @@ package com.google.gerrit.acceptance.git;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume; import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.GitUtil.cloneProject;
import static com.google.gerrit.acceptance.GitUtil.createCommit;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.concurrent.TimeUnit.SECONDS;
@@ -23,6 +26,7 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.GitUtil.Commit;
import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.extensions.client.InheritableBoolean; import com.google.gerrit.extensions.client.InheritableBoolean;
@@ -230,6 +234,45 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
"Uploaded patch set 3."); "Uploaded patch set 3.");
} }
/**
* There was a bug that allowed a user with Forge Committer Identity access
* right to upload a commit and put *votes on behalf of another user* on it.
* This test checks that this is not possible, but that the votes that are
* specified on push are applied only on behalf of the uploader.
*
* This particular bug only occurred when there was more than one label
* defined. However to test that the votes that are specified on push are
* applied on behalf of the uploader a single label is sufficient.
*/
@Test
public void testPushForMasterWithApprovalsForgeCommitterButNoForgeVote()
throws GitAPIException, RestApiException {
// Create a commit with "User" as author and committer
Commit c = createCommit(git, user.getIdent(), PushOneCommit.SUBJECT);
// Push this commit as "Administrator" (requires Forge Committer Identity)
pushHead(git, "refs/for/master/%l=Code-Review+1", false);
// Expected Code-Review votes:
// 1. 0 from User (committer):
// When the committer is forged, the committer is automatically added as
// reviewer, hence we expect a dummy 0 vote for the committer.
// 2. +1 from Administrator (uploader):
// On push Code-Review+1 was specified, hence we expect a +1 vote from
// the uploader.
ChangeInfo ci = get(c.getChangeId());
LabelInfo cr = ci.labels.get("Code-Review");
assertThat(cr.all).hasSize(2);
int indexAdmin = admin.fullName.equals(cr.all.get(0).name) ? 0 : 1;
int indexUser = indexAdmin == 0 ? 1 : 0;
assertThat(cr.all.get(indexAdmin).name).isEqualTo(admin.fullName);
assertThat(cr.all.get(indexAdmin).value.intValue()).is(1);
assertThat(cr.all.get(indexUser).name).isEqualTo(user.fullName);
assertThat(cr.all.get(indexUser).value.intValue()).is(0);
assertThat(Iterables.getLast(ci.messages).message).isEqualTo(
"Uploaded patch set 1: Code-Review+1.");
}
@Test @Test
public void testPushNewPatchsetToRefsChanges() throws Exception { public void testPushNewPatchsetToRefsChanges() throws Exception {
PushOneCommit.Result r = pushTo("refs/for/master"); PushOneCommit.Result r = pushTo("refs/for/master");

View File

@@ -19,7 +19,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo; import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import java.util.Collections;
import java.util.List; import java.util.List;
public class PatchSetDetail { public class PatchSetDetail {
@@ -27,7 +26,6 @@ public class PatchSetDetail {
protected PatchSetInfo info; protected PatchSetInfo info;
protected List<Patch> patches; protected List<Patch> patches;
protected Project.NameKey project; protected Project.NameKey project;
protected List<UiCommandDetail> commands;
public PatchSetDetail() { public PatchSetDetail() {
} }
@@ -63,15 +61,4 @@ public class PatchSetDetail {
public void setProject(final Project.NameKey p) { public void setProject(final Project.NameKey p) {
project = p; project = p;
} }
public List<UiCommandDetail> getCommands() {
if (commands != null) {
return commands;
}
return Collections.emptyList();
}
public void setCommands(List<UiCommandDetail> cmds) {
commands = cmds.isEmpty() ? null : cmds;
}
} }

View File

@@ -1,24 +0,0 @@
// 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.common.data;
/** Detail necessary to display an action. */
public class UiCommandDetail {
public String id;
public String method;
public String label;
public String title;
public boolean enabled;
}

View File

@@ -72,7 +72,8 @@ public class RestApi {
} }
return sce.getStatusCode() == Response.SC_FORBIDDEN return sce.getStatusCode() == Response.SC_FORBIDDEN
&& (sce.getEncodedResponse().equals("Authentication required") && (sce.getEncodedResponse().equals("Authentication required")
|| sce.getEncodedResponse().startsWith("Must be signed-in")); || sce.getEncodedResponse().startsWith("Must be signed-in")
|| sce.getEncodedResponse().startsWith("Invalid authentication"));
} }
return false; return false;
} }

View File

@@ -14,16 +14,11 @@
package com.google.gerrit.httpd.rpc.changedetail; package com.google.gerrit.httpd.rpc.changedetail;
import com.google.common.base.Function;
import com.google.common.base.Optional; import com.google.common.base.Optional;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.PatchSetDetail; import com.google.gerrit.common.data.PatchSetDetail;
import com.google.gerrit.common.data.UiCommandDetail;
import com.google.gerrit.common.errors.NoSuchEntityException; import com.google.gerrit.common.errors.NoSuchEntityException;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.httpd.rpc.Handler; import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountDiffPreference; import com.google.gerrit.reviewdb.client.AccountDiffPreference;
@@ -38,12 +33,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil; import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Revisions;
import com.google.gerrit.server.edit.ChangeEdit; import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil; import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.extensions.webui.UiActions;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchList; import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache; import com.google.gerrit.server.patch.PatchListCache;
@@ -57,7 +48,6 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import com.google.inject.util.Providers;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.slf4j.Logger; import org.slf4j.Logger;
@@ -86,8 +76,6 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
private final PatchListCache patchListCache; private final PatchListCache patchListCache;
private final Provider<CurrentUser> userProvider; private final Provider<CurrentUser> userProvider;
private final ChangeControl.GenericFactory changeControlFactory; private final ChangeControl.GenericFactory changeControlFactory;
private final ChangesCollection changes;
private final Revisions revisions;
private final PatchLineCommentsUtil plcUtil; private final PatchLineCommentsUtil plcUtil;
private final ChangeEditUtil editUtil; private final ChangeEditUtil editUtil;
@@ -107,8 +95,6 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
final PatchListCache patchListCache, final PatchListCache patchListCache,
final Provider<CurrentUser> userProvider, final Provider<CurrentUser> userProvider,
final ChangeControl.GenericFactory changeControlFactory, final ChangeControl.GenericFactory changeControlFactory,
final ChangesCollection changes,
final Revisions revisions,
final PatchLineCommentsUtil plcUtil, final PatchLineCommentsUtil plcUtil,
ChangeEditUtil editUtil, ChangeEditUtil editUtil,
@Assisted("psIdBase") @Nullable final PatchSet.Id psIdBase, @Assisted("psIdBase") @Nullable final PatchSet.Id psIdBase,
@@ -119,8 +105,6 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
this.patchListCache = patchListCache; this.patchListCache = patchListCache;
this.userProvider = userProvider; this.userProvider = userProvider;
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.changes = changes;
this.revisions = revisions;
this.plcUtil = plcUtil; this.plcUtil = plcUtil;
this.editUtil = editUtil; this.editUtil = editUtil;
@@ -216,23 +200,6 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
} }
} }
detail.setCommands(Lists.newArrayList(Iterables.transform(
UiActions.sorted(UiActions.plugins(UiActions.from(
revisions,
new RevisionResource(changes.parse(control), patchSet),
Providers.of(user)))),
new Function<UiAction.Description, UiCommandDetail>() {
@Override
public UiCommandDetail apply(UiAction.Description in) {
UiCommandDetail r = new UiCommandDetail();
r.method = in.getMethod();
r.id = in.getId();
r.label = in.getLabel();
r.title = in.getTitle();
r.enabled = in.isEnabled();
return r;
}
})));
return detail; return detail;
} }

View File

@@ -222,9 +222,8 @@ public class ApprovalsUtil {
} }
public void addApprovals(ReviewDb db, ChangeUpdate update, public void addApprovals(ReviewDb db, ChangeUpdate update,
LabelTypes labelTypes, PatchSet ps, PatchSetInfo info, LabelTypes labelTypes, PatchSet ps, ChangeControl changeCtl,
ChangeControl changeCtl, Map<String, Short> approvals) Map<String, Short> approvals) throws OrmException {
throws OrmException {
if (!approvals.isEmpty()) { if (!approvals.isEmpty()) {
checkApprovals(approvals, changeCtl); checkApprovals(approvals, changeCtl);
List<PatchSetApproval> cells = new ArrayList<>(approvals.size()); List<PatchSetApproval> cells = new ArrayList<>(approvals.size());
@@ -233,7 +232,7 @@ public class ApprovalsUtil {
LabelType lt = labelTypes.byLabel(vote.getKey()); LabelType lt = labelTypes.byLabel(vote.getKey());
cells.add(new PatchSetApproval(new PatchSetApproval.Key( cells.add(new PatchSetApproval(new PatchSetApproval.Key(
ps.getId(), ps.getId(),
info.getCommitter().getAccount(), ps.getUploader(),
lt.getLabelId()), lt.getLabelId()),
vote.getValue(), vote.getValue(),
ts)); ts));

View File

@@ -278,7 +278,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
LabelTypes labelTypes = ctl.getProjectControl().getLabelTypes(); LabelTypes labelTypes = ctl.getProjectControl().getLabelTypes();
approvalsUtil.addReviewers(db, update, labelTypes, change, approvalsUtil.addReviewers(db, update, labelTypes, change,
patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet()); patchSet, patchSetInfo, reviewers, Collections.<Account.Id> emptySet());
approvalsUtil.addApprovals(db, update, labelTypes, patchSet, patchSetInfo, approvalsUtil.addApprovals(db, update, labelTypes, patchSet,
ctx.getChangeControl(), approvals); ctx.getChangeControl(), approvals);
if (message != null) { if (message != null) {
changeMessage = changeMessage =

View File

@@ -18,7 +18,6 @@ import com.google.common.base.Function;
import com.google.common.base.Predicate; import com.google.common.base.Predicate;
import com.google.common.base.Predicates; import com.google.common.base.Predicates;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
@@ -34,10 +33,6 @@ import com.google.inject.Provider;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
public class UiActions { public class UiActions {
private static final Logger log = LoggerFactory.getLogger(UiActions.class); private static final Logger log = LoggerFactory.getLogger(UiActions.class);
@@ -50,27 +45,6 @@ public class UiActions {
}; };
} }
public static List<UiAction.Description> sorted(Iterable<UiAction.Description> in) {
List<UiAction.Description> s = Lists.newArrayList(in);
Collections.sort(s, new Comparator<UiAction.Description>() {
@Override
public int compare(UiAction.Description a, UiAction.Description b) {
return a.getId().compareTo(b.getId());
}
});
return s;
}
public static Iterable<UiAction.Description> plugins(Iterable<UiAction.Description> in) {
return Iterables.filter(in,
new Predicate<UiAction.Description>() {
@Override
public boolean apply(UiAction.Description input) {
return input.getId().indexOf('~') > 0;
}
});
}
public static <R extends RestResource> Iterable<UiAction.Description> from( public static <R extends RestResource> Iterable<UiAction.Description> from(
RestCollection<?, R> collection, RestCollection<?, R> collection,
R resource, R resource,

View File

@@ -2258,7 +2258,7 @@ public class ReceiveCommits {
approvalCopier.copy(db, changeCtl, newPatchSet); approvalCopier.copy(db, changeCtl, newPatchSet);
approvalsUtil.addReviewers(db, update, labelTypes, change, newPatchSet, approvalsUtil.addReviewers(db, update, labelTypes, change, newPatchSet,
info, recipients.getReviewers(), oldRecipients.getAll()); info, recipients.getReviewers(), oldRecipients.getAll());
approvalsUtil.addApprovals(db, update, labelTypes, newPatchSet, info, approvalsUtil.addApprovals(db, update, labelTypes, newPatchSet,
changeCtl, approvals); changeCtl, approvals);
recipients.add(oldRecipients); recipients.add(oldRecipients);

View File

@@ -49,7 +49,7 @@ public interface UploadValidationListener {
* These may be RevObject or RevCommit if the processor parsed them. * These may be RevObject or RevCommit if the processor parsed them.
* Implementors should not rely on the values being parsed. * Implementors should not rely on the values being parsed.
* @throws ValidationException to block the upload and send a message * @throws ValidationException to block the upload and send a message
* back to the end-used over the client's protocol connection. * back to the end-user over the client's protocol connection.
*/ */
public void onPreUpload(Repository repository, Project project, public void onPreUpload(Repository repository, Project project,
String remoteHost, UploadPack up, Collection<? extends ObjectId> wants, String remoteHost, UploadPack up, Collection<? extends ObjectId> wants,

View File

@@ -111,6 +111,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
public static final String FIELD_HASHTAG = "hashtag"; public static final String FIELD_HASHTAG = "hashtag";
public static final String FIELD_LABEL = "label"; public static final String FIELD_LABEL = "label";
public static final String FIELD_LIMIT = "limit"; public static final String FIELD_LIMIT = "limit";
public static final String FIELD_MERGE = "merge";
public static final String FIELD_MERGEABLE = "mergeable"; public static final String FIELD_MERGEABLE = "mergeable";
public static final String FIELD_MESSAGE = "message"; public static final String FIELD_MESSAGE = "message";
public static final String FIELD_OWNER = "owner"; public static final String FIELD_OWNER = "owner";

View File

@@ -43,8 +43,12 @@ import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevFlag; import org.eclipse.jgit.revwalk.RevFlag;
import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.revwalk.filter.RevFilter;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.treewalk.filter.TreeFilter;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
@@ -65,8 +69,7 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
for (final Change c : changes) { for (final Change c : changes) {
final ChangeDataCache changeDataCache = new ChangeDataCache( final ChangeDataCache changeDataCache = new ChangeDataCache(
c, db, args.changeDataFactory, args.projectCache); c, db, args.changeDataFactory, args.projectCache);
List<String> files = args.changeDataFactory.create(db.get(), c) List<String> files = listFiles(c, args, changeDataCache);
.currentFilePaths();
List<Predicate<ChangeData>> filePredicates = List<Predicate<ChangeData>> filePredicates =
Lists.newArrayListWithCapacity(files.size()); Lists.newArrayListWithCapacity(files.size());
for (String file : files) { for (String file : files) {
@@ -82,7 +85,32 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
new ProjectPredicate(c.getProject().get())); new ProjectPredicate(c.getProject().get()));
predicatesForOneChange.add( predicatesForOneChange.add(
new RefPredicate(c.getDest().get())); new RefPredicate(c.getDest().get()));
predicatesForOneChange.add(or(filePredicates));
OperatorPredicate<ChangeData> isMerge = new OperatorPredicate<ChangeData>(
ChangeQueryBuilder.FIELD_MERGE, value) {
@Override
public boolean match(ChangeData cd) throws OrmException {
ObjectId id = ObjectId.fromString(
cd.currentPatchSet().getRevision().get());
try (Repository repo =
args.repoManager.openRepository(cd.change().getProject());
RevWalk rw = CodeReviewCommit.newRevWalk(repo)) {
RevCommit commit = rw.parseCommit(id);
return commit.getParentCount() > 1;
} catch (IOException e) {
throw new IllegalStateException(e);
}
}
@Override
public int getCost() {
return 2;
}
};
predicatesForOneChange.add(or(or(filePredicates), isMerge));
predicatesForOneChange.add(new OperatorPredicate<ChangeData>( predicatesForOneChange.add(new OperatorPredicate<ChangeData>(
ChangeQueryBuilder.FIELD_CONFLICTS, value) { ChangeQueryBuilder.FIELD_CONFLICTS, value) {
@@ -170,6 +198,42 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
return changePredicates; return changePredicates;
} }
private static List<String> listFiles(Change c, Arguments args,
ChangeDataCache changeDataCache) throws OrmException {
try (Repository repo = args.repoManager.openRepository(c.getProject());
RevWalk rw = new RevWalk(repo)) {
RevCommit ps = rw.parseCommit(changeDataCache.getTestAgainst());
if (ps.getParentCount() > 1) {
String dest = c.getDest().get();
Ref destBranch = repo.getRefDatabase().getRef(dest);
destBranch.getObjectId();
rw.setRevFilter(RevFilter.MERGE_BASE);
rw.markStart(rw.parseCommit(destBranch.getObjectId()));
rw.markStart(ps);
RevCommit base = rw.next();
// TODO(zivkov): handle the case with multiple merge bases
List<String> files = new ArrayList<>();
try (TreeWalk tw = new TreeWalk(repo)) {
if (base != null) {
tw.setFilter(TreeFilter.ANY_DIFF);
tw.addTree(base.getTree());
}
tw.addTree(ps.getTree());
tw.setRecursive(true);
while (tw.next()) {
files.add(tw.getPathString());
}
}
return files;
} else {
return args.changeDataFactory.create(args.db.get(), c).currentFilePaths();
}
} catch (IOException e) {
throw new OrmException(e);
}
}
@Override @Override
public String toString() { public String toString() {
return ChangeQueryBuilder.FIELD_CONFLICTS + ":" + value; return ChangeQueryBuilder.FIELD_CONFLICTS + ":" + value;

View File

@@ -99,7 +99,8 @@ public class QueryChanges implements RestReadView<TopLevelResource> {
out = query(); out = query();
} catch (QueryParseException e) { } catch (QueryParseException e) {
// This is a hack to detect an operator that requires authentication. // This is a hack to detect an operator that requires authentication.
Pattern p = Pattern.compile("^Error in operator (.*:self)$"); Pattern p = Pattern.compile(
"^Error in operator (.*:self|is:watched|is:owner|is:reviewer|has:.*)$");
Matcher m = p.matcher(e.getMessage()); Matcher m = p.matcher(e.getMessage());
if (m.matches()) { if (m.matches()) {
String op = m.group(1); String op = m.group(1);