Fix repository search function (#2689)

* Fix and remove FIXME

* Respect membership visibility

* Fix/rewrite searchRepositoryByName function

* Add unit tests

* Add integration tests

* Remove Searcher completely

* Remove trailing space

1 files changed, 38 insertions, 51 deletions

diff --git a/models/repo_list.go b/models/repo_list.go
index 4aeda90677..2c4c66ac3e 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     SearchOrderBy `json:"-"`
 	Private     bool          `json:"-"` // Include private repositories in results
 	Collaborate bool          `json:"-"` // Include collaborative repositories
@@ -136,57 +135,48 @@ const (
 
 // 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) (RepositoryList, int64, error) {
 	if opts.Page <= 0 {
 		opts.Page = 1
 	}
 
-	var starJoin bool
-	if opts.Starred && opts.OwnerID > 0 {
-		cond = builder.Eq{
-			"star.uid": opts.OwnerID,
-		}
-		starJoin = true
-	}
-
-	// 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
-
-			ownerIds = append(ownerIds, opts.Searcher.ID)
-			err = opts.Searcher.GetOrganizations(true)
+	var cond = builder.NewCond()
 
-			if err != nil {
-				return nil, 0, fmt.Errorf("Organization: %v", err)
-			}
+	if !opts.Private {
+		cond = cond.And(builder.Eq{"is_private": false})
+	}
 
-			for _, org := range opts.Searcher.Orgs {
-				ownerIds = append(ownerIds, org.ID)
+	starred := false
+	if opts.OwnerID > 0 {
+		if opts.Starred {
+			starred = true
+			cond = builder.Eq{
+				"star.uid": opts.OwnerID,
 			}
+		} else {
+			var accessCond builder.Cond = builder.Eq{"owner_id": opts.OwnerID}
 
-			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))
+				collaborateCond := builder.And(
+					builder.Expr("id IN (SELECT repo_id FROM `access` WHERE access.user_id = ?)", opts.OwnerID),
+					builder.Neq{"owner_id": opts.OwnerID})
+				if !opts.Private {
+					collaborateCond = collaborateCond.And(builder.Expr("owner_id NOT IN (SELECT org_id FROM org_user WHERE org_user.uid = ? AND org_user.is_public = ?)", opts.OwnerID, false))
+				}
+
+				accessCond = accessCond.Or(collaborateCond)
 			}
-		}
-		cond = cond.And(searcherReposCond)
-	}
 
-	if !opts.Private {
-		cond = cond.And(builder.Eq{"is_private": false})
+			cond = cond.And(accessCond)
+		}
 	}
 
 	if opts.OwnerID > 0 && opts.AllPublic {
 		cond = cond.Or(builder.Eq{"is_private": false})
 	}
 
-	opts.Keyword = strings.ToLower(opts.Keyword)
 	if opts.Keyword != "" {
-		cond = cond.And(builder.Like{"lower_name", opts.Keyword})
+		cond = cond.And(builder.Like{"lower_name", strings.ToLower(opts.Keyword)})
 	}
 
 	if len(opts.OrderBy) == 0 {
@@ -196,26 +186,23 @@ 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 starred {
+		sess.Join("INNER", "star", "star.repo_id = repository.id")
+	}
 
+	count, err := sess.
+		Where(cond).
+		Count(new(Repository))
+	if err != nil {
+		return nil, 0, fmt.Errorf("Count: %v", err)
+	}
+
+	// Set again after reset by Count()
+	if starred {
 		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)
-		}
 	}
 
-	repos = make([]*Repository, 0, opts.PageSize)
+	repos := make(RepositoryList, 0, opts.PageSize)
 	if err = sess.
 		Where(cond).
 		Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).
@@ -230,5 +217,5 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (repos RepositoryList, coun
 		}
 	}
 
-	return
+	return repos, count, nil
 }