Skip to content
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.

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

Create a "Builder" to Order SQL by X #19834

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 19 additions & 1 deletion models/db/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,29 @@

package db

import "xorm.io/xorm"

// SearchOrderBy is used to sort the result
type SearchOrderBy string

func (s SearchOrderBy) String() string {
return string(s)
switch s {
case SearchOrderByAlphabetically, SearchOrderByAlphabeticallyReverse, SearchOrderByLeastUpdated,
SearchOrderByRecentUpdated, SearchOrderByOldest, SearchOrderByNewest,
SearchOrderBySize, SearchOrderBySizeReverse, SearchOrderByID,
SearchOrderByIDReverse, SearchOrderByStars, SearchOrderByStarsReverse,
SearchOrderByForks, SearchOrderByForksReverse:
return string(s)
default:
return ""

}
}

func (s SearchOrderBy) Builder() func(sess *xorm.Session) *xorm.Session {
return func(sess *xorm.Session) *xorm.Session {
return sess.OrderBy(s.String())
}
}

// Strings for sorting result
Expand Down
32 changes: 23 additions & 9 deletions models/repo_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"code.gitea.io/gitea/modules/util"

"xorm.io/builder"
"xorm.io/xorm"
)

// RepositoryListDefaultPageSize is the default number of repositories
Expand Down Expand Up @@ -116,7 +117,7 @@ type SearchRepoOptions struct {
OwnerID int64
PriorityOwnerID int64
TeamID int64
OrderBy db.SearchOrderBy
OrderBy func(sess *xorm.Session) *xorm.Session
Private bool // Include private repositories in results
StarredByID int64
WatchedByID int64
Copy link
Member

Choose a reason for hiding this comment

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

What I just noticed: Why do we have two times the same constants?
Once in db/search, and once here in L172+?

(The comment is here because GitHub doesn't let me comment down below)

Expand Down Expand Up @@ -552,16 +553,20 @@ func searchRepositoryByCondition(ctx context.Context, opts *SearchRepoOptions, c
opts.Page = 1
}

if len(opts.OrderBy) == 0 {
opts.OrderBy = db.SearchOrderByAlphabetically
if opts.OrderBy == nil {
opts.OrderBy = db.SearchOrderByAlphabetically.Builder()
}

if opts.PriorityOwnerID > 0 {
opts.OrderBy = db.SearchOrderBy(fmt.Sprintf("CASE WHEN owner_id = %d THEN 0 ELSE owner_id END, %s", opts.PriorityOwnerID, opts.OrderBy))
opts.OrderBy = func(sess *xorm.Session) *xorm.Session {
return sess.SQL("ORDER BY CASE WHEN owner_id = ? THEN 0 ELSE owner_id END, ?", opts.PriorityOwnerID, db.SearchOrderByAlphabetically.String())
}
} else if strings.Count(opts.Keyword, "/") == 1 {
// With "owner/repo" search times, prioritise results which match the owner field
orgName := strings.Split(opts.Keyword, "/")[0]
opts.OrderBy = db.SearchOrderBy(fmt.Sprintf("CASE WHEN owner_name LIKE '%s' THEN 0 ELSE 1 END, %s", orgName, opts.OrderBy))
opts.OrderBy = func(sess *xorm.Session) *xorm.Session {
return sess.SQL("ORDER BY CASE WHEN owner_name LIKE ? THEN 0 ELSE 1 END, ?", orgName, db.SearchOrderByAlphabetically.String())
}
}

sess := db.GetEngine(ctx)
Expand All @@ -577,7 +582,12 @@ func searchRepositoryByCondition(ctx context.Context, opts *SearchRepoOptions, c
}
}

sess = sess.Where(cond).OrderBy(opts.OrderBy.String())
if opts.OrderBy != nil {
sess = opts.OrderBy(sess.Where(cond))
} else {
sess = sess.Where(cond)
}

if opts.PageSize > 0 {
sess = sess.Limit(opts.PageSize, (opts.Page-1)*opts.PageSize)
}
Expand Down Expand Up @@ -674,8 +684,8 @@ func FindUserAccessibleRepoIDs(user *user_model.User) ([]int64, error) {

// GetUserRepositories returns a list of repositories of given user.
func GetUserRepositories(opts *SearchRepoOptions) (RepositoryList, int64, error) {
if len(opts.OrderBy) == 0 {
opts.OrderBy = "updated_unix DESC"
if opts.OrderBy == nil {
opts.OrderBy = db.SearchOrderByRecentUpdated.Builder()
}

cond := builder.NewCond()
Expand All @@ -695,7 +705,11 @@ func GetUserRepositories(opts *SearchRepoOptions) (RepositoryList, int64, error)
return nil, 0, fmt.Errorf("Count: %v", err)
}

sess = sess.Where(cond).OrderBy(opts.OrderBy.String())
if opts.OrderBy != nil {
sess = opts.OrderBy(sess.Where(cond))
} else {
sess = sess.Where(cond)
}
repos := make(RepositoryList, 0, opts.PageSize)
return repos, count, db.SetSessionPagination(sess, opts).Find(&repos)
}
2 changes: 1 addition & 1 deletion modules/indexer/issues/indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ func populateIssueIndexer(ctx context.Context) {
}
repos, _, err := models.SearchRepositoryByName(&models.SearchRepoOptions{
ListOptions: db.ListOptions{Page: page, PageSize: models.RepositoryListDefaultPageSize},
OrderBy: db.SearchOrderByID,
OrderBy: db.SearchOrderByID.Builder(),
Private: true,
Collaborate: util.OptionalBoolFalse,
})
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func SearchIssues(ctx *context.APIContext) {
Collaborate: util.OptionalBoolNone,
// This needs to be a column that is not nil in fixtures or
// MySQL will return different results when sorting by null in some cases
OrderBy: db.SearchOrderByAlphabetically,
OrderBy: db.SearchOrderByAlphabetically.Builder(),
Actor: ctx.Doer,
}
if ctx.IsSigned {
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func Search(ctx *context.APIContext) {
}
if searchModeMap, ok := context.SearchOrderByMap[sortOrder]; ok {
if orderBy, ok := searchModeMap[sortMode]; ok {
opts.OrderBy = orderBy
opts.OrderBy = orderBy.Builder()
} else {
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("Invalid sort mode: \"%s\"", sortMode))
return
Expand Down
3 changes: 2 additions & 1 deletion routers/api/v1/user/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
user_model "code.gitea.io/gitea/models/user"
Expand All @@ -25,7 +26,7 @@ func listUserRepos(ctx *context.APIContext, u *user_model.User, private bool) {
Actor: u,
Private: private,
ListOptions: opts,
OrderBy: "id ASC",
OrderBy: db.SearchOrderByID.Builder(),
})
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetUserRepositories", err)
Expand Down
2 changes: 1 addition & 1 deletion routers/web/explore/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) {
PageSize: opts.PageSize,
},
Actor: ctx.Doer,
OrderBy: orderBy,
OrderBy: orderBy.Builder(),
Private: opts.Private,
Keyword: keyword,
OwnerID: opts.OwnerID,
Expand Down
2 changes: 1 addition & 1 deletion routers/web/org/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func Home(ctx *context.Context) {
},
Keyword: keyword,
OwnerID: org.ID,
OrderBy: orderBy,
OrderBy: orderBy.Builder(),
Private: ctx.IsSigned,
Actor: ctx.Doer,
Language: language,
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -2152,7 +2152,7 @@ func SearchIssues(ctx *context.Context) {
Collaborate: util.OptionalBoolNone,
// This needs to be a column that is not nil in fixtures or
// MySQL will return different results when sorting by null in some cases
OrderBy: db.SearchOrderByAlphabetically,
OrderBy: db.SearchOrderByAlphabetically.Builder(),
Actor: ctx.Doer,
}
if ctx.IsSigned {
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ func SearchRepo(ctx *context.Context) {
}
if searchModeMap, ok := context.SearchOrderByMap[sortOrder]; ok {
if orderBy, ok := searchModeMap[sortMode]; ok {
opts.OrderBy = orderBy
opts.OrderBy = orderBy.Builder()
} else {
ctx.Error(http.StatusUnprocessableEntity, fmt.Sprintf("Invalid sort mode: \"%s\"", sortMode))
return
Expand Down
6 changes: 3 additions & 3 deletions routers/web/user/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func Profile(ctx *context.Context) {
},
Actor: ctx.Doer,
Keyword: keyword,
OrderBy: orderBy,
OrderBy: orderBy.Builder(),
Private: ctx.IsSigned,
StarredByID: ctx.ContextUser.ID,
Collaborate: util.OptionalBoolFalse,
Expand Down Expand Up @@ -234,7 +234,7 @@ func Profile(ctx *context.Context) {
},
Actor: ctx.Doer,
Keyword: keyword,
OrderBy: orderBy,
OrderBy: orderBy.Builder(),
Private: ctx.IsSigned,
WatchedByID: ctx.ContextUser.ID,
Collaborate: util.OptionalBoolFalse,
Expand All @@ -257,7 +257,7 @@ func Profile(ctx *context.Context) {
Actor: ctx.Doer,
Keyword: keyword,
OwnerID: ctx.ContextUser.ID,
OrderBy: orderBy,
OrderBy: orderBy.Builder(),
Private: ctx.IsSigned,
Collaborate: util.OptionalBoolFalse,
TopicOnly: topicOnly,
Expand Down