Rework create/fork/adopt/generate repository to make sure resources will be cleanup once failed

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

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

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)

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,50 @@ 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)
-		}
+	// 3 - adopt the repository from disk
+	if err = adoptRepository(ctx, repo, opts.DefaultBranch); err != nil {
+		return nil, fmt.Errorf("adoptRepository: %w", err)
+	}
 
-		if err := repo_module.CheckDaemonExportOK(ctx, repo); err != nil {
-			return fmt.Errorf("checkDaemonExportOK: %w", err)
-		}
+	if err = updateGitRepoAfterCreate(ctx, repo); err != nil {
+		return nil, fmt.Errorf("updateGitRepoAfterCreate: %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
+	// 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

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,34 @@ 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")))
+
+	// a successful adopt
+	destDir := filepath.Join(setting.RepoRootPath, "user2", "test-adopt.git")
+	assert.NoError(t, unittest.SyncDirs(filepath.Join(setting.RepoRootPath, "user2", "repo1.git"), destDir))
 	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
-	_, err := AdoptRepository(db.DefaultContext, user2, user2, CreateRepoOptions{Name: "test-adopt"})
+	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: "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: "test-adopt"})
+
+	exist, err := util.IsExist(repo_model.RepoPath("user2", "test-adopt"))
+	assert.NoError(t, err)
+	assert.True(t, exist) // the repository should be still in the disk
 }