diff --git a/modules/gitrepo/gitrepo.go b/modules/gitrepo/gitrepo.go index 746ae41f019c7..0df5fa255dcd6 100644 --- a/modules/gitrepo/gitrepo.go +++ b/modules/gitrepo/gitrepo.go @@ -55,7 +55,7 @@ func RepositoryFromContextOrOpen(ctx context.Context, repo Repository) (*git.Rep // RepositoryFromRequestContextOrOpen opens the repository at the given relative path in the provided request context // The repo will be automatically closed when the request context is done -func RepositoryFromRequestContextOrOpen(ctx *reqctx.RequestContext, repo Repository) (*git.Repository, error) { +func RepositoryFromRequestContextOrOpen(ctx reqctx.RequestContext, repo Repository) (*git.Repository, error) { ck := contextKey{repoPath: repoPath(repo)} if gitRepo, ok := ctx.Value(ck).(*git.Repository); ok { return gitRepo, nil diff --git a/modules/reqctx/reqctx.go b/modules/reqctx/reqctx.go index 49c9ba1cf5dd4..7e4a8fc89cf66 100644 --- a/modules/reqctx/reqctx.go +++ b/modules/reqctx/reqctx.go @@ -5,6 +5,7 @@ package reqctx import ( "context" + "io" "sync" "time" @@ -15,8 +16,16 @@ type RequestContextKeyType struct{} var RequestContextKey RequestContextKeyType +type RequestContext interface { + context.Context + SetContextValue(k, v any) + GetData() ContextData + AddCleanUp(f func()) + AddCloser(c io.Closer) +} + // RequestContext is a short-lived context that is used to store request-specific data. -type RequestContext struct { +type requestContext struct { ctx context.Context data ContextData @@ -26,19 +35,19 @@ type RequestContext struct { cleanUpFuncs []func() } -func (r *RequestContext) Deadline() (deadline time.Time, ok bool) { +func (r *requestContext) Deadline() (deadline time.Time, ok bool) { return r.ctx.Deadline() } -func (r *RequestContext) Done() <-chan struct{} { +func (r *requestContext) Done() <-chan struct{} { return r.ctx.Done() } -func (r *RequestContext) Err() error { +func (r *requestContext) Err() error { return r.ctx.Err() } -func (r *RequestContext) Value(key any) any { +func (r *requestContext) Value(key any) any { if key == RequestContextKey { return r } @@ -51,42 +60,46 @@ func (r *RequestContext) Value(key any) any { return r.ctx.Value(key) } -func (r *RequestContext) SetContextValue(k, v any) { +func (r *requestContext) SetContextValue(k, v any) { r.mu.Lock() r.values[k] = v r.mu.Unlock() } // GetData and the underlying ContextData are not thread-safe, callers should ensure thread-safety. -func (r *RequestContext) GetData() ContextData { +func (r *requestContext) GetData() ContextData { if r.data == nil { r.data = make(ContextData) } return r.data } -func (r *RequestContext) AddCleanUp(f func()) { +func (r *requestContext) AddCleanUp(f func()) { r.mu.Lock() r.cleanUpFuncs = append(r.cleanUpFuncs, f) r.mu.Unlock() } -func (r *RequestContext) cleanUp() { +func (r *requestContext) AddCloser(c io.Closer) { + r.AddCleanUp(func() { _ = c.Close() }) +} + +func (r *requestContext) cleanUp() { for _, f := range r.cleanUpFuncs { f() } } -func GetRequestContext(ctx context.Context) *RequestContext { - if req, ok := ctx.Value(RequestContextKey).(*RequestContext); ok { +func GetRequestContext(ctx context.Context) RequestContext { + if req, ok := ctx.Value(RequestContextKey).(*requestContext); ok { return req } return nil } -func NewRequestContext(parentCtx context.Context, profDesc string) (_ *RequestContext, finished func()) { +func NewRequestContext(parentCtx context.Context, profDesc string) (_ RequestContext, finished func()) { ctx, _, processFinished := process.GetManager().AddTypedContext(parentCtx, profDesc, process.RequestProcessType, true) - reqCtx := &RequestContext{ctx: ctx, values: make(map[any]any)} + reqCtx := &requestContext{ctx: ctx, values: make(map[any]any)} return reqCtx, func() { reqCtx.cleanUp() processFinished() @@ -95,7 +108,7 @@ func NewRequestContext(parentCtx context.Context, profDesc string) (_ *RequestCo // NewRequestContextForTest creates a new RequestContext for testing purposes // It doesn't add the context to the process manager, nor do cleanup -func NewRequestContextForTest(parentCtx context.Context) *RequestContext { - reqCtx := &RequestContext{ctx: parentCtx, values: make(map[any]any)} +func NewRequestContextForTest(parentCtx context.Context) RequestContext { + reqCtx := &requestContext{ctx: parentCtx, values: make(map[any]any)} return reqCtx } diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 9a31aec31446d..1a2b345b36c78 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -729,15 +729,11 @@ func CreateBranchProtection(ctx *context.APIContext) { } else { if !isPlainRule { if ctx.Repo.GitRepo == nil { - ctx.Repo.GitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return } - defer func() { - ctx.Repo.GitRepo.Close() - ctx.Repo.GitRepo = nil - }() } // FIXME: since we only need to recheck files protected rules, we could improve this matchedBranches, err := git_model.FindAllMatchedBranches(ctx, ctx.Repo.Repository.ID, ruleName) @@ -1066,10 +1062,6 @@ func EditBranchProtection(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return } - defer func() { - ctx.Repo.GitRepo.Close() - ctx.Repo.GitRepo = nil - }() } // FIXME: since we only need to recheck files protected rules, we could improve this diff --git a/routers/api/v1/repo/compare.go b/routers/api/v1/repo/compare.go index 1678bc033c639..a1813a8a76540 100644 --- a/routers/api/v1/repo/compare.go +++ b/routers/api/v1/repo/compare.go @@ -44,13 +44,12 @@ func CompareDiff(ctx *context.APIContext) { // "$ref": "#/responses/notFound" if ctx.Repo.GitRepo == nil { - gitRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + var err error + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return } - ctx.Repo.GitRepo = gitRepo - defer gitRepo.Close() } infoPath := ctx.PathParam("*") diff --git a/routers/api/v1/repo/download.go b/routers/api/v1/repo/download.go index 3620c1465fe9c..a8a23c4a8d425 100644 --- a/routers/api/v1/repo/download.go +++ b/routers/api/v1/repo/download.go @@ -28,13 +28,12 @@ func DownloadArchive(ctx *context.APIContext) { } if ctx.Repo.GitRepo == nil { - gitRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + var err error + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return } - ctx.Repo.GitRepo = gitRepo - defer gitRepo.Close() } r, err := archiver_service.NewRequest(ctx.Repo.Repository.ID, ctx.Repo.GitRepo, ctx.PathParam("*"), tp) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 83848b7add506..af7de1d329c00 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -287,13 +287,12 @@ func GetArchive(ctx *context.APIContext) { // "$ref": "#/responses/notFound" if ctx.Repo.GitRepo == nil { - gitRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + var err error + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return } - ctx.Repo.GitRepo = gitRepo - defer gitRepo.Close() } archiveDownload(ctx) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 40990a28cbdee..415a7a1b01ee6 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -726,12 +726,11 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err if ctx.Repo.GitRepo == nil && !repo.IsEmpty { var err error - ctx.Repo.GitRepo, err = gitrepo.OpenRepository(ctx, repo) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo) if err != nil { ctx.Error(http.StatusInternalServerError, "Unable to OpenRepository", err) return err } - defer ctx.Repo.GitRepo.Close() } // Default branch only updated if changed and exist or the repository is empty diff --git a/routers/api/v1/repo/transfer.go b/routers/api/v1/repo/transfer.go index 787ec34404f0e..b2090cac41cad 100644 --- a/routers/api/v1/repo/transfer.go +++ b/routers/api/v1/repo/transfer.go @@ -100,7 +100,7 @@ func Transfer(ctx *context.APIContext) { } if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() + _ = ctx.Repo.GitRepo.Close() ctx.Repo.GitRepo = nil } diff --git a/routers/private/internal_repo.go b/routers/private/internal_repo.go index aad0a3fb1aa83..318ab2ba8288f 100644 --- a/routers/private/internal_repo.go +++ b/routers/private/internal_repo.go @@ -4,7 +4,6 @@ package private import ( - "context" "fmt" "net/http" @@ -18,14 +17,14 @@ import ( // This file contains common functions relating to setting the Repository for the internal routes // RepoAssignment assigns the repository and gitrepository to the private context -func RepoAssignment(ctx *gitea_context.PrivateContext) context.CancelFunc { +func RepoAssignment(ctx *gitea_context.PrivateContext) { ownerName := ctx.PathParam(":owner") repoName := ctx.PathParam(":repo") repo := loadRepository(ctx, ownerName, repoName) if ctx.Written() { // Error handled in loadRepository - return nil + return } gitRepo, err := gitrepo.OpenRepository(ctx, repo) @@ -34,23 +33,14 @@ func RepoAssignment(ctx *gitea_context.PrivateContext) context.CancelFunc { ctx.JSON(http.StatusInternalServerError, private.Response{ Err: fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err), }) - return nil + return } ctx.Repo = &gitea_context.Repository{ Repository: repo, GitRepo: gitRepo, } - - // We opened it, we should close it - cancel := func() { - // If it's been set to nil then assume someone else has closed it. - if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() - } - } - - return cancel + ctx.AddCloser(gitRepo) // We opened it, we should close it } func loadRepository(ctx *gitea_context.PrivateContext, ownerName, repoName string) *repo_model.Repository { diff --git a/routers/web/web.go b/routers/web/web.go index aa37d4dc10528..4bcc097431495 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -4,7 +4,6 @@ package web import ( - gocontext "context" "net/http" "strings" @@ -1521,24 +1520,24 @@ func registerRoutes(m *web.Router) { m.Group("/blob_excerpt", func() { m.Get("/{sha}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.ExcerptBlob) - }, func(ctx *context.Context) gocontext.CancelFunc { + }, func(ctx *context.Context) { // FIXME: refactor this function, use separate routes for wiki/code if ctx.FormBool("wiki") { ctx.Data["PageIsWiki"] = true repo.MustEnableWiki(ctx) - return nil + return } if ctx.Written() { - return nil + return } - cancel := context.RepoRef()(ctx) + context.RepoRef()(ctx) if ctx.Written() { - return cancel + return } repo.MustBeNotEmpty(ctx) - return cancel + return }) m.Group("/media", func() { diff --git a/services/context/api.go b/services/context/api.go index 5fbcf3c786d85..099ff9f2776ec 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -5,7 +5,6 @@ package context import ( - "context" "fmt" "net/http" "net/url" @@ -265,31 +264,24 @@ func (ctx *APIContext) NotFound(objs ...any) { // ReferencesGitRepo injects the GitRepo into the Context // you can optional skip the IsEmpty check -func ReferencesGitRepo(allowEmpty ...bool) func(ctx *APIContext) (cancel context.CancelFunc) { - return func(ctx *APIContext) (cancel context.CancelFunc) { +func ReferencesGitRepo(allowEmpty ...bool) func(ctx *APIContext) { + return func(ctx *APIContext) { // Empty repository does not have reference information. if ctx.Repo.Repository.IsEmpty && !(len(allowEmpty) != 0 && allowEmpty[0]) { - return nil + return } // For API calls. if ctx.Repo.GitRepo == nil { - gitRepo, err := gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + var err error + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, fmt.Sprintf("Open Repository %v failed", ctx.Repo.Repository.FullName()), err) - return cancel - } - ctx.Repo.GitRepo = gitRepo - // We opened it, we should close it - return func() { - // If it's been set to nil then assume someone else has closed it. - if ctx.Repo.GitRepo != nil { - _ = ctx.Repo.GitRepo.Close() - } + return } } - return cancel + return } } diff --git a/services/context/base.go b/services/context/base.go index 64cdc2383a265..f5c1ffacc8428 100644 --- a/services/context/base.go +++ b/services/context/base.go @@ -4,6 +4,7 @@ package context import ( + "context" "fmt" "html/template" "io" @@ -29,7 +30,9 @@ type BaseContextKeyType struct{} var BaseContextKey BaseContextKeyType type Base struct { - *reqctx.RequestContext + context.Context + + reqCtx reqctx.RequestContext Resp ResponseWriter Req *http.Request @@ -42,6 +45,22 @@ type Base struct { Locale translation.Locale } +func (b *Base) SetContextValue(k, v any) { + b.reqCtx.SetContextValue(k, v) +} + +func (b *Base) GetData() reqctx.ContextData { + return b.reqCtx.GetData() +} + +func (b *Base) AddCleanUp(f func()) { + b.reqCtx.AddCleanUp(f) +} + +func (b *Base) AddCloser(c io.Closer) { + b.reqCtx.AddCloser(c) +} + // AppendAccessControlExposeHeaders append headers by name to "Access-Control-Expose-Headers" header func (b *Base) AppendAccessControlExposeHeaders(names ...string) { val := b.RespHeader().Get("Access-Control-Expose-Headers") @@ -262,18 +281,18 @@ func (b *Base) TrN(cnt any, key1, keyN string, args ...any) template.HTML { } func NewBaseContext(resp http.ResponseWriter, req *http.Request) *Base { - ctx := reqctx.GetRequestContext(req.Context()) + reqCtx := reqctx.GetRequestContext(req.Context()) b := &Base{ - RequestContext: ctx, - - Req: req, - Resp: WrapResponseWriter(resp), - Locale: middleware.Locale(resp, req), - Data: middleware.GetContextData(req.Context()), + Context: req.Context(), + reqCtx: reqCtx, + Req: req, + Resp: WrapResponseWriter(resp), + Locale: middleware.Locale(resp, req), + Data: reqCtx.GetData(), } b.Req = b.Req.WithContext(b) - ctx.SetContextValue(BaseContextKey, b) - ctx.SetContextValue(translation.ContextKey, b.Locale) - ctx.SetContextValue(httplib.RequestContextKey, b.Req) + reqCtx.SetContextValue(BaseContextKey, b) + reqCtx.SetContextValue(translation.ContextKey, b.Locale) + reqCtx.SetContextValue(httplib.RequestContextKey, b.Req) return b } diff --git a/services/context/repo.go b/services/context/repo.go index e96916ca421a3..12a0f65457fc3 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -397,11 +397,13 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) { } // RepoAssignment returns a middleware to handle repository assignment -func RepoAssignment(ctx *Context) context.CancelFunc { +func RepoAssignment(ctx *Context) { if _, repoAssignmentOnce := ctx.Data["repoAssignmentExecuted"]; repoAssignmentOnce { // FIXME: it should panic in dev/test modes to have a clear behavior - log.Trace("RepoAssignment was exec already, skipping second call ...") - return nil + if !setting.IsProd || setting.IsInTesting { + panic("RepoAssignment should not be executed twice") + } + return } ctx.Data["repoAssignmentExecuted"] = true @@ -429,7 +431,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { // https://github.com/golang/go/issues/19760 if ctx.FormString("go-get") == "1" { EarlyResponseForGoGetMeta(ctx) - return nil + return } if redirectUserID, err := user_model.LookupUserRedirect(ctx, userName); err == nil { @@ -442,7 +444,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { } else { ctx.ServerError("GetUserByName", err) } - return nil + return } } ctx.Repo.Owner = owner @@ -467,7 +469,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { redirectPath += "?" + ctx.Req.URL.RawQuery } ctx.Redirect(path.Join(setting.AppSubURL, redirectPath)) - return nil + return } // Get repository. @@ -480,7 +482,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { } else if repo_model.IsErrRedirectNotExist(err) { if ctx.FormString("go-get") == "1" { EarlyResponseForGoGetMeta(ctx) - return nil + return } ctx.NotFound("GetRepositoryByName", nil) } else { @@ -489,13 +491,13 @@ func RepoAssignment(ctx *Context) context.CancelFunc { } else { ctx.ServerError("GetRepositoryByName", err) } - return nil + return } repo.Owner = owner repoAssignment(ctx, repo) if ctx.Written() { - return nil + return } ctx.Repo.RepoLink = repo.Link() @@ -520,7 +522,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { }) if err != nil { ctx.ServerError("GetReleaseCountByRepoID", err) - return nil + return } ctx.Data["NumReleases"], err = db.Count[repo_model.Release](ctx, repo_model.FindReleasesOptions{ // only show draft releases for users who can write, read-only users shouldn't see draft releases. @@ -529,7 +531,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { }) if err != nil { ctx.ServerError("GetReleaseCountByRepoID", err) - return nil + return } ctx.Data["Title"] = owner.Name + "/" + repo.Name @@ -546,14 +548,14 @@ func RepoAssignment(ctx *Context) context.CancelFunc { canSignedUserFork, err := repo_module.CanUserForkRepo(ctx, ctx.Doer, ctx.Repo.Repository) if err != nil { ctx.ServerError("CanUserForkRepo", err) - return nil + return } ctx.Data["CanSignedUserFork"] = canSignedUserFork userAndOrgForks, err := repo_model.GetForksByUserAndOrgs(ctx, ctx.Doer, ctx.Repo.Repository) if err != nil { ctx.ServerError("GetForksByUserAndOrgs", err) - return nil + return } ctx.Data["UserAndOrgForks"] = userAndOrgForks @@ -587,14 +589,14 @@ func RepoAssignment(ctx *Context) context.CancelFunc { if repo.IsFork { RetrieveBaseRepo(ctx, repo) if ctx.Written() { - return nil + return } } if repo.IsGenerated() { RetrieveTemplateRepo(ctx, repo) if ctx.Written() { - return nil + return } } @@ -609,10 +611,18 @@ func RepoAssignment(ctx *Context) context.CancelFunc { if !isHomeOrSettings { ctx.Redirect(ctx.Repo.RepoLink) } - return nil + return } - gitRepo, err := gitrepo.OpenRepository(ctx, repo) + if ctx.Repo.GitRepo != nil { + if !setting.IsProd || setting.IsInTesting { + panic("RepoAssignment: GitRepo should be nil") + } + _ = ctx.Repo.GitRepo.Close() + ctx.Repo.GitRepo = nil + } + + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo) if err != nil { if strings.Contains(err.Error(), "repository does not exist") || strings.Contains(err.Error(), "no such file or directory") { log.Error("Repository %-v has a broken repository on the file system: %s Error: %v", ctx.Repo.Repository, ctx.Repo.Repository.RepoPath(), err) @@ -622,28 +632,16 @@ func RepoAssignment(ctx *Context) context.CancelFunc { if !isHomeOrSettings { ctx.Redirect(ctx.Repo.RepoLink) } - return nil + return } ctx.ServerError("RepoAssignment Invalid repo "+repo.FullName(), err) - return nil - } - if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() - } - ctx.Repo.GitRepo = gitRepo - - // We opened it, we should close it - cancel := func() { - // If it's been set to nil then assume someone else has closed it. - if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() - } + return } // Stop at this point when the repo is empty. if ctx.Repo.Repository.IsEmpty { ctx.Data["BranchName"] = ctx.Repo.Repository.DefaultBranch - return cancel + return } branchOpts := git_model.FindBranchOptions{ @@ -654,7 +652,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { branchesTotal, err := db.Count[git_model.Branch](ctx, branchOpts) if err != nil { ctx.ServerError("CountBranches", err) - return cancel + return } // non-empty repo should have at least 1 branch, so this repository's branches haven't been synced yet @@ -662,7 +660,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { branchesTotal, err = repo_module.SyncRepoBranches(ctx, ctx.Repo.Repository.ID, 0) if err != nil { ctx.ServerError("SyncRepoBranches", err) - return cancel + return } } @@ -670,7 +668,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { // If no branch is set in the request URL, try to guess a default one. if len(ctx.Repo.BranchName) == 0 { - if len(ctx.Repo.Repository.DefaultBranch) > 0 && gitRepo.IsBranchExist(ctx.Repo.Repository.DefaultBranch) { + if len(ctx.Repo.Repository.DefaultBranch) > 0 && ctx.Repo.GitRepo.IsBranchExist(ctx.Repo.Repository.DefaultBranch) { ctx.Repo.BranchName = ctx.Repo.Repository.DefaultBranch } else { ctx.Repo.BranchName, _ = gitrepo.GetDefaultBranch(ctx, ctx.Repo.Repository) @@ -711,12 +709,12 @@ func RepoAssignment(ctx *Context) context.CancelFunc { repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, ctx.Repo.Repository) if err != nil { ctx.ServerError("GetPendingRepositoryTransfer", err) - return cancel + return } if err := repoTransfer.LoadAttributes(ctx); err != nil { ctx.ServerError("LoadRecipient", err) - return cancel + return } ctx.Data["RepoTransfer"] = repoTransfer @@ -731,7 +729,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { ctx.Data["GoDocDirectory"] = fullURLPrefix + "{/dir}" ctx.Data["GoDocFile"] = fullURLPrefix + "{/dir}/{file}#L{line}" } - return cancel + return } // RepoRefType type of repo reference @@ -750,7 +748,7 @@ const headRefName = "HEAD" // RepoRef handles repository reference names when the ref name is not // explicitly given -func RepoRef() func(*Context) context.CancelFunc { +func RepoRef() func(*Context) { // since no ref name is explicitly specified, ok to just use branch return RepoRefByType(RepoRefBranch) } @@ -865,9 +863,9 @@ type RepoRefByTypeOptions struct { // RepoRefByType handles repository reference name for a specific type // of repository reference -func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func(*Context) context.CancelFunc { +func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func(*Context) { opt := util.OptionalArg(opts) - return func(ctx *Context) (cancel context.CancelFunc) { + return func(ctx *Context) { refType := detectRefType // Empty repository does not have reference information. if ctx.Repo.Repository.IsEmpty { @@ -875,7 +873,6 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.IsViewBranch = true ctx.Repo.BranchName = ctx.Repo.Repository.DefaultBranch ctx.Data["TreePath"] = "" - return nil } var ( @@ -884,17 +881,10 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ) if ctx.Repo.GitRepo == nil { - ctx.Repo.GitRepo, err = gitrepo.OpenRepository(ctx, ctx.Repo.Repository) + 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 nil - } - // We opened it, we should close it - cancel = func() { - // If it's been set to nil then assume someone else has closed it. - if ctx.Repo.GitRepo != nil { - ctx.Repo.GitRepo.Close() - } + return } } @@ -924,7 +914,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.Repository.MarkAsBrokenEmpty() } else { ctx.ServerError("GetBranchCommit", err) - return cancel + return } ctx.Repo.IsViewBranch = true } else { @@ -941,7 +931,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Flash.Info(ctx.Tr("repo.branch.renamed", refName, renamedBranchName)) link := setting.AppSubURL + strings.Replace(ctx.Req.URL.EscapedPath(), util.PathEscapeSegments(refName), util.PathEscapeSegments(renamedBranchName), 1) ctx.Redirect(link) - return cancel + return } if refType == RepoRefBranch && ctx.Repo.GitRepo.IsBranchExist(refName) { @@ -951,7 +941,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refName) if err != nil { ctx.ServerError("GetBranchCommit", err) - return cancel + return } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() } else if refType == RepoRefTag && ctx.Repo.GitRepo.IsTagExist(refName) { @@ -962,10 +952,10 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func if err != nil { if git.IsErrNotExist(err) { ctx.NotFound("GetTagCommit", err) - return cancel + return } ctx.ServerError("GetTagCommit", err) - return cancel + return } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() } else if git.IsStringLikelyCommitID(ctx.Repo.GetObjectFormat(), refName, 7) { @@ -975,7 +965,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName) if err != nil { ctx.NotFound("GetCommit", err) - return cancel + return } // If short commit ID add canonical link header if len(refName) < ctx.Repo.GetObjectFormat().FullLength() { @@ -984,10 +974,10 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func } } else { if opt.IgnoreNotExistErr { - return cancel + return } ctx.NotFound("RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refName)) - return cancel + return } if guessLegacyPath { @@ -999,7 +989,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.BranchNameSubURL(), util.PathEscapeSegments(ctx.Repo.TreePath)) ctx.Redirect(redirect) - return cancel + return } } @@ -1017,12 +1007,11 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.Repo.CommitsCount, err = ctx.Repo.GetCommitsCount() if err != nil { ctx.ServerError("GetCommitsCount", err) - return cancel + return } ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount ctx.Repo.GitRepo.LastCommitCache = git.NewLastCommitCache(ctx.Repo.CommitsCount, ctx.Repo.Repository.FullName(), ctx.Repo.GitRepo, cache.GetCache()) - - return cancel + return } }