Refactor ref type (#33242)

Major changes:

1. do not sync ".keep" file during tests
2. fix incorrect route handler and empty repo handling (backported as #33253 with tests)
3. do not use `RepoRef`: most of the calls are abuses.
4. Use `git.RefType` instead of a new type definition `RepoRefType` on `context`.

---------

Co-authored-by: wxiaoguang <[email protected]>

Author: lunny

models/unittest/fscopy.go

@@ -11,35 +11,13 @@ import (
 	"code.gitea.io/gitea/modules/util"
 )
 
-// Copy copies file from source to target path.
-func Copy(src, dest string) error {
-	// Gather file information to set back later.
-	si, err := os.Lstat(src)
-	if err != nil {
-		return err
-	}
-
-	// Handle symbolic link.
-	if si.Mode()&os.ModeSymlink != 0 {
-		target, err := os.Readlink(src)
-		if err != nil {
-			return err
-		}
-		// NOTE: os.Chmod and os.Chtimes don't recognize symbolic link,
-		// which will lead "no such file or directory" error.
-		return os.Symlink(target, dest)
-	}
-
-	return util.CopyFile(src, dest)
-}
-
-// Sync synchronizes the two files. This is skipped if both files
+// SyncFile synchronizes the two files. This is skipped if both files
 // exist and the size, modtime, and mode match.
-func Sync(srcPath, destPath string) error {
+func SyncFile(srcPath, destPath string) error {
 	dest, err := os.Stat(destPath)
 	if err != nil {
 		if os.IsNotExist(err) {
-			return Copy(srcPath, destPath)
+			return util.CopyFile(srcPath, destPath)
 		}
 		return err
 	}
@@ -55,7 +33,7 @@ func Sync(srcPath, destPath string) error {
 		return nil
 	}
 
-	return Copy(srcPath, destPath)
+	return util.CopyFile(srcPath, destPath)
 }
 
 // SyncDirs synchronizes files recursively from source to target directory.
@@ -66,23 +44,31 @@ func SyncDirs(srcPath, destPath string) error {
 		return err
 	}
 
+	// the keep file is used to keep the directory in a git repository, it doesn't need to be synced
+	// and go-git doesn't work with the ".keep" file (it would report errors like "ref is empty")
+	const keepFile = ".keep"
+
 	// find and delete all untracked files
 	destFiles, err := util.ListDirRecursively(destPath, &util.ListDirOptions{IncludeDir: true})
 	if err != nil {
 		return err
 	}
 	for _, destFile := range destFiles {
 		destFilePath := filepath.Join(destPath, destFile)
+		shouldRemove := filepath.Base(destFilePath) == keepFile
 		if _, err = os.Stat(filepath.Join(srcPath, destFile)); err != nil {
 			if os.IsNotExist(err) {
-				// if src file does not exist, remove dest file
-				if err = os.RemoveAll(destFilePath); err != nil {
-					return err
-				}
+				shouldRemove = true
 			} else {
 				return err
 			}
 		}
+		// if src file does not exist, remove dest file
+		if shouldRemove {
+			if err = os.RemoveAll(destFilePath); err != nil {
+				return err
+			}
+		}
 	}
 
 	// sync src files to dest
@@ -95,8 +81,8 @@ func SyncDirs(srcPath, destPath string) error {
 		// util.ListDirRecursively appends a slash to the directory name
 		if strings.HasSuffix(srcFile, "/") {
 			err = os.MkdirAll(destFilePath, os.ModePerm)
-		} else {
-			err = Sync(filepath.Join(srcPath, srcFile), destFilePath)
+		} else if filepath.Base(destFilePath) != keepFile {
+			err = SyncFile(filepath.Join(srcPath, srcFile), destFilePath)
 		}
 		if err != nil {
 			return err

routers/web/repo/repo.go

@@ -51,7 +51,7 @@ func MustBeNotEmpty(ctx *context.Context) {
 
 // MustBeEditable check that repo can be edited
 func MustBeEditable(ctx *context.Context) {
-	if !ctx.Repo.Repository.CanEnableEditor() || ctx.Repo.IsViewCommit {
+	if !ctx.Repo.Repository.CanEnableEditor() {
 		ctx.NotFound("", nil)
 		return
 	}

routers/web/repo/view_home.go

@@ -249,7 +249,7 @@ func handleRepoEmptyOrBroken(ctx *context.Context) {
 		} else if reallyEmpty {
 			showEmpty = true // the repo is really empty
 			updateContextRepoEmptyAndStatus(ctx, true, repo_model.RepositoryReady)
-		} else if ctx.Repo.Commit == nil {
+		} else if branches, _, _ := ctx.Repo.GitRepo.GetBranches(0, 1); len(branches) == 0 {
 			showEmpty = true // it is not really empty, but there is no branch
 			// at the moment, other repo units like "actions" are not able to handle such case,
 			// so we just mark the repo as empty to prevent from displaying these units.

routers/web/web.go

@@ -11,6 +11,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/perm"
 	"code.gitea.io/gitea/models/unit"
+	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/metrics"
 	"code.gitea.io/gitea/modules/public"
@@ -817,7 +818,6 @@ func registerRoutes(m *web.Router) {
 
 	reqRepoAdmin := context.RequireRepoAdmin()
 	reqRepoCodeWriter := context.RequireUnitWriter(unit.TypeCode)
-	canEnableEditor := context.CanEnableEditor()
 	reqRepoReleaseWriter := context.RequireUnitWriter(unit.TypeReleases)
 	reqRepoReleaseReader := context.RequireUnitReader(unit.TypeReleases)
 	reqRepoIssuesOrPullsWriter := context.RequireUnitWriter(unit.TypeIssues, unit.TypePullRequests)
@@ -1152,16 +1152,16 @@ func registerRoutes(m *web.Router) {
 	// end "/{username}/{reponame}/settings"
 
 	// user/org home, including rss feeds
-	m.Get("/{username}/{reponame}", optSignIn, context.RepoAssignment, context.RepoRef(), repo.SetEditorconfigIfExists, repo.Home)
+	m.Get("/{username}/{reponame}", optSignIn, context.RepoAssignment, context.RepoRefByType(git.RefTypeBranch), repo.SetEditorconfigIfExists, repo.Home)
 
 	m.Post("/{username}/{reponame}/markup", optSignIn, context.RepoAssignment, reqUnitsWithMarkdown, web.Bind(structs.MarkupOption{}), misc.Markup)
 
 	m.Group("/{username}/{reponame}", func() {
 		m.Get("/find/*", repo.FindFiles)
 		m.Group("/tree-list", func() {
-			m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.TreeList)
-			m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.TreeList)
-			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.TreeList)
+			m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.TreeList)
+			m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.TreeList)
+			m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.TreeList)
 		})
 		m.Get("/compare", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff)
 		m.Combo("/compare/*", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists).
@@ -1306,18 +1306,18 @@ func registerRoutes(m *web.Router) {
 					Post(web.Bind(forms.EditRepoFileForm{}), repo.NewDiffPatchPost)
 				m.Combo("/_cherrypick/{sha:([a-f0-9]{7,64})}/*").Get(repo.CherryPick).
 					Post(web.Bind(forms.CherryPickForm{}), repo.CherryPickPost)
-			}, repo.MustBeEditable)
+			}, context.RepoRefByType(git.RefTypeBranch), context.CanWriteToBranch())
 			m.Group("", func() {
 				m.Post("/upload-file", repo.UploadFileToServer)
 				m.Post("/upload-remove", web.Bind(forms.RemoveUploadFileForm{}), repo.RemoveUploadFileFromServer)
-			}, repo.MustBeEditable, repo.MustBeAbleToUpload)
-		}, context.RepoRef(), canEnableEditor, context.RepoMustNotBeArchived())
+			}, repo.MustBeAbleToUpload, reqRepoCodeWriter)
+		}, repo.MustBeEditable, context.RepoMustNotBeArchived())
 
 		m.Group("/branches", func() {
 			m.Group("/_new", func() {
-				m.Post("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.CreateBranch)
-				m.Post("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.CreateBranch)
-				m.Post("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.CreateBranch)
+				m.Post("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.CreateBranch)
+				m.Post("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.CreateBranch)
+				m.Post("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.CreateBranch)
 			}, web.Bind(forms.NewBranchForm{}))
 			m.Post("/delete", repo.DeleteBranchPost)
 			m.Post("/restore", repo.RestoreBranchPost)
@@ -1332,39 +1332,36 @@ func registerRoutes(m *web.Router) {
 	m.Group("/{username}/{reponame}", func() { // repo tags
 		m.Group("/tags", func() {
 			m.Get("", repo.TagsList)
-			m.Get("/list", repo.GetTagList)
 			m.Get(".rss", feedEnabled, repo.TagsListFeedRSS)
 			m.Get(".atom", feedEnabled, repo.TagsListFeedAtom)
-		}, ctxDataSet("EnableFeed", setting.Other.EnableFeed),
-			repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true}))
-		m.Post("/tags/delete", repo.DeleteTag, reqSignIn,
-			repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoCodeWriter, context.RepoRef())
-	}, optSignIn, context.RepoAssignment, reqUnitCodeReader)
+			m.Get("/list", repo.GetTagList)
+		}, ctxDataSet("EnableFeed", setting.Other.EnableFeed))
+		m.Post("/tags/delete", reqSignIn, reqRepoCodeWriter, context.RepoMustNotBeArchived(), repo.DeleteTag)
+	}, optSignIn, context.RepoAssignment, repo.MustBeNotEmpty, reqUnitCodeReader)
 	// end "/{username}/{reponame}": repo tags
 
 	m.Group("/{username}/{reponame}", func() { // repo releases
 		m.Group("/releases", func() {
 			m.Get("", repo.Releases)
-			m.Get("/tag/*", repo.SingleRelease)
-			m.Get("/latest", repo.LatestRelease)
 			m.Get(".rss", feedEnabled, repo.ReleasesFeedRSS)
 			m.Get(".atom", feedEnabled, repo.ReleasesFeedAtom)
-		}, ctxDataSet("EnableFeed", setting.Other.EnableFeed),
-			repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true}))
-		m.Get("/releases/attachments/{uuid}", repo.MustBeNotEmpty, repo.GetAttachment)
-		m.Get("/releases/download/{vTag}/{fileName}", repo.MustBeNotEmpty, repo.RedirectDownload)
+			m.Get("/tag/*", repo.SingleRelease)
+			m.Get("/latest", repo.LatestRelease)
+		}, ctxDataSet("EnableFeed", setting.Other.EnableFeed))
+		m.Get("/releases/attachments/{uuid}", repo.GetAttachment)
+		m.Get("/releases/download/{vTag}/{fileName}", repo.RedirectDownload)
 		m.Group("/releases", func() {
 			m.Get("/new", repo.NewRelease)
 			m.Post("/new", web.Bind(forms.NewReleaseForm{}), repo.NewReleasePost)
 			m.Post("/delete", repo.DeleteRelease)
 			m.Post("/attachments", repo.UploadReleaseAttachment)
 			m.Post("/attachments/remove", repo.DeleteAttachment)
-		}, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, context.RepoRef())
+		}, reqSignIn, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, context.RepoRef())
 		m.Group("/releases", func() {
 			m.Get("/edit/*", repo.EditRelease)
 			m.Post("/edit/*", web.Bind(forms.EditReleaseForm{}), repo.EditReleasePost)
-		}, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, repo.CommitInfoCache)
-	}, optSignIn, context.RepoAssignment, reqRepoReleaseReader)
+		}, reqSignIn, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, repo.CommitInfoCache)
+	}, optSignIn, context.RepoAssignment, repo.MustBeNotEmpty, reqRepoReleaseReader)
 	// end "/{username}/{reponame}": repo releases
 
 	m.Group("/{username}/{reponame}", func() { // to maintain compatibility with old attachments
@@ -1528,42 +1525,39 @@ func registerRoutes(m *web.Router) {
 		}, repo.MustBeNotEmpty, context.RepoRef())
 
 		m.Group("/media", func() {
-			m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.SingleDownloadOrLFS)
-			m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.SingleDownloadOrLFS)
-			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownloadOrLFS)
 			m.Get("/blob/{sha}", repo.DownloadByIDOrLFS)
