api: fix multiple bugs with statuses endpoints (#7785) (#7807)

* 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 <lauris@nix.lv> * remove return * Update routers/api/v1/repo/status.go Co-Authored-By: Lauris BH <lauris@nix.lv>
go-gitea · Aug 9, 2019 · 38e7997 · 38e7997
1 parent 4f39e56
commit 38e7997
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 23 deletions.
diff --git 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
@@ -87,7 +87,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
@@ -19,20 +19,25 @@ func TestGetCommitStatuses(t *testing.T) {
 
 	statuses, err := GetCommitStatuses(repo1, sha1, 0)
 	assert.NoError(t, err)
-	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
@@ -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
@@ -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{
@@ -121,7 +118,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) {