mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-02-03 20:51:07 -05:00
fix: migrations/github: avoid getting the first issues page twice (#10798)
For the previous code with the Page attribute present in ListCursorOptions for page 1, github would not return an "After" cursor, such that the request for page 2 would request what effectively is the content of page 1 a second time. This would lead to an attempt to insert the same issues twice. Note that this is not the only reason why this can happen with the current code base. We fix this particular issue by not using the Page attribute so github does return an "After" cursor. Fixes #10794 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10798 Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
This commit is contained in:
parent
b4412c2206
commit
01fbb1499f
9 changed files with 176 additions and 39 deletions
|
|
@ -432,9 +432,6 @@ func (g *GithubDownloaderV3) GetReleases() ([]*base.Release, error) {
|
|||
|
||||
// GetIssues returns issues according start and limit
|
||||
func (g *GithubDownloaderV3) GetIssues(page, perPage int) ([]*base.Issue, bool, error) {
|
||||
var issues []*github.Issue
|
||||
var resp *github.Response
|
||||
var err error
|
||||
if perPage > g.maxPerPage {
|
||||
perPage = g.maxPerPage
|
||||
}
|
||||
|
|
@ -442,29 +439,17 @@ func (g *GithubDownloaderV3) GetIssues(page, perPage int) ([]*base.Issue, bool,
|
|||
allIssues := make([]*base.Issue, 0, perPage)
|
||||
g.waitAndPickClient()
|
||||
|
||||
if page == 1 {
|
||||
issues, resp, err = g.getClient().Issues.ListByRepo(g.ctx, g.repoOwner, g.repoName, &github.IssueListByRepoOptions{
|
||||
Sort: "created",
|
||||
Direction: "asc",
|
||||
State: "all",
|
||||
ListCursorOptions: github.ListCursorOptions{
|
||||
PerPage: perPage,
|
||||
Page: strconv.Itoa(page),
|
||||
},
|
||||
})
|
||||
g.githubPagingInfo.After = resp.After
|
||||
} else {
|
||||
issues, resp, err = g.getClient().Issues.ListByRepo(g.ctx, g.repoOwner, g.repoName, &github.IssueListByRepoOptions{
|
||||
Sort: "created",
|
||||
Direction: "asc",
|
||||
State: "all",
|
||||
ListCursorOptions: github.ListCursorOptions{
|
||||
PerPage: perPage,
|
||||
After: g.githubPagingInfo.After,
|
||||
},
|
||||
})
|
||||
g.githubPagingInfo.After = resp.After
|
||||
}
|
||||
issues, resp, err := g.getClient().Issues.ListByRepo(g.ctx, g.repoOwner, g.repoName, &github.IssueListByRepoOptions{
|
||||
Sort: "created",
|
||||
Direction: "asc",
|
||||
State: "all",
|
||||
ListCursorOptions: github.ListCursorOptions{
|
||||
PerPage: perPage,
|
||||
After: g.githubPagingInfo.After,
|
||||
},
|
||||
})
|
||||
|
||||
g.githubPagingInfo.After = resp.After
|
||||
|
||||
if err != nil {
|
||||
return nil, false, fmt.Errorf("error while listing repos: %w", err)
|
||||
|
|
|
|||
|
|
@ -493,12 +493,14 @@ func TestGithubMultiToken(t *testing.T) {
|
|||
func TestGithubIssuePagination(t *testing.T) {
|
||||
GithubLimitRateRemaining = 3 // Wait at 3 remaining since we could have 3 CI in //
|
||||
|
||||
token := os.Getenv("GITHUB_READ_TOKEN")
|
||||
if token == "" {
|
||||
t.Skip()
|
||||
}
|
||||
token := os.Getenv("GITHUB_READ_TOKEN_NIGOROLL")
|
||||
liveMode := token != ""
|
||||
|
||||
downloader := NewGithubDownloaderV3(t.Context(), "https://api.github.com", true, true, "", "", token, "galaxyproject", "galaxy")
|
||||
fixturePath := "./testdata/github/pagination"
|
||||
server := unittest.NewMockWebServer(t, "https://api.github.com", fixturePath, liveMode)
|
||||
defer server.Close()
|
||||
|
||||
downloader := NewGithubDownloaderV3(t.Context(), server.URL, true, true, "", "", token, "nigoroll", "libvmod-dynamic")
|
||||
downloader.SkipReactions = true
|
||||
err := downloader.RefreshRate()
|
||||
require.NoError(t, err)
|
||||
|
|
@ -507,18 +509,26 @@ func TestGithubIssuePagination(t *testing.T) {
|
|||
require.NoError(t, err)
|
||||
|
||||
assertRepositoryEqual(t, &base.Repository{
|
||||
Name: "galaxy",
|
||||
Owner: "galaxyproject",
|
||||
Description: "Data intensive science for everyone.",
|
||||
CloneURL: "https://github.com/galaxyproject/galaxy.git",
|
||||
OriginalURL: "https://github.com/galaxyproject/galaxy",
|
||||
DefaultBranch: "dev",
|
||||
Website: "https://galaxyproject.org",
|
||||
Name: "libvmod-dynamic",
|
||||
Owner: "nigoroll",
|
||||
Description: "The Varnish dns/named director continued",
|
||||
CloneURL: server.URL + "/nigoroll/libvmod-dynamic.git",
|
||||
OriginalURL: server.URL + "/nigoroll/libvmod-dynamic",
|
||||
DefaultBranch: "master",
|
||||
}, repo)
|
||||
|
||||
seen := make(map[int64]bool)
|
||||
|
||||
perPage := 45
|
||||
for page := 1; page <= 250; page++ {
|
||||
_, _, err = downloader.GetIssues(page, perPage)
|
||||
issues, last, err := downloader.GetIssues(page, perPage)
|
||||
require.NoError(t, err)
|
||||
for _, issue := range issues {
|
||||
assert.False(t, seen[issue.Number])
|
||||
seen[issue.Number] = true
|
||||
}
|
||||
if last {
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
22
services/migrations/testdata/github/pagination/GET_%2Frate_limit
vendored
Normal file
22
services/migrations/testdata/github/pagination/GET_%2Frate_limit
vendored
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
X-Ratelimit-Resource: core
|
||||
Access-Control-Expose-Headers: ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset
|
||||
X-Frame-Options: deny
|
||||
X-Content-Type-Options: nosniff
|
||||
X-Xss-Protection: 0
|
||||
Content-Security-Policy: default-src 'none'
|
||||
Cache-Control: no-cache
|
||||
X-Github-Api-Version-Selected: 2022-11-28
|
||||
X-Ratelimit-Limit: 5000
|
||||
X-Ratelimit-Used: 0
|
||||
Access-Control-Allow-Origin: *
|
||||
Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
|
||||
Referrer-Policy: origin-when-cross-origin, strict-origin-when-cross-origin
|
||||
X-Github-Media-Type: github.v3; format=json
|
||||
X-Accepted-Github-Permissions: allows_permissionless_access=true
|
||||
X-Ratelimit-Remaining: 5000
|
||||
X-Ratelimit-Reset: 1769189499
|
||||
Vary: Accept-Encoding, Accept, X-Requested-With
|
||||
Content-Type: application/json; charset=utf-8
|
||||
X-Github-Request-Id: 545E:1E338F:1CE3AD6:1995EED:6973A26A
|
||||
|
||||
{"resources":{"core":{"limit":5000,"used":0,"remaining":5000,"reset":1769189499},"search":{"limit":30,"used":0,"remaining":30,"reset":1769185959},"graphql":{"limit":5000,"used":0,"remaining":5000,"reset":1769189499},"integration_manifest":{"limit":5000,"used":0,"remaining":5000,"reset":1769189499},"source_import":{"limit":100,"used":0,"remaining":100,"reset":1769185959},"code_scanning_upload":{"limit":5000,"used":0,"remaining":5000,"reset":1769189499},"code_scanning_autofix":{"limit":10,"used":0,"remaining":10,"reset":1769185959},"actions_runner_registration":{"limit":10000,"used":0,"remaining":10000,"reset":1769189499},"scim":{"limit":15000,"used":0,"remaining":15000,"reset":1769189499},"dependency_snapshots":{"limit":100,"used":0,"remaining":100,"reset":1769185959},"dependency_sbom":{"limit":100,"used":0,"remaining":100,"reset":1769185959},"audit_log":{"limit":1750,"used":0,"remaining":1750,"reset":1769189499},"audit_log_streaming":{"limit":15,"used":0,"remaining":15,"reset":1769189499},"code_search":{"limit":10,"used":0,"remaining":10,"reset":1769185959}},"rate":{"limit":5000,"used":0,"remaining":5000,"reset":1769189499}}
|
||||
24
services/migrations/testdata/github/pagination/GET_%2Frepos%2Fnigoroll%2Flibvmod-dynamic
vendored
Normal file
24
services/migrations/testdata/github/pagination/GET_%2Frepos%2Fnigoroll%2Flibvmod-dynamic
vendored
Normal file
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
|
|
@ -0,0 +1,24 @@
|
|||
X-Ratelimit-Reset: 1769189499
|
||||
X-Ratelimit-Used: 5
|
||||
Link: <https://api.github.com/repositories/68390476/issues?direction=asc&per_page=45&sort=created&state=all&before=Y3Vyc29yOnYyOpLPAAABmw6cnwDO3cGf4g%3D%3D>; rel="prev"
|
||||
Access-Control-Expose-Headers: ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset
|
||||
Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
|
||||
X-Ratelimit-Limit: 5000
|
||||
X-Github-Request-Id: 545E:1E338F:1CE503C:199716F:6973A26E
|
||||
Vary: Accept, Authorization, Cookie, X-GitHub-OTP,Accept-Encoding, Accept, X-Requested-With
|
||||
X-Github-Media-Type: github.v3; param=squirrel-girl-preview
|
||||
X-Frame-Options: deny
|
||||
Referrer-Policy: origin-when-cross-origin, strict-origin-when-cross-origin
|
||||
X-Ratelimit-Resource: core
|
||||
Content-Type: application/json; charset=utf-8
|
||||
Cache-Control: private, max-age=60, s-maxage=60
|
||||
Etag: W/"c62d1ce27432411ccc529108455637a8bb6afea88b1be0bc1285beac77d2d917"
|
||||
X-Github-Api-Version-Selected: 2022-11-28
|
||||
X-Content-Type-Options: nosniff
|
||||
X-Ratelimit-Remaining: 4995
|
||||
X-Accepted-Github-Permissions: issues=read
|
||||
Access-Control-Allow-Origin: *
|
||||
X-Xss-Protection: 0
|
||||
Content-Security-Policy: default-src 'none'
|
||||
|
||||
[{"url":"https://api.github.com/repos/nigoroll/libvmod-dynamic/issues/137","repository_url":"https://api.github.com/repos/nigoroll/libvmod-dynamic","labels_url":"https://api.github.com/repos/nigoroll/libvmod-dynamic/issues/137/labels{/name}","comments_url":"https://api.github.com/repos/nigoroll/libvmod-dynamic/issues/137/comments","events_url":"https://api.github.com/repos/nigoroll/libvmod-dynamic/issues/137/events","html_url":"https://github.com/nigoroll/libvmod-dynamic/issues/137","id":3720454114,"node_id":"I_kwDOBBOOTM7dwZ_i","number":137,"title":"resolver_test01.vtc is broken to the redirection","user":{"login":"gquintard","id":3776553,"node_id":"MDQ6VXNlcjM3NzY1NTM=","avatar_url":"https://avatars.githubusercontent.com/u/3776553?v=4","gravatar_id":"","url":"https://api.github.com/users/gquintard","html_url":"https://github.com/gquintard","followers_url":"https://api.github.com/users/gquintard/followers","following_url":"https://api.github.com/users/gquintard/following{/other_user}","gists_url":"https://api.github.com/users/gquintard/gists{/gist_id}","starred_url":"https://api.github.com/users/gquintard/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gquintard/subscriptions","organizations_url":"https://api.github.com/users/gquintard/orgs","repos_url":"https://api.github.com/users/gquintard/repos","events_url":"https://api.github.com/users/gquintard/events{/privacy}","received_events_url":"https://api.github.com/users/gquintard/received_events","type":"User","user_view_type":"public","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2025-12-11T18:11:44Z","updated_at":"2025-12-12T11:12:19Z","closed_at":"2025-12-12T11:12:19Z","author_association":"CONTRIBUTOR","active_lock_reason":null,"sub_issues_summary":{"total":0,"completed":0,"percent_completed":0},"issue_dependencies_summary":{"blocked_by":0,"total_blocked_by":0,"blocking":0,"total_blocking":0},"body":"https://github.com/nigoroll/libvmod-dynamic/blob/master/src/vtc/resolver/resolver_test01.vtc#L25 now gets a 301.\n\nCan we get a release soon fixing this? Until then I'll disable tests while packaging as this is blocking the docker image builds","closed_by":{"login":"nigoroll","id":1528104,"node_id":"MDQ6VXNlcjE1MjgxMDQ=","avatar_url":"https://avatars.githubusercontent.com/u/1528104?v=4","gravatar_id":"","url":"https://api.github.com/users/nigoroll","html_url":"https://github.com/nigoroll","followers_url":"https://api.github.com/users/nigoroll/followers","following_url":"https://api.github.com/users/nigoroll/following{/other_user}","gists_url":"https://api.github.com/users/nigoroll/gists{/gist_id}","starred_url":"https://api.github.com/users/nigoroll/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nigoroll/subscriptions","organizations_url":"https://api.github.com/users/nigoroll/orgs","repos_url":"https://api.github.com/users/nigoroll/repos","events_url":"https://api.github.com/users/nigoroll/events{/privacy}","received_events_url":"https://api.github.com/users/nigoroll/received_events","type":"User","user_view_type":"public","site_admin":false},"reactions":{"url":"https://api.github.com/repos/nigoroll/libvmod-dynamic/issues/137/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"timeline_url":"https://api.github.com/repos/nigoroll/libvmod-dynamic/issues/137/timeline","performed_via_github_app":null,"state_reason":"completed"}]
|
||||
File diff suppressed because one or more lines are too long
Loading…
Reference in a new issue