-			// "/*" route is deprecated, and kept for backward compatibility
-			m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.SingleDownloadOrLFS)
+			m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.SingleDownloadOrLFS)
+			m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.SingleDownloadOrLFS)
+			m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.SingleDownloadOrLFS)
+			m.Get("/*", context.RepoRefByType(""), repo.SingleDownloadOrLFS) // "/*" route is deprecated, and kept for backward compatibility
 		}, repo.MustBeNotEmpty)
 
 		m.Group("/raw", func() {
-			m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.SingleDownload)
-			m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.SingleDownload)
-			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownload)
 			m.Get("/blob/{sha}", repo.DownloadByID)
-			// "/*" route is deprecated, and kept for backward compatibility
-			m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.SingleDownload)
+			m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.SingleDownload)
+			m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.SingleDownload)
+			m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.SingleDownload)
+			m.Get("/*", context.RepoRefByType(""), repo.SingleDownload) // "/*" route is deprecated, and kept for backward compatibility
 		}, repo.MustBeNotEmpty)
 
 		m.Group("/render", func() {
-			m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.RenderFile)
-			m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.RenderFile)
-			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.RenderFile)
+			m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.RenderFile)
+			m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.RenderFile)
+			m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.RenderFile)
 			m.Get("/blob/{sha}", repo.RenderFile)
 		}, repo.MustBeNotEmpty)
 
 		m.Group("/commits", func() {
-			m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.RefCommits)
-			m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.RefCommits)
-			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.RefCommits)
-			// "/*" route is deprecated, and kept for backward compatibility
-			m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.RefCommits)
+			m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.RefCommits)
+			m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.RefCommits)
+			m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.RefCommits)
+			m.Get("/*", context.RepoRefByType(""), repo.RefCommits) // "/*" route is deprecated, and kept for backward compatibility
 		}, repo.MustBeNotEmpty)
 
 		m.Group("/blame", func() {
-			m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.RefBlame)
-			m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.RefBlame)
-			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.RefBlame)
+			m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.RefBlame)
+			m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.RefBlame)
+			m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.RefBlame)
 		}, repo.MustBeNotEmpty)
 
 		m.Get("/blob_excerpt/{sha}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.ExcerptBlob)
@@ -1575,20 +1569,20 @@ func registerRoutes(m *web.Router) {
 			m.Get("/cherry-pick/{sha:([a-f0-9]{7,64})$}", repo.SetEditorconfigIfExists, repo.CherryPick)
 		}, repo.MustBeNotEmpty, context.RepoRef())
 
