Skip to content

Commit c5e78fc

Browse files
Do not mutate incoming options to SearchRepositoryByName (#34553)
Similar to #34544, this PR changes the `opts` argument in `SearchRepositoryByName()` to be passed by value instead of by pointer, as its mutations do not escape the function scope and are not used elsewhere. This simplifies reasoning about the function and avoids unnecessary pointer usage. This insight emerged during an initial attempt to refactor `RenderUserSearch()`, which currently intermixes multiple concerns. --------- Co-authored-by: Philip Peterson <[email protected]>
1 parent f48c013 commit c5e78fc

File tree

25 files changed

+91
-89
lines changed

25 files changed

+91
-89
lines changed

cmd/admin.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func runRepoSyncReleases(_ *cli.Context) error {
107107

108108
log.Trace("Synchronizing repository releases (this may take a while)")
109109
for page := 1; ; page++ {
110-
repos, count, err := repo_model.SearchRepositoryByName(ctx, &repo_model.SearchRepoOptions{
110+
repos, count, err := repo_model.SearchRepositoryByName(ctx, repo_model.SearchRepoOptions{
111111
ListOptions: db.ListOptions{
112112
PageSize: repo_model.RepositoryListDefaultPageSize,
113113
Page: page,

models/git/branch.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ func FindRecentlyPushedNewBranches(ctx context.Context, doer *user_model.User, o
487487
ForkFrom: opts.BaseRepo.ID,
488488
Archived: optional.Some(false),
489489
}
490-
repoCond := repo_model.SearchRepositoryCondition(&repoOpts).And(repo_model.AccessibleRepositoryCondition(doer, unit.TypeCode))
490+
repoCond := repo_model.SearchRepositoryCondition(repoOpts).And(repo_model.AccessibleRepositoryCondition(doer, unit.TypeCode))
491491
if opts.Repo.ID == opts.BaseRepo.ID {
492492
// should also include the base repo's branches
493493
repoCond = repoCond.Or(builder.Eq{"id": opts.BaseRepo.ID})

models/repo/repo_list.go

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ func UserOrgPublicUnitRepoCond(userID, orgID int64) builder.Cond {
359359
}
360360

361361
// SearchRepositoryCondition creates a query condition according search repository options
362-
func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond {
362+
func SearchRepositoryCondition(opts SearchRepoOptions) builder.Cond {
363363
cond := builder.NewCond()
364364

365365
if opts.Private {
@@ -551,18 +551,18 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond {
551551

552552
// SearchRepository returns repositories based on search options,
553553
// it returns results in given range and number of total results.
554-
func SearchRepository(ctx context.Context, opts *SearchRepoOptions) (RepositoryList, int64, error) {
554+
func SearchRepository(ctx context.Context, opts SearchRepoOptions) (RepositoryList, int64, error) {
555555
cond := SearchRepositoryCondition(opts)
556556
return SearchRepositoryByCondition(ctx, opts, cond, true)
557557
}
558558

559559
// CountRepository counts repositories based on search options,
560-
func CountRepository(ctx context.Context, opts *SearchRepoOptions) (int64, error) {
560+
func CountRepository(ctx context.Context, opts SearchRepoOptions) (int64, error) {
561561
return db.GetEngine(ctx).Where(SearchRepositoryCondition(opts)).Count(new(Repository))
562562
}
563563

564564
// SearchRepositoryByCondition search repositories by condition
565-
func SearchRepositoryByCondition(ctx context.Context, opts *SearchRepoOptions, cond builder.Cond, loadAttributes bool) (RepositoryList, int64, error) {
565+
func SearchRepositoryByCondition(ctx context.Context, opts SearchRepoOptions, cond builder.Cond, loadAttributes bool) (RepositoryList, int64, error) {
566566
sess, count, err := searchRepositoryByCondition(ctx, opts, cond)
567567
if err != nil {
568568
return nil, 0, err
@@ -590,23 +590,25 @@ func SearchRepositoryByCondition(ctx context.Context, opts *SearchRepoOptions, c
590590
return repos, count, nil
591591
}
592592

593-
func searchRepositoryByCondition(ctx context.Context, opts *SearchRepoOptions, cond builder.Cond) (db.Engine, int64, error) {
594-
if opts.Page <= 0 {
595-
opts.Page = 1
593+
func searchRepositoryByCondition(ctx context.Context, opts SearchRepoOptions, cond builder.Cond) (db.Engine, int64, error) {
594+
page := opts.Page
595+
if page <= 0 {
596+
page = 1
596597
}
597598

598-
if len(opts.OrderBy) == 0 {
599-
opts.OrderBy = db.SearchOrderByAlphabetically
599+
orderBy := opts.OrderBy
600+
if len(orderBy) == 0 {
601+
orderBy = db.SearchOrderByAlphabetically
600602
}
601603

602604
args := make([]any, 0)
603605
if opts.PriorityOwnerID > 0 {
604-
opts.OrderBy = db.SearchOrderBy(fmt.Sprintf("CASE WHEN owner_id = ? THEN 0 ELSE owner_id END, %s", opts.OrderBy))
606+
orderBy = db.SearchOrderBy(fmt.Sprintf("CASE WHEN owner_id = ? THEN 0 ELSE owner_id END, %s", orderBy))
605607
args = append(args, opts.PriorityOwnerID)
606608
} else if strings.Count(opts.Keyword, "/") == 1 {
607609
// With "owner/repo" search times, prioritise results which match the owner field
608610
orgName := strings.Split(opts.Keyword, "/")[0]
609-
opts.OrderBy = db.SearchOrderBy(fmt.Sprintf("CASE WHEN owner_name LIKE ? THEN 0 ELSE 1 END, %s", opts.OrderBy))
611+
orderBy = db.SearchOrderBy(fmt.Sprintf("CASE WHEN owner_name LIKE ? THEN 0 ELSE 1 END, %s", orderBy))
610612
args = append(args, orgName)
611613
}
612614

@@ -623,9 +625,9 @@ func searchRepositoryByCondition(ctx context.Context, opts *SearchRepoOptions, c
623625
}
624626
}
625627

626-
sess = sess.Where(cond).OrderBy(opts.OrderBy.String(), args...)
628+
sess = sess.Where(cond).OrderBy(orderBy.String(), args...)
627629
if opts.PageSize > 0 {
628-
sess = sess.Limit(opts.PageSize, (opts.Page-1)*opts.PageSize)
630+
sess = sess.Limit(opts.PageSize, (page-1)*opts.PageSize)
629631
}
630632
return sess, count, nil
631633
}
@@ -689,14 +691,14 @@ func AccessibleRepositoryCondition(user *user_model.User, unitType unit.Type) bu
689691

690692
// SearchRepositoryByName takes keyword and part of repository name to search,
691693
// it returns results in given range and number of total results.
692-
func SearchRepositoryByName(ctx context.Context, opts *SearchRepoOptions) (RepositoryList, int64, error) {
694+
func SearchRepositoryByName(ctx context.Context, opts SearchRepoOptions) (RepositoryList, int64, error) {
693695
opts.IncludeDescription = false
694696
return SearchRepository(ctx, opts)
695697
}
696698

697699
// SearchRepositoryIDs takes keyword and part of repository name to search,
698700
// it returns results in given range and number of total results.
699-
func SearchRepositoryIDs(ctx context.Context, opts *SearchRepoOptions) ([]int64, int64, error) {
701+
func SearchRepositoryIDs(ctx context.Context, opts SearchRepoOptions) ([]int64, int64, error) {
700702
opts.IncludeDescription = false
701703

702704
cond := SearchRepositoryCondition(opts)
@@ -740,7 +742,7 @@ func FindUserCodeAccessibleOwnerRepoIDs(ctx context.Context, ownerID int64, user
740742
}
741743

742744
// GetUserRepositories returns a list of repositories of given user.
743-
func GetUserRepositories(ctx context.Context, opts *SearchRepoOptions) (RepositoryList, int64, error) {
745+
func GetUserRepositories(ctx context.Context, opts SearchRepoOptions) (RepositoryList, int64, error) {
744746
if len(opts.OrderBy) == 0 {
745747
opts.OrderBy = "updated_unix DESC"
746748
}
@@ -767,5 +769,5 @@ func GetUserRepositories(ctx context.Context, opts *SearchRepoOptions) (Reposito
767769

768770
sess = sess.Where(cond).OrderBy(opts.OrderBy.String())
769771
repos := make(RepositoryList, 0, opts.PageSize)
770-
return repos, count, db.SetSessionPagination(sess, opts).Find(&repos)
772+
return repos, count, db.SetSessionPagination(sess, &opts).Find(&repos)
771773
}

0 commit comments

Comments
 (0)