Skip to content

Commit

Permalink
Refactor ref type (go-gitea#33242)
Browse files Browse the repository at this point in the history 
Major changes:

1. do not sync ".keep" file during tests
2. fix incorrect route handler and empty repo handling (backported as go-gitea#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]>
  • Loading branch information
@lunny @wxiaoguang
lunny and wxiaoguang authored Jan 14, 2025
1 parent 3a749fc commit 6410c34
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 143 deletions.
50 changes: 18 additions & 32 deletions models/unittest/fscopy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/repo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/view_home.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
102 changes: 48 additions & 54 deletions routers/web/web.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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

Expand Down
4 changes: 2 additions & 2 deletions services/context/permission.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 6410c34

Please sign in to comment.