diff --git a/modules/gitrepo/gitrepo.go b/modules/gitrepo/gitrepo.go index 831b9d7bb7..540b724489 100644 --- a/modules/gitrepo/gitrepo.go +++ b/modules/gitrepo/gitrepo.go @@ -43,19 +43,20 @@ type contextKey struct { } // RepositoryFromContextOrOpen attempts to get the repository from the context or just opens it +// The caller must call "defer gitRepo.Close()" func RepositoryFromContextOrOpen(ctx context.Context, repo Repository) (*git.Repository, io.Closer, error) { - ds := reqctx.GetRequestDataStore(ctx) - if ds != nil { - gitRepo, err := RepositoryFromRequestContextOrOpen(ctx, ds, repo) + reqCtx := reqctx.FromContext(ctx) + if reqCtx != nil { + gitRepo, err := RepositoryFromRequestContextOrOpen(reqCtx, repo) return gitRepo, util.NopCloser{}, err } gitRepo, err := OpenRepository(ctx, repo) return gitRepo, gitRepo, err } -// 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 context.Context, ds reqctx.RequestDataStore, repo Repository) (*git.Repository, error) { +// RepositoryFromRequestContextOrOpen opens the repository at the given relative path in the provided request context. +// Caller shouldn't close the git repo manually, the git repo will be automatically closed when the request context is done. +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 @@ -64,7 +65,7 @@ func RepositoryFromRequestContextOrOpen(ctx context.Context, ds reqctx.RequestDa if err != nil { return nil, err } - ds.AddCloser(gitRepo) - ds.SetContextValue(ck, gitRepo) + ctx.AddCloser(gitRepo) + ctx.SetContextValue(ck, gitRepo) return gitRepo, nil } diff --git a/modules/reqctx/datastore.go b/modules/reqctx/datastore.go index 66361a4587..94232450f3 100644 --- a/modules/reqctx/datastore.go +++ b/modules/reqctx/datastore.go @@ -88,6 +88,21 @@ func (r *requestDataStore) cleanUp() { } } +type RequestContext interface { + context.Context + RequestDataStore +} + +func FromContext(ctx context.Context) RequestContext { + // here we must use the current ctx and the underlying store + // the current ctx guarantees that the ctx deadline/cancellation/values are respected + // the underlying store guarantees that the request-specific data is available + if store := GetRequestDataStore(ctx); store != nil { + return &requestContext{Context: ctx, RequestDataStore: store} + } + return nil +} + func GetRequestDataStore(ctx context.Context) RequestDataStore { if req, ok := ctx.Value(RequestDataStoreKey).(*requestDataStore); ok { return req @@ -97,11 +112,11 @@ func GetRequestDataStore(ctx context.Context) RequestDataStore { type requestContext struct { context.Context - dataStore *requestDataStore + RequestDataStore } func (c *requestContext) Value(key any) any { - if v := c.dataStore.GetContextValue(key); v != nil { + if v := c.GetContextValue(key); v != nil { return v } return c.Context.Value(key) @@ -109,9 +124,10 @@ func (c *requestContext) Value(key any) any { func NewRequestContext(parentCtx context.Context, profDesc string) (_ context.Context, finished func()) { ctx, _, processFinished := process.GetManager().AddTypedContext(parentCtx, profDesc, process.RequestProcessType, true) - reqCtx := &requestContext{Context: ctx, dataStore: &requestDataStore{values: make(map[any]any)}} + store := &requestDataStore{values: make(map[any]any)} + reqCtx := &requestContext{Context: ctx, RequestDataStore: store} return reqCtx, func() { - reqCtx.dataStore.cleanUp() + store.cleanUp() processFinished() } } @@ -119,5 +135,5 @@ func NewRequestContext(parentCtx context.Context, profDesc string) (_ context.Co // 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) context.Context { - return &requestContext{Context: parentCtx, dataStore: &requestDataStore{values: make(map[any]any)}} + return &requestContext{Context: parentCtx, RequestDataStore: &requestDataStore{values: make(map[any]any)}} } diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 0116300629..965b4a24cd 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -286,7 +286,6 @@ func QueryBuild(a ...any) template.URL { reqPath = s1 + "?" s = s2 } - } for i := len(a) % 2; i < len(a); i += 2 { k, ok := a[i].(string) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 67dfda39a8..2fcdd02058 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -729,7 +729,7 @@ func CreateBranchProtection(ctx *context.APIContext) { } else { if !isPlainRule { if ctx.Repo.GitRepo == nil { - ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return @@ -1057,7 +1057,7 @@ func EditBranchProtection(ctx *context.APIContext) { } else { if !isPlainRule { if ctx.Repo.GitRepo == nil { - ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return diff --git a/routers/api/v1/repo/compare.go b/routers/api/v1/repo/compare.go index 87b890cb62..a1813a8a76 100644 --- a/routers/api/v1/repo/compare.go +++ b/routers/api/v1/repo/compare.go @@ -45,7 +45,7 @@ func CompareDiff(ctx *context.APIContext) { if ctx.Repo.GitRepo == nil { var err error - ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return diff --git a/routers/api/v1/repo/download.go b/routers/api/v1/repo/download.go index eb967772ed..a8a23c4a8d 100644 --- a/routers/api/v1/repo/download.go +++ b/routers/api/v1/repo/download.go @@ -29,7 +29,7 @@ func DownloadArchive(ctx *context.APIContext) { if ctx.Repo.GitRepo == nil { var err error - ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 8a4f78a3d7..1ad55d225b 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -282,7 +282,7 @@ func GetArchive(ctx *context.APIContext) { if ctx.Repo.GitRepo == nil { var err error - ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) if err != nil { ctx.Error(http.StatusInternalServerError, "OpenRepository", err) return diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index a192e241b7..ce09e7fc0f 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -726,7 +726,7 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err if ctx.Repo.GitRepo == nil && !repo.IsEmpty { var err error - ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, repo) + ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo) if err != nil { ctx.Error(http.StatusInternalServerError, "Unable to OpenRepository", err) return err diff --git a/routers/private/internal_repo.go b/routers/private/internal_repo.go index 8a53e1ed23..e111d6689e 100644 --- a/routers/private/internal_repo.go +++ b/routers/private/internal_repo.go @@ -27,7 +27,7 @@ func RepoAssignment(ctx *gitea_context.PrivateContext) { return } - gitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, repo) + gitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo) if err != nil { log.Error("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err) ctx.JSON(http.StatusInternalServerError, private.Response{ diff --git a/routers/web/org/home.go b/routers/web/org/home.go index 33115c9259..037e53e214 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -178,8 +178,7 @@ type prepareOrgProfileReadmeOptions struct { func prepareOrgProfileReadme(ctx *context.Context, opts prepareOrgProfileReadmeOptions) bool { profileRepoName := util.Iif(opts.viewAsPrivate, shared_user.RepoNameProfilePrivate, shared_user.RepoNameProfile) - profileDbRepo, profileGitRepo, profileReadme, profileClose := shared_user.FindOwnerProfileReadme(ctx, ctx.Doer, profileRepoName) - defer profileClose() + profileDbRepo, profileReadme := shared_user.FindOwnerProfileReadme(ctx, ctx.Doer, profileRepoName) if opts.viewAsPrivate { ctx.Data["HasPrivateProfileReadme"] = profileReadme != nil @@ -187,7 +186,7 @@ func prepareOrgProfileReadme(ctx *context.Context, opts prepareOrgProfileReadmeO ctx.Data["HasPublicProfileReadme"] = profileReadme != nil } - if profileGitRepo == nil || profileReadme == nil || opts.viewRepositories { + if profileReadme == nil || opts.viewRepositories { return false } diff --git a/routers/web/shared/user/header.go b/routers/web/shared/user/header.go index 54f2b4a4af..c9af52c7f0 100644 --- a/routers/web/shared/user/header.go +++ b/routers/web/shared/user/header.go @@ -103,37 +103,45 @@ func PrepareContextForProfileBigAvatar(ctx *context.Context) { } } -func FindOwnerProfileReadme(ctx *context.Context, doer *user_model.User, optProfileRepoName ...string) (profileDbRepo *repo_model.Repository, profileGitRepo *git.Repository, profileReadmeBlob *git.Blob, profileClose func()) { - profileRepoName := util.OptionalArg(optProfileRepoName, ".profile") +func FindOwnerProfileReadme(ctx *context.Context, doer *user_model.User, optProfileRepoName ...string) (profileDbRepo *repo_model.Repository, profileReadmeBlob *git.Blob) { + profileRepoName := util.OptionalArg(optProfileRepoName, RepoNameProfile) profileDbRepo, err := repo_model.GetRepositoryByName(ctx, ctx.ContextUser.ID, profileRepoName) - if err == nil { - perm, err := access_model.GetUserRepoPermission(ctx, profileDbRepo, doer) - if err == nil && !profileDbRepo.IsEmpty && perm.CanRead(unit.TypeCode) { - if profileGitRepo, err = gitrepo.OpenRepository(ctx, profileDbRepo); err != nil { - log.Error("FindOwnerProfileReadme failed to OpenRepository: %v", err) - } else { - if commit, err := profileGitRepo.GetBranchCommit(profileDbRepo.DefaultBranch); err != nil { - log.Error("FindOwnerProfileReadme failed to GetBranchCommit: %v", err) - } else { - profileReadmeBlob, _ = commit.GetBlobByPath("README.md") - } - } + if err != nil { + if !repo_model.IsErrRepoNotExist(err) { + log.Error("FindOwnerProfileReadme failed to GetRepositoryByName: %v", err) } - } else if !repo_model.IsErrRepoNotExist(err) { + return nil, nil + } + + perm, err := access_model.GetUserRepoPermission(ctx, profileDbRepo, doer) + if err != nil { log.Error("FindOwnerProfileReadme failed to GetRepositoryByName: %v", err) + return nil, nil } - return profileDbRepo, profileGitRepo, profileReadmeBlob, func() { - if profileGitRepo != nil { - _ = profileGitRepo.Close() - } + if profileDbRepo.IsEmpty || !perm.CanRead(unit.TypeCode) { + return nil, nil } + + profileGitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, profileDbRepo) + if err != nil { + log.Error("FindOwnerProfileReadme failed to OpenRepository: %v", err) + return nil, nil + } + + commit, err := profileGitRepo.GetBranchCommit(profileDbRepo.DefaultBranch) + if err != nil { + log.Error("FindOwnerProfileReadme failed to GetBranchCommit: %v", err) + return nil, nil + } + + profileReadmeBlob, _ = commit.GetBlobByPath("README.md") // no need to handle this error + return profileDbRepo, profileReadmeBlob } func RenderUserHeader(ctx *context.Context) { prepareContextForCommonProfile(ctx) - _, _, profileReadmeBlob, profileClose := FindOwnerProfileReadme(ctx, ctx.Doer) - defer profileClose() + _, profileReadmeBlob := FindOwnerProfileReadme(ctx, ctx.Doer) ctx.Data["HasPublicProfileReadme"] = profileReadmeBlob != nil } @@ -182,13 +190,11 @@ func RenderOrgHeader(ctx *context.Context) error { } // FIXME: only do database query, do not open it - _, _, profileReadmeBlob, profileClose := FindOwnerProfileReadme(ctx, ctx.Doer) - defer profileClose() + _, profileReadmeBlob := FindOwnerProfileReadme(ctx, ctx.Doer) ctx.Data["HasPublicProfileReadme"] = profileReadmeBlob != nil // FIXME: only do database query, do not open it - _, _, profileReadmeBlob, profileClose = FindOwnerProfileReadme(ctx, ctx.Doer, RepoNameProfilePrivate) - defer profileClose() + _, profileReadmeBlob = FindOwnerProfileReadme(ctx, ctx.Doer, RepoNameProfilePrivate) ctx.Data["HasPrivateProfileReadme"] = profileReadmeBlob != nil return nil diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 7ea6c2b388..e11a0c86ea 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -74,8 +74,7 @@ func userProfile(ctx *context.Context) { ctx.Data["HeatmapTotalContributions"] = activities_model.GetTotalContributionsInHeatmap(data) } - profileDbRepo, _ /*profileGitRepo*/, profileReadmeBlob, profileClose := shared_user.FindOwnerProfileReadme(ctx, ctx.Doer) - defer profileClose() + profileDbRepo, profileReadmeBlob := shared_user.FindOwnerProfileReadme(ctx, ctx.Doer) showPrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == ctx.ContextUser.ID) prepareUserProfileTabData(ctx, showPrivate, profileDbRepo, profileReadmeBlob) diff --git a/services/context/api.go b/services/context/api.go index 7b604c5ea1..bda705cb48 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -274,7 +274,7 @@ func ReferencesGitRepo(allowEmpty ...bool) func(ctx *APIContext) { // For API calls. if ctx.Repo.GitRepo == nil { var err error - ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository) + 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 diff --git a/services/context/repo.go b/services/context/repo.go index b537a05036..2a473f4a54 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -622,7 +622,7 @@ func RepoAssignment(ctx *Context) { ctx.Repo.GitRepo = nil } - ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, repo) + 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) @@ -881,7 +881,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ) if ctx.Repo.GitRepo == nil { - ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, 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 diff --git a/templates/org/home.tmpl b/templates/org/home.tmpl index c71cbb6fa7..ce9426a7d0 100644 --- a/templates/org/home.tmpl +++ b/templates/org/home.tmpl @@ -8,7 +8,7 @@ {{if or .ProfileReadmeContent}} {{if and .ShowMemberAndTeamTab .HasPublicProfileReadme .HasPrivateProfileReadme}}
-