Fix Repository Contents API issues (#2802)

2025-11-04 20:45:58 +01:00 · 2021-06-21 00:55:03 +09:00
parent 1004c83bc4
commit e0bab59846
6 changed files with 172 additions and 99 deletions
--- a/src/main/scala/gitbucket/core/api/ApiContents.scala
+++ b/src/main/scala/gitbucket/core/api/ApiContents.scala
@@ -28,7 +28,7 @@ object ApiContents {
              "file",
              fileInfo.name,
              fileInfo.path,
-              fileInfo.commitId,
+              fileInfo.id.getName,
              Some(Base64.getEncoder.encodeToString(arr)),
              Some("base64")
            )(repositoryName)
--- a/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala
+++ b/src/main/scala/gitbucket/core/controller/RepositoryViewerController.scala
@@ -15,7 +15,6 @@ import gitbucket.core.util.Directory._
 import gitbucket.core.model.{Account, WebHook}
 import gitbucket.core.service.RepositoryService.RepositoryInfo
 import gitbucket.core.service.WebHookService.{WebHookCreatePayload, WebHookPushPayload}
-import gitbucket.core.util.JGitUtil.CommitInfo
 import gitbucket.core.view
 import gitbucket.core.view.helpers
 import org.apache.commons.compress.archivers.{ArchiveEntry, ArchiveOutputStream}
@@ -267,26 +266,12 @@ trait RepositoryViewerControllerBase extends ControllerBase {
              branchName,
              repository,
              logs
-                .map {
-                  commit =>
-                    (
-                      CommitInfo(
-                        id = commit.id,
-                        shortMessage = commit.shortMessage,
-                        fullMessage = commit.fullMessage,
-                        parents = commit.parents,
-                        authorTime = commit.authorTime,
-                        authorName = commit.authorName,
-                        authorEmailAddress = commit.authorEmailAddress,
-                        commitTime = commit.commitTime,
-                        committerName = commit.committerName,
-                        committerEmailAddress = commit.committerEmailAddress,
-                        commitSign = commit.commitSign,
-                        verified = commit.commitSign.flatMap(GpgUtil.verifySign)
-                      ),
-                      JGitUtil.getTagsOnCommit(git, commit.id),
-                      getCommitStatusWithSummary(repository.owner, repository.name, commit.id)
-                    )
+                .map { commit =>
+                  (
+                    commit.copy(verified = commit.commitSign.flatMap(GpgUtil.verifySign)),
+                    JGitUtil.getTagsOnCommit(git, commit.id),
+                    getCommitStatusWithSummary(repository.owner, repository.name, commit.id)
+                  )
                }
                .splitWith {
                  case ((commit1, _, _), (commit2, _, _)) =>
@@ -356,11 +341,11 @@ trait RepositoryViewerControllerBase extends ControllerBase {

        val newFiles = files.map { file =>
          file.copy(name = if (form.path.length == 0) file.name else s"${form.path}/${file.name}")
-        }.toSeq
+        }

        if (form.newBranch) {
          val newBranchName = createNewBranchForPullRequest(repository, form.branch, loginAccount)
-          val objectId = _commit(newBranchName, files, newFiles, loginAccount)
+          val objectId = _commit(newBranchName, newFiles, loginAccount)
          val issueId =
            createIssueAndPullRequest(
              repository,
@@ -373,7 +358,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
            )
          redirect(s"/${repository.owner}/${repository.name}/pull/${issueId}")
        } else {
-          _commit(form.branch, files, newFiles, loginAccount)
+          _commit(form.branch, newFiles, loginAccount)
          if (form.path.length == 0) {
            redirect(s"/${repository.owner}/${repository.name}/tree/${form.branch}")
          } else {
@@ -384,15 +369,15 @@ trait RepositoryViewerControllerBase extends ControllerBase {

    def _commit(
      branchName: String,
-      files: Seq[CommitFile],
+      //files: Seq[CommitFile],
      newFiles: Seq[CommitFile],
      loginAccount: Account
    ): ObjectId = {
      commitFiles(
        repository = repository,
        branch = branchName,
-        path = form.path,
-        files = files.toIndexedSeq,
+        //path = form.path,
+        //files = files.toIndexedSeq,
        message = form.message.getOrElse("Add files via upload"),
        loginAccount = loginAccount,
        settings = context.settings
@@ -509,7 +494,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
        commit = form.commit,
        loginAccount = loginAccount,
        settings = context.settings
-      )
+      )._1
    }
  })

@@ -556,7 +541,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
        commit = form.commit,
        loginAccount = loginAccount,
        settings = context.settings
-      )
+      )._1
    }
  })

