diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index 9c23927..35ae255 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -1,7 +1,7 @@ package gitbucket.core.controller import gitbucket.core.api._ -import gitbucket.core.model.{Account, CommitState, Repository, PullRequest, Issue} +import gitbucket.core.model.{Account, CommitStatus, CommitState, Repository, PullRequest, Issue, WebHook} import gitbucket.core.pulls.html import gitbucket.core.service.CommitStatusService import gitbucket.core.service.MergeService @@ -119,7 +119,8 @@ commits, diffs, hasWritePermission(owner, name, context.loginAccount), - repository) + repository, + flash.toMap.map(f => f._1 -> f._2.toString)) } } } getOrElse NotFound @@ -166,26 +167,31 @@ } getOrElse NotFound }) - ajaxGet("/:owner/:repository/pull/:id/mergeguide")(collaboratorsOnly { repository => + ajaxGet("/:owner/:repository/pull/:id/mergeguide")(referrersOnly { repository => params("id").toIntOpt.flatMap{ issueId => val owner = repository.owner val name = repository.name getPullRequest(owner, name, issueId) map { case(issue, pullreq) => - val branchProtection = getProtectedBranchInfo(owner, name, pullreq.branch) - val statuses = branchProtection.withRequireStatues(getCommitStatues(owner, name, pullreq.commitIdTo)) - val hasConfrict = LockUtil.lock(s"${owner}/${name}"){ + val hasConflict = LockUtil.lock(s"${owner}/${name}"){ checkConflict(owner, name, pullreq.branch, issueId) } - val hasRequiredStatusProblem = branchProtection.hasProblem(statuses, pullreq.commitIdTo, context.loginAccount.get.userName) - val hasProblem = hasRequiredStatusProblem || hasConfrict || (!statuses.isEmpty && CommitState.combine(statuses.map(_.state).toSet) != CommitState.SUCCESS) + val hasMergePermission = hasWritePermission(owner, name, context.loginAccount) + val branchProtection = getProtectedBranchInfo(owner, name, pullreq.branch) + val state = PullRequestsController.MergeStatus( + hasConflict = hasConflict, + commitStatues = getCommitStatues(owner, name, pullreq.commitIdTo), + branchProtection = branchProtection, + branchIsOutOfDate = JGitUtil.getShaByRef(owner, name, pullreq.branch) != Some(pullreq.commitIdFrom), + needStatusCheck = branchProtection.needStatusCheck(context.loginAccount.map(_.userName)), + hasUpdatePermission = hasWritePermission(pullreq.requestUserName, pullreq.requestRepositoryName, context.loginAccount) && + !getProtectedBranchInfo(pullreq.requestUserName, pullreq.requestRepositoryName, pullreq.requestBranch) + .needStatusCheck(context.loginAccount.map(_.userName)), + hasMergePermission = hasMergePermission, + commitIdTo = pullreq.commitIdTo) html.mergeguide( - hasConfrict, - hasProblem, + state, issue, pullreq, - statuses, - branchProtection, - hasRequiredStatusProblem, repository, getRepository(pullreq.requestUserName, pullreq.requestRepositoryName, context.baseUrl).get) } @@ -207,6 +213,75 @@ } getOrElse NotFound }) + post("/:owner/:repository/pull/:id/update_branch")(referrersOnly { baseRepository => + (for { + issueId <- params("id").toIntOpt + loginAccount <- context.loginAccount + (issue, pullreq) <- getPullRequest(baseRepository.owner, baseRepository.name, issueId) + owner = pullreq.requestUserName + name = pullreq.requestRepositoryName + if hasWritePermission(owner, name, context.loginAccount) + } yield { + val branchProtection = getProtectedBranchInfo(owner, name, pullreq.requestBranch) + if(branchProtection.needStatusCheck(loginAccount.userName)){ + flash += "error" -> s"branch ${pullreq.requestBranch} is protected need status check." + } else { + val repository = getRepository(owner, name, context.baseUrl).get + LockUtil.lock(s"${owner}/${name}"){ + val alias = if(pullreq.repositoryName == pullreq.requestRepositoryName && pullreq.userName == pullreq.requestUserName){ + pullreq.branch + }else{ + s"${pullreq.userName}:${pullreq.branch}" + } + val existIds = using(Git.open(Directory.getRepositoryDir(owner, name))) { git => JGitUtil.getAllCommitIds(git) }.toSet + pullRemote(owner, name, pullreq.requestBranch, pullreq.userName, pullreq.repositoryName, pullreq.branch, loginAccount, + "Merge branch '${alias}' into ${pullreq.requestBranch}") match { + case None => // conflict + flash += "error" -> s"Can't automatic merging branch '${alias}' into ${pullreq.requestBranch}." + case Some(oldId) => + // update pull request + updatePullRequests(owner, name, pullreq.requestBranch) + + using(Git.open(Directory.getRepositoryDir(owner, name))) { git => + // after update branch + + val newCommitId = git.getRepository.resolve(s"refs/heads/${pullreq.requestBranch}") + val commits = git.log.addRange(oldId, newCommitId).call.iterator.asScala.map(c => new JGitUtil.CommitInfo(c)).toList + + commits.foreach{ commit => + if(!existIds.contains(commit.id)){ + createIssueComment(owner, name, commit) + } + } + + // record activity + recordPushActivity(owner, name, loginAccount.userName, pullreq.branch, commits) + + // close issue by commit message + if(pullreq.requestBranch == repository.repository.defaultBranch){ + commits.map{ commit => + closeIssuesFromMessage(commit.fullMessage, loginAccount.userName, owner, name) + } + } + + // call web hook + callPullRequestWebHookByRequestBranch("synchronize", repository, pullreq.requestBranch, baseUrl, loginAccount) + callWebHookOf(owner, name, WebHook.Push) { + for { + ownerAccount <- getAccountByUserName(owner) + } yield { + WebHookService.WebHookPushPayload(git, loginAccount, pullreq.requestBranch, repository, commits, ownerAccount, oldId = oldId, newId = newCommitId) + } + } + } + flash += "info" -> s"Merge branch '${alias}' into ${pullreq.requestBranch}" + } + } + redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}") + } + }) getOrElse NotFound + }) + post("/:owner/:repository/pull/:id/merge", mergeForm)(collaboratorsOnly { (form, repository) => params("id").toIntOpt.flatMap { issueId => val owner = repository.owner @@ -532,4 +607,43 @@ repository, hasWritePermission(owner, repoName, context.loginAccount)) } + + // TODO: same as gitbucket.core.servlet.CommitLogHook ... + private def createIssueComment(owner: String, repository: String, commit: CommitInfo) = { + StringUtil.extractIssueId(commit.fullMessage).foreach { issueId => + if(getIssue(owner, repository, issueId).isDefined){ + getAccountByMailAddress(commit.committerEmailAddress).foreach { account => + createComment(owner, repository, account.userName, issueId.toInt, commit.fullMessage + " " + commit.id, "commit") + } + } + } + } +} + +object PullRequestsController { + case class MergeStatus( + hasConflict: Boolean, + commitStatues:List[CommitStatus], + branchProtection: ProtectedBrancheService.ProtectedBranchInfo, + branchIsOutOfDate: Boolean, + hasUpdatePermission: Boolean, + needStatusCheck: Boolean, + hasMergePermission: Boolean, + commitIdTo: String){ + + val statuses: List[CommitStatus] = + commitStatues ++ (branchProtection.contexts.toSet -- commitStatues.map(_.context).toSet).map(branchProtection.pendingCommitStatus(_)) + val hasRequiredStatusProblem = needStatusCheck && branchProtection.contexts.exists(context => statuses.find(_.context == context).map(_.state) != Some(CommitState.SUCCESS)) + val hasProblem = hasRequiredStatusProblem || hasConflict || (!statuses.isEmpty && CommitState.combine(statuses.map(_.state).toSet) != CommitState.SUCCESS) + val canUpdate = branchIsOutOfDate && !hasConflict + val canMerge = hasMergePermission && !hasConflict && !hasRequiredStatusProblem + lazy val commitStateSummary:(CommitState, String) = { + val stateMap = statuses.groupBy(_.state) + val state = CommitState.combine(stateMap.keySet) + val summary = stateMap.map{ case (keyState, states) => states.size+" "+keyState.name }.mkString(", ") + state -> summary + } + lazy val statusesAndRequired:List[(CommitStatus, Boolean)] = statuses.map{ s => s -> branchProtection.contexts.exists(_==s.context) } + lazy val isAllSuccess = commitStateSummary._1==CommitState.SUCCESS + } } diff --git a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala index 045adab..30eb01d 100644 --- a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala +++ b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala @@ -14,7 +14,6 @@ import gitbucket.core.util.Implicits._ import gitbucket.core.util.Directory._ import gitbucket.core.model.{Account, CommitState, WebHook} -import gitbucket.core.service.{CommitStatusService, ProtectedBrancheService} import gitbucket.core.service.WebHookService._ import gitbucket.core.view import gitbucket.core.view.helpers diff --git a/src/main/scala/gitbucket/core/service/MergeService.scala b/src/main/scala/gitbucket/core/service/MergeService.scala index 6614fd4..f09211c 100644 --- a/src/main/scala/gitbucket/core/service/MergeService.scala +++ b/src/main/scala/gitbucket/core/service/MergeService.scala @@ -10,10 +10,9 @@ 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 org.eclipse.jgit.lib.{ObjectId, CommitBuilder, PersonIdent, Repository} import org.eclipse.jgit.revwalk.RevWalk - trait MergeService { import MergeService._ /** @@ -52,26 +51,30 @@ /** * 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}" + def tryMergeRemote(localUserName: String, localRepositoryName: String, localBranch: String, + remoteUserName: String, remoteRepositoryName: String, remoteBranch: String): Option[(ObjectId, ObjectId, ObjectId)] = { + using(Git.open(getRepositoryDir(localUserName, localRepositoryName))) { git => + val remoteRefName = s"refs/heads/${remoteBranch}" + val tmpRefName = s"refs/remote-temp/${remoteUserName}/${remoteRepositoryName}/${remoteBranch}" val refSpec = new RefSpec(s"${remoteRefName}:${tmpRefName}").setForceUpdate(true) try { // fetch objects from origin repository branch git.fetch - .setRemote(getRepositoryDir(userName, repositoryName).toURI.toString) + .setRemote(getRepositoryDir(remoteUserName, remoteRepositoryName).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 mergeBaseTip = git.getRepository.resolve(s"refs/heads/${localBranch}") val mergeTip = git.getRepository.resolve(tmpRefName) try { - !merger.merge(mergeBaseTip, mergeTip) + if(merger.merge(mergeBaseTip, mergeTip)){ + Some((merger.getResultTreeId, mergeBaseTip, mergeTip)) + }else{ + None + } } catch { - case e: NoMergeBaseException => true + case e: NoMergeBaseException => None } } finally { val refUpdate = git.getRepository.updateRef(refSpec.getDestination) @@ -80,8 +83,54 @@ } } } + /** + * 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 = + tryMergeRemote(userName, repositoryName, branch, requestUserName, requestRepositoryName, requestBranch).isEmpty + + def pullRemote(localUserName: String, localRepositoryName: String, localBranch: String, + remoteUserName: String, remoteRepositoryName: String, remoteBranch: String, + loginAccount: Account, message: String): Option[ObjectId] = { + tryMergeRemote(localUserName, localRepositoryName, localBranch, remoteUserName, remoteRepositoryName, remoteBranch).map{ case (newTreeId, oldBaseId, oldHeadId) => + using(Git.open(getRepositoryDir(localUserName, localRepositoryName))) { git => + val committer = new PersonIdent(loginAccount.fullName, loginAccount.mailAddress) + val newCommit = Util.createMergeCommit(git.getRepository, newTreeId, committer, message, Seq(oldBaseId, oldHeadId)) + Util.updateRefs(git.getRepository, s"refs/heads/${localBranch}", newCommit, false, committer, Some("merge")) + } + oldBaseId + } + } + } object MergeService{ + object Util{ + // return treeId + def createMergeCommit(repository: Repository, treeId: ObjectId, committer: PersonIdent, message: String, parents: Seq[ObjectId]): ObjectId = { + val mergeCommit = new CommitBuilder() + mergeCommit.setTreeId(treeId) + mergeCommit.setParentIds(parents:_*) + 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.close() + mergeCommitId + } + def updateRefs(repository: Repository, ref: String, newObjectId: ObjectId, force: Boolean, committer: PersonIdent, refLogMessage: Option[String] = None):Unit = { + // update refs + val refUpdate = repository.updateRef(ref) + refUpdate.setNewObjectId(newObjectId) + refUpdate.setForceUpdate(force) + refUpdate.setRefLogIdent(committer) + refLogMessage.map(refUpdate.setRefLogMessage(_, true)) + refUpdate.update() + } + } case class MergeCacheInfo(git:Git, branch:String, issueId:Int){ val repository = git.getRepository val mergedBranchName = s"refs/pull/${issueId}/merge" @@ -120,12 +169,7 @@ 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() + Util.updateRefs(repository, branchName, mergeCommitId, true, committer) } if(!conflicted){ updateBranch(merger.getResultTreeId, s"Merge ${mergeTip.name} into ${mergeBaseTip.name}", mergedBranchName) @@ -145,28 +189,12 @@ // 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() + Util.updateRefs(repository, s"refs/heads/${branch}", mergeCommitId, false, committer, Some("merged")) } // 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.close() - mergeCommitId - } + private def createMergeCommit(treeId: ObjectId, committer: PersonIdent, message: String) = + Util.createMergeCommit(repository, treeId, committer, message, Seq[ObjectId](mergeBaseTip, mergeTip)) + private def parseCommit(id:ObjectId) = using(new RevWalk( repository ))(_.parseCommit(id)) } } \ No newline at end of file diff --git a/src/main/scala/gitbucket/core/service/ProtectedBrancheService.scala b/src/main/scala/gitbucket/core/service/ProtectedBrancheService.scala index 2d2fd9a..89a1955 100644 --- a/src/main/scala/gitbucket/core/service/ProtectedBrancheService.scala +++ b/src/main/scala/gitbucket/core/service/ProtectedBrancheService.scala @@ -101,6 +101,7 @@ } else { contexts.toSet -- getCommitStatues(owner, repository, sha1).filter(_.state == CommitState.SUCCESS).map(_.context).toSet } + def needStatusCheck(pusher: Option[String])(implicit session: Session): Boolean = pusher.map(needStatusCheck).getOrElse(false) def needStatusCheck(pusher: String)(implicit session: Session): Boolean = if(!enabled || contexts.isEmpty){ false @@ -109,8 +110,7 @@ }else{ !isAdministrator(pusher) } - def withRequireStatues(statuses: List[CommitStatus]): List[CommitStatus] = { - statuses ++ (contexts.toSet -- statuses.map(_.context).toSet).map{ context => CommitStatus( + def pendingCommitStatus(context: String) = CommitStatus( commitStatusId = 0, userName = owner, repositoryName = repository, @@ -122,10 +122,6 @@ creator = "", registeredDate = new java.util.Date(), updatedDate = new java.util.Date()) - } - } - def hasProblem(statuses: List[CommitStatus], sha1: String, account: String)(implicit session: Session): Boolean = - needStatusCheck(account) && contexts.exists(context => statuses.find(_.context == context).map(_.state) != Some(CommitState.SUCCESS)) } object ProtectedBranchInfo{ def disabled(owner: String, repository: String): ProtectedBranchInfo = ProtectedBranchInfo(owner, repository, false, Nil, false) diff --git a/src/main/scala/gitbucket/core/service/PullRequestService.scala b/src/main/scala/gitbucket/core/service/PullRequestService.scala index 67db3c8..8fb8521 100644 --- a/src/main/scala/gitbucket/core/service/PullRequestService.scala +++ b/src/main/scala/gitbucket/core/service/PullRequestService.scala @@ -1,6 +1,6 @@ package gitbucket.core.service -import gitbucket.core.model.{Account, Issue, PullRequest, WebHook} +import gitbucket.core.model.{Account, Issue, PullRequest, WebHook, CommitStatus, CommitState} import gitbucket.core.model.Profile._ import gitbucket.core.util.JGitUtil import profile.simple._ diff --git a/src/main/twirl/gitbucket/core/pulls/conversation.scala.html b/src/main/twirl/gitbucket/core/pulls/conversation.scala.html index 6b13496..a4e4749 100644 --- a/src/main/twirl/gitbucket/core/pulls/conversation.scala.html +++ b/src/main/twirl/gitbucket/core/pulls/conversation.scala.html @@ -20,7 +20,7 @@ case comment: gitbucket.core.model.IssueComment => Some(comment) case other => None }.exists(_.action == "merge")){ merged => - @if(hasWritePermission && !issue.closed){ + @if(!issue.closed){
@issues.html.issueinfo(Some(issue), comments, issueLabels, collaborators, milestones, labels, hasWritePermission, repository) @@ -55,10 +55,12 @@ $('#merge-pull-request').show(); }); - @if(hasWritePermission){ - $('.check-conflict').show(); + var checkConflict = $('.check-conflict').show(); + if(checkConflict.length){ $.get('@url(repository)/pull/@issue.issueId/mergeguide', function(data){ $('.check-conflict').html(data); }); + } + @if(hasWritePermission){ $('.delete-branch').click(function(e){ var branchName = $(e.target).data('name'); return confirm('Are you sure you want to remove the ' + branchName + ' branch?'); diff --git a/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html b/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html index 2ca5fd3..9817d0a 100644 --- a/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html +++ b/src/main/twirl/gitbucket/core/pulls/mergeguide.scala.html @@ -1,57 +1,66 @@ -@(hasConflict: Boolean, - hasProblem: Boolean, +@(status: gitbucket.core.controller.PullRequestsController.MergeStatus, issue: gitbucket.core.model.Issue, pullreq: gitbucket.core.model.PullRequest, - statuses: List[model.CommitStatus], - branchProtection: gitbucket.core.service.ProtectedBrancheService.ProtectedBranchInfo, - hasRequiredStatusProblem: Boolean, originRepository: gitbucket.core.service.RepositoryService.RepositoryInfo, forkedRepository: gitbucket.core.service.RepositoryService.RepositoryInfo)(implicit context: gitbucket.core.controller.Context) @import gitbucket.core.service.SystemSettingsService @import context._ @import gitbucket.core.view.helpers._ @import model.CommitState -
-
+
+
- @if(!statuses.isEmpty){ + @if(!status.statuses.isEmpty){
- @defining(statuses.groupBy(_.state)){ stateMap => - @defining(CommitState.combine(stateMap.keySet)){ state => -
- - @commitStateIcon(state) - @commitStateText(state, pullreq.commitIdTo) - — @{stateMap.map{ case (keyState, states) => states.size+" "+keyState.name }.mkString(", ")} checks -
-
- @statuses.map{ status => -
-
- @branchProtection.contexts.find(_==status.context).map{ url => Required } - @status.targetUrl.map{ url => Details } -
- @commitStateIcon(status.state) - @status.context - @status.description.map{ desc => — @desc } -
- } -
- } + @defining(status.commitStateSummary){ case (summaryState, summary) => +
+ + @commitStateIcon(summaryState) + @commitStateText(summaryState, pullreq.commitIdTo) + — @summary checks +
} +
+ @status.statusesAndRequired.map{ case (status, required) => +
+
+ @if(required){ Required } + @status.targetUrl.map{ url => Details } +
+ @commitStateIcon(status.state) + @status.context + @status.description.map{ desc => — @desc } +
+ } +
} -
- -
-
- @if(hasConflict){ +
+ @if(status.hasConflict){
This branch has conflicts that must be resolved
- Use the command line to resolve conflicts before continuing. + @if(status.hasMergePermission){ + Use the command line to resolve conflicts before continuing. + } else { + Only those with write access to this repository can merge pull requests. + }
- } else { @if(hasRequiredStatusProblem) { + } else { @if(status.branchIsOutOfDate){ + @if(status.hasUpdatePermission){ +
+
+ + +
+
+ } +
+ This branch is out-of-date with the base branch +
+ Merge the latest changes from @pullreq.branch into this branch. +
+ } else { @if(status.hasRequiredStatusProblem) {
Required statuses must pass before merging.
@@ -59,63 +68,72 @@
} else {
- Merging can be performed automatically. -
- Merging can be performed automatically. -
- } } -
-
- - You can also merge branches on the command line. -