diff --git a/src/main/scala/app/PullRequestsController.scala b/src/main/scala/app/PullRequestsController.scala index 1a3a2ca..a946ee4 100644 --- a/src/main/scala/app/PullRequestsController.scala +++ b/src/main/scala/app/PullRequestsController.scala @@ -24,12 +24,12 @@ class PullRequestsController extends PullRequestsControllerBase with RepositoryService with AccountService with IssuesService with PullRequestService with MilestonesService with LabelsService with CommitsService with ActivityService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator - with CommitStatusService + with CommitStatusService with MergeService trait PullRequestsControllerBase extends ControllerBase { self: RepositoryService with AccountService with IssuesService with MilestonesService with LabelsService with CommitsService with ActivityService with PullRequestService with WebHookPullRequestService with ReferrerAuthenticator with CollaboratorsAuthenticator - with CommitStatusService => + with CommitStatusService with MergeService => private val logger = LoggerFactory.getLogger(classOf[PullRequestsControllerBase]) @@ -161,7 +161,9 @@ val name = repository.name getPullRequest(owner, name, issueId) map { case(issue, pullreq) => val statuses = getCommitStatues(owner, name, pullreq.commitIdTo) - val hasConfrict = checkConflictInPullRequest(owner, name, pullreq.branch, pullreq.requestUserName, name, pullreq.requestBranch, issueId) + val hasConfrict = LockUtil.lock(s"${owner}/${name}"){ + checkConflict(owner, name, pullreq.branch, issueId) + } val hasProblem = hasConfrict || (!statuses.isEmpty && CommitState.combine(statuses.map(_.state).toSet) != CommitState.SUCCESS) pulls.html.mergeguide( hasConfrict, @@ -206,43 +208,10 @@ // record activity recordMergeActivity(owner, name, loginAccount.userName, issueId, form.message) - // merge - val mergeBaseRefName = s"refs/heads/${pullreq.branch}" - val merger = MergeStrategy.RECURSIVE.newMerger(git.getRepository, true) - val mergeBaseTip = git.getRepository.resolve(mergeBaseRefName) - val mergeTip = git.getRepository.resolve(s"refs/pull/${issueId}/head") - val conflicted = try { - !merger.merge(mergeBaseTip, mergeTip) - } catch { - case e: NoMergeBaseException => true - } - if (conflicted) { - throw new RuntimeException("This pull request can't merge automatically.") - } - - // creates merge commit - val mergeCommit = new CommitBuilder() - mergeCommit.setTreeId(merger.getResultTreeId) - mergeCommit.setParentIds(Array[ObjectId](mergeBaseTip, mergeTip): _*) - val personIdent = new PersonIdent(loginAccount.fullName, loginAccount.mailAddress) - mergeCommit.setAuthor(personIdent) - mergeCommit.setCommitter(personIdent) - mergeCommit.setMessage(s"Merge pull request #${issueId} from ${pullreq.requestUserName}/${pullreq.requestBranch}\n\n" + - form.message) - - // insertObject and got mergeCommit Object Id - val inserter = git.getRepository.newObjectInserter - val mergeCommitId = inserter.insert(mergeCommit) - inserter.flush() - inserter.release() - - // update refs - val refUpdate = git.getRepository.updateRef(mergeBaseRefName) - refUpdate.setNewObjectId(mergeCommitId) - refUpdate.setForceUpdate(false) - refUpdate.setRefLogIdent(personIdent) - refUpdate.setRefLogMessage("merged", true) - refUpdate.update() + // merge git repository + mergePullRequest(git, pullreq.branch, issueId, + s"Merge pull request #${issueId} from ${pullreq.requestUserName}/${pullreq.requestBranch}\n\n" + form.message, + new PersonIdent(loginAccount.fullName, loginAccount.mailAddress)) val (commits, _) = getRequestCompareInfo(owner, name, pullreq.commitIdFrom, pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.commitIdTo) @@ -376,9 +345,11 @@ val originBranch = JGitUtil.getDefaultBranch(oldGit, originRepository, tmpOriginBranch).get._2 val forkedBranch = JGitUtil.getDefaultBranch(newGit, forkedRepository, tmpForkedBranch).get._2 - pulls.html.mergecheck( + val conflict = LockUtil.lock(s"${originRepository.owner}/${originRepository.name}"){ checkConflict(originRepository.owner, originRepository.name, originBranch, - forkedRepository.owner, forkedRepository.name, forkedBranch)) + forkedRepository.owner, forkedRepository.name, forkedBranch) + } + pulls.html.mergecheck(conflict) } }) getOrElse NotFound }) @@ -408,12 +379,7 @@ commitIdTo = form.commitIdTo) // fetch requested branch - using(Git.open(getRepositoryDir(repository.owner, repository.name))){ git => - git.fetch - .setRemote(getRepositoryDir(form.requestUserName, form.requestRepositoryName).toURI.toString) - .setRefSpecs(new RefSpec(s"refs/heads/${form.requestBranch}:refs/pull/${issueId}/head")) - .call - } + fetchAsPullRequest(repository.owner, repository.name, form.requestUserName, form.requestRepositoryName, form.requestBranch, issueId) // record activity recordPullRequestActivity(repository.owner, repository.name, loginUserName, issueId, form.title) @@ -430,62 +396,6 @@ }) /** - * Checks whether conflict will be caused in merging. Returns true if conflict will be caused. - */ - private def checkConflict(userName: String, repositoryName: String, branch: String, - requestUserName: String, requestRepositoryName: String, requestBranch: String): Boolean = { - LockUtil.lock(s"${userName}/${repositoryName}"){ - using(Git.open(getRepositoryDir(requestUserName, requestRepositoryName))) { git => - val remoteRefName = s"refs/heads/${branch}" - val tmpRefName = s"refs/merge-check/${userName}/${branch}" - val refSpec = new RefSpec(s"${remoteRefName}:${tmpRefName}").setForceUpdate(true) - try { - // fetch objects from origin repository branch - git.fetch - .setRemote(getRepositoryDir(userName, repositoryName).toURI.toString) - .setRefSpecs(refSpec) - .call - - // merge conflict check - val merger = MergeStrategy.RECURSIVE.newMerger(git.getRepository, true) - val mergeBaseTip = git.getRepository.resolve(s"refs/heads/${requestBranch}") - val mergeTip = git.getRepository.resolve(tmpRefName) - try { - !merger.merge(mergeBaseTip, mergeTip) - } catch { - case e: NoMergeBaseException => true - } - } finally { - val refUpdate = git.getRepository.updateRef(refSpec.getDestination) - refUpdate.setForceUpdate(true) - refUpdate.delete() - } - } - } - } - - /** - * Checks whether conflict will be caused in merging within pull request. Returns true if conflict will be caused. - */ - private def checkConflictInPullRequest(userName: String, repositoryName: String, branch: String, - requestUserName: String, requestRepositoryName: String, requestBranch: String, - issueId: Int): Boolean = { - LockUtil.lock(s"${userName}/${repositoryName}") { - using(Git.open(getRepositoryDir(userName, repositoryName))) { git => - // merge - val merger = MergeStrategy.RECURSIVE.newMerger(git.getRepository, true) - val mergeBaseTip = git.getRepository.resolve(s"refs/heads/${branch}") - val mergeTip = git.getRepository.resolve(s"refs/pull/${issueId}/head") - try { - !merger.merge(mergeBaseTip, mergeTip) - } catch { - case e: NoMergeBaseException => true - } - } - } - } - - /** * Parses branch identifier and extracts owner and branch name as tuple. * * - "owner:branch" to ("owner", "branch") diff --git a/src/main/scala/service/MergeService.scala b/src/main/scala/service/MergeService.scala new file mode 100644 index 0000000..1b7d81a --- /dev/null +++ b/src/main/scala/service/MergeService.scala @@ -0,0 +1,168 @@ +package service +import util.LockUtil +import util.Directory._ +import util.Implicits._ +import util.ControlUtil._ +import org.eclipse.jgit.merge.MergeStrategy +import org.eclipse.jgit.api.Git +import org.eclipse.jgit.transport.RefSpec +import org.eclipse.jgit.errors.NoMergeBaseException +import org.eclipse.jgit.lib.{ObjectId, CommitBuilder, PersonIdent} +import model.Account +import org.eclipse.jgit.revwalk.RevWalk +trait MergeService { + import MergeService._ + /** + * Checks whether conflict will be caused in merging within pull request. + * Returns true if conflict will be caused. + */ + def checkConflict(userName: String, repositoryName: String, branch: String, issueId: Int): Boolean = { + using(Git.open(getRepositoryDir(userName, repositoryName))) { git => + MergeCacheInfo(git, branch, issueId).checkConflict() + } + } + /** + * Checks whether conflict will be caused in merging within pull request. + * only cache check. + * Returns Some(true) if conflict will be caused. + * Returns None if cache has not created yet. + */ + def checkConflictCache(userName: String, repositoryName: String, branch: String, issueId: Int): Option[Boolean] = { + using(Git.open(getRepositoryDir(userName, repositoryName))) { git => + MergeCacheInfo(git, branch, issueId).checkConflictCache() + } + } + /** merge pull request */ + def mergePullRequest(git:Git, branch: String, issueId: Int, message:String, committer:PersonIdent): Unit = { + MergeCacheInfo(git, branch, issueId).merge(message, committer) + } + /** fetch remote branch to my repository refs/pull/{issueId}/head */ + def fetchAsPullRequest(userName: String, repositoryName: String, requestUserName: String, requestRepositoryName: String, requestBranch:String, issueId:Int){ + using(Git.open(getRepositoryDir(userName, repositoryName))){ git => + git.fetch + .setRemote(getRepositoryDir(requestUserName, requestRepositoryName).toURI.toString) + .setRefSpecs(new RefSpec(s"refs/heads/${requestBranch}:refs/pull/${issueId}/head")) + .call + } + } + /** + * Checks whether conflict will be caused in merging. Returns true if conflict will be caused. + */ + def checkConflict(userName: String, repositoryName: String, branch: String, + requestUserName: String, requestRepositoryName: String, requestBranch: String): Boolean = { + using(Git.open(getRepositoryDir(requestUserName, requestRepositoryName))) { git => + val remoteRefName = s"refs/heads/${branch}" + val tmpRefName = s"refs/merge-check/${userName}/${branch}" + val refSpec = new RefSpec(s"${remoteRefName}:${tmpRefName}").setForceUpdate(true) + try { + // fetch objects from origin repository branch + git.fetch + .setRemote(getRepositoryDir(userName, repositoryName).toURI.toString) + .setRefSpecs(refSpec) + .call + // merge conflict check + val merger = MergeStrategy.RECURSIVE.newMerger(git.getRepository, true) + val mergeBaseTip = git.getRepository.resolve(s"refs/heads/${requestBranch}") + val mergeTip = git.getRepository.resolve(tmpRefName) + try { + !merger.merge(mergeBaseTip, mergeTip) + } catch { + case e: NoMergeBaseException => true + } + } finally { + val refUpdate = git.getRepository.updateRef(refSpec.getDestination) + refUpdate.setForceUpdate(true) + refUpdate.delete() + } + } + } +} +object MergeService{ + case class MergeCacheInfo(git:Git, branch:String, issueId:Int){ + val repository = git.getRepository + val mergedBranchName = s"refs/pull/${issueId}/merge" + val conflictedBranchName = s"refs/pull/${issueId}/conflict" + lazy val mergeBaseTip = repository.resolve(s"refs/heads/${branch}") + lazy val mergeTip = repository.resolve(s"refs/pull/${issueId}/head") + def checkConflictCache(): Option[Boolean] = { + Option(repository.resolve(mergedBranchName)).flatMap{ merged => + if(parseCommit( merged ).getParents().toSet == Set( mergeBaseTip, mergeTip )){ + // merged branch exists + Some(false) + }else{ + None + } + }.orElse(Option(repository.resolve(conflictedBranchName)).flatMap{ conflicted => + if(parseCommit( conflicted ).getParents().toSet == Set( mergeBaseTip, mergeTip )){ + // conflict branch exists + Some(true) + }else{ + None + } + }) + } + def checkConflict():Boolean ={ + checkConflictCache.getOrElse(checkConflictForce) + } + def checkConflictForce():Boolean ={ + val merger = MergeStrategy.RECURSIVE.newMerger(repository, true) + val conflicted = try { + !merger.merge(mergeBaseTip, mergeTip) + } catch { + case e: NoMergeBaseException => true + } + val mergeTipCommit = using(new RevWalk( repository ))(_.parseCommit( mergeTip )) + val committer = mergeTipCommit.getCommitterIdent; + def updateBranch(treeId:ObjectId, message:String, branchName:String){ + // creates merge commit + val mergeCommitId = createMergeCommit(treeId, committer, message) + // update refs + val refUpdate = repository.updateRef(branchName) + refUpdate.setNewObjectId(mergeCommitId) + refUpdate.setForceUpdate(true) + refUpdate.setRefLogIdent(committer) + refUpdate.update() + } + if(!conflicted){ + updateBranch(merger.getResultTreeId, s"Merge ${mergeTip.name} into ${mergeBaseTip.name}", mergedBranchName) + git.branchDelete().setForce(true).setBranchNames(conflictedBranchName).call() + }else{ + updateBranch(mergeTipCommit.getTree().getId(), s"can't merge ${mergeTip.name} into ${mergeBaseTip.name}", conflictedBranchName) + git.branchDelete().setForce(true).setBranchNames(mergedBranchName).call() + } + conflicted + } + // update branch from cache + def merge(message:String, committer:PersonIdent) = { + if(checkConflict()){ + throw new RuntimeException("This pull request can't merge automatically.") + } + val mergeResultCommit = parseCommit( Option(repository.resolve(mergedBranchName)).getOrElse(throw new RuntimeException(s"not found branch ${mergedBranchName}")) ) + // creates merge commit + val mergeCommitId = createMergeCommit(mergeResultCommit.getTree().getId(), committer, message) + // update refs + val refUpdate = repository.updateRef(s"refs/heads/${branch}") + refUpdate.setNewObjectId(mergeCommitId) + refUpdate.setForceUpdate(false) + refUpdate.setRefLogIdent(committer) + refUpdate.setRefLogMessage("merged", true) + refUpdate.update() + } + // return treeId + private def createMergeCommit(treeId:ObjectId, committer:PersonIdent, message:String) = { + val mergeCommit = new CommitBuilder() + mergeCommit.setTreeId(treeId) + mergeCommit.setParentIds(Array[ObjectId](mergeBaseTip, mergeTip): _*) + mergeCommit.setAuthor(committer) + mergeCommit.setCommitter(committer) + mergeCommit.setMessage(message) + // insertObject and got mergeCommit Object Id + val inserter = repository.newObjectInserter + val mergeCommitId = inserter.insert(mergeCommit) + inserter.flush() + inserter.release() + mergeCommitId + } + private def parseCommit(id:ObjectId) = using(new RevWalk( repository ))(_.parseCommit(id)) + } +} \ No newline at end of file diff --git a/src/test/scala/service/MergeServiceSpec.scala b/src/test/scala/service/MergeServiceSpec.scala new file mode 100644 index 0000000..44e30f4 --- /dev/null +++ b/src/test/scala/service/MergeServiceSpec.scala @@ -0,0 +1,155 @@ +package service +import org.specs2.mutable.Specification +import java.util.Date +import model._ +import util.JGitUtil +import util.Directory._ +import java.nio.file._ +import util.Implicits._ +import util.ControlUtil._ +import org.eclipse.jgit.api.Git +import org.eclipse.jgit.dircache.DirCache +import org.eclipse.jgit.lib._ +import org.eclipse.jgit.treewalk._ +import org.eclipse.jgit.revwalk._ +import org.apache.commons.io.FileUtils + +class MergeServiceSpec extends Specification { + sequential + val service = new MergeService{} + val branch = "master" + val issueId = 10 + def initRepository(owner:String, name:String) = { + val repo1Dir = getRepositoryDir(owner, name) + RepositoryCache.clear() + FileUtils.deleteQuietly(repo1Dir) + Files.createDirectories(repo1Dir.toPath()) + JGitUtil.initRepository(repo1Dir) + using(Git.open(repo1Dir)){ git => + createFile(git, s"refs/heads/master", "test.txt", "hoge" ) + git.branchCreate().setStartPoint(s"refs/heads/master").setName(s"refs/pull/${issueId}/head").call() + } + repo1Dir + } + def createFile(git:Git, branch:String, name:String, content:String){ + val builder = DirCache.newInCore.builder() + val inserter = git.getRepository.newObjectInserter() + val headId = git.getRepository.resolve(branch + "^{commit}") + builder.add(JGitUtil.createDirCacheEntry(name, FileMode.REGULAR_FILE, + inserter.insert(Constants.OBJ_BLOB, content.getBytes("UTF-8")))) + builder.finish() + JGitUtil.createNewCommit(git, inserter, headId, builder.getDirCache.writeTree(inserter), + branch, "dummy", "dummy@example.com", "Initial commit") + } + def getFile(git:Git, branch:String, path:String) = { + val revCommit = JGitUtil.getRevCommitFromId(git, git.getRepository.resolve(branch)) + val objectId = using(new TreeWalk(git.getRepository)){ walk => + walk.addTree(revCommit.getTree) + walk.setRecursive(true) + @scala.annotation.tailrec + def _getPathObjectId: ObjectId = walk.next match { + case true if(walk.getPathString == path) => walk.getObjectId(0) + case true => _getPathObjectId + case false => throw new Exception(s"not found ${branch} / ${path}") + } + _getPathObjectId + } + JGitUtil.getContentInfo(git, path, objectId) + } + def createConfrict(git:Git) = { + createFile(git, s"refs/heads/${branch}", "test.txt", "hoge2" ) + createFile(git, s"refs/pull/${issueId}/head", "test.txt", "hoge4" ) + } + "checkConflict, checkConflictCache" should { + "checkConflict false if not conflicted, and create cache" in { + val repo1Dir = initRepository("user1","repo1") + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual None + val conflicted = service.checkConflict("user1", "repo1", branch, issueId) + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual Some(false) + conflicted mustEqual false + } + "checkConflict true if not conflicted, and create cache" in { + val repo1Dir = initRepository("user1","repo1") + using(Git.open(repo1Dir)){ git => + createConfrict(git) + } + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual None + val conflicted = service.checkConflict("user1", "repo1", branch, issueId) + conflicted mustEqual true + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual Some(true) + } + } + "checkConflictCache" should { + "merged cache invalid if origin branch moved" in { + val repo1Dir = initRepository("user1","repo1") + service.checkConflict("user1", "repo1", branch, issueId) mustEqual false + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual Some(false) + using(Git.open(repo1Dir)){ git => + createFile(git, s"refs/heads/${branch}", "test.txt", "hoge2" ) + } + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual None + } + "merged cache invalid if request branch moved" in { + val repo1Dir = initRepository("user1","repo1") + service.checkConflict("user1", "repo1", branch, issueId) mustEqual false + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual Some(false) + using(Git.open(repo1Dir)){ git => + createFile(git, s"refs/pull/${issueId}/head", "test.txt", "hoge4" ) + } + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual None + } + "merged cache invalid if origin branch moved" in { + val repo1Dir = initRepository("user1","repo1") + service.checkConflict("user1", "repo1", branch, issueId) mustEqual false + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual Some(false) + using(Git.open(repo1Dir)){ git => + createFile(git, s"refs/heads/${branch}", "test.txt", "hoge2" ) + } + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual None + } + "conflicted cache invalid if request branch moved" in { + val repo1Dir = initRepository("user1","repo1") + using(Git.open(repo1Dir)){ git => + createConfrict(git) + } + service.checkConflict("user1", "repo1", branch, issueId) mustEqual true + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual Some(true) + using(Git.open(repo1Dir)){ git => + createFile(git, s"refs/pull/${issueId}/head", "test.txt", "hoge4" ) + } + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual None + } + "conflicted cache invalid if origin branch moved" in { + val repo1Dir = initRepository("user1","repo1") + using(Git.open(repo1Dir)){ git => + createConfrict(git) + } + service.checkConflict("user1", "repo1", branch, issueId) mustEqual true + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual Some(true) + using(Git.open(repo1Dir)){ git => + createFile(git, s"refs/heads/${branch}", "test.txt", "hoge4" ) + } + service.checkConflictCache("user1", "repo1", branch, issueId) mustEqual None + } + } + "mergePullRequest" should { + "can merge" in { + val repo1Dir = initRepository("user1","repo1") + using(Git.open(repo1Dir)){ git => + createFile(git, s"refs/pull/${issueId}/head", "test.txt", "hoge2" ) + val committer = new PersonIdent("dummy2", "dummy2@example.com") + getFile(git, branch, "test.txt").content.get mustEqual "hoge" + val requestBranchId = git.getRepository.resolve(s"refs/pull/${issueId}/head") + val masterId = git.getRepository.resolve(branch) + service.mergePullRequest(git, branch, issueId, "merged", committer) + val lastCommitId = git.getRepository.resolve(branch); + val commit = using(new RevWalk(git.getRepository))(_.parseCommit(lastCommitId)) + commit.getCommitterIdent() mustEqual committer + commit.getAuthorIdent() mustEqual committer + commit.getFullMessage() mustEqual "merged" + commit.getParents.toSet mustEqual Set( requestBranchId, masterId ) + getFile(git, branch, "test.txt").content.get mustEqual "hoge2" + } + } + } +} \ No newline at end of file