fix: prevent panic on gitlab import (releases/issues) (#11282)

It is unfortunately all mixed up, because refreshing the data, means breaking the tests. And changing the code means needing fresh data.

- tests: ignore some more headers and sort the rest when dumping http responses
- code: fixed #10234 by requesting the latest issues first.
- tests: created a new repo to replace the disappeared repo, needed for the skip-numbers test
- refreshed the testdata.
- follow-up fixes to get the tests green.
- including a cherry-pick of go-gitea/gitea#36295 and #11272

Co-authored-by: Joakim Olsson <joakim@unbound.se>
Co-authored-by: Robert Wolff <mahlzahn@posteo.de>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11282
Reviewed-by: Robert Wolff <mahlzahn@posteo.de>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: patdyn <patdyn@noreply.codeberg.org>
Co-authored-by: oliverpool <git@olivier.pfad.fr>
Co-committed-by: oliverpool <git@olivier.pfad.fr>
(cherry picked from commit a0d6970)

Author: 3 people

models/unittest/mock_http.go

@@ -7,6 +7,7 @@ import (
 	"bufio"
 	"fmt"
 	"io"
+	"maps"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -29,7 +30,7 @@ import (
 //     test data files
 func NewMockWebServer(t *testing.T, liveServerBaseURL, testDataDir string, liveMode bool) *httptest.Server {
 	mockServerBaseURL := ""
-	ignoredHeaders := []string{"cf-ray", "server", "date", "report-to", "nel", "x-request-id"}
+	ignoredHeaders := []string{"cf-ray", "server", "date", "report-to", "nel", "x-request-id", "set-cookie", "x-gitlab-meta"}
 
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		path := NormalizedFullPath(r.URL)
@@ -46,6 +47,7 @@ func NewMockWebServer(t *testing.T, liveServerBaseURL, testDataDir string, liveM
 			fixturePath = fmt.Sprintf("%s/%s", testDataDir, strings.TrimLeft(r.URL.Path, "/"))
 		}
 		if liveMode {
+			require.NoError(t, os.MkdirAll(testDataDir, 0o755))
 			liveURL := fmt.Sprintf("%s%s", liveServerBaseURL, path)
 
 			request, err := http.NewRequest(r.Method, liveURL, nil)
@@ -68,8 +70,8 @@ func NewMockWebServer(t *testing.T, liveServerBaseURL, testDataDir string, liveM
 			defer fixture.Close()
 			fixtureWriter := bufio.NewWriter(fixture)
 
-			for headerName, headerValues := range response.Header {
-				for _, headerValue := range headerValues {
+			for _, headerName := range slices.Sorted(maps.Keys(response.Header)) {
+				for _, headerValue := range response.Header[headerName] {
 					if !slices.Contains(ignoredHeaders, strings.ToLower(headerName)) {
 						_, err := fmt.Fprintf(fixtureWriter, "%s: %s\n", headerName, headerValue)
 						require.NoError(t, err, "writing the header of the HTTP response to the fixture file failed")

modules/migration/release.go

@@ -10,9 +10,9 @@ import (
 
 // ReleaseAsset represents a release asset
 type ReleaseAsset struct {
-	ID            int64
-	Name          string
-	ContentType   *string `yaml:"content_type"`
+	ID   int64
+	Name string
+
 	Size          *int
 	DownloadCount *int `yaml:"download_count"`
 	Created       time.Time

release-notes/11282.md

@@ -0,0 +1,3 @@
+fix: prevent panic when importing issues from GitLab
+fix: prevent panic when importing releases with more than 4 release assets from GitLab
+fix: correct re-mapping of merge-request numbers mentioned in GitLab comments

services/migrations/github.go

@@ -352,7 +352,6 @@ func (g *GithubDownloaderV3) convertGithubRelease(rel *github.RepositoryRelease)
 		r.Assets = append(r.Assets, &base.ReleaseAsset{
 			ID:            asset.GetID(),
 			Name:          asset.GetName(),
-			ContentType:   asset.ContentType,
 			Size:          asset.Size,
 			DownloadCount: asset.DownloadCount,
 			Created:       asset.CreatedAt.Time,

services/migrations/github_test.go

@@ -236,7 +236,6 @@ func TestGitHubDownloadRepo(t *testing.T) {
 	}, labels)
 
 	id := int64(280443629)
-	ct := "application/pdf"
 	size := 550175
 	dc := 0
 
@@ -256,7 +255,6 @@ func TestGitHubDownloadRepo(t *testing.T) {
 				{
 					ID:            id,
 					Name:          "wireguard.pdf",
-					ContentType:   &ct,
 					Size:          &size,
 					DownloadCount: &dc,
 					Created:       time.Date(2025, time.August, 7, 23, 39, 27, 0, time.UTC),

services/migrations/gitlab.go

@@ -65,10 +65,13 @@ type gitlabIIDResolver struct {
 }
 
 func (r *gitlabIIDResolver) recordIssueIID(issueIID int) {
+	if int64(issueIID) <= r.maxIssueIID {
+		return
+	}
 	if r.frozen {
-		panic("cannot record issue IID after pull request IID generation has started")
+		panic("cannot record bigger issue IID after pull request IID generation has started")
 	}
-	r.maxIssueIID = max(r.maxIssueIID, int64(issueIID))
+	r.maxIssueIID = int64(issueIID)
 }
 
 func (r *gitlabIIDResolver) generatePullRequestNumber(mrIID int) int64 {
@@ -332,12 +335,11 @@ func (g *GitlabDownloader) convertGitlabRelease(rel *gitlab.Release) *base.Relea
 
 	httpClient := NewMigrationHTTPClient()
 
-	for k, asset := range rel.Assets.Links {
+	for _, asset := range rel.Assets.Links {
 		assetID := asset.ID // Don't optimize this, for closure we need a local variable
 		r.Assets = append(r.Assets, &base.ReleaseAsset{
 			ID:            int64(asset.ID),
 			Name:          asset.Name,
-			ContentType:   &rel.Assets.Sources[k].Format,
 			Size:          &zero,
 			DownloadCount: &zero,
 			DownloadFunc: func() (io.ReadCloser, error) {
@@ -403,15 +405,18 @@ type gitlabIssueContext struct {
 //	Note: issue label description and colors are not supported by the go-gitlab library at this time
 func (g *GitlabDownloader) GetIssues(page, perPage int) ([]*base.Issue, bool, error) {
 	state := "all"
-	sort := "asc"
+	// we want most recent issues first, to get the biggest issue IID immediately
+	sort := "desc"
+	orderBy := "created_at"
 
 	if perPage > g.maxPerPage {
 		perPage = g.maxPerPage
 	}
 
 	opt := &gitlab.ListProjectIssuesOptions{
-		State: &state,
-		Sort:  &sort,
+		State:   &state,
+		Sort:    &sort,
+		OrderBy: &orderBy,
 		ListOptions: gitlab.ListOptions{
 			PerPage: perPage,
 			Page:    page,
@@ -795,39 +800,14 @@ func (g *GitlabDownloader) awardsToReactions(awards []*gitlab.AwardEmoji) []*bas
 	return result
 }
 
-// Build on the assumption, that PR IDs will resolve after Issue IDs
-func (g *GitlabDownloader) convertMRReference(body string) string {
-	maxLength := len(body)
-	for i := 0; i < maxLength; i++ {
-		if body[i] == '!' {
-			var collected string
-			for k := i + 1; k < maxLength; k++ { // for each rune after ! check if next rune is integer
-				if body[k]-'0' <= 9 {
-					collected += string(body[k])
-					if k == maxLength-1 { // The last rune in the string was an integer
-						body = g.updateAndInsert(body, collected, i+1, k)
-					}
-				} else if len(collected) > 0 { // Integers have been collected, update value
-					body = g.updateAndInsert(body, collected, i+1, k)
-					maxLength = len(body)
-					i = k
-					break // We're done, continue after our replacement
-				}
-			}
-		}
-	}
-	return body
-}
+var mrFinder = regexp.MustCompile(`![0-9]+`)
 
-func (g *GitlabDownloader) updateAndInsert(description, oldReference string, endFirst, startSecond int) string {
-	oldVal, _ := strconv.Atoi(oldReference)
-	newVal := oldVal + int(g.iidResolver.maxIssueIID)
-	firstPart := description[0:endFirst]
-	firstPart += strconv.Itoa(newVal)
-	var secondPart string
-	if startSecond < len(description)-1 {
-		secondPart = description[startSecond:]
-	}
-	description = firstPart + secondPart
-	return description
+// In gitlab, issues and merge-request have split numbering
+// Adjust the merge-request numbers (preserve the issue numbers)
+func (g *GitlabDownloader) convertMRReference(body string) string {
+	return mrFinder.ReplaceAllStringFunc(body, func(s string) string {
+		oldVal, _ := strconv.Atoi(s[1:]) // skip the leading exclamation mark
+		newVal := g.iidResolver.generatePullRequestNumber(oldVal)
+		return "!" + strconv.FormatInt(newVal, 10)
+	})
 }