@@ -599,7 +584,7 @@ trait RepositoryViewerControllerBase extends ControllerBase {
        commit = form.commit,
        loginAccount = loginAccount,
        settings = context.settings
-      )
+      )._1
    }
  })

--- a/src/main/scala/gitbucket/core/controller/api/ApiRepositoryContentsControllerBase.scala
+++ b/src/main/scala/gitbucket/core/controller/api/ApiRepositoryContentsControllerBase.scala
@@ -1,9 +1,9 @@
 package gitbucket.core.controller.api
-import gitbucket.core.api.{ApiContents, ApiError, CreateAFile, JsonFormat}
+import gitbucket.core.api.{ApiCommit, ApiContents, ApiError, CreateAFile, JsonFormat}
 import gitbucket.core.controller.ControllerBase
 import gitbucket.core.service.{RepositoryCommitFileService, RepositoryService}
 import gitbucket.core.util.Directory.getRepositoryDir
-import gitbucket.core.util.JGitUtil.{FileInfo, getContentFromId, getFileList}
+import gitbucket.core.util.JGitUtil.{CommitInfo, FileInfo, getContentFromId, getFileList}
 import gitbucket.core.util._
 import gitbucket.core.view.helpers.{isRenderable, renderMarkup}
 import gitbucket.core.util.Implicits._
