API add/generalize pagination

[hilariocoelho]

This PR adds pagination to List requests. It should be merged along with gitea/go-sdk#203.

It is backwards compatible, no breaking changes found. Unit tests and integration tests are all successful without making any significant change.

[codecov-io]

Codecov Report

Merging #9452 into master will increase coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #9452      +/-   ##
==========================================
+ Coverage   42.18%   42.24%   +0.05%     
==========================================
  Files         610      610              
  Lines       80274    80274              
==========================================
+ Hits        33865    33909      +44     
+ Misses      42249    42202      -47     
- Partials     4160     4163       +3

Impacted Files	Coverage Δ
modules/notification/indexer/indexer.go	55.07% <100%> (+4.34%)	⬆️
models/repo.go	49.69% <0%> (-0.14%)	⬇️
services/pull/patch.go	67.92% <0%> (+1.25%)	⬆️
modules/git/tree_entry.go	73.33% <0%> (+1.66%)	⬆️
modules/indexer/code/bleve.go	66.07% <0%> (+4.4%)	⬆️
models/repo_indexer.go	70.58% <0%> (+7.84%)	⬆️
modules/indexer/code/git.go	69.14% <0%> (+26.59%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 465c955...f7a5c9f. Read the comment docs.

[zeripath]

Great work. This is a huge PR!

Some comments:

• If you are passed in an engine you cannot use x. You MUST use the engine. The engine is often a session in a transaction and you must do the queries within that transaction. This is why my comment is request changes.
• Paging will often require counts to precalculate the number of pages or else at least some way of telling if there is another page - requesting one more entity may be a way of doing that if it's not too expensive. The UI needs to be considered.
• AFAICS this adds mandatory paging to large parts of the API - this likely constitutes a breaking change. We can make this non breaking by making paging non mandatory but that likely obviates some of the benefits. I'm not sure and it needs discussion.

In future if we do want to provide all results we should stream the results to the ctx.Writer likely with some internal batching of requests.

[hilariocoelho]

Like spoken on Discord channel, this PR doesn't make any breaking change. Although this is a quick implementation of pagination to Gitea's API.

API will return all results to requests that are sent without a page field greater than 0. If the request does send that field, then Gitea uses the pagination implemented on this PR.

As a future work the API should be changed in order to add a has_more_pages field to responses when using pagination. The approach I suggest in order to add that has_more_pages field is to get always N+1 requested fields, validates if the result contains N+1, return N and the bool has_more_pages.
It would be something like the following:

// getAPIResponse returns the requested stuff and a "has_more_pages" bool
func getAPIResponse() ([]Stuff, bool) {
	res, err := models.GetStuff(page, pageSize)
	
	apiRes := res[:len(res) -1]
	hasMorePages = len(res) == pageSize+1
	return apiRes, hasMorePages
}

For now I think we can add the currently implemented pagination to Gitea v1 and the has_more_pages implementation goes to Gitea v2.

It is also important to synchronize these changes with gitea/go-sdk#203. Will work on that as soon as this is ready to be merged.

[zeripath]

Please add page to /api/v1/users/search

[zeripath]

make lg-tm work

[zeripath]

make lg-tm work

[guillep2k]

Thank you for your hard work!!!

[bjj]

This breaks drone.io integration. When it fetches /api/user/repos it triggers pagination and only gets the first 10 results. tl;dr seems to be that it sends no pagination options at all, but gets some kind of default.

The drone.io loop for fetching repo lists is here: https://github.com/drone/drone/blob/master/service/repo/repo.go which appears to try to set a size 100. The driver is in another source base here: https://github.com/drone/go-scm/blob/master/scm/driver/gitea/repo.go . It ignores the options (including size and pagination) and there might also be issues with https://github.com/drone/go-scm/blob/master/scm/client.go at the bottom where it parses out pagination information to pass back to the outer loop.

This has also been discussed here: https://discourse.drone.io/t/drone-only-shows-10-repositories-from-gitea/6922

[6543]

so in short #11800

[mgeerdsen]

Sorry to add to this merged PR, but I just wanted to point out that this also leads to problems when using the gitea plugin in Jenkins. This can potentially lead to disabled jobs in Jenkins which then could potentially be discarded as old items according to configuration.
An issue has been opened for this at https://issues.jenkins-ci.org/browse/JENKINS-63048

Just as a suggestion, it would be nice to have these kind of changes (especially API related) in the BREAKING section of the changelog.

[richmahn]

@spawn2kill How is this not a breaking change? We didn't use to page these results, now with the new version, our apps couldn't find the record they wanted because they were only getting 30, rather than all. Wouldn't that be a breaking change if you now have to use page=# ? We updated to this version and had a bunch of problems due to this.

[richmahn]

@zeripath You were right in this being a breaking change. If an app expects it to not be paged, but now you have to loop through page numbers until no results, that can be an issue.

[richmahn]

@spawn2kill I now see your comment about if you don't use page=# in your URL it will return all. But we don't use page=#, and it only returned 30, the number set in the app.ini file.

[6543]

@richmahn pleace open a new issue specific for that kind of api you have issues with - of course you can link to this pull

To All: DONT comment on closed pulls, open new issues an reference :)