add test coverage for original author conversion during migrations (#18506)

* add test coverage for original author conversion during migrations

And create a function to factorize a code snippet that is repeated
five times and would otherwise be more difficult to test and maintain
consistently.

Signed-off-by: Loïc Dachary <loic@dachary.org>

* fix variable scope and int64 formatting

* add missing calls to remapExternalUser and fix misplaced %d

Co-authored-by: Loïc Dachary <loic@dachary.org>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>

2 files changed, 89 insertions, 173 deletions

diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go
index 6e823031ed..9edb6258d8 100644
--- a/services/migrations/gitea_uploader.go
+++ b/services/migrations/gitea_uploader.go
@@ -254,25 +254,8 @@ func (g *GiteaLocalUploader) CreateReleases(releases ...*base.Release) error {
 			CreatedUnix:  timeutil.TimeStamp(release.Created.Unix()),
 		}
 
-		userid, ok := g.userMap[release.PublisherID]
-		tp := g.gitServiceType.Name()
-		if !ok && tp != "" {
-			var err error
-			userid, err = user_model.GetUserIDByExternalUserID(tp, fmt.Sprintf("%v", release.PublisherID))
-			if err != nil {
-				log.Error("GetUserIDByExternalUserID: %v", err)
-			}
-			if userid > 0 {
-				g.userMap[release.PublisherID] = userid
-			}
-		}
-
-		if userid > 0 {
-			rel.PublisherID = userid
-		} else {
-			rel.PublisherID = g.doer.ID
-			rel.OriginalAuthor = release.PublisherName
-			rel.OriginalAuthorID = release.PublisherID
+		if err := g.remapExternalUser(release, &rel); err != nil {
+			return err
 		}
 
 		// calc NumCommits if no draft
@@ -394,25 +377,8 @@ func (g *GiteaLocalUploader) CreateIssues(issues ...*base.Issue) error {
 			UpdatedUnix: timeutil.TimeStamp(issue.Updated.Unix()),
 		}
 
-		userid, ok := g.userMap[issue.PosterID]
-		tp := g.gitServiceType.Name()
-		if !ok && tp != "" {
-			var err error
-			userid, err = user_model.GetUserIDByExternalUserID(tp, fmt.Sprintf("%v", issue.PosterID))
-			if err != nil {
-				log.Error("GetUserIDByExternalUserID: %v", err)
-			}
-			if userid > 0 {
-				g.userMap[issue.PosterID] = userid
-			}
-		}
-
-		if userid > 0 {
-			is.PosterID = userid
-		} else {
-			is.PosterID = g.doer.ID
-			is.OriginalAuthor = issue.PosterName
-			is.OriginalAuthorID = issue.PosterID
+		if err := g.remapExternalUser(issue, &is); err != nil {
+			return err
 		}
 
 		if issue.Closed != nil {
@@ -420,27 +386,12 @@ func (g *GiteaLocalUploader) CreateIssues(issues ...*base.Issue) error {
 		}
 		// add reactions
 		for _, reaction := range issue.Reactions {
-			userid, ok := g.userMap[reaction.UserID]
-			if !ok && tp != "" {
-				var err error
-				userid, err = user_model.GetUserIDByExternalUserID(tp, fmt.Sprintf("%v", reaction.UserID))
-				if err != nil {
-					log.Error("GetUserIDByExternalUserID: %v", err)
-				}
-				if userid > 0 {
-					g.userMap[reaction.UserID] = userid
-				}
-			}
 			res := models.Reaction{
 				Type:        reaction.Content,
 				CreatedUnix: timeutil.TimeStampNow(),
 			}
-			if userid > 0 {
-				res.UserID = userid
-			} else {
-				res.UserID = g.doer.ID
-				res.OriginalAuthorID = reaction.UserID
-				res.OriginalAuthor = reaction.UserName
+			if err := g.remapExternalUser(reaction, &res); err != nil {
+				return err
 			}
 			is.Reactions = append(is.Reactions, &res)
 		}
@@ -477,19 +428,6 @@ func (g *GiteaLocalUploader) CreateComments(comments ...*base.Comment) error {
 			issue = issueInter.(*models.Issue)
 		}
 
-		userid, ok := g.userMap[comment.PosterID]
-		tp := g.gitServiceType.Name()
-		if !ok && tp != "" {
-			var err error
-			userid, err = user_model.GetUserIDByExternalUserID(tp, fmt.Sprintf("%v", comment.PosterID))
-			if err != nil {
-				log.Error("GetUserIDByExternalUserID: %v", err)
-			}
-			if userid > 0 {
-				g.userMap[comment.PosterID] = userid
-			}
-		}
-
 		if comment.Created.IsZero() {
 			comment.Created = time.Unix(int64(issue.CreatedUnix), 0)
 		}
@@ -505,37 +443,18 @@ func (g *GiteaLocalUploader) CreateComments(comments ...*base.Comment) error {
 			UpdatedUnix: timeutil.TimeStamp(comment.Updated.Unix()),
 		}
 
-		if userid > 0 {
-			cm.PosterID = userid
-		} else {
-			cm.PosterID = g.doer.ID
-			cm.OriginalAuthor = comment.PosterName
-			cm.OriginalAuthorID = comment.PosterID
+		if err := g.remapExternalUser(comment, &cm); err != nil {
+			return err
 		}
 
 		// add reactions
 		for _, reaction := range comment.Reactions {
-			userid, ok := g.userMap[reaction.UserID]
-			if !ok && tp != "" {
-				var err error
-				userid, err = user_model.GetUserIDByExternalUserID(tp, fmt.Sprintf("%v", reaction.UserID))
-				if err != nil {
-					log.Error("GetUserIDByExternalUserID: %v", err)
-				}
-				if userid > 0 {
-					g.userMap[reaction.UserID] = userid
-				}
-			}
 			res := models.Reaction{
 				Type:        reaction.Content,
 				CreatedUnix: timeutil.TimeStampNow(),
 			}
-			if userid > 0 {
-				res.UserID = userid
-			} else {
-				res.UserID = g.doer.ID
-				res.OriginalAuthorID = reaction.UserID
-				res.OriginalAuthor = reaction.UserName
+			if err := g.remapExternalUser(reaction, &res); err != nil {
+				return err
 			}
 			cm.Reactions = append(cm.Reactions, &res)
 		}
@@ -558,25 +477,8 @@ func (g *GiteaLocalUploader) CreatePullRequests(prs ...*base.PullRequest) error
 			return err
 		}
 
-		userid, ok := g.userMap[pr.PosterID]
-		tp := g.gitServiceType.Name()
-		if !ok && tp != "" {
-			var err error
-			userid, err = user_model.GetUserIDByExternalUserID(tp, fmt.Sprintf("%v", pr.PosterID))
-			if err != nil {
-				log.Error("GetUserIDByExternalUserID: %v", err)
-			}
-			if userid > 0 {
-				g.userMap[pr.PosterID] = userid
-			}
-		}
-
-		if userid > 0 {
-			gpr.Issue.PosterID = userid
-		} else {
-			gpr.Issue.PosterID = g.doer.ID
-			gpr.Issue.OriginalAuthor = pr.PosterName
-			gpr.Issue.OriginalAuthorID = pr.PosterID
+		if err := g.remapExternalUser(pr, gpr.Issue); err != nil {
+			return err
 		}
 
 		gprs = append(gprs, gpr)
@@ -736,51 +638,18 @@ func (g *GiteaLocalUploader) newPullRequest(pr *base.PullRequest) (*models.PullR
 		UpdatedUnix: timeutil.TimeStamp(pr.Updated.Unix()),
 	}
 
-	tp := g.gitServiceType.Name()
-
-	userid, ok := g.userMap[pr.PosterID]
-	if !ok && tp != "" {
-		var err error
-		userid, err = user_model.GetUserIDByExternalUserID(tp, fmt.Sprintf("%v", pr.PosterID))
-		if err != nil {
-			log.Error("GetUserIDByExternalUserID: %v", err)
-		}
-		if userid > 0 {
-			g.userMap[pr.PosterID] = userid
-		}
-	}
-
-	if userid > 0 {
-		issue.PosterID = userid
-	} else {
-		issue.PosterID = g.doer.ID
-		issue.OriginalAuthor = pr.PosterName
-		issue.OriginalAuthorID = pr.PosterID
+	if err := g.remapExternalUser(pr, &issue); err != nil {
+		return nil, err
 	}
 
 	// add reactions
 	for _, reaction := range pr.Reactions {
-		userid, ok := g.userMap[reaction.UserID]
-		if !ok && tp != "" {
-			var err error
-			userid, err = user_model.GetUserIDByExternalUserID(tp, fmt.Sprintf("%v", reaction.UserID))
-			if err != nil {
-				log.Error("GetUserIDByExternalUserID: %v", err)
-			}
-			if userid > 0 {
-				g.userMap[reaction.UserID] = userid
-			}
-		}
 		res := models.Reaction{
 			Type:        reaction.Content,
 			CreatedUnix: timeutil.TimeStampNow(),
 		}
-		if userid > 0 {
-			res.UserID = userid
-		} else {
-			res.UserID = g.doer.ID
-			res.OriginalAuthorID = reaction.UserID
-			res.OriginalAuthor = reaction.UserName
+		if err := g.remapExternalUser(reaction, &res); err != nil {
+			return nil, err
 		}
 		issue.Reactions = append(issue.Reactions, &res)
 	}
@@ -843,19 +712,6 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
 			issue = issueInter.(*models.Issue)
 		}
 
-		userid, ok := g.userMap[review.ReviewerID]
-		tp := g.gitServiceType.Name()
-		if !ok && tp != "" {
-			var err error
-			userid, err = user_model.GetUserIDByExternalUserID(tp, fmt.Sprintf("%v", review.ReviewerID))
-			if err != nil {
-				log.Error("GetUserIDByExternalUserID: %v", err)
-			}
-			if userid > 0 {
-				g.userMap[review.ReviewerID] = userid
-			}
-		}
-
 		if review.CreatedAt.IsZero() {
 			review.CreatedAt = time.Unix(int64(issue.CreatedUnix), 0)
 		}
@@ -869,12 +725,8 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
 			UpdatedUnix: timeutil.TimeStamp(review.CreatedAt.Unix()),
 		}
 
-		if userid > 0 {
-			cm.ReviewerID = userid
-		} else {
-			cm.ReviewerID = g.doer.ID
-			cm.OriginalAuthor = review.ReviewerName
-			cm.OriginalAuthorID = review.ReviewerID
+		if err := g.remapExternalUser(review, &cm); err != nil {
+			return err
 		}
 
 		// get pr
@@ -926,7 +778,6 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
 
 			c := models.Comment{
 				Type:        models.CommentTypeCode,
-				PosterID:    comment.PosterID,
 				IssueID:     issue.ID,
 				Content:     comment.Content,
 				Line:        int64(line + comment.Position - 1),
@@ -937,12 +788,8 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error {
 				UpdatedUnix: timeutil.TimeStamp(comment.UpdatedAt.Unix()),
 			}
 
-			if userid > 0 {
-				c.PosterID = userid
-			} else {
-				c.PosterID = g.doer.ID
-				c.OriginalAuthor = review.ReviewerName
-				c.OriginalAuthorID = review.ReviewerID
+			if err := g.remapExternalUser(review, &c); err != nil {
+				return err
 			}
 
 			cm.Comments = append(cm.Comments, &c)
@@ -983,3 +830,24 @@ func (g *GiteaLocalUploader) Finish() error {
 	g.repo.Status = repo_model.RepositoryReady
 	return repo_model.UpdateRepositoryCols(g.repo, "status")
 }
+
+func (g *GiteaLocalUploader) remapExternalUser(source user_model.ExternalUserMigrated, target user_model.ExternalUserRemappable) (err error) {
+	userid, ok := g.userMap[source.GetExternalID()]
+	tp := g.gitServiceType.Name()
+	if !ok && tp != "" {
+		userid, err = user_model.GetUserIDByExternalUserID(tp, fmt.Sprintf("%d", source.GetExternalID()))
+		if err != nil {
+			log.Error("GetUserIDByExternalUserID: %v", err)
+		}
+		if userid > 0 {
+			g.userMap[source.GetExternalID()] = userid
+		}
+	}
+
+	if userid > 0 {
+		err = target.RemapExternalUser("", 0, userid)
+	} else {
+		err = target.RemapExternalUser(source.GetExternalName(), source.GetExternalID(), g.doer.ID)
+	}
+	return
+}
diff --git a/services/migrations/gitea_uploader_test.go b/services/migrations/gitea_uploader_test.go
index 7d4f77eac8..7552245d74 100644
--- a/services/migrations/gitea_uploader_test.go
+++ b/services/migrations/gitea_uploader_test.go
@@ -7,6 +7,7 @@ package migrations
 
 import (
 	"context"
+	"strconv"
 	"testing"
 	"time"
 
@@ -115,3 +116,50 @@ func TestGiteaUploadRepo(t *testing.T) {
 	assert.NoError(t, pulls[0].Issue.LoadDiscussComments())
 	assert.Len(t, pulls[0].Issue.Comments, 2)
 }
+
+func TestGiteaUploadRemapExternalUser(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+	doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}).(*user_model.User)
+
+	repoName := "migrated"
+	uploader := NewGiteaLocalUploader(context.Background(), doer, doer.Name, repoName)
+	uploader.gitServiceType = structs.GiteaService
+
+	externalID := int64(1234567)
+	externalName := "url.or.something"
+	source := base.Release{
+		PublisherID:   externalID,
+		PublisherName: externalName,
+	}
+
+	//
+	// When there is no user linked to the external ID, the migrated data is authored
+	// by the doer
+	//
+	target := models.Release{}
+	err := uploader.remapExternalUser(&source, &target)
+	assert.NoError(t, err)
+	assert.EqualValues(t, doer.ID, target.GetUserID())
+
+	//
+	// Link the external ID to an existing user
+	//
+	linkedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User)
+	externalLoginUser := &user_model.ExternalLoginUser{
+		ExternalID:    strconv.FormatInt(externalID, 10),
+		UserID:        linkedUser.ID,
+		LoginSourceID: 0,
+		Provider:      structs.GiteaService.Name(),
+	}
+	err = user_model.LinkExternalToUser(linkedUser, externalLoginUser)
+	assert.NoError(t, err)
+
+	//
+	// When a user is linked to the external ID, it becomes the author of
+	// the migrated data
+	//
+	target = models.Release{}
+	err = uploader.remapExternalUser(&source, &target)
+	assert.NoError(t, err)
+	assert.EqualValues(t, target.GetUserID(), linkedUser.ID)
+}