Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API] Add pagination to ListBranches #14524

Merged
merged 16 commits into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions integrations/branches_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ func branchAction(t *testing.T, button string) (*HTMLDoc, string) {

htmlDoc := NewHTMLParser(t, resp.Body)
link, exists := htmlDoc.doc.Find(button).Attr("data-url")
assert.True(t, exists, "The template has changed")
if !assert.True(t, exists, "The template has changed") {
t.Skip()
}

req = NewRequestWithValues(t, "POST", link, map[string]string{
"_csrf": getCsrf(t, htmlDoc.doc),
Expand All @@ -69,7 +71,7 @@ func branchAction(t *testing.T, button string) (*HTMLDoc, string) {
req = NewRequest(t, "GET", "/user2/repo1/branches")
resp = session.MakeRequest(t, req, http.StatusOK)

return NewHTMLParser(t, resp.Body), url.Query()["name"][0]
return NewHTMLParser(t, resp.Body), url.Query().Get("name")
}

func getCsrf(t *testing.T, doc *goquery.Document) string {
Expand Down
4 changes: 2 additions & 2 deletions modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ func RepoAssignment() func(http.Handler) http.Handler {
}
ctx.Data["Tags"] = tags

brs, err := ctx.Repo.GitRepo.GetBranches()
brs, _, err := ctx.Repo.GitRepo.GetBranches(0, 0)
if err != nil {
ctx.ServerError("GetBranches", err)
return
Expand Down Expand Up @@ -747,7 +747,7 @@ func RepoRefByType(refType RepoRefType) func(http.Handler) http.Handler {
refName = ctx.Repo.Repository.DefaultBranch
ctx.Repo.BranchName = refName
if !ctx.Repo.GitRepo.IsBranchExist(refName) {
brs, err := ctx.Repo.GitRepo.GetBranches()
brs, _, err := ctx.Repo.GitRepo.GetBranches(0, 0)
if err != nil {
ctx.ServerError("GetBranches", err)
return
Expand Down
11 changes: 6 additions & 5 deletions modules/git/repo_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,17 @@ func (repo *Repository) GetBranch(branch string) (*Branch, error) {
}

// GetBranchesByPath returns a branch by it's path
func GetBranchesByPath(path string) ([]*Branch, error) {
// if limit = 0 it will not limit
func GetBranchesByPath(path string, skip, limit int) ([]*Branch, int, error) {
gitRepo, err := OpenRepository(path)
if err != nil {
return nil, err
return nil, 0, err
}
defer gitRepo.Close()

brs, err := gitRepo.GetBranches()
brs, countAll, err := gitRepo.GetBranches(skip, limit)
if err != nil {
return nil, err
return nil, 0, err
}

branches := make([]*Branch, len(brs))
Expand All @@ -99,7 +100,7 @@ func GetBranchesByPath(path string) ([]*Branch, error) {
}
}

return branches, nil
return branches, countAll, nil
}

// DeleteBranchOptions Option(s) for delete branch
Expand Down
19 changes: 15 additions & 4 deletions modules/git/repo_branch_gogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,32 @@ func (repo *Repository) IsBranchExist(name string) bool {
return reference.Type() != plumbing.InvalidReference
}

// GetBranches returns all branches of the repository.
func (repo *Repository) GetBranches() ([]string, error) {
// GetBranches returns branches from the repository, skipping skip initial branches and
// returning at most limit branches, or all branches if limit is 0.
func (repo *Repository) GetBranches(skip, limit int) ([]string, int, error) {
var branchNames []string

branches, err := repo.gogitRepo.Branches()
if err != nil {
return nil, err
return nil, 0, err
}

i := 0
count := 0
_ = branches.ForEach(func(branch *plumbing.Reference) error {
count++
if i < skip {
i++
return nil
} else if limit != 0 && count > skip+limit {
return nil
}

branchNames = append(branchNames, strings.TrimPrefix(branch.Name().String(), BranchPrefix))
return nil
})

// TODO: Sort?

return branchNames, nil
return branchNames, count, nil
}
50 changes: 39 additions & 11 deletions modules/git/repo_branch_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ func (repo *Repository) IsBranchExist(name string) bool {
return IsReferenceExist(repo.Path, BranchPrefix+name)
}

// GetBranches returns all branches of the repository.
func (repo *Repository) GetBranches() ([]string, error) {
return callShowRef(repo.Path, BranchPrefix, "--heads")
// GetBranches returns branches from the repository, skipping skip initial branches and
// returning at most limit branches, or all branches if limit is 0.
func (repo *Repository) GetBranches(skip, limit int) ([]string, int, error) {
return callShowRef(repo.Path, BranchPrefix, "--heads", skip, limit)
}

func callShowRef(repoPath, prefix, arg string) ([]string, error) {
var branchNames []string

// callShowRef return refs, if limit = 0 it will not limit
func callShowRef(repoPath, prefix, arg string, skip, limit int) (branchNames []string, countAll int, err error) {
stdoutReader, stdoutWriter := io.Pipe()
defer func() {
_ = stdoutReader.Close()
Expand All @@ -49,8 +49,21 @@ func callShowRef(repoPath, prefix, arg string) ([]string, error) {
}
}()

i := 0
bufReader := bufio.NewReader(stdoutReader)
for {
for i < skip {
_, isPrefix, err := bufReader.ReadLine()
if err == io.EOF {
return branchNames, i, nil
}
if err != nil {
return nil, 0, err
}
if !isPrefix {
i++
}
}
for limit == 0 || i < skip+limit {
// The output of show-ref is simply a list:
// <sha> SP <ref> LF
_, err := bufReader.ReadSlice(' ')
Expand All @@ -59,24 +72,39 @@ func callShowRef(repoPath, prefix, arg string) ([]string, error) {
_, err = bufReader.ReadSlice(' ')
}
if err == io.EOF {
return branchNames, nil
return branchNames, i, nil
}
if err != nil {
return nil, err
return nil, 0, err
}

branchName, err := bufReader.ReadString('\n')
if err == io.EOF {
// This shouldn't happen... but we'll tolerate it for the sake of peace
return branchNames, nil
return branchNames, i, nil
}
if err != nil {
return nil, err
return nil, i, err
}
branchName = strings.TrimPrefix(branchName, prefix)
if len(branchName) > 0 {
branchName = branchName[:len(branchName)-1]
}
branchNames = append(branchNames, branchName)
i++
}
// count all refs
for limit != 0 {
_, isPrefix, err := bufReader.ReadLine()
if err == io.EOF {
return branchNames, i, nil
}
if err != nil {
return nil, 0, err
}
if !isPrefix {
i++
}
}
return branchNames, i, nil
}
19 changes: 17 additions & 2 deletions modules/git/repo_branch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,26 @@ func TestRepository_GetBranches(t *testing.T) {
assert.NoError(t, err)
defer bareRepo1.Close()

branches, err := bareRepo1.GetBranches()
branches, countAll, err := bareRepo1.GetBranches(0, 2)

assert.NoError(t, err)
assert.Len(t, branches, 2)
assert.EqualValues(t, 3, countAll)
assert.ElementsMatch(t, []string{"branch1", "branch2"}, branches)

branches, countAll, err = bareRepo1.GetBranches(0, 0)

assert.NoError(t, err)
assert.Len(t, branches, 3)
assert.EqualValues(t, 3, countAll)
assert.ElementsMatch(t, []string{"branch1", "branch2", "master"}, branches)

branches, countAll, err = bareRepo1.GetBranches(5, 1)

assert.NoError(t, err)
assert.Len(t, branches, 0)
assert.EqualValues(t, 3, countAll)
assert.ElementsMatch(t, []string{}, branches)
}

func BenchmarkRepository_GetBranches(b *testing.B) {
Expand All @@ -33,7 +48,7 @@ func BenchmarkRepository_GetBranches(b *testing.B) {
defer bareRepo1.Close()

for i := 0; i < b.N; i++ {
_, err := bareRepo1.GetBranches()
_, _, err := bareRepo1.GetBranches(0, 0)
if err != nil {
b.Fatal(err)
}
Expand Down
5 changes: 3 additions & 2 deletions modules/git/repo_tag_nogogit.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ func (repo *Repository) IsTagExist(name string) bool {
}

// GetTags returns all tags of the repository.
func (repo *Repository) GetTags() ([]string, error) {
return callShowRef(repo.Path, TagPrefix, "--tags")
func (repo *Repository) GetTags() (tags []string, err error) {
tags, _, err = callShowRef(repo.Path, TagPrefix, "--tags", 0, 0)
return
}
12 changes: 8 additions & 4 deletions modules/repository/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import (

// GetBranch returns a branch by its name
func GetBranch(repo *models.Repository, branch string) (*git.Branch, error) {
if len(branch) == 0 {
return nil, fmt.Errorf("GetBranch: empty string for branch")
}
gitRepo, err := git.OpenRepository(repo.RepoPath())
if err != nil {
return nil, err
Expand All @@ -22,9 +25,10 @@ func GetBranch(repo *models.Repository, branch string) (*git.Branch, error) {
return gitRepo.GetBranch(branch)
}

// GetBranches returns all the branches of a repository
func GetBranches(repo *models.Repository) ([]*git.Branch, error) {
return git.GetBranchesByPath(repo.RepoPath())
// GetBranches returns branches from the repository, skipping skip initial branches and
// returning at most limit branches, or all branches if limit is 0.
func GetBranches(repo *models.Repository, skip, limit int) ([]*git.Branch, int, error) {
return git.GetBranchesByPath(repo.RepoPath(), skip, limit)
}

// checkBranchName validates branch name with existing repository branches
Expand All @@ -35,7 +39,7 @@ func checkBranchName(repo *models.Repository, name string) error {
}
defer gitRepo.Close()

branches, err := GetBranches(repo)
branches, _, err := GetBranches(repo, 0, 0)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion modules/repository/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func adoptRepository(ctx models.DBContext, repoPath string, u *models.User, repo

repo.DefaultBranch = strings.TrimPrefix(repo.DefaultBranch, git.BranchPrefix)
}
branches, _ := gitRepo.GetBranches()
branches, _, _ := gitRepo.GetBranches(0, 0)
found := false
hasDefault := false
hasMaster := false
Expand Down
34 changes: 34 additions & 0 deletions modules/util/paginate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package util

import "reflect"

// PaginateSlice cut a slice as per pagination options
// if page = 0 it do not paginate
func PaginateSlice(list interface{}, page, pageSize int) interface{} {
if page <= 0 || pageSize <= 0 {
return list
}
listValue := reflect.ValueOf(list)

if reflect.TypeOf(list).Kind() != reflect.Slice {
return list
}
6543 marked this conversation as resolved.
Show resolved Hide resolved

page--

if page*pageSize >= listValue.Len() {
return listValue.Slice(listValue.Len(), listValue.Len()).Interface()
}

listValue = listValue.Slice(page*pageSize, listValue.Len())

if listValue.Len() > pageSize {
return listValue.Slice(0, pageSize).Interface()
}

return listValue.Interface()
}
47 changes: 47 additions & 0 deletions modules/util/paginate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package util

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestPaginateSlice(t *testing.T) {
stringSlice := []string{"a", "b", "c", "d", "e"}
result, ok := PaginateSlice(stringSlice, 1, 2).([]string)
assert.True(t, ok)
assert.EqualValues(t, []string{"a", "b"}, result)

result, ok = PaginateSlice(stringSlice, 100, 2).([]string)
assert.True(t, ok)
assert.EqualValues(t, []string{}, result)

result, ok = PaginateSlice(stringSlice, 3, 2).([]string)
assert.True(t, ok)
assert.EqualValues(t, []string{"e"}, result)

result, ok = PaginateSlice(stringSlice, 1, 0).([]string)
assert.True(t, ok)
assert.EqualValues(t, []string{"a", "b", "c", "d", "e"}, result)

result, ok = PaginateSlice(stringSlice, 1, -1).([]string)
assert.True(t, ok)
assert.EqualValues(t, []string{"a", "b", "c", "d", "e"}, result)

type Test struct {
Val int
}

var testVar = []*Test{{Val: 2}, {Val: 3}, {Val: 4}}
testVar, ok = PaginateSlice(testVar, 1, 50).([]*Test)
assert.True(t, ok)
assert.EqualValues(t, []*Test{{Val: 2}, {Val: 3}, {Val: 4}}, testVar)

testVar, ok = PaginateSlice(testVar, 2, 2).([]*Test)
assert.True(t, ok)
assert.EqualValues(t, []*Test{{Val: 4}}, testVar)
}
5 changes: 3 additions & 2 deletions routers/api/v1/org/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down
Loading