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>

models/fixtures/external_login_user.yml

@@ -0,0 +1 @@
+[] # empty

models/issue.go

@@ -2329,3 +2329,20 @@ func deleteIssuesByRepoID(sess db.Engine, repoID int64) (attachmentPaths []strin
 
 	return
 }
+
+// RemapExternalUser ExternalUserRemappable interface
+func (issue *Issue) RemapExternalUser(externalName string, externalID, userID int64) error {
+	issue.OriginalAuthor = externalName
+	issue.OriginalAuthorID = externalID
+	issue.PosterID = userID
+	return nil
+}
+
+// GetUserID ExternalUserRemappable interface
+func (issue *Issue) GetUserID() int64 { return issue.PosterID }
+
+// GetExternalName ExternalUserRemappable interface
+func (issue *Issue) GetExternalName() string { return issue.OriginalAuthor }
+
+// GetExternalID ExternalUserRemappable interface
+func (issue *Issue) GetExternalID() int64 { return issue.OriginalAuthorID }

models/issue_comment.go

@@ -1464,3 +1464,20 @@ func commitBranchCheck(gitRepo *git.Repository, startCommit *git.Commit, endComm
 	}
 	return nil
 }
+
+// RemapExternalUser ExternalUserRemappable interface
+func (c *Comment) RemapExternalUser(externalName string, externalID, userID int64) error {
+	c.OriginalAuthor = externalName
+	c.OriginalAuthorID = externalID
+	c.PosterID = userID
+	return nil
+}
+
+// GetUserID ExternalUserRemappable interface
+func (c *Comment) GetUserID() int64 { return c.PosterID }
+
+// GetExternalName ExternalUserRemappable interface
+func (c *Comment) GetExternalName() string { return c.OriginalAuthor }
+
+// GetExternalID ExternalUserRemappable interface
+func (c *Comment) GetExternalID() int64 { return c.OriginalAuthorID }

models/issue_reaction.go

@@ -343,3 +343,20 @@ func (list ReactionList) GetMoreUserCount() int {
 	}
 	return len(list) - setting.UI.ReactionMaxUserNum
 }
+
+// RemapExternalUser ExternalUserRemappable interface
+func (r *Reaction) RemapExternalUser(externalName string, externalID, userID int64) error {
+	r.OriginalAuthor = externalName
+	r.OriginalAuthorID = externalID
+	r.UserID = userID
+	return nil
+}
+
+// GetUserID ExternalUserRemappable interface
+func (r *Reaction) GetUserID() int64 { return r.UserID }
+
+// GetExternalName ExternalUserRemappable interface
+func (r *Reaction) GetExternalName() string { return r.OriginalAuthor }
+
+// GetExternalID ExternalUserRemappable interface
+func (r *Reaction) GetExternalID() int64 { return r.OriginalAuthorID }

models/release.go

@@ -456,3 +456,20 @@ func SaveOrUpdateTag(repo *repo_model.Repository, newRel *Release) error {
 	}
 	return nil
 }
+
+// RemapExternalUser ExternalUserRemappable interface
+func (r *Release) RemapExternalUser(externalName string, externalID, userID int64) error {
+	r.OriginalAuthor = externalName
+	r.OriginalAuthorID = externalID
+	r.PublisherID = userID
+	return nil
+}
+
+// UserID ExternalUserRemappable interface
+func (r *Release) GetUserID() int64 { return r.PublisherID }
+
+// ExternalName ExternalUserRemappable interface
+func (r *Release) GetExternalName() string { return r.OriginalAuthor }
+
+// ExternalID ExternalUserRemappable interface
+func (r *Release) GetExternalID() int64 { return r.OriginalAuthorID }

models/review.go

@@ -987,3 +987,20 @@ func (r *Review) HTMLURL() string {
 	}
 	return comment.HTMLURL()
 }
