Skip to content

Commit dc9f5a7

Browse files
6543zeripath
6543
and
zeripath
authored
[API] Only Return Json (#13511) (#13564)
Backport #13511 Co-authored-by: zeripath <[email protected]>
1 parent da0460d commit dc9f5a7

File tree

5 files changed

+103
-48
lines changed

5 files changed

+103
-48
lines changed

‎modules/context/api.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,3 +251,61 @@ func (ctx *APIContext) NotFound(objs ...interface{}) {
251251
"errors": errors,
252252
})
253253
}
254+
255+
// RepoRefForAPI handles repository reference names when the ref name is not explicitly given
256+
func RepoRefForAPI() macaron.Handler {
257+
return func(ctx *APIContext) {
258+
// Empty repository does not have reference information.
259+
if ctx.Repo.Repository.IsEmpty {
260+
return
261+
}
262+
263+
var err error
264+
265+
if ctx.Repo.GitRepo == nil {
266+
repoPath := models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
267+
ctx.Repo.GitRepo, err = git.OpenRepository(repoPath)
268+
if err != nil {
269+
ctx.InternalServerError(err)
270+
return
271+
}
272+
// We opened it, we should close it
273+
defer func() {
274+
// If it's been set to nil then assume someone else has closed it.
275+
if ctx.Repo.GitRepo != nil {
276+
ctx.Repo.GitRepo.Close()
277+
}
278+
}()
279+
}
280+
281+
refName := getRefName(ctx.Context, RepoRefAny)
282+
283+
if ctx.Repo.GitRepo.IsBranchExist(refName) {
284+
ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(refName)
285+
if err != nil {
286+
ctx.InternalServerError(err)
287+
return
288+
}
289+
ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
290+
} else if ctx.Repo.GitRepo.IsTagExist(refName) {
291+
ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetTagCommit(refName)
292+
if err != nil {
293+
ctx.InternalServerError(err)
294+
return
295+
}
296+
ctx.Repo.CommitID = ctx.Repo.Commit.ID.String()
297+
} else if len(refName) == 40 {
298+
ctx.Repo.CommitID = refName
299+
ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName)
300+
if err != nil {
301+
ctx.NotFound("GetCommit", err)
302+
return
303+
}
304+
} else {
305+
ctx.NotFound(fmt.Errorf("not exist: '%s'", ctx.Params("*")))
306+
return
307+
}
308+
309+
ctx.Next()
310+
}
311+
}

‎modules/context/repo.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,6 @@ func RepoRefByType(refType RepoRefType) macaron.Handler {
690690
err error
691691
)
692692

693-
// For API calls.
694693
if ctx.Repo.GitRepo == nil {
695694
repoPath := models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
696695
ctx.Repo.GitRepo, err = git.OpenRepository(repoPath)
@@ -759,7 +758,7 @@ func RepoRefByType(refType RepoRefType) macaron.Handler {
759758

760759
ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName)
761760
if err != nil {
762-
ctx.NotFound("GetCommit", nil)
761+
ctx.NotFound("GetCommit", err)
763762
return
764763
}
765764
} else {

‎routers/api/v1/api.go

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -184,14 +184,14 @@ func reqToken() macaron.Handler {
184184
ctx.RequireCSRF()
185185
return
186186
}
187-
ctx.Context.Error(http.StatusUnauthorized)
187+
ctx.Error(http.StatusUnauthorized, "reqToken", "token is required")
188188
}
189189
}
190190