-		m.Get("/rss/branch/*", context.RepoRefByType(context.RepoRefBranch), feedEnabled, feed.RenderBranchFeed)
-		m.Get("/atom/branch/*", context.RepoRefByType(context.RepoRefBranch), feedEnabled, feed.RenderBranchFeed)
+		m.Get("/rss/branch/*", context.RepoRefByType(git.RefTypeBranch), feedEnabled, feed.RenderBranchFeed)
+		m.Get("/atom/branch/*", context.RepoRefByType(git.RefTypeBranch), feedEnabled, feed.RenderBranchFeed)
 
 		m.Group("/src", func() {
 			m.Get("", func(ctx *context.Context) { ctx.Redirect(ctx.Repo.RepoLink) }) // there is no "{owner}/{repo}/src" page, so redirect to "{owner}/{repo}" to avoid 404
-			m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.Home)
-			m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.Home)
-			m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.Home)
-			m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.Home) // "/*" route is deprecated, and kept for backward compatibility
+			m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.Home)
+			m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.Home)
+			m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.Home)
+			m.Get("/*", context.RepoRefByType(""), repo.Home) // "/*" route is deprecated, and kept for backward compatibility
 		}, repo.SetEditorconfigIfExists)
 
 		m.Get("/forks", context.RepoRef(), repo.Forks)
 		m.Get("/commit/{sha:([a-f0-9]{7,64})}.{ext:patch|diff}", repo.MustBeNotEmpty, repo.RawDiff)
-		m.Post("/lastcommit/*", context.RepoRefByType(context.RepoRefCommit), repo.LastCommit)
+		m.Post("/lastcommit/*", context.RepoRefByType(git.RefTypeCommit), repo.LastCommit)
 	}, optSignIn, context.RepoAssignment, reqUnitCodeReader)
 	// end "/{username}/{reponame}": repo code
 

services/context/permission.go

@@ -21,8 +21,8 @@ func RequireRepoAdmin() func(ctx *Context) {
 	}
 }
 
-// CanEnableEditor checks if the user is allowed to write to the branch of the repo
-func CanEnableEditor() func(ctx *Context) {
+// CanWriteToBranch checks if the user is allowed to write to the branch of the repo
+func CanWriteToBranch() func(ctx *Context) {
 	return func(ctx *Context) {
 		if !ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.BranchName) {
 			ctx.NotFound("CanWriteToBranch denies permission", nil)