mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-09 22:18:39 +02:00
fix(migrations): stop endless comment paging when migrating from Gitea (#39419)
`GET /repos/{owner}/{repo}/issues/{index}/comments` ignores `page` and
`limit` and always returns every comment. The Gitea downloader pages it
and stops only on a short page, so migrating from Gitea or Forgejo loops
forever on any issue with at least `MAX_RESPONSE_ITEMS` comments,
without an error.
Paging is kept in case the endpoint gets paginated
(https://github.com/go-gitea/gitea/issues/6132,
https://github.com/go-gitea/gitea/issues/18082). The loop now stops when
a page is longer than the limit or starts with an already seen comment.
Prior art: Forgejo fixed its copy in
https://codeberg.org/forgejo/forgejo/pulls/9274 (report:
https://codeberg.org/Codeberg/Community/issues/1542).
---
AI-assisted: drafted with Claude Code (claude-opus-5-5), reviewed by me.
Please let me know if you have any suggestions or comments, I ran into
this issue myself when I was trying to migrate repositories from Forgejo
-> Gitea.
---------
Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
1 parent
27d787d70a
commit
03058691c3
2 files changed
+58
-1
No files matched your search
@@ -13,6 +13,7 @@ import (
|
|||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"gitea.dev/modules/container"
|
||||||
"gitea.dev/modules/log"
|
"gitea.dev/modules/log"
|
||||||
base "gitea.dev/modules/migration"
|
base "gitea.dev/modules/migration"
|
||||||
"gitea.dev/modules/structs"
|
"gitea.dev/modules/structs"
|
||||||
@@ -472,6 +473,7 @@ func (g *GiteaDownloader) GetIssues(ctx context.Context, page, perPage int) ([]*
|
|||||||
// GetComments returns comments according issueNumber
|
// GetComments returns comments according issueNumber
|
||||||
func (g *GiteaDownloader) GetComments(ctx context.Context, commentable base.Commentable) ([]*base.Comment, bool, error) {
|
func (g *GiteaDownloader) GetComments(ctx context.Context, commentable base.Commentable) ([]*base.Comment, bool, error) {
|
||||||
allComments := make([]*base.Comment, 0, g.maxPerPage)
|
allComments := make([]*base.Comment, 0, g.maxPerPage)
|
||||||
|
seenIDs := container.Set[int64]{}
|
||||||
|
|
||||||
for i := 1; ; i++ {
|
for i := 1; ; i++ {
|
||||||
// make sure gitea can shutdown gracefully
|
// make sure gitea can shutdown gracefully
|
||||||
@@ -488,8 +490,12 @@ func (g *GiteaDownloader) GetComments(ctx context.Context, commentable base.Comm
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, false, fmt.Errorf("error while listing comments for issue #%d. Error: %w", commentable.GetForeignIndex(), err)
|
return nil, false, fmt.Errorf("error while listing comments for issue #%d. Error: %w", commentable.GetForeignIndex(), err)
|
||||||
}
|
}
|
||||||
|
if len(comments) == 0 || seenIDs.Contains(comments[0].ID) {
|
||||||
|
break // the endpoint ignores page and limit, so page 2 repeats page 1
|
||||||
|
}
|
||||||
|
|
||||||
for _, comment := range comments {
|
for _, comment := range comments {
|
||||||
|
seenIDs.Add(comment.ID)
|
||||||
reactions, err := g.getCommentReactions(comment.ID)
|
reactions, err := g.getCommentReactions(comment.ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
WarnAndNotice("Unable to load comment reactions during migrating issue #%d for comment %d in %s. Error: %v", commentable.GetForeignIndex(), comment.ID, g, err)
|
WarnAndNotice("Unable to load comment reactions during migrating issue #%d for comment %d in %s. Error: %v", commentable.GetForeignIndex(), comment.ID, g, err)
|
||||||
@@ -508,7 +514,7 @@ func (g *GiteaDownloader) GetComments(ctx context.Context, commentable base.Comm
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
if !g.pagination || len(comments) < g.maxPerPage {
|
if !g.pagination || len(comments) != g.maxPerPage {
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,10 +4,15 @@
|
|||||||
package migrations
|
package migrations
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"fmt"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"runtime"
|
"runtime"
|
||||||
"sort"
|
"sort"
|
||||||
|
"strconv"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -309,3 +314,49 @@ func TestGiteaDownloadRepo(t *testing.T) {
|
|||||||
},
|
},
|
||||||
}, reviews)
|
}, reviews)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestGiteaDownloadCommentsPaging(t *testing.T) {
|
||||||
|
for _, tc := range []struct {
|
||||||
|
maxResponseItems, commentCount, requests int
|
||||||
|
paginated bool
|
||||||
|
}{
|
||||||
|
{maxResponseItems: 2, commentCount: 2, requests: 2},
|
||||||
|
{maxResponseItems: 2, commentCount: 3, requests: 1},
|
||||||
|
{maxResponseItems: 2, commentCount: 4, requests: 3, paginated: true},
|
||||||
|
{maxResponseItems: 0, commentCount: 0, requests: 1},
|
||||||
|
} {
|
||||||
|
t.Run(strconv.Itoa(tc.commentCount), func(t *testing.T) {
|
||||||
|
commentRequests := 0
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
switch r.URL.Path {
|
||||||
|
case "/api/v1/version":
|
||||||
|
_, _ = w.Write([]byte(`{"version":"1.27.0"}`))
|
||||||
|
case "/api/v1/settings/api":
|
||||||
|
_, _ = fmt.Fprintf(w, `{"max_response_items":%d}`, tc.maxResponseItems)
|
||||||
|
case "/api/v1/repos/o/r/issues/1/comments":
|
||||||
|
commentRequests++
|
||||||
|
comments := make([]string, tc.commentCount)
|
||||||
|
for i := range comments {
|
||||||
|
comments[i] = fmt.Sprintf(`{"id":%d,"user":{}}`, i+1)
|
||||||
|
}
|
||||||
|
if tc.paginated {
|
||||||
|
page, _ := strconv.Atoi(r.URL.Query().Get("page"))
|
||||||
|
comments = comments[(page-1)*tc.maxResponseItems : min(page*tc.maxResponseItems, len(comments))]
|
||||||
|
}
|
||||||
|
_, _ = w.Write([]byte("[" + strings.Join(comments, ",") + "]"))
|
||||||
|
default:
|
||||||
|
_, _ = w.Write([]byte(`[]`))
|
||||||
|
}
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
downloader, err := NewGiteaDownloader(t.Context(), server.URL, "o/r", "", "", "")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
comments, _, err := downloader.GetComments(t.Context(), &base.Issue{Number: 1})
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Len(t, comments, tc.commentCount)
|
||||||
|
assert.Equal(t, tc.requests, commentRequests)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user