191191
func reqBasicAuth() macaron.Handler {
192192
return func(ctx *context.APIContext) {
193193
if !ctx.Context.IsBasicAuth {
194-
ctx.Context.Error(http.StatusUnauthorized)
194+
ctx.Error(http.StatusUnauthorized, "reqBasicAuth", "basic auth required")
195195
return
196196
}
197197
ctx.CheckForOTP()
@@ -200,59 +200,59 @@ func reqBasicAuth() macaron.Handler {
200200

201201
// reqSiteAdmin user should be the site admin
202202
func reqSiteAdmin() macaron.Handler {
203-
return func(ctx *context.Context) {
203+
return func(ctx *context.APIContext) {
204204
if !ctx.IsUserSiteAdmin() {
205-
ctx.Error(http.StatusForbidden)
205+
ctx.Error(http.StatusForbidden, "reqSiteAdmin", "user should be the site admin")
206206
return
207207
}
208208
}
209209
}
210210

211211
// reqOwner user should be the owner of the repo or site admin.
212212
func reqOwner() macaron.Handler {
213-
return func(ctx *context.Context) {
213+
return func(ctx *context.APIContext) {
214214
if !ctx.IsUserRepoOwner() && !ctx.IsUserSiteAdmin() {
215-
ctx.Error(http.StatusForbidden)
215+
ctx.Error(http.StatusForbidden, "reqOwner", "user should be the owner of the repo")
216216
return
217217
}
218218
}
219219
}
220220

221221
// reqAdmin user should be an owner or a collaborator with admin write of a repository, or site admin
222222
func reqAdmin() macaron.Handler {
223-
return func(ctx *context.Context) {
223+
return func(ctx *context.APIContext) {
224224
if !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() {
225-
ctx.Error(http.StatusForbidden)
225+
ctx.Error(http.StatusForbidden, "reqAdmin", "user should be an owner or a collaborator with admin write of a repository")
226226
return
227227
}
228228
}
229229
}
230230

231231
// reqRepoWriter user should have a permission to write to a repo, or be a site admin
232232
func reqRepoWriter(unitTypes ...models.UnitType) macaron.Handler {
233-
return func(ctx *context.Context) {
233+
return func(ctx *context.APIContext) {
234234
if !ctx.IsUserRepoWriter(unitTypes) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() {
235-
ctx.Error(http.StatusForbidden)
235+
ctx.Error(http.StatusForbidden, "reqRepoWriter", "user should have a permission to write to a repo")
236236
return
237237
}
238238
}
239239
}
240240

241241
// reqRepoReader user should have specific read permission or be a repo admin or a site admin
242242
func reqRepoReader(unitType models.UnitType) macaron.Handler {
243-
return func(ctx *context.Context) {
243+
return func(ctx *context.APIContext) {
244244
if !ctx.IsUserRepoReaderSpecific(unitType) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() {
245-
ctx.Error(http.StatusForbidden)
245+
ctx.Error(http.StatusForbidden, "reqRepoReader", "user should have specific read permission or be a repo admin or a site admin")
246246
return
247247
}
248248
}
249249
}
250250

251251
// reqAnyRepoReader user should have any permission to read repository or permissions of site admin
252252
func reqAnyRepoReader() macaron.Handler {
253-
return func(ctx *context.Context) {
253+
return func(ctx *context.APIContext) {
254254
if !ctx.IsUserRepoReaderAny() && !ctx.IsUserSiteAdmin() {
255-
ctx.Error(http.StatusForbidden)
255+
ctx.Error(http.StatusForbidden, "reqAnyRepoReader", "user should have any permission to read repository or permissions of site admin")
256256
return
257257
}
258258
}
@@ -495,7 +495,6 @@ func mustNotBeArchived(ctx *context.APIContext) {
495495
}
496496

497497
// RegisterRoutes registers all v1 APIs routes to web application.
498-
// FIXME: custom form error response
499498
func RegisterRoutes(m *macaron.Macaron) {
500499
bind := binding.Bind
501500

@@ -628,7 +627,7 @@ func RegisterRoutes(m *macaron.Macaron) {
628627
m.Group("/:username/:reponame", func() {
629628
m.Combo("").Get(reqAnyRepoReader(), repo.Get).
630629
Delete(reqToken(), reqOwner(), repo.Delete).
631-
Patch(reqToken(), reqAdmin(), bind(api.EditRepoOption{}), context.RepoRef(), repo.Edit)
630+
Patch(reqToken(), reqAdmin(), bind(api.EditRepoOption{}), context.RepoRefForAPI(), repo.Edit)
632631
m.Post("/transfer", reqOwner(), bind(api.TransferRepoOption{}), repo.Transfer)
633632
m.Combo("/notifications").
634633
Get(reqToken(), notify.ListRepoNotifications).
@@ -640,7 +639,7 @@ func RegisterRoutes(m *macaron.Macaron) {
640639
m.Combo("").Get(repo.GetHook).
641640
Patch(bind(api.EditHookOption{}), repo.EditHook).
642641
Delete(repo.DeleteHook)
643-
m.Post("/tests", context.RepoRef(), repo.TestHook)
642+
m.Post("/tests", context.RepoRefForAPI(), repo.TestHook)
644643
})
645644
m.Group("/git", func() {
646645
m.Combo("").Get(repo.ListGitHooks)
@@ -657,14 +656,14 @@ func RegisterRoutes(m *macaron.Macaron) {
657656
Put(reqAdmin(), bind(api.AddCollaboratorOption{}), repo.AddCollaborator).
658657
Delete(reqAdmin(), repo.DeleteCollaborator)
659658
}, reqToken())
660-
m.Get("/raw/*", context.RepoRefByType(context.RepoRefAny), reqRepoReader(models.UnitTypeCode), repo.GetRawFile)
659+
m.Get("/raw/*", context.RepoRefForAPI(), reqRepoReader(models.UnitTypeCode), repo.GetRawFile)
661660
m.Get("/archive/*", reqRepoReader(models.UnitTypeCode), repo.GetArchive)
662661
m.Combo("/forks").Get(repo.ListForks).
663662
Post(reqToken(), reqRepoReader(models.UnitTypeCode), bind(api.CreateForkOption{}), repo.CreateFork)
664663
m.Group("/branches", func() {
665664
m.Get("", repo.ListBranches)
666-
m.Get("/*", context.RepoRefByType(context.RepoRefBranch), repo.GetBranch)
667-
m.Delete("/*", reqRepoWriter(models.UnitTypeCode), context.RepoRefByType(context.RepoRefBranch), repo.DeleteBranch)
665+
m.Get("/*", repo.GetBranch)
666+
m.Delete("/*", context.ReferencesGitRepo(false), reqRepoWriter(models.UnitTypeCode), repo.DeleteBranch)
668667
}, reqRepoReader(models.UnitTypeCode))
669668
m.Group("/branch_protections", func() {
670669
m.Get("", repo.ListBranchProtections)
@@ -785,7 +784,7 @@ func RegisterRoutes(m *macaron.Macaron) {
785784
})
786785
}, reqRepoReader(models.UnitTypeReleases))
787786
m.Post("/mirror-sync", reqToken(), reqRepoWriter(models.UnitTypeCode), repo.MirrorSync)
788-
m.Get("/editorconfig/:filename", context.RepoRef(), reqRepoReader(models.UnitTypeCode), repo.GetEditorconfig)
787+
m.Get("/editorconfig/:filename", context.RepoRefForAPI(), reqRepoReader(models.UnitTypeCode), repo.GetEditorconfig)
789788
m.Group("/pulls", func() {
790789
m.Combo("").Get(bind(api.ListPullRequestsOptions{}), repo.ListPullRequests).
791790
Post(reqToken(), mustNotBeArchived, bind(api.CreatePullRequestOption{}), repo.CreatePullRequest)
@@ -827,9 +826,9 @@ func RegisterRoutes(m *macaron.Macaron) {
827826
})
828827
m.Get("/refs", repo.GetGitAllRefs)
829828
m.Get("/refs/*", repo.GetGitRefs)
830-
m.Get("/trees/:sha", context.RepoRef(), repo.GetTree)
831-
m.Get("/blobs/:sha", context.RepoRef(), repo.GetBlob)
832-
m.Get("/tags/:sha", context.RepoRef(), repo.GetTag)
829+
m.Get("/trees/:sha", context.RepoRefForAPI(), repo.GetTree)
830+
m.Get("/blobs/:sha", context.RepoRefForAPI(), repo.GetBlob)
831+
m.Get("/tags/:sha", context.RepoRefForAPI(), repo.GetTag)
833832
}, reqRepoReader(models.UnitTypeCode))
834833
m.Group("/contents", func() {
835834
m.Get("", repo.GetContentsList)

‎routers/api/v1/repo/branch.go

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,12 @@ func GetBranch(ctx *context.APIContext) {
4545
// responses:
4646
// "200":
4747
// "$ref": "#/responses/Branch"
48+
// "404":
49+
// "$ref": "#/responses/notFound"
4850

49-
if ctx.Repo.TreePath != "" {
50-
// if TreePath != "", then URL contained extra slashes
51-
// (i.e. "master/subbranch" instead of "master"), so branch does
52-
// not exist
53-
ctx.NotFound()
54-
return
55-
}
56-
branch, err := repo_module.GetBranch(ctx.Repo.Repository, ctx.Repo.BranchName)
51+
branchName := ctx.Params("*")
52+
53+
branch, err := repo_module.GetBranch(ctx.Repo.Repository, branchName)
5754
if err != nil {
5855
if git.IsErrBranchNotExist(err) {
5956
ctx.NotFound(err)
@@ -69,7 +66,7 @@ func GetBranch(ctx *context.APIContext) {
6966
return
7067
}
7168

72-
branchProtection, err := ctx.Repo.Repository.GetBranchProtection(ctx.Repo.BranchName)
69+
branchProtection, err := ctx.Repo.Repository.GetBranchProtection(branchName)
7370
if err != nil {
7471
ctx.Error(http.StatusInternalServerError, "GetBranchProtection", err)
7572
return
@@ -112,21 +109,17 @@ func DeleteBranch(ctx *context.APIContext) {
112109
// "$ref": "#/responses/empty"
113110
// "403":
114111
// "$ref": "#/responses/error"
112+
// "404":
113+
// "$ref": "#/responses/notFound"
115114

116-
if ctx.Repo.TreePath != "" {
117-
// if TreePath != "", then URL contained extra slashes
118-
// (i.e. "master/subbranch" instead of "master"), so branch does
119-
// not exist
120-
ctx.NotFound()
121-
return
122-
}
115+
branchName := ctx.Params("*")
123116

124-
if ctx.Repo.Repository.DefaultBranch == ctx.Repo.BranchName {
117+
if ctx.Repo.Repository.DefaultBranch == branchName {
125118
ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
126119
return
127120
}
128121

129-
isProtected, err := ctx.Repo.Repository.IsProtectedBranch(ctx.Repo.BranchName, ctx.User)
122+
isProtected, err := ctx.Repo.Repository.IsProtectedBranch(branchName, ctx.User)
130123
if err != nil {
131124
ctx.InternalServerError(err)
132125
return
@@ -136,7 +129,7 @@ func DeleteBranch(ctx *context.APIContext) {
136129
return
137130
}
138131

139-
branch, err := repo_module.GetBranch(ctx.Repo.Repository, ctx.Repo.BranchName)
132+
branch, err := repo_module.GetBranch(ctx.Repo.Repository, branchName)
140133
if err != nil {
141134
if git.IsErrBranchNotExist(err) {
142135
ctx.NotFound(err)
@@ -152,7 +145,7 @@ func DeleteBranch(ctx *context.APIContext) {
152145
return
153146
}
154147

155-
if err := ctx.Repo.GitRepo.DeleteBranch(ctx.Repo.BranchName, git.DeleteBranchOptions{
148+
if err := ctx.Repo.GitRepo.DeleteBranch(branchName, git.DeleteBranchOptions{
156149
Force: true,
157150
}); err != nil {
158151
ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
@@ -164,7 +157,7 @@ func DeleteBranch(ctx *context.APIContext) {
164157
ctx.Repo.Repository,
165158
ctx.Repo.BranchName,
166159
repofiles.PushUpdateOptions{
167-
RefFullName: git.BranchPrefix + ctx.Repo.BranchName,
160+
RefFullName: git.BranchPrefix + branchName,
168161
OldCommitID: c.ID.String(),
169162
NewCommitID: git.EmptySHA,
170163
PusherID: ctx.User.ID,
@@ -175,7 +168,7 @@ func DeleteBranch(ctx *context.APIContext) {
175168
log.Error("Update: %v", err)
176169
}
177170

178-
if err := ctx.Repo.Repository.AddDeletedBranch(ctx.Repo.BranchName, c.ID.String(), ctx.User.ID); err != nil {
171+
if err := ctx.Repo.Repository.AddDeletedBranch(branchName, c.ID.String(), ctx.User.ID); err != nil {
179172
log.Warn("AddDeletedBranch: %v", err)
180173
}
181174

‎templates/swagger/v1_json.tmpl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2318,6 +2318,9 @@
23182318
"responses": {
23192319
"200": {
23202320
"$ref": "#/responses/Branch"
2321+
},
2322+
"404": {
2323+
"$ref": "#/responses/notFound"
23212324
}
23222325
}
23232326
},
@@ -2359,6 +2362,9 @@
23592362
},
23602363
"403": {
23612364
"$ref": "#/responses/error"
2365+
},
2366+
"404": {
2367+
"$ref": "#/responses/notFound"
23622368
}
23632369
}
23642370
}

0 commit comments

Comments
 (0)