From 32b8172e56c38d98ce493f31a30e1642e29a9030 Mon Sep 17 00:00:00 2001 From: Cirno the Strongest <1447794+CirnoT@users.noreply.github.com> Date: Mon, 11 May 2020 11:44:36 +0200 Subject: [PATCH] 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 --- integrations/api_repo_git_commits_test.go | 28 +++--------- routers/api/v1/api.go | 3 +- routers/api/v1/repo/commits.go | 53 +++-------------------- templates/swagger/v1_json.tmpl | 50 +-------------------- 4 files changed, 15 insertions(+), 119 deletions(-) diff --git a/integrations/api_repo_git_commits_test.go b/integrations/api_repo_git_commits_test.go index f710811ea..5b0f82e85 100644 --- 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) } } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 754e146fc..6973e1df5 100644 --- 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) diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index aa949aa9e..f9bc2efa9 100644 --- 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 { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 01ad43a90..66a07419b 100644 --- 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