@@ -35,7 +35,7 @@ trait ApiRepositoryContentsControllerBase extends ControllerBase {

  /**
   * ii. Get contents
-   * https://developer.github.com/v3/repos/contents/#get-contents
+   * https://docs.github.com/en/rest/reference/repos#get-repository-content
   */
  get("/api/v3/repos/:owner/:repository/contents")(referrersOnly { repository =>
    getContents(repository, ".", params.getOrElse("ref", repository.repository.defaultBranch))
@@ -43,34 +43,34 @@ trait ApiRepositoryContentsControllerBase extends ControllerBase {

  /**
   * ii. Get contents
-   * https://developer.github.com/v3/repos/contents/#get-contents
+   * https://docs.github.com/en/rest/reference/repos#get-repository-content
   */
  get("/api/v3/repos/:owner/:repository/contents/*")(referrersOnly { repository =>
    getContents(repository, multiParams("splat").head, params.getOrElse("ref", repository.repository.defaultBranch))
  })

+  private def getFileInfo(git: Git, revision: String, pathStr: String, ignoreCase: Boolean): Option[FileInfo] = {
+    val (dirName, fileName) = pathStr.lastIndexOf('/') match {
+      case -1 =>
+        (".", pathStr)
+      case n =>
+        (pathStr.take(n), pathStr.drop(n + 1))
+    }
+    if (ignoreCase) {
+      getFileList(git, revision, dirName, maxFiles = context.settings.repositoryViewer.maxFiles)
+        .find(_.name.toLowerCase.equals(fileName.toLowerCase))
+    } else {
+      getFileList(git, revision, dirName, maxFiles = context.settings.repositoryViewer.maxFiles)
+        .find(_.name.equals(fileName))
+    }
+  }
+
  private def getContents(
    repository: RepositoryService.RepositoryInfo,
    path: String,
    refStr: String,
    ignoreCase: Boolean = false
  ) = {
-    def getFileInfo(git: Git, revision: String, pathStr: String, ignoreCase: Boolean): Option[FileInfo] = {
-      val (dirName, fileName) = pathStr.lastIndexOf('/') match {
-        case -1 =>
-          (".", pathStr)
-        case n =>
-          (pathStr.take(n), pathStr.drop(n + 1))
-      }
-      if (ignoreCase) {
-        getFileList(git, revision, dirName, maxFiles = context.settings.repositoryViewer.maxFiles)
-          .find(_.name.toLowerCase.equals(fileName.toLowerCase))
-      } else {
-        getFileList(git, revision, dirName, maxFiles = context.settings.repositoryViewer.maxFiles)
-          .find(_.name.equals(fileName))
-      }
-    }
-
    Using.resource(Git.open(getRepositoryDir(params("owner"), params("repository")))) { git =>
      val fileList = getFileList(git, refStr, path, maxFiles = context.settings.repositoryViewer.maxFiles)
      if (fileList.isEmpty) { // file or NotFound
@@ -126,14 +126,13 @@ trait ApiRepositoryContentsControllerBase extends ControllerBase {
      }
    }
  }
-  /*
+
+  /**
   * iii. Create a file or iv. Update a file
-   * https://developer.github.com/v3/repos/contents/#create-a-file
-   * https://developer.github.com/v3/repos/contents/#update-a-file
+   * https://docs.github.com/en/rest/reference/repos#create-or-update-file-contents
   * if sha is presented, update a file else create a file.
   * requested #2112
   */
-
  put("/api/v3/repos/:owner/:repository/contents/*")(writableUsersOnly { repository =>
    context.withLoginAccount {
      loginAccount =>
@@ -147,27 +146,53 @@ trait ApiRepositoryContentsControllerBase extends ControllerBase {
          }
          val paths = multiParams("splat").head.split("/")
          val path = paths.take(paths.size - 1).toList.mkString("/")
-          if (data.sha.isDefined && data.sha.get != commit) {
-            ApiError(
-              "The blob SHA is not matched.",
-              Some("https://developer.github.com/v3/repos/contents/#update-a-file")
-            )
-          } else {
-            val objectId = commitFile(
-              repository,
-              branch,
-              path,
-              Some(paths.last),
-              data.sha.map(_ => paths.last),
-              StringUtil.base64Decode(data.content),
-              data.message,
-              commit,
-              loginAccount,
-              data.committer.map(_.name).getOrElse(loginAccount.fullName),
-              data.committer.map(_.email).getOrElse(loginAccount.mailAddress),
-              context.settings
-            )
-            ApiContents("file", paths.last, path, objectId.name, None, None)(RepositoryName(repository))
+          Using.resource(Git.open(getRepositoryDir(params("owner"), params("repository")))) {
+            git =>
+              val fileInfo = getFileInfo(git, commit, path, false)
+
+              fileInfo match {
+                case Some(f) if !data.sha.contains(f.id.getName) =>
+                  ApiError(
+                    "The blob SHA is not matched.",
+                    Some("https://docs.github.com/en/rest/reference/repos#create-or-update-file-contents")
+                  )
+                case _ =>
+                  val (commitId, blobId) = commitFile(
+                    repository,
+                    branch,
+                    path,
+                    Some(paths.last),
+                    data.sha.map(_ => paths.last),
+                    StringUtil.base64Decode(data.content),
+                    data.message,
+                    commit,
+                    loginAccount,
+                    data.committer.map(_.name).getOrElse(loginAccount.fullName),
+                    data.committer.map(_.email).getOrElse(loginAccount.mailAddress),
+                    context.settings
+                  )
+
+                  blobId match {
+                    case None =>
+                      ApiError("Failed to commit a file.", None)
+                    case Some(blobId) =>
+                      Map(
+                        "content" -> ApiContents(
+                          "file",
+                          paths.last,
+                          path,
+                          blobId.name,
+                          Some(data.content),
+                          Some("base64")
+                        )(RepositoryName(repository)),
+                        "commit" -> ApiCommit(
+                          git,
+                          RepositoryName(repository),
+                          new CommitInfo(JGitUtil.getRevCommitFromId(git, commitId))
+                        )
+                      )
+                  }
+              }
          }
        })
    }
@@ -175,13 +200,14 @@ trait ApiRepositoryContentsControllerBase extends ControllerBase {

  /*
   * v. Delete a file
-   * https://developer.github.com/v3/repos/contents/#delete-a-file
+   * https://docs.github.com/en/rest/reference/repos#delete-a-file
   * should be implemented
   */

  /*
- * vi. Get archive link
- * https://developer.github.com/v3/repos/contents/#get-archive-link
+ * vi. Download a repository archive (tar/zip)
+ * https://docs.github.com/en/rest/reference/repos#download-a-repository-archive-tar
+ * https://docs.github.com/en/rest/reference/repos#download-a-repository-archive-zip
 */

 }
--- a/src/main/scala/gitbucket/core/service/RepositoryCommitFileService.scala
+++ b/src/main/scala/gitbucket/core/service/RepositoryCommitFileService.scala
@@ -23,23 +23,29 @@ trait RepositoryCommitFileService {
    with PullRequestService
    with WebHookPullRequestService
    with RepositoryService =>
-  import RepositoryCommitFileService._

+  /**
+   * Create multiple files by callback function.
+   * Returns commitId.
+   */
  def commitFiles(
    repository: RepositoryService.RepositoryInfo,
-    files: Seq[CommitFile],
    branch: String,
-    path: String,
    message: String,
    loginAccount: Account,
    settings: SystemSettings
  )(
    f: (Git, ObjectId, DirCacheBuilder, ObjectInserter) => Unit
-  )(implicit s: Session, c: JsonFormat.Context) = {
-    // prepend path to the filename
-    _commitFile(repository, branch, message, loginAccount, loginAccount.fullName, loginAccount.mailAddress, settings)(f)
+  )(implicit s: Session, c: JsonFormat.Context): ObjectId = {
+    _createFiles(repository, branch, message, loginAccount, loginAccount.fullName, loginAccount.mailAddress, settings)(
+      f
+    )._1
  }

+  /**
+   * Create a file from string content.
+   * Returns commitId + blobId.
+   */
  def commitFile(
    repository: RepositoryService.RepositoryInfo,
    branch: String,
@@ -52,7 +58,7 @@ trait RepositoryCommitFileService {
    commit: String,
    loginAccount: Account,
    settings: SystemSettings
-  )(implicit s: Session, c: JsonFormat.Context): ObjectId = {
+  )(implicit s: Session, c: JsonFormat.Context): (ObjectId, Option[ObjectId]) = {
    commitFile(
      repository,
      branch,
@@ -69,6 +75,10 @@ trait RepositoryCommitFileService {
    )
  }

+  /**
+   * Create a file from byte array content.
+   * Returns commitId + blobId.
+   */
  def commitFile(
    repository: RepositoryService.RepositoryInfo,
    branch: String,
@@ -82,7 +92,7 @@ trait RepositoryCommitFileService {
    committerName: String,
    committerMailAddress: String,
    settings: SystemSettings
-  )(implicit s: Session, c: JsonFormat.Context): ObjectId = {
+  )(implicit s: Session, c: JsonFormat.Context): (ObjectId, Option[ObjectId]) = {

    val newPath = newFileName.map { newFileName =>
      if (path.length == 0) newFileName else s"${path}/${newFileName}"
@@ -91,7 +101,7 @@ trait RepositoryCommitFileService {
      if (path.length == 0) oldFileName else s"${path}/${oldFileName}"
    }

-    _commitFile(repository, branch, message, pusherAccount, committerName, committerMailAddress, settings) {
+    _createFiles(repository, branch, message, pusherAccount, committerName, committerMailAddress, settings) {
      case (git, headTip, builder, inserter) =>
        if (headTip.getName == commit) {
          val permission = JGitUtil
@@ -105,18 +115,23 @@ trait RepositoryCommitFileService {
            }
            .flatten
            .headOption
-
-          newPath.foreach { newPath =>
-            builder.add(JGitUtil.createDirCacheEntry(newPath, permission.map { bits =>
+            .map { bits =>
              FileMode.fromBits(bits)
-            } getOrElse FileMode.REGULAR_FILE, inserter.insert(Constants.OBJ_BLOB, content)))
+            }
+            .getOrElse(FileMode.REGULAR_FILE)
+
+          val objectId = newPath.map { newPath =>
+            val objectId = inserter.insert(Constants.OBJ_BLOB, content)
+            builder.add(JGitUtil.createDirCacheEntry(newPath, permission, objectId))
+            objectId
          }
          builder.finish()
-        }
+          objectId
+        } else None
    }
  }

-  private def _commitFile(
+  private def _createFiles[R](
    repository: RepositoryService.RepositoryInfo,
    branch: String,
    message: String,
@@ -125,8 +140,8 @@ trait RepositoryCommitFileService {
    committerMailAddress: String,
    settings: SystemSettings
  )(
-    f: (Git, ObjectId, DirCacheBuilder, ObjectInserter) => Unit
-  )(implicit s: Session, c: JsonFormat.Context): ObjectId = {
+    f: (Git, ObjectId, DirCacheBuilder, ObjectInserter) => R
+  )(implicit s: Session, c: JsonFormat.Context): (ObjectId, R) = {

    LockUtil.lock(s"${repository.owner}/${repository.name}") {
      Using.resource(Git.open(getRepositoryDir(repository.owner, repository.name))) { git =>
@@ -135,7 +150,7 @@ trait RepositoryCommitFileService {
        val headName = s"refs/heads/${branch}"
        val headTip = git.getRepository.resolve(headName)

-        f(git, headTip, builder, inserter)
+        val result = f(git, headTip, builder, inserter)

        val commitId = JGitUtil.createNewCommit(
          git,
@@ -228,7 +243,7 @@ trait RepositoryCommitFileService {
              }
            }
        }
-        commitId
+        (commitId, result)
      }
    }
  }
--- a/src/main/scala/gitbucket/core/service/WebHookService.scala
+++ b/src/main/scala/gitbucket/core/service/WebHookService.scala
@@ -20,7 +20,6 @@ import gitbucket.core.model.Profile._
 import gitbucket.core.model.Profile.profile.blockingApi._
 import org.apache.http.client.utils.URLEncodedUtils
 import gitbucket.core.util.JGitUtil.CommitInfo
-import gitbucket.core.util.Implicits._
 import gitbucket.core.util.{HttpClientUtil, RepositoryName, StringUtil}
 import gitbucket.core.service.RepositoryService.RepositoryInfo
 import org.apache.http.NameValuePair
--- a/src/test/scala/gitbucket/core/api/ApiIntegrationTest.scala
+++ b/src/test/scala/gitbucket/core/api/ApiIntegrationTest.scala
@@ -1,11 +1,15 @@
 package gitbucket.core.api

 import gitbucket.core.TestingGitBucketServer
+import org.apache.commons.io.IOUtils
 import org.scalatest.funsuite.AnyFunSuite

 import scala.util.Using
 import org.kohsuke.github.{GHCommitState, GitHub}

+/**
+ * Need to run `sbt package` before running this test.
+ */
 class ApiIntegrationTest extends AnyFunSuite {

  test("create repository") {
@@ -133,4 +137,48 @@ class ApiIntegrationTest extends AnyFunSuite {
    }
  }

+  test("create and update contents") {
+    Using.resource(new TestingGitBucketServer(19999)) { server =>
+      val github = server.client("root", "root")
+
+      val repo = github.createRepository("create_contents_test").autoInit(true).create()
+
+      val createResult =
+        repo
+          .createContent()
+          .branch("master")
+          .content("create")
+          .message("Create content")
+          .path("README.md")
+          .commit();
+
+      assert(createResult.getContent.isFile == true)
+      assert(IOUtils.toString(createResult.getContent.read(), "UTF-8") == "create")
+
+      val content1 = repo.getFileContent("README.md")
+      assert(content1.isFile == true)
+      assert(IOUtils.toString(content1.read(), "UTF-8") == "create")
+      assert(content1.getSha == createResult.getContent.getSha)
+
+      val updateResult =
+        repo
+          .createContent()
+          .branch("master")
+          .content("update")
+          .message("Update content")
+          .path("README.md")
+          .sha(content1.getSha)
+          .commit();
+
+      assert(updateResult.getContent.isFile == true)
+      assert(IOUtils.toString(updateResult.getContent.read(), "UTF-8") == "update")
+
+      val content2 = repo.getFileContent("README.md")
+      assert(content2.isFile == true)
+      assert(IOUtils.toString(content2.read(), "UTF-8") == "update")
+      assert(content2.getSha == updateResult.getContent.getSha)
+      assert(content1.getSha != content2.getSha)
+    }
+  }
+
 }