From 042089fbaf8a818042405a75bee08dea8db347c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20Schw=C3=B6rer?= Date: Mon, 26 Aug 2019 16:09:10 +0200 Subject: [PATCH] API method to list all commits of a repository (#6408) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Added API endpoint ListAllCommits (/repos/{owner}/{repo}/git/commits) Signed-off-by: Mike Schwörer * Fixed failing drone build Signed-off-by: Mike Schwörer * Implemented requested changes (PR reviews) Signed-off-by: Mike Schwörer * gofmt Signed-off-by: Mike Schwörer * Changed api route from "/repos/{owner}/{repo}/git/commits" to "/repos/{owner}/{repo}/commits" * Removed unnecessary line * better error message when git repo is empty * make generate-swagger * fixed removed return * Update routers/api/v1/repo/commits.go Co-Authored-By: Lauris BH * Update routers/api/v1/repo/commits.go Co-Authored-By: Lauris BH * go fmt * Refactored common code into ToCommit() * made toCommit not exported * added check for userCache == nil --- integrations/api_repo_git_commits_test.go | 58 ++++++ modules/structs/miscellaneous.go | 6 + routers/api/v1/api.go | 11 +- routers/api/v1/repo/commits.go | 207 +++++++++++++++++++--- routers/api/v1/swagger/repo.go | 29 +++ templates/swagger/v1_json.tmpl | 107 +++++++++++ 6 files changed, 392 insertions(+), 26 deletions(-) diff --git a/integrations/api_repo_git_commits_test.go b/integrations/api_repo_git_commits_test.go index 587e9de5b..16db1e871 100644 --- a/integrations/api_repo_git_commits_test.go +++ b/integrations/api_repo_git_commits_test.go @@ -9,6 +9,9 @@ import ( "testing" "code.gitea.io/gitea/models" + api "code.gitea.io/gitea/modules/structs" + + "github.com/stretchr/testify/assert" ) func TestAPIReposGitCommits(t *testing.T) { @@ -30,3 +33,58 @@ func TestAPIReposGitCommits(t *testing.T) { req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo1/git/commits/unknown?token="+token, user.Name) session.MakeRequest(t, req, http.StatusNotFound) } + +func TestAPIReposGitCommitList(t *testing.T) { + prepareTestEnv(t) + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + // Login as User2. + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session) + + // Test getting commits (Page 1) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo16/commits?token="+token, user.Name) + resp := session.MakeRequest(t, req, http.StatusOK) + + var apiData []api.Commit + DecodeJSON(t, resp, &apiData) + + assert.Equal(t, 3, len(apiData)) + assert.Equal(t, "69554a64c1e6030f051e5c3f94bfbd773cd6a324", apiData[0].CommitMeta.SHA) + assert.Equal(t, "27566bd5738fc8b4e3fef3c5e72cce608537bd95", apiData[1].CommitMeta.SHA) + assert.Equal(t, "5099b81332712fe655e34e8dd63574f503f61811", apiData[2].CommitMeta.SHA) +} + +func TestAPIReposGitCommitListPage2Empty(t *testing.T) { + prepareTestEnv(t) + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + // Login as User2. + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session) + + // Test getting commits (Page=2) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo16/commits?token="+token+"&page=2", user.Name) + resp := session.MakeRequest(t, req, http.StatusOK) + + var apiData []api.Commit + DecodeJSON(t, resp, &apiData) + + assert.Equal(t, 0, len(apiData)) +} + +func TestAPIReposGitCommitListDifferentBranch(t *testing.T) { + prepareTestEnv(t) + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + // Login as User2. + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session) + + // Test getting commits (Page=1, Branch=good-sign) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/repo16/commits?token="+token+"&sha=good-sign", user.Name) + resp := session.MakeRequest(t, req, http.StatusOK) + + var apiData []api.Commit + DecodeJSON(t, resp, &apiData) + + assert.Equal(t, 1, len(apiData)) + assert.Equal(t, "f27c2b2b03dcab38beaf89b0ab4ff61f6de63441", apiData[0].CommitMeta.SHA) +} diff --git a/modules/structs/miscellaneous.go b/modules/structs/miscellaneous.go index 8eca90330..c21c466cb 100644 --- a/modules/structs/miscellaneous.go +++ b/modules/structs/miscellaneous.go @@ -44,3 +44,9 @@ type MarkdownRender string type ServerVersion struct { Version string `json:"version"` } + +// APIError is an api error with a message +type APIError struct { + Message string `json:"message"` + URL string `json:"url"` +} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 64c4b47a6..2842d78cd 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -744,10 +744,13 @@ func RegisterRoutes(m *macaron.Macaron) { m.Combo("/:sha").Get(repo.GetCommitStatuses). Post(reqToken(), bind(api.CreateStatusOption{}), repo.NewCommitStatus) }, reqRepoReader(models.UnitTypeCode)) - m.Group("/commits/:ref", func() { - // TODO: Add m.Get("") for single commit (https://developer.github.com/v3/repos/commits/#get-a-single-commit) - m.Get("/status", repo.GetCombinedCommitStatusByRef) - m.Get("/statuses", repo.GetCommitStatusesByRef) + m.Group("/commits", func() { + m.Get("", repo.GetAllCommits) + m.Group("/:ref", func() { + // TODO: Add m.Get("") for single commit (https://developer.github.com/v3/repos/commits/#get-a-single-commit) + m.Get("/status", repo.GetCombinedCommitStatusByRef) + m.Get("/statuses", repo.GetCommitStatusesByRef) + }) }, reqRepoReader(models.UnitTypeCode)) m.Group("/git", func() { m.Group("/commits", func() { diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index 795ac1f22..0156aaaa0 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -6,6 +6,8 @@ package repo import ( + "math" + "strconv" "time" "code.gitea.io/gitea/models" @@ -55,25 +57,186 @@ func GetSingleCommit(ctx *context.APIContext) { return } - // Retrieve author and committer information - var apiAuthor, apiCommitter *api.User - author, err := models.GetUserByEmail(commit.Author.Email) - if err != nil && !models.IsErrUserNotExist(err) { - ctx.ServerError("Get user by author email", err) + json, err := toCommit(ctx, ctx.Repo.Repository, commit, nil) + if err != nil { + ctx.ServerError("toCommit", err) + return + } + + ctx.JSON(200, json) +} + +// GetAllCommits get all commits via +func GetAllCommits(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/commits repository repoGetAllCommits + // --- + // summary: Get a list of all commits 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: sha + // in: query + // description: SHA or branch to start listing commits from (usually 'master') + // type: string + // - name: page + // in: query + // description: page number of requested commits + // type: integer + // responses: + // "200": + // "$ref": "#/responses/CommitList" + // "404": + // "$ref": "#/responses/notFound" + // "409": + // "$ref": "#/responses/EmptyRepository" + + if ctx.Repo.Repository.IsEmpty { + ctx.JSON(409, api.APIError{ + Message: "Git Repository is empty.", + URL: setting.API.SwaggerURL, + }) + return + } + + gitRepo, err := git.OpenRepository(ctx.Repo.Repository.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return + } + + page := ctx.QueryInt("page") + if page <= 0 { + page = 1 + } + + sha := ctx.Query("sha") + + var baseCommit *git.Commit + if len(sha) == 0 { + // no sha supplied - use default branch + head, err := gitRepo.GetHEADBranch() + if err != nil { + ctx.ServerError("GetHEADBranch", err) + return + } + + baseCommit, err = gitRepo.GetBranchCommit(head.Name) + if err != nil { + ctx.ServerError("GetCommit", err) + return + } + } else { + // get commit specified by sha + baseCommit, err = gitRepo.GetCommit(sha) + if err != nil { + ctx.ServerError("GetCommit", err) + return + } + } + + // Total commit count + commitsCountTotal, err := baseCommit.CommitsCount() + if err != nil { + ctx.ServerError("GetCommitsCount", err) + return + } + + pageCount := int(math.Ceil(float64(commitsCountTotal) / float64(git.CommitsRangeSize))) + + // Query commits + commits, err := baseCommit.CommitsByRange(page) + if err != nil { + ctx.ServerError("CommitsByRange", err) return - } else if err == nil { - apiAuthor = author.APIFormat() } - // Save one query if the author is also the committer - if commit.Committer.Email == commit.Author.Email { - apiCommitter = apiAuthor + + userCache := make(map[string]*models.User) + + apiCommits := make([]*api.Commit, commits.Len()) + + i := 0 + for commitPointer := commits.Front(); commitPointer != nil; commitPointer = commitPointer.Next() { + commit := commitPointer.Value.(*git.Commit) + + // Create json struct + apiCommits[i], err = toCommit(ctx, ctx.Repo.Repository, commit, userCache) + if err != nil { + ctx.ServerError("toCommit", err) + return + } + + i++ + } + + ctx.SetLinkHeader(int(commitsCountTotal), git.CommitsRangeSize) + + ctx.Header().Set("X-Page", strconv.Itoa(page)) + ctx.Header().Set("X-PerPage", strconv.Itoa(git.CommitsRangeSize)) + ctx.Header().Set("X-Total", strconv.FormatInt(commitsCountTotal, 10)) + ctx.Header().Set("X-PageCount", strconv.Itoa(pageCount)) + ctx.Header().Set("X-HasMore", strconv.FormatBool(page < pageCount)) + + ctx.JSON(200, &apiCommits) +} + +func toCommit(ctx *context.APIContext, repo *models.Repository, commit *git.Commit, userCache map[string]*models.User) (*api.Commit, error) { + + var apiAuthor, apiCommitter *api.User + + // Retrieve author and committer information + + var cacheAuthor *models.User + var ok bool + if userCache == nil { + cacheAuthor = ((*models.User)(nil)) + ok = false + } else { + cacheAuthor, ok = userCache[commit.Author.Email] + } + + if ok { + apiAuthor = cacheAuthor.APIFormat() + } else { + author, err := models.GetUserByEmail(commit.Author.Email) + if err != nil && !models.IsErrUserNotExist(err) { + return nil, err + } else if err == nil { + apiAuthor = author.APIFormat() + if userCache != nil { + userCache[commit.Author.Email] = author + } + } + } + + var cacheCommitter *models.User + if userCache == nil { + cacheCommitter = ((*models.User)(nil)) + ok = false + } else { + cacheCommitter, ok = userCache[commit.Committer.Email] + } + + if ok { + apiCommitter = cacheCommitter.APIFormat() } else { committer, err := models.GetUserByEmail(commit.Committer.Email) if err != nil && !models.IsErrUserNotExist(err) { - ctx.ServerError("Get user by committer email", err) - return + return nil, err } else if err == nil { apiCommitter = committer.APIFormat() + if userCache != nil { + userCache[commit.Committer.Email] = committer + } } } @@ -82,23 +245,23 @@ func GetSingleCommit(ctx *context.APIContext) { for i := 0; i < commit.ParentCount(); i++ { sha, _ := commit.ParentID(i) apiParents[i] = &api.CommitMeta{ - URL: ctx.Repo.Repository.APIURL() + "/git/commits/" + sha.String(), + URL: repo.APIURL() + "/git/commits/" + sha.String(), SHA: sha.String(), } } - ctx.JSON(200, &api.Commit{ + return &api.Commit{ CommitMeta: &api.CommitMeta{ - URL: setting.AppURL + ctx.Link[1:], + URL: repo.APIURL() + "/git/commits/" + commit.ID.String(), SHA: commit.ID.String(), }, - HTMLURL: ctx.Repo.Repository.HTMLURL() + "/commit/" + commit.ID.String(), + HTMLURL: repo.HTMLURL() + "/commit/" + commit.ID.String(), RepoCommit: &api.RepoCommit{ - URL: setting.AppURL + ctx.Link[1:], + URL: repo.APIURL() + "/git/commits/" + commit.ID.String(), Author: &api.CommitUser{ Identity: api.Identity{ - Name: commit.Author.Name, - Email: commit.Author.Email, + Name: commit.Committer.Name, + Email: commit.Committer.Email, }, Date: commit.Author.When.Format(time.RFC3339), }, @@ -109,14 +272,14 @@ func GetSingleCommit(ctx *context.APIContext) { }, Date: commit.Committer.When.Format(time.RFC3339), }, - Message: commit.Message(), + Message: commit.Summary(), Tree: &api.CommitMeta{ - URL: ctx.Repo.Repository.APIURL() + "/git/trees/" + commit.ID.String(), + URL: repo.APIURL() + "/git/trees/" + commit.ID.String(), SHA: commit.ID.String(), }, }, Author: apiAuthor, Committer: apiCommitter, Parents: apiParents, - }) + }, nil } diff --git a/routers/api/v1/swagger/repo.go b/routers/api/v1/swagger/repo.go index 2cab5b0ed..422cc0861 100644 --- a/routers/api/v1/swagger/repo.go +++ b/routers/api/v1/swagger/repo.go @@ -190,6 +190,35 @@ type swaggerCommit struct { Body api.Commit `json:"body"` } +// CommitList +// swagger:response CommitList +type swaggerCommitList struct { + // The current page + Page int `json:"X-Page"` + + // Commits per page + PerPage int `json:"X-PerPage"` + + // Total commit count + Total int `json:"X-Total"` + + // Total number of pages + PageCount int `json:"X-PageCount"` + + // True if there is another page + HasMore bool `json:"X-HasMore"` + + //in: body + Body []api.Commit `json:"body"` +} + +// EmptyRepository +// swagger:response EmptyRepository +type swaggerEmptyRepository struct { + //in: body + Body api.APIError `json:"body"` +} + // FileResponse // swagger:response FileResponse type swaggerFileResponse struct { diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 64fce4a9f..de670156d 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -1556,6 +1556,57 @@ } } }, + "/repos/{owner}/{repo}/commits": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Get a list of all commits from a repository", + "operationId": "repoGetAllCommits", + "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": "SHA or branch to start listing commits from (usually 'master')", + "name": "sha", + "in": "query" + }, + { + "type": "integer", + "description": "page number of requested commits", + "name": "page", + "in": "query" + } + ], + "responses": { + "200": { + "$ref": "#/responses/CommitList" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "409": { + "$ref": "#/responses/EmptyRepository" + } + } + } + }, "/repos/{owner}/{repo}/commits/{ref}/statuses": { "get": { "produces": [ @@ -6878,6 +6929,21 @@ } }, "definitions": { + "APIError": { + "description": "APIError is an api error with a message", + "type": "object", + "properties": { + "message": { + "type": "string", + "x-go-name": "Message" + }, + "url": { + "type": "string", + "x-go-name": "URL" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "AddCollaboratorOption": { "description": "AddCollaboratorOption options when adding a user as a collaborator of a repository", "type": "object", @@ -10044,6 +10110,41 @@ "$ref": "#/definitions/Commit" } }, + "CommitList": { + "description": "CommitList", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/Commit" + } + }, + "headers": { + "X-HasMore": { + "type": "boolean", + "description": "True if there is another page" + }, + "X-Page": { + "type": "integer", + "format": "int64", + "description": "The current page" + }, + "X-PageCount": { + "type": "integer", + "format": "int64", + "description": "Total number of pages" + }, + "X-PerPage": { + "type": "integer", + "format": "int64", + "description": "Commits per page" + }, + "X-Total": { + "type": "integer", + "format": "int64", + "description": "Total commit count" + } + } + }, "ContentsListResponse": { "description": "ContentsListResponse", "schema": { @@ -10083,6 +10184,12 @@ } } }, + "EmptyRepository": { + "description": "EmptyRepository", + "schema": { + "$ref": "#/definitions/APIError" + } + }, "FileDeleteResponse": { "description": "FileDeleteResponse", "schema": {