Skip to content

Remove redudant functions and code #2652

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 5 commits into from
Oct 10, 2017
Merged

Remove redudant functions and code #2652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7570ff4
Remove redudant functions and code
Morlinest Oct 4, 2017
69c23f2
Lowercase column names
Morlinest Oct 4, 2017
82b6465
Remove Searcher field
Morlinest Oct 4, 2017
ceb8174
Include all public repositories in search result
Morlinest Oct 10, 2017
d3e349b
Add unit tests
Morlinest Oct 10, 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
4 changes: 3 additions & 1 deletion models/issue_indexer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ func InitIssueIndexer() {
func populateIssueIndexer() error {
batch := indexer.IssueIndexerBatch()
for page := 1; ; page++ {
repos, _, err := Repositories(&SearchRepoOptions{
repos, _, err := SearchRepositoryByName(&SearchRepoOptions{
Page: page,
PageSize: 10,
OrderBy: SearchOrderByID,
Private: true,
})
if err != nil {
return fmt.Errorf("Repositories: %v", err)
Expand Down
92 changes: 12 additions & 80 deletions models/repo_list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ type SearchRepoOptions struct {
Starred bool `json:"-"`
Page int `json:"-"`
IsProfile bool `json:"-"`
AllPublic bool `json:"-"` // Include also all public repositories
// Limit of result
//
// maximum: setting.ExplorePagingNum
Expand All @@ -129,6 +130,8 @@ const (
SearchOrderByNewest = "created_unix DESC"
SearchOrderBySize = "size ASC"
SearchOrderBySizeReverse = "size DESC"
SearchOrderByID = "id ASC"
SearchOrderByIDReverse = "id DESC"
)

// SearchRepositoryByName takes keyword and part of repository name to search,
Expand All @@ -147,11 +150,6 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, coun
starJoin = true
}

opts.Keyword = strings.ToLower(opts.Keyword)
if 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}
Expand Down Expand Up @@ -182,6 +180,15 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, coun
cond = cond.And(builder.Eq{"is_private": false})
}

if opts.OwnerID > 0 && opts.AllPublic {
cond = cond.Or(builder.Eq{"is_private": false})
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a bug. If opts.OwnerID > 0 && opts.AllPublic is true, then we will search for all public repos, even those that do not match the keyword.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

}

opts.Keyword = strings.ToLower(opts.Keyword)
if opts.Keyword != "" {
cond = cond.And(builder.Like{"lower_name", opts.Keyword})
}

if len(opts.OrderBy) == 0 {
opts.OrderBy = SearchOrderByAlphabetically
}
Expand Down Expand Up @@ -225,78 +232,3 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, coun

return
}

// Repositories returns all repositories
func Repositories(opts *SearchRepoOptions) (_ RepositoryList, count int64, err error) {
if len(opts.OrderBy) == 0 {
opts.OrderBy = "id ASC"
}

repos := make(RepositoryList, 0, opts.PageSize)

if err = x.
Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
OrderBy(opts.OrderBy.String()).
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)
}

count = countRepositories(-1, opts.Private)

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 = SearchOrderByRecentUpdated
}

if !opts.Private {
cond = builder.Eq{
"is_private": false,
}
}

if opts.Searcher != nil && !opts.Searcher.IsAdmin {
var ownerIds []int64

ownerIds = append(ownerIds, opts.Searcher.ID)
err := opts.Searcher.GetOrganizations(true)

if err != nil {
return nil, 0, fmt.Errorf("Organization: %v", err)
}

for _, org := range opts.Searcher.Orgs {
ownerIds = append(ownerIds, org.ID)
}

cond = cond.Or(builder.In("owner_id", ownerIds))
}

count, err := x.Where(cond).Count(new(Repository))
if err != nil {
return nil, 0, fmt.Errorf("Count: %v", err)
}

if err = x.Where(cond).
Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
Limit(opts.PageSize).
OrderBy(opts.OrderBy.String()).
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
}
20 changes: 19 additions & 1 deletion models/repo_list_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,24 @@ func TestSearchRepositoryByName(t *testing.T) {
{name: "PublicAndPrivateRepositoriesOfOrganization",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 17, Private: true},
count: 2},
{name: "AllPublic/PublicRepositoriesByName",
opts: &SearchRepoOptions{Keyword: "big_test_", PageSize: 10, AllPublic: true},
count: 4},
{name: "AllPublic/PublicAndPrivateRepositoriesByName",
opts: &SearchRepoOptions{Keyword: "big_test_", Page: 1, PageSize: 10, Private: true, AllPublic: true},
count: 8},
{name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Collaborate: true, AllPublic: true},
count: 12},
{name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Private: true, Collaborate: true, AllPublic: true},
count: 16},
{name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName",
opts: &SearchRepoOptions{Keyword: "test", Page: 1, PageSize: 10, OwnerID: 15, Private: true, Collaborate: true, AllPublic: true},
count: 10},
{name: "AllPublic/PublicRepositoriesOfOrganization",
opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 17, AllPublic: true},
count: 12},
}