+
+// RemapExternalUser ExternalUserRemappable interface
+func (r *Review) RemapExternalUser(externalName string, externalID, userID int64) error {
+	r.OriginalAuthor = externalName
+	r.OriginalAuthorID = externalID
+	r.ReviewerID = userID
+	return nil
+}
+
+// GetUserID ExternalUserRemappable interface
+func (r *Review) GetUserID() int64 { return r.ReviewerID }
+
+// GetExternalName ExternalUserRemappable interface
+func (r *Review) GetExternalName() string { return r.OriginalAuthor }
+
+// GetExternalID ExternalUserRemappable interface
+func (r *Review) GetExternalID() int64 { return r.OriginalAuthorID }

models/user/external_login_user.go

@@ -68,6 +68,17 @@ type ExternalLoginUser struct {
 	ExpiresAt         time.Time
 }
 
+type ExternalUserMigrated interface {
+	GetExternalName() string
+	GetExternalID() int64
+}
+
+type ExternalUserRemappable interface {
+	GetUserID() int64
+	RemapExternalUser(externalName string, externalID, userID int64) error
+	ExternalUserMigrated
+}
+
 func init() {
 	db.RegisterModel(new(ExternalLoginUser))
 }

modules/migration/comment.go

@@ -18,3 +18,9 @@ type Comment struct {
 	Content     string
 	Reactions   []*Reaction
 }
+
+// GetExternalName ExternalUserMigrated interface
+func (c *Comment) GetExternalName() string { return c.PosterName }
+
+// ExternalID ExternalUserMigrated interface
+func (c *Comment) GetExternalID() int64 { return c.PosterID }

modules/migration/issue.go

@@ -46,3 +46,9 @@ type Issue struct {
 	Assignees   []string     `json:"assignees"`
 	Context     IssueContext `yaml:"-"`
 }
+
+// GetExternalName ExternalUserMigrated interface
+func (i *Issue) GetExternalName() string { return i.PosterName }
+
+// GetExternalID ExternalUserMigrated interface
+func (i *Issue) GetExternalID() int64 { return i.PosterID }

modules/migration/pullrequest.go

@@ -61,3 +61,9 @@ type PullRequestBranch struct {
 func (p PullRequestBranch) RepoPath() string {
 	return fmt.Sprintf("%s/%s", p.OwnerName, p.RepoName)
 }
+
+// GetExternalName ExternalUserMigrated interface
+func (p *PullRequest) GetExternalName() string { return p.PosterName }
+
+// ExternalID ExternalUserMigrated interface
+func (p *PullRequest) GetExternalID() int64 { return p.PosterID }

modules/migration/reaction.go

@@ -10,3 +10,9 @@ type Reaction struct {
 	UserName string `yaml:"user_name" json:"user_name"`
 	Content  string `json:"content"`
 }
+
+// GetExternalName ExternalUserMigrated interface
+func (r *Reaction) GetExternalName() string { return r.UserName }
+
+// GetExternalID ExternalUserMigrated interface
+func (r *Reaction) GetExternalID() int64 { return r.UserID }

modules/migration/release.go

@@ -38,3 +38,9 @@ type Release struct {
 	Created         time.Time
 	Published       time.Time
 }
+
+// GetExternalName ExternalUserMigrated interface
+func (r *Release) GetExternalName() string { return r.PublisherName }
+
+// GetExternalID ExternalUserMigrated interface
+func (r *Release) GetExternalID() int64 { return r.PublisherID }

modules/migration/review.go

@@ -28,6 +28,12 @@ type Review struct {
 	Comments     []*ReviewComment
 }
 
+// GetExternalName ExternalUserMigrated interface
+func (r *Review) GetExternalName() string { return r.ReviewerName }
+
+// ExternalID ExternalUserMigrated interface
+func (r *Review) GetExternalID() int64 { return r.ReviewerID }
+
 // ReviewComment represents a review comment
 type ReviewComment struct {
 	ID        int64

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)
+}