From a50f7a8cbf9b38270afe8b080033204eac178ef9 Mon Sep 17 00:00:00 2001 From: morlinest Date: Thu, 31 Aug 2017 01:09:41 +0200 Subject: [PATCH 01/21] Add more repo search tests --- models/fixtures/access.yml | 7 +++ models/fixtures/org_user.yml | 8 +++ models/fixtures/repository.yml | 97 ++++++++++++++++++++++++++++++++ models/fixtures/team.yml | 9 +++ models/fixtures/team_repo.yml | 12 ++++ models/fixtures/team_user.yml | 6 ++ models/fixtures/user.yml | 47 ++++++++++++++++ models/org_test.go | 2 +- models/repo_list_test.go | 100 +++++++++++++++++++++++++++++++++ 9 files changed, 287 insertions(+), 1 deletion(-) diff --git a/models/fixtures/access.yml b/models/fixtures/access.yml index 8722b248db2fb..ba38c000ddab9 100644 --- a/models/fixtures/access.yml +++ b/models/fixtures/access.yml @@ -15,3 +15,10 @@ user_id: 4 repo_id: 3 mode: 2 # write + +- + id: 4 + user_id: 15 + repo_id: 22 + mode: 2 # write + diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml index a7b8d178aa13a..c7fe2cf369522 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 51812d358553b..eb83dfcff7946 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -188,3 +188,100 @@ 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 + +- + 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 + +- + 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 + +- + 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 + +- + 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 + +- + 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 + +- + 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 + +- + 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 diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index 795d5cda6c998..298de648ddf78 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: 2 + 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 1f7385e7e9c17..5154453f7b4dd 100644 --- a/models/fixtures/team_repo.yml +++ b/models/fixtures/team_repo.yml @@ -15,3 +15,15 @@ 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 \ No newline at end of file diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml index 955a80e4cdf03..8d9d9203782e4 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 abd72b1168105..e1fbc9e578bfe 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: 4 + 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: 2 + 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 07da2d56af906..c7bdb8b5d3dbb 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_test.go b/models/repo_list_test.go index 8ac0d804892da..a02e0c5abb10c 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -66,4 +66,104 @@ func TestSearchRepositoryByName(t *testing.T) { assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(3), count) + + // Get all public repositories by name + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Keyword: "big_test_", + Page: 1, + PageSize: 10, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, int64(4), count) + + // Get all public + private repositories by name + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Keyword: "big_test_", + Page: 1, + PageSize: 10, + Private: true, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, int64(8), count) + + // Get all public repositories of user + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Page: 1, + PageSize: 10, + OwnerID: 15, + Searcher: &User{ID: 15}, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, int64(3), count) + + // Get all public + private repositories of user + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Page: 1, + PageSize: 10, + OwnerID: 15, + Private: true, + Searcher: &User{ID: 15}, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, int64(6), count) + + // Get all public (including collaborative) repositories of user + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Page: 1, + PageSize: 10, + OwnerID: 15, + Collaborate: true, + Searcher: &User{ID: 15}, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, int64(3), count) + + // Get all public + private (including collaborative) repositories of user + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Page: 1, + PageSize: 10, + OwnerID: 15, + Private: true, + Collaborate: true, + Searcher: &User{ID: 15}, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, int64(7), count) + + // Get all public repositories of organization + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Page: 1, + PageSize: 10, + OwnerID: 17, + Searcher: &User{ID: 17}, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, int64(1), count) + + // Get all public + private repositories of organization + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Page: 1, + PageSize: 10, + OwnerID: 17, + Private: true, + Searcher: &User{ID: 17}, + }) + + assert.NotNil(t, repos) + assert.NoError(t, err) + assert.Equal(t, int64(2), count) } From 50867dfe7f3e97fbffe19dafae7b43f40c48422b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=BDeby?= Date: Wed, 6 Sep 2017 12:16:14 +0200 Subject: [PATCH 02/21] Add and fix /api/repo/search integration tests --- integrations/api_repo_test.go | 214 ++++++++++++++++++++++++++++++++++ models/fixtures/access.yml | 5 + models/repo_list_test.go | 8 +- 3 files changed, 221 insertions(+), 6 deletions(-) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index 8e383dbda2d7a..c13b1422e5b70 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -46,6 +46,220 @@ func TestAPISearchRepoNotLogin(t *testing.T) { assert.Contains(t, repo.Name, keyword) assert.False(t, repo.Private) } + + // Should return all (max 50) public repositories + req = NewRequest(t, "GET", "/api/v1/repos/search?limit=50") + resp = MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 12) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.False(t, repo.Private) + } + + // Should return (max 10) public repositories + req = NewRequest(t, "GET", "/api/v1/repos/search?limit=10") + resp = MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 10) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.False(t, repo.Private) + } + + const keyword2 = "big_test_" + // Should return all public repositories which (partial) match keyword + req = NewRequestf(t, "GET", "/api/v1/repos/search?q=%s", keyword2) + resp = MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 4) + for _, repo := range body.Data { + assert.Contains(t, repo.Name, keyword2) + assert.False(t, repo.Private) + } + + // Should return all public repositories accessible and related to user + const userID = int64(15) + req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", userID) + resp = MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 4) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.False(t, repo.Private) + } + + // Should return all public repositories accessible and related to user + const user2ID = int64(16) + req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", user2ID) + resp = MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 1) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.False(t, repo.Private) + } + + // Should return all public repositories owned by organization + const orgID = int64(17) + req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", orgID) + resp = MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 1) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.Equal(t, repo.Owner.ID, orgID) + assert.False(t, repo.Private) + } +} + +func TestAPISearchRepoLoggedUser(t *testing.T) { + prepareTestEnv(t) + + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 15}).(*models.User) + user2 := models.AssertExistsAndLoadBean(t, &models.User{ID: 16}).(*models.User) + session := loginUser(t, user.Name) + session2 := loginUser(t, user2.Name) + + var body api.SearchResults + + // Get public repositories accessible and not related to logged in user that match the keyword + // Should return all public repositories which (partial) match keyword + const keyword = "big_test_" + req := NewRequestf(t, "GET", "/api/v1/repos/search?q=%s", keyword) + resp := session.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 4) + for _, repo := range body.Data { + assert.Contains(t, repo.Name, keyword) + assert.False(t, repo.Private) + } + // Test when user2 is logged in + resp = session2.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 4) + for _, repo := range body.Data { + assert.Contains(t, repo.Name, keyword) + assert.False(t, repo.Private) + } + + // Get all public repositories accessible and not related to logged in user + // Should return all (max 50) public repositories + req = NewRequest(t, "GET", "/api/v1/repos/search?limit=50") + resp = session.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 12) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.False(t, repo.Private) + } + // Test when user2 is logged in + resp = session2.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 12) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.False(t, repo.Private) + } + + // Get all public repositories accessible and not related to logged in user + // Should return all (max 10) public repositories + req = NewRequest(t, "GET", "/api/v1/repos/search?limit=10") + resp = session.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 10) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.False(t, repo.Private) + } + // Test when user2 is logged in + resp = session2.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 10) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.False(t, repo.Private) + } + + // Get repositories of logged in user + // Should return all public and private repositories accessible and related to user + req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", user.ID) + resp = session.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 8) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + } + // Test when user2 is logged in + req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", user2.ID) + resp = session2.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 2) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + } + + // Get repositories of another user + // Should return all public repositories accessible and related to user + req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", user2.ID) + resp = session.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 1) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.False(t, repo.Private) + } + // Test when user2 is logged in + req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", user.ID) + resp = session2.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 4) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.False(t, repo.Private) + } + + // Get repositories of organization owned by logged in user + // Should return all public and private repositories owned by organization + const orgID = int64(17) + req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", orgID) + resp = session.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 2) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.Equal(t, repo.Owner.ID, orgID) + } + + // Get repositories of organization owned by another user + // Should return all public repositories owned by organization + req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", orgID) + resp = session2.MakeRequest(t, req, http.StatusOK) + + DecodeJSON(t, resp, &body) + assert.Len(t, body.Data, 1) + for _, repo := range body.Data { + assert.NotEmpty(t, repo.Name) + assert.Equal(t, repo.Owner.ID, orgID) + assert.False(t, repo.Private) + } } func TestAPIViewRepo(t *testing.T) { diff --git a/models/fixtures/access.yml b/models/fixtures/access.yml index ba38c000ddab9..2c8a31d6bc730 100644 --- a/models/fixtures/access.yml +++ b/models/fixtures/access.yml @@ -22,3 +22,8 @@ repo_id: 22 mode: 2 # write +- + id: 5 + user_id: 15 + repo_id: 21 + mode: 2 # write diff --git a/models/repo_list_test.go b/models/repo_list_test.go index a02e0c5abb10c..35cf6214ac971 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -18,7 +18,6 @@ func TestSearchRepositoryByName(t *testing.T) { Keyword: "repo_12", Page: 1, PageSize: 10, - Searcher: nil, }) assert.NotNil(t, repos) @@ -32,7 +31,6 @@ func TestSearchRepositoryByName(t *testing.T) { Keyword: "test_repo", Page: 1, PageSize: 10, - Searcher: nil, }) assert.NotNil(t, repos) @@ -45,7 +43,6 @@ func TestSearchRepositoryByName(t *testing.T) { Page: 1, PageSize: 10, Private: true, - Searcher: &User{ID: 14}, }) assert.NotNil(t, repos) @@ -60,7 +57,6 @@ func TestSearchRepositoryByName(t *testing.T) { Page: 1, PageSize: 10, Private: true, - Searcher: &User{ID: 14}, }) assert.NotNil(t, repos) @@ -126,7 +122,7 @@ func TestSearchRepositoryByName(t *testing.T) { assert.NotNil(t, repos) assert.NoError(t, err) - assert.Equal(t, int64(3), count) + assert.Equal(t, int64(4), count) // Get all public + private (including collaborative) repositories of user repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -140,7 +136,7 @@ func TestSearchRepositoryByName(t *testing.T) { assert.NotNil(t, repos) assert.NoError(t, err) - assert.Equal(t, int64(7), count) + assert.Equal(t, int64(8), count) // Get all public repositories of organization repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ From 2d8b046ea0f7383a15a545d207d714ce04d69d14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=BDeby?= Date: Wed, 6 Sep 2017 11:17:16 +0200 Subject: [PATCH 03/21] Add owner to access table for organization repositories --- models/fixtures/access.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/models/fixtures/access.yml b/models/fixtures/access.yml index 2c8a31d6bc730..34d5c5c59405c 100644 --- a/models/fixtures/access.yml +++ b/models/fixtures/access.yml @@ -27,3 +27,15 @@ 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 \ No newline at end of file From 27f037d321826dbe4c85592975e10ecddee3a3f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=BDeby?= Date: Mon, 4 Sep 2017 11:09:40 +0200 Subject: [PATCH 04/21] Fix repo search tests --- models/repo_list_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/repo_list_test.go b/models/repo_list_test.go index 35cf6214ac971..81f66cf6fb58d 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -96,7 +96,7 @@ func TestSearchRepositoryByName(t *testing.T) { assert.NotNil(t, repos) assert.NoError(t, err) - assert.Equal(t, int64(3), count) + assert.Equal(t, int64(2), count) // Get all public + private repositories of user repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -109,7 +109,7 @@ func TestSearchRepositoryByName(t *testing.T) { assert.NotNil(t, repos) assert.NoError(t, err) - assert.Equal(t, int64(6), count) + assert.Equal(t, int64(4), count) // Get all public (including collaborative) repositories of user repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ From 4fcbded80930fe562f33482b2e1d368cb8ac854f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=BDeby?= Date: Mon, 4 Sep 2017 11:17:32 +0200 Subject: [PATCH 05/21] Always test returned repos length --- models/repo_list_test.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/models/repo_list_test.go b/models/repo_list_test.go index 81f66cf6fb58d..b48e7bf6c9c29 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -20,7 +20,6 @@ func TestSearchRepositoryByName(t *testing.T) { PageSize: 10, }) - assert.NotNil(t, repos) assert.NoError(t, err) if assert.Len(t, repos, 1) { assert.Equal(t, "test_repo_12", repos[0].Name) @@ -33,9 +32,9 @@ func TestSearchRepositoryByName(t *testing.T) { PageSize: 10, }) - 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,7 +44,6 @@ func TestSearchRepositoryByName(t *testing.T) { Private: true, }) - assert.NotNil(t, repos) assert.NoError(t, err) if assert.Len(t, repos, 1) { assert.Equal(t, "test_repo_13", repos[0].Name) @@ -59,9 +57,9 @@ func TestSearchRepositoryByName(t *testing.T) { Private: true, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(3), count) + assert.Len(t, repos, 3) // Get all public repositories by name repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -70,9 +68,9 @@ func TestSearchRepositoryByName(t *testing.T) { PageSize: 10, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(4), count) + assert.Len(t, repos, 4) // Get all public + private repositories by name repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -82,9 +80,9 @@ func TestSearchRepositoryByName(t *testing.T) { Private: true, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(8), count) + assert.Len(t, repos, 8) // Get all public repositories of user repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -94,9 +92,9 @@ func TestSearchRepositoryByName(t *testing.T) { Searcher: &User{ID: 15}, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(2), count) + assert.Len(t, repos, 2) // Get all public + private repositories of user repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -107,9 +105,9 @@ func TestSearchRepositoryByName(t *testing.T) { Searcher: &User{ID: 15}, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(4), count) + assert.Len(t, repos, 4) // Get all public (including collaborative) repositories of user repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -120,9 +118,9 @@ func TestSearchRepositoryByName(t *testing.T) { Searcher: &User{ID: 15}, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(4), count) + assert.Len(t, repos, 4) // Get all public + private (including collaborative) repositories of user repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -134,9 +132,9 @@ func TestSearchRepositoryByName(t *testing.T) { Searcher: &User{ID: 15}, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(8), count) + assert.Len(t, repos, 8) // Get all public repositories of organization repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -146,9 +144,9 @@ func TestSearchRepositoryByName(t *testing.T) { Searcher: &User{ID: 17}, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(1), count) + assert.Len(t, repos, 1) // Get all public + private repositories of organization repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ @@ -159,7 +157,7 @@ func TestSearchRepositoryByName(t *testing.T) { Searcher: &User{ID: 17}, }) - assert.NotNil(t, repos) assert.NoError(t, err) assert.Equal(t, int64(2), count) + assert.Len(t, repos, 2) } From 8cb7ce2afb733bf83a76d38ab1719547247fc677 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=BDeby?= Date: Mon, 4 Sep 2017 11:20:23 +0200 Subject: [PATCH 06/21] Add test with lower pagesize limit (test more pages) --- models/repo_list_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/models/repo_list_test.go b/models/repo_list_test.go index b48e7bf6c9c29..bf5b708744a38 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -84,6 +84,30 @@ func TestSearchRepositoryByName(t *testing.T) { assert.Equal(t, int64(8), count) assert.Len(t, repos, 8) + // Get all public + private repositories by name with pagesize limit (first page) + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Keyword: "big_test_", + Page: 1, + PageSize: 5, + Private: true, + }) + + assert.NoError(t, err) + assert.Equal(t, int64(8), count) + assert.Len(t, repos, 5) + + // Get all public + private repositories by name with pagesize limit (second page) + repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ + Keyword: "big_test_", + Page: 2, + PageSize: 5, + Private: true, + }) + + assert.NoError(t, err) + assert.Equal(t, int64(8), count) + assert.Len(t, repos, 3) + // Get all public repositories of user repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ Page: 1, From dae998e50b9781db60d640eda40ae475c0333268 Mon Sep 17 00:00:00 2001 From: morlinest Date: Fri, 25 Aug 2017 20:33:54 +0200 Subject: [PATCH 07/21] Fix/rewrite searchRepositoryByName function --- models/repo_list.go | 114 ++++++++++++++++++++++++++++---------------- 1 file changed, 72 insertions(+), 42 deletions(-) diff --git a/models/repo_list.go b/models/repo_list.go index 15e6144d06366..9d4092d9d0c86 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -114,53 +114,91 @@ type SearchRepoOptions struct { // 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) { + // Set owner of searched repositories if present in options + searchOwnerID := opts.OwnerID + + // Set owner ID to Searcher ID if Owner is not filled + if opts.OwnerID <= 0 && opts.Searcher != nil { + searchOwnerID = opts.Searcher.ID + } + // Check if user with Owner ID exists + if searchOwnerID > 0 { + userExists, err := GetUser(&User{ID: searchOwnerID}) + if err != nil { + return nil, 0, err + } + if userExists == false { + return nil, 0, ErrUserNotExist{UID: searchOwnerID} + } + } + + // Set public search by default + isPublicSearch := true + + // Allow to search for owners private data + if opts.Searcher != nil && (opts.Searcher.ID == searchOwnerID || opts.Searcher.IsAdmin) { + isPublicSearch = false + } + + // Check and set page to correct number if opts.Page <= 0 { opts.Page = 1 } - var starJoin bool - if opts.Starred && opts.OwnerID > 0 { + var cond = builder.NewCond() + + // Include starred repositories by Owner + if opts.Starred && searchOwnerID > 0 { cond = builder.Eq{ - "star.uid": opts.OwnerID, + "star.uid": searchOwnerID, } - starJoin = true } - 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 + // Exclude private repositories + // if it is set in options + // or is public search + if !opts.Private || isPublicSearch { + cond = cond.And(builder.Eq{"is_private": false}) + } + + if searchOwnerID > 0 { + // Set user access conditions + var accessCond = builder.NewCond() - ownerIds = append(ownerIds, opts.Searcher.ID) - err = opts.Searcher.GetOrganizations(true) + // Add Owner ID to access conditions + accessCond = builder.Eq{"owner_id": searchOwnerID} + + // Include collaborative repositories + if opts.Collaborate { + // Get owner organizations + orgs, err := GetOrgUsersByUserID(searchOwnerID, !isPublicSearch) if err != nil { return nil, 0, fmt.Errorf("Organization: %v", err) } - for _, org := range opts.Searcher.Orgs { - ownerIds = append(ownerIds, org.ID) + var ownerIds []int64 + for _, org := range orgs { + ownerIds = append(ownerIds, org.OrgID) } - 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)) - } + // Add repositories from related organizations + accessCond = accessCond.Or(builder.In("owner_id", ownerIds)) + + // Add repositories where user is set as collaborator directly + accessCond = accessCond.Or(builder.Expr("id IN (SELECT repo_id FROM `access` WHERE access.user_id = ? AND owner_id != ?)", + searchOwnerID, searchOwnerID)) } - cond = cond.And(searcherReposCond) - } - if !opts.Private { - cond = cond.And(builder.Eq{"is_private": false}) + // Add user access conditions to search + cond = cond.And(accessCond) } if len(opts.OrderBy) == 0 { @@ -170,23 +208,15 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, coun 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 opts.Starred { + sess = sess.Join("INNER", "star", "star.repo_id = repository.id") + } - 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) - } + 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) @@ -204,7 +234,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, coun } } - return + return repos, count, nil } // Repositories returns all repositories From 7c9323dc1fbefe316dff5673da22e80080306fa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=BDeby?= Date: Thu, 24 Aug 2017 10:28:17 +0200 Subject: [PATCH 08/21] Remove authorization decisions making --- models/repo_list.go | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/models/repo_list.go b/models/repo_list.go index 9d4092d9d0c86..7ac2bc99918bf 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -128,19 +128,11 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in if err != nil { return nil, 0, err } - if userExists == false { + if !userExists { return nil, 0, ErrUserNotExist{UID: searchOwnerID} } } - // Set public search by default - isPublicSearch := true - - // Allow to search for owners private data - if opts.Searcher != nil && (opts.Searcher.ID == searchOwnerID || opts.Searcher.IsAdmin) { - isPublicSearch = false - } - // Check and set page to correct number if opts.Page <= 0 { opts.Page = 1 @@ -162,9 +154,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in } // Exclude private repositories - // if it is set in options - // or is public search - if !opts.Private || isPublicSearch { + if !opts.Private { cond = cond.And(builder.Eq{"is_private": false}) } @@ -178,7 +168,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in // Include collaborative repositories if opts.Collaborate { // Get owner organizations - orgs, err := GetOrgUsersByUserID(searchOwnerID, !isPublicSearch) + orgs, err := GetOrgUsersByUserID(searchOwnerID, true) if err != nil { return nil, 0, fmt.Errorf("Organization: %v", err) From 9da9e960a641bfacb6781e871882b4c731644fe1 Mon Sep 17 00:00:00 2001 From: morlinest Date: Thu, 24 Aug 2017 22:04:14 +0200 Subject: [PATCH 09/21] Fix search of starred repositories --- models/repo_list.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/models/repo_list.go b/models/repo_list.go index 7ac2bc99918bf..605a34aeaca52 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -141,7 +141,9 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in var cond = builder.NewCond() // Include starred repositories by Owner + includeStarred := false if opts.Starred && searchOwnerID > 0 { + includeStarred = true cond = builder.Eq{ "star.uid": searchOwnerID, } @@ -198,7 +200,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in sess := x.NewSession() defer sess.Close() - if opts.Starred { + if includeStarred { sess = sess.Join("INNER", "star", "star.repo_id = repository.id") } @@ -209,6 +211,11 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in return nil, 0, fmt.Errorf("Count: %v", err) } + // Set again after reset by Count() + if includeStarred { + sess = sess.Join("INNER", "star", "star.repo_id = repository.id") + } + repos = make([]*Repository, 0, opts.PageSize) if err = sess. Where(cond). From 14f73a3795f759ed2d81bbd99290a590c72903d5 Mon Sep 17 00:00:00 2001 From: morlinest Date: Thu, 24 Aug 2017 22:28:37 +0200 Subject: [PATCH 10/21] Simplify code --- models/repo_list.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/models/repo_list.go b/models/repo_list.go index 605a34aeaca52..59c24dc7e74e6 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -162,10 +162,8 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in if searchOwnerID > 0 { // Set user access conditions - var accessCond = builder.NewCond() - // Add Owner ID to access conditions - accessCond = builder.Eq{"owner_id": searchOwnerID} + var accessCond builder.Cond = builder.Eq{"owner_id": searchOwnerID} // Include collaborative repositories if opts.Collaborate { @@ -201,7 +199,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in defer sess.Close() if includeStarred { - sess = sess.Join("INNER", "star", "star.repo_id = repository.id") + sess.Join("INNER", "star", "star.repo_id = repository.id") } count, err := sess. @@ -213,7 +211,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in // Set again after reset by Count() if includeStarred { - sess = sess.Join("INNER", "star", "star.repo_id = repository.id") + sess.Join("INNER", "star", "star.repo_id = repository.id") } repos = make([]*Repository, 0, opts.PageSize) From 3982925c88ddeda62f04c2159b3298b44a30112c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=BDeby?= Date: Fri, 25 Aug 2017 16:31:17 +0200 Subject: [PATCH 11/21] Remove "Searcher" option from SearchRepoOptions --- models/repo_list.go | 90 +++++++++++++++++-------------------- models/repo_list_test.go | 6 --- routers/api/v1/repo/repo.go | 3 -- routers/home.go | 29 +++++------- 4 files changed, 51 insertions(+), 77 deletions(-) diff --git a/models/repo_list.go b/models/repo_list.go index 59c24dc7e74e6..710d161de0656 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -98,7 +98,6 @@ type SearchRepoOptions struct { // // 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 @@ -115,21 +114,14 @@ type SearchRepoOptions struct { // 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, _ int64, _ error) { - // Set owner of searched repositories if present in options - searchOwnerID := opts.OwnerID - - // Set owner ID to Searcher ID if Owner is not filled - if opts.OwnerID <= 0 && opts.Searcher != nil { - searchOwnerID = opts.Searcher.ID - } // Check if user with Owner ID exists - if searchOwnerID > 0 { - userExists, err := GetUser(&User{ID: searchOwnerID}) + 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: searchOwnerID} + return nil, 0, ErrUserNotExist{UID: opts.OwnerID} } } @@ -140,15 +132,6 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in var cond = builder.NewCond() - // Include starred repositories by Owner - includeStarred := false - if opts.Starred && searchOwnerID > 0 { - includeStarred = true - cond = builder.Eq{ - "star.uid": searchOwnerID, - } - } - // Add repository name keyword to search for if opts.Keyword != "" { opts.Keyword = strings.ToLower(opts.Keyword) @@ -160,35 +143,44 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in cond = cond.And(builder.Eq{"is_private": false}) } - if searchOwnerID > 0 { - // Set user access conditions - // Add Owner ID to access conditions - var accessCond builder.Cond = builder.Eq{"owner_id": searchOwnerID} - - // Include collaborative repositories - if opts.Collaborate { - // Get owner organizations - orgs, err := GetOrgUsersByUserID(searchOwnerID, true) - - 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, } - - var ownerIds []int64 - for _, org := range orgs { - ownerIds = append(ownerIds, org.OrgID) + } else { + // Set user access conditions + // Add Owner ID to access conditions + var accessCond builder.Cond = builder.Eq{"owner_id": opts.OwnerID} + + // Include collaborative repositories + if opts.Collaborate { + // Get owner organizations + orgs, err := GetOrgUsersByUserID(opts.OwnerID, opts.Private) + + if err != nil { + return nil, 0, fmt.Errorf("Organization: %v", err) + } + + var ownerIds []int64 + for _, org := range orgs { + ownerIds = append(ownerIds, org.OrgID) + } + + // Add repositories from related organizations + accessCond = accessCond.Or(builder.In("owner_id", ownerIds)) + + // Add repositories where user is set as collaborator directly + accessCond = accessCond.Or(builder.Expr("id IN (SELECT repo_id FROM `access` WHERE access.user_id = ? AND owner_id != ?)", + opts.OwnerID, opts.OwnerID)) } - // Add repositories from related organizations - accessCond = accessCond.Or(builder.In("owner_id", ownerIds)) - - // Add repositories where user is set as collaborator directly - accessCond = accessCond.Or(builder.Expr("id IN (SELECT repo_id FROM `access` WHERE access.user_id = ? AND owner_id != ?)", - searchOwnerID, searchOwnerID)) + // Add user access conditions to search + cond = cond.And(accessCond) } - - // Add user access conditions to search - cond = cond.And(accessCond) } if len(opts.OrderBy) == 0 { @@ -270,17 +262,17 @@ func GetRecentUpdatedRepositories(opts *SearchRepoOptions) (repos RepositoryList } } - if opts.Searcher != nil && !opts.Searcher.IsAdmin { + if opts.OwnerID > 0 && opts.Collaborate { var ownerIds []int64 - ownerIds = append(ownerIds, opts.Searcher.ID) - err := opts.Searcher.GetOrganizations(true) + ownerIds = append(ownerIds, opts.OwnerID) + orgs, err := GetOrgUsersByUserID(opts.OwnerID, opts.Private) if err != nil { return nil, 0, fmt.Errorf("Organization: %v", err) } - for _, org := range opts.Searcher.Orgs { + for _, org := range orgs { ownerIds = append(ownerIds, org.ID) } diff --git a/models/repo_list_test.go b/models/repo_list_test.go index bf5b708744a38..04f8bbaa28824 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -113,7 +113,6 @@ func TestSearchRepositoryByName(t *testing.T) { Page: 1, PageSize: 10, OwnerID: 15, - Searcher: &User{ID: 15}, }) assert.NoError(t, err) @@ -126,7 +125,6 @@ func TestSearchRepositoryByName(t *testing.T) { PageSize: 10, OwnerID: 15, Private: true, - Searcher: &User{ID: 15}, }) assert.NoError(t, err) @@ -139,7 +137,6 @@ func TestSearchRepositoryByName(t *testing.T) { PageSize: 10, OwnerID: 15, Collaborate: true, - Searcher: &User{ID: 15}, }) assert.NoError(t, err) @@ -153,7 +150,6 @@ func TestSearchRepositoryByName(t *testing.T) { OwnerID: 15, Private: true, Collaborate: true, - Searcher: &User{ID: 15}, }) assert.NoError(t, err) @@ -165,7 +161,6 @@ func TestSearchRepositoryByName(t *testing.T) { Page: 1, PageSize: 10, OwnerID: 17, - Searcher: &User{ID: 17}, }) assert.NoError(t, err) @@ -178,7 +173,6 @@ func TestSearchRepositoryByName(t *testing.T) { PageSize: 10, OwnerID: 17, Private: true, - Searcher: &User{ID: 17}, }) assert.NoError(t, err) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 20393102fc825..ee71936e6825b 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -34,9 +34,6 @@ func Search(ctx *context.APIContext) { OwnerID: ctx.QueryInt64("uid"), PageSize: convert.ToCorrectPageSize(ctx.QueryInt("limit")), } - if ctx.User != nil && ctx.User.ID == opts.OwnerID { - opts.Searcher = ctx.User - } // Check visibility. if ctx.IsSigned && opts.OwnerID > 0 { diff --git a/routers/home.go b/routers/home.go index 16d0720551c40..f0866ed4ecae1 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 @@ -109,31 +108,24 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) { orderBy = "created_unix DESC" } + searchOpts := &models.SearchRepoOptions{ + Page: page, + PageSize: opts.PageSize, + OrderBy: orderBy, + Private: opts.Private, + } + 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, - }) + repos, count, err = opts.Ranger(searchOpts) if err != nil { ctx.Handle(500, "opts.Ranger", 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, - }) + searchOpts.Keyword = keyword + repos, count, err = models.SearchRepositoryByName(searchOpts) if err != nil { ctx.Handle(500, "SearchRepositoryByName", err) return @@ -157,7 +149,6 @@ func ExploreRepos(ctx *context.Context) { RenderRepoSearch(ctx, &RepoSearchOptions{ Ranger: models.GetRecentUpdatedRepositories, PageSize: setting.UI.ExplorePagingNum, - Searcher: ctx.User, Private: ctx.User != nil && ctx.User.IsAdmin, TplName: tplExploreRepos, }) From 41d483876e1bb9d3a035c86c09fe05a80424d5f3 Mon Sep 17 00:00:00 2001 From: morlinest Date: Fri, 25 Aug 2017 23:54:21 +0200 Subject: [PATCH 12/21] Fix visibility of organizations repositories --- models/repo_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo_list.go b/models/repo_list.go index 710d161de0656..407c7cba1dfa8 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -171,7 +171,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in } // Add repositories from related organizations - accessCond = accessCond.Or(builder.In("owner_id", ownerIds)) + accessCond = accessCond.Or(builder.And(builder.In("owner_id", ownerIds), builder.Eq{"is_private": false})) // Add repositories where user is set as collaborator directly accessCond = accessCond.Or(builder.Expr("id IN (SELECT repo_id FROM `access` WHERE access.user_id = ? AND owner_id != ?)", From 886033f15f4e6a06ebfc7f9b1b4493d291e6e4d9 Mon Sep 17 00:00:00 2001 From: morlinest Date: Sat, 26 Aug 2017 02:45:33 +0200 Subject: [PATCH 13/21] Simplify searching --- models/repo_list.go | 90 +++++++++++++---------------------------- routers/home.go | 38 +++++++---------- routers/user/profile.go | 20 ++++----- 3 files changed, 53 insertions(+), 95 deletions(-) diff --git a/models/repo_list.go b/models/repo_list.go index 407c7cba1dfa8..6285849dd64da 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -97,13 +97,13 @@ type SearchRepoOptions struct { // Owner in we search search // // in: query - OwnerID int64 `json:"uid"` - 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 @@ -111,6 +111,25 @@ type SearchRepoOptions struct { PageSize int `json:"limit"` // Can be smaller than or equal to setting.ExplorePagingNum } +//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, _ int64, _ error) { @@ -184,7 +203,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in } if len(opts.OrderBy) == 0 { - opts.OrderBy = "name ASC" + opts.OrderBy = SearchOrderByAlphabetically } sess := x.NewSession() @@ -210,7 +229,7 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in 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) } @@ -234,7 +253,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) } @@ -247,54 +266,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.OwnerID > 0 && opts.Collaborate { - var ownerIds []int64 - - ownerIds = append(ownerIds, opts.OwnerID) - orgs, err := GetOrgUsersByUserID(opts.OwnerID, opts.Private) - - if err != nil { - return nil, 0, fmt.Errorf("Organization: %v", err) - } - - for _, org := range 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/routers/home.go b/routers/home.go index f0866ed4ecae1..13060d716ca8d 100644 --- a/routers/home.go +++ b/routers/home.go @@ -85,27 +85,27 @@ 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 } searchOpts := &models.SearchRepoOptions{ @@ -113,26 +113,17 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) { PageSize: opts.PageSize, OrderBy: orderBy, Private: opts.Private, + Keyword: strings.Trim(ctx.Query("q"), " "), } - keyword := strings.Trim(ctx.Query("q"), " ") - if len(keyword) == 0 { - repos, count, err = opts.Ranger(searchOpts) + 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) { - searchOpts.Keyword = keyword - repos, count, err = models.SearchRepositoryByName(searchOpts) - 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 @@ -147,7 +138,6 @@ func ExploreRepos(ctx *context.Context) { ctx.Data["PageIsExploreRepositories"] = true RenderRepoSearch(ctx, &RepoSearchOptions{ - Ranger: models.GetRecentUpdatedRepositories, PageSize: setting.UI.ExplorePagingNum, Private: ctx.User != nil && ctx.User.IsAdmin, TplName: tplExploreRepos, diff --git a/routers/user/profile.go b/routers/user/profile.go index 23f5057727cab..50d0c2397fbbb 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 From c091ee35acaff36f1f3d10ca5846ee393729c134 Mon Sep 17 00:00:00 2001 From: morlinest Date: Mon, 4 Sep 2017 00:27:29 +0200 Subject: [PATCH 14/21] Fix /api/repo/search checks --- routers/api/v1/repo/repo.go | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index ee71936e6825b..eef2d0604c91f 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -35,27 +35,24 @@ func Search(ctx *context.APIContext) { PageSize: convert.ToCorrectPageSize(ctx.QueryInt("limit")), } - // 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 } } From 4af3945f4331609d008070331cc5c02effda3f47 Mon Sep 17 00:00:00 2001 From: morlinest Date: Sun, 3 Sep 2017 23:36:01 +0200 Subject: [PATCH 15/21] Remove redudant condition --- models/repo_list.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/models/repo_list.go b/models/repo_list.go index 6285849dd64da..5542001c5cce5 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -177,21 +177,6 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in // Include collaborative repositories if opts.Collaborate { - // Get owner organizations - orgs, err := GetOrgUsersByUserID(opts.OwnerID, opts.Private) - - if err != nil { - return nil, 0, fmt.Errorf("Organization: %v", err) - } - - var ownerIds []int64 - for _, org := range orgs { - ownerIds = append(ownerIds, org.OrgID) - } - - // Add repositories from related organizations - accessCond = accessCond.Or(builder.And(builder.In("owner_id", ownerIds), builder.Eq{"is_private": false})) - // Add repositories where user is set as collaborator directly accessCond = accessCond.Or(builder.Expr("id IN (SELECT repo_id FROM `access` WHERE access.user_id = ? AND owner_id != ?)", opts.OwnerID, opts.OwnerID)) From 11ef70a4ed37bacb01bad9d73d1fef5e0fe4aeef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=BDeby?= Date: Fri, 8 Sep 2017 14:25:57 +0200 Subject: [PATCH 16/21] Fix access condition expression --- models/repo_list.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/models/repo_list.go b/models/repo_list.go index 5542001c5cce5..1e2fa8c6693da 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -178,8 +178,9 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in // Include collaborative repositories if opts.Collaborate { // Add repositories where user is set as collaborator directly - accessCond = accessCond.Or(builder.Expr("id IN (SELECT repo_id FROM `access` WHERE access.user_id = ? AND owner_id != ?)", - opts.OwnerID, opts.OwnerID)) + 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 From 76e3d48a300942828d5ad85184003f9f79661f15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=BDeby?= Date: Thu, 14 Sep 2017 19:39:57 +0200 Subject: [PATCH 17/21] Simplify TestSearchRepositoryByName code --- models/repo_list_test.go | 168 ++++++++++++--------------------------- 1 file changed, 52 insertions(+), 116 deletions(-) diff --git a/models/repo_list_test.go b/models/repo_list_test.go index 04f8bbaa28824..af14166101891 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -61,121 +61,57 @@ func TestSearchRepositoryByName(t *testing.T) { assert.Equal(t, int64(3), count) assert.Len(t, repos, 3) - // Get all public repositories by name - repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ - Keyword: "big_test_", - Page: 1, - PageSize: 10, - }) - - assert.NoError(t, err) - assert.Equal(t, int64(4), count) - assert.Len(t, repos, 4) - - // Get all public + private repositories by name - repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ - Keyword: "big_test_", - Page: 1, - PageSize: 10, - Private: true, - }) - - assert.NoError(t, err) - assert.Equal(t, int64(8), count) - assert.Len(t, repos, 8) - - // Get all public + private repositories by name with pagesize limit (first page) - repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ - Keyword: "big_test_", - Page: 1, - PageSize: 5, - Private: true, - }) - - assert.NoError(t, err) - assert.Equal(t, int64(8), count) - assert.Len(t, repos, 5) - - // Get all public + private repositories by name with pagesize limit (second page) - repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ - Keyword: "big_test_", - Page: 2, - PageSize: 5, - Private: true, - }) - - assert.NoError(t, err) - assert.Equal(t, int64(8), count) - assert.Len(t, repos, 3) - - // Get all public repositories of user - repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ - Page: 1, - PageSize: 10, - OwnerID: 15, - }) - - assert.NoError(t, err) - assert.Equal(t, int64(2), count) - assert.Len(t, repos, 2) - - // Get all public + private repositories of user - repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ - Page: 1, - PageSize: 10, - OwnerID: 15, - Private: true, - }) - - assert.NoError(t, err) - assert.Equal(t, int64(4), count) - assert.Len(t, repos, 4) - - // Get all public (including collaborative) repositories of user - repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ - Page: 1, - PageSize: 10, - OwnerID: 15, - Collaborate: true, - }) - - assert.NoError(t, err) - assert.Equal(t, int64(4), count) - assert.Len(t, repos, 4) - - // Get all public + private (including collaborative) repositories of user - repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ - Page: 1, - PageSize: 10, - OwnerID: 15, - Private: true, - Collaborate: true, - }) - - assert.NoError(t, err) - assert.Equal(t, int64(8), count) - assert.Len(t, repos, 8) - - // Get all public repositories of organization - repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ - Page: 1, - PageSize: 10, - OwnerID: 17, - }) - - assert.NoError(t, err) - assert.Equal(t, int64(1), count) - assert.Len(t, repos, 1) - - // Get all public + private repositories of organization - repos, count, err = SearchRepositoryByName(&SearchRepoOptions{ - Page: 1, - PageSize: 10, - OwnerID: 17, - Private: true, - }) + testCases := []struct { + name string + opts *SearchRepoOptions + count int + }{ + {name: "PublicRepositoriesByName", + opts: &SearchRepoOptions{Keyword: "big_test_", Page: 1, PageSize: 10}, + count: 4}, + {name: "PublicAndPrivateRepositoriesByName", + opts: &SearchRepoOptions{Keyword: "big_test_", Page: 1, PageSize: 10, Private: true}, + count: 8}, + {name: "PublicAndPrivateRepositoriesByNameWithPagesizeLimitFirstPage", + opts: &SearchRepoOptions{Keyword: "big_test_", Page: 1, PageSize: 5, Private: true}, + count: 8}, + {name: "PublicAndPrivateRepositoriesByNameWithPagesizeLimitSecondPage", + opts: &SearchRepoOptions{Keyword: "big_test_", Page: 2, PageSize: 5, Private: true}, + count: 8}, + {name: "PublicRepositoriesOfUser", + opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15}, + count: 2}, + {name: "PublicAndPrivateRepositoriesOfUser", + opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Private: true}, + count: 4}, + {name: "PublicRepositoriesOfUserIncludingCollaborative", + opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Collaborate: true}, + count: 4}, + {name: "PublicAndPrivateRepositoriesOfUserIncludingCollaborative", + opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Private: true, Collaborate: true}, + count: 8}, + {name: "PublicRepositoriesOfOrganization", + opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 17}, + count: 1}, + {name: "PublicAndPrivateRepositoriesOfOrganization", + opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 17, Private: true}, + count: 2}, + } - assert.NoError(t, err) - assert.Equal(t, int64(2), count) - assert.Len(t, repos, 2) + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + repos, count, err := SearchRepositoryByName(testCase.opts) + + assert.NoError(t, err) + assert.Equal(t, int64(testCase.count), count) + + var expectedLen int + if testCase.opts.PageSize*testCase.opts.Page > testCase.count { + expectedLen = testCase.count % testCase.opts.PageSize + } else { + expectedLen = testCase.opts.PageSize + } + assert.Len(t, repos, expectedLen) + }) + } } From ce0fe4eea3dadb3d37a6d66182dbbb76bb3c8938 Mon Sep 17 00:00:00 2001 From: morlinest Date: Thu, 14 Sep 2017 23:38:16 +0200 Subject: [PATCH 18/21] Simplify and unify TestAPISearchRepo code --- integrations/api_repo_test.go | 314 +++++++++++----------------------- 1 file changed, 103 insertions(+), 211 deletions(-) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index c13b1422e5b70..a917b2f6c0f4e 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" @@ -47,218 +48,109 @@ func TestAPISearchRepoNotLogin(t *testing.T) { assert.False(t, repo.Private) } - // Should return all (max 50) public repositories - req = NewRequest(t, "GET", "/api/v1/repos/search?limit=50") - resp = MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 12) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.False(t, repo.Private) - } - - // Should return (max 10) public repositories - req = NewRequest(t, "GET", "/api/v1/repos/search?limit=10") - resp = MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 10) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.False(t, repo.Private) - } - - const keyword2 = "big_test_" - // Should return all public repositories which (partial) match keyword - req = NewRequestf(t, "GET", "/api/v1/repos/search?q=%s", keyword2) - resp = MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 4) - for _, repo := range body.Data { - assert.Contains(t, repo.Name, keyword2) - assert.False(t, repo.Private) - } - - // Should return all public repositories accessible and related to user - const userID = int64(15) - req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", userID) - resp = MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 4) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.False(t, repo.Private) - } - - // Should return all public repositories accessible and related to user - const user2ID = int64(16) - req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", user2ID) - resp = MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 1) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.False(t, repo.Private) - } - - // Should return all public repositories owned by organization - const orgID = int64(17) - req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", orgID) - resp = MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 1) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.Equal(t, repo.Owner.ID, orgID) - assert.False(t, repo.Private) - } -} - -func TestAPISearchRepoLoggedUser(t *testing.T) { - prepareTestEnv(t) - user := models.AssertExistsAndLoadBean(t, &models.User{ID: 15}).(*models.User) user2 := models.AssertExistsAndLoadBean(t, &models.User{ID: 16}).(*models.User) - session := loginUser(t, user.Name) - session2 := loginUser(t, user2.Name) - - var body api.SearchResults - - // Get public repositories accessible and not related to logged in user that match the keyword - // Should return all public repositories which (partial) match keyword - const keyword = "big_test_" - req := NewRequestf(t, "GET", "/api/v1/repos/search?q=%s", keyword) - resp := session.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 4) - for _, repo := range body.Data { - assert.Contains(t, repo.Name, keyword) - assert.False(t, repo.Private) - } - // Test when user2 is logged in - resp = session2.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 4) - for _, repo := range body.Data { - assert.Contains(t, repo.Name, keyword) - assert.False(t, repo.Private) - } - - // Get all public repositories accessible and not related to logged in user - // Should return all (max 50) public repositories - req = NewRequest(t, "GET", "/api/v1/repos/search?limit=50") - resp = session.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 12) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.False(t, repo.Private) - } - // Test when user2 is logged in - resp = session2.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 12) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.False(t, repo.Private) - } - - // Get all public repositories accessible and not related to logged in user - // Should return all (max 10) public repositories - req = NewRequest(t, "GET", "/api/v1/repos/search?limit=10") - resp = session.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 10) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.False(t, repo.Private) - } - // Test when user2 is logged in - resp = session2.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 10) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.False(t, repo.Private) - } - - // Get repositories of logged in user - // Should return all public and private repositories accessible and related to user - req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", user.ID) - resp = session.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 8) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - } - // Test when user2 is logged in - req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", user2.ID) - resp = session2.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 2) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - } - - // Get repositories of another user - // Should return all public repositories accessible and related to user - req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", user2.ID) - resp = session.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 1) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.False(t, repo.Private) - } - // Test when user2 is logged in - req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", user.ID) - resp = session2.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 4) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.False(t, repo.Private) - } - - // Get repositories of organization owned by logged in user - // Should return all public and private repositories owned by organization - const orgID = int64(17) - req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", orgID) - resp = session.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 2) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.Equal(t, repo.Owner.ID, orgID) - } - - // Get repositories of organization owned by another user - // Should return all public repositories owned by organization - req = NewRequestf(t, "GET", "/api/v1/repos/search?uid=%d", orgID) - resp = session2.MakeRequest(t, req, http.StatusOK) - - DecodeJSON(t, resp, &body) - assert.Len(t, body.Data, 1) - for _, repo := range body.Data { - assert.NotEmpty(t, repo.Name) - assert.Equal(t, repo.Owner.ID, orgID) - assert.False(t, repo.Private) + 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: 12, privacy: privacyTypePublic}, + user: {count: 12, privacy: privacyTypePublic}, + user2: {count: 12, 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: 4, repoName: "big_test_", privacy: privacyTypePublic}, + user: {count: 4, repoName: "big_test_", privacy: privacyTypePublic}, + user2: {count: 4, repoName: "big_test_", privacy: privacyTypePublic}}, + }, + {name: "RepositoriesAccessibleAndRelatedToUser", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", user.ID), expectedResults: expectedResults{ + nil: {count: 4, privacy: privacyTypePublic}, + user: {count: 8}, + user2: {count: 4, 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: 1, repoOwnerID: orgUser.ID, privacy: privacyTypePublic}, + user: {count: 2, repoOwnerID: orgUser.ID}, + user2: {count: 1, 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) + } + } + }) + } + }) } } From ab5d7cbf3d71b36f76d90e89dd82f65a5a858c8b Mon Sep 17 00:00:00 2001 From: morlinest Date: Thu, 21 Sep 2017 11:01:21 +0200 Subject: [PATCH 19/21] Add specific tests to improve test coverage --- models/repo_list_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/models/repo_list_test.go b/models/repo_list_test.go index af14166101891..43cd4efbbd1f7 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -61,13 +61,25 @@ func TestSearchRepositoryByName(t *testing.T) { 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) + testCases := []struct { name string opts *SearchRepoOptions count int }{ {name: "PublicRepositoriesByName", - opts: &SearchRepoOptions{Keyword: "big_test_", Page: 1, PageSize: 10}, + opts: &SearchRepoOptions{Keyword: "big_test_", PageSize: 10}, count: 4}, {name: "PublicAndPrivateRepositoriesByName", opts: &SearchRepoOptions{Keyword: "big_test_", Page: 1, PageSize: 10, Private: true}, From 62076a3098ecb4c0715cfe0d4ba7c7d191b4514c Mon Sep 17 00:00:00 2001 From: morlinest Date: Sat, 16 Sep 2017 16:39:58 +0200 Subject: [PATCH 20/21] Add repo type option to /api/repo/search --- models/repo_list.go | 40 +++++++++++++++++++++++++++++++++++-- public/swagger.v1.json | 7 +++++++ routers/api/v1/repo/repo.go | 7 +++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/models/repo_list.go b/models/repo_list.go index 1e2fa8c6693da..7959baee12cd7 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -109,8 +109,28 @@ type SearchRepoOptions struct { // 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 @@ -172,11 +192,15 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in } } else { // Set user access conditions + var accessCond builder.Cond = builder.NewCond() + // Add Owner ID to access conditions - var accessCond builder.Cond = builder.Eq{"owner_id": opts.OwnerID} + if opts.RepoType != RepoTypeCollaborative { + accessCond = accessCond.Or(builder.Eq{"owner_id": opts.OwnerID}) + } // Include collaborative repositories - if opts.Collaborate { + 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), @@ -192,6 +216,18 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in opts.OrderBy = SearchOrderByAlphabetically } + // 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() diff --git a/public/swagger.v1.json b/public/swagger.v1.json index e640b4e83160d..643b265b389de 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 eef2d0604c91f..2dd759d544bdf 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -28,11 +28,18 @@ 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")), + RepoType: repoTypes[ctx.Query("type")], } // Include collaborative and private repositories From 19c15bb844da9446462c4853bdd070fbd6085a24 Mon Sep 17 00:00:00 2001 From: morlinest Date: Sun, 17 Sep 2017 03:40:33 +0200 Subject: [PATCH 21/21] Add tests and fix result of collaborative filter in specific condition --- integrations/api_repo_test.go | 26 +++--- models/fixtures/access.yml | 11 +++ models/fixtures/repository.yml | 94 ++++++++++++++++++++ models/fixtures/team.yml | 2 +- models/fixtures/team_repo.yml | 14 ++- models/fixtures/user.yml | 4 +- models/repo_list.go | 4 + models/repo_list_test.go | 157 ++++++++++++++++++++++++++------- 8 files changed, 261 insertions(+), 51 deletions(-) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index a917b2f6c0f4e..01afc7f8ee34c 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -73,9 +73,9 @@ func TestAPISearchRepo(t *testing.T) { expectedResults }{ {name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50", expectedResults: expectedResults{ - nil: {count: 12, privacy: privacyTypePublic}, - user: {count: 12, privacy: privacyTypePublic}, - user2: {count: 12, privacy: privacyTypePublic}}, + 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}, @@ -88,14 +88,14 @@ func TestAPISearchRepo(t *testing.T) { user2: {count: 10, privacy: privacyTypePublic}}, }, {name: "RepositoriesByName", requestURL: fmt.Sprintf("/api/v1/repos/search?q=%s", "big_test_"), expectedResults: expectedResults{ - nil: {count: 4, repoName: "big_test_", privacy: privacyTypePublic}, - user: {count: 4, repoName: "big_test_", privacy: privacyTypePublic}, - user2: {count: 4, repoName: "big_test_", privacy: privacyTypePublic}}, + 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", user.ID), expectedResults: expectedResults{ - nil: {count: 4, privacy: privacyTypePublic}, - user: {count: 8}, - user2: {count: 4, 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}, @@ -103,9 +103,9 @@ func TestAPISearchRepo(t *testing.T) { user2: {count: 2}}, }, {name: "RepositoriesOwnedByOrganization", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", orgUser.ID), expectedResults: expectedResults{ - nil: {count: 1, repoOwnerID: orgUser.ID, privacy: privacyTypePublic}, - user: {count: 2, repoOwnerID: orgUser.ID}, - user2: {count: 1, repoOwnerID: orgUser.ID, privacy: privacyTypePublic}}, + nil: {count: 2, repoOwnerID: orgUser.ID, privacy: privacyTypePublic}, + user: {count: 4, repoOwnerID: orgUser.ID}, + user2: {count: 2, repoOwnerID: orgUser.ID, privacy: privacyTypePublic}}, }, } diff --git a/models/fixtures/access.yml b/models/fixtures/access.yml index 34d5c5c59405c..683d36ca13933 100644 --- a/models/fixtures/access.yml +++ b/models/fixtures/access.yml @@ -38,4 +38,15 @@ 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/repository.yml b/models/fixtures/repository.yml index eb83dfcff7946..e48a102eef281 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -201,6 +201,7 @@ num_closed_pulls: 0 num_watches: 0 is_mirror: false + is_fork: false - id: 18 @@ -213,6 +214,7 @@ num_pulls: 0 num_closed_pulls: 0 is_mirror: false + is_fork: false - id: 19 @@ -225,6 +227,7 @@ num_pulls: 0 num_closed_pulls: 0 is_mirror: false + is_fork: false - id: 20 @@ -237,6 +240,7 @@ num_pulls: 0 num_closed_pulls: 0 is_mirror: false + is_fork: false - id: 21 @@ -249,6 +253,7 @@ num_pulls: 0 num_closed_pulls: 0 is_mirror: false + is_fork: false - id: 22 @@ -261,6 +266,7 @@ num_pulls: 0 num_closed_pulls: 0 is_mirror: false + is_fork: false - id: 23 @@ -273,6 +279,7 @@ num_pulls: 0 num_closed_pulls: 0 is_mirror: false + is_fork: false - id: 24 @@ -285,3 +292,90 @@ 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 298de648ddf78..0c3f8d8398c79 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -43,6 +43,6 @@ lower_name: owners name: Owners authorize: 4 # owner - num_repos: 2 + 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 5154453f7b4dd..722a72ba9aa52 100644 --- a/models/fixtures/team_repo.yml +++ b/models/fixtures/team_repo.yml @@ -26,4 +26,16 @@ id: 5 org_id: 17 team_id: 5 - repo_id: 24 \ No newline at end of file + 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/user.yml b/models/fixtures/user.yml index e1fbc9e578bfe..17eff052320fb 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -231,7 +231,7 @@ is_admin: false avatar: avatar15 avatar_email: user15@example.com - num_repos: 4 + num_repos: 8 is_active: true - @@ -261,7 +261,7 @@ is_admin: false avatar: avatar17 avatar_email: user17@example.com - num_repos: 2 + num_repos: 4 is_active: true num_members: 1 num_teams: 1 \ No newline at end of file diff --git a/models/repo_list.go b/models/repo_list.go index 7959baee12cd7..3cf79410cf5b3 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -164,6 +164,10 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, _ in } } + 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 diff --git a/models/repo_list_test.go b/models/repo_list_test.go index 43cd4efbbd1f7..5adfce4b4caa9 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" @@ -73,57 +74,145 @@ func TestSearchRepositoryByName(t *testing.T) { 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 - count int + name string + opts *SearchRepoOptions + expectedCounts }{ {name: "PublicRepositoriesByName", - opts: &SearchRepoOptions{Keyword: "big_test_", PageSize: 10}, - count: 4}, + 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}, - count: 8}, + 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}, - count: 8}, + 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}, - count: 8}, + 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: 10, OwnerID: 15}, - count: 2}, + 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: 10, OwnerID: 15, Private: true}, - count: 4}, + 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: 10, OwnerID: 15, Collaborate: true}, - count: 4}, + 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: 10, OwnerID: 15, Private: true, Collaborate: true}, - count: 8}, + 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: 10, OwnerID: 17}, - count: 1}, + 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: 10, OwnerID: 17, Private: true}, - count: 2}, + 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) { - repos, count, err := SearchRepositoryByName(testCase.opts) - - assert.NoError(t, err) - assert.Equal(t, int64(testCase.count), count) - - var expectedLen int - if testCase.opts.PageSize*testCase.opts.Page > testCase.count { - expectedLen = testCase.count % testCase.opts.PageSize - } else { - expectedLen = testCase.opts.PageSize + 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) + } + } + }) } - assert.Len(t, repos, expectedLen) }) } } + +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) +}