Skip to content
This repository has been archived by the owner on Sep 12, 2018. It is now read-only.

Commit

Permalink
PullRequest: Fix server error when conflict occurs
Browse files Browse the repository at this point in the history 
Reason: NPE occurred when a merge conflicts because Yobi tries to get
the number of the merged commits even if the list is null.

Solution: Set the list of commits Yobi tries to merge even if the merge
conflicts.
  • Loading branch information
Yi EungJun committed Nov 27, 2014
1 parent 6aff39c commit 34b8706
Showing 1 changed file with 70 additions and 32 deletions.
102 changes: 70 additions & 32 deletions app/models/PullRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ public Merger(String leftRef, String rightRef) throws IOException {
this.rightRef = Objects.requireNonNull(rightRef);
}

public CommitCreation createTree() throws IOException {
public MergeResult merge() throws IOException {
merger = MergeStrategy.RECURSIVE.newMerger(getRepository(), true);
String refNotExistMessageFormat = "Ref '%s' does not exist in Git repository '%s'";
ObjectId leftParent = Objects.requireNonNull(getRepository().resolve(leftRef),
Expand All @@ -385,26 +385,46 @@ public CommitCreation createTree() throws IOException {
String.format(refNotExistMessageFormat, rightRef, getRepository()));

if (merger.merge(leftParent, rightParent)) {
return new CommitCreation(merger.getResultTreeId(), leftParent, rightParent);
return new Success(merger.getResultTreeId(), leftParent, rightParent);
} else {
return null;
return new Conflict(leftParent, rightParent);
}
}

public class CommitCreation {
public class Conflict extends MergeResult {
private Conflict(ObjectId leftParent, ObjectId rightParent) {
this.leftParent = Objects.requireNonNull(leftParent);
this.rightParent = Objects.requireNonNull(rightParent);
}

@Override
public MergeRefUpdate createCommit() throws IOException, GitAPIException {
throw new UnsupportedOperationException();
}

@Override
public MergeRefUpdate createCommit(PersonIdent whoMerges) throws IOException, GitAPIException {
throw new UnsupportedOperationException();
}

@Nullable
@Override
public ObjectId getMergeCommitId() {
throw new UnsupportedOperationException();
}
}

public class Success extends MergeResult {
private ObjectId mergeCommitId;
private ObjectId treeId;
private ObjectId leftParent;
private ObjectId rightParent;
protected ObjectId treeId;

private CommitCreation(
private Success(
ObjectId treeId, ObjectId leftParent, ObjectId rightParent) {
this.treeId = Objects.requireNonNull(treeId);
this.leftParent = Objects.requireNonNull(leftParent);
this.rightParent = Objects.requireNonNull(rightParent);
}

public MergeRefUpdate createCommit() throws IOException, GitAPIException {
public MergeRefUpdate createCommit() throws IOException, GitAPIException {
return createCommit(new PersonIdent(utils.Config.getSiteName(),
utils.Config.getSystemEmailAddress()));
}
Expand Down Expand Up @@ -459,6 +479,19 @@ public MergeRefUpdate createCommit(PersonIdent whoMerges) throws IOException,
public ObjectId getMergeCommitId() {
return mergeCommitId;
}
}

public abstract class MergeResult {
protected ObjectId leftParent;
protected ObjectId rightParent;

abstract public MergeRefUpdate createCommit() throws IOException, GitAPIException;

abstract public MergeRefUpdate createCommit(PersonIdent whoMerges) throws IOException,
GitAPIException;

@Nullable
abstract public ObjectId getMergeCommitId();

public ObjectId getLeftParentId() {
return leftParent;
Expand All @@ -467,6 +500,10 @@ public ObjectId getLeftParentId() {
public ObjectId getRightParentId() {
return rightParent;
}

boolean conflicts() {
return this instanceof Conflict;
}
}

public class MergeRefUpdate {
Expand Down Expand Up @@ -507,16 +544,16 @@ public void updateRef(String ref) throws
}

public void merge(final PullRequestEventMessage message) throws IOException, GitAPIException, PullRequestException {
Merger.CommitCreation mergeCommitCreation =
new Merger(toBranch, fetchSourceBranch()).createTree();
Merger.MergeResult result =
new Merger(toBranch, fetchSourceBranch()).merge();

if (mergeCommitCreation != null) {
if (!result.conflicts()) {
User sender = message.getSender();
mergeCommitCreation.createCommit(new PersonIdent(sender.name,
result.createCommit(new PersonIdent(sender.name,
sender.email)).updateRef(toBranch);

// Update the pull request
updateMergedCommitId(mergeCommitCreation);
updateMergedCommitId(result);
changeState(State.MERGED, sender);

// Add event
Expand All @@ -533,7 +570,7 @@ public String fetchSourceBranch() throws IOException, GitAPIException {
return destination;
}

public void updateMergedCommitId(Merger.CommitCreation merger) {
public void updateMergedCommitId(Merger.MergeResult merger) {
mergedCommitIdFrom = merger.getLeftParentId().getName();
mergedCommitIdTo = merger.getMergeCommitId().getName();
update();
Expand Down Expand Up @@ -807,30 +844,30 @@ public PullRequestMergeResult updateMerge() throws IOException, GitAPIException,
}

// merge
Merger.CommitCreation mergeCommitCreation = new Merger(toBranch,
fetchSourceBranch()).createTree();
Merger.MergeResult mergeResult = new Merger(toBranch,
fetchSourceBranch()).merge();

// Make a PullRequestMergeResult to return
PullRequestMergeResult pullRequestMergeResult = new PullRequestMergeResult();
pullRequestMergeResult.setPullRequest(this);

if (mergeCommitCreation == null) {
if (mergeResult instanceof Merger.Conflict) {
pullRequestMergeResult.setConflictStateOfPullRequest();
} else {
// Commit and update the ref to merge commit of this pullrequest
mergeCommitCreation.createCommit().updateRef(getNameOfRefToMerged());
mergeResult.createCommit().updateRef(getNameOfRefToMerged());

List<GitCommit> commits = GitRepository.diffCommits(
getRepository(),
mergeCommitCreation.getLeftParentId(),
mergeCommitCreation.getRightParentId());
pullRequestMergeResult.setGitCommits(commits);
pullRequestMergeResult.setResolvedStateOfPullRequest();

// Update the pullrequest
updateMergedCommitId(mergeCommitCreation);
updateMergedCommitId(mergeResult);
}

pullRequestMergeResult.setGitCommits(GitRepository.diffCommits(
getRepository(),
mergeResult.getLeftParentId(),
mergeResult.getRightParentId()));

return pullRequestMergeResult;
}

Expand All @@ -856,20 +893,21 @@ public PullRequestMergeResult attemptMerge() throws IOException, GitAPIException
String tempBranchToCheckConflict = fetchSourceTemporarilly();

// merge
Merger.CommitCreation commit = new Merger(toBranch,
tempBranchToCheckConflict).createTree();
Merger.MergeResult mergeResult = new Merger(toBranch,
tempBranchToCheckConflict).merge();

// Make a PullRequestMergeResult to return
PullRequestMergeResult pullRequestMergeResult = new PullRequestMergeResult();
pullRequestMergeResult.setPullRequest(this);
if (commit == null) {
if (mergeResult.conflicts()) {
pullRequestMergeResult.setConflictStateOfPullRequest();
} else {
List<GitCommit> commits = GitRepository.diffCommits(
getRepository(), commit.getLeftParentId(), commit.getRightParentId());
pullRequestMergeResult.setGitCommits(commits);
pullRequestMergeResult.setResolvedStateOfPullRequest();
}
pullRequestMergeResult.setGitCommits(GitRepository.diffCommits(
getRepository(),
mergeResult.getLeftParentId(),
mergeResult.getRightParentId()));

// Clean Up: Delete the temporary branch
RefUpdate refUpdate = getRepository().updateRef(tempBranchToCheckConflict);
Expand Down

0 comments on commit 34b8706

Please sign in to comment.