Reduce paging inconsistency

* disallow bypassing configured limits in api
* allow per_page without documenting it
* allow limit for the only endpoint only allowing per_page

This is an paging inconsistency api review, please comment your opinion about the added code comments

* 	ctx.SetLinkHeader(int(totalNumOfBranches), listOptions.PageSize) missing for several api endpoints should I add them everywhere?

* do we want to accept per_page in api?
  * better compatibility with existing keda github_runner

models/issues/issue.go

@@ -690,6 +690,7 @@ func (issue *Issue) BlockedByDependencies(ctx context.Context, opts db.ListOptio
 		Where("issue_id = ?", issue.ID).
 		// sort by repo id then created date, with the issues of the same repo at the beginning of the list
 		OrderBy("CASE WHEN issue.repo_id = ? THEN 0 ELSE issue.repo_id END, issue.created_unix DESC", issue.RepoID)
+	// Pagination bypass needed by ViewIssue => prepareIssueViewSidebarDependency
 	if opts.Page > 0 {
 		sess = db.SetSessionPagination(sess, &opts)
 	}

models/issues/issue_watch.go

@@ -105,14 +105,11 @@ func GetIssueWatchers(ctx context.Context, issueID int64, listOptions db.ListOpt
 		And("`user`.prohibit_login = ?", false).
 		Join("INNER", "`user`", "`user`.id = `issue_watch`.user_id")
 
-	if listOptions.Page > 0 {
+	listOptions.SetDefaultValues()
 	sess = db.SetSessionPagination(sess, &listOptions)
 	watches := make([]*IssueWatch, 0, listOptions.PageSize)
 	return watches, sess.Find(&watches)
 }
-	watches := make([]*IssueWatch, 0, 8)
-	return watches, sess.Find(&watches)
-}
 
 // CountIssueWatchers count watchers/unwatchers of a given issue
 func CountIssueWatchers(ctx context.Context, issueID int64) (int64, error) {

models/issues/label.go

@@ -408,6 +408,7 @@ func GetLabelsByRepoID(ctx context.Context, repoID int64, sortType string, listO
 		sess.Asc("name")
 	}
 
+	// Pagination bypass used by some callers
 	if listOptions.Page > 0 {
 		sess = db.SetSessionPagination(sess, &listOptions)
 	}
@@ -483,6 +484,7 @@ func GetLabelsByOrgID(ctx context.Context, orgID int64, sortType string, listOpt
 		sess.Asc("name")
 	}
 
+	// Why can we bypass limits here?
 	if listOptions.Page > 0 {
 		sess = db.SetSessionPagination(sess, &listOptions)
 	}

models/issues/review.go

@@ -389,6 +389,7 @@ func GetCurrentReview(ctx context.Context, reviewer *user_model.User, issue *Iss
 	if reviewer == nil {
 		return nil, nil
 	}
+	// Missing db.ListOptionsAll
 	reviews, err := FindReviews(ctx, FindReviewOptions{
 		Types:      []ReviewType{ReviewTypePending},
 		IssueID:    issue.ID,

models/issues/review_list.go

@@ -120,6 +120,7 @@ func (opts *FindReviewOptions) toCond() builder.Cond {
 func FindReviews(ctx context.Context, opts FindReviewOptions) (ReviewList, error) {
 	reviews := make([]*Review, 0, 10)
 	sess := db.GetEngine(ctx).Where(opts.toCond())
+	// Pagination bypass used by some callers
 	if opts.Page > 0 && !opts.IsListAll() {
 		sess = db.SetSessionPagination(sess, &opts)
 	}

models/issues/stopwatch.go

@@ -91,6 +91,7 @@ func GetUIDsAndStopwatch(ctx context.Context) ([]*UserStopwatch, error) {
 func GetUserStopwatches(ctx context.Context, userID int64, listOptions db.ListOptions) ([]*Stopwatch, error) {
 	sws := make([]*Stopwatch, 0, 8)
 	sess := db.GetEngine(ctx).Where("stopwatch.user_id = ?", userID)
+	// Pagination bypass used by CancelStopwatch
 	if listOptions.Page > 0 {
 		sess = db.SetSessionPagination(sess, &listOptions)
 	}

models/packages/package_version.go

@@ -187,6 +187,7 @@ type PackageSearchOptions struct {
 	HasFileWithName string                // only results are found which are associated with a file with the specific name
 	HasFiles        optional.Option[bool] // only results are found which have associated files
 	Sort            VersionSort
+	// Only one with interface
 	db.Paginator
 }
 

models/repo/star.go

@@ -79,6 +79,7 @@ func IsStaring(ctx context.Context, userID, repoID int64) bool {
 func GetStargazers(ctx context.Context, repo *Repository, opts db.ListOptions) ([]*user_model.User, error) {
 	sess := db.GetEngine(ctx).Where("star.repo_id = ?", repo.ID).
 		Join("LEFT", "star", "`user`.id = star.uid")
+	// Paging bypass used by UI
 	if opts.Page > 0 {
 		sess = db.SetSessionPagination(sess, &opts)
 

models/repo/watch.go

@@ -151,17 +151,12 @@ func GetRepoWatchers(ctx context.Context, repoID int64, opts db.ListOptions) ([]
 	sess := db.GetEngine(ctx).Where("watch.repo_id=?", repoID).
 		Join("LEFT", "watch", "`user`.id=`watch`.user_id").
 		And("`watch`.mode<>?", WatchModeDont)
-	if opts.Page > 0 {
 	sess = db.SetSessionPagination(sess, &opts)
 	users := make([]*user_model.User, 0, opts.PageSize)
 
 	return users, sess.Find(&users)
 }
 
-	users := make([]*user_model.User, 0, 8)
-	return users, sess.Find(&users)
-}
-
 // WatchIfAuto subscribes to repo if AutoWatchOnChanges is set
 func WatchIfAuto(ctx context.Context, userID, repoID int64, isWrite bool) error {
 	if !isWrite || !setting.Service.AutoWatchOnChanges {

models/user/search.go

@@ -151,6 +151,7 @@ func SearchUsers(ctx context.Context, opts SearchUserOptions) (users []*User, _
 
 	sessQuery := opts.toSearchQueryBase(ctx).OrderBy(opts.OrderBy.String())
 	defer sessQuery.Close()
+	// Pagination bypass used by UI
 	if opts.Page > 0 {
 		sessQuery = db.SetSessionPagination(sessQuery, &opts)
 	}

models/user/user.go

@@ -321,7 +321,6 @@ func GetUserFollowers(ctx context.Context, u, viewer *User, listOptions db.ListO
 		And("`user`.type=?", UserTypeIndividual).
 		And(isUserVisibleToViewerCond(viewer))
 
-	if listOptions.Page > 0 {
 	sess = db.SetSessionPagination(sess, &listOptions)
 
 	users := make([]*User, 0, listOptions.PageSize)
@@ -329,11 +328,6 @@ func GetUserFollowers(ctx context.Context, u, viewer *User, listOptions db.ListO
 	return users, count, err
 }
 
-	users := make([]*User, 0, 8)
-	count, err := sess.FindAndCount(&users)
-	return users, count, err
-}
-
 // GetUserFollowing returns range of user's following.
 func GetUserFollowing(ctx context.Context, u, viewer *User, listOptions db.ListOptions) ([]*User, int64, error) {
 	sess := db.GetEngine(ctx).
@@ -343,6 +337,7 @@ func GetUserFollowing(ctx context.Context, u, viewer *User, listOptions db.ListO
 		And("`user`.type IN (?, ?)", UserTypeIndividual, UserTypeOrganization).
 		And(isUserVisibleToViewerCond(viewer))
 
+	// pagination bypass, otherwise 8
 	if listOptions.Page > 0 {
 		sess = db.SetSessionPagination(sess, &listOptions)
 

modules/git/repo_tag.go

@@ -147,6 +147,7 @@ func (repo *Repository) GetTagInfos(page, pageSize int) ([]*Tag, int, error) {
 
 	sortTagsByTime(tags)
 	tagsTotal := len(tags)
+	// pagination bypass
 	if page != 0 {
 		tags = util.PaginateSlice(tags, page, pageSize).([]*Tag)
 	}

routers/api/v1/admin/adopt.go

@@ -41,9 +41,7 @@ func ListUnadoptedRepositories(ctx *context.APIContext) {
 	//     "$ref": "#/responses/forbidden"
 
 	listOptions := utils.GetListOptions(ctx)
-	if listOptions.Page == 0 {
+	listOptions.SetDefaultValues()
-		listOptions.Page = 1
-	}
 	repoNames, count, err := repo_service.ListUnadoptedRepositories(ctx, ctx.FormString("query"), &listOptions)
 	if err != nil {
 		ctx.APIErrorInternal(err)

routers/api/v1/admin/cron.go

@@ -39,6 +39,7 @@ func ListCronTasks(ctx *context.APIContext) {
 	count := len(tasks)
 
 	listOpts := utils.GetListOptions(ctx)
+	listOpts.SetDefaultValues()
 	tasks = util.PaginateSlice(tasks, listOpts.Page, listOpts.PageSize).(cron.TaskTable)
 
 	res := make([]structs.Cron, len(tasks))

routers/api/v1/admin/org.go

@@ -100,6 +100,8 @@ func GetAllOrgs(ctx *context.APIContext) {
 	//     "$ref": "#/responses/forbidden"
 
 	listOptions := utils.GetListOptions(ctx)
+	// SearchUsers allows pagination bypass
+	listOptions.SetDefaultValues()
 
 	users, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
 		Actor:       ctx.Doer,

routers/api/v1/admin/user.go

@@ -422,6 +422,8 @@ func SearchUsers(ctx *context.APIContext) {
 	//     "$ref": "#/responses/forbidden"
 
 	listOptions := utils.GetListOptions(ctx)
+	// SearchUsers allows pagination bypass
+	listOptions.SetDefaultValues()
 
 	users, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
 		Actor:       ctx.Doer,

routers/api/v1/org/org.go

@@ -200,6 +200,8 @@ func GetAll(ctx *context.APIContext) {
 	}
 
 	listOptions := utils.GetListOptions(ctx)
+	// SearchUsers allows pagination bypass
+	listOptions.SetDefaultValues()
 
 	publicOrgs, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
 		Actor:       ctx.Doer,

routers/api/v1/repo/commits.go

@@ -184,6 +184,7 @@ func GetAllCommits(ctx *context.APIContext) {
 		return
 	}
 
+	// Unusual use of ListOptions, but we need to support pagination
 	listOptions := utils.GetListOptions(ctx)
 	if listOptions.Page <= 0 {
 		listOptions.Page = 1

routers/api/v1/repo/issue.go

@@ -21,7 +21,6 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	issue_indexer "code.gitea.io/gitea/modules/indexer/issues"
 	"code.gitea.io/gitea/modules/optional"
-	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/web"
@@ -256,20 +255,12 @@ func SearchIssues(ctx *context.APIContext) {
 		}
 	}
 
-	// this api is also used in UI,
+	opts := utils.GetListOptions(ctx)
-	// so the default limit is set to fit UI needs
+	// field opts.PageSize is used below
-	limit := ctx.FormInt("limit")
+	opts.SetDefaultValues()
-	if limit == 0 {
-		limit = setting.UI.IssuePagingNum
-	} else if limit > setting.API.MaxResponseItems {
-		limit = setting.API.MaxResponseItems
-	}
 
 	searchOpt := &issue_indexer.SearchOptions{
-		Paginator: &db.ListOptions{
+		Paginator:           &opts,
-			PageSize: limit,
-			Page:     ctx.FormInt("page"),
-		},
 		Keyword:             keyword,
 		RepoIDs:             repoIDs,
 		AllPublic:           allPublic,
@@ -321,7 +312,7 @@ func SearchIssues(ctx *context.APIContext) {
 		return
 	}
 
-	ctx.SetLinkHeader(int(total), limit)
+	ctx.SetLinkHeader(int(total), opts.PageSize)
 	ctx.SetTotalCountHeader(total)
 	ctx.JSON(http.StatusOK, convert.ToAPIIssueList(ctx, ctx.Doer, issues))
 }

routers/api/v1/repo/issue_dependency.go

@@ -7,13 +7,13 @@ package repo
 import (
 	"net/http"
 
-	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	access_model "code.gitea.io/gitea/models/perm/access"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/routers/api/v1/utils"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/convert"
 )
@@ -77,26 +77,16 @@ func GetIssueDependencies(ctx *context.APIContext) {
 		return
 	}
 
-	page := ctx.FormInt("page")
+	opts := utils.GetListOptions(ctx)
-	if page <= 1 {
+	// field opts.PageSize is used below
-		page = 1
+	opts.SetDefaultValues()
-	}
-	limit := ctx.FormInt("limit")
-	if limit == 0 {
-		limit = setting.API.DefaultPagingNum
-	} else if limit > setting.API.MaxResponseItems {
-		limit = setting.API.MaxResponseItems
-	}
 
 	canWrite := ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull)
 
-	blockerIssues := make([]*issues_model.Issue, 0, limit)
+	blockerIssues := make([]*issues_model.Issue, 0, opts.PageSize)
 
 	// 2. Get the issues this issue depends on, i.e. the `<#b>`: `<issue> <- <#b>`
-	blockersInfo, err := issue.BlockedByDependencies(ctx, db.ListOptions{
+	blockersInfo, err := issue.BlockedByDependencies(ctx, opts)
-		Page:     page,
-		PageSize: limit,
-	})
 	if err != nil {
 		ctx.APIErrorInternal(err)
 		return
@@ -328,17 +318,10 @@ func GetIssueBlocks(ctx *context.APIContext) {
 		return
 	}
 
-	page := ctx.FormInt("page")
+	opts := utils.GetListOptions(ctx)
-	if page <= 1 {
-		page = 1
-	}
-	limit := ctx.FormInt("limit")
-	if limit <= 1 {
-		limit = setting.API.DefaultPagingNum
-	}
 
-	skip := (page - 1) * limit
+	skip, limit := opts.GetSkipTake()
-	maxNum := page * limit
+	maxNum := skip + limit
 
 	deps, err := issue.BlockingDependencies(ctx)
 	if err != nil {

routers/api/v1/repo/label.go

@@ -49,7 +49,11 @@ func ListLabels(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	labels, err := issues_model.GetLabelsByRepoID(ctx, ctx.Repo.Repository.ID, ctx.FormString("sort"), utils.GetListOptions(ctx))
+	opts := utils.GetListOptions(ctx)
+	// GetLabelsByRepoID allows pagination bypass
+	opts.SetDefaultValues()
+
+	labels, err := issues_model.GetLabelsByRepoID(ctx, ctx.Repo.Repository.ID, ctx.FormString("sort"), opts)
 	if err != nil {
 		ctx.APIErrorInternal(err)
 		return

routers/api/v1/repo/mirror.go

@@ -165,8 +165,11 @@ func ListPushMirrors(ctx *context.APIContext) {
 	}
 
 	repo := ctx.Repo.Repository
+	opts := utils.GetListOptions(ctx)
+	// field opts.PageSize is used below
+	opts.SetDefaultValues()
 	// Get all push mirrors for the specified repository.
-	pushMirrors, count, err := repo_model.GetPushMirrorsByRepoID(ctx, repo.ID, utils.GetListOptions(ctx))
+	pushMirrors, count, err := repo_model.GetPushMirrorsByRepoID(ctx, repo.ID, opts)
 	if err != nil {
 		ctx.APIError(http.StatusNotFound, err)
 		return
@@ -179,7 +182,7 @@ func ListPushMirrors(ctx *context.APIContext) {
 			responsePushMirrors = append(responsePushMirrors, m)
 		}
 	}
-	ctx.SetLinkHeader(len(responsePushMirrors), utils.GetListOptions(ctx).PageSize)
+	ctx.SetLinkHeader(len(responsePushMirrors), opts.PageSize)
 	ctx.SetTotalCountHeader(count)
 	ctx.JSON(http.StatusOK, responsePushMirrors)
 }

routers/api/v1/repo/pull_review.go

@@ -85,6 +85,8 @@ func ListPullReviews(ctx *context.APIContext) {
 		ListOptions: utils.GetListOptions(ctx),
 		IssueID:     pr.IssueID,
 	}
+	// FindReviews allows pagination bypass
+	opts.SetDefaultValues()
 
 	allReviews, err := issues_model.FindReviews(ctx, opts)
 	if err != nil {

routers/api/v1/repo/star.go

@@ -47,7 +47,11 @@ func ListStargazers(ctx *context.APIContext) {
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
 
-	stargazers, err := repo_model.GetStargazers(ctx, ctx.Repo.Repository, utils.GetListOptions(ctx))
+	opts := utils.GetListOptions(ctx)
+	// GetStargazers allows pagination bypass
+	opts.SetDefaultValues()
+
+	stargazers, err := repo_model.GetStargazers(ctx, ctx.Repo.Repository, opts)
 	if err != nil {
 		ctx.APIErrorInternal(err)
 		return

routers/api/v1/repo/tag.go

@@ -54,6 +54,8 @@ func ListTags(ctx *context.APIContext) {
 	//     "$ref": "#/responses/notFound"
 
 	listOpts := utils.GetListOptions(ctx)
+	// GetTagInfos allows pagination bypass
+	listOpts.SetDefaultValues()
 
 	tags, total, err := ctx.Repo.GitRepo.GetTagInfos(listOpts.Page, listOpts.PageSize)
 	if err != nil {

routers/api/v1/repo/tree.go

@@ -6,6 +6,7 @@ package repo
 import (
 	"net/http"
 
+	"code.gitea.io/gitea/routers/api/v1/utils"
 	"code.gitea.io/gitea/services/context"
 	files_service "code.gitea.io/gitea/services/repository/files"
 )
@@ -61,7 +62,9 @@ func GetTree(ctx *context.APIContext) {
 		ctx.APIError(http.StatusBadRequest, "sha not provided")
 		return
 	}
-	if tree, err := files_service.GetTreeBySHA(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, sha, ctx.FormInt("page"), ctx.FormInt("per_page"), ctx.FormBool("recursive")); err != nil {
+	opts := utils.GetListOptions(ctx)
+	opts.SetDefaultValues()
+	if tree, err := files_service.GetTreeBySHA(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, sha, opts.Page, opts.PageSize, ctx.FormBool("recursive")); err != nil {
 		ctx.APIError(http.StatusBadRequest, err.Error())
 	} else {
 		ctx.SetTotalCountHeader(int64(tree.TotalCount))

routers/api/v1/repo/wiki.go

@@ -16,6 +16,7 @@ import (
 	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/utils"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/convert"
 	notify_service "code.gitea.io/gitea/services/notify"
@@ -298,17 +299,9 @@ func ListWikiPages(ctx *context.APIContext) {
 		return
 	}
 
-	page := ctx.FormInt("page")
+	opts := utils.GetListOptions(ctx)
-	if page <= 1 {
+	skip, limit := opts.GetSkipTake()
-		page = 1
+	maxNum := skip + limit
-	}
-	limit := ctx.FormInt("limit")
-	if limit <= 1 {
-		limit = setting.API.DefaultPagingNum
-	}
-
-	skip := (page - 1) * limit
-	maxNum := page * limit
 
 	entries, err := commit.ListEntries()
 	if err != nil {

routers/api/v1/user/follower.go

@@ -88,7 +88,10 @@ func ListFollowers(ctx *context.APIContext) {
 }
 
 func listUserFollowing(ctx *context.APIContext, u *user_model.User) {
-	users, count, err := user_model.GetUserFollowing(ctx, u, ctx.Doer, utils.GetListOptions(ctx))
+	opts := utils.GetListOptions(ctx)
+	// GetUserFollowing allows pagination bypass
+	opts.SetDefaultValues()
+	users, count, err := user_model.GetUserFollowing(ctx, u, ctx.Doer, opts)
 	if err != nil {
 		ctx.APIErrorInternal(err)
 		return

routers/api/v1/user/user.go

@@ -55,6 +55,8 @@ func Search(ctx *context.APIContext) {
 	//             "$ref": "#/definitions/User"
 
 	listOptions := utils.GetListOptions(ctx)
+	// SearchUsers allows pagination bypass
+	listOptions.SetDefaultValues()
 
 	uid := ctx.FormInt64("uid")
 	var users []*user_model.User

routers/api/v1/utils/page.go

@@ -11,8 +11,14 @@ import (
 
 // GetListOptions returns list options using the page and limit parameters
 func GetListOptions(ctx *context.APIContext) db.ListOptions {
+	limit := ctx.FormInt("limit")
+	if limit == 0 {
+		// Compatibility with foreign API clients that used "per_page" instead of "limit"
+		limit = ctx.FormInt("per_page")
+	}
+
 	return db.ListOptions{
 		Page:     ctx.FormInt("page"),
-		PageSize: convert.ToCorrectPageSize(ctx.FormInt("limit")),
+		PageSize: convert.ToCorrectPageSize(limit),
 	}
 }

services/pull/review.go

@@ -428,6 +428,7 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string,
 	}
 
 	if dismissPriors {
+		// missing db.ListOptionsAll
 		reviews, err := issues_model.FindReviews(ctx, issues_model.FindReviewOptions{
 			IssueID:    review.IssueID,
 			ReviewerID: review.ReviewerID,