diff options
author | David Pursehouse <david.pursehouse@sonymobile.com> | 2012-08-08 17:47:40 +0900 |
---|---|---|
committer | Edwin Kempin <edwin.kempin@sap.com> | 2012-10-25 20:58:51 +0200 |
commit | 470cc60a682f0f4567172c01f42a05cae55ed8ff (patch) | |
tree | 7ab820d86a90daf736d41ad14245c9dd2552f30e | |
parent | b8adb7f36f24b6ecea84a7f72e8af21d142e4d94 (diff) |
NPE when strategy is cherry-pick and changeMerge.test enabled
Commit 31050f21 adds a new patch set when a change is submitted
with the cherry-pick strategy. The uploader of the new patch
set is set to the ID of one of the change's approvers.
However this causes NPE if the change does not have any
approvers, which can be the case when the method is called
during mergeability testing.
Only attempt to create the new patch set if an approver was
found.
Bug: issue 1626
Change-Id: Icd0269b1a181bd89c5233a1cd044f1b5efbf1e22
-rw-r--r-- | gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java | 97 |
1 files changed, 50 insertions, 47 deletions
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index 26fd2ea945..cec284173f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -936,56 +936,59 @@ public class MergeOp { final ObjectId id = commit(mergeCommit); final CodeReviewCommit newCommit = (CodeReviewCommit) rw.parseCommit(id); - final Change oldChange = n.change; - - n.change = - db.changes().atomicUpdate(n.change.getId(), - new AtomicUpdate<Change>() { - @Override - public Change update(Change change) { - change.nextPatchSetId(); - return change; - } - }); - - final PatchSet ps = new PatchSet(n.change.currPatchSetId()); - ps.setCreatedOn(new Timestamp(System.currentTimeMillis())); - ps.setUploader(submitAudit.getAccountId()); - ps.setRevision(new RevId(id.getName())); - insertAncestors(ps.getId(), newCommit); - db.patchSets().insert(Collections.singleton(ps)); - - n.change = - db.changes().atomicUpdate(n.change.getId(), - new AtomicUpdate<Change>() { - @Override - public Change update(Change change) { - change.setCurrentPatchSet(patchSetInfoFactory.get(newCommit, - ps.getId())); - return change; - } - }); - - this.submitted.remove(oldChange); - this.submitted.add(n.change); - - if (approvalList != null) { - for (PatchSetApproval a : approvalList) { - db.patchSetApprovals().insert( - Collections.singleton(new PatchSetApproval(ps.getId(), a))); + + if (submitAudit != null) { + final Change oldChange = n.change; + + n.change = + db.changes().atomicUpdate(n.change.getId(), + new AtomicUpdate<Change>() { + @Override + public Change update(Change change) { + change.nextPatchSetId(); + return change; + } + }); + + final PatchSet ps = new PatchSet(n.change.currPatchSetId()); + ps.setCreatedOn(new Timestamp(System.currentTimeMillis())); + ps.setUploader(submitAudit.getAccountId()); + ps.setRevision(new RevId(id.getName())); + insertAncestors(ps.getId(), newCommit); + db.patchSets().insert(Collections.singleton(ps)); + + n.change = + db.changes().atomicUpdate(n.change.getId(), + new AtomicUpdate<Change>() { + @Override + public Change update(Change change) { + change.setCurrentPatchSet(patchSetInfoFactory.get(newCommit, + ps.getId())); + return change; + } + }); + + this.submitted.remove(oldChange); + this.submitted.add(n.change); + + if (approvalList != null) { + for (PatchSetApproval a : approvalList) { + db.patchSetApprovals().insert( + Collections.singleton(new PatchSetApproval(ps.getId(), a))); + } } - } - final RefUpdate ru = repo.updateRef(ps.getRefName()); - ru.setExpectedOldObjectId(ObjectId.zeroId()); - ru.setNewObjectId(newCommit); - ru.disableRefLog(); - if (ru.update(rw) != RefUpdate.Result.NEW) { - throw new IOException(String.format( - "Failed to create ref %s in %s: %s", ps.getRefName(), - n.change.getDest().getParentKey().get(), ru.getResult())); + final RefUpdate ru = repo.updateRef(ps.getRefName()); + ru.setExpectedOldObjectId(ObjectId.zeroId()); + ru.setNewObjectId(newCommit); + ru.disableRefLog(); + if (ru.update(rw) != RefUpdate.Result.NEW) { + throw new IOException(String.format( + "Failed to create ref %s in %s: %s", ps.getRefName(), n.change + .getDest().getParentKey().get(), ru.getResult())); + } + replication.fire(n.change.getProject(), ru.getName()); } - replication.fire(n.change.getProject(), ru.getName()); newCommit.copyFrom(n); newCommit.statusCode = CommitMergeStatus.CLEAN_PICK; |