From 6473bd333a54f38f333920a6e567edafe6692ff5 Mon Sep 17 00:00:00 2001 From: Hugo Hoitink <10838836+hoitih@users.noreply.github.com> Date: Thu, 16 Jun 2022 01:24:10 +0200 Subject: [PATCH] In code search, get code unit accessible repos in one (main) query (#19764) * When non-admin users use code search, get code unit accessible repos in one main query * Modified some comments to match the changes * Removed unnecessary check for Access Mode in Collaboration table Co-authored-by: wxiaoguang Co-authored-by: Lauris BH --- models/git/lfs.go | 5 +- models/issues/issue.go | 4 +- models/org.go | 3 +- models/repo/repo_list.go | 69 +++++++++++------ routers/web/explore/code.go | 144 ++++++++++++++---------------------- 5 files changed, 112 insertions(+), 113 deletions(-) diff --git a/models/git/lfs.go b/models/git/lfs.go index 13b8b234f..ec963cf59 100644 --- a/models/git/lfs.go +++ b/models/git/lfs.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" @@ -213,7 +214,7 @@ func LFSObjectAccessible(user *user_model.User, oid string) (bool, error) { count, err := db.GetEngine(db.DefaultContext).Count(&LFSMetaObject{Pointer: lfs.Pointer{Oid: oid}}) return count > 0, err } - cond := repo_model.AccessibleRepositoryCondition(user) + cond := repo_model.AccessibleRepositoryCondition(user, unit.TypeInvalid) count, err := db.GetEngine(db.DefaultContext).Where(cond).Join("INNER", "repository", "`lfs_meta_object`.repository_id = `repository`.id").Count(&LFSMetaObject{Pointer: lfs.Pointer{Oid: oid}}) return count > 0, err } @@ -244,7 +245,7 @@ func LFSAutoAssociate(metas []*LFSMetaObject, user *user_model.User, repoID int6 newMetas := make([]*LFSMetaObject, 0, len(metas)) cond := builder.In( "`lfs_meta_object`.repository_id", - builder.Select("`repository`.id").From("repository").Where(repo_model.AccessibleRepositoryCondition(user)), + builder.Select("`repository`.id").From("repository").Where(repo_model.AccessibleRepositoryCondition(user, unit.TypeInvalid)), ) err = sess.Cols("oid").Where(cond).In("oid", oids...).GroupBy("oid").Find(&newMetas) if err != nil { diff --git a/models/issues/issue.go b/models/issues/issue.go index 0f4af3e84..76a0ea7d0 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -1430,7 +1430,7 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organizati cond = cond.And( builder.Or( repo_model.UserOwnedRepoCond(userID), // owned repos - repo_model.UserCollaborationRepoCond(repoIDstr, userID), // collaboration repos + repo_model.UserAccessRepoCond(repoIDstr, userID), // user can access repo in a unit independent way repo_model.UserAssignedRepoCond(repoIDstr, userID), // user has been assigned accessible public repos repo_model.UserMentionedRepoCond(repoIDstr, userID), // user has been mentioned accessible public repos repo_model.UserCreateIssueRepoCond(repoIDstr, userID, isPull), // user has created issue/pr accessible public repos @@ -1499,7 +1499,7 @@ func GetRepoIDsForIssuesOptions(opts *IssuesOptions, user *user_model.User) ([]i opts.setupSessionNoLimit(sess) - accessCond := repo_model.AccessibleRepositoryCondition(user) + accessCond := repo_model.AccessibleRepositoryCondition(user, unit.TypeInvalid) if err := sess.Where(accessCond). Distinct("issue.repo_id"). Table("issue"). diff --git a/models/org.go b/models/org.go index 009fe758b..849c9b985 100644 --- a/models/org.go +++ b/models/org.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models/organization" access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "xorm.io/builder" @@ -54,7 +55,7 @@ func GetUserOrgsList(user *user_model.User) ([]*MinimalOrg, error) { Join("LEFT", builder. Select("id as repo_id, owner_id as repo_owner_id"). From("repository"). - Where(repo_model.AccessibleRepositoryCondition(user)), "`repository`.repo_owner_id = `team`.org_id"). + Where(repo_model.AccessibleRepositoryCondition(user, unit.TypeInvalid)), "`repository`.repo_owner_id = `team`.org_id"). Where("`team_user`.uid = ?", user.ID). GroupBy(groupByStr) diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index 1bec35d57..a70fc8efd 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -269,8 +269,8 @@ func UserMentionedRepoCond(id string, userID int64) builder.Cond { ) } -// UserCollaborationRepoCond returns user as collabrators repositories list -func UserCollaborationRepoCond(idStr string, userID int64) builder.Cond { +// UserAccessRepoCond returns a condition for selecting all repositories a user has unit independent access to +func UserAccessRepoCond(idStr string, userID int64) builder.Cond { return builder.In(idStr, builder.Select("repo_id"). From("`access`"). Where(builder.And( @@ -280,8 +280,18 @@ func UserCollaborationRepoCond(idStr string, userID int64) builder.Cond { ) } -// userOrgTeamRepoCond selects repos that the given user has access to through team membership -func userOrgTeamRepoCond(idStr string, userID int64) builder.Cond { +// userCollaborationRepoCond returns a condition for selecting all repositories a user is collaborator in +func UserCollaborationRepoCond(idStr string, userID int64) builder.Cond { + return builder.In(idStr, builder.Select("repo_id"). + From("`collaboration`"). + Where(builder.And( + builder.Eq{"`collaboration`.user_id": userID}, + )), + ) +} + +// UserOrgTeamRepoCond selects repos that the given user has access to through team membership +func UserOrgTeamRepoCond(idStr string, userID int64) builder.Cond { return builder.In(idStr, userOrgTeamRepoBuilder(userID)) } @@ -297,7 +307,13 @@ func userOrgTeamRepoBuilder(userID int64) *builder.Builder { func userOrgTeamUnitRepoBuilder(userID int64, unitType unit.Type) *builder.Builder { return userOrgTeamRepoBuilder(userID). Join("INNER", "team_unit", "`team_unit`.team_id = `team_repo`.team_id"). - Where(builder.Eq{"`team_unit`.`type`": unitType}) + Where(builder.Eq{"`team_unit`.`type`": unitType}). + And(builder.Gt{"`team_unit`.`access_mode`": int(perm.AccessModeNone)}) +} + +// userOrgTeamUnitRepoCond returns a condition to select repo ids where user's teams can access the special unit. +func userOrgTeamUnitRepoCond(idStr string, userID int64, unitType unit.Type) builder.Cond { + return builder.In(idStr, userOrgTeamUnitRepoBuilder(userID, unitType)) } // UserOrgUnitRepoCond selects repos that the given user has access to through org and the special unit @@ -350,7 +366,7 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { if opts.Private { if opts.Actor != nil && !opts.Actor.IsAdmin && opts.Actor.ID != opts.OwnerID { // OK we're in the context of a User - cond = cond.And(AccessibleRepositoryCondition(opts.Actor)) + cond = cond.And(AccessibleRepositoryCondition(opts.Actor, unit.TypeInvalid)) } } else { // Not looking at private organisations and users @@ -395,10 +411,10 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { builder.Neq{"owner_id": opts.OwnerID}, // 2. But we can see because of: builder.Or( - // A. We have access - UserCollaborationRepoCond("`repository`.id", opts.OwnerID), + // A. We have unit independent access + UserAccessRepoCond("`repository`.id", opts.OwnerID), // B. We are in a team for - userOrgTeamRepoCond("`repository`.id", opts.OwnerID), + UserOrgTeamRepoCond("`repository`.id", opts.OwnerID), // C. Public repositories in organizations that we are member of userOrgPublicRepoCondPrivate(opts.OwnerID), ), @@ -479,7 +495,7 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { } if opts.Actor != nil && opts.Actor.IsRestricted { - cond = cond.And(AccessibleRepositoryCondition(opts.Actor)) + cond = cond.And(AccessibleRepositoryCondition(opts.Actor, unit.TypeInvalid)) } if opts.Archived != util.OptionalBoolNone { @@ -574,7 +590,7 @@ func searchRepositoryByCondition(ctx context.Context, opts *SearchRepoOptions, c } // AccessibleRepositoryCondition takes a user a returns a condition for checking if a repository is accessible -func AccessibleRepositoryCondition(user *user_model.User) builder.Cond { +func AccessibleRepositoryCondition(user *user_model.User, unitType unit.Type) builder.Cond { cond := builder.NewCond() if user == nil || !user.IsRestricted || user.ID <= 0 { @@ -594,13 +610,24 @@ func AccessibleRepositoryCondition(user *user_model.User) builder.Cond { } if user != nil { + // 2. Be able to see all repositories that we have unit independent access to + // 3. Be able to see all repositories through team membership(s) + if unitType == unit.TypeInvalid { + // Regardless of UnitType + cond = cond.Or( + UserAccessRepoCond("`repository`.id", user.ID), + UserOrgTeamRepoCond("`repository`.id", user.ID), + ) + } else { + // For a specific UnitType + cond = cond.Or( + UserCollaborationRepoCond("`repository`.id", user.ID), + userOrgTeamUnitRepoCond("`repository`.id", user.ID, unitType), + ) + } cond = cond.Or( - // 2. Be able to see all repositories that we have access to - UserCollaborationRepoCond("`repository`.id", user.ID), - // 3. Repositories that we directly own + // 4. Repositories that we directly own builder.Eq{"`repository`.owner_id": user.ID}, - // 4. Be able to see all repositories that we are in a team - userOrgTeamRepoCond("`repository`.id", user.ID), // 5. Be able to see all public repos in private organizations that we are an org_user of userOrgPublicRepoCond(user.ID), ) @@ -645,18 +672,18 @@ func SearchRepositoryIDs(opts *SearchRepoOptions) ([]int64, int64, error) { // AccessibleRepoIDsQuery queries accessible repository ids. Usable as a subquery wherever repo ids need to be filtered. func AccessibleRepoIDsQuery(user *user_model.User) *builder.Builder { // NB: Please note this code needs to still work if user is nil - return builder.Select("id").From("repository").Where(AccessibleRepositoryCondition(user)) + return builder.Select("id").From("repository").Where(AccessibleRepositoryCondition(user, unit.TypeInvalid)) } -// FindUserAccessibleRepoIDs find all accessible repositories' ID by user's id -func FindUserAccessibleRepoIDs(user *user_model.User) ([]int64, error) { +// FindUserCodeAccessibleRepoIDs finds all at Code level accessible repositories' ID by the user's id +func FindUserCodeAccessibleRepoIDs(user *user_model.User) ([]int64, error) { repoIDs := make([]int64, 0, 10) if err := db.GetEngine(db.DefaultContext). Table("repository"). Cols("id"). - Where(AccessibleRepositoryCondition(user)). + Where(AccessibleRepositoryCondition(user, unit.TypeCode)). Find(&repoIDs); err != nil { - return nil, fmt.Errorf("FindUserAccesibleRepoIDs: %v", err) + return nil, fmt.Errorf("FindUserCodeAccesibleRepoIDs: %v", err) } return repoIDs, nil } diff --git a/routers/web/explore/code.go b/routers/web/explore/code.go index 3fba2be37..3afb2110d 100644 --- a/routers/web/explore/code.go +++ b/routers/web/explore/code.go @@ -7,9 +7,7 @@ package explore import ( "net/http" - "code.gitea.io/gitea/models" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" code_indexer "code.gitea.io/gitea/modules/indexer/code" @@ -44,106 +42,78 @@ func Code(ctx *context.Context) { queryType := ctx.FormTrim("t") isMatch := queryType == "match" - var ( - repoIDs []int64 - err error - isAdmin bool - ) - if ctx.Doer != nil { - isAdmin = ctx.Doer.IsAdmin - } - - // guest user or non-admin user - if ctx.Doer == nil || !isAdmin { - repoIDs, err = repo_model.FindUserAccessibleRepoIDs(ctx.Doer) - if err != nil { - ctx.ServerError("SearchResults", err) - return - } - } - - var ( - total int - searchResults []*code_indexer.Result - searchResultLanguages []*code_indexer.SearchResultLanguages - ) - - // if non-admin login user, we need check UnitTypeCode at first - if ctx.Doer != nil && len(repoIDs) > 0 { - repoMaps, err := repo_model.GetRepositoriesMapByIDs(repoIDs) - if err != nil { - ctx.ServerError("SearchResults", err) - return + if keyword != "" { + var ( + repoIDs []int64 + err error + isAdmin bool + ) + if ctx.Doer != nil { + isAdmin = ctx.Doer.IsAdmin } - rightRepoMap := make(map[int64]*repo_model.Repository, len(repoMaps)) - repoIDs = make([]int64, 0, len(repoMaps)) - for id, repo := range repoMaps { - if models.CheckRepoUnitUser(ctx, repo, ctx.Doer, unit.TypeCode) { - rightRepoMap[id] = repo - repoIDs = append(repoIDs, id) - } - } - - ctx.Data["RepoMaps"] = rightRepoMap - - total, searchResults, searchResultLanguages, err = code_indexer.PerformSearch(ctx, repoIDs, language, keyword, page, setting.UI.RepoSearchPagingNum, isMatch) - if err != nil { - if code_indexer.IsAvailable() { + // guest user or non-admin user + if ctx.Doer == nil || !isAdmin { + repoIDs, err = repo_model.FindUserCodeAccessibleRepoIDs(ctx.Doer) + if err != nil { ctx.ServerError("SearchResults", err) return } - ctx.Data["CodeIndexerUnavailable"] = true - } else { - ctx.Data["CodeIndexerUnavailable"] = !code_indexer.IsAvailable() } - // if non-login user or isAdmin, no need to check UnitTypeCode - } else if (ctx.Doer == nil && len(repoIDs) > 0) || isAdmin { - total, searchResults, searchResultLanguages, err = code_indexer.PerformSearch(ctx, repoIDs, language, keyword, page, setting.UI.RepoSearchPagingNum, isMatch) - if err != nil { - if code_indexer.IsAvailable() { - ctx.ServerError("SearchResults", err) - return + + var ( + total int + searchResults []*code_indexer.Result + searchResultLanguages []*code_indexer.SearchResultLanguages + ) + + if (len(repoIDs) > 0) || isAdmin { + total, searchResults, searchResultLanguages, err = code_indexer.PerformSearch(ctx, repoIDs, language, keyword, page, setting.UI.RepoSearchPagingNum, isMatch) + if err != nil { + if code_indexer.IsAvailable() { + ctx.ServerError("SearchResults", err) + return + } + ctx.Data["CodeIndexerUnavailable"] = true + } else { + ctx.Data["CodeIndexerUnavailable"] = !code_indexer.IsAvailable() } - ctx.Data["CodeIndexerUnavailable"] = true - } else { - ctx.Data["CodeIndexerUnavailable"] = !code_indexer.IsAvailable() - } - loadRepoIDs := make([]int64, 0, len(searchResults)) - for _, result := range searchResults { - var find bool - for _, id := range loadRepoIDs { - if id == result.RepoID { - find = true - break + loadRepoIDs := make([]int64, 0, len(searchResults)) + for _, result := range searchResults { + var find bool + for _, id := range loadRepoIDs { + if id == result.RepoID { + find = true + break + } + } + if !find { + loadRepoIDs = append(loadRepoIDs, result.RepoID) } } - if !find { - loadRepoIDs = append(loadRepoIDs, result.RepoID) + + repoMaps, err := repo_model.GetRepositoriesMapByIDs(loadRepoIDs) + if err != nil { + ctx.ServerError("SearchResults", err) + return } - } - repoMaps, err := repo_model.GetRepositoriesMapByIDs(loadRepoIDs) - if err != nil { - ctx.ServerError("SearchResults", err) - return + ctx.Data["RepoMaps"] = repoMaps } - ctx.Data["RepoMaps"] = repoMaps + ctx.Data["Keyword"] = keyword + ctx.Data["Language"] = language + ctx.Data["queryType"] = queryType + ctx.Data["SearchResults"] = searchResults + ctx.Data["SearchResultLanguages"] = searchResultLanguages + ctx.Data["PageIsViewCode"] = true + + pager := context.NewPagination(total, setting.UI.RepoSearchPagingNum, page, 5) + pager.SetDefaultParams(ctx) + pager.AddParam(ctx, "l", "Language") + ctx.Data["Page"] = pager } - ctx.Data["Keyword"] = keyword - ctx.Data["Language"] = language - ctx.Data["queryType"] = queryType - ctx.Data["SearchResults"] = searchResults - ctx.Data["SearchResultLanguages"] = searchResultLanguages - ctx.Data["PageIsViewCode"] = true - - pager := context.NewPagination(total, setting.UI.RepoSearchPagingNum, page, 5) - pager.SetDefaultParams(ctx) - pager.AddParam(ctx, "l", "Language") - ctx.Data["Page"] = pager - ctx.HTML(http.StatusOK, tplExploreCode) }