From 0159851cc3fa80e4df4908a5e760afa20452f712 Mon Sep 17 00:00:00 2001 From: Cirno the Strongest <1447794+CirnoT@users.noreply.github.com> Date: Sat, 13 Jun 2020 13:35:59 +0200 Subject: [PATCH] Rework api/user/repos for pagination (#11827) * Add count to `GetUserRepositories` so that pagination can be supported for `/user/{username}/repos` * Rework ListMyRepos to use models.SearchRepository ListMyRepos was an odd one. It first fetched all user repositories and then tried to supplement them with accessible map. The end result was that: * Limit for pagination did not work because accessible repos would always be appended * The amount of pages was incorrect if one were to calculate it * When paginating, all accessible repos would be shown on every page Hopefully it should now work properly. Fixes #11800 and does not require any change on Drone-side as it can properly interpret and act on Link header which we now set. Co-authored-by: Lauris BH --- models/repo.go | 21 ++++++++++----- models/user.go | 2 +- routers/api/v1/user/repo.go | 54 ++++++++++++++++++++++--------------- 3 files changed, 48 insertions(+), 29 deletions(-) diff --git a/models/repo.go b/models/repo.go index bf7bf018a1..3b874f3359 100644 --- a/models/repo.go +++ b/models/repo.go @@ -35,6 +35,7 @@ import ( "code.gitea.io/gitea/modules/util" "github.com/unknwon/com" + "xorm.io/builder" ) var ( @@ -1774,22 +1775,28 @@ func GetRepositoriesMapByIDs(ids []int64) (map[int64]*Repository, error) { } // GetUserRepositories returns a list of repositories of given user. -func GetUserRepositories(opts *SearchRepoOptions) ([]*Repository, error) { +func GetUserRepositories(opts *SearchRepoOptions) ([]*Repository, int64, error) { if len(opts.OrderBy) == 0 { opts.OrderBy = "updated_unix DESC" } - sess := x. - Where("owner_id = ?", opts.Actor.ID). - OrderBy(opts.OrderBy.String()) + var cond = builder.NewCond() + cond = cond.And(builder.Eq{"owner_id": opts.Actor.ID}) if !opts.Private { - sess.And("is_private=?", false) + cond = cond.And(builder.Eq{"is_private": false}) } - sess = opts.setSessionPagination(sess) + sess := x.NewSession() + defer sess.Close() + count, err := sess.Where(cond).Count(new(Repository)) + if err != nil { + return nil, 0, fmt.Errorf("Count: %v", err) + } + + sess.Where(cond).OrderBy(opts.OrderBy.String()) repos := make([]*Repository, 0, opts.PageSize) - return repos, opts.setSessionPagination(sess).Find(&repos) + return repos, count, opts.setSessionPagination(sess).Find(&repos) } // GetUserMirrorRepositories returns a list of mirror repositories of given user. diff --git a/models/user.go b/models/user.go index 0ecb1b9a48..8056e30cea 100644 --- a/models/user.go +++ b/models/user.go @@ -646,7 +646,7 @@ func (u *User) GetOrganizationCount() (int64, error) { // GetRepositories returns repositories that user owns, including private repositories. func (u *User) GetRepositories(listOpts ListOptions) (err error) { - u.Repos, err = GetUserRepositories(&SearchRepoOptions{Actor: u, Private: true, ListOptions: listOpts}) + u.Repos, _, err = GetUserRepositories(&SearchRepoOptions{Actor: u, Private: true, ListOptions: listOpts}) return err } diff --git a/routers/api/v1/user/repo.go b/routers/api/v1/user/repo.go index 887d606998..519a999a98 100644 --- a/routers/api/v1/user/repo.go +++ b/routers/api/v1/user/repo.go @@ -6,6 +6,7 @@ package user import ( "net/http" + "strconv" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" @@ -15,10 +16,12 @@ import ( // listUserRepos - List the repositories owned by the given user. func listUserRepos(ctx *context.APIContext, u *models.User, private bool) { - repos, err := models.GetUserRepositories(&models.SearchRepoOptions{ + opts := utils.GetListOptions(ctx) + + repos, count, err := models.GetUserRepositories(&models.SearchRepoOptions{ Actor: u, Private: private, - ListOptions: utils.GetListOptions(ctx), + ListOptions: opts, }) if err != nil { ctx.Error(http.StatusInternalServerError, "GetUserRepositories", err) @@ -36,6 +39,9 @@ func listUserRepos(ctx *context.APIContext, u *models.User, private bool) { apiRepos = append(apiRepos, repos[i].APIFormat(access)) } } + + ctx.SetLinkHeader(int(count), opts.PageSize) + ctx.Header().Set("X-Total-Count", strconv.FormatInt(count, 10)) ctx.JSON(http.StatusOK, &apiRepos) } @@ -92,31 +98,37 @@ func ListMyRepos(ctx *context.APIContext) { // "200": // "$ref": "#/responses/RepositoryList" - ownRepos, err := models.GetUserRepositories(&models.SearchRepoOptions{ - Actor: ctx.User, - Private: true, - ListOptions: utils.GetListOptions(ctx), - }) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetUserRepositories", err) - return + opts := &models.SearchRepoOptions{ + ListOptions: utils.GetListOptions(ctx), + Actor: ctx.User, + OwnerID: ctx.User.ID, + Private: ctx.IsSigned, + IncludeDescription: true, } - accessibleReposMap, err := ctx.User.GetRepositoryAccesses() + + var err error + repos, count, err := models.SearchRepository(opts) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetRepositoryAccesses", err) + ctx.Error(http.StatusInternalServerError, "SearchRepository", err) return } - apiRepos := make([]*api.Repository, len(ownRepos)+len(accessibleReposMap)) - for i := range ownRepos { - apiRepos[i] = ownRepos[i].APIFormat(models.AccessModeOwner) + results := make([]*api.Repository, len(repos)) + for i, repo := range repos { + if err = repo.GetOwner(); err != nil { + ctx.Error(http.StatusInternalServerError, "GetOwner", err) + return + } + accessMode, err := models.AccessLevel(ctx.User, repo) + if err != nil { + ctx.Error(http.StatusInternalServerError, "AccessLevel", err) + } + results[i] = repo.APIFormat(accessMode) } - i := len(ownRepos) - for repo, access := range accessibleReposMap { - apiRepos[i] = repo.APIFormat(access) - i++ - } - ctx.JSON(http.StatusOK, &apiRepos) + + ctx.SetLinkHeader(int(count), opts.ListOptions.PageSize) + ctx.Header().Set("X-Total-Count", strconv.FormatInt(count, 10)) + ctx.JSON(http.StatusOK, &results) } // ListOrgRepos - list the repositories of an organization.