for _, testCase := range testCases {
Expand Down Expand Up @@ -126,7 +144,7 @@ func TestSearchRepositoryByName(t *testing.T) {
}

// FIXME: Can't check, need to fix current behaviour (see previous FIXME comments in test cases)
/*if testCase.opts.OwnerID > 0 && !testCase.opts.Collaborate {
/*if testCase.opts.OwnerID > 0 && !testCase.opts.Collaborate && !AllPublic {
assert.Equal(t, testCase.opts.OwnerID, repo.Owner.ID)
}*/

Expand Down
1 change: 0 additions & 1 deletion routers/admin/repos.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ func Repos(ctx *context.Context) {
ctx.Data["PageIsAdminRepositories"] = true

routers.RenderRepoSearch(ctx, &routers.RepoSearchOptions{
Ranger: models.Repositories,
Private: true,
PageSize: setting.UI.Admin.RepoPagingNum,
TplName: tplRepos,
Expand Down
57 changes: 23 additions & 34 deletions routers/home.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ func Swagger(ctx *context.Context) {

// RepoSearchOptions when calling search repositories
type RepoSearchOptions struct {
Ranger func(*models.SearchRepoOptions) (models.RepositoryList, int64, error)
Searcher *models.User
OwnerID int64
Private bool
PageSize int
TplName base.TplName
Expand Down Expand Up @@ -113,35 +112,21 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) {
}

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,
})
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,
})
if err != nil {
ctx.Handle(500, "SearchRepositoryByName", err)
return
}
}

repos, count, err = models.SearchRepositoryByName(&models.SearchRepoOptions{
Page: page,
PageSize: opts.PageSize,
OrderBy: orderBy,
Private: opts.Private,
Keyword: keyword,
OwnerID: opts.OwnerID,
Searcher: ctx.User,
Collaborate: true,
AllPublic: true,
})
if err != nil {
ctx.Handle(500, "SearchRepositoryByName", err)
return
}
ctx.Data["Keyword"] = keyword
ctx.Data["Total"] = count
Expand All @@ -157,11 +142,15 @@ func ExploreRepos(ctx *context.Context) {
ctx.Data["PageIsExplore"] = true
ctx.Data["PageIsExploreRepositories"] = true

var ownerID int64
if ctx.User != nil && !ctx.User.IsAdmin {
ownerID = ctx.User.ID
}

RenderRepoSearch(ctx, &RepoSearchOptions{
Ranger: models.GetRecentUpdatedRepositories,
PageSize: setting.UI.ExplorePagingNum,
Searcher: ctx.User,
Private: ctx.User != nil && ctx.User.IsAdmin,
OwnerID: ownerID,
Private: ctx.User != nil,
TplName: tplExploreRepos,
})
}
Expand Down