Consolidate API for getting single commit (#11368)

* Allow Git ref for /repos/{owner}/{repo}/git/commits/{sha} * Consolidate API for getting single commit * Fix tests and do it differently Co-authored-by: zeripath <art27@cantab.net>
2024-12-25 11:44:52 +01:00 · 2020-05-11 11:44:36 +02:00 · 2020-05-11 11:44:36 +02:00 · 32b8172e56
commit 32b8172e56
parent 59b9b77a0d
4 changed files with 15 additions and 119 deletions
--- a/integrations/api_repo_git_commits_test.go
+++ b/integrations/api_repo_git_commits_test.go
@ -21,18 +21,14 @@ func TestAPIReposGitCommits(t *testing.T) {
 	session := loginUser(t, user.Name)
 	token := getTokenForLoggedInUser(t, session)

-	//check invalid requests for GetCommitsBySHA
-	req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/master?token="+token, user.Name)
-	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
-
-	req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/12345?token="+token, user.Name)
+	// check invalid requests
+	req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/12345?token="+token, user.Name)
 	session.MakeRequest(t, req, http.StatusNotFound)

-	//check invalid requests for GetCommitsByRef
-	req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/commits/..?token="+token, user.Name)
+	req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/..?token="+token, user.Name)
 	session.MakeRequest(t, req, http.StatusUnprocessableEntity)

-	req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/commits/branch-not-exist?token="+token, user.Name)
+	req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/branch-not-exist?token="+token, user.Name)
 	session.MakeRequest(t, req, http.StatusNotFound)

 	for _, ref := range [...]string{
@ -41,20 +37,8 @@ func TestAPIReposGitCommits(t *testing.T) {
 		"65f1",   // short sha
 		"65f1bf27bc3bf70f64657658635e66094edbcb4d", // full sha
 	} {
-		req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/commits/%s?token="+token, user.Name, ref)
-		resp := session.MakeRequest(t, req, http.StatusOK)
-		commitByRef := new(api.Commit)
-		DecodeJSON(t, resp, commitByRef)
-		assert.Len(t, commitByRef.SHA, 40)
-		assert.EqualValues(t, commitByRef.SHA, commitByRef.RepoCommit.Tree.SHA)
-		req = NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/%s?token="+token, user.Name, commitByRef.SHA)
-		resp = session.MakeRequest(t, req, http.StatusOK)
-		commitBySHA := new(api.Commit)
-		DecodeJSON(t, resp, commitBySHA)
-
-		assert.EqualValues(t, commitByRef.SHA, commitBySHA.SHA)
-		assert.EqualValues(t, commitByRef.HTMLURL, commitBySHA.HTMLURL)
-		assert.EqualValues(t, commitByRef.RepoCommit.Message, commitBySHA.RepoCommit.Message)
+		req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/%s?token="+token, user.Name, ref)
+		session.MakeRequest(t, req, http.StatusOK)
 	}
 }

--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@ -817,14 +817,13 @@ func RegisterRoutes(m *macaron.Macaron) {
 				m.Group("/commits", func() {
 					m.Get("", repo.GetAllCommits)
 					m.Group("/:ref", func() {
-						m.Get("", repo.GetSingleCommitByRef)
 						m.Get("/status", repo.GetCombinedCommitStatusByRef)
 						m.Get("/statuses", repo.GetCommitStatusesByRef)
 					})
 				}, reqRepoReader(models.UnitTypeCode))
 				m.Group("/git", func() {
 					m.Group("/commits", func() {
-						m.Get("/:sha", repo.GetSingleCommitBySHA)
+						m.Get("/:sha", repo.GetSingleCommit)
 					})
 					m.Get("/refs", repo.GetGitAllRefs)
 					m.Get("/refs/*", repo.GetGitRefs)
--- a/routers/api/v1/repo/commits.go
+++ b/routers/api/v1/repo/commits.go
@ -21,9 +21,9 @@ import (
 	"code.gitea.io/gitea/routers/api/v1/utils"
 )

-// GetSingleCommitBySHA get a commit via sha
-func GetSingleCommitBySHA(ctx *context.APIContext) {
-	// swagger:operation GET /repos/{owner}/{repo}/git/commits/{sha} repository repoGetSingleCommitBySHA
+// GetSingleCommit get a commit via sha
+func GetSingleCommit(ctx *context.APIContext) {
+	// swagger:operation GET /repos/{owner}/{repo}/git/commits/{sha} repository repoGetSingleCommit
 	// ---
 	// summary: Get a single commit from a repository
 	// produces:
@ -41,7 +41,7 @@ func GetSingleCommitBySHA(ctx *context.APIContext) {
 	//   required: true
 	// - name: sha
 	//   in: path
-	//   description: the commit hash
+	//   description: a git ref or commit sha
 	//   type: string
 	//   required: true
 	// responses:
@ -53,54 +53,13 @@ func GetSingleCommitBySHA(ctx *context.APIContext) {
 	//     "$ref": "#/responses/notFound"

 	sha := ctx.Params(":sha")
-	if !git.SHAPattern.MatchString(sha) {
-		ctx.Error(http.StatusUnprocessableEntity, "no valid sha", fmt.Sprintf("no valid sha: %s", sha))
+	if (validation.GitRefNamePatternInvalid.MatchString(sha) || !validation.CheckGitRefAdditionalRulesValid(sha)) && !git.SHAPattern.MatchString(sha) {
+		ctx.Error(http.StatusUnprocessableEntity, "no valid ref or sha", fmt.Sprintf("no valid ref or sha: %s", sha))
 		return
 	}
 	getCommit(ctx, sha)
 }

-// GetSingleCommitByRef get a commit via ref
-func GetSingleCommitByRef(ctx *context.APIContext) {
-	// swagger:operation GET /repos/{owner}/{repo}/commits/{ref} repository repoGetSingleCommitByRef
-	// ---
-	// summary: Get a single commit from a repository
-	// produces:
-	// - application/json
-	// parameters:
-	// - name: owner
-	//   in: path
-	//   description: owner of the repo
-	//   type: string
-	//   required: true
-	// - name: repo
-	//   in: path
-	//   description: name of the repo
-	//   type: string
-	//   required: true
-	// - name: ref
-	//   in: path
-	//   description: a git ref
-	//   type: string
-	//   required: true
-	// responses:
-	//   "200":
-	//     "$ref": "#/responses/Commit"
-	//   "422":
-	//     "$ref": "#/responses/validationError"
-	//   "404":
-	//     "$ref": "#/responses/notFound"
-
-	ref := ctx.Params("ref")
-
-	if validation.GitRefNamePatternInvalid.MatchString(ref) || !validation.CheckGitRefAdditionalRulesValid(ref) {
-		ctx.Error(http.StatusUnprocessableEntity, "no valid sha", fmt.Sprintf("no valid ref: %s", ref))
-		return
-	}
-
-	getCommit(ctx, ref)
-}
-
 func getCommit(ctx *context.APIContext, identifier string) {
 	gitRepo, err := git.OpenRepository(ctx.Repo.Repository.RepoPath())
 	if err != nil {
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@ -2549,52 +2549,6 @@
        }
      }
    },
-    "/repos/{owner}/{repo}/commits/{ref}": {
-      "get": {
-        "produces": [
-          "application/json"
-        ],
-        "tags": [
-          "repository"
-        ],
-        "summary": "Get a single commit from a repository",
-        "operationId": "repoGetSingleCommitByRef",
-        "parameters": [
-          {
-            "type": "string",
-            "description": "owner of the repo",
-            "name": "owner",
-            "in": "path",
-            "required": true
-          },
-          {
-            "type": "string",
-            "description": "name of the repo",
-            "name": "repo",
-            "in": "path",
-            "required": true
-          },
-          {
-            "type": "string",
-            "description": "a git ref",
-            "name": "ref",
-            "in": "path",
-            "required": true
-          }
-        ],
-        "responses": {
-          "200": {
-            "$ref": "#/responses/Commit"
-          },
-          "404": {
-            "$ref": "#/responses/notFound"
-          },
-          "422": {
-            "$ref": "#/responses/validationError"
-          }
-        }
-      }
-    },
    "/repos/{owner}/{repo}/commits/{ref}/statuses": {
      "get": {
        "produces": [
@ -3075,7 +3029,7 @@
          "repository"
        ],
        "summary": "Get a single commit from a repository",
-        "operationId": "repoGetSingleCommitBySHA",
+        "operationId": "repoGetSingleCommit",
        "parameters": [
          {
            "type": "string",
@ -3093,7 +3047,7 @@
          },
          {
            "type": "string",
-            "description": "the commit hash",
+            "description": "a git ref or commit sha",
            "name": "sha",
            "in": "path",
            "required": true