From cde818dbb7daae77114f5049b2d16e96cbc4a5d4 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 21 Apr 2022 00:53:14 +0200 Subject: [PATCH 1/8] check if GitRepo already open and close if --- modules/context/repo.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/context/repo.go b/modules/context/repo.go index a7c9a982c42b2..8455a98b56aee 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -602,6 +602,9 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { ctx.ServerError("RepoAssignment Invalid repo "+repo_model.RepoPath(userName, repoName), err) return } + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + } ctx.Repo.GitRepo = gitRepo // We opened it, we should close it From 58a16752f3391bc778b9fe31f634a411508bb66b Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 21 Apr 2022 00:53:28 +0200 Subject: [PATCH 2/8] Only run RepoAssignment once --- modules/context/repo.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/modules/context/repo.go b/modules/context/repo.go index 8455a98b56aee..96b310c986409 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -407,6 +407,13 @@ func RepoIDAssignment() func(ctx *Context) { // RepoAssignment returns a middleware to handle repository assignment func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { + repoAssignmentOnce, ok := ctx.Data["RepoAssignment"].(bool) + if ok && repoAssignmentOnce { + log.Trace("RepoAssignment was exec already skiping second call ...") + return + } + ctx.Data["RepoAssignment"] = true + var ( owner *user_model.User err error From e401065e54a47f8437e78e380bf6b3b52e6732fb Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 21 Apr 2022 02:18:56 +0200 Subject: [PATCH 3/8] Apply suggestions from code review Co-authored-by: delvh --- modules/context/repo.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/context/repo.go b/modules/context/repo.go index 96b310c986409..96efd71622a4c 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -407,9 +407,9 @@ func RepoIDAssignment() func(ctx *Context) { // RepoAssignment returns a middleware to handle repository assignment func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { - repoAssignmentOnce, ok := ctx.Data["RepoAssignment"].(bool) - if ok && repoAssignmentOnce { - log.Trace("RepoAssignment was exec already skiping second call ...") + repoAssignmentOnce, _ := ctx.Data["RepoAssignment"].(bool) + if repoAssignmentOnce { + log.Trace("RepoAssignment was exec already, skipping second call ...") return } ctx.Data["RepoAssignment"] = true From 2630bdec0273cfd3bbbc33dc35550db7065af5ee Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 21 Apr 2022 13:38:09 +0200 Subject: [PATCH 4/8] Update modules/context/repo.go --- modules/context/repo.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/modules/context/repo.go b/modules/context/repo.go index 96efd71622a4c..e7bd3ba81ff34 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -407,12 +407,11 @@ func RepoIDAssignment() func(ctx *Context) { // RepoAssignment returns a middleware to handle repository assignment func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { - repoAssignmentOnce, _ := ctx.Data["RepoAssignment"].(bool) - if repoAssignmentOnce { + if _, repoAssignmentOnce := ctx.Data["repoAssignmentExecuted"]; if repoAssignmentOnce { log.Trace("RepoAssignment was exec already, skipping second call ...") return } - ctx.Data["RepoAssignment"] = true + ctx.Data["repoAssignmentExecuted"] = true var ( owner *user_model.User From f5c5982fc6c57a0ed68d590a622c6478d8568296 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 21 Apr 2022 14:31:16 +0200 Subject: [PATCH 5/8] open GitRepo refactor --- modules/context/api.go | 9 ++------- modules/context/repo.go | 2 +- routers/api/v1/api.go | 36 ++++++++++++++++++------------------ routers/api/v1/repo/notes.go | 12 +++++------- 4 files changed, 26 insertions(+), 33 deletions(-) diff --git a/modules/context/api.go b/modules/context/api.go index e5c2eeda0a334..dfb0ae966289f 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -285,14 +285,9 @@ func APIContexter() func(http.Handler) http.Handler { } } -// ReferencesGitRepo injects the GitRepo into the Context -func ReferencesGitRepo(allowEmpty bool) func(ctx *APIContext) (cancel context.CancelFunc) { +// ReferencesGitRepoAllowEmpty injects the GitRepo into the Context even it it is marked as empty +func ReferencesGitRepoAllowEmpty() func(ctx *APIContext) (cancel context.CancelFunc) { return func(ctx *APIContext) (cancel context.CancelFunc) { - // Empty repository does not have reference information. - if !allowEmpty && ctx.Repo.Repository.IsEmpty { - return - } - // For API calls. if ctx.Repo.GitRepo == nil { repoPath := repo_model.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) diff --git a/modules/context/repo.go b/modules/context/repo.go index e7bd3ba81ff34..0cf4651994c98 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -407,7 +407,7 @@ func RepoIDAssignment() func(ctx *Context) { // RepoAssignment returns a middleware to handle repository assignment func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { - if _, repoAssignmentOnce := ctx.Data["repoAssignmentExecuted"]; if repoAssignmentOnce { + if _, repoAssignmentOnce := ctx.Data["repoAssignmentExecuted"]; repoAssignmentOnce { log.Trace("RepoAssignment was exec already, skipping second call ...") return } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index a430eb453aa9b..c4c5f1a5b18a0 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -788,7 +788,7 @@ func Routes() *web.Route { Patch(bind(api.EditGitHookOption{}), repo.EditGitHook). Delete(repo.DeleteGitHook) }) - }, reqToken(), reqAdmin(), reqGitHook(), context.ReferencesGitRepo(true)) + }, reqToken(), reqAdmin(), reqGitHook(), context.ReferencesGitRepoAllowEmpty()) m.Group("/hooks", func() { m.Combo("").Get(repo.ListHooks). Post(bind(api.CreateHookOption{}), repo.CreateHook) @@ -818,11 +818,11 @@ func Routes() *web.Route { m.Combo("/forks").Get(repo.ListForks). Post(reqToken(), reqRepoReader(unit.TypeCode), bind(api.CreateForkOption{}), repo.CreateFork) m.Group("/branches", func() { - m.Get("", context.ReferencesGitRepo(false), repo.ListBranches) - m.Get("/*", context.ReferencesGitRepo(false), repo.GetBranch) - m.Delete("/*", reqRepoWriter(unit.TypeCode), context.ReferencesGitRepo(false), repo.DeleteBranch) - m.Post("", reqRepoWriter(unit.TypeCode), context.ReferencesGitRepo(false), bind(api.CreateBranchRepoOption{}), repo.CreateBranch) - }, reqRepoReader(unit.TypeCode)) + m.Get("", repo.ListBranches) + m.Get("/*", repo.GetBranch) + m.Delete("/*", reqRepoWriter(unit.TypeCode), repo.DeleteBranch) + m.Post("", reqRepoWriter(unit.TypeCode), bind(api.CreateBranchRepoOption{}), repo.CreateBranch) + }, context.RepoRefForAPI, reqRepoReader(unit.TypeCode)) m.Group("/branch_protections", func() { m.Get("", repo.ListBranchProtections) m.Post("", bind(api.CreateBranchProtectionOption{}), repo.CreateBranchProtection) @@ -837,7 +837,7 @@ func Routes() *web.Route { m.Get("/*", repo.GetTag) m.Post("", reqRepoWriter(unit.TypeCode), bind(api.CreateTagOption{}), repo.CreateTag) m.Delete("/*", repo.DeleteTag) - }, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo(true)) + }, reqRepoReader(unit.TypeCode), context.ReferencesGitRepoAllowEmpty()) m.Group("/keys", func() { m.Combo("").Get(repo.ListDeployKeys). Post(bind(api.CreateKeyOption{}), repo.CreateDeployKey) @@ -941,10 +941,10 @@ func Routes() *web.Route { }) m.Group("/releases", func() { m.Combo("").Get(repo.ListReleases). - Post(reqToken(), reqRepoWriter(unit.TypeReleases), context.ReferencesGitRepo(false), bind(api.CreateReleaseOption{}), repo.CreateRelease) + Post(reqToken(), reqRepoWriter(unit.TypeReleases), context.RepoRefForAPI, bind(api.CreateReleaseOption{}), repo.CreateRelease) m.Group("/{id}", func() { m.Combo("").Get(repo.GetRelease). - Patch(reqToken(), reqRepoWriter(unit.TypeReleases), context.ReferencesGitRepo(false), bind(api.EditReleaseOption{}), repo.EditRelease). + Patch(reqToken(), reqRepoWriter(unit.TypeReleases), context.RepoRefForAPI, bind(api.EditReleaseOption{}), repo.EditRelease). Delete(reqToken(), reqRepoWriter(unit.TypeReleases), repo.DeleteRelease) m.Group("/assets", func() { m.Combo("").Get(repo.ListReleaseAttachments). @@ -992,30 +992,30 @@ func Routes() *web.Route { Delete(reqToken(), bind(api.PullReviewRequestOptions{}), repo.DeleteReviewRequests). Post(reqToken(), bind(api.PullReviewRequestOptions{}), repo.CreateReviewRequests) }) - }, mustAllowPulls, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo(false)) + }, mustAllowPulls, reqRepoReader(unit.TypeCode), context.RepoRefForAPI) m.Group("/statuses", func() { m.Combo("/{sha}").Get(repo.GetCommitStatuses). Post(reqToken(), bind(api.CreateStatusOption{}), repo.NewCommitStatus) }, reqRepoReader(unit.TypeCode)) m.Group("/commits", func() { - m.Get("", context.ReferencesGitRepo(false), repo.GetAllCommits) + m.Get("", context.RepoRefForAPI, repo.GetAllCommits) m.Group("/{ref}", func() { m.Get("/status", repo.GetCombinedCommitStatusByRef) m.Get("/statuses", repo.GetCommitStatusesByRef) - }) + }, context.RepoRefForAPI) }, reqRepoReader(unit.TypeCode)) m.Group("/git", func() { m.Group("/commits", func() { - m.Get("/{sha}", context.ReferencesGitRepo(false), repo.GetSingleCommit) + m.Get("/{sha}", repo.GetSingleCommit) m.Get("/{sha}.{diffType:diff|patch}", repo.DownloadCommitDiffOrPatch) }) m.Get("/refs", repo.GetGitAllRefs) m.Get("/refs/*", repo.GetGitRefs) - m.Get("/trees/{sha}", context.RepoRefForAPI, repo.GetTree) - m.Get("/blobs/{sha}", context.RepoRefForAPI, repo.GetBlob) - m.Get("/tags/{sha}", context.RepoRefForAPI, repo.GetAnnotatedTag) + m.Get("/trees/{sha}", repo.GetTree) + m.Get("/blobs/{sha}", repo.GetBlob) + m.Get("/tags/{sha}", repo.GetAnnotatedTag) m.Get("/notes/{sha}", repo.GetNote) - }, reqRepoReader(unit.TypeCode)) + }, context.RepoRefForAPI, reqRepoReader(unit.TypeCode)) m.Post("/diffpatch", reqRepoWriter(unit.TypeCode), reqToken(), bind(api.ApplyDiffPatchFileOptions{}), repo.ApplyDiffPatch) m.Group("/contents", func() { m.Get("", repo.GetContentsList) @@ -1035,7 +1035,7 @@ func Routes() *web.Route { Delete(reqToken(), repo.DeleteTopic) }, reqAdmin()) }, reqAnyRepoReader()) - m.Get("/issue_templates", context.ReferencesGitRepo(false), repo.GetIssueTemplates) + m.Get("/issue_templates", context.RepoRefForAPI, repo.GetIssueTemplates) m.Get("/languages", reqRepoReader(unit.TypeCode), repo.GetLanguages) }, repoAssignment()) }) diff --git a/routers/api/v1/repo/notes.go b/routers/api/v1/repo/notes.go index f85883566f012..bd8e27e40bf40 100644 --- a/routers/api/v1/repo/notes.go +++ b/routers/api/v1/repo/notes.go @@ -55,15 +55,13 @@ func GetNote(ctx *context.APIContext) { } func getNote(ctx *context.APIContext, identifier string) { - gitRepo, err := git.OpenRepository(ctx, ctx.Repo.Repository.RepoPath()) - if err != nil { - ctx.Error(http.StatusInternalServerError, "OpenRepository", err) + if ctx.Repo.GitRepo == nil { + ctx.InternalServerError(fmt.Errorf("no open git repo")) return } - defer gitRepo.Close() + var note git.Note - err = git.GetNote(ctx, gitRepo, identifier, ¬e) - if err != nil { + if err := git.GetNote(ctx, ctx.Repo.GitRepo, identifier, ¬e); err != nil { if git.IsErrNotExist(err) { ctx.NotFound(identifier) return @@ -72,7 +70,7 @@ func getNote(ctx *context.APIContext, identifier string) { return } - cmt, err := convert.ToCommit(ctx.Repo.Repository, gitRepo, note.Commit, nil) + cmt, err := convert.ToCommit(ctx.Repo.Repository, ctx.Repo.GitRepo, note.Commit, nil) if err != nil { ctx.Error(http.StatusInternalServerError, "ToCommit", err) return From e2ab82ad7ae5969f74f32e79d7a297187a7c5e5f Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 21 Apr 2022 16:18:38 +0200 Subject: [PATCH 6/8] more refactoring --- modules/context/api.go | 76 ++++++++++++----------- modules/context/repo.go | 16 +++-- routers/api/v1/api.go | 34 +++++----- routers/api/v1/repo/blob.go | 3 +- routers/api/v1/repo/commits.go | 1 + routers/api/v1/repo/file.go | 24 +++---- routers/api/v1/repo/hook.go | 5 ++ services/repository/files/content.go | 9 +-- services/repository/files/content_test.go | 9 ++- 9 files changed, 94 insertions(+), 83 deletions(-) diff --git a/modules/context/api.go b/modules/context/api.go index dfb0ae966289f..41d559f5b1c5f 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -285,31 +285,6 @@ func APIContexter() func(http.Handler) http.Handler { } } -// ReferencesGitRepoAllowEmpty injects the GitRepo into the Context even it it is marked as empty -func ReferencesGitRepoAllowEmpty() func(ctx *APIContext) (cancel context.CancelFunc) { - return func(ctx *APIContext) (cancel context.CancelFunc) { - // For API calls. - if ctx.Repo.GitRepo == nil { - repoPath := repo_model.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) - gitRepo, err := git.OpenRepository(ctx, repoPath) - if err != nil { - ctx.Error(http.StatusInternalServerError, "RepoRef Invalid repo "+repoPath, err) - return - } - 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 - } -} - // NotFound handles 404s for APIContext // String will replace message, errors will be added to a slice func (ctx *APIContext) NotFound(objs ...interface{}) { @@ -335,33 +310,62 @@ func (ctx *APIContext) NotFound(objs ...interface{}) { }) } -// RepoRefForAPI handles repository reference names when the ref name is not explicitly given -func RepoRefForAPI(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - ctx := GetAPIContext(req) +// 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) { // Empty repository does not have reference information. - if ctx.Repo.Repository.IsEmpty { + if ctx.Repo.Repository.IsEmpty && !(len(allowEmpty) != 0 && allowEmpty[0]) { return } - var err error - + // For API calls. if ctx.Repo.GitRepo == nil { repoPath := repo_model.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) - ctx.Repo.GitRepo, err = git.OpenRepository(ctx, repoPath) + gitRepo, err := git.OpenRepository(ctx, repoPath) if err != nil { - ctx.InternalServerError(err) + ctx.Error(http.StatusInternalServerError, "RepoRef Invalid repo "+repoPath, err) return } + ctx.Repo.GitRepo = gitRepo // We opened it, we should close it - defer func() { + 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 + } +} + +// RepoRefForAPI handles repository reference names when the ref name is not explicitly given +func RepoRefForAPI(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + ctx := GetAPIContext(req) + + if ctx.Repo.GitRepo == nil { + ctx.InternalServerError(fmt.Errorf("no open git repo")) + return + } + + if ref := ctx.FormTrim("ref"); len(ref) > 0 { + commit, err := ctx.Repo.GitRepo.GetCommit(ref) + if err != nil { + if git.IsErrNotExist(err) { + ctx.NotFound() + } else { + ctx.Error(http.StatusInternalServerError, "GetBlobByPath", err) + } + return + } + ctx.Repo.Commit = commit + return + } + + var err error refName := getRefName(ctx.Context, RepoRefAny) if ctx.Repo.GitRepo.IsBranchExist(refName) { diff --git a/modules/context/repo.go b/modules/context/repo.go index 0cf4651994c98..4687434455a39 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -221,13 +221,21 @@ func (r *Repository) FileExists(path, branch string) (bool, error) { // GetEditorconfig returns the .editorconfig definition if found in the // HEAD of the default repo branch. -func (r *Repository) GetEditorconfig() (*editorconfig.Editorconfig, error) { +func (r *Repository) GetEditorconfig(optCommit ...*git.Commit) (*editorconfig.Editorconfig, error) { if r.GitRepo == nil { return nil, nil } - commit, err := r.GitRepo.GetBranchCommit(r.Repository.DefaultBranch) - if err != nil { - return nil, err + var ( + err error + commit *git.Commit + ) + if len(optCommit) != 0 { + commit = optCommit[0] + } else { + commit, err = r.GitRepo.GetBranchCommit(r.Repository.DefaultBranch) + if err != nil { + return nil, err + } } treeEntry, err := commit.GetTreeEntryByPath(".editorconfig") if err != nil { diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index c4c5f1a5b18a0..9536155b30cf0 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -788,7 +788,7 @@ func Routes() *web.Route { Patch(bind(api.EditGitHookOption{}), repo.EditGitHook). Delete(repo.DeleteGitHook) }) - }, reqToken(), reqAdmin(), reqGitHook(), context.ReferencesGitRepoAllowEmpty()) + }, reqToken(), reqAdmin(), reqGitHook(), context.ReferencesGitRepo(true)) m.Group("/hooks", func() { m.Combo("").Get(repo.ListHooks). Post(bind(api.CreateHookOption{}), repo.CreateHook) @@ -796,7 +796,7 @@ func Routes() *web.Route { m.Combo("").Get(repo.GetHook). Patch(bind(api.EditHookOption{}), repo.EditHook). Delete(repo.DeleteHook) - m.Post("/tests", context.RepoRefForAPI, repo.TestHook) + m.Post("/tests", context.ReferencesGitRepo(), context.RepoRefForAPI, repo.TestHook) }) }, reqToken(), reqAdmin(), reqWebhooksEnabled()) m.Group("/collaborators", func() { @@ -813,16 +813,16 @@ func Routes() *web.Route { Put(reqAdmin(), repo.AddTeam). Delete(reqAdmin(), repo.DeleteTeam) }, reqToken()) - m.Get("/raw/*", context.RepoRefForAPI, reqRepoReader(unit.TypeCode), repo.GetRawFile) + m.Get("/raw/*", context.ReferencesGitRepo(), context.RepoRefForAPI, reqRepoReader(unit.TypeCode), repo.GetRawFile) m.Get("/archive/*", reqRepoReader(unit.TypeCode), repo.GetArchive) m.Combo("/forks").Get(repo.ListForks). Post(reqToken(), reqRepoReader(unit.TypeCode), bind(api.CreateForkOption{}), repo.CreateFork) m.Group("/branches", func() { - m.Get("", repo.ListBranches) - m.Get("/*", repo.GetBranch) - m.Delete("/*", reqRepoWriter(unit.TypeCode), repo.DeleteBranch) - m.Post("", reqRepoWriter(unit.TypeCode), bind(api.CreateBranchRepoOption{}), repo.CreateBranch) - }, context.RepoRefForAPI, reqRepoReader(unit.TypeCode)) + m.Get("", context.ReferencesGitRepo(), repo.ListBranches) + m.Get("/*", context.ReferencesGitRepo(), repo.GetBranch) + m.Delete("/*", reqRepoWriter(unit.TypeCode), context.ReferencesGitRepo(), repo.DeleteBranch) + m.Post("", reqRepoWriter(unit.TypeCode), context.ReferencesGitRepo(), bind(api.CreateBranchRepoOption{}), repo.CreateBranch) + }, reqRepoReader(unit.TypeCode)) m.Group("/branch_protections", func() { m.Get("", repo.ListBranchProtections) m.Post("", bind(api.CreateBranchProtectionOption{}), repo.CreateBranchProtection) @@ -837,7 +837,7 @@ func Routes() *web.Route { m.Get("/*", repo.GetTag) m.Post("", reqRepoWriter(unit.TypeCode), bind(api.CreateTagOption{}), repo.CreateTag) m.Delete("/*", repo.DeleteTag) - }, reqRepoReader(unit.TypeCode), context.ReferencesGitRepoAllowEmpty()) + }, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo(true)) m.Group("/keys", func() { m.Combo("").Get(repo.ListDeployKeys). Post(bind(api.CreateKeyOption{}), repo.CreateDeployKey) @@ -941,10 +941,10 @@ func Routes() *web.Route { }) m.Group("/releases", func() { m.Combo("").Get(repo.ListReleases). - Post(reqToken(), reqRepoWriter(unit.TypeReleases), context.RepoRefForAPI, bind(api.CreateReleaseOption{}), repo.CreateRelease) + Post(reqToken(), reqRepoWriter(unit.TypeReleases), context.ReferencesGitRepo(), bind(api.CreateReleaseOption{}), repo.CreateRelease) m.Group("/{id}", func() { m.Combo("").Get(repo.GetRelease). - Patch(reqToken(), reqRepoWriter(unit.TypeReleases), context.RepoRefForAPI, bind(api.EditReleaseOption{}), repo.EditRelease). + Patch(reqToken(), reqRepoWriter(unit.TypeReleases), context.ReferencesGitRepo(), bind(api.EditReleaseOption{}), repo.EditRelease). Delete(reqToken(), reqRepoWriter(unit.TypeReleases), repo.DeleteRelease) m.Group("/assets", func() { m.Combo("").Get(repo.ListReleaseAttachments). @@ -961,7 +961,7 @@ func Routes() *web.Route { }) }, reqRepoReader(unit.TypeReleases)) m.Post("/mirror-sync", reqToken(), reqRepoWriter(unit.TypeCode), repo.MirrorSync) - m.Get("/editorconfig/{filename}", context.RepoRefForAPI, reqRepoReader(unit.TypeCode), repo.GetEditorconfig) + m.Get("/editorconfig/{filename}", context.ReferencesGitRepo(), context.RepoRefForAPI, reqRepoReader(unit.TypeCode), repo.GetEditorconfig) m.Group("/pulls", func() { m.Combo("").Get(repo.ListPullRequests). Post(reqToken(), mustNotBeArchived, bind(api.CreatePullRequestOption{}), repo.CreatePullRequest) @@ -992,17 +992,17 @@ func Routes() *web.Route { Delete(reqToken(), bind(api.PullReviewRequestOptions{}), repo.DeleteReviewRequests). Post(reqToken(), bind(api.PullReviewRequestOptions{}), repo.CreateReviewRequests) }) - }, mustAllowPulls, reqRepoReader(unit.TypeCode), context.RepoRefForAPI) + }, mustAllowPulls, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo()) m.Group("/statuses", func() { m.Combo("/{sha}").Get(repo.GetCommitStatuses). Post(reqToken(), bind(api.CreateStatusOption{}), repo.NewCommitStatus) }, reqRepoReader(unit.TypeCode)) m.Group("/commits", func() { - m.Get("", context.RepoRefForAPI, repo.GetAllCommits) + m.Get("", context.ReferencesGitRepo(), repo.GetAllCommits) m.Group("/{ref}", func() { m.Get("/status", repo.GetCombinedCommitStatusByRef) m.Get("/statuses", repo.GetCommitStatusesByRef) - }, context.RepoRefForAPI) + }) }, reqRepoReader(unit.TypeCode)) m.Group("/git", func() { m.Group("/commits", func() { @@ -1015,7 +1015,7 @@ func Routes() *web.Route { m.Get("/blobs/{sha}", repo.GetBlob) m.Get("/tags/{sha}", repo.GetAnnotatedTag) m.Get("/notes/{sha}", repo.GetNote) - }, context.RepoRefForAPI, reqRepoReader(unit.TypeCode)) + }, context.ReferencesGitRepo(), reqRepoReader(unit.TypeCode)) m.Post("/diffpatch", reqRepoWriter(unit.TypeCode), reqToken(), bind(api.ApplyDiffPatchFileOptions{}), repo.ApplyDiffPatch) m.Group("/contents", func() { m.Get("", repo.GetContentsList) @@ -1035,7 +1035,7 @@ func Routes() *web.Route { Delete(reqToken(), repo.DeleteTopic) }, reqAdmin()) }, reqAnyRepoReader()) - m.Get("/issue_templates", context.RepoRefForAPI, repo.GetIssueTemplates) + m.Get("/issue_templates", context.ReferencesGitRepo(), repo.GetIssueTemplates) m.Get("/languages", reqRepoReader(unit.TypeCode), repo.GetLanguages) }, repoAssignment()) }) diff --git a/routers/api/v1/repo/blob.go b/routers/api/v1/repo/blob.go index 19d893a68b92e..035f2dc1e1de9 100644 --- a/routers/api/v1/repo/blob.go +++ b/routers/api/v1/repo/blob.go @@ -45,7 +45,8 @@ func GetBlob(ctx *context.APIContext) { ctx.Error(http.StatusBadRequest, "", "sha not provided") return } - if blob, err := files_service.GetBlobBySHA(ctx, ctx.Repo.Repository, sha); err != nil { + + if blob, err := files_service.GetBlobBySHA(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, sha); err != nil { ctx.Error(http.StatusBadRequest, "", err) } else { ctx.JSON(http.StatusOK, blob) diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index b6c47e0685186..c79c34ec42961 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -269,6 +269,7 @@ func DownloadCommitDiffOrPatch(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" repoPath := repo_model.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name) + // TODO: use gitRepo from context if err := git.GetRawDiff( ctx, repoPath, diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index a4811b370a911..ed51f6f4df2a2 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -62,22 +62,7 @@ func GetRawFile(ctx *context.APIContext) { return } - commit := ctx.Repo.Commit - - if ref := ctx.FormTrim("ref"); len(ref) > 0 { - var err error - commit, err = ctx.Repo.GitRepo.GetCommit(ref) - if err != nil { - if git.IsErrNotExist(err) { - ctx.NotFound() - } else { - ctx.Error(http.StatusInternalServerError, "GetBlobByPath", err) - } - return - } - } - - blob, err := commit.GetBlobByPath(ctx.Repo.TreePath) + blob, err := ctx.Repo.Commit.GetBlobByPath(ctx.Repo.TreePath) if err != nil { if git.IsErrNotExist(err) { ctx.NotFound() @@ -157,13 +142,18 @@ func GetEditorconfig(ctx *context.APIContext) { // description: filepath of file to get // type: string // required: true + // - name: ref + // in: query + // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // type: string + // required: false // responses: // 200: // description: success // "404": // "$ref": "#/responses/notFound" - ec, err := ctx.Repo.GetEditorconfig() + ec, err := ctx.Repo.GetEditorconfig(ctx.Repo.Commit) if err != nil { if git.IsErrNotExist(err) { ctx.NotFound(err) diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go index c79a1d6b13522..7ec6cd88aba63 100644 --- a/routers/api/v1/repo/hook.go +++ b/routers/api/v1/repo/hook.go @@ -138,6 +138,11 @@ func TestHook(ctx *context.APIContext) { // type: integer // format: int64 // required: true + // - name: ref + // in: query + // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // type: string + // required: false // responses: // "204": // "$ref": "#/responses/empty" diff --git a/services/repository/files/content.go b/services/repository/files/content.go index 9037a84349e5a..2237671a60cbb 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -164,7 +164,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref // Now populate the rest of the ContentsResponse based on entry type if entry.IsRegular() || entry.IsExecutable() { contentsResponse.Type = string(ContentTypeRegular) - if blobResponse, err := GetBlobBySHA(ctx, repo, entry.ID.String()); err != nil { + if blobResponse, err := GetBlobBySHA(ctx, repo, gitRepo, entry.ID.String()); err != nil { return nil, err } else if !forList { // We don't show the content if we are getting a list of FileContentResponses @@ -220,12 +220,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref } // GetBlobBySHA get the GitBlobResponse of a repository using a sha hash. -func GetBlobBySHA(ctx context.Context, repo *repo_model.Repository, sha string) (*api.GitBlobResponse, error) { - gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, repo.RepoPath()) - if err != nil { - return nil, err - } - defer closer.Close() +func GetBlobBySHA(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, sha string) (*api.GitBlobResponse, error) { gitBlob, err := gitRepo.GetBlob(sha) if err != nil { return nil, err diff --git a/services/repository/files/content_test.go b/services/repository/files/content_test.go index 8a3e589bdf9b1..342ebae329168 100644 --- a/services/repository/files/content_test.go +++ b/services/repository/files/content_test.go @@ -8,7 +8,9 @@ import ( "path/filepath" "testing" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/git" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" @@ -234,7 +236,12 @@ func TestGetBlobBySHA(t *testing.T) { ctx.SetParams(":id", "1") ctx.SetParams(":sha", sha) - gbr, err := GetBlobBySHA(ctx, ctx.Repo.Repository, ctx.Params(":sha")) + gitRepo, err := git.OpenRepository(ctx, repo_model.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)) + if err != nil { + t.Fail() + } + + gbr, err := GetBlobBySHA(ctx, ctx.Repo.Repository, gitRepo, ctx.Params(":sha")) expectedGBR := &api.GitBlobResponse{ Content: "dHJlZSAyYTJmMWQ0NjcwNzI4YTJlMTAwNDllMzQ1YmQ3YTI3NjQ2OGJlYWI2CmF1dGhvciB1c2VyMSA8YWRkcmVzczFAZXhhbXBsZS5jb20+IDE0ODk5NTY0NzkgLTA0MDAKY29tbWl0dGVyIEV0aGFuIEtvZW5pZyA8ZXRoYW50a29lbmlnQGdtYWlsLmNvbT4gMTQ4OTk1NjQ3OSAtMDQwMAoKSW5pdGlhbCBjb21taXQK", Encoding: "base64", From 7a2f0903cd53382b39f016fbb182f739908b5a61 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 21 Apr 2022 16:27:09 +0200 Subject: [PATCH 7/8] next --- routers/api/v1/api.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 9536155b30cf0..aec2a6d7b2c42 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -818,11 +818,11 @@ func Routes() *web.Route { m.Combo("/forks").Get(repo.ListForks). Post(reqToken(), reqRepoReader(unit.TypeCode), bind(api.CreateForkOption{}), repo.CreateFork) m.Group("/branches", func() { - m.Get("", context.ReferencesGitRepo(), repo.ListBranches) - m.Get("/*", context.ReferencesGitRepo(), repo.GetBranch) - m.Delete("/*", reqRepoWriter(unit.TypeCode), context.ReferencesGitRepo(), repo.DeleteBranch) - m.Post("", reqRepoWriter(unit.TypeCode), context.ReferencesGitRepo(), bind(api.CreateBranchRepoOption{}), repo.CreateBranch) - }, reqRepoReader(unit.TypeCode)) + m.Get("", repo.ListBranches) + m.Get("/*", repo.GetBranch) + m.Delete("/*", reqRepoWriter(unit.TypeCode), repo.DeleteBranch) + m.Post("", reqRepoWriter(unit.TypeCode), bind(api.CreateBranchRepoOption{}), repo.CreateBranch) + }, context.ReferencesGitRepo(), reqRepoReader(unit.TypeCode)) m.Group("/branch_protections", func() { m.Get("", repo.ListBranchProtections) m.Post("", bind(api.CreateBranchProtectionOption{}), repo.CreateBranchProtection) From 0a9e7949d9d3c43e83d4e020ef5a9d9a258be1b2 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 21 Apr 2022 16:27:58 +0200 Subject: [PATCH 8/8] update api docs --- templates/swagger/v1_json.tmpl | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index de74fd8fa399e..0374a53a6555c 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -3668,6 +3668,12 @@ "name": "filepath", "in": "path", "required": true + }, + { + "type": "string", + "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "name": "ref", + "in": "query" } ], "responses": { @@ -4559,6 +4565,12 @@ "name": "id", "in": "path", "required": true + }, + { + "type": "string", + "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "name": "ref", + "in": "query" } ], "responses": {