opendev/gerrit
Code Issues Proposed changes

Pass ChangeControl into CodeReviewCommit error instances

Browse Source 
Within MergeOp these are keyed by Change.Id, but some error handling
code only receives the CodeReviewCommit and not the ID. That code is
reasonable in assuming there is a change/notes/control available, so
ensure this is the case when creating error instances.

Bug: issue 2911
Change-Id: If65398652f0e5b1a2b10a2fadc9a3b74bce7d556
This commit is contained in:
Dave Borowitz 2014-09-10 11:23:22 +02:00 committed by David Pursehouse
parent 002b9ee7b3
commit 5766bc02d8
2 changed files with 21 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -27,12 +27,12 @@ import java.util.List;
/** Extended commit entity with code review specific metadata. */
public class CodeReviewCommit extends RevCommit {
static CodeReviewCommit revisionGone() {
return error(CommitMergeStatus.REVISION_GONE);
static CodeReviewCommit revisionGone(ChangeControl ctl) {
return error(ctl, CommitMergeStatus.REVISION_GONE);
}
static CodeReviewCommit noPatchSet() {
return error(CommitMergeStatus.NO_PATCH_SET);
static CodeReviewCommit noPatchSet(ChangeControl ctl) {
return error(ctl, CommitMergeStatus.NO_PATCH_SET);
}
/**
@ -42,11 +42,14 @@ public class CodeReviewCommit extends RevCommit {
* 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(CommitMergeStatus s) {
private static CodeReviewCommit error(ChangeControl ctl,
CommitMergeStatus s) {
CodeReviewCommit r = new CodeReviewCommit(ObjectId.zeroId());
r.setControl(ctl);
r.statusCode = s;
return r;
}

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

@ -460,9 +460,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.noPatchSet());
commits.put(changeId, CodeReviewCommit.noPatchSet(ctl));
toUpdate.add(chg);
continue;
}
@ -475,7 +482,7 @@ public class MergeOp {
}
if (ps == null || ps.getRevision() == null
|| ps.getRevision().get() == null) {
commits.put(changeId, CodeReviewCommit.noPatchSet());
commits.put(changeId, CodeReviewCommit.noPatchSet(ctl));
toUpdate.add(chg);
continue;
}
@ -485,7 +492,7 @@ public class MergeOp {
try {
id = ObjectId.fromString(idstr);
} catch (IllegalArgumentException iae) {
commits.put(changeId, CodeReviewCommit.noPatchSet());
commits.put(changeId, CodeReviewCommit.noPatchSet(ctl));
toUpdate.add(chg);
continue;
}
@ -500,7 +507,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.revisionGone());
commits.put(changeId, CodeReviewCommit.revisionGone(ctl));
toUpdate.add(chg);
continue;
}
@ -510,17 +517,12 @@ 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.revisionGone());
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);