SearchRepositoryByName improvements and unification (#6897) (#7002)

* SearchRepositoryByName improvements and unification (#6897)

* Update tests

Co-Authored-By: zeripath <art27@cantab.net>

* Update tests

Co-Authored-By: zeripath <art27@cantab.net>

* Update tests

Co-Authored-By: zeripath <art27@cantab.net>

* Fix tests
This commit is contained in:
Lauris BH 2019-05-21 18:07:07 +03:00 committed by GitHub
parent 895764e7f5
commit 608f46e59c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 180 additions and 242 deletions

View file

@ -69,40 +69,41 @@ func TestAPISearchRepo(t *testing.T) {
name, requestURL string
expectedResults
}{
{name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50", expectedResults: expectedResults{
{name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50&private=false", expectedResults: expectedResults{
nil: {count: 19},
user: {count: 19},
user2: {count: 19}},
},
{name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10", expectedResults: expectedResults{
{name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10&private=false", expectedResults: expectedResults{
nil: {count: 10},
user: {count: 10},
user2: {count: 10}},
},
{name: "RepositoriesDefaultMax10", requestURL: "/api/v1/repos/search?default", expectedResults: expectedResults{
{name: "RepositoriesDefaultMax10", requestURL: "/api/v1/repos/search?default&private=false", expectedResults: expectedResults{
nil: {count: 10},
user: {count: 10},
user2: {count: 10}},
},
{name: "RepositoriesByName", requestURL: fmt.Sprintf("/api/v1/repos/search?q=%s", "big_test_"), expectedResults: expectedResults{
{name: "RepositoriesByName", requestURL: fmt.Sprintf("/api/v1/repos/search?q=%s&private=false", "big_test_"), expectedResults: expectedResults{
nil: {count: 7, repoName: "big_test_"},
user: {count: 7, repoName: "big_test_"},
user2: {count: 7, repoName: "big_test_"}},
},
{name: "RepositoriesAccessibleAndRelatedToUser", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", user.ID), expectedResults: expectedResults{
nil: {count: 4},
user: {count: 8, includesPrivate: true},
user2: {count: 4}},
nil: {count: 5},
user: {count: 9, includesPrivate: true},
user2: {count: 5, includesPrivate: true}},
},
{name: "RepositoriesAccessibleAndRelatedToUser2", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", user2.ID), expectedResults: expectedResults{
nil: {count: 1},
user: {count: 1},
user2: {count: 2, includesPrivate: true}},
user: {count: 2, includesPrivate: true},
user2: {count: 2, includesPrivate: true},
user4: {count: 1}},
},
{name: "RepositoriesAccessibleAndRelatedToUser3", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", user3.ID), expectedResults: expectedResults{
nil: {count: 1},
user: {count: 1},
user2: {count: 1},
user: {count: 4, includesPrivate: true},
user2: {count: 2, includesPrivate: true},
user3: {count: 4, includesPrivate: true}},
},
{name: "RepositoriesOwnedByOrganization", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", orgUser.ID), expectedResults: expectedResults{
@ -112,12 +113,12 @@ func TestAPISearchRepo(t *testing.T) {
},
{name: "RepositoriesAccessibleAndRelatedToUser4", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", user4.ID), expectedResults: expectedResults{
nil: {count: 3},
user: {count: 3},
user4: {count: 6, includesPrivate: true}}},
user: {count: 4, includesPrivate: true},
user4: {count: 7, includesPrivate: true}}},
{name: "RepositoriesAccessibleAndRelatedToUser4/SearchModeSource", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d&mode=%s", user4.ID, "source"), expectedResults: expectedResults{
nil: {count: 0},
user: {count: 0},
user4: {count: 0, includesPrivate: true}}},
user: {count: 1, includesPrivate: true},
user4: {count: 1, includesPrivate: true}}},
{name: "RepositoriesAccessibleAndRelatedToUser4/SearchModeFork", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d&mode=%s", user4.ID, "fork"), expectedResults: expectedResults{
nil: {count: 1},
user: {count: 1},
@ -136,8 +137,8 @@ func TestAPISearchRepo(t *testing.T) {
user4: {count: 2, includesPrivate: true}}},
{name: "RepositoriesAccessibleAndRelatedToUser4/SearchModeCollaborative", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d&mode=%s", user4.ID, "collaborative"), expectedResults: expectedResults{
nil: {count: 0},
user: {count: 0},
user4: {count: 0, includesPrivate: true}}},
user: {count: 1, includesPrivate: true},
user4: {count: 1, includesPrivate: true}}},
}
for _, testCase := range testCases {
@ -164,14 +165,19 @@ func TestAPISearchRepo(t *testing.T) {
var body api.SearchResults
DecodeJSON(t, response, &body)
assert.Len(t, body.Data, expected.count)
repoNames := make([]string, 0, len(body.Data))
for _, repo := range body.Data {
repoNames = append(repoNames, fmt.Sprintf("%d:%s:%t", repo.ID, repo.FullName, repo.Private))
}
assert.Len(t, repoNames, expected.count)
for _, repo := range body.Data {
r := getRepo(t, repo.ID)
hasAccess, err := models.HasAccess(userID, r)
assert.NoError(t, err)
assert.True(t, hasAccess)
assert.NoError(t, err, "Error when checking if User: %d has access to %s: %v", userID, repo.FullName, err)
assert.True(t, hasAccess, "User: %d does not have access to %s", userID, repo.FullName)
assert.NotEmpty(t, repo.Name)
assert.Equal(t, repo.Name, r.Name)
if len(expected.repoName) > 0 {
assert.Contains(t, repo.Name, expected.repoName)
@ -182,7 +188,7 @@ func TestAPISearchRepo(t *testing.T) {
}
if !expected.includesPrivate {
assert.False(t, repo.Private)
assert.False(t, repo.Private, "User: %d not expecting private repository: %s", userID, repo.FullName)
}
}
})

View file

@ -12,7 +12,6 @@ import (
"code.gitea.io/gitea/modules/util"
"github.com/go-xorm/builder"
"github.com/go-xorm/core"
)
// RepositoryListDefaultPageSize is the default number of repositories
@ -112,11 +111,13 @@ func (repos MirrorRepositoryList) LoadAttributes() error {
// SearchRepoOptions holds the search options
type SearchRepoOptions struct {
UserID int64
UserIsAdmin bool
Keyword string
OwnerID int64
OrderBy SearchOrderBy
Private bool // Include private repositories in results
Starred bool
StarredByID int64
Page int
IsProfile bool
AllPublic bool // Include also all public repositories
@ -168,21 +169,53 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (RepositoryList, int64, err
if opts.Page <= 0 {
opts.Page = 1
}
var cond = builder.NewCond()
if !opts.Private {
if opts.Private {
if !opts.UserIsAdmin && opts.UserID != 0 && opts.UserID != opts.OwnerID {
// OK we're in the context of a User
// We should be Either
cond = cond.And(builder.Or(
// 1. Be able to see all non-private repositories that either:
cond.And(
builder.Eq{"is_private": false},
builder.Or(
// A. Aren't in organisations __OR__
builder.NotIn("owner_id", builder.Select("id").From("`user`").Where(builder.Eq{"type": UserTypeOrganization})),
// B. Isn't a private organisation. (Limited is OK because we're logged in)
builder.NotIn("owner_id", builder.Select("id").From("`user`").Where(builder.Eq{"visibility": structs.VisibleTypePrivate}))),
),
// 2. Be able to see all repositories that we have access to
builder.In("id", builder.Select("repo_id").
From("`access`").
Where(builder.And(
builder.Eq{"user_id": opts.UserID},
builder.Gt{"mode": int(AccessModeNone)}))),
// 3. Be able to see all repositories that we are in a team
builder.In("id", builder.Select("`team_repo`.repo_id").
From("team_repo").
Where(builder.Eq{"`team_user`.uid": opts.UserID}).
Join("INNER", "team_user", "`team_user`.team_id = `team_repo`.team_id"))))
}
} else {
// Not looking at private organisations
// We should be able to see all non-private repositories that either:
cond = cond.And(builder.Eq{"is_private": false})
accessCond := builder.Or(
builder.NotIn("owner_id", builder.Select("id").From("`user`").Where(builder.Or(builder.Eq{"visibility": structs.VisibleTypeLimited}, builder.Eq{"visibility": structs.VisibleTypePrivate}))),
builder.NotIn("owner_id", builder.Select("id").From("`user`").Where(builder.Eq{"type": UserTypeOrganization})))
// A. Aren't in organisations __OR__
builder.NotIn("owner_id", builder.Select("id").From("`user`").Where(builder.Eq{"type": UserTypeOrganization})),
// B. Isn't a private or limited organisation.
builder.NotIn("owner_id", builder.Select("id").From("`user`").Where(builder.Or(builder.Eq{"visibility": structs.VisibleTypeLimited}, builder.Eq{"visibility": structs.VisibleTypePrivate}))))
cond = cond.And(accessCond)
}
// Restrict to starred repositories
if opts.StarredByID > 0 {
cond = cond.And(builder.In("id", builder.Select("repo_id").From("star").Where(builder.Eq{"uid": opts.StarredByID})))
}
// Restrict repositories to those the OwnerID owns or contributes to as per opts.Collaborate
if opts.OwnerID > 0 {
if opts.Starred {
cond = cond.And(builder.In("id", builder.Select("repo_id").From("star").Where(builder.Eq{"uid": opts.OwnerID})))
} else {
var accessCond = builder.NewCond()
if opts.Collaborate != util.OptionalBoolTrue {
accessCond = builder.Eq{"owner_id": opts.OwnerID}
@ -190,7 +223,12 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (RepositoryList, int64, err
if opts.Collaborate != util.OptionalBoolFalse {
collaborateCond := builder.And(
builder.Or(
builder.Expr("repository.id IN (SELECT repo_id FROM `access` WHERE access.user_id = ?)", opts.OwnerID),
builder.In("id", builder.Select("`team_repo`.repo_id").
From("team_repo").
Where(builder.Eq{"`team_user`.uid": opts.OwnerID}).
Join("INNER", "team_user", "`team_user`.team_id = `team_repo`.team_id"))),
builder.Neq{"owner_id": opts.OwnerID})
if !opts.Private {
collaborateCond = collaborateCond.And(builder.Expr("owner_id NOT IN (SELECT org_id FROM org_user WHERE org_user.uid = ? AND org_user.is_public = ?)", opts.OwnerID, false))
@ -199,42 +237,12 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (RepositoryList, int64, err
accessCond = accessCond.Or(collaborateCond)
}
var exprCond builder.Cond
if DbCfg.Type == core.POSTGRES {
exprCond = builder.Expr("org_user.org_id = \"user\".id")
} else if DbCfg.Type == core.MSSQL {
exprCond = builder.Expr("org_user.org_id = [user].id")
} else {
exprCond = builder.Eq{"org_user.org_id": "user.id"}
}
visibilityCond := builder.Or(
builder.In("owner_id",
builder.Select("org_id").From("org_user").
LeftJoin("`user`", exprCond).
Where(
builder.And(
builder.Eq{"uid": opts.OwnerID},
builder.Eq{"visibility": structs.VisibleTypePrivate})),
),
builder.In("owner_id",
builder.Select("id").From("`user`").
Where(
builder.Or(
builder.Eq{"visibility": structs.VisibleTypePublic},
builder.Eq{"visibility": structs.VisibleTypeLimited})),
),
builder.NotIn("owner_id", builder.Select("id").From("`user`").Where(builder.Eq{"type": UserTypeOrganization})),
)
cond = cond.And(visibilityCond)
if opts.AllPublic {
accessCond = accessCond.Or(builder.Eq{"is_private": false})
}
cond = cond.And(accessCond)
}
}
if opts.Keyword != "" {
// separate keyword

View file

@ -117,7 +117,7 @@ func TestSearchRepositoryByName(t *testing.T) {
count: 4},
{name: "PublicRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15},
count: 4},
count: 5},
{name: "PublicRepositoriesOfUser2IncludingCollaborative",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 18},
count: 1},
@ -126,13 +126,13 @@ func TestSearchRepositoryByName(t *testing.T) {
count: 3},
{name: "PublicAndPrivateRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Private: true},
count: 8},
count: 9},
{name: "PublicAndPrivateRepositoriesOfUser2IncludingCollaborative",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 18, Private: true},
count: 4},
{name: "PublicAndPrivateRepositoriesOfUser3IncludingCollaborative",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 20, Private: true},
count: 6},
count: 7},
{name: "PublicRepositoriesOfOrganization",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 17, Collaborate: util.OptionalBoolFalse},
count: 1},
@ -150,7 +150,7 @@ func TestSearchRepositoryByName(t *testing.T) {
count: 19},
{name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Private: true, AllPublic: true},
count: 24},
count: 25},
{name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName",
opts: &SearchRepoOptions{Keyword: "test", Page: 1, PageSize: 10, OwnerID: 15, Private: true, AllPublic: true},
count: 13},

