aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLunny Xiao <xiaolunwen@gmail.com>2025-01-13 23:35:34 -0800
committerGitHub <noreply@github.com>2025-01-14 15:35:34 +0800
commit6410c34b7f6b7ee5daf27b057b197e63fdeb9be6 (patch)
treec1dea60bc8d05e28d1df64cae47d5c9d1970a0d9
parent3a749fc816ed2957527bd9529e688adf84dadecf (diff)
downloadgitea-6410c34b7f6b7ee5daf27b057b197e63fdeb9be6.tar.gz
gitea-6410c34b7f6b7ee5daf27b057b197e63fdeb9be6.zip
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>
-rw-r--r--models/unittest/fscopy.go50
-rw-r--r--routers/web/repo/repo.go2
-rw-r--r--routers/web/repo/view_home.go2
-rw-r--r--routers/web/web.go102
-rw-r--r--services/context/permission.go4
-rw-r--r--services/context/repo.go78
-rw-r--r--tests/integration/empty_repo_test.go25
7 files changed, 120 insertions, 143 deletions
diff --git a/models/unittest/fscopy.go b/models/unittest/fscopy.go
index 690089bbc5..b7ba6b7ef5 100644
--- a/models/unittest/fscopy.go
+++ b/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
diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go
index e5c397ec54..6c2bc74e69 100644
--- a/routers/web/repo/repo.go
+++ b/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
}
diff --git a/routers/web/repo/view_home.go b/routers/web/repo/view_home.go
index 622072adeb..54a58cd8be 100644
--- a/routers/web/repo/view_home.go
+++ b/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.
diff --git a/routers/web/web.go b/routers/web/web.go
index d3d0360396..c2dac70546 100644
--- a/routers/web/web.go
+++ b/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
diff --git a/services/context/permission.go b/services/context/permission.go
index 359d51c272..0d69ccc4a4 100644
--- a/services/context/permission.go
+++ b/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)
diff --git a/services/context/repo.go b/services/context/repo.go
index cd5ac7c6c9..33a39dced1 100644
--- a/services/context/repo.go
+++ b/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
}
diff --git a/tests/integration/empty_repo_test.go b/tests/integration/empty_repo_test.go
index 8cab04a5a5..b19774a826 100644
--- a/tests/integration/empty_repo_test.go
+++ b/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) {