From 2b6f45299d6e96b633a309d68055f9ae0699b8f2 Mon Sep 17 00:00:00 2001 From: Antoine GIRARD Date: Fri, 9 Aug 2019 04:13:03 +0200 Subject: [PATCH] api: fix multiple bugs with statuses endpoints (#7785) * fix commit statuses api url * search refs before passing sha * adjust tests * directly search tags and branches names + remove un-needed check in NewCommitStatus * fix comment * de-duplicate code * test: use relative setting.AppURL * Update routers/api/v1/repo/status.go Co-Authored-By: Lauris BH * remove return * Update routers/api/v1/repo/status.go Co-Authored-By: Lauris BH --- integrations/repo_commits_test.go | 26 +++++++++++++++++++++ models/commit_status.go | 2 +- models/commit_status_test.go | 29 +++++++++++++---------- routers/api/v1/repo/git_ref.go | 13 +++++++---- routers/api/v1/repo/status.go | 38 +++++++++++++++++++++++++++---- 5 files changed, 85 insertions(+), 23 deletions(-) diff --git a/integrations/repo_commits_test.go b/integrations/repo_commits_test.go index 1f70d692519..7a4761adc64 100644 --- a/integrations/repo_commits_test.go +++ b/integrations/repo_commits_test.go @@ -5,10 +5,13 @@ package integrations import ( + "encoding/json" "net/http" + "net/http/httptest" "path" "testing" + "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "github.com/stretchr/testify/assert" @@ -67,6 +70,29 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) { for _, class := range classes { assert.True(t, sel.HasClass(class)) } + + //By SHA + req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/"+path.Base(commitURL)+"/statuses") + testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), state) + //By Ref + req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/master/statuses") + testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), state) + req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/commits/v1.1/statuses") + testRepoCommitsWithStatus(t, session.MakeRequest(t, req, http.StatusOK), state) +} + +func testRepoCommitsWithStatus(t *testing.T, resp *httptest.ResponseRecorder, state string) { + decoder := json.NewDecoder(resp.Body) + statuses := []*api.Status{} + assert.NoError(t, decoder.Decode(&statuses)) + assert.Len(t, statuses, 1) + for _, s := range statuses { + assert.Equal(t, api.StatusState(state), s.State) + assert.Equal(t, setting.AppURL+"api/v1/repos/user2/repo1/statuses/65f1bf27bc3bf70f64657658635e66094edbcb4d", s.URL) + assert.Equal(t, "http://test.ci/", s.TargetURL) + assert.Equal(t, "", s.Description) + assert.Equal(t, "testci", s.Context) + } } func TestRepoCommitsWithStatusPending(t *testing.T) { diff --git a/models/commit_status.go b/models/commit_status.go index ecd319c5fec..e864dc30366 100644 --- a/models/commit_status.go +++ b/models/commit_status.go @@ -89,7 +89,7 @@ func (status *CommitStatus) loadRepo(e Engine) (err error) { // APIURL returns the absolute APIURL to this commit-status. func (status *CommitStatus) APIURL() string { _ = status.loadRepo(x) - return fmt.Sprintf("%sapi/v1/%s/statuses/%s", + return fmt.Sprintf("%sapi/v1/repos/%s/statuses/%s", setting.AppURL, status.Repo.FullName(), status.SHA) } diff --git a/models/commit_status_test.go b/models/commit_status_test.go index 580db127f6c..97783ae6f1a 100644 --- a/models/commit_status_test.go +++ b/models/commit_status_test.go @@ -20,20 +20,25 @@ func TestGetCommitStatuses(t *testing.T) { statuses, maxResults, err := GetCommitStatuses(repo1, sha1, &CommitStatusOptions{}) assert.NoError(t, err) assert.Equal(t, int(maxResults), 5) - if assert.Len(t, statuses, 5) { - assert.Equal(t, statuses[0].Context, "ci/awesomeness") - assert.Equal(t, statuses[0].State, CommitStatusPending) + assert.Len(t, statuses, 5) - assert.Equal(t, statuses[1].Context, "cov/awesomeness") - assert.Equal(t, statuses[1].State, CommitStatusWarning) + assert.Equal(t, "ci/awesomeness", statuses[0].Context) + assert.Equal(t, CommitStatusPending, statuses[0].State) + assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[0].APIURL()) - assert.Equal(t, statuses[2].Context, "cov/awesomeness") - assert.Equal(t, statuses[2].State, CommitStatusSuccess) + assert.Equal(t, "cov/awesomeness", statuses[1].Context) + assert.Equal(t, CommitStatusWarning, statuses[1].State) + assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[1].APIURL()) - assert.Equal(t, statuses[3].Context, "ci/awesomeness") - assert.Equal(t, statuses[3].State, CommitStatusFailure) + assert.Equal(t, "cov/awesomeness", statuses[2].Context) + assert.Equal(t, CommitStatusSuccess, statuses[2].State) + assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[2].APIURL()) - assert.Equal(t, statuses[4].Context, "deploy/awesomeness") - assert.Equal(t, statuses[4].State, CommitStatusError) - } + assert.Equal(t, "ci/awesomeness", statuses[3].Context) + assert.Equal(t, CommitStatusFailure, statuses[3].State) + assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[3].APIURL()) + + assert.Equal(t, "deploy/awesomeness", statuses[4].Context) + assert.Equal(t, CommitStatusError, statuses[4].State) + assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[4].APIURL()) } diff --git a/routers/api/v1/repo/git_ref.go b/routers/api/v1/repo/git_ref.go index e15f699a1da..d7acc139f0d 100644 --- a/routers/api/v1/repo/git_ref.go +++ b/routers/api/v1/repo/git_ref.go @@ -71,19 +71,22 @@ func GetGitRefs(ctx *context.APIContext) { getGitRefsInternal(ctx, ctx.Params("*")) } -func getGitRefsInternal(ctx *context.APIContext, filter string) { +func getGitRefs(ctx *context.APIContext, filter string) ([]*git.Reference, string, error) { gitRepo, err := git.OpenRepository(ctx.Repo.Repository.RepoPath()) if err != nil { - ctx.Error(500, "OpenRepository", err) - return + return nil, "OpenRepository", err } if len(filter) > 0 { filter = "refs/" + filter } - refs, err := gitRepo.GetRefsFiltered(filter) + return refs, "GetRefsFiltered", err +} + +func getGitRefsInternal(ctx *context.APIContext, filter string) { + refs, lastMethodName, err := getGitRefs(ctx, filter) if err != nil { - ctx.Error(500, "GetRefsFiltered", err) + ctx.Error(500, lastMethodName, err) return } diff --git a/routers/api/v1/repo/status.go b/routers/api/v1/repo/status.go index e4afb599eed..b3d16e79bc4 100644 --- a/routers/api/v1/repo/status.go +++ b/routers/api/v1/repo/status.go @@ -46,10 +46,7 @@ func NewCommitStatus(ctx *context.APIContext, form api.CreateStatusOption) { // "$ref": "#/responses/StatusList" sha := ctx.Params("sha") if len(sha) == 0 { - sha = ctx.Params("ref") - } - if len(sha) == 0 { - ctx.Error(400, "ref/sha not given", nil) + ctx.Error(400, "sha not given", nil) return } status := &models.CommitStatus{ @@ -155,7 +152,38 @@ func GetCommitStatusesByRef(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/StatusList" - getCommitStatuses(ctx, ctx.Params("ref")) + + filter := ctx.Params("ref") + if len(filter) == 0 { + ctx.Error(400, "ref not given", nil) + return + } + + for _, reftype := range []string{"heads", "tags"} { //Search branches and tags + refSHA, lastMethodName, err := searchRefCommitByType(ctx, reftype, filter) + if err != nil { + ctx.Error(500, lastMethodName, err) + return + } + if refSHA != "" { + filter = refSHA + break + } + + } + + getCommitStatuses(ctx, filter) //By default filter is maybe the raw SHA +} + +func searchRefCommitByType(ctx *context.APIContext, refType, filter string) (string, string, error) { + refs, lastMethodName, err := getGitRefs(ctx, refType+"/"+filter) //Search by type + if err != nil { + return "", lastMethodName, err + } + if len(refs) > 0 { + return refs[0].Object.String(), "", nil //Return found SHA + } + return "", "", nil } func getCommitStatuses(ctx *context.APIContext, sha string) {