[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>

3 files changed, 31 insertions, 30 deletions

diff --git a/routers/repo/branch.go b/routers/repo/branch.go
index 7d844abe5a..cf6abc08df 100644
--- a/routers/repo/branch.go
+++ b/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 {
diff --git a/routers/repo/compare.go b/routers/repo/compare.go
index aa4b3191b2..218f712469 100644
--- a/routers/repo/compare.go
+++ b/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
diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index fb7107451f..3bc20839aa 100644
--- a/routers/repo/issue.go
+++ b/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