Skip to content

Commit 4e278b5

Browse files
65436543
committed
Let Branch and Raw Endpoint return json error if not found
1 parent 3fd060e commit 4e278b5

File tree

5 files changed

+55
-28
lines changed

5 files changed

+55
-28
lines changed

‎modules/context/repo.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,6 @@ func RepoRefByType(refType RepoRefType) macaron.Handler {
716716
err error
717717
)
718718

719-
// For API calls.
720719
if ctx.Repo.GitRepo == nil {
721720
repoPath := models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
722721
ctx.Repo.GitRepo, err = git.OpenRepository(repoPath)
@@ -785,7 +784,7 @@ func RepoRefByType(refType RepoRefType) macaron.Handler {
785784

786785
ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetCommit(refName)
787786
if err != nil {
788-
ctx.NotFound("GetCommit", nil)
787+
ctx.NotFound("GetCommit", err)
789788
return
790789
}
791790
} else {

‎routers/api/v1/api.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -670,14 +670,14 @@ func RegisterRoutes(m *macaron.Macaron) {
670670
Put(reqAdmin(), bind(api.AddCollaboratorOption{}), repo.AddCollaborator).
671671
Delete(reqAdmin(), repo.DeleteCollaborator)
672672
}, reqToken())
673-
m.Get("/raw/*", context.RepoRefByType(context.RepoRefAny), reqRepoReader(models.UnitTypeCode), repo.GetRawFile)
673+
m.Get("/raw/*", reqRepoReader(models.UnitTypeCode), repo.GetRawFile)
674674
m.Get("/archive/*", reqRepoReader(models.UnitTypeCode), repo.GetArchive)
675675
m.Combo("/forks").Get(repo.ListForks).
676676
Post(reqToken(), reqRepoReader(models.UnitTypeCode), bind(api.CreateForkOption{}), repo.CreateFork)
677677
m.Group("/branches", func() {
678678
m.Get("", repo.ListBranches)
679-
m.Get("/*", context.RepoRefByType(context.RepoRefBranch), repo.GetBranch)
680-
m.Delete("/*", reqRepoWriter(models.UnitTypeCode), context.RepoRefByType(context.RepoRefBranch), repo.DeleteBranch)
679+
m.Get("/*", repo.GetBranch)
680+
m.Delete("/*", reqRepoWriter(models.UnitTypeCode), repo.DeleteBranch)
681681
m.Post("", reqRepoWriter(models.UnitTypeCode), bind(api.CreateBranchRepoOption{}), repo.CreateBranch)
682682
}, reqRepoReader(models.UnitTypeCode))
683683
m.Group("/branch_protections", func() {

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

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

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

73-
branchProtection, err := ctx.Repo.Repository.GetBranchProtection(ctx.Repo.BranchName)
70+
branchProtection, err := ctx.Repo.Repository.GetBranchProtection(branchName)
7471
if err != nil {
7572
ctx.Error(http.StatusInternalServerError, "GetBranchProtection", err)
7673
return
@@ -113,21 +110,17 @@ func DeleteBranch(ctx *context.APIContext) {
113110
// "$ref": "#/responses/empty"
114111
// "403":
115112
// "$ref": "#/responses/error"
113+
// "404":
114+
// "$ref": "#/responses/notFound"
116115

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

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

130-
isProtected, err := ctx.Repo.Repository.IsProtectedBranch(ctx.Repo.BranchName, ctx.User)
123+
isProtected, err := ctx.Repo.Repository.IsProtectedBranch(branchName, ctx.User)
131124
if err != nil {
132125
ctx.InternalServerError(err)
133126
return
@@ -137,7 +130,7 @@ func DeleteBranch(ctx *context.APIContext) {
137130
return
138131
}
139132

140-
branch, err := repo_module.GetBranch(ctx.Repo.Repository, ctx.Repo.BranchName)
133+
branch, err := repo_module.GetBranch(ctx.Repo.Repository, branchName)
141134
if err != nil {
142135
if git.IsErrBranchNotExist(err) {
143136
ctx.NotFound(err)
@@ -153,7 +146,17 @@ func DeleteBranch(ctx *context.APIContext) {
153146
return
154147
}
155148

156-
if err := ctx.Repo.GitRepo.DeleteBranch(ctx.Repo.BranchName, git.DeleteBranchOptions{
149+
if ctx.Repo.GitRepo == nil {
150+
repoPath := models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
151+
ctx.Repo.GitRepo, err = git.OpenRepository(repoPath)
152+
if err != nil {
153+
ctx.InternalServerError(err)
154+
return
155+
}
156+
defer ctx.Repo.GitRepo.Close()
157+
}
158+
159+
if err := ctx.Repo.GitRepo.DeleteBranch(branchName, git.DeleteBranchOptions{
157160
Force: true,
158161
}); err != nil {
159162
ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
@@ -163,7 +166,7 @@ func DeleteBranch(ctx *context.APIContext) {
163166
// Don't return error below this
164167
if err := repo_service.PushUpdate(
165168
&repo_module.PushUpdateOptions{
166-
RefFullName: git.BranchPrefix + ctx.Repo.BranchName,
169+
RefFullName: git.BranchPrefix + branchName,
167170
OldCommitID: c.ID.String(),
168171
NewCommitID: git.EmptySHA,
169172
PusherID: ctx.User.ID,
@@ -174,7 +177,7 @@ func DeleteBranch(ctx *context.APIContext) {
174177
log.Error("Update: %v", err)
175178
}
176179

177-
if err := ctx.Repo.Repository.AddDeletedBranch(ctx.Repo.BranchName, c.ID.String(), ctx.User.ID); err != nil {
180+
if err := ctx.Repo.Repository.AddDeletedBranch(branchName, c.ID.String(), ctx.User.ID); err != nil {
178181
log.Warn("AddDeletedBranch: %v", err)
179182
}
180183

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,26 @@ func GetRawFile(ctx *context.APIContext) {
5353
return
5454
}
5555

56-
blob, err := ctx.Repo.Commit.GetBlobByPath(ctx.Repo.TreePath)
56+
filepath := ctx.Params("*")
57+
var err error
58+
59+
if ctx.Repo.GitRepo == nil {
60+
repoPath := models.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
61+
ctx.Repo.GitRepo, err = git.OpenRepository(repoPath)
62+
if err != nil {
63+
ctx.InternalServerError(err)
64+
return
65+
}
66+
defer ctx.Repo.GitRepo.Close()
67+
}
68+
69+
ctx.Repo.Commit, err = ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch)
70+
if err != nil {
71+
ctx.InternalServerError(err)
72+
return
73+
}
74+
75+
blob, err := ctx.Repo.Commit.GetBlobByPath(filepath)
5776
if err != nil {
5877
if git.IsErrNotExist(err) {
5978
ctx.NotFound()

‎templates/swagger/v1_json.tmpl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2541,6 +2541,9 @@
25412541
"responses": {
25422542
"200": {
25432543
"$ref": "#/responses/Branch"
2544+
},
2545+
"404": {
2546+
"$ref": "#/responses/notFound"
25442547
}
25452548
}
25462549
},
@@ -2582,6 +2585,9 @@
25822585
},
25832586
"403": {
25842587
"$ref": "#/responses/error"
2588+
},
2589+
"404": {
2590+
"$ref": "#/responses/notFound"
25852591
}
25862592
}
25872593
}

0 commit comments

Comments
 (0)