From 8f1d62ad3b038186320cd40c99165b23a7f0f05c Mon Sep 17 00:00:00 2001
From: Ethan Koenig <etk39@cornell.edu>
Date: Mon, 10 Jul 2017 07:07:39 -0400
Subject: [PATCH] Fix GET /users/:username/repos endpoint (#2125)

---
 integrations/api_repo_test.go | 37 ++++++++++++++++++---
 routers/api/v1/user/repo.go   | 60 +++++++++++++++++------------------
 2 files changed, 63 insertions(+), 34 deletions(-)

diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go
index 180105d4c0..8f17fce391 100644
--- a/integrations/api_repo_test.go
+++ b/integrations/api_repo_test.go
@@ -6,19 +6,48 @@ package integrations
 
 import (
 	"net/http"
+	"strings"
 	"testing"
+
+	"code.gitea.io/gitea/models"
+	api "code.gitea.io/sdk/gitea"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestAPIUserReposNotLogin(t *testing.T) {
 	prepareTestEnv(t)
+	user := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
 
-	req := NewRequest(t, "GET", "/api/v1/users/user2/repos")
-	MakeRequest(t, req, http.StatusOK)
+	req := NewRequestf(t, "GET", "/api/v1/users/%s/repos", user.Name)
+	resp := MakeRequest(t, req, http.StatusOK)
+
+	var apiRepos []api.Repository
+	DecodeJSON(t, resp, &apiRepos)
+	expectedLen := models.GetCount(t, models.Repository{OwnerID: user.ID},
+		models.Cond("is_private = ?", false))
+	assert.Len(t, apiRepos, expectedLen)
+	for _, repo := range apiRepos {
+		assert.EqualValues(t, user.ID, repo.Owner.ID)
+		assert.False(t, repo.Private)
+	}
+}
+
+type searchResponseBody struct {
+	ok   bool
+	data []api.Repository
 }
 
 func TestAPISearchRepoNotLogin(t *testing.T) {
 	prepareTestEnv(t)
+	const keyword = "test"
 
-	req := NewRequest(t, "GET", "/api/v1/repos/search?q=Test")
-	MakeRequest(t, req, http.StatusOK)
+	req := NewRequestf(t, "GET", "/api/v1/repos/search?q=%s", keyword)
+	resp := MakeRequest(t, req, http.StatusOK)
+
+	var body searchResponseBody
+	DecodeJSON(t, resp, &body)
+	for _, repo := range body.data {
+		assert.True(t, strings.Contains(repo.Name, keyword))
+	}
 }
diff --git a/routers/api/v1/user/repo.go b/routers/api/v1/user/repo.go
index ddb932bce9..c929da5e37 100644
--- a/routers/api/v1/user/repo.go
+++ b/routers/api/v1/user/repo.go
@@ -8,33 +8,29 @@ import (
 
 // listUserRepos - List the repositories owned by the given user.
 func listUserRepos(ctx *context.APIContext, u *models.User) {
-	userID := u.ID
-	showPrivateRepos := ctx.IsSigned && (ctx.User.ID == userID || ctx.User.IsAdmin)
-	ownRepos, err := models.GetUserRepositories(userID, showPrivateRepos, 1, u.NumRepos, "")
+	showPrivateRepos := ctx.IsSigned && (ctx.User.ID == u.ID || ctx.User.IsAdmin)
+	repos, err := models.GetUserRepositories(u.ID, showPrivateRepos, 1, u.NumRepos, "")
 	if err != nil {
 		ctx.Error(500, "GetUserRepositories", err)
 		return
 	}
-	var accessibleRepos []*api.Repository
+	apiRepos := make([]*api.Repository, len(repos))
+	var ctxUserID int64
 	if ctx.User != nil {
-		accessibleRepos, err = getAccessibleRepos(ctx)
+		ctxUserID = ctx.User.ID
+	}
+	for i := range repos {
+		access, err := models.AccessLevel(ctxUserID, repos[i])
 		if err != nil {
-			ctx.Error(500, "GetAccessibleRepos", err)
+			ctx.Error(500, "AccessLevel", err)
+			return
 		}
-	}
-	apiRepos := make([]*api.Repository, len(ownRepos)+len(accessibleRepos))
-	// Set owned repositories.
-	for i := range ownRepos {
-		apiRepos[i] = ownRepos[i].APIFormat(models.AccessModeOwner)
-	}
-	// Set repositories user has access to.
-	for i := 0; i < len(accessibleRepos); i++ {
-		apiRepos[i+len(ownRepos)] = accessibleRepos[i]
+		apiRepos[i] = repos[i].APIFormat(access)
 	}
 	ctx.JSON(200, &apiRepos)
 }
 
-// ListUserRepos - list the repos owned and accessible by the given user.
+// ListUserRepos - list the repos owned by the given user.
 func ListUserRepos(ctx *context.APIContext) {
 	// swagger:route GET /users/{username}/repos userListRepos
 	//
@@ -52,7 +48,7 @@ func ListUserRepos(ctx *context.APIContext) {
 	listUserRepos(ctx, user)
 }
 
-// ListMyRepos - list the repositories owned by you.
+// ListMyRepos - list the repositories you own or have access to.
 func ListMyRepos(ctx *context.APIContext) {
 	// swagger:route GET /user/repos userCurrentListRepos
 	//
@@ -62,21 +58,25 @@ func ListMyRepos(ctx *context.APIContext) {
 	//     Responses:
 	//       200: RepositoryList
 	//       500: error
-
-	listUserRepos(ctx, ctx.User)
-}
-
-// getAccessibleRepos - Get the repositories a user has access to.
-func getAccessibleRepos(ctx *context.APIContext) ([]*api.Repository, error) {
-	accessibleRepos, err := ctx.User.GetRepositoryAccesses()
+	ownRepos, err := models.GetUserRepositories(ctx.User.ID, true, 1, ctx.User.NumRepos, "")
 	if err != nil {
-		return nil, err
+		ctx.Error(500, "GetUserRepositories", err)
+		return
 	}
-	i := 0
-	repos := make([]*api.Repository, len(accessibleRepos))
-	for repo, access := range accessibleRepos {
-		repos[i] = repo.APIFormat(access)
+	accessibleReposMap, err := ctx.User.GetRepositoryAccesses()
+	if err != nil {
+		ctx.Error(500, "GetRepositoryAccesses", err)
+		return
+	}
+
+	apiRepos := make([]*api.Repository, len(ownRepos)+len(accessibleReposMap))
+	for i := range ownRepos {
+		apiRepos[i] = ownRepos[i].APIFormat(models.AccessModeOwner)
+	}
+	i := len(ownRepos)
+	for repo, access := range accessibleReposMap {
+		apiRepos[i] = repo.APIFormat(access)
 		i++
 	}
-	return repos, nil
+	ctx.JSON(200, &apiRepos)
 }