create merge status cache as branch refs/pull/{issueId}/{merge,conflict}

2025-11-02 03:26:06 +01:00 · 2015-02-26 11:49:16 +09:00
parent 3057a31a6c
commit 9221bfa045
3 changed files with 337 additions and 104 deletions
--- a/src/main/scala/app/PullRequestsController.scala
+++ b/src/main/scala/app/PullRequestsController.scala
@@ -24,12 +24,12 @@ import model.{PullRequest, Issue, CommitState}
 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 @@ trait PullRequestsControllerBase extends ControllerBase {
      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 @@ trait PullRequestsControllerBase extends ControllerBase {
            // 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 @@ trait PullRequestsControllerBase extends ControllerBase {
        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 @@ trait PullRequestsControllerBase extends ControllerBase {
      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)
@@ -429,62 +395,6 @@ trait PullRequestsControllerBase extends ControllerBase {
    redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}")
  })

-  /**
-   * 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.
   *
--- a/src/main/scala/service/MergeService.scala
+++ 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))
+  }
+}
--- a/src/test/scala/service/MergeServiceSpec.scala
+++ 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"
+      }
+    }
+  }
+}