View file

@ -55,6 +55,15 @@ func Search(ctx *context.APIContext) {
// description: search only for repos that the user with the given id owns or contributes to
// type: integer
// format: int64
// - name: starredBy
// in: query
// description: search only for repos that the user with the given id has starred
// type: integer
// format: int64
// - name: private
// in: query
// description: include private repositories this user has access to (defaults to true)
// type: boolean
// - name: page
// in: query
// description: page number of results to return (1-based)
@ -95,6 +104,10 @@ func Search(ctx *context.APIContext) {
PageSize: convert.ToCorrectPageSize(ctx.QueryInt("limit")),
TopicOnly: ctx.QueryBool("topic"),
Collaborate: util.OptionalBoolNone,
Private: ctx.IsSigned && (ctx.Query("private") == "" || ctx.QueryBool("private")),
UserIsAdmin: ctx.IsSigned && ctx.User.IsAdmin,
UserID: ctx.Data["SignedUserID"].(int64),
StarredByID: ctx.QueryInt64("starredBy"),
}
if ctx.QueryBool("exclusive") {
@ -139,42 +152,6 @@ func Search(ctx *context.APIContext) {
}
var err error
if opts.OwnerID > 0 {
var repoOwner *models.User
if ctx.User != nil && ctx.User.ID == opts.OwnerID {
repoOwner = ctx.User
} else {
repoOwner, err = models.GetUserByID(opts.OwnerID)
if err != nil {
ctx.JSON(500, api.SearchError{
OK: false,
Error: err.Error(),
})
return
}
}
if repoOwner.IsOrganization() {
opts.Collaborate = util.OptionalBoolFalse
}
// Check visibility.
if ctx.IsSigned {
if ctx.User.ID == repoOwner.ID {
opts.Private = true
} else if repoOwner.IsOrganization() {
opts.Private, err = repoOwner.IsOwnedBy(ctx.User.ID)
if err != nil {
ctx.JSON(500, api.SearchError{
OK: false,
Error: err.Error(),
})
return
}
}
}
}
repos, count, err := models.SearchRepositoryByName(opts)
if err != nil {
ctx.JSON(500, api.SearchError{

View file

@ -435,41 +435,13 @@ func showOrgProfile(ctx *context.Context) {
count int64
err error
)
if ctx.IsSigned && !ctx.User.IsAdmin {
env, err := org.AccessibleReposEnv(ctx.User.ID)
if err != nil {
ctx.ServerError("AccessibleReposEnv", err)
return
}
env.SetSort(orderBy)
if len(keyword) != 0 {
env.AddKeyword(keyword)
}
repos, err = env.Repos(page, setting.UI.User.RepoPagingNum)
if err != nil {
ctx.ServerError("env.Repos", err)
return
}
count, err = env.CountRepos()
if err != nil {
ctx.ServerError("env.CountRepos", err)
return
}
} else {
showPrivate := ctx.IsSigned && ctx.User.IsAdmin
if len(keyword) == 0 {
repos, err = models.GetUserRepositories(org.ID, showPrivate, page, setting.UI.User.RepoPagingNum, orderBy.String())
if err != nil {
ctx.ServerError("GetRepositories", err)
return
}
count = models.CountUserRepositories(org.ID, showPrivate)
} else {
repos, count, err = models.SearchRepositoryByName(&models.SearchRepoOptions{
Keyword: keyword,
OwnerID: org.ID,
OrderBy: orderBy,
Private: showPrivate,
Private: ctx.IsSigned,
UserIsAdmin: ctx.IsSigned && ctx.User.IsAdmin,
UserID: ctx.Data["SignedUserID"].(int64),
Page: page,
IsProfile: true,
PageSize: setting.UI.User.RepoPagingNum,
@ -478,8 +450,6 @@ func showOrgProfile(ctx *context.Context) {
ctx.ServerError("SearchRepositoryByName", err)
return
}
}
}
if err := org.GetMembers(); err != nil {
ctx.ServerError("GetMembers", err)

View file

@ -158,27 +158,15 @@ func Profile(ctx *context.Context) {
}
case "stars":
ctx.Data["PageIsProfileStarList"] = true
if len(keyword) == 0 {
repos, err = ctxUser.GetStarredRepos(showPrivate, page, setting.UI.User.RepoPagingNum, orderBy.String())
if err != nil {
ctx.ServerError("GetStarredRepos", err)
return
}
count, err = ctxUser.GetStarredRepoCount(showPrivate)
if err != nil {
ctx.ServerError("GetStarredRepoCount", err)
return
}
} else {
repos, count, err = models.SearchRepositoryByName(&models.SearchRepoOptions{
Keyword: keyword,
OwnerID: ctxUser.ID,
OrderBy: orderBy,
Private: showPrivate,
Private: ctx.IsSigned,
UserIsAdmin: ctx.IsSigned && ctx.User.IsAdmin,
UserID: ctx.Data["SignedUserID"].(int64),
Page: page,
PageSize: setting.UI.User.RepoPagingNum,
Starred: true,
StarredByID: ctxUser.ID,
Collaborate: util.OptionalBoolFalse,
TopicOnly: topicOnly,
})
@ -186,40 +174,18 @@ func Profile(ctx *context.Context) {
ctx.ServerError("SearchRepositoryByName", err)
return
}
}
ctx.Data["Repos"] = repos
ctx.Data["Page"] = paginater.New(int(count), setting.UI.User.RepoPagingNum, page, 5)
ctx.Data["Total"] = count
default:
if len(keyword) == 0 {
var total int
repos, err = models.GetUserRepositories(ctxUser.ID, showPrivate, page, setting.UI.User.RepoPagingNum, orderBy.String())
if err != nil {
ctx.ServerError("GetRepositories", err)
return
}
ctx.Data["Repos"] = repos
if showPrivate {
total = ctxUser.NumRepos
} else {
count, err := models.GetPublicRepositoryCount(ctxUser)
if err != nil {
ctx.ServerError("GetPublicRepositoryCount", err)
return
}
total = int(count)
}
ctx.Data["Page"] = paginater.New(total, setting.UI.User.RepoPagingNum, page, 5)
ctx.Data["Total"] = total
} else {
repos, count, err = models.SearchRepositoryByName(&models.SearchRepoOptions{
Keyword: keyword,
OwnerID: ctxUser.ID,
OrderBy: orderBy,
Private: showPrivate,
Private: ctx.IsSigned,
UserIsAdmin: ctx.IsSigned && ctx.User.IsAdmin,
UserID: ctx.Data["SignedUserID"].(int64),
Page: page,
IsProfile: true,
PageSize: setting.UI.User.RepoPagingNum,
@ -230,12 +196,10 @@ func Profile(ctx *context.Context) {
ctx.ServerError("SearchRepositoryByName", err)
return
}
ctx.Data["Repos"] = repos
ctx.Data["Page"] = paginater.New(int(count), setting.UI.User.RepoPagingNum, page, 5)
ctx.Data["Total"] = count
}
}
ctx.Data["ShowUserEmail"] = len(ctxUser.Email) > 0 && ctx.IsSigned && (!ctxUser.KeepEmailPrivate || ctxUser.ID == ctx.User.ID)

View file

@ -1085,6 +1085,19 @@
"name": "uid",
"in": "query"
},
{
"type": "integer",
"format": "int64",
"description": "search only for repos that the user with the given id has starred",
"name": "starredBy",
"in": "query"
},
{
"type": "boolean",
"description": "include private repositories this user has access to (defaults to true)",
"name": "private",
"in": "query"
},
{
"type": "integer",
"description": "page number of results to return (1-based)",