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 <lauris@nix.lv>
This commit is contained in:
Cirno the Strongest 2020-06-13 13:35:59 +02:00 committed by GitHub
parent 2447ffc74a
commit 0159851cc3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 48 additions and 29 deletions

View file

@ -35,6 +35,7 @@ import (
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
"github.com/unknwon/com" "github.com/unknwon/com"
"xorm.io/builder"
) )
var ( var (
@ -1774,22 +1775,28 @@ func GetRepositoriesMapByIDs(ids []int64) (map[int64]*Repository, error) {
} }
// GetUserRepositories returns a list of repositories of given user. // 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 { if len(opts.OrderBy) == 0 {
opts.OrderBy = "updated_unix DESC" opts.OrderBy = "updated_unix DESC"
} }
sess := x. var cond = builder.NewCond()
Where("owner_id = ?", opts.Actor.ID). cond = cond.And(builder.Eq{"owner_id": opts.Actor.ID})
OrderBy(opts.OrderBy.String())
if !opts.Private { 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) 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. // GetUserMirrorRepositories returns a list of mirror repositories of given user.

View file

@ -646,7 +646,7 @@ func (u *User) GetOrganizationCount() (int64, error) {
// GetRepositories returns repositories that user owns, including private repositories. // GetRepositories returns repositories that user owns, including private repositories.
func (u *User) GetRepositories(listOpts ListOptions) (err error) { 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 return err
} }

View file

@ -6,6 +6,7 @@ package user
import ( import (
"net/http" "net/http"
"strconv"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/context"
@ -15,10 +16,12 @@ import (
// listUserRepos - List the repositories owned by the given user. // listUserRepos - List the repositories owned by the given user.
func listUserRepos(ctx *context.APIContext, u *models.User, private bool) { 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, Actor: u,
Private: private, Private: private,
ListOptions: utils.GetListOptions(ctx), ListOptions: opts,
}) })
if err != nil { if err != nil {
ctx.Error(http.StatusInternalServerError, "GetUserRepositories", err) 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)) 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) ctx.JSON(http.StatusOK, &apiRepos)
} }
@ -92,31 +98,37 @@ func ListMyRepos(ctx *context.APIContext) {
// "200": // "200":
// "$ref": "#/responses/RepositoryList" // "$ref": "#/responses/RepositoryList"
ownRepos, err := models.GetUserRepositories(&models.SearchRepoOptions{ opts := &models.SearchRepoOptions{
Actor: ctx.User, ListOptions: utils.GetListOptions(ctx),
Private: true, Actor: ctx.User,
ListOptions: utils.GetListOptions(ctx), OwnerID: ctx.User.ID,
}) Private: ctx.IsSigned,
if err != nil { IncludeDescription: true,
ctx.Error(http.StatusInternalServerError, "GetUserRepositories", err)
return
} }
accessibleReposMap, err := ctx.User.GetRepositoryAccesses()
var err error
repos, count, err := models.SearchRepository(opts)
if err != nil { if err != nil {
ctx.Error(http.StatusInternalServerError, "GetRepositoryAccesses", err) ctx.Error(http.StatusInternalServerError, "SearchRepository", err)
return return
} }
apiRepos := make([]*api.Repository, len(ownRepos)+len(accessibleReposMap)) results := make([]*api.Repository, len(repos))
for i := range ownRepos { for i, repo := range repos {
apiRepos[i] = ownRepos[i].APIFormat(models.AccessModeOwner) 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 { ctx.SetLinkHeader(int(count), opts.ListOptions.PageSize)
apiRepos[i] = repo.APIFormat(access) ctx.Header().Set("X-Total-Count", strconv.FormatInt(count, 10))
i++ ctx.JSON(http.StatusOK, &results)
}
ctx.JSON(http.StatusOK, &apiRepos)
} }
// ListOrgRepos - list the repositories of an organization. // ListOrgRepos - list the repositories of an organization.