Skip to content

Commit aef1d49

Browse files
committed
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
1 parent 0cec4b8 commit aef1d49

File tree

31 files changed

+82
-82
lines changed

31 files changed

+82
-82
lines changed

models/issues/issue.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,7 @@ func (issue *Issue) BlockedByDependencies(ctx context.Context, opts db.ListOptio
690690
Where("issue_id = ?", issue.ID).
691691
// sort by repo id then created date, with the issues of the same repo at the beginning of the list
692692
OrderBy("CASE WHEN issue.repo_id = ? THEN 0 ELSE issue.repo_id END, issue.created_unix DESC", issue.RepoID)
693+
// Pagination bypass needed by ViewIssue => prepareIssueViewSidebarDependency
693694
if opts.Page > 0 {
694695
sess = db.SetSessionPagination(sess, &opts)
695696
}

models/issues/issue_watch.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,9 @@ func GetIssueWatchers(ctx context.Context, issueID int64, listOptions db.ListOpt
105105
And("`user`.prohibit_login = ?", false).
106106
Join("INNER", "`user`", "`user`.id = `issue_watch`.user_id")
107107

108-
if listOptions.Page > 0 {
109-
sess = db.SetSessionPagination(sess, &listOptions)
110-
watches := make([]*IssueWatch, 0, listOptions.PageSize)
111-
return watches, sess.Find(&watches)
112-
}
113-
watches := make([]*IssueWatch, 0, 8)
108+
listOptions.SetDefaultValues()
109+
sess = db.SetSessionPagination(sess, &listOptions)
110+
watches := make([]*IssueWatch, 0, listOptions.PageSize)
114111
return watches, sess.Find(&watches)
115112
}
116113

models/issues/label.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,7 @@ func GetLabelsByRepoID(ctx context.Context, repoID int64, sortType string, listO
408408
sess.Asc("name")
409409
}
410410

411+
// Pagination bypass used by some callers
411412
if listOptions.Page > 0 {
412413
sess = db.SetSessionPagination(sess, &listOptions)
413414
}
@@ -483,6 +484,7 @@ func GetLabelsByOrgID(ctx context.Context, orgID int64, sortType string, listOpt
483484
sess.Asc("name")
484485
}
485486

487+
// Why can we bypass limits here?
486488
if listOptions.Page > 0 {
487489
sess = db.SetSessionPagination(sess, &listOptions)
488490
}

models/issues/review.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ func GetCurrentReview(ctx context.Context, reviewer *user_model.User, issue *Iss
389389
if reviewer == nil {
390390
return nil, nil
391391
}
392+
// Missing db.ListOptionsAll
392393
reviews, err := FindReviews(ctx, FindReviewOptions{
393394
Types: []ReviewType{ReviewTypePending},
394395
IssueID: issue.ID,

models/issues/review_list.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ func (opts *FindReviewOptions) toCond() builder.Cond {
120120
func FindReviews(ctx context.Context, opts FindReviewOptions) (ReviewList, error) {
121121
reviews := make([]*Review, 0, 10)
122122
sess := db.GetEngine(ctx).Where(opts.toCond())
123+
// Pagination bypass used by some callers
123124
if opts.Page > 0 && !opts.IsListAll() {
124125
sess = db.SetSessionPagination(sess, &opts)
125126
}

models/issues/stopwatch.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ func GetUIDsAndStopwatch(ctx context.Context) ([]*UserStopwatch, error) {
9191
func GetUserStopwatches(ctx context.Context, userID int64, listOptions db.ListOptions) ([]*Stopwatch, error) {
9292
sws := make([]*Stopwatch, 0, 8)
9393
sess := db.GetEngine(ctx).Where("stopwatch.user_id = ?", userID)
94+
// Pagination bypass used by CancelStopwatch
9495
if listOptions.Page > 0 {
9596
sess = db.SetSessionPagination(sess, &listOptions)
9697
}

models/packages/package_version.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ type PackageSearchOptions struct {
187187
HasFileWithName string // only results are found which are associated with a file with the specific name
188188
HasFiles optional.Option[bool] // only results are found which have associated files
189189
Sort VersionSort
190+
// Only one with interface
190191
db.Paginator
191192
}
192193

models/repo/star.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ func IsStaring(ctx context.Context, userID, repoID int64) bool {
7979
func GetStargazers(ctx context.Context, repo *Repository, opts db.ListOptions) ([]*user_model.User, error) {
8080
sess := db.GetEngine(ctx).Where("star.repo_id = ?", repo.ID).
8181
Join("LEFT", "star", "`user`.id = star.uid")
82+
// Paging bypass used by UI
8283
if opts.Page > 0 {
8384
sess = db.SetSessionPagination(sess, &opts)
8485

models/repo/watch.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -151,14 +151,9 @@ func GetRepoWatchers(ctx context.Context, repoID int64, opts db.ListOptions) ([]
151151
sess := db.GetEngine(ctx).Where("watch.repo_id=?", repoID).
152152
Join("LEFT", "watch", "`user`.id=`watch`.user_id").
153153
And("`watch`.mode<>?", WatchModeDont)
154-
if opts.Page > 0 {
155-
sess = db.SetSessionPagination(sess, &opts)
156-
users := make([]*user_model.User, 0, opts.PageSize)
154+
sess = db.SetSessionPagination(sess, &opts)
155+
users := make([]*user_model.User, 0, opts.PageSize)
157156

158-
return users, sess.Find(&users)
159-
}
160-
161-
users := make([]*user_model.User, 0, 8)
162157
return users, sess.Find(&users)
163158
}
164159

models/user/search.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ func SearchUsers(ctx context.Context, opts SearchUserOptions) (users []*User, _
151151

152152
sessQuery := opts.toSearchQueryBase(ctx).OrderBy(opts.OrderBy.String())
153153
defer sessQuery.Close()
154+
// Pagination bypass used by UI
154155
if opts.Page > 0 {
155156
sessQuery = db.SetSessionPagination(sessQuery, &opts)
156157
}

0 commit comments

Comments
 (0)