RepoAssignment ensure to close before overwrite (#19449)

* check if GitRepo already open and close if

* only run RepoAssignment once

* refactor context helper for api to open GitRepo

6 files changed, 38 insertions, 43 deletions

diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
index a430eb453a..aec2a6d7b2 100644
--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@@ -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("", 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.ReferencesGitRepo(), reqRepoReader(unit.TypeCode))
 				m.Group("/branch_protections", func() {
 					m.Get("", repo.ListBranchProtections)
 					m.Post("", bind(api.CreateBranchProtectionOption{}), repo.CreateBranchProtection)
@@ -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.ReferencesGitRepo(), 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.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,13 +992,13 @@ 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.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.ReferencesGitRepo(false), repo.GetAllCommits)
+					m.Get("", context.ReferencesGitRepo(), repo.GetAllCommits)
 					m.Group("/{ref}", func() {
 						m.Get("/status", repo.GetCombinedCommitStatusByRef)
 						m.Get("/statuses", repo.GetCommitStatusesByRef)
@@ -1006,16 +1006,16 @@ func Routes() *web.Route {
 				}, 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.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.ReferencesGitRepo(false), 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 19d893a68b..035f2dc1e1 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 b6c47e0685..c79c34ec42 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 a4811b370a..ed51f6f4df 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 c79a1d6b13..7ec6cd88ab 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/routers/api/v1/repo/notes.go b/routers/api/v1/repo/notes.go
index f85883566f..bd8e27e40b 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, &note)
-	if err != nil {
+	if err := git.GetNote(ctx, ctx.Repo.GitRepo, identifier, &note); 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