Skip to content

Add search mode option to /api/repo/search #2756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Oct 26, 2017
Merged

Add search mode option to /api/repo/search #2756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
00b167c
Add repo type option to /api/repo/search
Morlinest Oct 20, 2017
67b4c4a
Add tests and fix result of collaborative filter in specific condition
Morlinest Oct 20, 2017
ebc86a2
Fix/optimize search & tests
Morlinest Oct 21, 2017
5dbdc93
Improve integration tests
Morlinest Oct 21, 2017
ebff7e0
Fix lint errors
Morlinest Oct 21, 2017
d750b30
Change and improve internal implementation of repo search
Morlinest Oct 22, 2017
949cbdb
Fix unit tests
Morlinest Oct 22, 2017
e2c10a0
Use NonexistentID
Morlinest Oct 23, 2017
96eaf52
Make search api more general
Morlinest Oct 23, 2017
4329334
Change mirror and fork search behaviour
Morlinest Oct 25, 2017
2cf2561
Fix tests & typo in comment
Morlinest Oct 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 51 additions & 6 deletions integrations/api_repo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func TestAPISearchRepo(t *testing.T) {
user := models.AssertExistsAndLoadBean(t, &models.User{ID: 15}).(*models.User)
user2 := models.AssertExistsAndLoadBean(t, &models.User{ID: 16}).(*models.User)
user3 := models.AssertExistsAndLoadBean(t, &models.User{ID: 18}).(*models.User)
user4 := models.AssertExistsAndLoadBean(t, &models.User{ID: 20}).(*models.User)
orgUser := models.AssertExistsAndLoadBean(t, &models.User{ID: 17}).(*models.User)

// Map of expected results, where key is user for login
Expand All @@ -66,9 +67,9 @@ func TestAPISearchRepo(t *testing.T) {
expectedResults
}{
{name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50", expectedResults: expectedResults{
nil: {count: 12},
user: {count: 12},
user2: {count: 12}},
nil: {count: 15},
user: {count: 15},
user2: {count: 15}},
},
{name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10", expectedResults: expectedResults{
nil: {count: 10},
Expand All @@ -81,9 +82,9 @@ func TestAPISearchRepo(t *testing.T) {
user2: {count: 10}},
},
{name: "RepositoriesByName", requestURL: fmt.Sprintf("/api/v1/repos/search?q=%s", "big_test_"), expectedResults: expectedResults{
nil: {count: 4, repoName: "big_test_"},
user: {count: 4, repoName: "big_test_"},
user2: {count: 4, repoName: "big_test_"}},
nil: {count: 7, repoName: "big_test_"},
user: {count: 7, repoName: "big_test_"},
user2: {count: 7, repoName: "big_test_"}},
},
{name: "RepositoriesAccessibleAndRelatedToUser", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", user.ID), expectedResults: expectedResults{
nil: {count: 4},
Expand All @@ -106,16 +107,46 @@ func TestAPISearchRepo(t *testing.T) {
user: {count: 2, repoOwnerID: orgUser.ID, includesPrivate: true},
user2: {count: 1, repoOwnerID: orgUser.ID}},
},
{name: "RepositoriesAccessibleAndRelatedToUser4", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d", user4.ID), expectedResults: expectedResults{
nil: {count: 3},
user: {count: 3},
user4: {count: 6, includesPrivate: true}}},
{name: "RepositoriesAccessibleAndRelatedToUser4/SearchModeSource", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d&mode=%s", user4.ID, "source"), expectedResults: expectedResults{
nil: {count: 0},
user: {count: 0},
user4: {count: 0, includesPrivate: true}}},
{name: "RepositoriesAccessibleAndRelatedToUser4/SearchModeFork", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d&mode=%s", user4.ID, "fork"), expectedResults: expectedResults{
nil: {count: 1},
user: {count: 1},
user4: {count: 2, includesPrivate: true}}},
{name: "RepositoriesAccessibleAndRelatedToUser4/SearchModeFork/Exclusive", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d&mode=%s&exclusive=1", user4.ID, "fork"), expectedResults: expectedResults{
nil: {count: 1},
user: {count: 1},
user4: {count: 2, includesPrivate: true}}},
{name: "RepositoriesAccessibleAndRelatedToUser4/SearchModeMirror", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d&mode=%s", user4.ID, "mirror"), expectedResults: expectedResults{
nil: {count: 2},
user: {count: 2},
user4: {count: 4, includesPrivate: true}}},
{name: "RepositoriesAccessibleAndRelatedToUser4/SearchModeMirror/Exclusive", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d&mode=%s&exclusive=1", user4.ID, "mirror"), expectedResults: expectedResults{
nil: {count: 1},
user: {count: 1},
user4: {count: 2, includesPrivate: true}}},
{name: "RepositoriesAccessibleAndRelatedToUser4/SearchModeCollaborative", requestURL: fmt.Sprintf("/api/v1/repos/search?uid=%d&mode=%s", user4.ID, "collaborative"), expectedResults: expectedResults{
nil: {count: 0},
user: {count: 0},
user4: {count: 0, includesPrivate: true}}},
}

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
for userToLogin, expected := range testCase.expectedResults {
var session *TestSession
var testName string
var userID int64
if userToLogin != nil && userToLogin.ID > 0 {
testName = fmt.Sprintf("LoggedUser%d", userToLogin.ID)
session = loginUser(t, userToLogin.Name)
userID = userToLogin.ID
} else {
testName = "AnonymousUser"
session = emptyTestSession(t)
Expand All @@ -130,6 +161,11 @@ func TestAPISearchRepo(t *testing.T) {

assert.Len(t, body.Data, expected.count)
for _, repo := range body.Data {
r := getRepo(t, repo.ID)
hasAccess, err := models.HasAccess(userID, r, models.AccessModeRead)
assert.NoError(t, err)
assert.True(t, hasAccess)

assert.NotEmpty(t, repo.Name)

if len(expected.repoName) > 0 {
Expand All @@ -150,6 +186,15 @@ func TestAPISearchRepo(t *testing.T) {
}
}

var repoCache = make(map[int64]*models.Repository)

func getRepo(t *testing.T, repoID int64) *models.Repository {
if _, ok := repoCache[repoID]; !ok {
repoCache[repoID] = models.AssertExistsAndLoadBean(t, &models.Repository{ID: repoID}).(*models.Repository)
}
return repoCache[repoID]
}

func TestAPIViewRepo(t *testing.T) {
prepareTestEnv(t)

Expand Down
14 changes: 13 additions & 1 deletion models/fixtures/access.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,16 @@
id: 11
user_id: 18
repo_id: 21
mode: 2 # write
mode: 2 # write

-
id: 12
user_id: 20
repo_id: 27
mode: 4 # owner

-
id: 13
user_id: 20
repo_id: 28
mode: 4 # owner
8 changes: 8 additions & 0 deletions models/fixtures/org_user.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,12 @@
org_id: 17
is_public: false
is_owner: true
num_teams: 1

-
id: 7
uid: 20
org_id: 19
is_public: true
is_owner: true
num_teams: 1
94 changes: 94 additions & 0 deletions models/fixtures/repository.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@
num_closed_pulls: 0
num_watches: 0
is_mirror: false
is_fork: false

-
id: 18
Expand All @@ -213,6 +214,7 @@
num_pulls: 0
num_closed_pulls: 0
is_mirror: false
is_fork: false

-
id: 19
Expand All @@ -225,6 +227,7 @@
num_pulls: 0
num_closed_pulls: 0
is_mirror: false
is_fork: false

-
id: 20
Expand All @@ -237,6 +240,7 @@
num_pulls: 0
num_closed_pulls: 0
is_mirror: false
is_fork: false

-
id: 21
Expand All @@ -249,6 +253,7 @@
num_pulls: 0
num_closed_pulls: 0
is_mirror: false
is_fork: false

-
id: 22
Expand All @@ -261,6 +266,7 @@
num_pulls: 0
num_closed_pulls: 0
is_mirror: false
is_fork: false

-
id: 23
Expand All @@ -273,6 +279,7 @@
num_pulls: 0
num_closed_pulls: 0
is_mirror: false
is_fork: false

-
id: 24
Expand All @@ -285,3 +292,90 @@
num_pulls: 0
num_closed_pulls: 0
is_mirror: false
is_fork: false

-
id: 25
owner_id: 20
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: 20
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: 19
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: 19
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: 20
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: 20
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
11 changes: 11 additions & 0 deletions models/fixtures/team.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
num_repos: 0
num_members: 1
unit_types: '[1,2,3,4,5,6,7]'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: please remove

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was missing 1 empty line, now it is consistent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, my mistake

-
id: 5
org_id: 17
Expand All @@ -45,4 +46,14 @@
authorize: 4 # owner
num_repos: 2
num_members: 2
unit_types: '[1,2,3,4,5,6,7]'

-
id: 6
org_id: 19
lower_name: owners
name: Owners
authorize: 4 # owner
num_repos: 2
num_members: 1
unit_types: '[1,2,3,4,5,6,7]'
14 changes: 13 additions & 1 deletion models/fixtures/team_repo.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,16 @@
id: 5
org_id: 17
team_id: 5
repo_id: 24
repo_id: 24

-
id: 6
org_id: 19
team_id: 6
repo_id: 27

-
id: 7
org_id: 19
team_id: 6
repo_id: 28
8 changes: 7 additions & 1 deletion models/fixtures/team_user.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,10 @@
id: 7
org_id: 17
team_id: 5
uid: 18
uid: 18

-
id: 8
org_id: 19
team_id: 6
uid: 20
32 changes: 32 additions & 0 deletions models/fixtures/user.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,4 +281,36 @@
avatar: avatar18
avatar_email: [email protected]
num_repos: 0
is_active: true

-
id: 19
lower_name: user19
name: user19
full_name: User 19
email: [email protected]
passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password
type: 1 # organization
salt: ZogKvWdyEx
is_admin: false
avatar: avatar19
avatar_email: [email protected]
num_repos: 2
is_active: true
num_members: 1
num_teams: 1

-
id: 20
lower_name: user20
name: user20
full_name: User 20
email: [email protected]
passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password
type: 0 # individual
salt: ZogKvWdyEx
is_admin: false
avatar: avatar20
avatar_email: [email protected]
num_repos: 4
is_active: true
Loading