Nemcio/GitBucket
1
0
Fork 0
mirror of https://github.com/gitbucket/gitbucket.git synced 2026-07-15 00:22:21 +02:00
Code Issues Packages Projects Releases Wiki Activity

Support for revert pull request (#3819)

Browse Source
This commit is contained in:
Naoki Takezoe
2026-03-07 08:57:12 +09:00
committed by GitHub
parent df60395ea2
commit f9e4500bbb
11 changed files with 674 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/main/resources/update/gitbucket-core_4.0.xml
View File

@@ -237,7 +237,7 @@
<addForeignKeyConstraint constraintName="IDX_ISSUE_ID_FK1" baseTableName="ISSUE_ID" baseColumnNames="USER_NAME, REPOSITORY_NAME" referencedTableName="REPOSITORY" referencedColumnNames="USER_NAME, REPOSITORY_NAME"/>
<!--================================================================================================-->
<!-- ISSUE_ID -->
<!-- ISSUE_LABEL -->
<!--================================================================================================-->
<createTable tableName="ISSUE_LABEL">
<column name="USER_NAME" type="varchar(100)" nullable="false"/>

10
src/main/resources/update/gitbucket-core_4.46.xml Normal file
View File

@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<changeSet>
<!--================================================================================================-->
<!-- PULL_REQUEST -->
<!--================================================================================================-->
<addColumn tableName="PULL_REQUEST">
<column name="MERGED_COMMIT_IDS" type="text" nullable="true"/>
</addColumn>
</changeSet>

3
src/main/scala/gitbucket/core/GitBucketCoreModule.scala
View File

@@ -122,7 +122,8 @@ object GitBucketCoreModule
new Version("4.42.1"),
new Version("4.43.0"),
new Version("4.44.0", new LiquibaseMigration("update/gitbucket-core_4.44.xml")),
new Version("4.45.0")
new Version("4.45.0"),
new Version("4.46.0", new LiquibaseMigration("update/gitbucket-core_4.46.xml"))
) {
java.util.logging.Logger.getLogger("liquibase").setLevel(Level.SEVERE)
}

147
src/main/scala/gitbucket/core/controller/PullRequestsController.scala
View File

@@ -247,41 +247,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 +363,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,15 +727,84 @@ 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)
val revertCommitId = pullreq.mergedCommitIds match {
case Some(mergedCommitIds) =>
createRevertCommit(
git,
revertBranch,
mergedCommitIds.split(",").toSeq,
loginAccount.fullName,
loginAccount.mailAddress,
s"Revert #$issueId"
)
case None =>
Left("No merged commit IDs found for this pull request")
}
revertCommitId match {
case Right(revertCommitObjectId) =>
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 = git.getRepository.resolve(s"refs/heads/$baseBranch").getName,
commitIdTo = revertCommitObjectId.name(),
isDraft = false,
loginAccount = loginAccount,
settings = context.settings
)
redirect(s"/${repository.owner}/${repository.name}/pull/$newIssueId")
case Left(errorMessage) =>
// Clean up the branch we created
git.branchDelete().setForce(true).setBranchNames(revertBranch).call()
BadRequest(s"Failed to create revert commit: $errorMessage")
}
} catch {
case ex: Exception =>
BadRequest(s"Revert failed: ${ex.getMessage}")
}
}
}) getOrElse NotFound()
}
})
/**
* Tests whether an logged-in user can manage pull requests.
* Tests whether the logged-in user can manage pull requests.
*/
private def isManageable(repository: RepositoryInfo)(implicit context: Context): Boolean = {
hasDeveloperRole(repository.owner, repository.name, context.loginAccount)
}
/**
* Tests whether an logged-in user can post pull requests.
* Tests whether the logged-in user can post pull requests.
*/
private def isEditable(repository: RepositoryInfo)(implicit context: Context): Boolean = {
repository.repository.options.issuesOption match {
@@ -740,5 +814,4 @@ trait PullRequestsControllerBase extends ControllerBase {
case "DISABLE" => false
}
}
}

