From 7065637e61d91a0262a4df5faa2e32e2c528be8c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 24 Jul 2026 01:42:55 +0800 Subject: [PATCH] refactor: hide git repo path details from more packages (#38601) Remove `RepoPath` from "models/repo" package, remove `UserPath` from "models/user" package, use `WithRepo` for more places, fine tune tests. --- models/repo/repo.go | 11 --- models/repo/repo_location.go | 4 +- models/user/user.go | 7 -- modules/git/gitrepo/dbrepo.go | 10 ++- modules/git/gitrepo/gitrepo.go | 14 ++++ modules/git/localfs.go | 5 -- modules/git/repo_compare.go | 3 +- routers/web/repo/githttp.go | 3 +- routers/web/user/setting/profile.go | 3 +- services/gitdiff/gitdiff_test.go | 4 +- services/org/org.go | 3 +- services/pull/merge_tree_test.go | 13 ++-- services/pull/temp_repo.go | 5 +- services/repository/adopt_test.go | 53 +++++++------- services/repository/create_test.go | 60 +++++++++------- services/repository/fork_test.go | 64 +++++++++-------- services/repository/transfer_test.go | 13 ++-- services/user/user.go | 7 +- tests/integration/compare_test.go | 3 +- tests/integration/mirror_pull_test.go | 5 +- tests/integration/pull_merge_test.go | 13 ++-- tests/integration/repo_test.go | 99 ++++++++++++++------------- 22 files changed, 216 insertions(+), 186 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 2161ae564d..f3ad15b367 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -11,7 +11,6 @@ import ( "maps" "net" "net/url" - "path/filepath" "regexp" "strconv" "strings" @@ -564,16 +563,6 @@ func (repo *Repository) IsGenerated() bool { return repo.TemplateID != 0 } -// RepoPath returns repository path by given user and repository name. -func RepoPath(userName, repoName string) string { //revive:disable-line:exported - return filepath.Join(setting.RepoRootPath, filepath.Clean(strings.ToLower(userName)), filepath.Clean(strings.ToLower(repoName)+".git")) -} - -// RepoPath returns the repository path -func (repo *Repository) RepoPath() string { - return RepoPath(repo.OwnerName, repo.Name) -} - // Link returns the repository relative url func (repo *Repository) Link() string { return setting.AppSubURL + "/" + url.PathEscape(repo.OwnerName) + "/" + url.PathEscape(repo.Name) diff --git a/models/repo/repo_location.go b/models/repo/repo_location.go index 1057eee8d7..747d1a7f76 100644 --- a/models/repo/repo_location.go +++ b/models/repo/repo_location.go @@ -15,8 +15,8 @@ func repoCodeGitRepoManagedID(repoID int64) string { func (repo *Repository) CodeStorageRepo() gitrepo.RepositoryFacade { id := repoCodeGitRepoManagedID(repo.ID) - repoPath := gitrepo.RepoCodeGitRepoRelativePath(repo.OwnerName, repo.Name) - return gitrepo.RepositoryManaged(id, repoPath) + relPath := gitrepo.RepoCodeGitRepoRelativePath(repo.OwnerName, repo.Name) + return gitrepo.RepositoryManaged(id, relPath) } func (repo *Repository) GitRepoLocation() string { diff --git a/models/user/user.go b/models/user/user.go index a34ca9ec1f..804d2b7a4e 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -13,7 +13,6 @@ import ( "mime" "net/mail" "net/url" - "path/filepath" "regexp" "strconv" "strings" @@ -993,12 +992,6 @@ func GetInactiveUsers(ctx context.Context, olderThan time.Duration) ([]*User, er Find(&users) } -// UserPath returns the path absolute path of user repositories. -// FIXME: it should be in "git/gitrepo" package -func UserPath(userName string) string { //revive:disable-line:exported - return filepath.Join(setting.RepoRootPath, filepath.Clean(strings.ToLower(userName))) -} - // GetUserByID returns the user object by given ID if exists. func GetUserByID(ctx context.Context, id int64) (*User, error) { u := new(User) diff --git a/modules/git/gitrepo/dbrepo.go b/modules/git/gitrepo/dbrepo.go index 0c555d3e88..85486fb1bb 100644 --- a/modules/git/gitrepo/dbrepo.go +++ b/modules/git/gitrepo/dbrepo.go @@ -3,14 +3,18 @@ package gitrepo -import "strings" +import ( + "strings" + + "gitea.dev/modules/util" +) func RepoCodeGitRepoRelativePath(ownerName, repoName string) string { - return strings.ToLower(ownerName) + "/" + strings.ToLower(repoName) + ".git" + return util.PathJoinRelX(strings.ToLower(ownerName), strings.ToLower(repoName)+".git") } func RepoWikiGitRepoRelativePath(ownerName, repoName string) string { - return strings.ToLower(ownerName) + "/" + strings.ToLower(repoName) + ".wiki.git" + return util.PathJoinRelX(strings.ToLower(ownerName), strings.ToLower(repoName)+".wiki.git") } // CodeRepoByName returns an unmanaged repository facade for the code repository of the given owner and repository name. diff --git a/modules/git/gitrepo/gitrepo.go b/modules/git/gitrepo/gitrepo.go index 903fd8c770..bdd55459c5 100644 --- a/modules/git/gitrepo/gitrepo.go +++ b/modules/git/gitrepo/gitrepo.go @@ -4,7 +4,10 @@ package gitrepo import ( + "io/fs" + "os" "path/filepath" + "strings" "sync/atomic" "gitea.dev/modules/setting" @@ -39,6 +42,13 @@ func RepoLocalPath(repo RepositoryFacade) string { return setting.RepoRootPath + string(filepath.Separator) + filepath.FromSlash(repoLoc) } +func UserLocalPath(userName string) string { + if setting.RepoRootPath == "" { + panic("repo root path is not initialized") + } + return filepath.Join(setting.RepoRootPath, filepath.Clean(strings.ToLower(userName))) +} + func repoLogNameByLocation(loc string) string { t := filepath.FromSlash(loc) // hide the parent paths, then the name should be safe for end users @@ -100,3 +110,7 @@ func (r *repositoryManaged) GitRepoLocation() string { func RepositoryManaged(id, loc string) RepositoryFacade { return &repositoryManaged{id: id, loc: filepath.Clean(loc)} } + +func RepoLocalFS(repo RepositoryFacade) fs.FS { + return os.DirFS(RepoLocalPath(repo)) +} diff --git a/modules/git/localfs.go b/modules/git/localfs.go index 8c8d8439b0..5bd867239d 100644 --- a/modules/git/localfs.go +++ b/modules/git/localfs.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "io" - "io/fs" "os" "path/filepath" @@ -43,10 +42,6 @@ func InitRepository(ctx context.Context, repo RepositoryFacade, objectFormatName return InitRepositoryLocal(ctx, gitrepo.RepoLocalPath(repo), true, objectFormatName) } -func GetRepoFS(repo RepositoryFacade) fs.FS { - return os.DirFS(gitrepo.RepoLocalPath(repo)) -} - func IsRepoFileExist(ctx context.Context, repo RepositoryFacade, relativeFilePath string) (bool, error) { absoluteFilePath := filepath.Join(gitrepo.RepoLocalPath(repo), relativeFilePath) return util.IsExist(absoluteFilePath) diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index c20ebc020c..7475df16f0 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -15,6 +15,7 @@ import ( "strings" "gitea.dev/modules/git/gitcmd" + "gitea.dev/modules/git/gitrepo" ) type lineCountWriter struct { @@ -113,7 +114,7 @@ func (repo *Repository) GetFilesChangedBetween(ctx context.Context, base, head s // ReadPatchCommit will check if a diff patch exists and return stats func (repo *Repository) ReadPatchCommit(prID int64) (commitSHA string, err error) { // Migrated repositories download patches to "pulls" location - repoFS := GetRepoFS(repo) + repoFS := gitrepo.RepoLocalFS(repo) loadPatch, err := repoFS.Open(fmt.Sprintf("pulls/%d.patch", prID)) if err != nil { return "", err diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index c8730b5961..da8c09f919 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -24,6 +24,7 @@ import ( "gitea.dev/models/unit" "gitea.dev/modules/git" "gitea.dev/modules/git/gitcmd" + "gitea.dev/modules/git/gitrepo" "gitea.dev/modules/log" repo_module "gitea.dev/modules/repository" "gitea.dev/modules/setting" @@ -342,7 +343,7 @@ func (h *serviceHandler) sendFile(ctx *context.Context, contentType, file string return } - fs := git.GetRepoFS(h.getStorageRepo()) + fs := gitrepo.RepoLocalFS(h.getStorageRepo()) ctx.Resp.Header().Set("Content-Type", contentType) http.ServeFileFS(ctx.Resp, ctx.Req, fs, path.Clean(file)) } diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go index 57053a0861..ce52d40e10 100644 --- a/routers/web/user/setting/profile.go +++ b/routers/web/user/setting/profile.go @@ -19,6 +19,7 @@ import ( "gitea.dev/models/organization" repo_model "gitea.dev/models/repo" user_model "gitea.dev/models/user" + "gitea.dev/modules/git/gitrepo" "gitea.dev/modules/log" "gitea.dev/modules/optional" "gitea.dev/modules/setting" @@ -251,7 +252,7 @@ func Repos(ctx *context.Context) { repoNames := make([]string, 0, setting.UI.Admin.UserPagingNum) repos := map[string]*repo_model.Repository{} // We're going to iterate by pagesize. - root := user_model.UserPath(ctxUser.Name) + root := gitrepo.UserLocalPath(ctxUser.Name) if err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { if err != nil { if os.IsNotExist(err) { diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index 1b5ea391e1..c27cc0cdc1 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -1186,9 +1186,9 @@ revert from :2 D test2.txt D test10.txt` - require.NoError(t, gitcmd.NewCommand("fast-import").WithDir(pull.BaseRepo.RepoPath()).WithStdinBytes([]byte(stdin)).Run(t.Context())) + require.NoError(t, gitcmd.NewCommand("fast-import").WithRepo(pull.BaseRepo).WithStdinBytes([]byte(stdin)).Run(t.Context())) - gitRepo, err := git.OpenRepositoryLocal(pull.BaseRepo.RepoPath()) + gitRepo, err := git.OpenRepository(pull.BaseRepo) assert.NoError(t, err) defer gitRepo.Close() diff --git a/services/org/org.go b/services/org/org.go index e51331cbe7..953e869426 100644 --- a/services/org/org.go +++ b/services/org/org.go @@ -16,6 +16,7 @@ import ( repo_model "gitea.dev/models/repo" secret_model "gitea.dev/models/secret" user_model "gitea.dev/models/user" + "gitea.dev/modules/git/gitrepo" issue_indexer "gitea.dev/modules/indexer/issues" "gitea.dev/modules/storage" "gitea.dev/modules/structs" @@ -87,7 +88,7 @@ func DeleteOrganization(ctx context.Context, org *org_model.Organization, purge // FIXME: system notice // Note: There are something just cannot be roll back, // so just keep error logs of those operations. - path := user_model.UserPath(org.Name) + path := gitrepo.UserLocalPath(org.Name) if err := util.RemoveAllWithRetry(path); err != nil { return fmt.Errorf("failed to RemoveAll %s: %w", path, err) diff --git a/services/pull/merge_tree_test.go b/services/pull/merge_tree_test.go index 91d6a9152f..7b5c47d832 100644 --- a/services/pull/merge_tree_test.go +++ b/services/pull/merge_tree_test.go @@ -11,6 +11,7 @@ import ( issues_model "gitea.dev/models/issues" "gitea.dev/models/unittest" "gitea.dev/modules/git/gitcmd" + "gitea.dev/modules/git/gitrepo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -51,7 +52,7 @@ func TestPullRequestMergeable(t *testing.T) { }) pr.BaseBranch, pr.HeadBranch = "test-merge-tree-conflict-base", "test-merge-tree-conflict-head" - conflictFiles := createConflictBranches(t, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.HeadBranch) + conflictFiles := createConflictBranches(t, pr.BaseRepo, pr.BaseBranch, pr.HeadBranch) t.Run("Conflict-MergeTree", func(t *testing.T) { testPullRequestMergeCheck(t, checkPullRequestMergeableByMergeTree, pr, issues_model.PullRequestStatusConflict, conflictFiles, nil) }) @@ -60,7 +61,7 @@ func TestPullRequestMergeable(t *testing.T) { }) pr.BaseBranch, pr.HeadBranch = "test-merge-tree-empty-base", "test-merge-tree-empty-head" - createEmptyBranches(t, pr.BaseRepo.RepoPath(), pr.BaseBranch, pr.HeadBranch) + createEmptyBranches(t, pr.BaseRepo, pr.BaseBranch, pr.HeadBranch) t.Run("Empty-MergeTree", func(t *testing.T) { testPullRequestMergeCheck(t, checkPullRequestMergeableByMergeTree, pr, issues_model.PullRequestStatusEmpty, nil, nil) }) @@ -69,7 +70,7 @@ func TestPullRequestMergeable(t *testing.T) { }) } -func createConflictBranches(t *testing.T, repoPath, baseBranch, headBranch string) []string { +func createConflictBranches(t *testing.T, repo gitrepo.RepositoryFacade, baseBranch, headBranch string) []string { conflictFile := "conflict.txt" stdin := fmt.Sprintf( `reset refs/heads/%[1]s @@ -107,12 +108,12 @@ M 100644 inline %[3]s data 11 head change `, baseBranch, headBranch, conflictFile) - err := gitcmd.NewCommand("fast-import").WithDir(repoPath).WithStdinBytes([]byte(stdin)).RunWithStderr(t.Context()) + err := gitcmd.NewCommand("fast-import").WithRepo(repo).WithStdinBytes([]byte(stdin)).RunWithStderr(t.Context()) require.NoError(t, err) return []string{conflictFile} } -func createEmptyBranches(t *testing.T, repoPath, baseBranch, headBranch string) { +func createEmptyBranches(t *testing.T, repo gitrepo.RepositoryFacade, baseBranch, headBranch string) { emptyFile := "empty.txt" stdin := fmt.Sprintf(`reset refs/heads/%[1]s from refs/heads/master @@ -149,6 +150,6 @@ M 100644 inline %[3]s data 4 base `, baseBranch, headBranch, emptyFile) - err := gitcmd.NewCommand("fast-import").WithDir(repoPath).WithStdinBytes([]byte(stdin)).RunWithStderr(t.Context()) + err := gitcmd.NewCommand("fast-import").WithRepo(repo).WithStdinBytes([]byte(stdin)).RunWithStderr(t.Context()) require.NoError(t, err) } diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index 2b92baf1a3..8b06aa45e5 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -16,6 +16,7 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/modules/git" "gitea.dev/modules/git/gitcmd" + "gitea.dev/modules/git/gitrepo" repo_module "gitea.dev/modules/repository" ) @@ -80,8 +81,8 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) outbuf: &bytes.Buffer{}, } - baseRepoPath := pr.BaseRepo.RepoPath() - headRepoPath := pr.HeadRepo.RepoPath() + baseRepoPath := gitrepo.RepoLocalPath(pr.BaseRepo.CodeStorageRepo()) + headRepoPath := gitrepo.RepoLocalPath(pr.HeadRepo.CodeStorageRepo()) if err := git.InitRepositoryLocal(ctx, tmpBasePath, false, pr.BaseRepo.ObjectFormatName); err != nil { return nil, nil, fmt.Errorf("InitRepository[PR:%d]: %w", pr.ID, err) diff --git a/services/repository/adopt_test.go b/services/repository/adopt_test.go index 89a506f669..d1077ca625 100644 --- a/services/repository/adopt_test.go +++ b/services/repository/adopt_test.go @@ -13,8 +13,9 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" + "gitea.dev/modules/git" + "gitea.dev/modules/git/gitrepo" "gitea.dev/modules/setting" - "gitea.dev/modules/util" "github.com/stretchr/testify/assert" ) @@ -91,35 +92,39 @@ func TestListUnadoptedRepositories_ListOptions(t *testing.T) { func TestAdoptRepository(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) + testRepoName := "test-adopt" user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + destDir := filepath.Join(setting.RepoRootPath, user2.Name, testRepoName+".git") - // 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)) + t.Run("Success", func(t *testing.T) { + // a successful adopt + assert.NoError(t, unittest.SyncDirs(filepath.Join(setting.RepoRootPath, user2.Name, "repo1.git"), destDir)) - adoptedRepo, err := AdoptRepository(t.Context(), 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) + adoptedRepo, err := AdoptRepository(t.Context(), user2, user2, CreateRepoOptions{Name: testRepoName}) + assert.NoError(t, err) + repoTestAdopt := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{Name: testRepoName}) + assert.Equal(t, "sha1", repoTestAdopt.ObjectFormatName) - // just delete the adopted repo's db records - err = deleteFailedAdoptRepository(adoptedRepo.ID) - assert.NoError(t, err) + // 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"}) + t.Run("Failure", func(t *testing.T) { + unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: testRepoName}) + // 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("dummy-content"), os.ModePerm)) - // 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(t.Context(), user2, user2, CreateRepoOptions{Name: testRepoName}) + assert.Error(t, err) + assert.Nil(t, adoptedRepo) - adoptedRepo, err = AdoptRepository(t.Context(), user2, user2, CreateRepoOptions{Name: "test-adopt"}) - assert.Error(t, err) - assert.Nil(t, adoptedRepo) + unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: testRepoName}) - 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 + exist, err := git.IsRepositoryExist(t.Context(), gitrepo.CodeRepoByName(user2.Name, testRepoName)) + assert.NoError(t, err) + assert.True(t, exist) // the repository should be still in the disk + }) } diff --git a/services/repository/create_test.go b/services/repository/create_test.go index 60f9f2b43b..86dd6188a3 100644 --- a/services/repository/create_test.go +++ b/services/repository/create_test.go @@ -10,7 +10,8 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" - "gitea.dev/modules/util" + "gitea.dev/modules/git" + "gitea.dev/modules/git/gitrepo" "github.com/stretchr/testify/assert" ) @@ -20,36 +21,43 @@ func TestCreateRepositoryDirectly(t *testing.T) { // a successful creating repository user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + testRepoName := "created-repo" + t.Run("Success", func(t *testing.T) { + createdRepo, err := CreateRepositoryDirectly(t.Context(), user2, user2, CreateRepoOptions{ + Name: testRepoName, + }, true) + assert.NoError(t, err) + assert.NotNil(t, createdRepo) - createdRepo, err := CreateRepositoryDirectly(t.Context(), user2, user2, CreateRepoOptions{ - Name: "created-repo", - }, true) - assert.NoError(t, err) - assert.NotNil(t, createdRepo) + exist, err := git.IsRepositoryExist(t.Context(), gitrepo.CodeRepoByName(user2.Name, createdRepo.Name)) + assert.NoError(t, err) + assert.True(t, exist) - 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}) - unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: createdRepo.Name}) + err = DeleteRepositoryDirectly(t.Context(), createdRepo.ID) + assert.NoError(t, err) + }) - err = DeleteRepositoryDirectly(t.Context(), createdRepo.ID) - assert.NoError(t, err) + t.Run("Failure", func(t *testing.T) { + // a failed creating because some mock data + // create the repository directory so that the creation will fail after database record created. + testFailureRepoName := testRepoName + testFailureRepo := gitrepo.CodeRepoByName(user2.Name, testFailureRepoName) + testFailurePath := gitrepo.RepoLocalPath(testFailureRepo) + assert.NoError(t, os.MkdirAll(testFailurePath, os.ModePerm)) - // 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(t.Context(), user2, user2, CreateRepoOptions{ + Name: testFailureRepoName, + }, true) + assert.Nil(t, createdRepo2) + assert.Error(t, err) - createdRepo2, err := CreateRepositoryDirectly(t.Context(), user2, user2, CreateRepoOptions{ - Name: "created-repo", - }, true) - assert.Nil(t, createdRepo2) - assert.Error(t, err) + // assert the cleanup is successful + unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: testFailureRepoName}) - // 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) + exist, err := git.IsRepositoryExist(t.Context(), testFailureRepo) + assert.NoError(t, err) + assert.False(t, exist) + }) } diff --git a/services/repository/fork_test.go b/services/repository/fork_test.go index 2549e69561..8929b5310e 100644 --- a/services/repository/fork_test.go +++ b/services/repository/fork_test.go @@ -10,9 +10,10 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" + "gitea.dev/modules/git" + "gitea.dev/modules/git/gitrepo" "gitea.dev/modules/setting" "gitea.dev/modules/test" - "gitea.dev/modules/util" "github.com/stretchr/testify/assert" ) @@ -52,39 +53,46 @@ func TestForkRepository(t *testing.T) { 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(t.Context(), user2, user2, ForkRepoOptions{ - BaseRepo: repo10, - Name: "test", + t.Run("Success", func(t *testing.T) { + // a successful fork + + fork, err := ForkRepository(t.Context(), user2, user2, ForkRepoOptions{ + BaseRepo: repo10, + Name: "test", + }) + assert.NoError(t, err) + assert.NotNil(t, fork) + + exist, err := git.IsRepositoryExist(t.Context(), gitrepo.CodeRepoByName(user2.Name, "test")) + assert.NoError(t, err) + assert.True(t, exist) + + err = DeleteRepositoryDirectly(t.Context(), fork.ID) + assert.NoError(t, err) }) - assert.NoError(t, err) - assert.NotNil(t, fork) + t.Run("Failure", func(t *testing.T) { + // a failed creating because some mock data + // create the repository directory so that the creation will fail after database record created. + testFailureRepoName := "test" + testFailureRepo := gitrepo.CodeRepoByName(user2.Name, testFailureRepoName) + testFailurePath := gitrepo.RepoLocalPath(testFailureRepo) + assert.NoError(t, os.MkdirAll(testFailurePath, os.ModePerm)) - exist, err := util.IsExist(repo_model.RepoPath(user2.Name, "test")) - assert.NoError(t, err) - assert.True(t, exist) + forkFailure, err := ForkRepository(t.Context(), user2, user2, ForkRepoOptions{ + BaseRepo: repo10, + Name: testFailureRepoName, + }) + assert.Nil(t, forkFailure) + assert.Error(t, err) - err = DeleteRepositoryDirectly(t.Context(), fork.ID) - assert.NoError(t, err) + // assert the cleanup is successful + unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: testFailureRepoName}) - // 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(t.Context(), user2, user2, ForkRepoOptions{ - BaseRepo: repo10, - Name: "test", + exist, err := git.IsRepositoryExist(t.Context(), testFailureRepo) + assert.NoError(t, err) + assert.False(t, exist) }) - 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/transfer_test.go b/services/repository/transfer_test.go index 85282a1135..02a62d531a 100644 --- a/services/repository/transfer_test.go +++ b/services/repository/transfer_test.go @@ -13,9 +13,9 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" + "gitea.dev/modules/git" "gitea.dev/modules/setting" "gitea.dev/modules/test" - "gitea.dev/modules/util" "gitea.dev/services/feed" notify_service "gitea.dev/services/notify" @@ -37,20 +37,21 @@ func TestTransferOwnership(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) - assert.NoError(t, repo.LoadOwner(t.Context())) + sourceRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + sourceRepoBak := new(*sourceRepo) + assert.NoError(t, sourceRepo.LoadOwner(t.Context())) repoTransfer := unittest.AssertExistsAndLoadBean(t, &repo_model.RepoTransfer{ID: 1}) assert.NoError(t, repoTransfer.LoadAttributes(t.Context())) - assert.NoError(t, AcceptTransferOwnership(t.Context(), repo, doer)) + assert.NoError(t, AcceptTransferOwnership(t.Context(), sourceRepo, doer)) transferredRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) assert.EqualValues(t, 1, transferredRepo.OwnerID) // repo_transfer.yml id=1 unittest.AssertNotExistsBean(t, &repo_model.RepoTransfer{ID: 1}) - exist, err := util.IsExist(repo_model.RepoPath("org3", "repo3")) + exist, err := git.IsRepositoryExist(t.Context(), sourceRepoBak) assert.NoError(t, err) assert.False(t, exist) - exist, err = util.IsExist(repo_model.RepoPath("user1", "repo3")) + exist, err = git.IsRepositoryExist(t.Context(), transferredRepo) assert.NoError(t, err) assert.True(t, exist) unittest.AssertExistsAndLoadBean(t, &activities_model.Action{ diff --git a/services/user/user.go b/services/user/user.go index 43052819d6..8c40102fb4 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -17,6 +17,7 @@ import ( system_model "gitea.dev/models/system" user_model "gitea.dev/models/user" "gitea.dev/modules/eventsource" + "gitea.dev/modules/git/gitrepo" "gitea.dev/modules/log" "gitea.dev/modules/setting" "gitea.dev/modules/storage" @@ -98,7 +99,7 @@ func RenameUser(ctx context.Context, u *user_model.User, newUserName string, doe } // Do not fail if directory does not exist - if err = util.RenameWithRetry(user_model.UserPath(oldUserName), user_model.UserPath(newUserName)); err != nil && !os.IsNotExist(err) { + if err = util.RenameWithRetry(gitrepo.UserLocalPath(oldUserName), gitrepo.UserLocalPath(newUserName)); err != nil && !os.IsNotExist(err) { u.Name = oldUserName u.LowerName = strings.ToLower(oldUserName) return fmt.Errorf("rename user directory: %w", err) @@ -107,7 +108,7 @@ func RenameUser(ctx context.Context, u *user_model.User, newUserName string, doe if err = committer.Commit(); err != nil { u.Name = oldUserName u.LowerName = strings.ToLower(oldUserName) - if err2 := util.RenameWithRetry(user_model.UserPath(newUserName), user_model.UserPath(oldUserName)); err2 != nil && !os.IsNotExist(err2) { + if err2 := util.RenameWithRetry(gitrepo.UserLocalPath(newUserName), gitrepo.UserLocalPath(oldUserName)); err2 != nil && !os.IsNotExist(err2) { log.Error("Unable to rollback directory change during failed username change from: %s to: %s. DB Error: %v. Filesystem Error: %v", oldUserName, newUserName, err, err2) return fmt.Errorf("failed to rollback directory change during failed username change from: %s to: %s. DB Error: %w. Filesystem Error: %v", oldUserName, newUserName, err, err2) } @@ -257,7 +258,7 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error { } // Note: There are something just cannot be roll back, so just keep error logs of those operations. - path := user_model.UserPath(u.Name) + path := gitrepo.UserLocalPath(u.Name) if err := util.RemoveAllWithRetry(path); err != nil { err = fmt.Errorf("failed to RemoveAll %s: %w", path, err) _ = system_model.CreateNotice(ctx, system_model.NoticeTask, fmt.Sprintf("delete user '%s': %v", u.Name, err)) diff --git a/tests/integration/compare_test.go b/tests/integration/compare_test.go index 817d7ea536..4975c853ea 100644 --- a/tests/integration/compare_test.go +++ b/tests/integration/compare_test.go @@ -211,8 +211,7 @@ func TestCompareBranchesNoCommonMergeBase(t *testing.T) { user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"}) repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: user2.ID, Name: "repo1"}) - repoPath := repo_model.RepoPath(user2.Name, repo1.Name) - _, _, runErr := gitcmd.NewCommand("fast-import").WithDir(repoPath).WithStdinBytes([]byte(strings.TrimSpace(` + _, _, runErr := gitcmd.NewCommand("fast-import").WithRepo(repo1).WithStdinBytes([]byte(strings.TrimSpace(` commit refs/heads/unrelated-history committer User 1714310400 +0000 data 13 diff --git a/tests/integration/mirror_pull_test.go b/tests/integration/mirror_pull_test.go index ecc3f316ab..82fc41efc6 100644 --- a/tests/integration/mirror_pull_test.go +++ b/tests/integration/mirror_pull_test.go @@ -16,6 +16,7 @@ import ( "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/git" + "gitea.dev/modules/git/gitrepo" "gitea.dev/modules/migration" "gitea.dev/modules/setting" "gitea.dev/modules/test" @@ -35,7 +36,7 @@ func TestMirrorPull(t *testing.T) { ctx := t.Context() user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - repoPath := repo_model.RepoPath(user.Name, repo.Name) + repoPath := gitrepo.RepoLocalPath(repo) opts := migration.MigrateOptions{ RepoName: "test_mirror", @@ -140,7 +141,7 @@ func TestMirrorPullSSRFRevalidation(t *testing.T) { ctx := t.Context() user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - repoPath := repo_model.RepoPath(user.Name, repo.Name) + repoPath := gitrepo.RepoLocalPath(repo) // an "internal" server that records whether it was reached var reached atomic.Bool diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index d775269dbb..57a766cfd2 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -385,13 +385,12 @@ func TestCantMergeUnrelated(t *testing.T) { OwnerID: user1.ID, Name: "repo1", }) - path := repo_model.RepoPath(user1.Name, repo1.Name) - err := gitcmd.NewCommand("read-tree", "--empty").WithDir(path).Run(t.Context()) + err := gitcmd.NewCommand("read-tree", "--empty").WithRepo(repo1).Run(t.Context()) assert.NoError(t, err) stdout, _, err := gitcmd.NewCommand("hash-object", "-w", "--stdin"). - WithDir(path). + WithRepo(repo1). WithStdinBytes([]byte("Unrelated File")). RunStdString(t.Context()) @@ -400,11 +399,11 @@ func TestCantMergeUnrelated(t *testing.T) { _, _, err = gitcmd.NewCommand("update-index", "--add", "--replace", "--cacheinfo"). AddDynamicArguments("100644", sha, "somewhere-over-the-rainbow"). - WithDir(path). + WithRepo(repo1). RunStdString(t.Context()) assert.NoError(t, err) - treeSha, _, err := gitcmd.NewCommand("write-tree").WithDir(path).RunStdString(t.Context()) + treeSha, _, err := gitcmd.NewCommand("write-tree").WithRepo(repo1).RunStdString(t.Context()) assert.NoError(t, err) treeSha = strings.TrimSpace(treeSha) @@ -425,7 +424,7 @@ func TestCantMergeUnrelated(t *testing.T) { stdout, _, err = gitcmd.NewCommand("commit-tree").AddDynamicArguments(treeSha). WithEnv(env). - WithDir(path). + WithRepo(repo1). WithStdinBytes(messageBytes.Bytes()). RunStdString(t.Context()) assert.NoError(t, err) @@ -433,7 +432,7 @@ func TestCantMergeUnrelated(t *testing.T) { _, _, err = gitcmd.NewCommand("branch", "unrelated"). AddDynamicArguments(commitSha). - WithDir(path). + WithRepo(repo1). RunStdString(t.Context()) assert.NoError(t, err) diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go index d1b1f017e9..06380305b8 100644 --- a/tests/integration/repo_test.go +++ b/tests/integration/repo_test.go @@ -21,9 +21,9 @@ import ( "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/git" + "gitea.dev/modules/git/gitrepo" "gitea.dev/modules/setting" "gitea.dev/modules/test" - "gitea.dev/modules/util" repo_service "gitea.dev/services/repository" "gitea.dev/tests" @@ -618,57 +618,64 @@ func TestGenerateRepository(t *testing.T) { user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) repo44 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 44}) - tmplRepoLabels := []*issues_model.Label{ - {RepoID: 44, Name: "priority/high", Exclusive: true, ExclusiveOrder: 2, Color: "#ee0000", Description: "desc-high"}, - {RepoID: 44, Name: "priority/low", Exclusive: true, ExclusiveOrder: 1, Color: "#0000ee", Description: "desc-low"}, - } + t.Run("Success", func(t *testing.T) { + tmplRepoLabels := []*issues_model.Label{ + {RepoID: 44, Name: "priority/high", Exclusive: true, ExclusiveOrder: 2, Color: "#ee0000", Description: "desc-high"}, + {RepoID: 44, Name: "priority/low", Exclusive: true, ExclusiveOrder: 1, Color: "#0000ee", Description: "desc-low"}, + } - require.NoError(t, issues_model.NewLabels(t.Context(), tmplRepoLabels...)) + require.NoError(t, issues_model.NewLabels(t.Context(), tmplRepoLabels...)) - generatedRepo, err := repo_service.GenerateRepository(t.Context(), user2, user2, repo44, repo_service.GenerateRepoOptions{ - Name: "generated-from-template-44", - GitContent: true, - IssueLabels: true, + generatedRepo, err := repo_service.GenerateRepository(t.Context(), user2, user2, repo44, repo_service.GenerateRepoOptions{ + Name: "generated-from-template-44", + GitContent: true, + IssueLabels: true, + }) + require.NoError(t, err) + require.NotNil(t, generatedRepo) + + exist, err := git.IsRepositoryExist(t.Context(), generatedRepo) + require.NoError(t, err) + require.True(t, exist) + + unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: generatedRepo.Name}) + + generatedLabels, err := issues_model.GetLabelsByRepoID(t.Context(), generatedRepo.ID, "", db.ListOptions{}) + require.NoError(t, err) + require.Len(t, generatedLabels, len(tmplRepoLabels)) + for i, tmplLabel := range tmplRepoLabels { + genLabel := generatedLabels[i] + assert.Equal(t, tmplLabel.Name, genLabel.Name) + assert.Equal(t, tmplLabel.Exclusive, genLabel.Exclusive) + assert.Equal(t, tmplLabel.ExclusiveOrder, genLabel.ExclusiveOrder) + assert.Equal(t, tmplLabel.Color, genLabel.Color) + assert.Equal(t, tmplLabel.Description, genLabel.Description) + } + + err = repo_service.DeleteRepositoryDirectly(t.Context(), generatedRepo.ID) + assert.NoError(t, err) }) - require.NoError(t, err) - require.NotNil(t, generatedRepo) - exist, err := util.IsExist(repo_model.RepoPath(user2.Name, generatedRepo.Name)) - require.NoError(t, err) - require.True(t, exist) + t.Run("Failure", func(t *testing.T) { + // a failed creating because some mock data + // create the repository directory so that the creation will fail after database record created. + testFailureRepoName := "generated-from-template-44" + testFailureRepo := gitrepo.CodeRepoByName(user2.Name, testFailureRepoName) + testFailurePath := gitrepo.RepoLocalPath(testFailureRepo) + assert.NoError(t, os.MkdirAll(testFailurePath, os.ModePerm)) - unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: generatedRepo.Name}) + generatedRepoFailure, err := repo_service.GenerateRepository(t.Context(), user2, user2, repo44, repo_service.GenerateRepoOptions{ + Name: testFailureRepoName, + GitContent: true, + }) + assert.Nil(t, generatedRepoFailure) + assert.Error(t, err) - generatedLabels, err := issues_model.GetLabelsByRepoID(t.Context(), generatedRepo.ID, "", db.ListOptions{}) - require.NoError(t, err) - require.Len(t, generatedLabels, len(tmplRepoLabels)) - for i, tmplLabel := range tmplRepoLabels { - genLabel := generatedLabels[i] - assert.Equal(t, tmplLabel.Name, genLabel.Name) - assert.Equal(t, tmplLabel.Exclusive, genLabel.Exclusive) - assert.Equal(t, tmplLabel.ExclusiveOrder, genLabel.ExclusiveOrder) - assert.Equal(t, tmplLabel.Color, genLabel.Color) - assert.Equal(t, tmplLabel.Description, genLabel.Description) - } + // assert the cleanup is successful + unittest.AssertNotExistsBean(t, &repo_model.Repository{OwnerName: user2.Name, Name: testFailureRepoName}) - err = repo_service.DeleteRepositoryDirectly(t.Context(), 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(t.Context(), user2, user2, repo44, repo_service.GenerateRepoOptions{ - Name: "generated-from-template-44", - GitContent: true, + exist, err := git.IsRepositoryExist(t.Context(), testFailureRepo) + assert.NoError(t, err) + assert.False(t, exist) }) - 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) }