[API] Add pagination to ListBranches (#14524)

* make PaginateUserSlice generic -> PaginateSlice

* Add pagination to ListBranches

* add skip, limit to Repository.GetBranches()

* Move routers/api/v1/utils/utils PaginateSlice -> modules/util/paginate.go

* repo_module.GetBranches paginate

* fix & rename & more logging

* better description

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: a1012112796 <1012112796@qq.com>

routers/api/v1/org/org.go

@@ -13,6 +13,7 @@ import (
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/convert"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/routers/api/v1/user"
 	"code.gitea.io/gitea/routers/api/v1/utils"
@@ -28,9 +29,9 @@ func listUserOrgs(ctx *context.APIContext, u *models.User) {
 		ctx.Error(http.StatusInternalServerError, "GetOrgsByUserID", err)
 		return
 	}
-	maxResults := len(orgs)
 
-	orgs = utils.PaginateUserSlice(orgs, listOptions.Page, listOptions.PageSize)
+	maxResults := len(orgs)
+	orgs, _ = util.PaginateSlice(orgs, listOptions.Page, listOptions.PageSize).([]*models.User)
 
 	apiOrgs := make([]*api.Organization, len(orgs))
 	for i := range orgs {

routers/api/v1/repo/branch.go

@@ -17,6 +17,7 @@ import (
 	repo_module "code.gitea.io/gitea/modules/repository"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/routers/api/v1/utils"
 	pull_service "code.gitea.io/gitea/services/pull"
 	repo_service "code.gitea.io/gitea/services/repository"
 )
@@ -284,11 +285,21 @@ func ListBranches(ctx *context.APIContext) {
 	//   description: name of the repo
 	//   type: string
 	//   required: true
+	// - name: page
+	//   in: query
+	//   description: page number of results to return (1-based)
+	//   type: integer
+	// - name: limit
+	//   in: query
+	//   description: page size of results
+	//   type: integer
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/BranchList"
 
-	branches, err := repo_module.GetBranches(ctx.Repo.Repository)
+	listOptions := utils.GetListOptions(ctx)
+	skip, _ := listOptions.GetStartEnd()
+	branches, totalNumOfBranches, err := repo_module.GetBranches(ctx.Repo.Repository, skip, listOptions.PageSize)
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "GetBranches", err)
 		return
@@ -313,6 +324,9 @@ func ListBranches(ctx *context.APIContext) {
 		}
 	}
 
+	ctx.SetLinkHeader(int(totalNumOfBranches), listOptions.PageSize)
+	ctx.Header().Set("X-Total-Count", fmt.Sprintf("%d", totalNumOfBranches))
+	ctx.Header().Set("Access-Control-Expose-Headers", "X-Total-Count, Link")
 	ctx.JSON(http.StatusOK, &apiBranches)
 }
 

routers/api/v1/utils/utils.go

@@ -66,22 +66,3 @@ func GetListOptions(ctx *context.APIContext) models.ListOptions {
 		PageSize: convert.ToCorrectPageSize(ctx.QueryInt("limit")),
 	}
 }
-
-// PaginateUserSlice cut a slice of Users as per pagination options
-// TODO: make it generic
-func PaginateUserSlice(items []*models.User, page, pageSize int) []*models.User {
-	if page != 0 {
-		page--
-	}
-
-	if page*pageSize >= len(items) {
-		return items[len(items):]
-	}
-
-	items = items[page*pageSize:]
-
-	if len(items) > pageSize {
-		return items[:pageSize]
-	}
-	return items
-}

routers/repo/branch.go

@@ -58,12 +58,14 @@ func Branches(ctx *context.Context) {
 		page = 1
 	}
 
-	pageSize := ctx.QueryInt("limit")
-	if pageSize <= 0 || pageSize > git.BranchesRangeSize {
-		pageSize = git.BranchesRangeSize
+	limit := ctx.QueryInt("limit")
+	if limit <= 0 || limit > git.BranchesRangeSize {
+		limit = git.BranchesRangeSize
 	}
 
-	branches, branchesCount := loadBranches(ctx, page, pageSize)
+	skip := (page - 1) * limit
+	log.Debug("Branches: skip: %d limit: %d", skip, limit)
+	branches, branchesCount := loadBranches(ctx, skip, limit)
 	if ctx.Written() {
 		return
 	}
@@ -80,6 +82,7 @@ func DeleteBranchPost(ctx *context.Context) {
 	defer redirect(ctx)
 	branchName := ctx.Query("name")
 	if branchName == ctx.Repo.Repository.DefaultBranch {
+		log.Debug("DeleteBranch: Can't delete default branch '%s'", branchName)
 		ctx.Flash.Error(ctx.Tr("repo.branch.default_deletion_failed", branchName))
 		return
 	}
@@ -92,16 +95,19 @@ func DeleteBranchPost(ctx *context.Context) {
 	}
 
 	if isProtected {
+		log.Debug("DeleteBranch: Can't delete protected branch '%s'", branchName)
 		ctx.Flash.Error(ctx.Tr("repo.branch.protected_deletion_failed", branchName))
 		return
 	}
 
-	if !ctx.Repo.GitRepo.IsBranchExist(branchName) || branchName == ctx.Repo.Repository.DefaultBranch {
+	if !ctx.Repo.GitRepo.IsBranchExist(branchName) {
+		log.Debug("DeleteBranch: Can't delete non existing branch '%s'", branchName)
 		ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
 		return
 	}
 
 	if err := deleteBranch(ctx, branchName); err != nil {
+		log.Error("DeleteBranch: %v", err)
 		ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName))
 		return
 	}
@@ -129,10 +135,11 @@ func RestoreBranchPost(ctx *context.Context) {
 		Env:    models.PushingEnvironment(ctx.User, ctx.Repo.Repository),
 	}); err != nil {
 		if strings.Contains(err.Error(), "already exists") {
+			log.Debug("RestoreBranch: Can't restore branch '%s', since one with same name already exist", deletedBranch.Name)
 			ctx.Flash.Error(ctx.Tr("repo.branch.already_exists", deletedBranch.Name))
 			return
 		}
-		log.Error("CreateBranch: %v", err)
+		log.Error("RestoreBranch: CreateBranch: %v", err)
 		ctx.Flash.Error(ctx.Tr("repo.branch.restore_failed", deletedBranch.Name))
 		return
 	}
