diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index 8e383dbda2d7..01afc7f8ee34 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -5,6 +5,7 @@ package integrations import ( + "fmt" "net/http" "testing" @@ -32,7 +33,7 @@ func TestAPIUserReposNotLogin(t *testing.T) { } } -func TestAPISearchRepoNotLogin(t *testing.T) { +func TestAPISearchRepo(t *testing.T) { prepareTestEnv(t) const keyword = "test" @@ -46,6 +47,111 @@ func TestAPISearchRepoNotLogin(t *testing.T) { assert.Contains(t, repo.Name, keyword) assert.False(t, repo.Private) } + + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 15}).(*models.User) + user2 := models.AssertExistsAndLoadBean(t, &models.User{ID: 16}).(*models.User) + orgUser := models.AssertExistsAndLoadBean(t, &models.User{ID: 17}).(*models.User) + + type privacyType int + + const ( + _ privacyType = iota + privacyTypePrivate + privacyTypePublic + ) + + // Map of expected results, where key is user for login + type expectedResults map[*models.User]struct { + count int + repoOwnerID int64 + repoName string + privacy privacyType + } + + testCases := []struct { + name, requestURL string + expectedResults + }{ + {name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50", expectedResults: expectedResults{ + nil: {count: 15, privacy: privacyTypePublic}, + user: {count: 15, privacy: privacyTypePublic}, + user2: {count: 15, privacy: privacyTypePublic}}, + }, + {name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10", expectedResults: expectedResults{ + nil: {count: 10, privacy: privacyTypePublic}, + user: {count: 10, privacy: privacyTypePublic}, + user2: {count: 10, privacy: privacyTypePublic}}, + }, + {name: "RepositoriesDefaultMax10", requestURL: "/api/v1/repos/search", expectedResults: expectedResults{ + nil: {count: 10, privacy: privacyTypePublic}, + user: {count: 10, privacy: privacyTypePublic}, + user2: {count: 10, privacy: privacyTypePublic}}, + }, + {name: "RepositoriesByName", requestURL: fmt.Sprintf("/api/v1/repos/search?q=%s", "big_test_"), expectedResults: expectedResults{ + nil: {count: 7, repoName: "big_test_", privacy: privacyTypePublic}, + user: {count: 7, repoName: "big_test_", privacy: privacyTypePublic}, + user2: {count: 7, repoName: "big_test_", privacy: privacyTypePublic}}, + }, + {name: "RepositoriesAccessibleAndRelatedToUser", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d&limit=50", user.ID), expectedResults: expectedResults{ + nil: {count: 7, privacy: privacyTypePublic}, + user: {count: 14}, + user2: {count: 7, privacy: privacyTypePublic}}, + }, + {name: "RepositoriesAccessibleAndRelatedToUser2", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", user2.ID), expectedResults: expectedResults{ + nil: {count: 1, privacy: privacyTypePublic}, + user: {count: 1, privacy: privacyTypePublic}, + user2: {count: 2}}, + }, + {name: "RepositoriesOwnedByOrganization", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", orgUser.ID), expectedResults: expectedResults{ + nil: {count: 2, repoOwnerID: orgUser.ID, privacy: privacyTypePublic}, + user: {count: 4, repoOwnerID: orgUser.ID}, + user2: {count: 2, repoOwnerID: orgUser.ID, privacy: privacyTypePublic}}, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + for userToLogin, expected := range testCase.expectedResults { + var session *TestSession + var testName string + if userToLogin != nil && userToLogin.ID > 0 { + testName = fmt.Sprintf("LoggedUser%d", userToLogin.ID) + session = loginUser(t, userToLogin.Name) + } else { + testName = "AnonymousUser" + session = emptyTestSession(t) + } + + t.Run(testName, func(t *testing.T) { + request := NewRequest(t, "GET", testCase.requestURL) + response := session.MakeRequest(t, request, http.StatusOK) + + var body api.SearchResults + DecodeJSON(t, response, &body) + + assert.Len(t, body.Data, expected.count) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + + if len(expected.repoName) > 0 { + assert.Contains(t, repo.Name, expected.repoName) + } + + if expected.repoOwnerID > 0 { + assert.Equal(t, expected.repoOwnerID, repo.Owner.ID) + } + + switch expected.privacy { + case privacyTypePrivate: + assert.True(t, repo.Private) + case privacyTypePublic: + assert.False(t, repo.Private) + } + } + }) + } + }) + } } func TestAPIViewRepo(t *testing.T) { diff --git a/models/fixtures/access.yml b/models/fixtures/access.yml index 8722b248db2f..683d36ca1393 100644 --- a/models/fixtures/access.yml +++ b/models/fixtures/access.yml @@ -15,3 +15,38 @@ user_id: 4 repo_id: 3 mode: 2 # write + +- + id: 4 + user_id: 15 + repo_id: 22 + mode: 2 # write + +- + id: 5 + user_id: 15 + repo_id: 21 + mode: 2 # write + +- + id: 6 + user_id: 15 + repo_id: 23 + mode: 4 # owner + +- + id: 7 + user_id: 15 + repo_id: 24 + mode: 4 # owner +- + id: 8 + user_id: 15 + repo_id: 27 + mode: 4 # owner + +- + id: 9 + user_id: 15 + repo_id: 28 + mode: 4 # owner \ No newline at end of file diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml index a7b8d178aa13..c7fe2cf36952 100644 --- a/models/fixtures/org_user.yml +++ b/models/fixtures/org_user.yml @@ -29,3 +29,11 @@ is_public: false is_owner: true num_teams: 1 + +- + id: 5 + uid: 15 + org_id: 17 + is_public: true + is_owner: true + num_teams: 1 diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 51812d358553..e48a102eef28 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -188,3 +188,194 @@ num_pulls: 0 num_closed_pulls: 0 num_watches: 0 + +- + id: 17 + owner_id: 15 + lower_name: big_test_public_1 + name: big_test_public_1 + is_private: false + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + num_watches: 0 + is_mirror: false + is_fork: false + +- + id: 18 + owner_id: 15 + lower_name: big_test_public_2 + name: big_test_public_2 + is_private: false + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + is_fork: false + +- + id: 19 + owner_id: 15 + lower_name: big_test_private_1 + name: big_test_private_1 + is_private: true + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + is_fork: false + +- + id: 20 + owner_id: 15 + lower_name: big_test_private_2 + name: big_test_private_2 + is_private: true + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + is_fork: false + +- + id: 21 + owner_id: 16 + lower_name: big_test_public_3 + name: big_test_public_3 + is_private: false + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + is_fork: false + +- + id: 22 + owner_id: 16 + lower_name: big_test_private_3 + name: big_test_private_3 + is_private: true + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + is_fork: false + +- + id: 23 + owner_id: 17 + lower_name: big_test_public_4 + name: big_test_public_4 + is_private: false + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + is_fork: false + +- + id: 24 + owner_id: 17 + lower_name: big_test_private_4 + name: big_test_private_4 + is_private: true + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + is_fork: false + +- + id: 25 + owner_id: 15 + lower_name: big_test_public_mirror_5 + name: big_test_public_mirror_5 + is_private: false + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + num_watches: 0 + is_mirror: true + is_fork: false + +- + id: 26 + owner_id: 15 + lower_name: big_test_private_mirror_5 + name: big_test_private_mirror_5 + is_private: true + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + num_watches: 0 + is_mirror: true + is_fork: false + +- + id: 27 + owner_id: 17 + lower_name: big_test_public_mirror_6 + name: big_test_public_mirror_6 + is_private: false + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + num_watches: 0 + is_mirror: true + num_forks: 1 + is_fork: false + +- + id: 28 + owner_id: 17 + lower_name: big_test_private_mirror_6 + name: big_test_private_mirror_6 + is_private: true + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + num_watches: 0 + is_mirror: true + num_forks: 1 + is_fork: false + +- + id: 29 + fork_id: 27 + owner_id: 15 + lower_name: big_test_public_fork_7 + name: big_test_public_fork_7 + is_private: false + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + is_fork: true + +- + id: 30 + fork_id: 28 + owner_id: 15 + lower_name: big_test_private_fork_7 + name: big_test_private_fork_7 + is_private: true + num_issues: 0 + num_closed_issues: 0 + num_pulls: 0 + num_closed_pulls: 0 + is_mirror: false + is_fork: true \ No newline at end of file diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index 795d5cda6c99..0c3f8d8398c7 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -37,3 +37,12 @@ num_repos: 0 num_members: 1 unit_types: '[1,2,3,4,5,6,7]' +- + id: 5 + org_id: 17 + lower_name: owners + name: Owners + authorize: 4 # owner + num_repos: 4 + num_members: 1 + unit_types: '[1,2,3,4,5,6,7]' diff --git a/models/fixtures/team_repo.yml b/models/fixtures/team_repo.yml index 1f7385e7e9c1..722a72ba9aa5 100644 --- a/models/fixtures/team_repo.yml +++ b/models/fixtures/team_repo.yml @@ -15,3 +15,27 @@ org_id: 3 team_id: 1 repo_id: 5 + +- + id: 4 + org_id: 17 + team_id: 5 + repo_id: 23 + +- + id: 5 + org_id: 17 + team_id: 5 + repo_id: 24 + +- + id: 6 + org_id: 17 + team_id: 5 + repo_id: 27 + +- + id: 7 + org_id: 17 + team_id: 5 + repo_id: 28 \ No newline at end of file diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml index 955a80e4cdf0..8d9d9203782e 100644 --- a/models/fixtures/team_user.yml +++ b/models/fixtures/team_user.yml @@ -27,3 +27,9 @@ org_id: 7 team_id: 4 uid: 5 + +- + id: 6 + org_id: 17 + team_id: 5 + uid: 15 \ No newline at end of file diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index abd72b116810..17eff052320f 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -218,3 +218,50 @@ avatar_email: user13@example.com num_repos: 3 is_active: true + +- + id: 15 + lower_name: user15 + name: user15 + full_name: User 15 + email: user15@example.com + passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password + type: 0 # individual + salt: ZogKvWdyEx + is_admin: false + avatar: avatar15 + avatar_email: user15@example.com + num_repos: 8 + is_active: true + +- + id: 16 + lower_name: user16 + name: user16 + full_name: User 16 + email: user16@example.com + passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password + type: 0 # individual + salt: ZogKvWdyEx + is_admin: false + avatar: avatar16 + avatar_email: user16@example.com + num_repos: 2 + is_active: true + +- + id: 17 + lower_name: user17 + name: user17 + full_name: User 17 + email: user17@example.com + passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password + type: 1 # organization + salt: ZogKvWdyEx + is_admin: false + avatar: avatar17 + avatar_email: user17@example.com + num_repos: 4 + is_active: true + num_members: 1 + num_teams: 1 \ No newline at end of file diff --git a/models/org_test.go b/models/org_test.go index 07da2d56af90..c7bdb8b5d3db 100644 --- a/models/org_test.go +++ b/models/org_test.go @@ -252,7 +252,7 @@ func TestOrganizations(t *testing.T) { []int64{3, 6}) testSuccess(&SearchUserOptions{OrderBy: "id ASC", Page: 2, PageSize: 2}, - []int64{7}) + []int64{7, 17}) testSuccess(&SearchUserOptions{Page: 3, PageSize: 2}, []int64{}) diff --git a/models/repo_list.go b/models/repo_list.go index 15e6144d0636..3cf79410cf5b 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -97,103 +97,165 @@ type SearchRepoOptions struct { // Owner in we search search // // in: query - OwnerID int64 `json:"uid"` - Searcher *User `json:"-"` //ID of the person who's seeking - OrderBy string `json:"-"` - Private bool `json:"-"` // Include private repositories in results - Collaborate bool `json:"-"` // Include collaborative repositories - Starred bool `json:"-"` - Page int `json:"-"` - IsProfile bool `json:"-"` + OwnerID int64 `json:"uid"` + OrderBy SearchOrderBy `json:"-"` + Private bool `json:"-"` // Include private repositories in results + Collaborate bool `json:"-"` // Include collaborative repositories + Starred bool `json:"-"` + Page int `json:"-"` + IsProfile bool `json:"-"` // Limit of result // // maximum: setting.ExplorePagingNum // in: query PageSize int `json:"limit"` // Can be smaller than or equal to setting.ExplorePagingNum + // Type of repository to search (related to owner if present) + // + // in: query + RepoType RepoType `json:"type"` +} + +// RepoType is repository filtering type identifier +type RepoType string + +const ( + // RepoTypeAny any type (default) + RepoTypeAny RepoType = "" + // RepoTypeFork fork type + RepoTypeFork = "FORK" + // RepoTypeMirror mirror type + RepoTypeMirror = "MIRROR" + // RepoTypeSource source type + RepoTypeSource = "SOURCE" + // RepoTypeCollaborative collaborative type + RepoTypeCollaborative = "COLLABORATIVE" +) + +//SearchOrderBy is used to sort the result +type SearchOrderBy string + +func (s SearchOrderBy) String() string { + return string(s) } +// Strings for sorting result +const ( + SearchOrderByAlphabetically SearchOrderBy = "name ASC" + SearchOrderByAlphabeticallyReverse = "name DESC" + SearchOrderByLeastUpdated = "updated_unix ASC" + SearchOrderByRecentUpdated = "updated_unix DESC" + SearchOrderByOldest = "created_unix ASC" + SearchOrderByNewest = "created_unix DESC" + SearchOrderBySize = "size ASC" + SearchOrderBySizeReverse = "size DESC" +) + // SearchRepositoryByName takes keyword and part of repository name to search, // it returns results in given range and number of total results. -func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, count int64, err error) { - var cond = builder.NewCond() +func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ int64, _ error) { + // Check if user with Owner ID exists + if opts.OwnerID > 0 { + userExists, err := GetUser(&User{ID: opts.OwnerID}) + if err != nil { + return nil, 0, err + } + if !userExists { + return nil, 0, ErrUserNotExist{UID: opts.OwnerID} + } + } + + if opts.RepoType == RepoTypeCollaborative && (!opts.Collaborate || opts.OwnerID <= 0) { + return repos, 0, nil + } + + // Check and set page to correct number if opts.Page <= 0 { opts.Page = 1 } - var starJoin bool - if opts.Starred && opts.OwnerID > 0 { - cond = builder.Eq{ - "star.uid": opts.OwnerID, - } - starJoin = true - } + var cond = builder.NewCond() - opts.Keyword = strings.ToLower(opts.Keyword) + // Add repository name keyword to search for if opts.Keyword != "" { + opts.Keyword = strings.ToLower(opts.Keyword) cond = cond.And(builder.Like{"lower_name", opts.Keyword}) } - // Append conditions - if !opts.Starred && opts.OwnerID > 0 { - var searcherReposCond builder.Cond = builder.Eq{"owner_id": opts.OwnerID} - if opts.Searcher != nil { - var ownerIds []int64 - - ownerIds = append(ownerIds, opts.Searcher.ID) - err = opts.Searcher.GetOrganizations(true) + // Exclude private repositories + if !opts.Private { + cond = cond.And(builder.Eq{"is_private": false}) + } - if err != nil { - return nil, 0, fmt.Errorf("Organization: %v", err) + includeStarred := false + if opts.OwnerID > 0 { + if opts.Starred { + // Return only starred repositories by Owner + includeStarred = true + cond = builder.Eq{ + "star.uid": opts.OwnerID, } + } else { + // Set user access conditions + var accessCond builder.Cond = builder.NewCond() - for _, org := range opts.Searcher.Orgs { - ownerIds = append(ownerIds, org.ID) + // Add Owner ID to access conditions + if opts.RepoType != RepoTypeCollaborative { + accessCond = accessCond.Or(builder.Eq{"owner_id": opts.OwnerID}) } - searcherReposCond = searcherReposCond.Or(builder.In("owner_id", ownerIds)) - if opts.Collaborate { - searcherReposCond = searcherReposCond.Or(builder.Expr("id IN (SELECT repo_id FROM `access` WHERE access.user_id = ? AND owner_id != ?)", - opts.Searcher.ID, opts.Searcher.ID)) + // Include collaborative repositories + if opts.Collaborate && (opts.RepoType == RepoTypeAny || opts.RepoType == RepoTypeMirror || opts.RepoType == RepoTypeCollaborative) { + // Add repositories where user is set as collaborator directly + accessCond = accessCond.Or(builder.And( + builder.Expr("id IN (SELECT repo_id FROM `access` WHERE access.user_id = ?)", opts.OwnerID), + builder.Neq{"owner_id": opts.OwnerID})) } + + // Add user access conditions to search + cond = cond.And(accessCond) } - cond = cond.And(searcherReposCond) } - if !opts.Private { - cond = cond.And(builder.Eq{"is_private": false}) + if len(opts.OrderBy) == 0 { + opts.OrderBy = SearchOrderByAlphabetically } - if len(opts.OrderBy) == 0 { - opts.OrderBy = "name ASC" + // Add general filters for repository + if opts.RepoType != RepoTypeAny { + cond = cond.And(builder.Eq{"is_mirror": opts.RepoType == RepoTypeMirror}) + + switch opts.RepoType { + case RepoTypeFork: + cond = cond.And(builder.Eq{"is_fork": true}) + case RepoTypeSource: + cond = cond.And(builder.Eq{"is_fork": false}) + } } sess := x.NewSession() defer sess.Close() - if starJoin { - count, err = sess. - Join("INNER", "star", "star.repo_id = repository.id"). - Where(cond). - Count(new(Repository)) - if err != nil { - return nil, 0, fmt.Errorf("Count: %v", err) - } + if includeStarred { + sess.Join("INNER", "star", "star.repo_id = repository.id") + } + count, err := sess. + Where(cond). + Count(new(Repository)) + if err != nil { + return nil, 0, fmt.Errorf("Count: %v", err) + } + + // Set again after reset by Count() + if includeStarred { sess.Join("INNER", "star", "star.repo_id = repository.id") - } else { - count, err = sess. - Where(cond). - Count(new(Repository)) - if err != nil { - return nil, 0, fmt.Errorf("Count: %v", err) - } } repos = make([]*Repository, 0, opts.PageSize) if err = sess. Where(cond). Limit(opts.PageSize, (opts.Page-1)*opts.PageSize). - OrderBy(opts.OrderBy). + OrderBy(opts.OrderBy.String()). Find(&repos); err != nil { return nil, 0, fmt.Errorf("Repo: %v", err) } @@ -204,7 +266,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, coun } } - return + return repos, count, nil } // Repositories returns all repositories @@ -217,7 +279,7 @@ func Repositories(opts *SearchRepoOptions) (_ RepositoryList, count int64, err e if err = x. Limit(opts.PageSize, (opts.Page-1)*opts.PageSize). - OrderBy(opts.OrderBy). + OrderBy(opts.OrderBy.String()). Find(&repos); err != nil { return nil, 0, fmt.Errorf("Repo: %v", err) } @@ -230,54 +292,3 @@ func Repositories(opts *SearchRepoOptions) (_ RepositoryList, count int64, err e return repos, count, nil } - -// GetRecentUpdatedRepositories returns the list of repositories that are recently updated. -func GetRecentUpdatedRepositories(opts *SearchRepoOptions) (repos RepositoryList, _ int64, _ error) { - var cond = builder.NewCond() - - if len(opts.OrderBy) == 0 { - opts.OrderBy = "updated_unix DESC" - } - - if !opts.Private { - cond = builder.Eq{ - "is_private": false, - } - } - - if opts.Searcher != nil && !opts.Searcher.IsAdmin { - var ownerIds []int64 - - ownerIds = append(ownerIds, opts.Searcher.ID) - err := opts.Searcher.GetOrganizations(true) - - if err != nil { - return nil, 0, fmt.Errorf("Organization: %v", err) - } - - for _, org := range opts.Searcher.Orgs { - ownerIds = append(ownerIds, org.ID) - } - - cond = cond.Or(builder.In("owner_id", ownerIds)) - } - - count, err := x.Where(cond).Count(new(Repository)) - if err != nil { - return nil, 0, fmt.Errorf("Count: %v", err) - } - - if err = x.Where(cond). - Limit(opts.PageSize, (opts.Page-1)*opts.PageSize). - Limit(opts.PageSize). - OrderBy(opts.OrderBy). - Find(&repos); err != nil { - return nil, 0, fmt.Errorf("Repo: %v", err) - } - - if err = repos.loadAttributes(x); err != nil { - return nil, 0, fmt.Errorf("LoadAttributes: %v", err) - } - - return repos, count, nil -} diff --git a/models/repo_list_test.go b/models/repo_list_test.go index 8ac0d804892d..5adfce4b4caa 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -5,6 +5,7 @@ package models import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -18,10 +19,8 @@ func TestSearchRepositoryByName(t *testing.T) { Keyword: "repo_12", Page: 1, PageSize: 10, - Searcher: nil, }) - assert.NotNil(t, repos) assert.NoError(t, err) if assert.Len(t, repos, 1) { assert.Equal(t, "test_repo_12", repos[0].Name) @@ -32,12 +31,11 @@ func TestSearchRepositoryByName(t *testing.T) { Keyword: "test_repo", Page: 1, PageSize: 10, - Searcher: nil, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(2), count) + assert.Len(t, repos, 2) // test search private repository on explore page repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -45,10 +43,8 @@ func TestSearchRepositoryByName(t *testing.T) { Page: 1, PageSize: 10, Private: true, - Searcher: &User{ID: 14}, }) - assert.NotNil(t, repos) assert.NoError(t, err) if assert.Len(t, repos, 1) { assert.Equal(t, "test_repo_13", repos[0].Name) @@ -60,10 +56,163 @@ func TestSearchRepositoryByName(t *testing.T) { Page: 1, PageSize: 10, Private: true, - Searcher: &User{ID: 14}, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(3), count) + assert.Len(t, repos, 3) + + // Test non existing owner + nonExistingUserID := int64(99999) + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + OwnerID: nonExistingUserID, + }) + + if assert.Error(t, err) { + assert.Equal(t, ErrUserNotExist{UID: nonExistingUserID}, err) + } + assert.Empty(t, repos) + assert.Equal(t, int64(0), count) + + type expectedCounts map[RepoType]int + + helperExpectedCounts := expectedCounts{RepoTypeAny: 14, RepoTypeSource: 8, RepoTypeFork: 2, RepoTypeMirror: 4, RepoTypeCollaborative: 0} + + testCases := []struct { + name string + opts *SearchRepoOptions + expectedCounts + }{ + {name: "PublicRepositoriesByName", + opts: &SearchRepoOptions{Keyword: "big_test_", PageSize: 10}, + expectedCounts: expectedCounts{RepoTypeAny: 7, RepoTypeSource: 4, RepoTypeFork: 1, RepoTypeMirror: 2, RepoTypeCollaborative: 0}, + }, + {name: "PublicAndPrivateRepositoriesByName", + opts: &SearchRepoOptions{Keyword: "big_test_", Page: 1, PageSize: 10, Private: true}, + expectedCounts: helperExpectedCounts, + }, + {name: "PublicAndPrivateRepositoriesByNameWithPagesizeLimitFirstPage", + opts: &SearchRepoOptions{Keyword: "big_test_", Page: 1, PageSize: 5, Private: true}, + expectedCounts: helperExpectedCounts, + }, + {name: "PublicAndPrivateRepositoriesByNameWithPagesizeLimitSecondPage", + opts: &SearchRepoOptions{Keyword: "big_test_", Page: 2, PageSize: 5, Private: true}, + expectedCounts: helperExpectedCounts, + }, + {name: "PublicAndPrivateRepositoriesByNameWithPagesizeLimitThirdPage", + opts: &SearchRepoOptions{Keyword: "big_test_", Page: 3, PageSize: 5, Private: true}, + expectedCounts: helperExpectedCounts, + }, + {name: "PublicAndPrivateRepositoriesByNameWithPagesizeLimitFourthPage", + opts: &SearchRepoOptions{Keyword: "big_test_", Page: 3, PageSize: 5, Private: true}, + expectedCounts: helperExpectedCounts, + }, + {name: "PublicRepositoriesOfUser", + opts: &SearchRepoOptions{Page: 1, PageSize: 50, OwnerID: 15}, + expectedCounts: expectedCounts{RepoTypeAny: 4, RepoTypeSource: 2, RepoTypeFork: 1, RepoTypeMirror: 1, RepoTypeCollaborative: 0}, + }, + {name: "PublicAndPrivateRepositoriesOfUser", + opts: &SearchRepoOptions{Page: 1, PageSize: 50, OwnerID: 15, Private: true}, + expectedCounts: expectedCounts{RepoTypeAny: 8, RepoTypeSource: 4, RepoTypeFork: 2, RepoTypeMirror: 2, RepoTypeCollaborative: 0}, + }, + {name: "PublicRepositoriesOfUserIncludingCollaborative", + opts: &SearchRepoOptions{Page: 1, PageSize: 50, OwnerID: 15, Collaborate: true}, + expectedCounts: expectedCounts{RepoTypeAny: 7, RepoTypeSource: 2, RepoTypeFork: 1, RepoTypeMirror: 2, RepoTypeCollaborative: 2}, + }, + {name: "PublicAndPrivateRepositoriesOfUserIncludingCollaborative", + opts: &SearchRepoOptions{Page: 1, PageSize: 50, OwnerID: 15, Private: true, Collaborate: true}, + expectedCounts: expectedCounts{RepoTypeAny: 14, RepoTypeSource: 4, RepoTypeFork: 2, RepoTypeMirror: 4, RepoTypeCollaborative: 4}, + }, + {name: "PublicRepositoriesOfOrganization", + opts: &SearchRepoOptions{Page: 1, PageSize: 50, OwnerID: 17}, + expectedCounts: expectedCounts{RepoTypeAny: 2, RepoTypeSource: 1, RepoTypeFork: 0, RepoTypeMirror: 1, RepoTypeCollaborative: 0}, + }, + {name: "PublicAndPrivateRepositoriesOfOrganization", + opts: &SearchRepoOptions{Page: 1, PageSize: 50, OwnerID: 17, Private: true}, + expectedCounts: expectedCounts{RepoTypeAny: 4, RepoTypeSource: 2, RepoTypeFork: 0, RepoTypeMirror: 2, RepoTypeCollaborative: 0}, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + for repoType, expectedCount := range testCase.expectedCounts { + testName := "RepoTypeANY" + testCase.opts.RepoType = repoType + if len(repoType) > 0 { + testName = fmt.Sprintf("RepoType%s", repoType) + } + + t.Run(testName, func(t *testing.T) { + repos, count, err := SearchRepositoryByName(testCase.opts) + + assert.NoError(t, err) + assert.Equal(t, int64(expectedCount), count) + + var expectedLen int + if testCase.opts.PageSize*testCase.opts.Page > expectedCount+testCase.opts.PageSize { + expectedLen = 0 + } else if testCase.opts.PageSize*testCase.opts.Page > expectedCount { + expectedLen = expectedCount % testCase.opts.PageSize + } else { + expectedLen = testCase.opts.PageSize + } + if assert.Len(t, repos, expectedLen) { + var tester func(t *testing.T, ownerID int64, repo *Repository) + + switch repoType { + case RepoTypeSource: + tester = repoTypeSourceTester + case RepoTypeFork: + tester = repoTypeForkTester + case RepoTypeMirror: + tester = repoTypeMirrorTester + case RepoTypeCollaborative: + tester = repoTypeCollaborativeTester + case RepoTypeAny: + tester = repoTypeDefaultTester + } + + for _, repo := range repos { + tester(t, testCase.opts.OwnerID, repo) + } + } + }) + } + }) + } +} + +func repoTypeSourceTester(t *testing.T, ownerID int64, repo *Repository) { + repoTypeDefaultTester(t, ownerID, repo) + assert.False(t, repo.IsFork) + assert.False(t, repo.IsMirror) + if ownerID > 0 { + assert.Equal(t, ownerID, repo.OwnerID) + } +} + +func repoTypeForkTester(t *testing.T, ownerID int64, repo *Repository) { + repoTypeDefaultTester(t, ownerID, repo) + assert.True(t, repo.IsFork) + assert.False(t, repo.IsMirror) + if ownerID > 0 { + assert.Equal(t, ownerID, repo.OwnerID) + } +} + +func repoTypeMirrorTester(t *testing.T, ownerID int64, repo *Repository) { + repoTypeDefaultTester(t, ownerID, repo) + assert.True(t, repo.IsMirror) +} + +func repoTypeCollaborativeTester(t *testing.T, ownerID int64, repo *Repository) { + repoTypeDefaultTester(t, ownerID, repo) + assert.False(t, repo.IsMirror) + if ownerID > 0 { + assert.NotEqual(t, ownerID, repo.OwnerID) + } +} + +func repoTypeDefaultTester(t *testing.T, ownerID int64, repo *Repository) { + assert.NotEmpty(t, repo.Name) } diff --git a/public/swagger.v1.json b/public/swagger.v1.json index e640b4e83160..643b265b389d 100644 --- a/public/swagger.v1.json +++ b/public/swagger.v1.json @@ -1113,6 +1113,13 @@ "description": "Limit of result\n\nmaximum: setting.ExplorePagingNum", "name": "limit", "in": "query" + }, + { + "type": "string", + "x-go-name": "RepoType", + "description": "Type of repository to search (related to owner if present)", + "name": "type", + "in": "query" } ], "responses": { diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 20393102fc82..2dd759d544bd 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -28,37 +28,38 @@ func Search(ctx *context.APIContext) { // Responses: // 200: SearchResults // 500: SearchError + repoTypes := map[string]models.RepoType{ + "fork": models.RepoTypeFork, + "mirror": models.RepoTypeMirror, + "source": models.RepoTypeSource, + "collaborative": models.RepoTypeCollaborative, + } opts := &models.SearchRepoOptions{ Keyword: strings.Trim(ctx.Query("q"), " "), OwnerID: ctx.QueryInt64("uid"), PageSize: convert.ToCorrectPageSize(ctx.QueryInt("limit")), - } - if ctx.User != nil && ctx.User.ID == opts.OwnerID { - opts.Searcher = ctx.User + RepoType: repoTypes[ctx.Query("type")], } - // Check visibility. - if ctx.IsSigned && opts.OwnerID > 0 { - if ctx.User.ID == opts.OwnerID { - opts.Private = true + // Include collaborative and private repositories + if opts.OwnerID > 0 { + owner, err := models.GetUserByID(opts.OwnerID) + if err != nil { + ctx.JSON(500, api.SearchError{ + OK: false, + Error: err.Error(), + }) + return + } + + if !owner.IsOrganization() { opts.Collaborate = true - } else { - u, err := models.GetUserByID(opts.OwnerID) - if err != nil { - ctx.JSON(500, api.SearchError{ - OK: false, - Error: err.Error(), - }) - return - } - if u.IsOrganization() && u.IsOwnedBy(ctx.User.ID) { - opts.Private = true - } + } - if !u.IsOrganization() { - opts.Collaborate = true - } + // Check visibility. + if ctx.IsSigned && (ctx.User.ID == owner.ID || (owner.IsOrganization() && owner.IsOwnedBy(ctx.User.ID))) { + opts.Private = true } } diff --git a/routers/home.go b/routers/home.go index 16d0720551c4..13060d716ca8 100644 --- a/routers/home.go +++ b/routers/home.go @@ -61,7 +61,6 @@ func Swagger(ctx *context.Context) { // RepoSearchOptions when calling search repositories type RepoSearchOptions struct { Ranger func(*models.SearchRepoOptions) (models.RepositoryList, int64, error) - Searcher *models.User Private bool PageSize int TplName base.TplName @@ -86,61 +85,45 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) { repos []*models.Repository count int64 err error - orderBy string + orderBy models.SearchOrderBy ) ctx.Data["SortType"] = ctx.Query("sort") switch ctx.Query("sort") { case "oldest": - orderBy = "created_unix ASC" + orderBy = models.SearchOrderByOldest case "recentupdate": - orderBy = "updated_unix DESC" + orderBy = models.SearchOrderByRecentUpdated case "leastupdate": - orderBy = "updated_unix ASC" + orderBy = models.SearchOrderByLeastUpdated case "reversealphabetically": - orderBy = "name DESC" + orderBy = models.SearchOrderByAlphabeticallyReverse case "alphabetically": - orderBy = "name ASC" + orderBy = models.SearchOrderByAlphabetically case "reversesize": - orderBy = "size DESC" + orderBy = models.SearchOrderBySizeReverse case "size": - orderBy = "size ASC" + orderBy = models.SearchOrderBySize default: - orderBy = "created_unix DESC" + orderBy = models.SearchOrderByNewest } - keyword := strings.Trim(ctx.Query("q"), " ") - if len(keyword) == 0 { - repos, count, err = opts.Ranger(&models.SearchRepoOptions{ - Page: page, - PageSize: opts.PageSize, - Searcher: ctx.User, - OrderBy: orderBy, - Private: opts.Private, - Collaborate: true, - }) + searchOpts := &models.SearchRepoOptions{ + Page: page, + PageSize: opts.PageSize, + OrderBy: orderBy, + Private: opts.Private, + Keyword: strings.Trim(ctx.Query("q"), " "), + } + + if len(searchOpts.Keyword) == 0 || isKeywordValid(searchOpts.Keyword) { + repos, count, err = models.SearchRepositoryByName(searchOpts) if err != nil { - ctx.Handle(500, "opts.Ranger", err) + ctx.Handle(500, "SearchRepositoryByName", err) return } - } else { - if isKeywordValid(keyword) { - repos, count, err = models.SearchRepositoryByName(&models.SearchRepoOptions{ - Keyword: keyword, - OrderBy: orderBy, - Private: opts.Private, - Page: page, - PageSize: opts.PageSize, - Searcher: ctx.User, - Collaborate: true, - }) - if err != nil { - ctx.Handle(500, "SearchRepositoryByName", err) - return - } - } } - ctx.Data["Keyword"] = keyword + ctx.Data["Keyword"] = searchOpts.Keyword ctx.Data["Total"] = count ctx.Data["Page"] = paginater.New(int(count), opts.PageSize, page, 5) ctx.Data["Repos"] = repos @@ -155,9 +138,7 @@ func ExploreRepos(ctx *context.Context) { ctx.Data["PageIsExploreRepositories"] = true RenderRepoSearch(ctx, &RepoSearchOptions{ - Ranger: models.GetRecentUpdatedRepositories, PageSize: setting.UI.ExplorePagingNum, - Searcher: ctx.User, Private: ctx.User != nil && ctx.User.IsAdmin, TplName: tplExploreRepos, }) diff --git a/routers/user/profile.go b/routers/user/profile.go index 23f5057727ca..50d0c2397fbb 100644 --- a/routers/user/profile.go +++ b/routers/user/profile.go @@ -107,26 +107,26 @@ func Profile(ctx *context.Context) { var ( repos []*models.Repository count int64 - orderBy string + orderBy models.SearchOrderBy ) ctx.Data["SortType"] = ctx.Query("sort") switch ctx.Query("sort") { case "newest": - orderBy = "created_unix DESC" + orderBy = models.SearchOrderByNewest case "oldest": - orderBy = "created_unix ASC" + orderBy = models.SearchOrderByOldest case "recentupdate": - orderBy = "updated_unix DESC" + orderBy = models.SearchOrderByRecentUpdated case "leastupdate": - orderBy = "updated_unix ASC" + orderBy = models.SearchOrderByLeastUpdated case "reversealphabetically": - orderBy = "name DESC" + orderBy = models.SearchOrderByAlphabeticallyReverse case "alphabetically": - orderBy = "name ASC" + orderBy = models.SearchOrderByAlphabetically default: ctx.Data["SortType"] = "recentupdate" - orderBy = "updated_unix DESC" + orderBy = models.SearchOrderByNewest } // set default sort value if sort is empty. @@ -149,7 +149,7 @@ 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) + repos, err = ctxUser.GetStarredRepos(showPrivate, page, setting.UI.User.RepoPagingNum, orderBy.String()) if err != nil { ctx.Handle(500, "GetStarredRepos", err) return @@ -182,7 +182,7 @@ func Profile(ctx *context.Context) { default: if len(keyword) == 0 { var total int - repos, err = models.GetUserRepositories(ctxUser.ID, showPrivate, page, setting.UI.User.RepoPagingNum, orderBy) + repos, err = models.GetUserRepositories(ctxUser.ID, showPrivate, page, setting.UI.User.RepoPagingNum, orderBy.String()) if err != nil { ctx.Handle(500, "GetRepositories", err) return