From 6410c34b7f6b7ee5daf27b057b197e63fdeb9be6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 13 Jan 2025 23:35:34 -0800 Subject: [PATCH] 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 --- models/unittest/fscopy.go | 50 +++++-------- routers/web/repo/repo.go | 2 +- routers/web/repo/view_home.go | 2 +- routers/web/web.go | 102 +++++++++++++-------------- services/context/permission.go | 4 +- services/context/repo.go | 78 +++++++------------- tests/integration/empty_repo_test.go | 25 +++++++ 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) {