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 <wxiaoguang@gmail.com>

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,6 +44,10 @@ 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 {
@@ -73,16 +55,20 @@ func SyncDirs(srcPath, destPath string) error {
 	}
 	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)

services/context/repo.go

@@ -677,24 +677,12 @@ func RepoAssignment(ctx *Context) {
 	}
 }
 
-// RepoRefType type of repo reference
-type RepoRefType int
-
-const (
-	// RepoRefUnknown is for legacy support, makes the code to "guess" the ref type
-	RepoRefUnknown RepoRefType = iota
-	RepoRefBranch
-	RepoRefTag
-	RepoRefCommit
-)
-
 const headRefName = "HEAD"
 
-// RepoRef handles repository reference names when the ref name is not
-// explicitly given
 func RepoRef() func(*Context) {
-	// since no ref name is explicitly specified, ok to just use branch
-	return RepoRefByType(RepoRefBranch)
+	// old code does: return RepoRefByType(git.RefTypeBranch)
+	// in most cases, it is an abuse, so we just disable it completely and fix the abuses one by one (if there is anything wrong)
+	return nil
 }
 
 func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool) string {
@@ -710,29 +698,29 @@ func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool
 	return ""
 }
 
-func getRefNameLegacy(ctx *Base, repo *Repository, reqPath, extraRef string) (string, RepoRefType) {
+func getRefNameLegacy(ctx *Base, repo *Repository, reqPath, extraRef string) (string, git.RefType) {
 	reqRefPath := path.Join(extraRef, reqPath)
 	reqRefPathParts := strings.Split(reqRefPath, "/")
-	if refName := getRefName(ctx, repo, reqRefPath, RepoRefBranch); refName != "" {
-		return refName, RepoRefBranch
+	if refName := getRefName(ctx, repo, reqRefPath, git.RefTypeBranch); refName != "" {
+		return refName, git.RefTypeBranch
 	}
-	if refName := getRefName(ctx, repo, reqRefPath, RepoRefTag); refName != "" {
-		return refName, RepoRefTag
+	if refName := getRefName(ctx, repo, reqRefPath, git.RefTypeTag); refName != "" {
+		return refName, git.RefTypeTag
 	}
 	if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), reqRefPathParts[0]) {
 		// FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists
 		repo.TreePath = strings.Join(reqRefPathParts[1:], "/")
-		return reqRefPathParts[0], RepoRefCommit
+		return reqRefPathParts[0], git.RefTypeCommit
 	}
 	// FIXME: the old code falls back to default branch if "ref" doesn't exist, there could be an edge case:
 	// "README?ref=no-such" would read the README file from the default branch, but the user might expect a 404
 	repo.TreePath = reqPath
-	return repo.Repository.DefaultBranch, RepoRefBranch
+	return repo.Repository.DefaultBranch, git.RefTypeBranch
 }
 
-func getRefName(ctx *Base, repo *Repository, path string, pathType RepoRefType) string {
-	switch pathType {
-	case RepoRefBranch:
+func getRefName(ctx *Base, repo *Repository, path string, refType git.RefType) string {
+	switch refType {
+	case git.RefTypeBranch:
 		ref := getRefNameFromPath(repo, path, repo.GitRepo.IsBranchExist)
 		if len(ref) == 0 {
 			// check if ref is HEAD
@@ -762,9 +750,9 @@ func getRefName(ctx *Base, repo *Repository, path string, pathType RepoRefType)
 		}
 
 		return ref
-	case RepoRefTag:
+	case git.RefTypeTag:
 		return getRefNameFromPath(repo, path, repo.GitRepo.IsTagExist)
-	case RepoRefCommit:
+	case git.RefTypeCommit:
 		parts := strings.Split(path, "/")
 		if git.IsStringLikelyCommitID(repo.GetObjectFormat(), parts[0], 7) {
 			// FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists
@@ -782,22 +770,18 @@ func getRefName(ctx *Base, repo *Repository, path string, pathType RepoRefType)
 			return commit.ID.String()
 		}
 	default:
-		panic(fmt.Sprintf("Unrecognized path type: %v", pathType))
+		panic(fmt.Sprintf("Unrecognized ref type: %v", refType))
 	}
 	return ""
 }
 
-type RepoRefByTypeOptions struct {
-	IgnoreNotExistErr bool
-}
-
-func repoRefFullName(shortName string, typ RepoRefType) git.RefName {
+func repoRefFullName(typ git.RefType, shortName string) git.RefName {
 	switch typ {
-	case RepoRefBranch:
+	case git.RefTypeBranch:
 		return git.RefNameFromBranch(shortName)
-	case RepoRefTag:
+	case git.RefTypeTag:
 		return git.RefNameFromTag(shortName)
-	case RepoRefCommit:
+	case git.RefTypeCommit:
 		return git.RefNameFromCommit(shortName)
 	default:
 		setting.PanicInDevOrTesting("Unknown RepoRefType: %v", typ)
@@ -807,8 +791,7 @@ func repoRefFullName(shortName string, typ RepoRefType) git.RefName {
 
 // RepoRefByType handles repository reference name for a specific type
 // of repository reference
-func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func(*Context) {
-	opt := util.OptionalArg(opts)
+func RepoRefByType(detectRefType git.RefType) func(*Context) {
 	return func(ctx *Context) {
 		var err error
 		refType := detectRefType
@@ -824,14 +807,6 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func
 			return
 		}
 
-		if ctx.Repo.GitRepo == nil {
-			ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository)
-			if err != nil {
-				ctx.ServerError(fmt.Sprintf("Open Repository %v failed", ctx.Repo.Repository.FullName()), err)
-				return
-			}
-		}
-
 		// Get default branch.
 		var refShortName string
 		reqPath := ctx.PathParam("*")
@@ -861,13 +836,13 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func
 			}
 			ctx.Repo.IsViewBranch = true
 		} else { // there is a path in request
-			guessLegacyPath := refType == RepoRefUnknown
+			guessLegacyPath := refType == ""
 			if guessLegacyPath {
 				refShortName, refType = getRefNameLegacy(ctx.Base, ctx.Repo, reqPath, "")
 			} else {
 				refShortName = getRefName(ctx.Base, ctx.Repo, reqPath, refType)
 			}
-			ctx.Repo.RefFullName = repoRefFullName(refShortName, refType)
+			ctx.Repo.RefFullName = repoRefFullName(refType, refShortName)
 			isRenamedBranch, has := ctx.Data["IsRenamedBranch"].(bool)
 			if isRenamedBranch && has {
 				renamedBranchName := ctx.Data["RenamedBranchName"].(string)
@@ -877,7 +852,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func
 				return
 			}
 
-			if refType == RepoRefBranch && ctx.Repo.GitRepo.IsBranchExist(refShortName) {
+			if refType == git.RefTypeBranch && ctx.Repo.GitRepo.IsBranchExist(refShortName) {
 				ctx.Repo.IsViewBranch = true
 				ctx.Repo.BranchName = refShortName
 				ctx.Repo.RefFullName = git.RefNameFromBranch(refShortName)
@@ -888,7 +863,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func
 					return
 				}
 				ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
-			} else if refType == RepoRefTag && ctx.Repo.GitRepo.IsTagExist(refShortName) {
+			} else if refType == git.RefTypeTag && ctx.Repo.GitRepo.IsTagExist(refShortName) {
 				ctx.Repo.IsViewTag = true
 				ctx.Repo.RefFullName = git.RefNameFromTag(refShortName)
 				ctx.Repo.TagName = refShortName
@@ -919,9 +894,6 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func
 					ctx.RespHeader().Set("Link", fmt.Sprintf(`<%s>; rel="canonical"`, canonicalURL))
 				}
 			} else {
-				if opt.IgnoreNotExistErr {
-					return
-				}
 				ctx.NotFound("RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refShortName))
 				return
 			}

tests/integration/empty_repo_test.go

@@ -14,6 +14,7 @@ import (
 	"testing"
 
 	auth_model "code.gitea.io/gitea/models/auth"
+	"code.gitea.io/gitea/models/db"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
@@ -24,6 +25,7 @@ import (
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func testAPINewFile(t *testing.T, session *TestSession, user, repo, branch, treePath, content string) *httptest.ResponseRecorder {
@@ -82,6 +84,29 @@ func TestEmptyRepoAddFile(t *testing.T) {
 	req = NewRequest(t, "GET", redirect)
 	resp = session.MakeRequest(t, req, http.StatusOK)
 	assert.Contains(t, resp.Body.String(), "newly-added-test-file")
+
+	// the repo is not empty anymore
+	req = NewRequest(t, "GET", "/user30/empty")
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	assert.Contains(t, resp.Body.String(), "test-file.md")
+
+	// if the repo is in incorrect state, it should be able to self-heal (recover to correct state)
+	user30EmptyRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 30, Name: "empty"})
+	user30EmptyRepo.IsEmpty = true
+	user30EmptyRepo.DefaultBranch = "no-such"
+	_, err := db.GetEngine(db.DefaultContext).ID(user30EmptyRepo.ID).Cols("is_empty", "default_branch").Update(user30EmptyRepo)
+	require.NoError(t, err)
+	user30EmptyRepo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 30, Name: "empty"})
+	assert.True(t, user30EmptyRepo.IsEmpty)
+
+	req = NewRequest(t, "GET", "/user30/empty")
+	resp = session.MakeRequest(t, req, http.StatusSeeOther)
+	redirect = test.RedirectURL(resp)
+	assert.Equal(t, "/user30/empty", redirect)
+
+	req = NewRequest(t, "GET", "/user30/empty")
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	assert.Contains(t, resp.Body.String(), "test-file.md")
 }
 
 func TestEmptyRepoUploadFile(t *testing.T) {