diff --git a/models/git/branch.go b/models/git/branch.go index 9ac6c45578..beeb7c0689 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -235,6 +235,11 @@ func GetDeletedBranchByID(ctx context.Context, repoID, branchID int64) (*Branch, return &branch, nil } +func DeleteRepoBranches(ctx context.Context, repoID int64) error { + _, err := db.GetEngine(ctx).Where("repo_id=?", repoID).Delete(new(Branch)) + return err +} + func DeleteBranches(ctx context.Context, repoID, doerID int64, branchIDs []int64) error { return db.WithTx(ctx, func(ctx context.Context) error { branches := make([]*Branch, 0, len(branchIDs)) diff --git a/models/repo/release.go b/models/repo/release.go index 1c2e4a48e3..663d310bc0 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -558,3 +558,8 @@ func FindTagsByCommitIDs(ctx context.Context, repoID int64, commitIDs ...string) } return res, nil } + +func DeleteRepoReleases(ctx context.Context, repoID int64) error { + _, err := db.GetEngine(ctx).Where("repo_id = ?", repoID).Delete(new(Release)) + return err +} diff --git a/modules/repository/init.go b/modules/repository/init.go index ace21254ba..e6331966ba 100644 --- a/modules/repository/init.go +++ b/modules/repository/init.go @@ -11,11 +11,7 @@ import ( "strings" issues_model "code.gitea.io/gitea/models/issues" - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/label" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/options" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -121,29 +117,6 @@ func LoadRepoConfig() error { return nil } -func CheckInitRepository(ctx context.Context, repo *repo_model.Repository) (err error) { - // Somehow the directory could exist. - isExist, err := gitrepo.IsRepositoryExist(ctx, repo) - if err != nil { - log.Error("Unable to check if %s exists. Error: %v", repo.FullName(), err) - return err - } - if isExist { - return repo_model.ErrRepoFilesAlreadyExist{ - Uname: repo.OwnerName, - Name: repo.Name, - } - } - - // Init git bare new repository. - if err = git.InitRepository(ctx, repo.RepoPath(), true, repo.ObjectFormatName); err != nil { - return fmt.Errorf("git.InitRepository: %w", err) - } else if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil { - return fmt.Errorf("createDelegateHooks: %w", err) - } - return nil -} - // InitializeLabels adds a label set to a repository using a template func InitializeLabels(ctx context.Context, id int64, labelTemplate string, isOrg bool) error { list, err := LoadTemplateLabelsByDisplayName(labelTemplate) diff --git a/services/repository/adopt.go b/services/repository/adopt.go index b7321156d9..6953970e28 100644 --- a/services/repository/adopt.go +++ b/services/repository/adopt.go @@ -16,7 +16,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" @@ -28,6 +27,18 @@ import ( "github.com/gobwas/glob" ) +func deleteFailedAdoptRepository(repoID int64) error { + return db.WithTx(db.DefaultContext, func(ctx context.Context) error { + if err := deleteDBRepository(ctx, repoID); err != nil { + return fmt.Errorf("deleteDBRepository: %w", err) + } + if err := git_model.DeleteRepoBranches(ctx, repoID); err != nil { + return fmt.Errorf("deleteRepoBranches: %w", err) + } + return repo_model.DeleteRepoReleases(ctx, repoID) + }) +} + // AdoptRepository adopts pre-existing repository files for the user/organization. func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateRepoOptions) (*repo_model.Repository, error) { if !doer.IsAdmin && !u.CanCreateRepo() { @@ -48,58 +59,51 @@ func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts CreateR IsPrivate: opts.IsPrivate, IsFsckEnabled: !opts.IsMirror, CloseIssuesViaCommitInAnyBranch: setting.Repository.DefaultCloseIssuesViaCommitsInAnyBranch, - Status: opts.Status, + Status: repo_model.RepositoryBeingMigrated, IsEmpty: !opts.AutoInit, } - if err := db.WithTx(ctx, func(ctx context.Context) error { - isExist, err := gitrepo.IsRepositoryExist(ctx, repo) + // 1 - create the repository database operations first + err := db.WithTx(ctx, func(ctx context.Context) error { + return createRepositoryInDB(ctx, doer, u, repo, false) + }) + if err != nil { + return nil, err + } + + // last - clean up if something goes wrong + // WARNING: Don't override all later err with local variables + defer func() { if err != nil { - log.Error("Unable to check if %s exists. Error: %v", repo.FullName(), err) - return err - } - if !isExist { - return repo_model.ErrRepoNotExist{ - OwnerName: u.Name, - Name: repo.Name, + // we can not use the ctx because it maybe canceled or timeout + if errDel := deleteFailedAdoptRepository(repo.ID); errDel != nil { + log.Error("Failed to delete repository %s that could not be adopted: %v", repo.FullName(), errDel) } } + }() - if err := CreateRepositoryByExample(ctx, doer, u, repo, true, false); err != nil { - return err - } - - // Re-fetch the repository from database before updating it (else it would - // override changes that were done earlier with sql) - if repo, err = repo_model.GetRepositoryByID(ctx, repo.ID); err != nil { - return fmt.Errorf("getRepositoryByID: %w", err) - } - return nil - }); err != nil { - return nil, err + // Re-fetch the repository from database before updating it (else it would + // override changes that were done earlier with sql) + if repo, err = repo_model.GetRepositoryByID(ctx, repo.ID); err != nil { + return nil, fmt.Errorf("getRepositoryByID: %w", err) } - if err := func() error { - if err := adoptRepository(ctx, repo, opts.DefaultBranch); err != nil { - return fmt.Errorf("adoptRepository: %w", err) - } - - if err := repo_module.CheckDaemonExportOK(ctx, repo); err != nil { - return fmt.Errorf("checkDaemonExportOK: %w", err) - } - - if stdout, _, err := git.NewCommand("update-server-info"). - RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil { - log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) - return fmt.Errorf("CreateRepository(git update-server-info): %w", err) - } - return nil - }(); err != nil { - if errDel := DeleteRepository(ctx, doer, repo, false /* no notify */); errDel != nil { - log.Error("Failed to delete repository %s that could not be adopted: %v", repo.FullName(), errDel) - } - return nil, err + // 2 - adopt the repository from disk + if err = adoptRepository(ctx, repo, opts.DefaultBranch); err != nil { + return nil, fmt.Errorf("adoptRepository: %w", err) } + + // 3 - Update the git repository + if err = updateGitRepoAfterCreate(ctx, repo); err != nil { + return nil, fmt.Errorf("updateGitRepoAfterCreate: %w", err) + } + + // 4 - update repository status + repo.Status = repo_model.RepositoryReady + if err = repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil { + return nil, fmt.Errorf("UpdateRepositoryCols: %w", err) + } + notify_service.AdoptRepository(ctx, doer, u, repo) return repo, nil diff --git a/services/repository/adopt_test.go b/services/repository/adopt_test.go index 294185ea1f..6e1dc417b3 100644 --- a/services/repository/adopt_test.go +++ b/services/repository/adopt_test.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -89,10 +90,36 @@ func TestListUnadoptedRepositories_ListOptions(t *testing.T) { func TestAdoptRepository(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - assert.NoError(t, unittest.SyncDirs(filepath.Join(setting.RepoRootPath, "user2", "repo1.git"), filepath.Join(setting.RepoRootPath, "user2", "test-adopt.git"))) + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - _, err := AdoptRepository(db.DefaultContext, user2, user2, CreateRepoOptions{Name: "test-adopt"}) + + // a successful adopt + destDir := filepath.Join(setting.RepoRootPath, user2.Name, "test-adopt.git") + assert.NoError(t, unittest.SyncDirs(filepath.Join(setting.RepoRootPath, user2.Name, "repo1.git"), destDir)) + + adoptedRepo, err := AdoptRepository(db.DefaultContext, user2, user2, CreateRepoOptions{Name: "test-adopt"}) assert.NoError(t, err) repoTestAdopt := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{Name: "test-adopt"}) assert.Equal(t, "sha1", repoTestAdopt.ObjectFormatName) + + // just delete the adopted repo's db records + err = deleteFailedAdoptRepository(adoptedRepo.ID) + assert.NoError(t, err) + + unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: "test-adopt"}) + + // a failed adopt because some mock data + // remove the hooks directory and create a file so that we cannot create the hooks successfully + _ = os.RemoveAll(filepath.Join(destDir, "hooks", "update.d")) + assert.NoError(t, os.WriteFile(filepath.Join(destDir, "hooks", "update.d"), []byte("tests"), os.ModePerm)) + + adoptedRepo, err = AdoptRepository(db.DefaultContext, user2, user2, CreateRepoOptions{Name: "test-adopt"}) + assert.Error(t, err) + assert.Nil(t, adoptedRepo) + + unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: "test-adopt"}) + + exist, err := util.IsExist(repo_model.RepoPath(user2.Name, "test-adopt")) + assert.NoError(t, err) + assert.True(t, exist) // the repository should be still in the disk } diff --git a/services/repository/create.go b/services/repository/create.go index af4e897151..22dfce91f0 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" + system_model "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/models/webhook" @@ -140,8 +141,11 @@ func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir // InitRepository initializes README and .gitignore if needed. func initRepository(ctx context.Context, u *user_model.User, repo *repo_model.Repository, opts CreateRepoOptions) (err error) { - if err = repo_module.CheckInitRepository(ctx, repo); err != nil { - return err + // Init git bare new repository. + if err = git.InitRepository(ctx, repo.RepoPath(), true, repo.ObjectFormatName); err != nil { + return fmt.Errorf("git.InitRepository: %w", err) + } else if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil { + return fmt.Errorf("createDelegateHooks: %w", err) } // Initialize repository according to user's choice. @@ -244,100 +248,93 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt ObjectFormatName: opts.ObjectFormatName, } - var rollbackRepo *repo_model.Repository - - if err := db.WithTx(ctx, func(ctx context.Context) error { - if err := CreateRepositoryByExample(ctx, doer, u, repo, false, false); err != nil { - return err - } - - // No need for init mirror. - if opts.IsMirror { - return nil - } - - isExist, err := gitrepo.IsRepositoryExist(ctx, repo) - if err != nil { - log.Error("Unable to check if %s exists. Error: %v", repo.FullName(), err) - return err - } - if isExist { - // repo already exists - We have two or three options. - // 1. We fail stating that the directory exists - // 2. We create the db repository to go with this data and adopt the git repo - // 3. We delete it and start afresh - // - // Previously Gitea would just delete and start afresh - this was naughty. - // So we will now fail and delegate to other functionality to adopt or delete - log.Error("Files already exist in %s and we are not going to adopt or delete.", repo.FullName()) - return repo_model.ErrRepoFilesAlreadyExist{ - Uname: u.Name, - Name: repo.Name, - } - } - - if err = initRepository(ctx, doer, repo, opts); err != nil { - if err2 := gitrepo.DeleteRepository(ctx, repo); err2 != nil { - log.Error("initRepository: %v", err) - return fmt.Errorf( - "delete repo directory %s/%s failed(2): %v", u.Name, repo.Name, err2) - } - return fmt.Errorf("initRepository: %w", err) - } - - // Initialize Issue Labels if selected - if len(opts.IssueLabels) > 0 { - if err = repo_module.InitializeLabels(ctx, repo.ID, opts.IssueLabels, false); err != nil { - rollbackRepo = repo - rollbackRepo.OwnerID = u.ID - return fmt.Errorf("InitializeLabels: %w", err) - } - } - - if err := repo_module.CheckDaemonExportOK(ctx, repo); err != nil { - return fmt.Errorf("checkDaemonExportOK: %w", err) - } - - if stdout, _, err := git.NewCommand("update-server-info"). - RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil { - log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) - rollbackRepo = repo - rollbackRepo.OwnerID = u.ID - return fmt.Errorf("CreateRepository(git update-server-info): %w", err) - } - - // update licenses - var licenses []string - if len(opts.License) > 0 { - licenses = append(licenses, opts.License) - - stdout, _, err := git.NewCommand("rev-parse", "HEAD").RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}) - if err != nil { - log.Error("CreateRepository(git rev-parse HEAD) in %v: Stdout: %s\nError: %v", repo, stdout, err) - rollbackRepo = repo - rollbackRepo.OwnerID = u.ID - return fmt.Errorf("CreateRepository(git rev-parse HEAD): %w", err) - } - if err := repo_model.UpdateRepoLicenses(ctx, repo, stdout, licenses); err != nil { - return err - } - } - return nil - }); err != nil { - if rollbackRepo != nil { - if errDelete := DeleteRepositoryDirectly(ctx, doer, rollbackRepo.ID); errDelete != nil { - log.Error("Rollback deleteRepository: %v", errDelete) - } - } + 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, u, repo, false) + }) + if err != nil { return nil, err } + // last - clean up if something goes wrong + // WARNING: Don't override all later err with local variables + defer func() { + if err != nil { + // we can not use the ctx because it maybe canceled or timeout + cleanupRepository(doer, repo.ID) + } + }() + + // No need for init mirror. + if opts.IsMirror { + return repo, nil + } + + // 2 - check whether the repository with the same storage exists + var isExist bool + isExist, err = gitrepo.IsRepositoryExist(ctx, repo) + if err != nil { + log.Error("Unable to check if %s exists. Error: %v", repo.FullName(), err) + return nil, err + } + if isExist { + log.Error("Files already exist in %s and we are not going to adopt or delete.", repo.FullName()) + // Don't return directly, we need err in defer to cleanupRepository + err = repo_model.ErrRepoFilesAlreadyExist{ + Uname: repo.OwnerName, + Name: repo.Name, + } + return nil, err + } + + // 3 - init git repository in storage + if err = initRepository(ctx, doer, repo, opts); err != nil { + return nil, fmt.Errorf("initRepository: %w", err) + } + + // 4 - Initialize Issue Labels if selected + if len(opts.IssueLabels) > 0 { + if err = repo_module.InitializeLabels(ctx, repo.ID, opts.IssueLabels, false); err != nil { + return nil, fmt.Errorf("InitializeLabels: %w", err) + } + } + + // 5 - Update the git repository + if err = updateGitRepoAfterCreate(ctx, repo); err != nil { + return nil, fmt.Errorf("updateGitRepoAfterCreate: %w", err) + } + + // 6 - update licenses + var licenses []string + if len(opts.License) > 0 { + licenses = append(licenses, opts.License) + + var stdout string + stdout, _, err = git.NewCommand("rev-parse", "HEAD").RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}) + if err != nil { + log.Error("CreateRepository(git rev-parse HEAD) in %v: Stdout: %s\nError: %v", repo, stdout, err) + return nil, fmt.Errorf("CreateRepository(git rev-parse HEAD): %w", err) + } + if err = repo_model.UpdateRepoLicenses(ctx, repo, stdout, licenses); err != nil { + return nil, err + } + } + + // 7 - update repository status to be ready + if needsUpdateStatus { + repo.Status = repo_model.RepositoryReady + if err = repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil { + return nil, fmt.Errorf("UpdateRepositoryCols: %w", err) + } + } + return repo, nil } -// CreateRepositoryByExample creates a repository for the user/organization. -func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository, overwriteOrAdopt, isFork bool) (err error) { +// createRepositoryInDB creates a repository for the user/organization. +func createRepositoryInDB(ctx context.Context, doer, u *user_model.User, repo *repo_model.Repository, isFork bool) (err error) { if err = repo_model.IsUsableRepoName(repo.Name); err != nil { return err } @@ -352,19 +349,6 @@ func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, re } } - isExist, err := gitrepo.IsRepositoryExist(ctx, repo) - if err != nil { - log.Error("Unable to check if %s exists. Error: %v", repo.FullName(), err) - return err - } - if !overwriteOrAdopt && isExist { - log.Error("Files already exist in %s and we are not going to adopt or delete.", repo.FullName()) - return repo_model.ErrRepoFilesAlreadyExist{ - Uname: u.Name, - Name: repo.Name, - } - } - if err = db.Insert(ctx, repo); err != nil { return err } @@ -473,3 +457,26 @@ func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, re return nil } + +func cleanupRepository(doer *user_model.User, repoID int64) { + if errDelete := DeleteRepositoryDirectly(db.DefaultContext, doer, repoID); errDelete != nil { + log.Error("cleanupRepository failed: %v", errDelete) + // add system notice + if err := system_model.CreateRepositoryNotice("DeleteRepositoryDirectly failed when cleanup repository: %v", errDelete); err != nil { + log.Error("CreateRepositoryNotice: %v", err) + } + } +} + +func updateGitRepoAfterCreate(ctx context.Context, repo *repo_model.Repository) error { + if err := repo_module.CheckDaemonExportOK(ctx, repo); err != nil { + return fmt.Errorf("checkDaemonExportOK: %w", err) + } + + if stdout, _, err := git.NewCommand("update-server-info"). + RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil { + log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) + return fmt.Errorf("CreateRepository(git update-server-info): %w", err) + } + return nil +} diff --git a/services/repository/create_test.go b/services/repository/create_test.go new file mode 100644 index 0000000000..9ecf404e8b --- /dev/null +++ b/services/repository/create_test.go @@ -0,0 +1,57 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repository + +import ( + "os" + "testing" + + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/util" + + "github.com/stretchr/testify/assert" +) + +func TestCreateRepositoryDirectly(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + // a successful creating repository + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + createdRepo, err := CreateRepositoryDirectly(git.DefaultContext, user2, user2, CreateRepoOptions{ + Name: "created-repo", + }) + assert.NoError(t, err) + assert.NotNil(t, createdRepo) + + exist, err := util.IsExist(repo_model.RepoPath(user2.Name, createdRepo.Name)) + assert.NoError(t, err) + assert.True(t, exist) + + unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: createdRepo.Name}) + + err = DeleteRepositoryDirectly(db.DefaultContext, user2, createdRepo.ID) + assert.NoError(t, err) + + // a failed creating because some mock data + // create the repository directory so that the creation will fail after database record created. + assert.NoError(t, os.MkdirAll(repo_model.RepoPath(user2.Name, createdRepo.Name), os.ModePerm)) + + createdRepo2, err := CreateRepositoryDirectly(db.DefaultContext, user2, user2, CreateRepoOptions{ + Name: "created-repo", + }) + assert.Nil(t, createdRepo2) + assert.Error(t, err) + + // assert the cleanup is successful + unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: createdRepo.Name}) + + exist, err = util.IsExist(repo_model.RepoPath(user2.Name, createdRepo.Name)) + assert.NoError(t, err) + assert.False(t, exist) +} diff --git a/services/repository/delete.go b/services/repository/delete.go index ff74a20817..cf960af8cf 100644 --- a/services/repository/delete.go +++ b/services/repository/delete.go @@ -32,6 +32,19 @@ import ( "xorm.io/builder" ) +func deleteDBRepository(ctx context.Context, repoID int64) error { + if cnt, err := db.GetEngine(ctx).ID(repoID).Delete(&repo_model.Repository{}); err != nil { + return err + } else if cnt != 1 { + return repo_model.ErrRepoNotExist{ + ID: repoID, + OwnerName: "", + Name: "", + } + } + return nil +} + // DeleteRepository deletes a repository for a user or organization. // make sure if you call this func to close open sessions (sqlite will otherwise get a deadlock) func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID int64, ignoreOrgTeams ...bool) error { @@ -82,14 +95,8 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID } needRewriteKeysFile := deleted > 0 - if cnt, err := sess.ID(repoID).Delete(&repo_model.Repository{}); err != nil { + if err := deleteDBRepository(ctx, repoID); err != nil { return err - } else if cnt != 1 { - return repo_model.ErrRepoNotExist{ - ID: repoID, - OwnerName: "", - Name: "", - } } if org != nil && org.IsOrganization() { diff --git a/services/repository/fork.go b/services/repository/fork.go index 5b1ba7a418..daf6913510 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -100,114 +100,106 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork IsFork: true, ForkID: opts.BaseRepo.ID, ObjectFormatName: opts.BaseRepo.ObjectFormatName, + Status: repo_model.RepositoryBeingMigrated, } - oldRepoPath := opts.BaseRepo.RepoPath() - - needsRollback := false - rollbackFn := func() { - if !needsRollback { - return - } - - if exists, _ := gitrepo.IsRepositoryExist(ctx, repo); !exists { - return - } - - // As the transaction will be failed and hence database changes will be destroyed we only need - // to delete the related repository on the filesystem - if errDelete := gitrepo.DeleteRepository(ctx, repo); errDelete != nil { - log.Error("Failed to remove fork repo") - } - } - - needsRollbackInPanic := true - defer func() { - panicErr := recover() - if panicErr == nil { - return - } - - if needsRollbackInPanic { - rollbackFn() - } - panic(panicErr) - }() - - err = db.WithTx(ctx, func(txCtx context.Context) error { - if err = CreateRepositoryByExample(txCtx, doer, owner, repo, false, true); err != nil { + // 1 - Create the repository in the database + err = db.WithTx(ctx, func(ctx context.Context) error { + if err = createRepositoryInDB(ctx, doer, owner, repo, true); err != nil { return err } - - if err = repo_model.IncrementRepoForkNum(txCtx, opts.BaseRepo.ID); err != nil { + if err = repo_model.IncrementRepoForkNum(ctx, opts.BaseRepo.ID); err != nil { return err } // copy lfs files failure should not be ignored - if err = git_model.CopyLFS(txCtx, repo, opts.BaseRepo); err != nil { - return err - } - - needsRollback = true - - cloneCmd := git.NewCommand("clone", "--bare") - if opts.SingleBranch != "" { - cloneCmd.AddArguments("--single-branch", "--branch").AddDynamicArguments(opts.SingleBranch) - } - if stdout, _, err := cloneCmd.AddDynamicArguments(oldRepoPath, repo.RepoPath()). - RunStdBytes(txCtx, &git.RunOpts{Timeout: 10 * time.Minute}); err != nil { - log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, opts.BaseRepo, stdout, err) - return fmt.Errorf("git clone: %w", err) - } - - if err := repo_module.CheckDaemonExportOK(txCtx, repo); err != nil { - return fmt.Errorf("checkDaemonExportOK: %w", err) - } - - if stdout, _, err := git.NewCommand("update-server-info"). - RunStdString(txCtx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil { - log.Error("Fork Repository (git update-server-info) failed for %v:\nStdout: %s\nError: %v", repo, stdout, err) - return fmt.Errorf("git update-server-info: %w", err) - } - - if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil { - return fmt.Errorf("createDelegateHooks: %w", err) - } - - gitRepo, err := gitrepo.OpenRepository(txCtx, repo) - if err != nil { - return fmt.Errorf("OpenRepository: %w", err) - } - defer gitRepo.Close() - - _, err = repo_module.SyncRepoBranchesWithRepo(txCtx, repo, gitRepo, doer.ID) - return err + return git_model.CopyLFS(ctx, repo, opts.BaseRepo) }) - needsRollbackInPanic = false if err != nil { - rollbackFn() return nil, err } - // even if below operations failed, it could be ignored. And they will be retried - if err := repo_module.UpdateRepoSize(ctx, repo); err != nil { - log.Error("Failed to update size for repository: %v", err) - } - if err := repo_model.CopyLanguageStat(ctx, opts.BaseRepo, repo); err != nil { - log.Error("Copy language stat from oldRepo failed: %v", err) - } - if err := repo_model.CopyLicense(ctx, opts.BaseRepo, repo); err != nil { - return nil, err - } - - gitRepo, err := gitrepo.OpenRepository(ctx, repo) - if err != nil { - log.Error("Open created git repository failed: %v", err) - } else { - defer gitRepo.Close() - if err := repo_module.SyncReleasesWithTags(ctx, repo, gitRepo); err != nil { - log.Error("Sync releases from git tags failed: %v", err) + // last - clean up if something goes wrong + // WARNING: Don't override all later err with local variables + defer func() { + if err != nil { + // we can not use the ctx because it maybe canceled or timeout + cleanupRepository(doer, repo.ID) } + }() + + // 2 - check whether the repository with the same storage exists + var isExist bool + isExist, err = gitrepo.IsRepositoryExist(ctx, repo) + if err != nil { + log.Error("Unable to check if %s exists. Error: %v", repo.FullName(), err) + return nil, err + } + if isExist { + log.Error("Files already exist in %s and we are not going to adopt or delete.", repo.FullName()) + // Don't return directly, we need err in defer to cleanupRepository + err = repo_model.ErrRepoFilesAlreadyExist{ + Uname: repo.OwnerName, + Name: repo.Name, + } + return nil, err + } + + // 3 - Clone the repository + cloneCmd := git.NewCommand("clone", "--bare") + if opts.SingleBranch != "" { + cloneCmd.AddArguments("--single-branch", "--branch").AddDynamicArguments(opts.SingleBranch) + } + var stdout []byte + if stdout, _, err = cloneCmd.AddDynamicArguments(opts.BaseRepo.RepoPath(), repo.RepoPath()). + RunStdBytes(ctx, &git.RunOpts{Timeout: 10 * time.Minute}); err != nil { + log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, opts.BaseRepo, stdout, err) + return nil, fmt.Errorf("git clone: %w", err) + } + + // 4 - Update the git repository + if err = updateGitRepoAfterCreate(ctx, repo); err != nil { + return nil, fmt.Errorf("updateGitRepoAfterCreate: %w", err) + } + + // 5 - Create hooks + if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil { + return nil, fmt.Errorf("createDelegateHooks: %w", err) + } + + // 6 - Sync the repository branches and tags + var gitRepo *git.Repository + gitRepo, err = gitrepo.OpenRepository(ctx, repo) + if err != nil { + return nil, fmt.Errorf("OpenRepository: %w", err) + } + defer gitRepo.Close() + + if _, err = repo_module.SyncRepoBranchesWithRepo(ctx, repo, gitRepo, doer.ID); err != nil { + return nil, fmt.Errorf("SyncRepoBranchesWithRepo: %w", err) + } + if err = repo_module.SyncReleasesWithTags(ctx, repo, gitRepo); err != nil { + return nil, fmt.Errorf("Sync releases from git tags failed: %v", err) + } + + // 7 - Update the repository + // even if below operations failed, it could be ignored. And they will be retried + if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { + log.Error("Failed to update size for repository: %v", err) + err = nil + } + if err = repo_model.CopyLanguageStat(ctx, opts.BaseRepo, repo); err != nil { + log.Error("Copy language stat from oldRepo failed: %v", err) + err = nil + } + if err = repo_model.CopyLicense(ctx, opts.BaseRepo, repo); err != nil { + return nil, err + } + + // 8 - update repository status to be ready + repo.Status = repo_model.RepositoryReady + if err = repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil { + return nil, fmt.Errorf("UpdateRepositoryCols: %w", err) } notify_service.ForkRepository(ctx, doer, opts.BaseRepo, repo) diff --git a/services/repository/fork_test.go b/services/repository/fork_test.go index 452798b25b..9edc0aa39f 100644 --- a/services/repository/fork_test.go +++ b/services/repository/fork_test.go @@ -4,13 +4,16 @@ package repository import ( + "os" "testing" + "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -46,3 +49,43 @@ func TestForkRepository(t *testing.T) { assert.Nil(t, fork2) assert.True(t, repo_model.IsErrReachLimitOfRepo(err)) } + +func TestForkRepositoryCleanup(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + // a successful fork + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo10 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) + + fork, err := ForkRepository(git.DefaultContext, user2, user2, ForkRepoOptions{ + BaseRepo: repo10, + Name: "test", + }) + assert.NoError(t, err) + assert.NotNil(t, fork) + + exist, err := util.IsExist(repo_model.RepoPath(user2.Name, "test")) + assert.NoError(t, err) + assert.True(t, exist) + + err = DeleteRepositoryDirectly(db.DefaultContext, user2, fork.ID) + assert.NoError(t, err) + + // a failed creating because some mock data + // create the repository directory so that the creation will fail after database record created. + assert.NoError(t, os.MkdirAll(repo_model.RepoPath(user2.Name, "test"), os.ModePerm)) + + fork2, err := ForkRepository(db.DefaultContext, user2, user2, ForkRepoOptions{ + BaseRepo: repo10, + Name: "test", + }) + assert.Nil(t, fork2) + assert.Error(t, err) + + // assert the cleanup is successful + unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: "test"}) + + exist, err = util.IsExist(repo_model.RepoPath(user2.Name, "test")) + assert.NoError(t, err) + assert.False(t, exist) +} diff --git a/services/repository/generate.go b/services/repository/generate.go index 9d2bbb1f7f..1c4d3b7b63 100644 --- a/services/repository/generate.go +++ b/services/repository/generate.go @@ -17,7 +17,6 @@ import ( git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" - user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" @@ -328,57 +327,6 @@ func (gro GenerateRepoOptions) IsValid() bool { gro.IssueLabels || gro.ProtectedBranch // or other items as they are added } -// generateRepository generates a repository from a template -func generateRepository(ctx context.Context, doer, owner *user_model.User, templateRepo *repo_model.Repository, opts GenerateRepoOptions) (_ *repo_model.Repository, err error) { - generateRepo := &repo_model.Repository{ - OwnerID: owner.ID, - Owner: owner, - OwnerName: owner.Name, - Name: opts.Name, - LowerName: strings.ToLower(opts.Name), - Description: opts.Description, - DefaultBranch: opts.DefaultBranch, - IsPrivate: opts.Private, - IsEmpty: !opts.GitContent || templateRepo.IsEmpty, - IsFsckEnabled: templateRepo.IsFsckEnabled, - TemplateID: templateRepo.ID, - TrustModel: templateRepo.TrustModel, - ObjectFormatName: templateRepo.ObjectFormatName, - } - - if err = CreateRepositoryByExample(ctx, doer, owner, generateRepo, false, false); err != nil { - return nil, err - } - - isExist, err := gitrepo.IsRepositoryExist(ctx, generateRepo) - if err != nil { - log.Error("Unable to check if %s exists. Error: %v", generateRepo.FullName(), err) - return nil, err - } - if isExist { - return nil, repo_model.ErrRepoFilesAlreadyExist{ - Uname: generateRepo.OwnerName, - Name: generateRepo.Name, - } - } - - if err = repo_module.CheckInitRepository(ctx, generateRepo); err != nil { - return generateRepo, err - } - - if err = repo_module.CheckDaemonExportOK(ctx, generateRepo); err != nil { - return generateRepo, fmt.Errorf("checkDaemonExportOK: %w", err) - } - - if stdout, _, err := git.NewCommand("update-server-info"). - RunStdString(ctx, &git.RunOpts{Dir: generateRepo.RepoPath()}); err != nil { - log.Error("GenerateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", generateRepo, stdout, err) - return generateRepo, fmt.Errorf("error in GenerateRepository(git update-server-info): %w", err) - } - - return generateRepo, nil -} - var fileNameSanitizeRegexp = regexp.MustCompile(`(?i)\.\.|[<>:\"/\\|?*\x{0000}-\x{001F}]|^(con|prn|aux|nul|com\d|lpt\d)$`) // Sanitize user input to valid OS filenames diff --git a/services/repository/migrate.go b/services/repository/migrate.go index 2a17e9acd9..003be1a9ab 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -118,14 +118,8 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, repo.Owner = u } - if err := repo_module.CheckDaemonExportOK(ctx, repo); err != nil { - return repo, fmt.Errorf("checkDaemonExportOK: %w", err) - } - - if stdout, _, err := git.NewCommand("update-server-info"). - RunStdString(ctx, &git.RunOpts{Dir: repoPath}); err != nil { - log.Error("MigrateRepositoryGitData(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err) - return repo, fmt.Errorf("error in MigrateRepositoryGitData(git update-server-info): %w", err) + if err := updateGitRepoAfterCreate(ctx, repo); err != nil { + return nil, fmt.Errorf("updateGitRepoAfterCreate: %w", err) } gitRepo, err := git.OpenRepository(ctx, repoPath) diff --git a/services/repository/template.go b/services/repository/template.go index 36a680c8e2..b3f34e4858 100644 --- a/services/repository/template.go +++ b/services/repository/template.go @@ -5,12 +5,17 @@ package repository import ( "context" + "fmt" + "strings" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/log" notify_service "code.gitea.io/gitea/services/notify" ) @@ -69,66 +74,120 @@ func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templ } } - var generateRepo *repo_model.Repository - if err = db.WithTx(ctx, func(ctx context.Context) error { - generateRepo, err = generateRepository(ctx, doer, owner, templateRepo, opts) - if err != nil { - return err - } + generateRepo := &repo_model.Repository{ + OwnerID: owner.ID, + Owner: owner, + OwnerName: owner.Name, + Name: opts.Name, + LowerName: strings.ToLower(opts.Name), + Description: opts.Description, + DefaultBranch: opts.DefaultBranch, + IsPrivate: opts.Private, + IsEmpty: !opts.GitContent || templateRepo.IsEmpty, + IsFsckEnabled: templateRepo.IsFsckEnabled, + TemplateID: templateRepo.ID, + TrustModel: templateRepo.TrustModel, + ObjectFormatName: templateRepo.ObjectFormatName, + Status: repo_model.RepositoryBeingMigrated, + } - // Git Content - if opts.GitContent && !templateRepo.IsEmpty { - if err = GenerateGitContent(ctx, templateRepo, generateRepo); err != nil { - return err - } - } - - // Topics - if opts.Topics { - if err = repo_model.GenerateTopics(ctx, templateRepo, generateRepo); err != nil { - return err - } - } - - // Git Hooks - if opts.GitHooks { - if err = GenerateGitHooks(ctx, templateRepo, generateRepo); err != nil { - return err - } - } - - // Webhooks - if opts.Webhooks { - if err = GenerateWebhooks(ctx, templateRepo, generateRepo); err != nil { - return err - } - } - - // Avatar - if opts.Avatar && len(templateRepo.Avatar) > 0 { - if err = generateAvatar(ctx, templateRepo, generateRepo); err != nil { - return err - } - } - - // Issue Labels - if opts.IssueLabels { - if err = GenerateIssueLabels(ctx, templateRepo, generateRepo); err != nil { - return err - } - } - - if opts.ProtectedBranch { - if err = GenerateProtectedBranch(ctx, templateRepo, generateRepo); err != nil { - return err - } - } - - return nil + // 1 - Create the repository in the database + if err := db.WithTx(ctx, func(ctx context.Context) error { + return createRepositoryInDB(ctx, doer, owner, generateRepo, false) }); err != nil { return nil, err } + // last - clean up the repository if something goes wrong + defer func() { + if err != nil { + // we can not use the ctx because it maybe canceled or timeout + cleanupRepository(doer, generateRepo.ID) + } + }() + + // 2 - check whether the repository with the same storage exists + isExist, err := gitrepo.IsRepositoryExist(ctx, generateRepo) + if err != nil { + log.Error("Unable to check if %s exists. Error: %v", generateRepo.FullName(), err) + return nil, err + } + if isExist { + // Don't return directly, we need err in defer to cleanupRepository + err = repo_model.ErrRepoFilesAlreadyExist{ + Uname: generateRepo.OwnerName, + Name: generateRepo.Name, + } + return nil, err + } + + // 3 -Init git bare new repository. + if err = git.InitRepository(ctx, generateRepo.RepoPath(), true, generateRepo.ObjectFormatName); err != nil { + return nil, fmt.Errorf("git.InitRepository: %w", err) + } else if err = gitrepo.CreateDelegateHooks(ctx, generateRepo); err != nil { + return nil, fmt.Errorf("createDelegateHooks: %w", err) + } + + // 4 - Update the git repository + if err = updateGitRepoAfterCreate(ctx, generateRepo); err != nil { + return nil, fmt.Errorf("updateGitRepoAfterCreate: %w", err) + } + + // 5 - generate the repository contents according to the template + // Git Content + if opts.GitContent && !templateRepo.IsEmpty { + if err = GenerateGitContent(ctx, templateRepo, generateRepo); err != nil { + return nil, err + } + } + + // Topics + if opts.Topics { + if err = repo_model.GenerateTopics(ctx, templateRepo, generateRepo); err != nil { + return nil, err + } + } + + // Git Hooks + if opts.GitHooks { + if err = GenerateGitHooks(ctx, templateRepo, generateRepo); err != nil { + return nil, err + } + } + + // Webhooks + if opts.Webhooks { + if err = GenerateWebhooks(ctx, templateRepo, generateRepo); err != nil { + return nil, err + } + } + + // Avatar + if opts.Avatar && len(templateRepo.Avatar) > 0 { + if err = generateAvatar(ctx, templateRepo, generateRepo); err != nil { + return nil, err + } + } + + // Issue Labels + if opts.IssueLabels { + if err = GenerateIssueLabels(ctx, templateRepo, generateRepo); err != nil { + return nil, err + } + } + + if opts.ProtectedBranch { + if err = GenerateProtectedBranch(ctx, templateRepo, generateRepo); err != nil { + return nil, err + } + } + + // 6 - update repository status to be ready + generateRepo.Status = repo_model.RepositoryReady + if err = repo_model.UpdateRepositoryCols(ctx, generateRepo, "status"); err != nil { + return nil, fmt.Errorf("UpdateRepositoryCols: %w", err) + } + notify_service.CreateRepository(ctx, doer, owner, generateRepo) return generateRepo, nil diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go index da5aaf3da0..c04d09af08 100644 --- a/tests/integration/repo_test.go +++ b/tests/integration/repo_test.go @@ -6,15 +6,23 @@ package integration import ( "fmt" "net/http" + "os" "path" "strconv" "strings" "testing" "time" + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/util" + repo_service "code.gitea.io/gitea/services/repository" "code.gitea.io/gitea/tests" "github.com/PuerkitoBio/goquery" @@ -493,3 +501,46 @@ func testViewCommit(t *testing.T) { resp := MakeRequest(t, req, http.StatusNotFound) assert.True(t, test.IsNormalPageCompleted(resp.Body.String()), "non-existing commit should render 404 page") } + +// TestGenerateRepository the test cannot succeed when moved as a unit test +func TestGenerateRepository(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // a successful generate from template + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo44 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 44}) + + generatedRepo, err := repo_service.GenerateRepository(git.DefaultContext, user2, user2, repo44, repo_service.GenerateRepoOptions{ + Name: "generated-from-template-44", + GitContent: true, + }) + assert.NoError(t, err) + assert.NotNil(t, generatedRepo) + + exist, err := util.IsExist(repo_model.RepoPath(user2.Name, generatedRepo.Name)) + assert.NoError(t, err) + assert.True(t, exist) + + unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: generatedRepo.Name}) + + err = repo_service.DeleteRepositoryDirectly(db.DefaultContext, user2, generatedRepo.ID) + assert.NoError(t, err) + + // a failed creating because some mock data + // create the repository directory so that the creation will fail after database record created. + assert.NoError(t, os.MkdirAll(repo_model.RepoPath(user2.Name, "generated-from-template-44"), os.ModePerm)) + + generatedRepo2, err := repo_service.GenerateRepository(db.DefaultContext, user2, user2, repo44, repo_service.GenerateRepoOptions{ + Name: "generated-from-template-44", + GitContent: true, + }) + assert.Nil(t, generatedRepo2) + assert.Error(t, err) + + // assert the cleanup is successful + unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: generatedRepo.Name}) + + exist, err = util.IsExist(repo_model.RepoPath(user2.Name, generatedRepo.Name)) + assert.NoError(t, err) + assert.False(t, exist) +}