Docker/forgejo
Docker/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2025-08-03 00:45:22 +02:00
Code Issues Releases Activity

feat: improve API /repos/{owner}/{repo}/compare/{basehead} error messages

Browse source
This commit is contained in:
Earl Warren 2025-06-28 18:48:37 +02:00
parent a300c0b9fd
commit b8e66a5552
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
2 changed files with 73 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

11
routers/api/v1/repo/pull.go
View file

@ -1084,7 +1084,6 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
err error
)
// If there is no head repository, it means pull request between same repository.
headInfos := strings.Split(form.Head, ":")
if len(headInfos) == 1 {
isSameRepo = true
@ -1094,7 +1093,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
headUser, err = user_model.GetUserByName(ctx, headInfos[0])
if err != nil {
if user_model.IsErrUserNotExist(err) {
ctx.NotFound("GetUserByName")
ctx.NotFound(fmt.Errorf("the owner %s does not exist", headInfos[0]))
} else {
ctx.Error(http.StatusInternalServerError, "GetUserByName", err)
}
@ -1104,7 +1103,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
// The head repository can also point to the same repo
isSameRepo = ctx.Repo.Owner.ID == headUser.ID
} else {
ctx.NotFound()
ctx.NotFound(fmt.Errorf("the head part of {basehead} %s must contain zero or one colon (:) but contains %d", form.Head, len(headInfos)-1))
return nil, nil, nil, "", ""
}
@ -1116,7 +1115,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
baseIsBranch := ctx.Repo.GitRepo.IsBranchExist(baseBranch)
baseIsTag := ctx.Repo.GitRepo.IsTagExist(baseBranch)
if !baseIsCommit && !baseIsBranch && !baseIsTag {
ctx.NotFound("BaseNotExist")
ctx.NotFound(fmt.Errorf("could not find '%s' to be a commit, branch or tag in the base repository %s/%s", baseBranch, baseRepo.Owner.Name, baseRepo.Name))
return nil, nil, nil, "", ""
}
@ -1130,7 +1129,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID {
log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID)
ctx.NotFound("GetBaseRepo")
ctx.NotFound(fmt.Errorf("%[1]s does not have a fork of %[2]s/%[3]s and %[2]s/%[3]s is not a fork of a repository from %[1]s", headUser.Name, baseRepo.Owner.Name, baseRepo.Name))
return nil, nil, nil, "", ""
}
headRepo = baseRepo.BaseRepo
@ -1198,7 +1197,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
headIsBranch := headGitRepo.IsBranchExist(headBranch)
headIsTag := headGitRepo.IsTagExist(headBranch)
if !headIsCommit && !headIsBranch && !headIsTag {
ctx.NotFound("IsRefExist", nil)
ctx.NotFound(fmt.Errorf("could not find '%s' to be a commit, branch or tag in the head repository %s/%s", headBranch, headRepo.Owner.Name, headRepo.Name))
return nil, nil, nil, "", ""
}

68
tests/integration/api_repo_compare_test.go
View file

@ -7,7 +7,9 @@ import (
"encoding/base64"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"time"
@ -50,6 +52,28 @@ func testAPICompareCommits(t *testing.T, objectFormat git.ObjectFormat) {
}
}
requireErrorContains := func(t *testing.T, resp *httptest.ResponseRecorder, expected string) {
t.Helper()
type response struct {
Message string `json:"message"`
Errors []string `json:"errors"`
}
var bodyResp response
DecodeJSON(t, resp, &bodyResp)
if strings.Contains(bodyResp.Message, expected) {
return
}
for _, error := range bodyResp.Errors {
if strings.Contains(error, expected) {
return
}
}
t.Log(fmt.Sprintf("expected %s in %+v", expected, bodyResp))
t.Fail()
}
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
user2repo := "repoA"
user2Ctx := NewAPITestContext(t, user2.Name, user2repo, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
@ -123,6 +147,14 @@ func testAPICompareCommits(t *testing.T, objectFormat git.ObjectFormat) {
user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
user4Ctx := NewAPITestContext(t, user4.Name, user2repo, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
t.Run("ForkNotFound", func(t *testing.T) {
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/compare/%s...%s:%s", user2.Name, user2repo, "master", user4.Name, user2branchName).
AddTokenAuth(user2Ctx.Token)
resp := MakeRequest(t, req, http.StatusNotFound)
requireErrorContains(t, resp, "user4 does not have a fork of user2/repoA and user2/repoA is not a fork of a repository from user4")
})
t.Run("User4ForksUser2Repository", doAPIForkRepository(user4Ctx, user2.Name))
user4branchName := "user4branch"
t.Run("CreateUser4RepositoryBranch", newBranchAndFile(user4Ctx, user4, user4branchName, "user4branchfilename.txt"))
@ -199,5 +231,41 @@ func testAPICompareCommits(t *testing.T, objectFormat git.ObjectFormat) {
assert.Len(t, apiResp.Files, 1)
})
}
t.Run("ForkUserDoesNotExist", func(t *testing.T) {
notUser := "notauser"
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/compare/master...%s:branchname", user2.Name, user2repo, notUser).
AddTokenAuth(user2Ctx.Token)
resp := MakeRequest(t, req, http.StatusNotFound)
requireErrorContains(t, resp, fmt.Sprintf("the owner %s does not exist", notUser))
})
t.Run("HeadHasTooManyColon", func(t *testing.T) {
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/compare/master...one:two:many", user2.Name, user2repo).
AddTokenAuth(user2Ctx.Token)
resp := MakeRequest(t, req, http.StatusNotFound)
requireErrorContains(t, resp, fmt.Sprintf("must contain zero or one colon (:) but contains 2"))
})
for _, testCase := range []struct {
what string
baseHead string
}{
{
what: "base",
baseHead: "notexists...master",
},
{
what: "head",
baseHead: "master...notexists",
},
} {
t.Run("BaseHeadNotExists "+testCase.what, func(t *testing.T) {
req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/compare/%s", user2.Name, user2repo, testCase.baseHead).
AddTokenAuth(user2Ctx.Token)
resp := MakeRequest(t, req, http.StatusNotFound)
requireErrorContains(t, resp, fmt.Sprintf("could not find 'notexists' to be a commit, branch or tag in the %s", testCase.what))
})
}
})
}