fix: calculate diffs between commits based on common ancestor; works for force push (#3647)

fixes #3476

Co-authored-by: Thomas Geier <thomas.geier@solidat.de>

src/main/scala/gitbucket/core/controller/ReleasesController.scala

@@ -130,11 +130,10 @@ trait ReleaseControllerBase extends ControllerBase {
   })
 
   get("/:owner/:repository/changelog/*...*")(writableUsersOnly { repository =>
-    val Seq(previousTag, currentTag) = multiParams("splat")
-    val previousTagId = repository.tags.collectFirst { case x if x.name == previousTag => x.commitId }.getOrElse("")
-
     val commitLog = Using.resource(Git.open(getRepositoryDir(repository.owner, repository.name))) { git =>
-      val commits = JGitUtil.getCommitLog(git, previousTagId, currentTag).reverse
+      val Seq(previousTag, currentTag) = multiParams("splat")
+
+      val commits = JGitUtil.getCommitLog(git, previousTag, currentTag).reverse
       commits
         .map { commit =>
           s"- ${commit.shortMessage} ${commit.id}"

src/main/scala/gitbucket/core/util/JGitUtil.scala

@@ -28,6 +28,7 @@ import org.eclipse.jgit.util.io.DisabledOutputStream
 import org.slf4j.LoggerFactory
 
 import scala.util.Using.Releasable
+import scala.util.{Try, Using}
 
 /**
  * Provides complex JGit operations.
@@ -659,17 +660,62 @@ object JGitUtil {
     }
   }
 
+  /**
+   * Returns the commit list between two revisions.
+   * `to` and `from` must be valid revision strings.
+   *
+   * @see [[org.eclipse.jgit.lib.Repository#resolve]]
+   * @param git  the Git object
+   * @param from Must refer to a valid commit object.
+   * @param to   Must refer to a valid commit object.
+   * @return The commits before 'to', that are not already present in the tree of 'from'.
+   */
+  def getCommitLog(git: Git, from: String, to: String): List[CommitInfo] = {
+    def resolveString(name: String): ObjectId = {
+      val objectId = git.getRepository.resolve(name)
+      git.getRepository.open(objectId).getType match {
+        case Constants.OBJ_COMMIT => objectId
+        case Constants.OBJ_TAG =>
+          val ref = git.getRepository.getRefDatabase.findRef(name)
+          git.getRepository.getRefDatabase.peel(ref).getPeeledObjectId
+        case _ => ObjectId.zeroId()
+      }
+    }
+
+    getCommitLog(git, resolveString(from), resolveString(to))
+  }
+
   /**
    * Returns the commit list between two revisions.
    *
    * @param git the Git object
-   * @param from the from revision
+   * @param from Must refer to a valid commit object.
-   * @param to the to revision
+   * @param to Must refer to a valid commit object.
-   * @return the commit list
+   * @return The commits before 'to', that are not already present in the tree of 'from'.
    */
-  // TODO swap parameters 'from' and 'to'!?
+  def getCommitLog(git: Git, from: ObjectId, to: ObjectId): List[CommitInfo] =
-  def getCommitLog(git: Git, from: String, to: String): List[CommitInfo] =
+    Option(from)
-    getCommitLogs(git, to)(_.getName == from)
+      .filter(f => f != ObjectId.zeroId)
+      // find the common ancestor of the two commits
+      .flatMap(f =>
+        git
+          .log()
+          .add(f)
+          .add(to)
+          .setRevFilter(RevFilter.MERGE_BASE)
+          .call()
+          .asScala
+          .headOption
+      )
+      .fold(
+        git.log() // no stop condition when merge base with 'from' is not found
+      )(f => git.log().not(f)) // we have a stop condition (start commit)
+      .add(to)
+      .call()
+      .asScala
+      .map(new CommitInfo(_))
+      .toList
+      .reverse
 
   /**
    * Returns the latest RevCommit of the specified path.

src/test/scala/gitbucket/core/util/JGitUtilSpec.scala

@@ -119,6 +119,62 @@ class JGitUtilSpec extends AnyFunSuite {
     }
   }
 
+  test("getCommitLog") {
+    withTestRepository { git =>
+      /** repo looks like this
+       * commit1 -> commit2 -> commit3 [main]
+       * \-> commit4 [branch1]
+       * */
+      val root = git.getRepository.resolve("main")
+
+      createFile(git, Constants.HEAD, "README.md", "body1", message = "commit1")
+      val commit1 = git.getRepository.resolve("main")
+
+      createFile(git, Constants.HEAD, "LICENSE", "Apache License", message = "commit2")
+      val commit2 = git.getRepository.resolve("main")
+      // also make a tag
+      JGitUtil.createTag(git, "t1", None, commit2.getName)
+
+      createFile(git, Constants.HEAD, "README.md", "body1\nbody2", message = "commit3")
+      val commit3 = git.getRepository.resolve("main")
+
+      // create branch
+      JGitUtil.createBranch(git, "main", "branch1")
+      createFile(git, "branch1", "README.md", "body2", message = "commit4")
+      val commit4 = git.getRepository.resolve("branch1")
+
+      // compare results for empty → commit3
+      assert(
+        JGitUtil.getCommitLogs(git, commit3.getName, includesLastCommit = true)(_ => false) == JGitUtil.getCommitLog(
+          git,
+          root,
+          commit3
+        )
+      )
+      // compare results for commit1 → commit3
+      assert(
+        JGitUtil.getCommitLogs(git, commit3.getName, includesLastCommit = true)(
+          _.getName != commit3.getName
+        ) == JGitUtil.getCommitLog(git, commit1, commit3)
+      )
+
+      // compare results for empty → commit4
+      assert(
+        JGitUtil.getCommitLogs(git, commit4.getName, includesLastCommit = true)(_ => false) == JGitUtil.getCommitLog(
+          git,
+          root,
+          commit4
+        )
+      )
+
+      // check with names
+      assert(JGitUtil.getCommitLog(git, "main", "branch1").size == 1)
+
+      // tag names must work, too
+      assertResult(JGitUtil.getCommitLog(git, "t1", "main").length)(1)
+    }
+  }
+
   test("createBranch, branchesOfCommit and getBranches") {
     withTestRepository { git =>
       createFile(git, Constants.HEAD, "README.md", "body1", message = "commit1")