diff --git a/src/main/resources/update/gitbucket-core_4.0.xml b/src/main/resources/update/gitbucket-core_4.0.xml index 1603e59b5..0c95711a5 100644 --- a/src/main/resources/update/gitbucket-core_4.0.xml +++ b/src/main/resources/update/gitbucket-core_4.0.xml @@ -237,7 +237,7 @@ - + diff --git a/src/main/resources/update/gitbucket-core_4.44.xml b/src/main/resources/update/gitbucket-core_4.44.xml index 049740185..d7b55a95d 100644 --- a/src/main/resources/update/gitbucket-core_4.44.xml +++ b/src/main/resources/update/gitbucket-core_4.44.xml @@ -29,4 +29,12 @@ + + + + + + + + diff --git a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala index f745adcc6..01071518a 100644 --- a/src/main/scala/gitbucket/core/controller/PullRequestsController.scala +++ b/src/main/scala/gitbucket/core/controller/PullRequestsController.scala @@ -13,8 +13,10 @@ import gitbucket.core.util.Implicits.* import gitbucket.core.util.* import org.scalatra.forms.* import org.eclipse.jgit.api.Git +import org.eclipse.jgit.revwalk.RevWalk import org.scalatra.BadRequest +import java.nio.file.Files import scala.util.Using class PullRequestsController @@ -247,41 +249,43 @@ trait PullRequestsControllerBase extends ControllerBase { }) get("/:owner/:repository/pull/:id/delete_branch")(readableUsersOnly { baseRepository => - (for { - issueId <- params("id").toIntOpt - loginAccount <- context.loginAccount - case (issue, pullreq) <- getPullRequest(baseRepository.owner, baseRepository.name, issueId) - owner = pullreq.requestUserName - name = pullreq.requestRepositoryName - if hasDeveloperRole(owner, name, context.loginAccount) - } yield { - val repository = getRepository(owner, name).get - val branchProtection = getProtectedBranchInfo(owner, name, pullreq.requestBranch) - if (branchProtection.enabled) { - flash.update("error", s"branch ${pullreq.requestBranch} is protected.") - } else { - if (repository.repository.defaultBranch != pullreq.requestBranch) { - val userName = context.loginAccount.get.userName - Using.resource(Git.open(getRepositoryDir(repository.owner, repository.name))) { git => - git.branchDelete().setForce(true).setBranchNames(pullreq.requestBranch).call() - val deleteBranchInfo = DeleteBranchInfo(repository.owner, repository.name, userName, pullreq.requestBranch) - recordActivity(deleteBranchInfo) - } - createComment( - baseRepository.owner, - baseRepository.name, - userName, - issueId, - pullreq.requestBranch, - "delete_branch" - ) + context.withLoginAccount { _ => + (for { + issueId <- params("id").toIntOpt + case (issue, pullreq) <- getPullRequest(baseRepository.owner, baseRepository.name, issueId) + owner = pullreq.requestUserName + name = pullreq.requestRepositoryName + if hasDeveloperRole(owner, name, context.loginAccount) + } yield { + val repository = getRepository(owner, name).get + val branchProtection = getProtectedBranchInfo(owner, name, pullreq.requestBranch) + if (branchProtection.enabled) { + flash.update("error", s"branch ${pullreq.requestBranch} is protected.") } else { - flash.update("error", s"""Can't delete the default branch "${pullreq.requestBranch}".""") + if (repository.repository.defaultBranch != pullreq.requestBranch) { + val userName = context.loginAccount.get.userName + Using.resource(Git.open(getRepositoryDir(repository.owner, repository.name))) { git => + git.branchDelete().setForce(true).setBranchNames(pullreq.requestBranch).call() + val deleteBranchInfo = + DeleteBranchInfo(repository.owner, repository.name, userName, pullreq.requestBranch) + recordActivity(deleteBranchInfo) + } + createComment( + baseRepository.owner, + baseRepository.name, + userName, + issueId, + pullreq.requestBranch, + "delete_branch" + ) + } else { + flash.update("error", s"""Can't delete the default branch "${pullreq.requestBranch}".""") + } } - } - redirect(s"/${baseRepository.owner}/${baseRepository.name}/pull/${issueId}") - }) getOrElse NotFound() + redirect(s"/${baseRepository.owner}/${baseRepository.name}/pull/${issueId}") + }) getOrElse NotFound() + } }) post("/:owner/:repository/pull/:id/update_branch")(readableUsersOnly { baseRepository => @@ -361,8 +365,11 @@ trait PullRequestsControllerBase extends ControllerBase { form.isDraft, context.settings ) match { - case Right(objectId) => redirect(s"/${repository.owner}/${repository.name}/pull/$issueId") - case Left(message) => Some(BadRequest(message)) + case Right(result) => + updateMergedCommitIds(repository.owner, repository.name, issueId, result.mergedCommitId) + redirect(s"/${repository.owner}/${repository.name}/pull/$issueId") + case Left(message) => + Some(BadRequest(message)) } } getOrElse NotFound() } @@ -722,6 +729,107 @@ trait PullRequestsControllerBase extends ControllerBase { ) } + post("/:owner/:repository/pull/:id/revert")(writableUsersOnly { repository => + context.withLoginAccount { loginAccount => + (for { + issueId <- params.get("id").map(_.toInt) + (issue, pullreq) <- getPullRequest(repository.owner, repository.name, issueId) if issue.closed + } yield { + val baseBranch = pullreq.branch + val revertBranch = s"revert-pr-$issueId-${System.currentTimeMillis()}" + + Using.resource(Git.open(getRepositoryDir(repository.owner, repository.name))) { git => + try { + // Create a new branch from base + JGitUtil.createBranch(git, baseBranch, revertBranch) + // TODO Call webhook ??? + + val tempDir = Files.createTempDirectory("jgit-revert-") + val revertCommitName = + try { + // Clone bare repository + Using.resource( + Git.cloneRepository + .setURI(getRepositoryDir(repository.owner, repository.name).getAbsolutePath) + .setDirectory(tempDir.toFile) + .setBranch(revertBranch) + .setBare(false) + .setNoCheckout(false) + .call() + ) { git => + // Get commit Ids to be reverted + val commitsToRevert = Using.resource(new RevWalk(git.getRepository)) { revWalk => + pullreq.mergedCommitIds + .map( + _.split(",") + .map { mergedCommitId => + revWalk.parseCommit(git.getRepository.resolve(mergedCommitId)) + } + .toSeq + .reverse + ) + .getOrElse(Nil) + } + + // revert + var revert = git.revert + commitsToRevert.foreach { id => + revert = revert.include(id) + } + val newCommit = revert.call() + if (newCommit != null) { + System.out.println("Reverted commit created: " + newCommit.getName) + git.push.call() + Some(newCommit.getName) + } else { + System.out.println("Revert resulted in conflicts.") + None + } + } + } finally { + FileUtil.deleteRecursively(tempDir.toFile) + } + + revertCommitName match { + case Some(revertCommitName) => + val newIssueId = insertIssue( + owner = repository.owner, + repository = repository.name, + loginUser = loginAccount.userName, + title = s"Revert #${issueId}", + content = Some(s"Revert #${issueId}"), + milestoneId = None, + priorityId = None, + isPullRequest = true + ) + createPullRequest( + originRepository = repository, + issueId = newIssueId, + originBranch = baseBranch, + requestUserName = repository.owner, + requestRepositoryName = repository.name, + requestBranch = revertBranch, + commitIdFrom = baseBranch, + commitIdTo = revertCommitName, + isDraft = false, + loginAccount = loginAccount, + settings = context.settings + ) + redirect(s"/${repository.owner}/${repository.name}/pull/$newIssueId") + + case None => + BadRequest("Failed to create revert commit.") + } + } catch { + case ex: Exception => + ex.printStackTrace() + BadRequest(s"Revert failed: ${ex.getMessage}") + } + } + }) getOrElse NotFound() + } + }) + /** * Tests whether an logged-in user can manage pull requests. */ @@ -740,5 +848,4 @@ trait PullRequestsControllerBase extends ControllerBase { case "DISABLE" => false } } - } diff --git a/src/main/scala/gitbucket/core/model/PullRequest.scala b/src/main/scala/gitbucket/core/model/PullRequest.scala index 570f5a7b1..2a2d60138 100644 --- a/src/main/scala/gitbucket/core/model/PullRequest.scala +++ b/src/main/scala/gitbucket/core/model/PullRequest.scala @@ -13,6 +13,7 @@ trait PullRequestComponent extends TemplateComponent { self: Profile => val commitIdFrom = column[String]("COMMIT_ID_FROM") val commitIdTo = column[String]("COMMIT_ID_TO") val isDraft = column[Boolean]("IS_DRAFT") + val mergedCommitIds = column[String]("MERGED_COMMIT_IDS") def * = ( userName, @@ -24,12 +25,13 @@ trait PullRequestComponent extends TemplateComponent { self: Profile => requestBranch, commitIdFrom, commitIdTo, - isDraft + isDraft, + mergedCommitIds.? ).mapTo[PullRequest] - def byPrimaryKey(userName: String, repositoryName: String, issueId: Int) = + def byPrimaryKey(userName: String, repositoryName: String, issueId: Int): Rep[Boolean] = byIssue(userName, repositoryName, issueId) - def byPrimaryKey(userName: Rep[String], repositoryName: Rep[String], issueId: Rep[Int]) = + def byPrimaryKey(userName: Rep[String], repositoryName: Rep[String], issueId: Rep[Int]): Rep[Boolean] = byIssue(userName, repositoryName, issueId) } } @@ -44,5 +46,6 @@ case class PullRequest( requestBranch: String, commitIdFrom: String, commitIdTo: String, - isDraft: Boolean + isDraft: Boolean, + mergedCommitIds: Option[String] ) diff --git a/src/main/scala/gitbucket/core/service/MergeService.scala b/src/main/scala/gitbucket/core/service/MergeService.scala index 1e7637940..fc721af48 100644 --- a/src/main/scala/gitbucket/core/service/MergeService.scala +++ b/src/main/scala/gitbucket/core/service/MergeService.scala @@ -7,7 +7,7 @@ import gitbucket.core.plugin.{PluginRegistry, ReceiveHook} import gitbucket.core.service.RepositoryService.RepositoryInfo import gitbucket.core.util.Directory._ import gitbucket.core.util.{JGitUtil, LockUtil} -import gitbucket.core.model.Profile.profile.blockingApi._ +import gitbucket.core.model.Profile.profile.blockingApi.* import gitbucket.core.model.activity.{CloseIssueInfo, MergeInfo, PushInfo} import gitbucket.core.service.SystemSettingsService.SystemSettings import gitbucket.core.service.WebHookService.WebHookPushPayload @@ -19,14 +19,14 @@ import org.eclipse.jgit.errors.NoMergeBaseException import org.eclipse.jgit.lib.{CommitBuilder, ObjectId, PersonIdent, Repository} import org.eclipse.jgit.revwalk.{RevCommit, RevWalk} -import scala.jdk.CollectionConverters._ +import scala.jdk.CollectionConverters.* import scala.util.Using trait MergeService { self: AccountService & ActivityService & IssuesService & RepositoryService & PullRequestService & WebHookPullRequestService & WebHookService => - import MergeService._ + import MergeService.* /** * Checks whether conflict will be caused in merging within pull request. @@ -61,15 +61,16 @@ trait MergeService { repository: RepositoryInfo, branch: String, issueId: Int, + commits: Seq[RevCommit], message: String, loginAccount: Account, settings: SystemSettings - )(implicit s: Session, c: JsonFormat.Context): ObjectId = { + )(implicit s: Session, c: JsonFormat.Context): MergeResult = { val beforeCommitId = git.getRepository.resolve(s"refs/heads/${branch}") - val afterCommitId = new MergeCacheInfo(git, repository.owner, repository.name, branch, issueId, getReceiveHooks()) - .merge(message, new PersonIdent(loginAccount.fullName, loginAccount.mailAddress), loginAccount.userName) - callWebHook(git, repository, branch, beforeCommitId, afterCommitId, loginAccount, settings) - afterCommitId + val mergeResult = new MergeCacheInfo(git, repository.owner, repository.name, branch, issueId, getReceiveHooks()) + .merge(message, new PersonIdent(loginAccount.fullName, loginAccount.mailAddress), loginAccount.userName, commits) + callWebHook(git, repository, branch, beforeCommitId, mergeResult.newCommitId, loginAccount, settings) + mergeResult } /** rebase to the head of the pull request branch */ @@ -81,13 +82,13 @@ trait MergeService { commits: Seq[RevCommit], loginAccount: Account, settings: SystemSettings - )(implicit s: Session, c: JsonFormat.Context): ObjectId = { + )(implicit s: Session, c: JsonFormat.Context): MergeResult = { val beforeCommitId = git.getRepository.resolve(s"refs/heads/${branch}") - val afterCommitId = + val mergeResult = new MergeCacheInfo(git, repository.owner, repository.name, branch, issueId, getReceiveHooks()) .rebase(new PersonIdent(loginAccount.fullName, loginAccount.mailAddress), loginAccount.userName, commits) - callWebHook(git, repository, branch, beforeCommitId, afterCommitId, loginAccount, settings) - afterCommitId + callWebHook(git, repository, branch, beforeCommitId, mergeResult.newCommitId, loginAccount, settings) + mergeResult } /** squash commits in the pull request and append it */ @@ -99,13 +100,13 @@ trait MergeService { message: String, loginAccount: Account, settings: SystemSettings - )(implicit s: Session, c: JsonFormat.Context): ObjectId = { + )(implicit s: Session, c: JsonFormat.Context): MergeResult = { val beforeCommitId = git.getRepository.resolve(s"refs/heads/${branch}") - val afterCommitId = + val mergeResult = new MergeCacheInfo(git, repository.owner, repository.name, branch, issueId, getReceiveHooks()) .squash(message, new PersonIdent(loginAccount.fullName, loginAccount.mailAddress), loginAccount.userName) - callWebHook(git, repository, branch, beforeCommitId, afterCommitId, loginAccount, settings) - afterCommitId + callWebHook(git, repository, branch, beforeCommitId, mergeResult.newCommitId, loginAccount, settings) + mergeResult } private def callWebHook( @@ -337,7 +338,7 @@ trait MergeService { strategy: String, isDraft: Boolean, settings: SystemSettings - )(implicit s: Session, c: JsonFormat.Context, context: Context): Either[String, ObjectId] = { + )(implicit s: Session, c: JsonFormat.Context, context: Context): Either[String, MergeResult] = { if (!isDraft) { if (repository.repository.options.mergeOptions.split(",").contains(strategy)) { LockUtil.lock(s"${repository.owner}/${repository.name}") { @@ -493,7 +494,7 @@ trait MergeService { commits: Seq[Seq[CommitInfo]], receiveHooks: Seq[ReceiveHook], settings: SystemSettings - )(implicit s: Session, c: JsonFormat.Context): Option[ObjectId] = { + )(implicit s: Session, c: JsonFormat.Context): Option[MergeResult] = { val revCommits = Using .resource(new RevWalk(git.getRepository)) { revWalk => commits.flatten.map { commit => @@ -510,6 +511,7 @@ trait MergeService { repository, pullRequest.branch, issue.issueId, + revCommits, s"Merge pull request #${issue.issueId} from ${pullRequest.requestUserName}/${pullRequest.requestBranch}\n\n" + message, loginAccount, settings @@ -600,13 +602,13 @@ object MergeService { private val mergedBranchName = s"refs/pull/${issueId}/merge" private val conflictedBranchName = s"refs/pull/${issueId}/conflict" - lazy val mergeBaseTip = git.getRepository.resolve(s"refs/heads/${branch}") - lazy val mergeTip = git.getRepository.resolve(s"refs/pull/${issueId}/head") + lazy val mergeBaseTip: ObjectId = git.getRepository.resolve(s"refs/heads/${branch}") + lazy val mergeTip: ObjectId = git.getRepository.resolve(s"refs/pull/${issueId}/head") def checkConflictCache(): Option[Option[String]] = { Option(git.getRepository.resolve(mergedBranchName)) .flatMap { merged => - if (parseCommit(merged).getParents().toSet == Set(mergeBaseTip, mergeTip)) { + if (parseCommit(merged).getParents.toSet == Set(mergeBaseTip, mergeTip)) { // merged branch exists Some(None) } else { @@ -615,7 +617,7 @@ object MergeService { } .orElse(Option(git.getRepository.resolve(conflictedBranchName)).flatMap { conflicted => val commit = parseCommit(conflicted) - if (commit.getParents().toSet == Set(mergeBaseTip, mergeTip)) { + if (commit.getParents.toSet == Set(mergeBaseTip, mergeTip)) { // conflict branch exists Some(Some(commit.getFullMessage)) } else { @@ -651,14 +653,16 @@ object MergeService { None } else { val message = createConflictMessage(mergeTip, mergeBaseTip, merger) - _updateBranch(mergeTipCommit.getTree().getId(), message, conflictedBranchName) + _updateBranch(mergeTipCommit.getTree.getId, message, conflictedBranchName) git.branchDelete().setForce(true).setBranchNames(mergedBranchName).call() Some(message) } } // update branch from cache - def merge(message: String, committer: PersonIdent, pusher: String)(implicit s: Session): ObjectId = { + def merge(message: String, committer: PersonIdent, pusher: String, commits: Seq[RevCommit])(implicit + s: Session + ): MergeResult = { if (checkConflict().isDefined) { throw new RuntimeException("This pull request can't merge automatically.") } @@ -666,7 +670,7 @@ object MergeService { throw new RuntimeException(s"Not found branch ${mergedBranchName}") }) // creates merge commit - val mergeCommitId = createMergeCommit(mergeResultCommit.getTree().getId(), committer, message) + val mergeCommitId = createMergeCommit(mergeResultCommit.getTree.getId, committer, message) val refName = s"refs/heads/${branch}" val currentObjectId = git.getRepository.resolve(refName) @@ -690,10 +694,10 @@ object MergeService { hook.postReceive(userName, repositoryName, receivePack, receiveCommand, committer.getName, true) } - objectId + MergeResult(objectId, commits.map(_.name())) } - def rebase(committer: PersonIdent, pusher: String, commits: Seq[RevCommit])(implicit s: Session): ObjectId = { + def rebase(committer: PersonIdent, pusher: String, commits: Seq[RevCommit])(implicit s: Session): MergeResult = { if (checkConflict().isDefined) { throw new RuntimeException("This pull request can't merge automatically.") } @@ -713,11 +717,13 @@ object MergeService { val mergeBaseTipCommit = Using.resource(new RevWalk(git.getRepository))(_.parseCommit(mergeBaseTip)) var previousId = mergeBaseTipCommit.getId + val mergedCommitIds = Seq.newBuilder[String] Using.resource(git.getRepository.newObjectInserter) { inserter => commits.foreach { commit => val nextCommit = _cloneCommit(commit, previousId, mergeBaseTipCommit.getId) previousId = inserter.insert(nextCommit) + mergedCommitIds += previousId.name() } inserter.flush() } @@ -745,10 +751,10 @@ object MergeService { hook.postReceive(userName, repositoryName, receivePack, receiveCommand, committer.getName, true) } - objectId + MergeResult(objectId, mergedCommitIds.result()) } - def squash(message: String, committer: PersonIdent, pusher: String)(implicit s: Session): ObjectId = { + def squash(message: String, committer: PersonIdent, pusher: String)(implicit s: Session): MergeResult = { if (checkConflict().isDefined) { throw new RuntimeException("This pull request can't merge automatically.") } @@ -804,7 +810,7 @@ object MergeService { hook.postReceive(userName, repositoryName, receivePack, receiveCommand, committer.getName, true) } - objectId + MergeResult(objectId, Seq(newCommitId.name())) } // return treeId @@ -823,4 +829,5 @@ object MergeService { mergeResults.asScala.map { case (key, _) => "- `" + key + "`\n" }.mkString } + case class MergeResult(newCommitId: ObjectId, mergedCommitId: Seq[String]) } diff --git a/src/main/scala/gitbucket/core/service/PullRequestService.scala b/src/main/scala/gitbucket/core/service/PullRequestService.scala index 08348cedf..769848351 100644 --- a/src/main/scala/gitbucket/core/service/PullRequestService.scala +++ b/src/main/scala/gitbucket/core/service/PullRequestService.scala @@ -63,6 +63,15 @@ trait PullRequestService { .update((baseBranch, commitIdTo)) } + def updateMergedCommitIds(owner: String, repository: String, issueId: Int, mergedCommitIds: Seq[String])(implicit + s: Session + ): Unit = { + PullRequests + .filter(_.byPrimaryKey(owner, repository, issueId)) + .map(pr => pr.mergedCommitIds) + .update(mergedCommitIds.mkString(",")) + } + def getPullRequestCountGroupByUser(closed: Boolean, owner: Option[String], repository: Option[String])(implicit s: Session ): List[PullRequestCount] = @@ -126,7 +135,8 @@ trait PullRequestService { requestBranch, commitIdFrom, commitIdTo, - isDraft + isDraft, + None ) // fetch requested branch @@ -408,11 +418,10 @@ trait PullRequestService { .find(x => x.oldPath == file) .map { diff => (diff.oldContent, diff.newContent) match { - case (Some(oldContent), Some(newContent)) => { + case (Some(oldContent), Some(newContent)) => val oldLines = convertLineSeparator(oldContent, "LF").split("\n") val newLines = convertLineSeparator(newContent, "LF").split("\n") file -> Option(DiffUtils.diff(oldLines.toList.asJava, newLines.toList.asJava)) - } case _ => file -> None } diff --git a/src/main/twirl/gitbucket/core/pulls/conversation.scala.html b/src/main/twirl/gitbucket/core/pulls/conversation.scala.html index 93878744f..fdb587d49 100644 --- a/src/main/twirl/gitbucket/core/pulls/conversation.scala.html +++ b/src/main/twirl/gitbucket/core/pulls/conversation.scala.html @@ -81,4 +81,13 @@ }); }); + @if(issue.closed && pullreq.mergedCommitIds.isDefined){ +
+
+
+ +
+
+
+ } } diff --git a/src/test/scala/gitbucket/core/api/ApiSpecModels.scala b/src/test/scala/gitbucket/core/api/ApiSpecModels.scala index f2d466755..94cb9a2df 100644 --- a/src/test/scala/gitbucket/core/api/ApiSpecModels.scala +++ b/src/test/scala/gitbucket/core/api/ApiSpecModels.scala @@ -154,7 +154,8 @@ object ApiSpecModels { requestBranch = "new-topic", commitIdFrom = sha1, commitIdTo = sha1, - isDraft = true + isDraft = true, + mergedCommitIds = None ) val commitComment: CommitComment = CommitComment( diff --git a/src/test/scala/gitbucket/core/service/MergeServiceSpec.scala b/src/test/scala/gitbucket/core/service/MergeServiceSpec.scala index 099998cb4..d560b8796 100644 --- a/src/test/scala/gitbucket/core/service/MergeServiceSpec.scala +++ b/src/test/scala/gitbucket/core/service/MergeServiceSpec.scala @@ -171,6 +171,7 @@ class MergeServiceSpec extends AnyFunSpec with ServiceSpecBase { repository, branch, issueId, + Seq(git.getRepository.parseCommit(commitId)), "merged", context.loginAccount.get, context.settings