API method to list all commits of a repository (#6408)

* Added API endpoint ListAllCommits (/repos/{owner}/{repo}/git/commits)

Signed-off-by: Mike Schwörer <mailport@mikescher.de>

* Fixed failing drone build

Signed-off-by: Mike Schwörer <mailport@mikescher.de>

* Implemented requested changes (PR reviews)

Signed-off-by: Mike Schwörer <mailport@mikescher.de>

* gofmt

Signed-off-by: Mike Schwörer <mailport@mikescher.de>

* 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 <lauris@nix.lv>

* Update routers/api/v1/repo/commits.go

Co-Authored-By: Lauris BH <lauris@nix.lv>

* go fmt

* Refactored common code into ToCommit()

* made toCommit not exported

* added check for userCache == nil

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)
+}

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"`
+}

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() {

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
-	} 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
+
+	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
+	}
+
+	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
 }

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 {

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": {