diff --git a/integrations/dump_restore_test.go b/integrations/dump_restore_test.go index c0e583293c93a..57968618bc322 100644 --- a/integrations/dump_restore_test.go +++ b/integrations/dump_restore_test.go @@ -129,6 +129,7 @@ func TestDumpRestore(t *testing.T) { assert.EqualValues(t, before[i].Created, after[i].Created) assert.EqualValues(t, before[i].Updated, after[i].Updated) assert.EqualValues(t, before[i].Labels, after[i].Labels) + assert.EqualValues(t, before[i].Reactions, after[i].Reactions) } } }) diff --git a/models/user/user.go b/models/user/user.go index 38352fe5e24c3..5ed850bdfa83d 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1025,6 +1025,19 @@ func GetUserNamesByIDs(ids []int64) ([]string, error) { return unames, err } +// GetUserNameByID returns username for the id +func GetUserNameByID(ctx context.Context, id int64) (string, error) { + var name string + has, err := db.GetEngine(db.DefaultContext).Table("user").Where("id = ?", id).Cols("name").Get(&name) + if err != nil { + return "", err + } + if has { + return name, nil + } + return "", nil +} + // GetUserIDsByNames returns a slice of ids corresponds to names. func GetUserIDsByNames(names []string, ignoreNonExistent bool) ([]int64, error) { ids := make([]int64, 0, len(names)) diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index da085c0d8db07..8f0505afc5d0b 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -46,6 +46,7 @@ type GiteaLocalUploader struct { issues map[int64]*models.Issue gitRepo *git.Repository prHeadCache map[string]struct{} + sameApp bool userMap map[int64]int64 // external user id mapping to user id prCache map[int64]*models.PullRequest gitServiceType structs.GitServiceType @@ -128,6 +129,7 @@ func (g *GiteaLocalUploader) CreateRepo(repo *base.Repository, opts base.Migrate MirrorInterval: opts.MirrorInterval, }, NewMigrationHTTPTransport()) + g.sameApp = strings.HasPrefix(repo.OriginalURL, setting.AppURL) g.repo = r if err != nil { return err @@ -256,7 +258,7 @@ func (g *GiteaLocalUploader) CreateReleases(releases ...*base.Release) error { CreatedUnix: timeutil.TimeStamp(release.Created.Unix()), } - if err := g.remapExternalUser(release, &rel); err != nil { + if err := g.remapUser(release, &rel); err != nil { return err } @@ -373,7 +375,7 @@ func (g *GiteaLocalUploader) CreateIssues(issues ...*base.Issue) error { UpdatedUnix: timeutil.TimeStamp(issue.Updated.Unix()), } - if err := g.remapExternalUser(issue, &is); err != nil { + if err := g.remapUser(issue, &is); err != nil { return err } @@ -386,7 +388,7 @@ func (g *GiteaLocalUploader) CreateIssues(issues ...*base.Issue) error { Type: reaction.Content, CreatedUnix: timeutil.TimeStampNow(), } - if err := g.remapExternalUser(reaction, &res); err != nil { + if err := g.remapUser(reaction, &res); err != nil { return err } is.Reactions = append(is.Reactions, &res) @@ -437,7 +439,7 @@ func (g *GiteaLocalUploader) CreateComments(comments ...*base.Comment) error { UpdatedUnix: timeutil.TimeStamp(comment.Updated.Unix()), } - if err := g.remapExternalUser(comment, &cm); err != nil { + if err := g.remapUser(comment, &cm); err != nil { return err } @@ -447,7 +449,7 @@ func (g *GiteaLocalUploader) CreateComments(comments ...*base.Comment) error { Type: reaction.Content, CreatedUnix: timeutil.TimeStampNow(), } - if err := g.remapExternalUser(reaction, &res); err != nil { + if err := g.remapUser(reaction, &res); err != nil { return err } cm.Reactions = append(cm.Reactions, &res) @@ -471,7 +473,7 @@ func (g *GiteaLocalUploader) CreatePullRequests(prs ...*base.PullRequest) error return err } - if err := g.remapExternalUser(pr, gpr.Issue); err != nil { + if err := g.remapUser(pr, gpr.Issue); err != nil { return err } @@ -626,7 +628,7 @@ func (g *GiteaLocalUploader) newPullRequest(pr *base.PullRequest) (*models.PullR UpdatedUnix: timeutil.TimeStamp(pr.Updated.Unix()), } - if err := g.remapExternalUser(pr, &issue); err != nil { + if err := g.remapUser(pr, &issue); err != nil { return nil, err } @@ -636,7 +638,7 @@ func (g *GiteaLocalUploader) newPullRequest(pr *base.PullRequest) (*models.PullR Type: reaction.Content, CreatedUnix: timeutil.TimeStampNow(), } - if err := g.remapExternalUser(reaction, &res); err != nil { + if err := g.remapUser(reaction, &res); err != nil { return nil, err } issue.Reactions = append(issue.Reactions, &res) @@ -710,7 +712,7 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error { UpdatedUnix: timeutil.TimeStamp(review.CreatedAt.Unix()), } - if err := g.remapExternalUser(review, &cm); err != nil { + if err := g.remapUser(review, &cm); err != nil { return err } @@ -773,7 +775,7 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error { UpdatedUnix: timeutil.TimeStamp(comment.UpdatedAt.Unix()), } - if err := g.remapExternalUser(review, &c); err != nil { + if err := g.remapUser(review, &c); err != nil { return err } @@ -816,23 +818,52 @@ func (g *GiteaLocalUploader) Finish() error { return repo_model.UpdateRepositoryCols(g.repo, "status") } -func (g *GiteaLocalUploader) remapExternalUser(source user_model.ExternalUserMigrated, target user_model.ExternalUserRemappable) (err error) { +func (g *GiteaLocalUploader) remapUser(source user_model.ExternalUserMigrated, target user_model.ExternalUserRemappable) error { + var userid int64 + var err error + if g.sameApp { + userid, err = g.remapLocalUser(source, target) + } else { + userid, err = g.remapExternalUser(source, target) + } + + if err != nil { + return err + } + + if userid > 0 { + return target.RemapExternalUser("", 0, userid) + } + return target.RemapExternalUser(source.GetExternalName(), source.GetExternalID(), g.doer.ID) +} + +func (g *GiteaLocalUploader) remapLocalUser(source user_model.ExternalUserMigrated, target user_model.ExternalUserRemappable) (int64, 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 !ok { + name, err := user_model.GetUserNameByID(g.ctx, source.GetExternalID()) if err != nil { - log.Error("GetUserIDByExternalUserID: %v", err) + return 0, err } - if userid > 0 { - g.userMap[source.GetExternalID()] = userid + // let's not reuse an ID when the user was deleted or has a different user name + if name != source.GetExternalName() { + userid = 0 + } else { + userid = source.GetExternalID() } + g.userMap[source.GetExternalID()] = userid } + return userid, nil +} - if userid > 0 { - err = target.RemapExternalUser("", 0, userid) - } else { - err = target.RemapExternalUser(source.GetExternalName(), source.GetExternalID(), g.doer.ID) +func (g *GiteaLocalUploader) remapExternalUser(source user_model.ExternalUserMigrated, target user_model.ExternalUserRemappable) (userid int64, err error) { + userid, ok := g.userMap[source.GetExternalID()] + if !ok { + userid, err = user_model.GetUserIDByExternalUserID(g.gitServiceType.Name(), fmt.Sprintf("%d", source.GetExternalID())) + if err != nil { + log.Error("GetUserIDByExternalUserID: %v", err) + return 0, err + } + g.userMap[source.GetExternalID()] = userid } - return + return userid, nil } diff --git a/services/migrations/gitea_uploader_test.go b/services/migrations/gitea_uploader_test.go index 7552245d74463..0a35b5a632d4d 100644 --- a/services/migrations/gitea_uploader_test.go +++ b/services/migrations/gitea_uploader_test.go @@ -117,6 +117,56 @@ func TestGiteaUploadRepo(t *testing.T) { assert.Len(t, pulls[0].Issue.Comments, 2) } +func TestGiteaUploadRemapLocalUser(t *testing.T) { + unittest.PrepareTestEnv(t) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}).(*user_model.User) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User) + + repoName := "migrated" + uploader := NewGiteaLocalUploader(context.Background(), doer, doer.Name, repoName) + // call remapLocalUser + uploader.sameApp = true + + externalID := int64(1234567) + externalName := "username" + source := base.Release{ + PublisherID: externalID, + PublisherName: externalName, + } + + // + // The externalID does not match any existing user, everything + // belongs to the doer + // + target := models.Release{} + uploader.userMap = make(map[int64]int64) + err := uploader.remapUser(&source, &target) + assert.NoError(t, err) + assert.EqualValues(t, doer.ID, target.GetUserID()) + + // + // The externalID matches a known user but the name does not match, + // everything belongs to the doer + // + source.PublisherID = user.ID + target = models.Release{} + uploader.userMap = make(map[int64]int64) + err = uploader.remapUser(&source, &target) + assert.NoError(t, err) + assert.EqualValues(t, doer.ID, target.GetUserID()) + + // + // The externalID and externalName match an existing user, everything + // belongs to the existing user + // + source.PublisherName = user.Name + target = models.Release{} + uploader.userMap = make(map[int64]int64) + err = uploader.remapUser(&source, &target) + assert.NoError(t, err) + assert.EqualValues(t, user.ID, target.GetUserID()) +} + func TestGiteaUploadRemapExternalUser(t *testing.T) { unittest.PrepareTestEnv(t) doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}).(*user_model.User) @@ -124,9 +174,11 @@ func TestGiteaUploadRemapExternalUser(t *testing.T) { repoName := "migrated" uploader := NewGiteaLocalUploader(context.Background(), doer, doer.Name, repoName) uploader.gitServiceType = structs.GiteaService + // call remapExternalUser + uploader.sameApp = false externalID := int64(1234567) - externalName := "url.or.something" + externalName := "username" source := base.Release{ PublisherID: externalID, PublisherName: externalName, @@ -136,8 +188,9 @@ func TestGiteaUploadRemapExternalUser(t *testing.T) { // When there is no user linked to the external ID, the migrated data is authored // by the doer // + uploader.userMap = make(map[int64]int64) target := models.Release{} - err := uploader.remapExternalUser(&source, &target) + err := uploader.remapUser(&source, &target) assert.NoError(t, err) assert.EqualValues(t, doer.ID, target.GetUserID()) @@ -158,8 +211,9 @@ func TestGiteaUploadRemapExternalUser(t *testing.T) { // When a user is linked to the external ID, it becomes the author of // the migrated data // + uploader.userMap = make(map[int64]int64) target = models.Release{} - err = uploader.remapExternalUser(&source, &target) + err = uploader.remapUser(&source, &target) assert.NoError(t, err) - assert.EqualValues(t, target.GetUserID(), linkedUser.ID) + assert.EqualValues(t, linkedUser.ID, target.GetUserID()) }