Don't create duplicated functions for code repositories and wiki repositories (#33924)

lunny · web-flow · commit a9e8ac0fe008 · 2025-03-19T11:17:19.000-07:00
Fix #33910 (review) This PR changed the Repositroy interface in `gitrepo` package which makes it only focus the relative path in the disk and abstract whether it's a wiki repository or not.
diff --git a/models/repo/repo.go b/models/repo/repo.go
@@ -215,12 +215,24 @@ func init() {
 	db.RegisterModel(new(Repository))
 }
 
-func (repo *Repository) GetName() string {
-	return repo.Name
+func RelativePath(ownerName, repoName string) string {
+	return strings.ToLower(ownerName) + "/" + strings.ToLower(repoName) + ".git"
 }
 
-func (repo *Repository) GetOwnerName() string {
-	return repo.OwnerName
+// RelativePath should be an unix style path like username/reponame.git
+func (repo *Repository) RelativePath() string {
+	return RelativePath(repo.OwnerName, repo.Name)
+}
+
+type StorageRepo string
+
+// RelativePath should be an unix style path like username/reponame.git
+func (sr StorageRepo) RelativePath() string {
+	return string(sr)
+}
+
+func (repo *Repository) WikiStorageRepo() StorageRepo {
+	return StorageRepo(strings.ToLower(repo.OwnerName) + "/" + strings.ToLower(repo.Name) + ".wiki.git")
 }
 
 // SanitizedOriginalURL returns a sanitized OriginalURL
diff --git a/modules/gitrepo/branch.go b/modules/gitrepo/branch.go
@@ -44,24 +44,12 @@ func GetDefaultBranch(ctx context.Context, repo Repository) (string, error) {
 	return git.GetDefaultBranch(ctx, repoPath(repo))
 }
 
-func GetWikiDefaultBranch(ctx context.Context, repo Repository) (string, error) {
-	return git.GetDefaultBranch(ctx, wikiPath(repo))
-}
-
 // IsReferenceExist returns true if given reference exists in the repository.
 func IsReferenceExist(ctx context.Context, repo Repository, name string) bool {
 	return git.IsReferenceExist(ctx, repoPath(repo), name)
 }
 
-func IsWikiReferenceExist(ctx context.Context, repo Repository, name string) bool {
-	return git.IsReferenceExist(ctx, wikiPath(repo), name)
-}
-
 // IsBranchExist returns true if given branch exists in the repository.
 func IsBranchExist(ctx context.Context, repo Repository, name string) bool {
 	return IsReferenceExist(ctx, repo, git.BranchPrefix+name)
 }
-
-func IsWikiBranchExist(ctx context.Context, repo Repository, name string) bool {
-	return IsWikiReferenceExist(ctx, repo, git.BranchPrefix+name)
-}
diff --git a/modules/gitrepo/gitrepo.go b/modules/gitrepo/gitrepo.go
@@ -8,40 +8,29 @@ import (
 	"fmt"
 	"io"
 	"path/filepath"
-	"strings"
 
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/reqctx"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 )
 
+// Repository represents a git repository which stored in a disk
 type Repository interface {
-	GetName() string
-	GetOwnerName() string
-}
-
-func absPath(owner, name string) string {
-	return filepath.Join(setting.RepoRootPath, strings.ToLower(owner), strings.ToLower(name)+".git")
+	RelativePath() string // We don't assume how the directory structure of the repository is, so we only need the relative path
 }
 
+// RelativePath should be an unix style path like username/reponame.git
+// This method should change it according to the current OS.
 func repoPath(repo Repository) string {
-	return absPath(repo.GetOwnerName(), repo.GetName())
-}
-
-func wikiPath(repo Repository) string {
-	return filepath.Join(setting.RepoRootPath, strings.ToLower(repo.GetOwnerName()), strings.ToLower(repo.GetName())+".wiki.git")
+	return filepath.Join(setting.RepoRootPath, filepath.FromSlash(repo.RelativePath()))
 }
 
 // OpenRepository opens the repository at the given relative path with the provided context.
 func OpenRepository(ctx context.Context, repo Repository) (*git.Repository, error) {
 	return git.OpenRepository(ctx, repoPath(repo))
 }
 
-func OpenWikiRepository(ctx context.Context, repo Repository) (*git.Repository, error) {
-	return git.OpenRepository(ctx, wikiPath(repo))
-}
-
 // contextKey is a value for use with context.WithValue.
 type contextKey struct {
 	repoPath string
@@ -86,9 +75,8 @@ func DeleteRepository(ctx context.Context, repo Repository) error {
 }
 
 // RenameRepository renames a repository's name on disk
-func RenameRepository(ctx context.Context, repo Repository, newName string) error {
-	newRepoPath := absPath(repo.GetOwnerName(), newName)
-	if err := util.Rename(repoPath(repo), newRepoPath); err != nil {
+func RenameRepository(ctx context.Context, repo, newRepo Repository) error {
+	if err := util.Rename(repoPath(repo), repoPath(newRepo)); err != nil {
 		return fmt.Errorf("rename repository directory: %w", err)
 	}
 	return nil
diff --git a/modules/gitrepo/hooks.go b/modules/gitrepo/hooks.go
@@ -106,16 +106,11 @@ done
 	return hookNames, hookTpls, giteaHookTpls
 }
 
-// CreateDelegateHooksForRepo creates all the hooks scripts for the repo
-func CreateDelegateHooksForRepo(_ context.Context, repo Repository) (err error) {
+// CreateDelegateHooks creates all the hooks scripts for the repo
+func CreateDelegateHooks(_ context.Context, repo Repository) (err error) {
 	return createDelegateHooks(filepath.Join(repoPath(repo), "hooks"))
 }
 
-// CreateDelegateHooksForWiki creates all the hooks scripts for the wiki repo
-func CreateDelegateHooksForWiki(_ context.Context, repo Repository) (err error) {
-	return createDelegateHooks(filepath.Join(wikiPath(repo), "hooks"))
-}
-
 func createDelegateHooks(hookDir string) (err error) {
 	hookNames, hookTpls, giteaHookTpls := getHookTemplates()
 
@@ -178,16 +173,11 @@ func ensureExecutable(filename string) error {
 	return os.Chmod(filename, mode)
 }
 
-// CheckDelegateHooksForRepo checks the hooks scripts for the repo
-func CheckDelegateHooksForRepo(_ context.Context, repo Repository) ([]string, error) {
+// CheckDelegateHooks checks the hooks scripts for the repo
+func CheckDelegateHooks(_ context.Context, repo Repository) ([]string, error) {
 	return checkDelegateHooks(filepath.Join(repoPath(repo), "hooks"))
 }
 
-// CheckDelegateHooksForWiki checks the hooks scripts for the repo
-func CheckDelegateHooksForWiki(_ context.Context, repo Repository) ([]string, error) {
-	return checkDelegateHooks(filepath.Join(wikiPath(repo), "hooks"))
-}
-
 func checkDelegateHooks(hookDir string) ([]string, error) {
 	hookNames, hookTpls, giteaHookTpls := getHookTemplates()
 
diff --git a/modules/repository/init.go b/modules/repository/init.go
@@ -138,7 +138,7 @@ func CheckInitRepository(ctx context.Context, repo *repo_model.Repository) (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.CreateDelegateHooksForRepo(ctx, repo); err != nil {
+	} else if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
 		return fmt.Errorf("createDelegateHooks: %w", err)
 	}
 	return nil
diff --git a/routers/api/v1/repo/wiki.go b/routers/api/v1/repo/wiki.go
@@ -476,7 +476,7 @@ func findEntryForFile(commit *git.Commit, target string) (*git.TreeEntry, error)
 // findWikiRepoCommit opens the wiki repo and returns the latest commit, writing to context on error.
 // The caller is responsible for closing the returned repo again
 func findWikiRepoCommit(ctx *context.APIContext) (*git.Repository, *git.Commit) {
-	wikiRepo, err := gitrepo.OpenWikiRepository(ctx, ctx.Repo.Repository)
+	wikiRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo())
 	if err != nil {
 		if git.IsErrNotExist(err) || err.Error() == "no such file or directory" {
 			ctx.APIErrorNotFound(err)
diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go
@@ -284,7 +284,7 @@ func Diff(ctx *context.Context) {
 	)
 
 	if ctx.Data["PageIsWiki"] != nil {
-		gitRepo, err = gitrepo.OpenWikiRepository(ctx, ctx.Repo.Repository)
+		gitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo())
 		if err != nil {
 			ctx.ServerError("Repo.GitRepo.GetCommit", err)
 			return
@@ -417,7 +417,7 @@ func Diff(ctx *context.Context) {
 func RawDiff(ctx *context.Context) {
 	var gitRepo *git.Repository
 	if ctx.Data["PageIsWiki"] != nil {
-		wikiRepo, err := gitrepo.OpenWikiRepository(ctx, ctx.Repo.Repository)
+		wikiRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo())
 		if err != nil {
 			ctx.ServerError("OpenRepository", err)
 			return
diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go
@@ -885,7 +885,7 @@ func ExcerptBlob(ctx *context.Context) {
 	gitRepo := ctx.Repo.GitRepo
 	if ctx.Data["PageIsWiki"] == true {
 		var err error
-		gitRepo, err = gitrepo.OpenWikiRepository(ctx, ctx.Repo.Repository)
+		gitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo())
 		if err != nil {
 			ctx.ServerError("OpenRepository", err)
 			return
diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go
@@ -96,7 +96,7 @@ func findEntryForFile(commit *git.Commit, target string) (*git.TreeEntry, error)
 }
 
 func findWikiRepoCommit(ctx *context.Context) (*git.Repository, *git.Commit, error) {
-	wikiGitRepo, errGitRepo := gitrepo.OpenWikiRepository(ctx, ctx.Repo.Repository)
+	wikiGitRepo, errGitRepo := gitrepo.OpenRepository(ctx, ctx.Repo.Repository.WikiStorageRepo())
 	if errGitRepo != nil {
 		ctx.ServerError("OpenRepository", errGitRepo)
 		return nil, nil, errGitRepo
@@ -105,7 +105,7 @@ func findWikiRepoCommit(ctx *context.Context) (*git.Repository, *git.Commit, err
 	commit, errCommit := wikiGitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultWikiBranch)
 	if git.IsErrNotExist(errCommit) {
 		// if the default branch recorded in database is out of sync, then re-sync it
-		gitRepoDefaultBranch, errBranch := gitrepo.GetWikiDefaultBranch(ctx, ctx.Repo.Repository)
+		gitRepoDefaultBranch, errBranch := gitrepo.GetDefaultBranch(ctx, ctx.Repo.Repository.WikiStorageRepo())
 		if errBranch != nil {
 			return wikiGitRepo, nil, errBranch
 		}
diff --git a/routers/web/repo/wiki_test.go b/routers/web/repo/wiki_test.go
@@ -29,7 +29,7 @@ const (
 )
 
 func wikiEntry(t *testing.T, repo *repo_model.Repository, wikiName wiki_service.WebPath) *git.TreeEntry {
-	wikiRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)
+	wikiRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())
 	assert.NoError(t, err)
 	defer wikiRepo.Close()
 	commit, err := wikiRepo.GetBranchCommit("master")
diff --git a/services/asymkey/sign.go b/services/asymkey/sign.go
@@ -204,7 +204,7 @@ Loop:
 				return false, "", nil, &ErrWontSign{twofa}
 			}
 		case parentSigned:
-			gitRepo, err := gitrepo.OpenWikiRepository(ctx, repo)
+			gitRepo, err := gitrepo.OpenRepository(ctx, repo.WikiStorageRepo())
 			if err != nil {
 				return false, "", nil, err
 			}
diff --git a/services/doctor/misc.go b/services/doctor/misc.go
@@ -49,14 +49,14 @@ func checkScriptType(ctx context.Context, logger log.Logger, autofix bool) error
 
 func checkHooks(ctx context.Context, logger log.Logger, autofix bool) error {
 	if err := iterateRepositories(ctx, func(repo *repo_model.Repository) error {
-		results, err := gitrepo.CheckDelegateHooksForRepo(ctx, repo)
+		results, err := gitrepo.CheckDelegateHooks(ctx, repo)
 		if err != nil {
 			logger.Critical("Unable to check delegate hooks for repo %-v. ERROR: %v", repo, err)
 			return fmt.Errorf("Unable to check delegate hooks for repo %-v. ERROR: %w", repo, err)
 		}
 		if len(results) > 0 && autofix {
 			logger.Warn("Regenerated hooks for %s", repo.FullName())
-			if err := gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
+			if err := gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
 				logger.Critical("Unable to recreate delegate hooks for %-v. ERROR: %v", repo, err)
 				return fmt.Errorf("Unable to recreate delegate hooks for %-v. ERROR: %w", repo, err)
 			}
diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go
@@ -143,7 +143,7 @@ func runPushSync(ctx context.Context, m *repo_model.PushMirror) error {
 
 			var gitRepo *git.Repository
 			if isWiki {
-				gitRepo, err = gitrepo.OpenWikiRepository(ctx, repo)
+				gitRepo, err = gitrepo.OpenRepository(ctx, repo.WikiStorageRepo())
 			} else {
 				gitRepo, err = gitrepo.OpenRepository(ctx, repo)
 			}
diff --git a/services/repository/adopt.go b/services/repository/adopt.go
@@ -115,7 +115,7 @@ func adoptRepository(ctx context.Context, repo *repo_model.Repository, defaultBr
 		return fmt.Errorf("adoptRepository: path does not already exist: %s", repo.FullName())
 	}
 
-	if err := gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
+	if err := gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
 		return fmt.Errorf("createDelegateHooks: %w", err)
 	}
 
diff --git a/services/repository/fork.go b/services/repository/fork.go
@@ -170,7 +170,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
 			return fmt.Errorf("git update-server-info: %w", err)
 		}
 
-		if err = gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
+		if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
 			return fmt.Errorf("createDelegateHooks: %w", err)
 		}
 
diff --git a/services/repository/hooks.go b/services/repository/hooks.go
@@ -31,11 +31,11 @@ func SyncRepositoryHooks(ctx context.Context) error {
 			default:
 			}
 
-			if err := gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
+			if err := gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
 				return fmt.Errorf("SyncRepositoryHook: %w", err)
 			}
 			if repo.HasWiki() {
-				if err := gitrepo.CreateDelegateHooksForWiki(ctx, repo); err != nil {
+				if err := gitrepo.CreateDelegateHooks(ctx, repo.WikiStorageRepo()); err != nil {
 					return fmt.Errorf("SyncRepositoryHook: %w", err)
 				}
 			}
diff --git a/services/repository/migrate.go b/services/repository/migrate.go
@@ -265,11 +265,11 @@ func cleanUpMigrateGitConfig(ctx context.Context, repoPath string) error {
 
 // CleanUpMigrateInfo finishes migrating repository and/or wiki with things that don't need to be done for mirrors.
 func CleanUpMigrateInfo(ctx context.Context, repo *repo_model.Repository) (*repo_model.Repository, error) {
-	if err := gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {
+	if err := gitrepo.CreateDelegateHooks(ctx, repo); err != nil {
 		return repo, fmt.Errorf("createDelegateHooks: %w", err)
 	}
 	if repo.HasWiki() {
-		if err := gitrepo.CreateDelegateHooksForWiki(ctx, repo); err != nil {
+		if err := gitrepo.CreateDelegateHooks(ctx, repo.WikiStorageRepo()); err != nil {
 			return repo, fmt.Errorf("createDelegateHooks.(wiki): %w", err)
 		}
 	}
diff --git a/services/repository/transfer.go b/services/repository/transfer.go
@@ -331,12 +331,13 @@ func changeRepositoryName(ctx context.Context, repo *repo_model.Repository, newR
 		return fmt.Errorf("IsRepositoryExist: %w", err)
 	} else if has {
 		return repo_model.ErrRepoAlreadyExist{
-			Uname: repo.Owner.Name,
+			Uname: repo.OwnerName,
 			Name:  newRepoName,
 		}
 	}
 
-	if err = gitrepo.RenameRepository(ctx, repo, newRepoName); err != nil {
+	if err = gitrepo.RenameRepository(ctx, repo,
+		repo_model.StorageRepo(repo_model.RelativePath(repo.OwnerName, newRepoName))); err != nil {
 		return fmt.Errorf("rename repository directory: %w", err)
 	}
 
diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go
@@ -41,7 +41,7 @@ func InitWiki(ctx context.Context, repo *repo_model.Repository) error {
 
 	if err := git.InitRepository(ctx, repo.WikiPath(), true, repo.ObjectFormatName); err != nil {
 		return fmt.Errorf("InitRepository: %w", err)
-	} else if err = gitrepo.CreateDelegateHooksForWiki(ctx, repo); err != nil {
+	} else if err = gitrepo.CreateDelegateHooks(ctx, repo.WikiStorageRepo()); err != nil {
 		return fmt.Errorf("createDelegateHooks: %w", err)
 	} else if _, _, err = git.NewCommand("symbolic-ref", "HEAD").AddDynamicArguments(git.BranchPrefix+repo.DefaultWikiBranch).RunStdString(ctx, &git.RunOpts{Dir: repo.WikiPath()}); err != nil {
 		return fmt.Errorf("unable to set default wiki branch to %q: %w", repo.DefaultWikiBranch, err)
@@ -100,7 +100,7 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
 		return fmt.Errorf("InitWiki: %w", err)
 	}
 
-	hasDefaultBranch := gitrepo.IsWikiBranchExist(ctx, repo, repo.DefaultWikiBranch)
+	hasDefaultBranch := gitrepo.IsBranchExist(ctx, repo.WikiStorageRepo(), repo.DefaultWikiBranch)
 
 	basePath, err := repo_module.CreateTemporaryPath("update-wiki")
 	if err != nil {
@@ -381,15 +381,15 @@ func ChangeDefaultWikiBranch(ctx context.Context, repo *repo_model.Repository, n
 			return nil
 		}
 
-		oldDefBranch, err := gitrepo.GetWikiDefaultBranch(ctx, repo)
+		oldDefBranch, err := gitrepo.GetDefaultBranch(ctx, repo.WikiStorageRepo())
 		if err != nil {
 			return fmt.Errorf("unable to get default branch: %w", err)
 		}
 		if oldDefBranch == newBranch {
 			return nil
 		}
 
-		gitRepo, err := gitrepo.OpenWikiRepository(ctx, repo)
+		gitRepo, err := gitrepo.OpenRepository(ctx, repo.WikiStorageRepo())
 		if errors.Is(err, util.ErrNotExist) {
 			return nil // no git repo on storage, no need to do anything else
 		} else if err != nil {
diff --git a/services/wiki/wiki_test.go b/services/wiki/wiki_test.go
@@ -166,7 +166,7 @@ func TestRepository_AddWikiPage(t *testing.T) {
 			webPath := UserTitleToWebPath("", userTitle)
 			assert.NoError(t, AddWikiPage(git.DefaultContext, doer, repo, webPath, wikiContent, commitMsg))
 			// Now need to show that the page has been added:
-			gitRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)
+			gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())
 			require.NoError(t, err)
 
 			defer gitRepo.Close()
@@ -213,7 +213,7 @@ func TestRepository_EditWikiPage(t *testing.T) {
 		assert.NoError(t, EditWikiPage(git.DefaultContext, doer, repo, "Home", webPath, newWikiContent, commitMsg))
 
 		// Now need to show that the page has been added:
-		gitRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)
+		gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())
 		assert.NoError(t, err)
 		masterTree, err := gitRepo.GetTree(repo.DefaultWikiBranch)
 		assert.NoError(t, err)
@@ -237,7 +237,7 @@ func TestRepository_DeleteWikiPage(t *testing.T) {
 	assert.NoError(t, DeleteWikiPage(git.DefaultContext, doer, repo, "Home"))
 
 	// Now need to show that the page has been added:
-	gitRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)
+	gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())
 	require.NoError(t, err)
 
 	defer gitRepo.Close()
@@ -251,7 +251,7 @@ func TestRepository_DeleteWikiPage(t *testing.T) {
 func TestPrepareWikiFileName(t *testing.T) {
 	unittest.PrepareTestEnv(t)
 	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
-	gitRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)
+	gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())
 	require.NoError(t, err)
 
 	defer gitRepo.Close()

Original file line number	Diff line number	Diff line change
`@@ -138,7 +138,7 @@ func CheckInitRepository(ctx context.Context, repo *repo_model.Repository) (err`
`138`	`138`	`// Init git bare new repository.`
`139`	`139`	`if err = git.InitRepository(ctx, repo.RepoPath(), true, repo.ObjectFormatName); err != nil {`
`140`	`140`	`return fmt.Errorf("git.InitRepository: %w", err)`
`141`		`- } else if err = gitrepo.CreateDelegateHooksForRepo(ctx, repo); err != nil {`
	`141`	`+ } else if err = gitrepo.CreateDelegateHooks(ctx, repo); err != nil {`
`142`	`142`	`return fmt.Errorf("createDelegateHooks: %w", err)`
`143`	`143`	`}`
`144`	`144`	`return nil`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ const (`
`29`	`29`	`)`
`30`	`30`
`31`	`31`	`func wikiEntry(t testing.T, repo repo_model.Repository, wikiName wiki_service.WebPath) *git.TreeEntry {`
`32`		`- wikiRepo, err := gitrepo.OpenWikiRepository(git.DefaultContext, repo)`
	`32`	`+ wikiRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo.WikiStorageRepo())`
`33`	`33`	`assert.NoError(t, err)`
`34`	`34`	`defer wikiRepo.Close()`
`35`	`35`	`commit, err := wikiRepo.GetBranchCommit("master")`