@@ -148,7 +155,7 @@ func RestoreBranchPost(ctx *context.Context) {
 			RepoUserName: ctx.Repo.Owner.Name,
 			RepoName:     ctx.Repo.Repository.Name,
 		}); err != nil {
-		log.Error("Update: %v", err)
+		log.Error("RestoreBranch: Update: %v", err)
 	}
 
 	ctx.Flash.Success(ctx.Tr("repo.branch.restore_success", deletedBranch.Name))
@@ -196,16 +203,18 @@ func deleteBranch(ctx *context.Context, branchName string) error {
 }
 
 // loadBranches loads branches from the repository limited by page & pageSize.
-// NOTE: May write to context on error. page & pageSize must be > 0
-func loadBranches(ctx *context.Context, page, pageSize int) ([]*Branch, int) {
+// NOTE: May write to context on error.
+func loadBranches(ctx *context.Context, skip, limit int) ([]*Branch, int) {
 	defaultBranch, err := repo_module.GetBranch(ctx.Repo.Repository, ctx.Repo.Repository.DefaultBranch)
 	if err != nil {
+		log.Error("loadBranches: get default branch: %v", err)
 		ctx.ServerError("GetDefaultBranch", err)
 		return nil, 0
 	}
 
-	rawBranches, err := repo_module.GetBranches(ctx.Repo.Repository)
+	rawBranches, totalNumOfBranches, err := repo_module.GetBranches(ctx.Repo.Repository, skip, limit)
 	if err != nil {
+		log.Error("GetBranches: %v", err)
 		ctx.ServerError("GetBranches", err)
 		return nil, 0
 	}
@@ -222,32 +231,23 @@ func loadBranches(ctx *context.Context, page, pageSize int) ([]*Branch, int) {
 	repoIDToGitRepo := map[int64]*git.Repository{}
 	repoIDToGitRepo[ctx.Repo.Repository.ID] = ctx.Repo.GitRepo
 
-	var totalNumOfBranches = len(rawBranches)
-	var startIndex = (page - 1) * pageSize
-	if startIndex > totalNumOfBranches {
-		startIndex = totalNumOfBranches - 1
-	}
-	var endIndex = startIndex + pageSize
-	if endIndex > totalNumOfBranches {
-		endIndex = totalNumOfBranches - 1
-	}
-
 	var branches []*Branch
-	for i := startIndex; i < endIndex; i++ {
+	for i := range rawBranches {
+		if rawBranches[i].Name == defaultBranch.Name {
+			// Skip default branch
+			continue
+		}
+
 		var branch = loadOneBranch(ctx, rawBranches[i], protectedBranches, repoIDToRepo, repoIDToGitRepo)
 		if branch == nil {
 			return nil, 0
 		}
 
-		if branch.Name == ctx.Repo.Repository.DefaultBranch {
-			// Skip default branch
-			continue
-		}
-
 		branches = append(branches, branch)
 	}
 
 	// Always add the default branch
+	log.Debug("loadOneBranch: load default: '%s'", defaultBranch.Name)
 	branches = append(branches, loadOneBranch(ctx, defaultBranch, protectedBranches, repoIDToRepo, repoIDToGitRepo))
 
 	if ctx.Repo.CanWrite(models.UnitTypeCode) {
@@ -259,12 +259,13 @@ func loadBranches(ctx *context.Context, page, pageSize int) ([]*Branch, int) {
 		branches = append(branches, deletedBranches...)
 	}
 
-	return branches, len(rawBranches) - 1
+	return branches, totalNumOfBranches - 1
 }
 
 func loadOneBranch(ctx *context.Context, rawBranch *git.Branch, protectedBranches []*models.ProtectedBranch,
 	repoIDToRepo map[int64]*models.Repository,
 	repoIDToGitRepo map[int64]*git.Repository) *Branch {
+	log.Trace("loadOneBranch: '%s'", rawBranch.Name)
 
 	commit, err := rawBranch.GetCommit()
 	if err != nil {

routers/repo/compare.go

@@ -520,7 +520,7 @@ func getBranchesForRepo(user *models.User, repo *models.Repository) (bool, []str
 	}
 	defer gitRepo.Close()
 
-	branches, err := gitRepo.GetBranches()
+	branches, _, err := gitRepo.GetBranches(0, 0)
 	if err != nil {
 		return false, nil, err
 	}
@@ -541,7 +541,7 @@ func CompareDiff(ctx *context.Context) {
 	}
 
 	if ctx.Data["PageIsComparePull"] == true {
-		headBranches, err := headGitRepo.GetBranches()
+		headBranches, _, err := headGitRepo.GetBranches(0, 0)
 		if err != nil {
 			ctx.ServerError("GetBranches", err)
 			return

routers/repo/issue.go

@@ -678,7 +678,7 @@ func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository, isPull boo
 		return nil
 	}
 
-	brs, err := ctx.Repo.GitRepo.GetBranches()
+	brs, _, err := ctx.Repo.GitRepo.GetBranches(0, 0)
 	if err != nil {
 		ctx.ServerError("GetBranches", err)
 		return nil