From f6474cf2e9c11510de4bfe76ea5fbf84fe001dc9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 13 Apr 2025 21:48:03 -0700 Subject: [PATCH] Fix bug when migrating repository (#34182) This PR fixed a bug which is a regression from #31035 --- routers/api/v1/repo/migrate.go | 2 +- services/migrations/gitea_uploader.go | 2 +- services/org/team_test.go | 5 +++-- services/packages/cargo/index.go | 2 +- services/repository/create.go | 9 +++++---- services/repository/create_test.go | 4 ++-- services/repository/repository.go | 2 +- services/task/task.go | 2 +- tests/integration/compare_test.go | 2 +- tests/integration/mirror_pull_test.go | 2 +- tests/integration/mirror_push_test.go | 2 +- tests/integration/pull_review_test.go | 2 +- 12 files changed, 19 insertions(+), 17 deletions(-) diff --git a/routers/api/v1/repo/migrate.go b/routers/api/v1/repo/migrate.go index 87bb269031..f2e0cad86c 100644 --- a/routers/api/v1/repo/migrate.go +++ b/routers/api/v1/repo/migrate.go @@ -181,7 +181,7 @@ func Migrate(ctx *context.APIContext) { IsPrivate: opts.Private || setting.Repository.ForcePrivate, IsMirror: opts.Mirror, Status: repo_model.RepositoryBeingMigrated, - }) + }, false) if err != nil { handleMigrateError(ctx, repoOwner, err) return diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index cd5af39910..82d756dc56 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -107,7 +107,7 @@ func (g *GiteaLocalUploader) CreateRepo(ctx context.Context, repo *base.Reposito IsPrivate: opts.Private || setting.Repository.ForcePrivate, IsMirror: opts.Mirror, Status: repo_model.RepositoryBeingMigrated, - }) + }, false) } else { r, err = repo_model.GetRepositoryByID(ctx, opts.MigrateToRepoID) } diff --git a/services/org/team_test.go b/services/org/team_test.go index 831fe1669f..aa39771cd2 100644 --- a/services/org/team_test.go +++ b/services/org/team_test.go @@ -205,7 +205,8 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) { // Create repos. repoIDs := make([]int64, 0) for i := 0; i < 3; i++ { - r, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user, org.AsUser(), repo_service.CreateRepoOptions{Name: fmt.Sprintf("repo-%d", i)}) + r, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user, org.AsUser(), + repo_service.CreateRepoOptions{Name: fmt.Sprintf("repo-%d", i)}, true) assert.NoError(t, err, "CreateRepository %d", i) if r != nil { repoIDs = append(repoIDs, r.ID) @@ -267,7 +268,7 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) { } // Create repo and check teams repositories. - r, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user, org.AsUser(), repo_service.CreateRepoOptions{Name: "repo-last"}) + r, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user, org.AsUser(), repo_service.CreateRepoOptions{Name: "repo-last"}, true) assert.NoError(t, err, "CreateRepository last") if r != nil { repoIDs = append(repoIDs, r.ID) diff --git a/services/packages/cargo/index.go b/services/packages/cargo/index.go index decb224b85..e8a2f189c8 100644 --- a/services/packages/cargo/index.go +++ b/services/packages/cargo/index.go @@ -213,7 +213,7 @@ func getOrCreateIndexRepository(ctx context.Context, doer, owner *user_model.Use if errors.Is(err, util.ErrNotExist) { repo, err = repo_service.CreateRepositoryDirectly(ctx, doer, owner, repo_service.CreateRepoOptions{ Name: IndexRepositoryName, - }) + }, true) if err != nil { return nil, fmt.Errorf("CreateRepository: %w", err) } diff --git a/services/repository/create.go b/services/repository/create.go index 897f2a1d14..c4a9dbb1b6 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -199,7 +199,10 @@ func initRepository(ctx context.Context, u *user_model.User, repo *repo_model.Re } // CreateRepositoryDirectly creates a repository for the user/organization. -func CreateRepositoryDirectly(ctx context.Context, doer, owner *user_model.User, opts CreateRepoOptions) (*repo_model.Repository, error) { +// if needsUpdateToReady is true, it will update the repository status to ready when success +func CreateRepositoryDirectly(ctx context.Context, doer, owner *user_model.User, + opts CreateRepoOptions, needsUpdateToReady bool, +) (*repo_model.Repository, error) { if !doer.CanCreateRepoIn(owner) { return nil, repo_model.ErrReachLimitOfRepo{ Limit: owner.MaxRepoCreation, @@ -243,8 +246,6 @@ func CreateRepositoryDirectly(ctx context.Context, doer, owner *user_model.User, ObjectFormatName: opts.ObjectFormatName, } - needsUpdateStatus := opts.Status != repo_model.RepositoryReady - // 1 - create the repository database operations first err := db.WithTx(ctx, func(ctx context.Context) error { return createRepositoryInDB(ctx, doer, owner, repo, false) @@ -318,7 +319,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, owner *user_model.User, } // 7 - update repository status to be ready - if needsUpdateStatus { + if needsUpdateToReady { repo.Status = repo_model.RepositoryReady if err = repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil { return nil, fmt.Errorf("UpdateRepositoryCols: %w", err) diff --git a/services/repository/create_test.go b/services/repository/create_test.go index 9ecf404e8b..8e3fdf88a5 100644 --- a/services/repository/create_test.go +++ b/services/repository/create_test.go @@ -25,7 +25,7 @@ func TestCreateRepositoryDirectly(t *testing.T) { createdRepo, err := CreateRepositoryDirectly(git.DefaultContext, user2, user2, CreateRepoOptions{ Name: "created-repo", - }) + }, true) assert.NoError(t, err) assert.NotNil(t, createdRepo) @@ -44,7 +44,7 @@ func TestCreateRepositoryDirectly(t *testing.T) { createdRepo2, err := CreateRepositoryDirectly(db.DefaultContext, user2, user2, CreateRepoOptions{ Name: "created-repo", - }) + }, true) assert.Nil(t, createdRepo2) assert.Error(t, err) diff --git a/services/repository/repository.go b/services/repository/repository.go index 90aad4f9d8..e078a8fc3c 100644 --- a/services/repository/repository.go +++ b/services/repository/repository.go @@ -47,7 +47,7 @@ type WebSearchResults struct { // CreateRepository creates a repository for the user/organization. func CreateRepository(ctx context.Context, doer, owner *user_model.User, opts CreateRepoOptions) (*repo_model.Repository, error) { - repo, err := CreateRepositoryDirectly(ctx, doer, owner, opts) + repo, err := CreateRepositoryDirectly(ctx, doer, owner, opts, true) if err != nil { // No need to rollback here we should do this in CreateRepository... return nil, err diff --git a/services/task/task.go b/services/task/task.go index 105aee2a25..ee5fa1f348 100644 --- a/services/task/task.go +++ b/services/task/task.go @@ -111,7 +111,7 @@ func CreateMigrateTask(ctx context.Context, doer, u *user_model.User, opts base. IsPrivate: opts.Private || setting.Repository.ForcePrivate, IsMirror: opts.Mirror, Status: repo_model.RepositoryBeingMigrated, - }) + }, false) if err != nil { task.EndTime = timeutil.TimeStampNow() task.Status = structs.TaskStatusFailed diff --git a/tests/integration/compare_test.go b/tests/integration/compare_test.go index cbf927813e..0648777fed 100644 --- a/tests/integration/compare_test.go +++ b/tests/integration/compare_test.go @@ -133,7 +133,7 @@ func TestCompareCodeExpand(t *testing.T) { Readme: "Default", AutoInit: true, DefaultBranch: "main", - }) + }, true) assert.NoError(t, err) session := loginUser(t, user1.Name) diff --git a/tests/integration/mirror_pull_test.go b/tests/integration/mirror_pull_test.go index fdf399f2bc..c33b2eb04d 100644 --- a/tests/integration/mirror_pull_test.go +++ b/tests/integration/mirror_pull_test.go @@ -48,7 +48,7 @@ func TestMirrorPull(t *testing.T) { IsPrivate: opts.Private, IsMirror: opts.Mirror, Status: repo_model.RepositoryBeingMigrated, - }) + }, false) assert.NoError(t, err) assert.True(t, mirrorRepo.IsMirror, "expected pull-mirror repo to be marked as a mirror immediately after its creation") diff --git a/tests/integration/mirror_push_test.go b/tests/integration/mirror_push_test.go index 40d100cd27..9b6d4c4017 100644 --- a/tests/integration/mirror_push_test.go +++ b/tests/integration/mirror_push_test.go @@ -40,7 +40,7 @@ func testMirrorPush(t *testing.T, u *url.URL) { mirrorRepo, err := repo_service.CreateRepositoryDirectly(db.DefaultContext, user, user, repo_service.CreateRepoOptions{ Name: "test-push-mirror", - }) + }, true) assert.NoError(t, err) session := loginUser(t, user.Name) diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index 3c96e4e6c4..13b2384e9c 100644 --- a/tests/integration/pull_review_test.go +++ b/tests/integration/pull_review_test.go @@ -58,7 +58,7 @@ func TestPullView_CodeOwner(t *testing.T) { AutoInit: true, ObjectFormatName: git.Sha1ObjectFormat.Name(), DefaultBranch: "master", - }) + }, true) assert.NoError(t, err) // add CODEOWNERS to default branch