11
src/main/scala/gitbucket/core/model/PullRequest.scala
View File

@@ -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]
)

67
src/main/scala/gitbucket/core/service/MergeService.scala
View File

@@ -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])
}

172
src/main/scala/gitbucket/core/service/PullRequestService.scala
View File

@@ -19,7 +19,10 @@ import gitbucket.core.util.StringUtil.*
import gitbucket.core.view
import gitbucket.core.view.helpers
import org.eclipse.jgit.api.Git
import org.eclipse.jgit.lib.ObjectId
import org.eclipse.jgit.dircache.{DirCache, DirCacheEntry}
import org.eclipse.jgit.lib.{CommitBuilder, FileMode, ObjectId, ObjectInserter, PersonIdent, Repository}
import org.eclipse.jgit.revwalk.{RevTree, RevWalk}
import org.eclipse.jgit.treewalk.{EmptyTreeIterator, TreeWalk}
import scala.jdk.CollectionConverters.*
import scala.util.Using
@@ -63,6 +66,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 +138,8 @@ trait PullRequestService {
requestBranch,
commitIdFrom,
commitIdTo,
isDraft
isDraft,
None
)
// fetch requested branch
@@ -408,11 +421,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
}
@@ -524,7 +536,6 @@ trait PullRequestService {
helpers.date(commit1.commitTime) == view.helpers.date(commit2.commitTime)
}
// TODO Isolate to an another method?
val diffs = JGitUtil.getDiffs(
git = newGit,
from = Some(oldId.getName),
@@ -634,6 +645,157 @@ trait PullRequestService {
}
}
}
/**
* Creates a revert commit directly on the bare repository without cloning.
* This works by creating a reverse diff of the merged commits and applying it to the base branch.
*/
def createRevertCommit(
git: Git,
targetBranch: String,
mergedCommitIds: Seq[String],
committerName: String,
committerEmail: String,
commitMessage: String
): Either[String, ObjectId] = {
try {
val repository = git.getRepository
val inserter = repository.newObjectInserter()
Using.resource(new RevWalk(repository)) { revWalk =>
// Get the target branch head
val targetHeadId = repository.resolve(s"refs/heads/$targetBranch")
if (targetHeadId == null) {
return Left(s"Branch $targetBranch not found")
}
val targetHead = revWalk.parseCommit(targetHeadId)
// Parse the commits to revert (in reverse order for proper reverting)
val commitsToRevert = mergedCommitIds.reverse.map { commitId =>
val objectId = repository.resolve(commitId)
if (objectId == null) {
throw new IllegalArgumentException(s"Commit $commitId not found")
}
revWalk.parseCommit(objectId)
}
// Start with the current tree of the target branch
var currentTreeId = targetHead.getTree.getId
// Apply reverse changes for each commit
for (commit <- commitsToRevert) {
val parentCommit = if (commit.getParentCount > 0) {
revWalk.parseCommit(commit.getParent(0))
} else {
// This is an initial commit, revert by creating empty tree
null
}
// Create new tree by applying reverse diff
currentTreeId = createTreeWithReverseDiff(
repository,
inserter,
currentTreeId,
if (parentCommit != null) parentCommit.getTree else null,
commit.getTree
)
}
// Create the revert commit
val commitBuilder = new CommitBuilder()
commitBuilder.setTreeId(currentTreeId)
commitBuilder.setParentId(targetHeadId)
commitBuilder.setAuthor(new PersonIdent(committerName, committerEmail))
commitBuilder.setCommitter(new PersonIdent(committerName, committerEmail))
commitBuilder.setMessage(commitMessage)
val revertCommitId = inserter.insert(commitBuilder)
inserter.flush()
// Update the branch to point to the new commit
val refUpdate = repository.updateRef(s"refs/heads/$targetBranch")
refUpdate.setNewObjectId(revertCommitId)
refUpdate.update()
Right(revertCommitId)
}
} catch {
case ex: Exception =>
Left(ex.getMessage)
}
}
/**
* Creates a new tree by applying the reverse of changes between fromTree and toTree to baseTree.
*/
private def createTreeWithReverseDiff(
repository: Repository,
inserter: ObjectInserter,
baseTreeId: ObjectId,
fromTree: RevTree,
toTree: RevTree
): ObjectId = {
val dirCache = DirCache.newInCore()
val builder = dirCache.builder()
val entries = scala.collection.mutable.Map[String, DirCacheEntry]()
// Start with all files from the base tree
if (baseTreeId != null) {
Using.resource(new TreeWalk(repository)) { walk =>
walk.addTree(baseTreeId)
walk.setRecursive(true)
while (walk.next()) {
val entry = new DirCacheEntry(walk.getPathString)
entry.setFileMode(walk.getFileMode(0))
entry.setObjectId(walk.getObjectId(0))
entries(walk.getPathString) = entry
}
}
}
// Apply reverse changes: if a file was added in the original change, remove it
// if a file was deleted, restore it; if modified, restore original content
Using.resource(new TreeWalk(repository)) { walk =>
if (fromTree != null) walk.addTree(fromTree) else walk.addTree(new EmptyTreeIterator())
walk.addTree(toTree)
walk.setRecursive(true)
while (walk.next()) {
val path = walk.getPathString
val fromMode = if (walk.getTreeCount > 1) walk.getFileMode(0) else FileMode.MISSING
val toMode = walk.getFileMode(walk.getTreeCount - 1)
if (fromMode == FileMode.MISSING && toMode != FileMode.MISSING) {
// File was added in the original change, so remove it in the revert
entries.remove(path)
} else if (fromMode != FileMode.MISSING && toMode == FileMode.MISSING) {
// File was deleted in the original change, so restore it in the revert
val entry = new DirCacheEntry(path)
entry.setFileMode(fromMode)
entry.setObjectId(walk.getObjectId(0))
entries(path) = entry
} else if (fromMode != FileMode.MISSING && toMode != FileMode.MISSING) {
val fromObjectId = walk.getObjectId(0)
val toObjectId = walk.getObjectId(walk.getTreeCount - 1)
if (!fromObjectId.equals(toObjectId)) {
// File was modified in the original change, restore original content
val entry = new DirCacheEntry(path)
entry.setFileMode(fromMode)
entry.setObjectId(fromObjectId)
entries(path) = entry
}
}
}
}
// Build the final tree
entries.values.toSeq.sortBy(_.getPathString).foreach(builder.add)
builder.finish()
dirCache.writeTree(inserter)
}
}
object PullRequestService {

38
src/main/twirl/gitbucket/core/pulls/conversation.scala.html
View File

@@ -36,17 +36,37 @@
</div>
</div>
}
@if(isManageableForkedRepository && issue.closed && merged &&
forkedRepository.exists(r => r.branchList.contains(pullreq.requestBranch) && r.repository.defaultBranch != pullreq.requestBranch)){
<div class="issue-comment-box" style="background-color: #d0eeff; margin-bottom: 20px;">
<div class="box-content" style="border: 1px solid #87a8c9; padding: 10px;">
<a href="@helpers.url(repository)/pull/@issue.issueId/delete_branch" class="btn btn-info pull-right delete-branch" data-name="@pullreq.requestBranch">Delete branch</a>
<div>
<span class="strong">Pull request successfully merged and closed</span>
@defining(
isManageableForkedRepository && issue.closed && merged &&
forkedRepository.exists(r => r.branchList.contains(pullreq.requestBranch) && r.repository.defaultBranch != pullreq.requestBranch),
issue.closed && pullreq.mergedCommitIds.isDefined
) { case (isBranchDeletable, isRevertible) =>
@if(isBranchDeletable || isRevertible) {
<div class="issue-comment-box" style="margin-bottom: 20px;">
<div class="box-content" style="border: 1px solid #ddd; padding: 10px;">
@if(isRevertible) {
<form method="post" action="@helpers.url(repository)/pull/@issue.issueId/revert" style="display:inline;">
<button type="submit" class="btn btn-default pull-right" onclick="return confirm('Would you create a revert pull request?');" style="margin-left: 4px;">Revert</button>
</form>
}
@if(isBranchDeletable) {
<a href="@helpers.url(repository)/pull/@issue.issueId/delete_branch" class="btn btn-default pull-right delete-branch" data-name="@pullreq.requestBranch">Delete branch</a>
}
<div>
<span class="strong">Pull request successfully merged and closed</span>
</div>
<span class="small muted">
@if(isBranchDeletable) {
The <code>@pullreq.requestBranch</code> branch can now be safely deleted.
} else {
You can create another pull request to revert this merge.
}
</span>
</div>
<span class="small muted">You're all set. The <span class="label label-info monospace">@pullreq.requestBranch</span> branch can now be safely deleted.</span>
</div>
</div>
}
@if(isBranchDeletable) {
}
}
@gitbucket.core.issues.html.commentform(issue, !merged, isEditable, isManageable, repository)
}

