opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.10'

Browse Source 
* stable-2.10:
  Fix IncludingGroupMembership.containsAnyOf infinite loop
  Fix using HTTP methods other than GET from plugins
  Pass ChangeControl into CodeReviewCommit error instances
  Avoid CodeReviewCommit.error() whenever possible
  Update replication plugin to latest revision

Conflicts:
	gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java

Change-Id: I9c59567bd54a493a5e262e7776df00f1b776b553
This commit is contained in:
Shawn Pearce 2014-10-16 21:52:13 -07:00
commit 79b7aa185e
4 changed files with 43 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
gerrit-server/src/main/java/com/google/gerrit/server/account/IncludingGroupMembership.java
View File

@ -16,7 +16,6 @@ package com.google.gerrit.server.account;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.IdentifiedUser;
@ -26,6 +25,7 @@ import com.google.inject.assistedinject.Assisted;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
/**
* Group membership checker for the internal group system.
@ -53,7 +53,7 @@ public class IncludingGroupMembership implements GroupMembership {
this.user = user;
Set<AccountGroup.UUID> groups = user.state().getInternalGroups();
memberOf = Maps.newHashMapWithExpectedSize(groups.size());
memberOf = new ConcurrentHashMap<>(groups.size());
for (AccountGroup.UUID g : groups) {
memberOf.put(g, true);
}

26
gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java
View File

@ -37,8 +37,30 @@ public class CodeReviewCommit extends RevCommit {
}
};
}
static CodeReviewCommit error(final CommitMergeStatus s) {
final CodeReviewCommit r = new CodeReviewCommit(ObjectId.zeroId());
static CodeReviewCommit revisionGone(ChangeControl ctl) {
return error(ctl, CommitMergeStatus.REVISION_GONE);
}
static CodeReviewCommit noPatchSet(ChangeControl ctl) {
return error(ctl, CommitMergeStatus.NO_PATCH_SET);
}
/**
* Create an error commit.
* <p>
* Should only be used for error statuses such that there is no possible
* non-zero commit on which we could call {@link
* #setStatusCode(CommitMergeStatus)}, enumerated in the methods above.
*
* @param ctl control for change that caused this error
* @param CommitMergeStatus status
* @return new commit instance
*/
private static CodeReviewCommit error(ChangeControl ctl,
CommitMergeStatus s) {
CodeReviewCommit r = new CodeReviewCommit(ObjectId.zeroId());
r.setControl(ctl);
r.statusCode = s;
return r;
}

37
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@ -461,10 +461,16 @@ public class MergeOp {
int commitOrder = 0;
for (final Change chg : submitted) {
ChangeControl ctl;
try {
ctl = changeControlFactory.controlFor(chg,
identifiedUserFactory.create(chg.getOwner()));
} catch (NoSuchChangeException e) {
throw new MergeException("Failed to validate changes", e);
}
final Change.Id changeId = chg.getId();
if (chg.currentPatchSetId() == null) {
commits.put(changeId, CodeReviewCommit
.error(CommitMergeStatus.NO_PATCH_SET));
commits.put(changeId, CodeReviewCommit.noPatchSet(ctl));
toUpdate.add(chg);
continue;
}
@ -477,8 +483,7 @@ public class MergeOp {
}
if (ps == null || ps.getRevision() == null
|| ps.getRevision().get() == null) {
commits.put(changeId, CodeReviewCommit
.error(CommitMergeStatus.NO_PATCH_SET));
commits.put(changeId, CodeReviewCommit.noPatchSet(ctl));
toUpdate.add(chg);
continue;
}
@ -488,8 +493,7 @@ public class MergeOp {
try {
id = ObjectId.fromString(idstr);
} catch (IllegalArgumentException iae) {
commits.put(changeId, CodeReviewCommit
.error(CommitMergeStatus.NO_PATCH_SET));
commits.put(changeId, CodeReviewCommit.noPatchSet(ctl));
toUpdate.add(chg);
continue;
}
@ -504,8 +508,7 @@ public class MergeOp {
// want to merge the issue. We can't safely do that if the
// tip is not reachable.
//
commits.put(changeId, CodeReviewCommit
.error(CommitMergeStatus.REVISION_GONE));
commits.put(changeId, CodeReviewCommit.revisionGone(ctl));
toUpdate.add(chg);
continue;
}
@ -515,32 +518,25 @@ public class MergeOp {
commit = (CodeReviewCommit) rw.parseCommit(id);
} catch (IOException e) {
log.error("Invalid commit " + id.name() + " on " + chg.getKey(), e);
commits.put(changeId, CodeReviewCommit
.error(CommitMergeStatus.REVISION_GONE));
commits.put(changeId, CodeReviewCommit.revisionGone(ctl));
toUpdate.add(chg);
continue;
}
try {
commit.setControl(changeControlFactory.controlFor(chg,
identifiedUserFactory.create(chg.getOwner())));
} catch (NoSuchChangeException e) {
throw new MergeException("Failed to validate changes", e);
}
commit.setControl(ctl);
commit.setPatchsetId(ps.getId());
commit.originalOrder = commitOrder++;
commits.put(changeId, commit);
MergeValidators mergeValidators = mergeValidatorsFactory.create();
try {
mergeValidators.validatePreMerge(repo, commit, destProject, destBranch, ps.getId());
} catch (MergeValidationException mve) {
commits.put(changeId, CodeReviewCommit.error(mve.getStatus()));
commit.setStatusCode(mve.getStatus());
toUpdate.add(chg);
continue;
}
commits.put(changeId, commit);
if (branchTip != null) {
// If this commit is already merged its a bug in the queuing code
// that we got back here. Just mark it complete and move on. It's
@ -563,8 +559,7 @@ public class MergeOp {
SubmitType submitType = getSubmitType(commit.getControl(), ps);
if (submitType == null) {
commits.put(changeId,
CodeReviewCommit.error(CommitMergeStatus.NO_SUBMIT_TYPE));
commit.setStatusCode(CommitMergeStatus.NO_SUBMIT_TYPE);
toUpdate.add(chg);
continue;
}

2
plugins/replication

@ -1 +1 @@
Subproject commit 319c5905d60bb13f3a62196f05a959ea016ac9b8
Subproject commit 211e9289cc5771293855845d31bc0ce756545851