Add Get-branch-protection, Delete-branch-protection, Get-all-status-check-contexts API (#2574)

2025-11-12 16:35:52 +01:00 · 2020-10-14 15:06:23 +09:00
parent eb64cdd9cd
commit e1e00c4b94
6 changed files with 146 additions and 57 deletions
--- a/src/main/scala/gitbucket/core/api/ApiBranchProtection.scala
+++ b/src/main/scala/gitbucket/core/api/ApiBranchProtection.scala
@@ -4,7 +4,11 @@ import gitbucket.core.service.ProtectedBranchService
 import org.json4s._

 /** https://developer.github.com/v3/repos/#enabling-and-disabling-branch-protection */
-case class ApiBranchProtection(enabled: Boolean, required_status_checks: Option[ApiBranchProtection.Status]) {
+case class ApiBranchProtection(
+  url: Option[ApiPath],
+  enabled: Boolean,
+  required_status_checks: Option[ApiBranchProtection.Status]
+) {
  def status: ApiBranchProtection.Status = required_status_checks.getOrElse(ApiBranchProtection.statusNone)
 }

@@ -15,13 +19,27 @@ object ApiBranchProtection {

  def apply(info: ProtectedBranchService.ProtectedBranchInfo): ApiBranchProtection =
    ApiBranchProtection(
+      url = Some(
+        ApiPath(
+          s"/api/v3/repos/${info.owner}/${info.repository}/branches/${info.branch}/protection"
+        )
+      ),
      enabled = info.enabled,
      required_status_checks = Some(
-        Status(EnforcementLevel(info.enabled && info.contexts.nonEmpty, info.includeAdministrators), info.contexts)
+        Status(
+          ApiPath(
+            s"/api/v3/repos/${info.owner}/${info.repository}/branches/${info.branch}/protection/required_status_checks"
+          ),
+          EnforcementLevel(info.enabled && info.contexts.nonEmpty, info.includeAdministrators),
+          info.contexts,
+          ApiPath(
+            s"/api/v3/repos/${info.owner}/${info.repository}/branches/${info.branch}/protection/required_status_checks/contexts"
+          )
+        )
      )
    )
-  val statusNone = Status(Off, Seq.empty)
-  case class Status(enforcement_level: EnforcementLevel, contexts: Seq[String])
+  val statusNone = Status(ApiPath(""), Off, Seq.empty, ApiPath(""))
+  case class Status(url: ApiPath, enforcement_level: EnforcementLevel, contexts: Seq[String], contexts_url: ApiPath)
  sealed class EnforcementLevel(val name: String)
  case object Off extends EnforcementLevel("off")
  case object NonAdmins extends EnforcementLevel("non_admins")
--- a/src/main/scala/gitbucket/core/controller/api/ApiIssueCommentControllerBase.scala
+++ b/src/main/scala/gitbucket/core/controller/api/ApiIssueCommentControllerBase.scala
@@ -15,8 +15,8 @@ trait ApiIssueCommentControllerBase extends ControllerBase {
    with ReadableUsersAuthenticator
    with ReferrerAuthenticator =>
  /*
-   * i. List comments on an issue
-   * https://developer.github.com/v3/issues/comments/#list-comments-on-an-issue
+   * i. List issue comments for a repository
+   * https://docs.github.com/en/rest/reference/issues#list-issue-comments-for-a-repository
   */
  get("/api/v3/repos/:owner/:repository/issues/:id/comments")(referrersOnly { repository =>
    (for {
@@ -31,12 +31,7 @@ trait ApiIssueCommentControllerBase extends ControllerBase {
  })

  /*
-   * ii. List comments in a repository
-   * https://developer.github.com/v3/issues/comments/#list-comments-in-a-repository
-   */
-
-  /*
-   * iii. Get an issue comment
+   * ii. Get an issue comment
   * https://docs.github.com/en/rest/reference/issues#get-an-issue-comment
   */
  get("/api/v3/repos/:owner/:repository/issues/comments/:id")(referrersOnly { repository =>
@@ -51,32 +46,7 @@ trait ApiIssueCommentControllerBase extends ControllerBase {
  })

  /*
-   * iv. Create a comment
-   * https://developer.github.com/v3/issues/comments/#create-a-comment
-   */
-  post("/api/v3/repos/:owner/:repository/issues/:id/comments")(readableUsersOnly { repository =>
-    (for {
-      issueId <- params("id").toIntOpt
-      issue <- getIssue(repository.owner, repository.name, issueId.toString)
-      body <- extractFromJsonBody[CreateAComment].map(_.body) if !body.isEmpty
-      action = params.get("action").filter(_ => isEditable(issue.userName, issue.repositoryName, issue.openedUserName))
-      (issue, id) <- handleComment(issue, Some(body), repository, action)
-      issueComment <- getComment(repository.owner, repository.name, id.toString())
-    } yield {
-      JsonFormat(
-        ApiComment(
-          issueComment,
-          RepositoryName(repository),
-          issueId,
-          ApiUser(context.loginAccount.get),
-          issue.isPullRequest
-        )
-      )
-    }) getOrElse NotFound()
-  })
-
-  /*
-   * v. Update an issue comment
+   * iii. Update an issue comment
   * https://docs.github.com/en/rest/reference/issues#update-an-issue-comment
   */
  patch("/api/v3/repos/:owner/:repository/issues/comments/:id")(readableUsersOnly { repository =>
@@ -112,9 +82,8 @@ trait ApiIssueCommentControllerBase extends ControllerBase {
  })

  /*
-   * vi. Delete a comment
+   * iv. Delete a comment
   * https://docs.github.com/en/rest/reference/issues#delete-an-issue-comment
-   *
   */
  delete("/api/v3/repos/{owner}/{repo}/issues/comments/:id")(readableUsersOnly { repository =>
    val maybeDeleteResponse: Option[Either[ActionResult, Option[Int]]] =
@@ -138,6 +107,36 @@ trait ApiIssueCommentControllerBase extends ControllerBase {
      .getOrElse(NotFound())
  })

+  /*
+   * v. List issue comments
+   * https://docs.github.com/en/rest/reference/issues#list-issue-comments
+   */
+
+  /*
+   * vi. Create an issue comment
+   * https://docs.github.com/en/rest/reference/issues#create-an-issue-comment
+   */
+  post("/api/v3/repos/:owner/:repository/issues/:id/comments")(readableUsersOnly { repository =>
+    (for {
+      issueId <- params("id").toIntOpt
+      issue <- getIssue(repository.owner, repository.name, issueId.toString)
+      body <- extractFromJsonBody[CreateAComment].map(_.body) if !body.isEmpty
+      action = params.get("action").filter(_ => isEditable(issue.userName, issue.repositoryName, issue.openedUserName))
+      (issue, id) <- handleComment(issue, Some(body), repository, action)
+      issueComment <- getComment(repository.owner, repository.name, id.toString())
+    } yield {
+      JsonFormat(
+        ApiComment(
+          issueComment,
+          RepositoryName(repository),
+          issueId,
+          ApiUser(context.loginAccount.get),
+          issue.isPullRequest
+        )
+      )
+    }) getOrElse NotFound()
+  })
+
  private def isEditable(owner: String, repository: String, author: String)(implicit context: Context): Boolean =
    hasDeveloperRole(owner, repository, context.loginAccount) || author == context.loginAccount.get.userName
 }
--- a/src/main/scala/gitbucket/core/controller/api/ApiRepositoryBranchControllerBase.scala
+++ b/src/main/scala/gitbucket/core/controller/api/ApiRepositoryBranchControllerBase.scala
@@ -7,6 +7,8 @@ import gitbucket.core.util.Directory._
 import gitbucket.core.util.Implicits._
 import gitbucket.core.util.JGitUtil.getBranches
 import org.eclipse.jgit.api.Git
+import org.scalatra.NoContent
+
 import scala.util.Using

 trait ApiRepositoryBranchControllerBase extends ControllerBase {
@@ -67,6 +69,15 @@ trait ApiRepositoryBranchControllerBase extends ControllerBase {
   * iii. Get branch protection
   * https://docs.github.com/en/rest/reference/repos#get-branch-protection
   */
+  get("/api/v3/repos/:owner/:repository/branches/:branch/protection")(referrersOnly { repository =>
+    val branch = params("branch")
+    if (repository.branchList.contains(branch)) {
+      val protection = getProtectedBranchInfo(repository.owner, repository.name, branch)
+      JsonFormat(
+        ApiBranchProtection(protection)
+      )
+    } else { NotFound() }
+  })

  /*
   * iv. Update branch protection
@@ -77,6 +88,16 @@ trait ApiRepositoryBranchControllerBase extends ControllerBase {
   * v. Delete branch protection
   * https://docs.github.com/en/rest/reference/repos#delete-branch-protection
   */
+  delete("/api/v3/repos/:owner/:repository/branches/:branch/protection")(writableUsersOnly { repository =>
+    val branch = params("branch")
+    if (repository.branchList.contains(branch)) {
+      val protection = getProtectedBranchInfo(repository.owner, repository.name, branch)
+      if (protection.enabled) {
+        disableBranchProtection(repository.owner, repository.name, branch)
+        NoContent()
+      } else NotFound()
+    } else NotFound()
+  })

  /*
   * vi. Get admin branch protection
@@ -127,6 +148,16 @@ trait ApiRepositoryBranchControllerBase extends ControllerBase {
   * xv. Get status checks protection
   * https://docs.github.com/en/rest/reference/repos#get-status-checks-protection
   */
+  get("/api/v3/repos/:owner/:repository/branches/:branch/protection/required_status_checks")(referrersOnly {
+    repository =>
+      val branch = params("branch")
+      if (repository.branchList.contains(branch)) {
+        val protection = getProtectedBranchInfo(repository.owner, repository.name, branch)
+        JsonFormat(
+          ApiBranchProtection(protection).required_status_checks
+        )
+      } else { NotFound() }
+  })

  /*
   * xvi. Update status check protection
@@ -142,6 +173,16 @@ trait ApiRepositoryBranchControllerBase extends ControllerBase {
   * xviii. Get all status check contexts
   * https://docs.github.com/en/free-pro-team@latest/rest/reference/repos#get-all-status-check-contexts
   */
+  get("/api/v3/repos/:owner/:repository/branches/:branch/protection/required_status_checks/contexts")(referrersOnly {
+    repository =>
+      val branch = params("branch")
+      if (repository.branchList.contains(branch)) {
+        val protection = getProtectedBranchInfo(repository.owner, repository.name, branch)
+        if (protection.enabled) {
+          protection.contexts.toList
+        } else NotFound()
+      } else NotFound()
+  })

  /*
   * xix. Add status check contexts
--- a/src/main/scala/gitbucket/core/service/ProtectedBranchService.scala
+++ b/src/main/scala/gitbucket/core/service/ProtectedBranchService.scala
@@ -24,13 +24,15 @@ trait ProtectedBranchService {
      }
      .map {
        case (t1, contexts) =>
-          new ProtectedBranchInfo(t1.userName, t1.repositoryName, true, contexts, t1.statusCheckAdmin)
+          new ProtectedBranchInfo(t1.userName, t1.repositoryName, t1.branch, true, contexts, t1.statusCheckAdmin)
      }

  def getProtectedBranchInfo(owner: String, repository: String, branch: String)(
    implicit session: Session
  ): ProtectedBranchInfo =
-    getProtectedBranchInfoOpt(owner, repository, branch).getOrElse(ProtectedBranchInfo.disabled(owner, repository))
+    getProtectedBranchInfoOpt(owner, repository, branch).getOrElse(
+      ProtectedBranchInfo.disabled(owner, repository, branch)
+    )

  def getProtectedBranchList(owner: String, repository: String)(implicit session: Session): List[String] =
    ProtectedBranches.filter(_.byRepository(owner, repository)).map(_.branch).list
@@ -91,6 +93,7 @@ object ProtectedBranchService {
  case class ProtectedBranchInfo(
    owner: String,
    repository: String,
+    branch: String,
    enabled: Boolean,
    /**
     * Require status checks to pass before merging
@@ -165,7 +168,7 @@ object ProtectedBranchService {
    }
  }
  object ProtectedBranchInfo {
-    def disabled(owner: String, repository: String): ProtectedBranchInfo =
-      ProtectedBranchInfo(owner, repository, false, Nil, false)
+    def disabled(owner: String, repository: String, branch: String): ProtectedBranchInfo =
+      ProtectedBranchInfo(owner, repository, branch, false, Nil, false)
  }
 }
--- a/src/test/scala/gitbucket/core/api/ApiSpecModels.scala
+++ b/src/test/scala/gitbucket/core/api/ApiSpecModels.scala
@@ -331,6 +331,7 @@ object ApiSpecModels {
    info = ProtectedBranchInfo(
      owner = repo1Name.owner,
      repository = repo1Name.name,
+      branch = "master",
      enabled = true,
      contexts = Seq("continuous-integration/travis-ci"),
      includeAdministrators = true
@@ -648,8 +649,13 @@ object ApiSpecModels {

  val jsonBranchProtection =
    """{
+       |"url":"http://gitbucket.exmple.com/api/v3/repos/octocat/Hello-World/branches/master/protection",
       |"enabled":true,
-       |"required_status_checks":{"enforcement_level":"everyone","contexts":["continuous-integration/travis-ci"]}
+       |"required_status_checks":{
+         |"url":"http://gitbucket.exmple.com/api/v3/repos/octocat/Hello-World/branches/master/protection/required_status_checks",
+         |"enforcement_level":"everyone",
+         |"contexts":["continuous-integration/travis-ci"],
+         |"contexts_url":"http://gitbucket.exmple.com/api/v3/repos/octocat/Hello-World/branches/master/protection/required_status_checks/contexts"}
       |}""".stripMargin

  val jsonBranch = s"""{
--- a/src/test/scala/gitbucket/core/service/ProtectedBranchServiceSpec.scala
+++ b/src/test/scala/gitbucket/core/service/ProtectedBranchServiceSpec.scala
@@ -21,7 +21,9 @@ class ProtectedBranchServiceSpec
  describe("getProtectedBranchInfo") {
    it("should empty is disabled") {
      withTestDB { implicit session =>
-        assert(getProtectedBranchInfo("user1", "repo1", "branch") == ProtectedBranchInfo.disabled("user1", "repo1"))
+        assert(
+          getProtectedBranchInfo("user1", "repo1", "branch") == ProtectedBranchInfo.disabled("user1", "repo1", "branch")
+        )
      }
    }
    it("should enable and update and disable") {
@@ -29,20 +31,30 @@ class ProtectedBranchServiceSpec
        generateNewUserWithDBRepository("user1", "repo1")
        enableBranchProtection("user1", "repo1", "branch", false, Nil)
        assert(
-          getProtectedBranchInfo("user1", "repo1", "branch") == ProtectedBranchInfo("user1", "repo1", true, Nil, false)
+          getProtectedBranchInfo("user1", "repo1", "branch") == ProtectedBranchInfo(
+            "user1",
+            "repo1",
+            "branch",
+            true,
+            Nil,
+            false
+          )
        )
        enableBranchProtection("user1", "repo1", "branch", true, Seq("hoge", "huge"))
        assert(
          getProtectedBranchInfo("user1", "repo1", "branch") == ProtectedBranchInfo(
            "user1",
            "repo1",
+            "branch",
            true,
            Seq("hoge", "huge"),
            true
          )
        )
        disableBranchProtection("user1", "repo1", "branch")
-        assert(getProtectedBranchInfo("user1", "repo1", "branch") == ProtectedBranchInfo.disabled("user1", "repo1"))
+        assert(
+          getProtectedBranchInfo("user1", "repo1", "branch") == ProtectedBranchInfo.disabled("user1", "repo1", "branch")
+        )
      }
    }
    it("should empty contexts is no-include-administrators") {
@@ -196,14 +208,14 @@ class ProtectedBranchServiceSpec
    it("administrator is owner") {
      withTestDB { implicit session =>
        generateNewUserWithDBRepository("user1", "repo1")
-        val x = ProtectedBranchInfo("user1", "repo1", true, Nil, false)
+        val x = ProtectedBranchInfo("user1", "repo1", "branch", true, Nil, false)
        assert(x.isAdministrator("user1") == true)
        assert(x.isAdministrator("user2") == false)
      }
    }
    it("administrator is manager") {
      withTestDB { implicit session =>
-        val x = ProtectedBranchInfo("grp1", "repo1", true, Nil, false)
+        val x = ProtectedBranchInfo("grp1", "repo1", "branch", true, Nil, false)
        x.createGroup("grp1", None, None)
        generateNewAccount("user1")
        generateNewAccount("user2")
@@ -218,7 +230,7 @@ class ProtectedBranchServiceSpec
    it("unSuccessedContexts") {
      withTestDB { implicit session =>
        val user1 = generateNewUserWithDBRepository("user1", "repo1")
-        val x = ProtectedBranchInfo("user1", "repo1", true, List("must"), false)
+        val x = ProtectedBranchInfo("user1", "repo1", "branch", true, List("must"), false)
        assert(x.unSuccessedContexts(sha) == Set("must"))
        createCommitStatus("user1", "repo1", sha, "context", CommitState.SUCCESS, None, None, now, user1)
        assert(x.unSuccessedContexts(sha) == Set("must"))
@@ -235,7 +247,7 @@ class ProtectedBranchServiceSpec
    it("unSuccessedContexts when empty") {
      withTestDB { implicit session =>
        val user1 = generateNewUserWithDBRepository("user1", "repo1")
-        val x = ProtectedBranchInfo("user1", "repo1", true, Nil, false)
+        val x = ProtectedBranchInfo("user1", "repo1", "branch", true, Nil, false)
        val sha = "0c77148632618b59b6f70004e3084002be2b8804"
        assert(x.unSuccessedContexts(sha) == Set())
        createCommitStatus("user1", "repo1", sha, "context", CommitState.SUCCESS, None, None, now, user1)
@@ -244,15 +256,25 @@ class ProtectedBranchServiceSpec
    }
    it("if disabled, needStatusCheck is false") {
      withTestDB { implicit session =>
-        assert(ProtectedBranchInfo("user1", "repo1", false, Seq("must"), true).needStatusCheck("user1") == false)
+        assert(
+          ProtectedBranchInfo("user1", "repo1", "branch", false, Seq("must"), true).needStatusCheck("user1") == false
+        )
      }
    }
    it("needStatusCheck includeAdministrators") {
      withTestDB { implicit session =>
-        assert(ProtectedBranchInfo("user1", "repo1", true, Seq("must"), false).needStatusCheck("user2") == true)
-        assert(ProtectedBranchInfo("user1", "repo1", true, Seq("must"), false).needStatusCheck("user1") == false)
-        assert(ProtectedBranchInfo("user1", "repo1", true, Seq("must"), true).needStatusCheck("user2") == true)
-        assert(ProtectedBranchInfo("user1", "repo1", true, Seq("must"), true).needStatusCheck("user1") == true)
+        assert(
+          ProtectedBranchInfo("user1", "repo1", "branch", true, Seq("must"), false).needStatusCheck("user2") == true
+        )
+        assert(
+          ProtectedBranchInfo("user1", "repo1", "branch", true, Seq("must"), false).needStatusCheck("user1") == false
+        )
+        assert(
+          ProtectedBranchInfo("user1", "repo1", "branch", true, Seq("must"), true).needStatusCheck("user2") == true
+        )
+        assert(
+          ProtectedBranchInfo("user1", "repo1", "branch", true, Seq("must"), true).needStatusCheck("user1") == true
+        )
      }
    }
  }