3
src/test/scala/gitbucket/core/api/ApiSpecModels.scala
View File

@@ -154,7 +154,8 @@ object ApiSpecModels {
requestBranch = "new-topic",
commitIdFrom = sha1,
commitIdTo = sha1,
isDraft = true
isDraft = true,
mergedCommitIds = None
)
val commitComment: CommitComment = CommitComment(

1
src/test/scala/gitbucket/core/service/MergeServiceSpec.scala
View File

@@ -171,6 +171,7 @@ class MergeServiceSpec extends AnyFunSpec with ServiceSpecBase {
repository,
branch,
issueId,
Seq(git.getRepository.parseCommit(commitId)),
"merged",
context.loginAccount.get,
context.settings

309
src/test/scala/gitbucket/core/service/PullRequestServiceSpec.scala
View File

@@ -1,8 +1,16 @@
package gitbucket.core.service
import gitbucket.core.model.*
import gitbucket.core.util.GitSpecUtil.*
import gitbucket.core.util.JGitUtil
import org.eclipse.jgit.api.Git
import org.eclipse.jgit.lib.Constants
import org.eclipse.jgit.treewalk.TreeWalk
import org.scalatest.funspec.AnyFunSpec
import scala.collection.mutable
import scala.util.Using
class PullRequestServiceSpec
extends AnyFunSpec
with ServiceSpecBase
@@ -21,7 +29,33 @@ class PullRequestServiceSpec
with WebHookPullRequestReviewCommentService
with RequestCache {
def swap(r: (Issue, PullRequest)) = (r._2 -> r._1)
private def swap(r: (Issue, PullRequest)) = (r._2 -> r._1)
/**
* Returns a map of (path -> content) for all files in the tree of the given branch.
*/
private def getFiles(git: Git, branch: String): Map[String, String] = {
val repository = git.getRepository
val headId = repository.resolve(s"refs/heads/$branch")
if (headId == null) return Map.empty
Using.resource(new org.eclipse.jgit.revwalk.RevWalk(repository)) { revWalk =>
val commit = revWalk.parseCommit(headId)
val tree = commit.getTree
val files = mutable.Map[String, String]()
Using.resource(new TreeWalk(repository)) { walk =>
walk.addTree(tree)
walk.setRecursive(true)
while (walk.next()) {
val path = walk.getPathString
val objectId = walk.getObjectId(0)
val loader = repository.open(objectId)
val content = new String(loader.getBytes, "UTF-8")
files(path) = content
}
}
files.toMap
}
}
describe("PullRequestService.getPullRequestFromBranch") {
it("""should
@@ -52,4 +86,277 @@ class PullRequestServiceSpec
}
}
}
describe("PullRequestService.createRevertCommit") {
it("should revert a single commit that modified a file") {
withTestRepository { git =>
createFile(git, Constants.HEAD, "README.md", "original content", message = "initial commit")
val commitToRevert = createFile(git, Constants.HEAD, "README.md", "modified content", message = "modify readme")
assert(getFiles(git, "main")("README.md") == "modified content")
val result = createRevertCommit(
git,
"main",
Seq(commitToRevert.getName),
"test-user",
"test@example.com",
"Revert modify readme"
)
assert(result.isRight)
assert(getFiles(git, "main")("README.md") == "original content")
}
}
it("should revert a single commit that added a new file") {
withTestRepository { git =>
createFile(git, Constants.HEAD, "README.md", "readme content", message = "initial commit")
val commitToRevert =
createFile(git, Constants.HEAD, "NEW_FILE.txt", "new file content", message = "add new file")
assert(getFiles(git, "main").contains("NEW_FILE.txt"))
val result = createRevertCommit(
git,
"main",
Seq(commitToRevert.getName),
"test-user",
"test@example.com",
"Revert add new file"
)
assert(result.isRight)
val files = getFiles(git, "main")
assert(files.contains("README.md"))
assert(!files.contains("NEW_FILE.txt"))
}
}
it("should revert a commit that deleted a file") {
withTestRepository { git =>
createFile(git, Constants.HEAD, "README.md", "readme content", message = "initial commit")
createFile(git, Constants.HEAD, "TO_DELETE.txt", "delete me", message = "add file to delete")
// Delete file by creating a commit without it
val repository = git.getRepository
val inserter = repository.newObjectInserter()
val headId = repository.resolve("main^{commit}")
val builder = org.eclipse.jgit.dircache.DirCache.newInCore.builder()
JGitUtil.processTree(git, headId) { (path, tree) =>
if (path != "TO_DELETE.txt") {
builder.add(JGitUtil.createDirCacheEntry(path, tree.getEntryFileMode, tree.getEntryObjectId))
}
}
builder.finish()
val deleteCommitId = JGitUtil.createNewCommit(
git,
inserter,
headId,
builder.getDirCache.writeTree(inserter),
Constants.HEAD,
"dummy",
"dummy@example.com",
"delete file"
)
inserter.flush()
inserter.close()
assert(!getFiles(git, "main").contains("TO_DELETE.txt"))
val result = createRevertCommit(
git,
"main",
Seq(deleteCommitId.getName),
"test-user",
"test@example.com",
"Revert delete file"
)
assert(result.isRight)
val files = getFiles(git, "main")
assert(files.contains("TO_DELETE.txt"))
assert(files("TO_DELETE.txt") == "delete me")
assert(files.contains("README.md"))
}
}
it("should revert multiple commits in sequence") {
withTestRepository { git =>
createFile(git, Constants.HEAD, "README.md", "v1", message = "initial commit")
val commit1 = createFile(git, Constants.HEAD, "README.md", "v2", message = "update to v2")
val commit2 = createFile(git, Constants.HEAD, "README.md", "v3", message = "update to v3")
assert(getFiles(git, "main")("README.md") == "v3")
val result = createRevertCommit(
git,
"main",
Seq(commit1.getName, commit2.getName),
"test-user",
"test@example.com",
"Revert two commits"
)
assert(result.isRight)
assert(getFiles(git, "main")("README.md") == "v1")
}
}
it("should preserve unrelated files") {
withTestRepository { git =>
createFile(git, Constants.HEAD, "README.md", "readme content", message = "initial commit")
createFile(git, Constants.HEAD, "OTHER.txt", "other content", message = "add other file")
val commitToRevert =
createFile(git, Constants.HEAD, "README.md", "changed readme", message = "modify readme only")
val result = createRevertCommit(
git,
"main",
Seq(commitToRevert.getName),
"test-user",
"test@example.com",
"Revert readme change"
)
assert(result.isRight)
val files = getFiles(git, "main")
assert(files("README.md") == "readme content")
assert(files("OTHER.txt") == "other content")
}
}
it("should return Left for non-existent branch") {
withTestRepository { git =>
createFile(git, Constants.HEAD, "README.md", "content", message = "initial commit")
val commitId = git.getRepository.resolve("main").getName
val result = createRevertCommit(
git,
"nonexistent-branch",
Seq(commitId),
"test-user",
"test@example.com",
"Revert"
)
assert(result.isLeft)
}
}
it("should revert commit with files in subdirectories") {
withTestRepository { git =>
createFile(git, Constants.HEAD, "README.md", "readme", message = "initial commit")
createFile(git, Constants.HEAD, "src/main/Hello.scala", "hello v1", message = "add hello")
val commitToRevert =
createFile(git, Constants.HEAD, "src/main/Hello.scala", "hello v2", message = "modify hello")
assert(getFiles(git, "main")("src/main/Hello.scala") == "hello v2")
val result = createRevertCommit(
git,
"main",
Seq(commitToRevert.getName),
"test-user",
"test@example.com",
"Revert hello change"
)
assert(result.isRight)
val files = getFiles(git, "main")
assert(files("src/main/Hello.scala") == "hello v1")
assert(files("README.md") == "readme")
}
}
it("should create proper commit metadata") {
withTestRepository { git =>
createFile(git, Constants.HEAD, "README.md", "v1", message = "initial commit")
val commitToRevert = createFile(git, Constants.HEAD, "README.md", "v2", message = "modify")
val result = createRevertCommit(
git,
"main",
Seq(commitToRevert.getName),
"revert-author",
"revert@example.com",
"Revert: modify"
)
assert(result.isRight)
val headId = git.getRepository.resolve("refs/heads/main")
Using.resource(new org.eclipse.jgit.revwalk.RevWalk(git.getRepository)) { revWalk =>
val revertCommit = revWalk.parseCommit(headId)
assert(revertCommit.getFullMessage == "Revert: modify")
assert(revertCommit.getAuthorIdent.getName == "revert-author")
assert(revertCommit.getAuthorIdent.getEmailAddress == "revert@example.com")
assert(revertCommit.getParentCount == 1)
assert(revertCommit.getParent(0).getId.getName == commitToRevert.getName)
}
}
}
it("should revert a commit that adds multiple files at once") {
withTestRepository { git =>
createFile(git, Constants.HEAD, "README.md", "readme", message = "initial commit")
// Create a commit that adds two files using the low-level API
val repository = git.getRepository
val inserter = repository.newObjectInserter()
val headId = repository.resolve("main^{commit}")
val builder = org.eclipse.jgit.dircache.DirCache.newInCore.builder()
JGitUtil.processTree(git, headId) { (path, tree) =>
builder.add(JGitUtil.createDirCacheEntry(path, tree.getEntryFileMode, tree.getEntryObjectId))
}
builder.add(
JGitUtil.createDirCacheEntry(
"file1.txt",
org.eclipse.jgit.lib.FileMode.REGULAR_FILE,
inserter.insert(Constants.OBJ_BLOB, "file1 content".getBytes("UTF-8"))
)
)
builder.add(
JGitUtil.createDirCacheEntry(
"file2.txt",
org.eclipse.jgit.lib.FileMode.REGULAR_FILE,
inserter.insert(Constants.OBJ_BLOB, "file2 content".getBytes("UTF-8"))
)
)
builder.finish()
val multiAddCommitId = JGitUtil.createNewCommit(
git,
inserter,
headId,
builder.getDirCache.writeTree(inserter),
Constants.HEAD,
"dummy",
"dummy@example.com",
"add two files"
)
inserter.flush()
inserter.close()
val filesBefore = getFiles(git, "main")
assert(filesBefore.contains("file1.txt"))
assert(filesBefore.contains("file2.txt"))
val result = createRevertCommit(
git,
"main",
Seq(multiAddCommitId.getName),
"test-user",
"test@example.com",
"Revert multi add"
)
assert(result.isRight)
val filesAfter = getFiles(git, "main")
assert(filesAfter.contains("README.md"))
assert(!filesAfter.contains("file1.txt"))
assert(!filesAfter.contains("file2.txt"))
}
}
}
}