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.
This commit is contained in:
wxiaoguang
2026-07-24 01:42:55 +08:00
committed by GitHub
parent 86a3048247
commit 7065637e61
22 changed files with 216 additions and 186 deletions
+2 -2
View File
@@ -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()
+2 -1
View File
@@ -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)
+7 -6
View File
@@ -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)
}
+3 -2
View File
@@ -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)
+29 -24
View File
@@ -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
})
}
+34 -26
View File
@@ -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)
})
}
+36 -28
View File
@@ -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)
}
+7 -6
View File
@@ -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{
+4 -3
View File
@@ -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))