From 1b5a937f926589bfb55379687521dd8f57f4d8d1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 27 Aug 2021 20:56:25 +0800 Subject: [PATCH 01/17] Use conditions but not repo ids as query condition --- models/issue.go | 25 ++++++++++ models/repo_list.go | 96 +++++++++++++++++++++++++++++++++++++++ models/repo_permission.go | 20 -------- routers/web/user/home.go | 81 ++++++--------------------------- 4 files changed, 136 insertions(+), 86 deletions(-) diff --git a/models/issue.go b/models/issue.go index ea8eb4a9aad44..2ac9e532be7be 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1186,6 +1186,9 @@ type IssuesOptions struct { // prioritize issues from this repo PriorityRepoID int64 IsArchived util.OptionalBool + Org *User // issues permission scope + Team *Team // issues permission scope + User *User // issues permission scope } // sortIssuesSession sort an issues-related session based on the provided @@ -1329,6 +1332,28 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { From("milestone"). Where(builder.In("name", opts.IncludeMilestones))) } + + if opts.User != nil { + if opts.Org != nil { + var unitType = UnitTypeIssues + if opts.IsPull.IsTrue() { + unitType = UnitTypePullRequests + } + + if opts.Team != nil { + sess.And(teamUnitsRepoCond("issue.repo_id", opts.User.ID, opts.Org.ID, opts.Team.ID, unitType)) + } else { + sess.And(orgUnitsRepoCond("issue.repo_id", opts.User.ID, opts.Org.ID, unitType)) + } + } else { + sess.And( + builder.Or( + userRepoCond(opts.User.ID), + userCollaborationRepoCond("issue.repo_id", opts.User.ID), + ), + ) + } + } } func applyReposCondition(sess *xorm.Session, repoIDs []int64) *xorm.Session { diff --git a/models/repo_list.go b/models/repo_list.go index 02440bec93da5..fef1fcc23c78b 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -11,6 +11,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/structs" "code.gitea.io/gitea/modules/util" @@ -145,6 +146,101 @@ type SearchRepoOptions struct { LowerNames []string } +// SearchOrderBy is used to sort the result +type SearchOrderBy string + +func (s SearchOrderBy) String() string { + return string(s) +} + +// Strings for sorting result +const ( + SearchOrderByAlphabetically SearchOrderBy = "name ASC" + SearchOrderByAlphabeticallyReverse SearchOrderBy = "name DESC" + SearchOrderByLeastUpdated SearchOrderBy = "updated_unix ASC" + SearchOrderByRecentUpdated SearchOrderBy = "updated_unix DESC" + SearchOrderByOldest SearchOrderBy = "created_unix ASC" + SearchOrderByNewest SearchOrderBy = "created_unix DESC" + SearchOrderBySize SearchOrderBy = "size ASC" + SearchOrderBySizeReverse SearchOrderBy = "size DESC" + SearchOrderByID SearchOrderBy = "id ASC" + SearchOrderByIDReverse SearchOrderBy = "id DESC" + SearchOrderByStars SearchOrderBy = "num_stars ASC" + SearchOrderByStarsReverse SearchOrderBy = "num_stars DESC" + SearchOrderByForks SearchOrderBy = "num_forks ASC" + SearchOrderByForksReverse SearchOrderBy = "num_forks DESC" +) + +// userRepoCond returns user ownered repositories +func userRepoCond(userID int64) builder.Cond { + return builder.Eq{ + "repository.owner_id": userID, + } +} + +// userCollaborationRepoCond return user as collabrators repositories list +func userCollaborationRepoCond(id string, userID int64) builder.Cond { + return builder.In(id, + builder.Select("repo_id").From("collaboration"). + Where(builder.Eq{ + "user_id": userID, + }), + ) +} + +func orgUnitsRepoCond(id string, userID, orgID int64, units ...unit.Type) builder.Cond { + return builder.In(id, + builder.Select("repo_id").From("team_repo").Where( + builder.Eq{ + "org_id": orgID, + }.And( + builder.In( + "team_id", builder.Select("team_id").From("team_user").Where( + builder.Eq{ + "uid": userID, + }, + ), + ), + ).And( + builder.In( + "team_id", builder.Select("team_id").From("team_unit").Where( + builder.Eq{ + "org_id": orgID, + }.And( + builder.In("type", units), + ), + ), + ), + ), + )) +} + +func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Type) builder.Cond { + return builder.In(id, + builder.Select("repo_id").From("team_repo").Where( + builder.Eq{ + "team_id": teamID, + }.And( + builder.In( + "team_id", builder.Select("team_id").From("team_user").Where( + builder.Eq{ + "uid": userID, + }, + ), + )).And( + builder.In( + "team_id", builder.Select("team_id").From("team_unit").Where( + builder.Eq{ + "org_id": orgID, + }.And( + builder.In("type", units), + ), + ), + ), + ), + )) +} + // SearchRepositoryCondition creates a query condition according search repository options func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { cond := builder.NewCond() diff --git a/models/repo_permission.go b/models/repo_permission.go index 45878c8ba4e08..40b63aa804313 100644 --- a/models/repo_permission.go +++ b/models/repo_permission.go @@ -400,26 +400,6 @@ func HasAccess(userID int64, repo *repo_model.Repository) (bool, error) { return hasAccess(db.DefaultContext, userID, repo) } -// FilterOutRepoIdsWithoutUnitAccess filter out repos where user has no access to repositories -func FilterOutRepoIdsWithoutUnitAccess(u *user_model.User, repoIDs []int64, units ...unit.Type) ([]int64, error) { - i := 0 - for _, rID := range repoIDs { - repo, err := repo_model.GetRepositoryByID(rID) - if err != nil { - return nil, err - } - perm, err := GetUserRepoPermission(repo, u) - if err != nil { - return nil, err - } - if perm.CanReadAny(units...) { - repoIDs[i] = rID - i++ - } - } - return repoIDs[:i], nil -} - // GetRepoReaders returns all users that have explicit read access or higher to the repository. func GetRepoReaders(repo *repo_model.Repository) (_ []*user_model.User, err error) { return getUsersWithAccessMode(db.DefaultContext, repo, perm_model.AccessModeRead) diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 975262cf9c78b..0bd2e47f744be 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -403,27 +403,26 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { // - Count Issues by repo // -------------------------------------------------------------------------- - isPullList := unitType == unit.TypePullRequests - opts := &models.IssuesOptions{ - IsPull: util.OptionalBoolOf(isPullList), - SortType: sortType, - IsArchived: util.OptionalBoolFalse, - } - // Get repository IDs where User/Org/Team has access. var team *models.Team + var org *models.User if ctx.Org != nil { + org = ctx.Org.Organization team = ctx.Org.Team } - userRepoIDs, err := getActiveUserRepoIDs(ctxUser, team, unitType) - if err != nil { - ctx.ServerError("userRepoIDs", err) - return + + isPullList := unitType == models.UnitTypePullRequests + opts := &models.IssuesOptions{ + IsPull: util.OptionalBoolOf(isPullList), + SortType: sortType, + IsArchived: util.OptionalBoolFalse, + Org: org, + Team: team, + User: ctx.User, } switch filterMode { case models.FilterModeAll: - opts.RepoIDs = userRepoIDs case models.FilterModeAssign: opts.AssigneeID = ctx.User.ID case models.FilterModeCreate: @@ -434,10 +433,6 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { opts.ReviewRequestedID = ctx.User.ID } - if ctxUser.IsOrganization() { - opts.RepoIDs = userRepoIDs - } - // keyword holds the search term entered into the search field. keyword := strings.Trim(ctx.FormString("q"), " ") ctx.Data["Keyword"] = keyword @@ -556,6 +551,10 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { // ------------------------------- // Fill stats to post to ctx.Data. // ------------------------------- + var userRepoIDs = make([]int64, 0, len(issueCountByRepo)) + for id := range issueCountByRepo { + userRepoIDs = append(userRepoIDs, id) + } userIssueStatsOpts := models.UserIssueStatsOptions{ UserID: ctx.User.ID, @@ -730,56 +729,6 @@ func getRepoIDs(reposQuery string) []int64 { return repoIDs } -func getActiveUserRepoIDs(ctxUser *user_model.User, team *models.Team, unitType unit.Type) ([]int64, error) { - var userRepoIDs []int64 - var err error - - if ctxUser.IsOrganization() { - userRepoIDs, err = getActiveTeamOrOrgRepoIds(ctxUser, team, unitType) - if err != nil { - return nil, fmt.Errorf("orgRepoIds: %v", err) - } - } else { - userRepoIDs, err = models.GetActiveAccessRepoIDs(ctxUser, unitType) - if err != nil { - return nil, fmt.Errorf("ctxUser.GetAccessRepoIDs: %v", err) - } - } - - if len(userRepoIDs) == 0 { - userRepoIDs = []int64{-1} - } - - return userRepoIDs, nil -} - -// getActiveTeamOrOrgRepoIds gets RepoIDs for ctxUser as Organization. -// Should be called if and only if ctxUser.IsOrganization == true. -func getActiveTeamOrOrgRepoIds(ctxUser *user_model.User, team *models.Team, unitType unit.Type) ([]int64, error) { - var orgRepoIDs []int64 - var err error - var env models.AccessibleReposEnvironment - - if team != nil { - env = models.OrgFromUser(ctxUser).AccessibleTeamReposEnv(team) - } else { - env, err = models.OrgFromUser(ctxUser).AccessibleReposEnv(ctxUser.ID) - if err != nil { - return nil, fmt.Errorf("AccessibleReposEnv: %v", err) - } - } - orgRepoIDs, err = env.RepoIDs(1, ctxUser.NumRepos) - if err != nil { - return nil, fmt.Errorf("env.RepoIDs: %v", err) - } - orgRepoIDs, err = models.FilterOutRepoIdsWithoutUnitAccess(ctxUser, orgRepoIDs, unitType) - if err != nil { - return nil, fmt.Errorf("FilterOutRepoIdsWithoutUnitAccess: %v", err) - } - - return orgRepoIDs, nil -} - func issueIDsFromSearch(ctxUser *user_model.User, keyword string, opts *models.IssuesOptions) ([]int64, error) { if len(keyword) == 0 { return []int64{}, nil From 5fd81e48bb3c7c823df5fbced1536e0b6119115a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 27 Aug 2021 23:13:14 +0800 Subject: [PATCH 02/17] Improve the performance of pulls/issue --- models/issue.go | 47 ++++++++++++------ models/issue_list.go | 11 ++++- models/repo_list.go | 6 +++ routers/web/user/home.go | 103 ++++++++++++++++----------------------- 4 files changed, 88 insertions(+), 79 deletions(-) diff --git a/models/issue.go b/models/issue.go index 2ac9e532be7be..699e0365b862d 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1334,26 +1334,35 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } if opts.User != nil { - if opts.Org != nil { - var unitType = UnitTypeIssues - if opts.IsPull.IsTrue() { - unitType = UnitTypePullRequests - } + sess.And( + accessibleRepositoryCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue()), + ) + } +} - if opts.Team != nil { - sess.And(teamUnitsRepoCond("issue.repo_id", opts.User.ID, opts.Org.ID, opts.Team.ID, unitType)) - } else { - sess.And(orgUnitsRepoCond("issue.repo_id", opts.User.ID, opts.Org.ID, unitType)) - } +// accessibleRepositoryCond user must not be nil +func accessibleRepositoryCond(repoIDstr string, userID int64, org *User, team *Team, isPull bool) builder.Cond { + var cond = builder.NewCond() + if org != nil { + var unitType = UnitTypeIssues + if isPull { + unitType = UnitTypePullRequests + } + + if team != nil { + cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, org.ID, team.ID, unitType)) } else { - sess.And( - builder.Or( - userRepoCond(opts.User.ID), - userCollaborationRepoCond("issue.repo_id", opts.User.ID), - ), - ) + cond = cond.And(orgUnitsRepoCond(repoIDstr, userID, org.ID, unitType)) } + } else { + cond = cond.And( + builder.Or( + userRepoCond(userID), + userCollaborationRepoCond(repoIDstr, userID), + ), + ) } + return cond } func applyReposCondition(sess *xorm.Session, repoIDs []int64) *xorm.Session { @@ -1672,6 +1681,8 @@ type UserIssueStatsOptions struct { IssueIDs []int64 IsArchived util.OptionalBool LabelIDs []int64 + Org *User + Team *Team } // GetUserIssueStats returns issue statistic information for dashboard by given conditions. @@ -1688,6 +1699,10 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) { cond = cond.And(builder.In("issue.id", opts.IssueIDs)) } + if opts.UserID > 0 { + cond = cond.And(accessibleRepositoryCond("issue.repo_id", opts.UserID, opts.Org, opts.Team, opts.IsPull)) + } + sess := func(cond builder.Cond) *xorm.Session { s := db.GetEngine(db.DefaultContext).Where(cond) if len(opts.LabelIDs) > 0 { diff --git a/models/issue_list.go b/models/issue_list.go index 5d8a9f692173b..b516e7336e026 100644 --- a/models/issue_list.go +++ b/models/issue_list.go @@ -25,6 +25,9 @@ const ( func (issues IssueList) getRepoIDs() []int64 { repoIDs := make(map[int64]struct{}, len(issues)) for _, issue := range issues { + if issue.Repo != nil { + continue + } if _, ok := repoIDs[issue.RepoID]; !ok { repoIDs[issue.RepoID] = struct{}{} } @@ -56,8 +59,12 @@ func (issues IssueList) loadRepositories(e db.Engine) ([]*repo_model.Repository, } for _, issue := range issues { - issue.Repo = repoMaps[issue.RepoID] - if issue.PullRequest != nil { + if issue.Repo == nil { + issue.Repo = repoMaps[issue.RepoID] + } else { + repoMaps[issue.RepoID] = issue.Repo + } + if issue.PullRequest != nil && issue.PullRequest.BaseRepo == nil { issue.PullRequest.BaseRepo = issue.Repo } } diff --git a/models/repo_list.go b/models/repo_list.go index fef1fcc23c78b..019b415dca08e 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -560,6 +560,7 @@ func FindUserAccessibleRepoIDs(user *user_model.User) ([]int64, error) { return repoIDs, nil } +<<<<<<< HEAD // GetUserRepositories returns a list of repositories of given user. func GetUserRepositories(opts *SearchRepoOptions) ([]*repo_model.Repository, int64, error) { if len(opts.OrderBy) == 0 { @@ -586,4 +587,9 @@ func GetUserRepositories(opts *SearchRepoOptions) ([]*repo_model.Repository, int sess = sess.Where(cond).OrderBy(opts.OrderBy.String()) repos := make([]*repo_model.Repository, 0, opts.PageSize) return repos, count, db.SetSessionPagination(sess, opts).Find(&repos) +======= +// FindReposMapByIDs find repos as map +func FindReposMapByIDs(repoIDs []int64, res map[int64]*Repository) error { + return x.In("id", repoIDs).Find(&res) +>>>>>>> f6ddec83b (Improve the performance of pulls/issue) } diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 0bd2e47f744be..6b1b5484146fd 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -519,27 +519,25 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { // ---------------------------------- // showReposMap maps repository IDs to their Repository pointers. - showReposMap, err := repoIDMap(ctxUser, issueCountByRepo, unitType) + showReposMap, err := loadRepoByIDs(ctxUser, issueCountByRepo, unitType) if err != nil { if repo_model.IsErrRepoNotExist(err) { ctx.NotFound("GetRepositoryByID", err) return } - ctx.ServerError("repoIDMap", err) + ctx.ServerError("loadRepoByIDs", err) return } // a RepositoryList showRepos := models.RepositoryListOfMap(showReposMap) sort.Sort(showRepos) - if err = showRepos.LoadAttributes(); err != nil { - ctx.ServerError("LoadAttributes", err) - return - } // maps pull request IDs to their CommitStatus. Will be posted to ctx.Data. for _, issue := range issues { - issue.Repo = showReposMap[issue.RepoID] + if issue.Repo == nil { + issue.Repo = showReposMap[issue.RepoID] + } } commitStatus, err := pull_service.GetIssuesLastCommitStatus(issues) @@ -551,26 +549,20 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { // ------------------------------- // Fill stats to post to ctx.Data. // ------------------------------- - var userRepoIDs = make([]int64, 0, len(issueCountByRepo)) - for id := range issueCountByRepo { - userRepoIDs = append(userRepoIDs, id) - } - userIssueStatsOpts := models.UserIssueStatsOptions{ - UserID: ctx.User.ID, - UserRepoIDs: userRepoIDs, - FilterMode: filterMode, - IsPull: isPullList, - IsClosed: isShowClosed, - IsArchived: util.OptionalBoolFalse, - LabelIDs: opts.LabelIDs, + UserID: ctx.User.ID, + FilterMode: filterMode, + IsPull: isPullList, + IsClosed: isShowClosed, + IsArchived: util.OptionalBoolFalse, + LabelIDs: opts.LabelIDs, + Org: org, + Team: team, } if len(repoIDs) > 0 { userIssueStatsOpts.UserRepoIDs = repoIDs } - if ctxUser.IsOrganization() { - userIssueStatsOpts.RepoIDs = userRepoIDs - } + userIssueStats, err := models.GetUserIssueStats(userIssueStatsOpts) if err != nil { ctx.ServerError("GetUserIssueStats User", err) @@ -580,19 +572,18 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { var shownIssueStats *models.IssueStats if !forceEmpty { statsOpts := models.UserIssueStatsOptions{ - UserID: ctx.User.ID, - UserRepoIDs: userRepoIDs, - FilterMode: filterMode, - IsPull: isPullList, - IsClosed: isShowClosed, - IssueIDs: issueIDsFromSearch, - IsArchived: util.OptionalBoolFalse, - LabelIDs: opts.LabelIDs, + UserID: ctx.User.ID, + FilterMode: filterMode, + IsPull: isPullList, + IsClosed: isShowClosed, + IssueIDs: issueIDsFromSearch, + IsArchived: util.OptionalBoolFalse, + LabelIDs: opts.LabelIDs, + Org: org, + Team: team, } if len(repoIDs) > 0 { statsOpts.RepoIDs = repoIDs - } else if ctxUser.IsOrganization() { - statsOpts.RepoIDs = userRepoIDs } shownIssueStats, err = models.GetUserIssueStats(statsOpts) if err != nil { @@ -606,17 +597,13 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { var allIssueStats *models.IssueStats if !forceEmpty { allIssueStatsOpts := models.UserIssueStatsOptions{ - UserID: ctx.User.ID, - UserRepoIDs: userRepoIDs, - FilterMode: filterMode, - IsPull: isPullList, - IsClosed: isShowClosed, - IssueIDs: issueIDsFromSearch, - IsArchived: util.OptionalBoolFalse, - LabelIDs: opts.LabelIDs, - } - if ctxUser.IsOrganization() { - allIssueStatsOpts.RepoIDs = userRepoIDs + UserID: ctx.User.ID, + FilterMode: filterMode, + IsPull: isPullList, + IsClosed: isShowClosed, + IssueIDs: issueIDsFromSearch, + IsArchived: util.OptionalBoolFalse, + LabelIDs: opts.LabelIDs, } allIssueStats, err = models.GetUserIssueStats(allIssueStatsOpts) if err != nil { @@ -746,33 +733,27 @@ func issueIDsFromSearch(ctxUser *user_model.User, keyword string, opts *models.I return issueIDsFromSearch, nil } -func repoIDMap(ctxUser *user_model.User, issueCountByRepo map[int64]int64, unitType unit.Type) (map[int64]*repo_model.Repository, error) { - repoByID := make(map[int64]*repo_model.Repository, len(issueCountByRepo)) +func loadRepoByIDs(ctxUser *user_model.User, issueCountByRepo map[int64]int64, unitType unit.Type) (map[int64]*repo_model.Repository, error) { + var totalRes = make(map[int64]*repo_model.Repository, len(issueCountByRepo)) + var repoIDs = make([]int64, 0, 500) for id := range issueCountByRepo { if id <= 0 { continue } - if _, ok := repoByID[id]; !ok { - repo, err := repo_model.GetRepositoryByID(id) - if repo_model.IsErrRepoNotExist(err) { + repoIDs = append(repoIDs, id) + if len(repoIDs) == 500 { + if err := models.FindReposMapByIDs(repoIDs, totalRes); err != nil { return nil, err - } else if err != nil { - return nil, fmt.Errorf("GetRepositoryByID: [%d]%v", id, err) } - repoByID[id] = repo + repoIDs = make([]int64, 0, 500) } - repo := repoByID[id] - - // Check if user has access to given repository. - perm, err := models.GetUserRepoPermission(repo, ctxUser) - if err != nil { - return nil, fmt.Errorf("GetUserRepoPermission: [%d]%v", id, err) - } - if !perm.CanRead(unitType) { - log.Debug("User created Issues in Repository which they no longer have access to: [%d]", id) + } + if len(repoIDs) > 0 { + if err := models.FindReposMapByIDs(repoIDs, totalRes); err != nil { + return nil, err } } - return repoByID, nil + return totalRes, nil } // ShowSSHKeys output all the ssh keys of user by uid From c798233593206d613e369541d38e5f8ea3b9f8e8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 27 Aug 2021 23:30:47 +0800 Subject: [PATCH 03/17] Remove duplicated code --- models/issue.go | 27 +++++++------ routers/web/user/home.go | 57 ++++------------------------ templates/user/dashboard/issues.tmpl | 4 +- 3 files changed, 23 insertions(+), 65 deletions(-) diff --git a/models/issue.go b/models/issue.go index 699e0365b862d..7b68248bd2404 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1672,17 +1672,16 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64) (*IssueStats, // UserIssueStatsOptions contains parameters accepted by GetUserIssueStats. type UserIssueStatsOptions struct { - UserID int64 - RepoIDs []int64 - UserRepoIDs []int64 - FilterMode int - IsPull bool - IsClosed bool - IssueIDs []int64 - IsArchived util.OptionalBool - LabelIDs []int64 - Org *User - Team *Team + UserID int64 + RepoIDs []int64 + FilterMode int + IsPull bool + IsClosed bool + IssueIDs []int64 + IsArchived util.OptionalBool + LabelIDs []int64 + Org *User + Team *Team } // GetUserIssueStats returns issue statistic information for dashboard by given conditions. @@ -1718,13 +1717,13 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) { switch opts.FilterMode { case FilterModeAll: - stats.OpenCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs). + stats.OpenCount, err = sess(cond). And("issue.is_closed = ?", false). Count(new(Issue)) if err != nil { return nil, err } - stats.ClosedCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs). + stats.ClosedCount, err = sess(cond). And("issue.is_closed = ?", true). Count(new(Issue)) if err != nil { @@ -1800,7 +1799,7 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) { return nil, err } - stats.YourRepositoriesCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs).Count(new(Issue)) + stats.YourRepositoriesCount, err = sess(cond).Count(new(Issue)) if err != nil { return nil, err } diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 6b1b5484146fd..b6e338ab641d8 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -549,27 +549,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { // ------------------------------- // Fill stats to post to ctx.Data. // ------------------------------- - userIssueStatsOpts := models.UserIssueStatsOptions{ - UserID: ctx.User.ID, - FilterMode: filterMode, - IsPull: isPullList, - IsClosed: isShowClosed, - IsArchived: util.OptionalBoolFalse, - LabelIDs: opts.LabelIDs, - Org: org, - Team: team, - } - if len(repoIDs) > 0 { - userIssueStatsOpts.UserRepoIDs = repoIDs - } - - userIssueStats, err := models.GetUserIssueStats(userIssueStatsOpts) - if err != nil { - ctx.ServerError("GetUserIssueStats User", err) - return - } - - var shownIssueStats *models.IssueStats + var issueStats *models.IssueStats if !forceEmpty { statsOpts := models.UserIssueStatsOptions{ UserID: ctx.User.ID, @@ -585,43 +565,23 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { if len(repoIDs) > 0 { statsOpts.RepoIDs = repoIDs } - shownIssueStats, err = models.GetUserIssueStats(statsOpts) + issueStats, err = models.GetUserIssueStats(statsOpts) if err != nil { ctx.ServerError("GetUserIssueStats Shown", err) return } } else { - shownIssueStats = &models.IssueStats{} - } - - var allIssueStats *models.IssueStats - if !forceEmpty { - allIssueStatsOpts := models.UserIssueStatsOptions{ - UserID: ctx.User.ID, - FilterMode: filterMode, - IsPull: isPullList, - IsClosed: isShowClosed, - IssueIDs: issueIDsFromSearch, - IsArchived: util.OptionalBoolFalse, - LabelIDs: opts.LabelIDs, - } - allIssueStats, err = models.GetUserIssueStats(allIssueStatsOpts) - if err != nil { - ctx.ServerError("GetUserIssueStats All", err) - return - } - } else { - allIssueStats = &models.IssueStats{} + issueStats = &models.IssueStats{} } // Will be posted to ctx.Data. var shownIssues int if !isShowClosed { - shownIssues = int(shownIssueStats.OpenCount) - ctx.Data["TotalIssueCount"] = int(allIssueStats.OpenCount) + shownIssues = int(issueStats.OpenCount) + ctx.Data["TotalIssueCount"] = shownIssues } else { - shownIssues = int(shownIssueStats.ClosedCount) - ctx.Data["TotalIssueCount"] = int(allIssueStats.ClosedCount) + shownIssues = int(issueStats.ClosedCount) + ctx.Data["TotalIssueCount"] = shownIssues } ctx.Data["IsShowClosed"] = isShowClosed @@ -657,8 +617,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { ctx.Data["CommitStatus"] = commitStatus ctx.Data["Repos"] = showRepos ctx.Data["Counts"] = issueCountByRepo - ctx.Data["IssueStats"] = userIssueStats - ctx.Data["ShownIssueStats"] = shownIssueStats + ctx.Data["IssueStats"] = issueStats ctx.Data["ViewType"] = viewType ctx.Data["SortType"] = sortType ctx.Data["RepoIDs"] = repoIDs diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index aca61f9ae906c..dc66859b8371b 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -61,11 +61,11 @@
{{svg "octicon-issue-opened" 16 "mr-3"}} - {{.i18n.Tr "repo.issues.open_tab" .ShownIssueStats.OpenCount}} + {{.i18n.Tr "repo.issues.open_tab" .IssueStats.OpenCount}} {{svg "octicon-issue-closed" 16 "mr-3"}} - {{.i18n.Tr "repo.issues.close_tab" .ShownIssueStats.ClosedCount}} + {{.i18n.Tr "repo.issues.close_tab" .IssueStats.ClosedCount}}
From bcf7ae4081554f386a8c162723c298291b6d3896 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 28 Aug 2021 15:24:39 +0800 Subject: [PATCH 04/17] fix lint --- models/issue_test.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/models/issue_test.go b/models/issue_test.go index eadeb66ab9207..613eee6aa4a49 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -239,21 +239,6 @@ func TestGetUserIssueStats(t *testing.T) { ClosedCount: 0, }, }, - { - UserIssueStatsOptions{ - UserID: 2, - UserRepoIDs: []int64{1, 2}, - FilterMode: FilterModeAll, - IsClosed: true, - }, - IssueStats{ - YourRepositoriesCount: 2, - AssignCount: 0, - CreateCount: 2, - OpenCount: 2, - ClosedCount: 2, - }, - }, { UserIssueStatsOptions{ UserID: 1, From a8095083de867f0bcabbe3a0bf07e9b0328e0adc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 28 Aug 2021 16:09:39 +0800 Subject: [PATCH 05/17] Fix bug --- models/issue.go | 21 +++++++++++---------- models/issue_test.go | 12 +++++++----- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/models/issue.go b/models/issue.go index 7b68248bd2404..83688750d8517 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1335,13 +1335,13 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { if opts.User != nil { sess.And( - accessibleRepositoryCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue()), + issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue()), ) } } -// accessibleRepositoryCond user must not be nil -func accessibleRepositoryCond(repoIDstr string, userID int64, org *User, team *Team, isPull bool) builder.Cond { +// issuePullAccessibleRepoCond userID must not be zero, this condition require join repository table +func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *User, team *Team, isPull bool) builder.Cond { var cond = builder.NewCond() if org != nil { var unitType = UnitTypeIssues @@ -1350,15 +1350,16 @@ func accessibleRepositoryCond(repoIDstr string, userID int64, org *User, team *T } if team != nil { - cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, org.ID, team.ID, unitType)) + cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, org.ID, team.ID, unitType)) // special team member repos } else { - cond = cond.And(orgUnitsRepoCond(repoIDstr, userID, org.ID, unitType)) + cond = cond.And(orgUnitsRepoCond(repoIDstr, userID, org.ID, unitType)) // team member repos } } else { cond = cond.And( builder.Or( - userRepoCond(userID), - userCollaborationRepoCond(repoIDstr, userID), + userRepoCond(userID), // owned repos + userCollaborationRepoCond(repoIDstr, userID), // collaboration repos + // created issue/pull repos ), ) } @@ -1699,18 +1700,18 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) { } if opts.UserID > 0 { - cond = cond.And(accessibleRepositoryCond("issue.repo_id", opts.UserID, opts.Org, opts.Team, opts.IsPull)) + cond = cond.And(issuePullAccessibleRepoCond("issue.repo_id", opts.UserID, opts.Org, opts.Team, opts.IsPull)) } sess := func(cond builder.Cond) *xorm.Session { s := db.GetEngine(db.DefaultContext).Where(cond) + s.Join("INNER", "repository", "issue.repo_id = repository.id") if len(opts.LabelIDs) > 0 { s.Join("INNER", "issue_label", "issue_label.issue_id = issue.id"). In("issue_label.label_id", opts.LabelIDs) } if opts.IsArchived != util.OptionalBoolNone { - s.Join("INNER", "repository", "issue.repo_id = repository.id"). - And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()}) + s.And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()}) } return s } diff --git a/models/issue_test.go b/models/issue_test.go index 613eee6aa4a49..1be5f6f270a43 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -267,11 +267,13 @@ func TestGetUserIssueStats(t *testing.T) { }, }, } { - stats, err := GetUserIssueStats(test.Opts) - if !assert.NoError(t, err) { - continue - } - assert.Equal(t, test.ExpectedIssueStats, *stats) + t.Run(fmt.Sprintf("%#v", test.Opts), func(t *testing.T) { + stats, err := GetUserIssueStats(test.Opts) + if !assert.NoError(t, err) { + return + } + assert.Equal(t, test.ExpectedIssueStats, *stats) + }) } } From 3f7b7f6c5f611f931e546eb73f568d3ddf224d0b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 30 Aug 2021 00:12:05 +0800 Subject: [PATCH 06/17] Fix stats --- models/issue.go | 17 ++++++------- models/issue_test.go | 29 ++++++++++++++++------ models/repo_list.go | 57 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 17 deletions(-) diff --git a/models/issue.go b/models/issue.go index 83688750d8517..b461024d09193 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1343,12 +1343,11 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { // issuePullAccessibleRepoCond userID must not be zero, this condition require join repository table func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *User, team *Team, isPull bool) builder.Cond { var cond = builder.NewCond() + var unitType = UnitTypeIssues + if isPull { + unitType = UnitTypePullRequests + } if org != nil { - var unitType = UnitTypeIssues - if isPull { - unitType = UnitTypePullRequests - } - if team != nil { cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, org.ID, team.ID, unitType)) // special team member repos } else { @@ -1357,9 +1356,11 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *User, team } else { cond = cond.And( builder.Or( - userRepoCond(userID), // owned repos - userCollaborationRepoCond(repoIDstr, userID), // collaboration repos - // created issue/pull repos + userOwnedRepoCond(userID), // owned repos + userCollaborationRepoCond(repoIDstr, userID), // collaboration repos + userAssignedRepoCond(repoIDstr, userID), // user has been assigned accessible repos + userMentionedRepoCond(repoIDstr, userID), // user has been mentioned accessible repos + userCreateIssueRepoCond(repoIDstr, userID, unitType), // user has created issue/pr accessible repos ), ) } diff --git a/models/issue_test.go b/models/issue_test.go index 1be5f6f270a43..9558789d97af1 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -206,11 +206,26 @@ func TestGetUserIssueStats(t *testing.T) { FilterMode: FilterModeAll, }, IssueStats{ - YourRepositoriesCount: 0, - AssignCount: 1, - CreateCount: 1, - OpenCount: 0, - ClosedCount: 0, + YourRepositoriesCount: 1, // 6 + AssignCount: 1, // 6 + CreateCount: 1, // 6 + OpenCount: 1, // 6 + ClosedCount: 1, // 1 + }, + }, + { + UserIssueStatsOptions{ + UserID: 1, + RepoIDs: []int64{1}, + FilterMode: FilterModeAll, + IsClosed: true, + }, + IssueStats{ + YourRepositoriesCount: 1, // 6 + AssignCount: 0, + CreateCount: 0, + OpenCount: 1, // 6 + ClosedCount: 1, // 1 }, }, { @@ -226,7 +241,7 @@ func TestGetUserIssueStats(t *testing.T) { ClosedCount: 0, }, }, - { + /*{ UserIssueStatsOptions{ UserID: 1, FilterMode: FilterModeCreate, @@ -265,7 +280,7 @@ func TestGetUserIssueStats(t *testing.T) { OpenCount: 1, ClosedCount: 0, }, - }, + },*/ } { t.Run(fmt.Sprintf("%#v", test.Opts), func(t *testing.T) { stats, err := GetUserIssueStats(test.Opts) diff --git a/models/repo_list.go b/models/repo_list.go index 019b415dca08e..c849ab542c7ba 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -171,8 +171,8 @@ const ( SearchOrderByForksReverse SearchOrderBy = "num_forks DESC" ) -// userRepoCond returns user ownered repositories -func userRepoCond(userID int64) builder.Cond { +// userOwnedRepoCond returns user ownered repositories +func userOwnedRepoCond(userID int64) builder.Cond { return builder.Eq{ "repository.owner_id": userID, } @@ -188,6 +188,59 @@ func userCollaborationRepoCond(id string, userID int64) builder.Cond { ) } +// userAssignedRepoCond return user as assignee repositories list +func userAssignedRepoCond(id string, userID int64) builder.Cond { + return builder.And( + builder.Eq{ + "repository.is_private": false, + }, + builder.In(id, + builder.Select("issue.repo_id").From("issue_assignees"). + InnerJoin("issue", "issue.id = issue_assignees.issue_id"). + Where(builder.Eq{ + "issue_assignees.assignee_id": userID, + }), + ), + ) +} + +// userCreateIssueRepoCond return user created issues repositories list +func userCreateIssueRepoCond(id string, userID int64, unitType unit.Type) builder.Cond { + var isPull = false + if unitType == unit.TypePullRequests { + isPull = true + } + return builder.And( + builder.Eq{ + "repository.is_private": false, + }, + builder.In(id, + builder.Select("issue.repo_id").From("issue"). + Where(builder.Eq{ + "issue.poster_id": userID, + "issue.is_pull": isPull, + }), + ), + ) +} + +// userMentionedRepoCond return user metinoed repositories list +func userMentionedRepoCond(id string, userID int64) builder.Cond { + return builder.And( + builder.Eq{ + "repository.is_private": false, + }, + builder.In(id, + builder.Select("issue.repo_id").From("issue_user"). + InnerJoin("issue", "issue.id = issue_user.issue_id"). + Where(builder.Eq{ + "issue_user.is_mentioned": true, + "issue_user.uid": userID, + }), + ), + ) +} + func orgUnitsRepoCond(id string, userID, orgID int64, units ...unit.Type) builder.Cond { return builder.In(id, builder.Select("repo_id").From("team_repo").Where( From 7d35f2d7a22158aac774e828c28ef713bbec56cd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 30 Aug 2021 16:17:59 +0800 Subject: [PATCH 07/17] More fixes --- models/issue.go | 17 +++-- models/issue_test.go | 35 ++++----- models/repo_list.go | 176 ++++++++++++++++++++++++------------------- 3 files changed, 129 insertions(+), 99 deletions(-) diff --git a/models/issue.go b/models/issue.go index b461024d09193..4e9ceefa54dec 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1351,16 +1351,21 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *User, team if team != nil { cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, org.ID, team.ID, unitType)) // special team member repos } else { - cond = cond.And(orgUnitsRepoCond(repoIDstr, userID, org.ID, unitType)) // team member repos + cond = cond.And( + builder.Or( + userOrgUnitRepoCond(repoIDstr, userID, org.ID, unitType), // team member repos + userOrgPublicUnitRepoCond(userID, org.ID), // user org public non-member repos, TODO: check repo has issues + ), + ) } } else { cond = cond.And( builder.Or( - userOwnedRepoCond(userID), // owned repos - userCollaborationRepoCond(repoIDstr, userID), // collaboration repos - userAssignedRepoCond(repoIDstr, userID), // user has been assigned accessible repos - userMentionedRepoCond(repoIDstr, userID), // user has been mentioned accessible repos - userCreateIssueRepoCond(repoIDstr, userID, unitType), // user has created issue/pr accessible repos + userOwnedRepoCond(userID), // owned repos + userCollaborationRepoCond(repoIDstr, userID), // collaboration repos + userAssignedRepoCond(repoIDstr, userID), // user has been assigned accessible public repos + userMentionedRepoCond(repoIDstr, userID), // user has been mentioned accessible public repos + userCreateIssueRepoCond(repoIDstr, userID, isPull), // user has created issue/pr accessible public repos ), ) } diff --git a/models/issue_test.go b/models/issue_test.go index 9558789d97af1..cebf20af9bbb7 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -234,23 +234,23 @@ func TestGetUserIssueStats(t *testing.T) { FilterMode: FilterModeAssign, }, IssueStats{ - YourRepositoriesCount: 0, - AssignCount: 2, - CreateCount: 2, - OpenCount: 2, + YourRepositoriesCount: 1, // 6 + AssignCount: 1, // 6 + CreateCount: 1, // 6 + OpenCount: 1, // 6 ClosedCount: 0, }, }, - /*{ + { UserIssueStatsOptions{ UserID: 1, FilterMode: FilterModeCreate, }, IssueStats{ - YourRepositoriesCount: 0, - AssignCount: 2, - CreateCount: 2, - OpenCount: 2, + YourRepositoriesCount: 1, // 6 + AssignCount: 1, // 6 + CreateCount: 1, // 6 + OpenCount: 1, // 6 ClosedCount: 0, }, }, @@ -260,9 +260,10 @@ func TestGetUserIssueStats(t *testing.T) { FilterMode: FilterModeMention, }, IssueStats{ - YourRepositoriesCount: 0, - AssignCount: 2, - CreateCount: 2, + YourRepositoriesCount: 1, // 6 + AssignCount: 1, // 6 + CreateCount: 1, // 6 + MentionCount: 0, OpenCount: 0, ClosedCount: 0, }, @@ -274,13 +275,13 @@ func TestGetUserIssueStats(t *testing.T) { IssueIDs: []int64{1}, }, IssueStats{ - YourRepositoriesCount: 0, - AssignCount: 1, - CreateCount: 1, - OpenCount: 1, + YourRepositoriesCount: 1, // 1 + AssignCount: 1, // 1 + CreateCount: 1, // 1 + OpenCount: 1, // 1 ClosedCount: 0, }, - },*/ + }, } { t.Run(fmt.Sprintf("%#v", test.Opts), func(t *testing.T) { stats, err := GetUserIssueStats(test.Opts) diff --git a/models/repo_list.go b/models/repo_list.go index c849ab542c7ba..62c7820adb764 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -178,16 +178,6 @@ func userOwnedRepoCond(userID int64) builder.Cond { } } -// userCollaborationRepoCond return user as collabrators repositories list -func userCollaborationRepoCond(id string, userID int64) builder.Cond { - return builder.In(id, - builder.Select("repo_id").From("collaboration"). - Where(builder.Eq{ - "user_id": userID, - }), - ) -} - // userAssignedRepoCond return user as assignee repositories list func userAssignedRepoCond(id string, userID int64) builder.Cond { return builder.And( @@ -205,11 +195,7 @@ func userAssignedRepoCond(id string, userID int64) builder.Cond { } // userCreateIssueRepoCond return user created issues repositories list -func userCreateIssueRepoCond(id string, userID int64, unitType unit.Type) builder.Cond { - var isPull = false - if unitType == unit.TypePullRequests { - isPull = true - } +func userCreateIssueRepoCond(id string, userID int64, isPull bool) builder.Cond { return builder.And( builder.Eq{ "repository.is_private": false, @@ -241,33 +227,6 @@ func userMentionedRepoCond(id string, userID int64) builder.Cond { ) } -func orgUnitsRepoCond(id string, userID, orgID int64, units ...unit.Type) builder.Cond { - return builder.In(id, - builder.Select("repo_id").From("team_repo").Where( - builder.Eq{ - "org_id": orgID, - }.And( - builder.In( - "team_id", builder.Select("team_id").From("team_user").Where( - builder.Eq{ - "uid": userID, - }, - ), - ), - ).And( - builder.In( - "team_id", builder.Select("team_id").From("team_unit").Where( - builder.Eq{ - "org_id": orgID, - }.And( - builder.In("type", units), - ), - ), - ), - ), - )) -} - func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Type) builder.Cond { return builder.In(id, builder.Select("repo_id").From("team_repo").Where( @@ -294,6 +253,61 @@ func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Typ )) } +// userCollaborationRepoCond return user as collabrators repositories list +func userCollaborationRepoCond(idStr string, userID int64) builder.Cond { + return builder.In(idStr, builder.Select("repo_id"). + From("`access`"). + Where(builder.And( + builder.Eq{"user_id": userID}, + builder.Gt{"mode": int(perm.AccessModeNone)}, + )), + ) +} + +func userOrgTeamRepoCond(idStr string, userID int64) builder.Cond { + return builder.In(idStr, userOrgTeamRepoBuilder(userID)) +} + +func userOrgTeamRepoBuilder(userID int64) *builder.Builder { + return builder.Select("`team_repo`.repo_id"). + From("team_repo"). + Join("INNER", "team_user", "`team_user`.team_id = `team_repo`.team_id"). + Where(builder.Eq{"`team_user`.uid": userID}) +} + +func userOrgTeamUnitRepoBuilder(userID int64, unitType UnitType) *builder.Builder { + return userOrgTeamRepoBuilder(userID). + Join("INNER", "team_unit", "`team_unit`.team_id = `team_repo`.team_id"). + Where(builder.Eq{"`team_unit`.`type`": unitType}) +} + +func userOrgTeamUnitRepoCond(idStr string, userID int64, unitType UnitType) builder.Cond { + return builder.In(idStr, userOrgTeamUnitRepoBuilder(userID, unitType)) +} + +func userOrgUnitRepoCond(idStr string, userID, orgID int64, unitType UnitType) builder.Cond { + return builder.In(idStr, + userOrgTeamUnitRepoBuilder(userID, unitType). + And(builder.Eq{"org_id": orgID}), + ) +} + +func userOrgPublicRepoCond(userID int64) builder.Cond { + return builder.And( + builder.Eq{"`repository`.is_private": false}, + builder.In("`repository`.owner_id", + builder.Select("`org_user`.org_id"). + From("org_user"). + Where(builder.Eq{"`org_user`.uid": userID}), + ), + ) +} + +func userOrgPublicUnitRepoCond(userID, orgID int64) builder.Cond { + return userOrgPublicRepoCond(userID). + And(builder.Eq{"`repository`.owner_id": orgID}) +} + // SearchRepositoryCondition creates a query condition according search repository options func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { cond := builder.NewCond() @@ -347,27 +361,12 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { // 2. But we can see because of: builder.Or( // A. We have access - builder.In("`repository`.id", - builder.Select("`access`.repo_id"). - From("access"). - Where(builder.Eq{"`access`.user_id": opts.OwnerID})), + userCollaborationRepoCond("`repository`.id", opts.OwnerID), // B. We are in a team for - builder.In("`repository`.id", builder.Select("`team_repo`.repo_id"). - From("team_repo"). - Where(builder.Eq{"`team_user`.uid": opts.OwnerID}). - Join("INNER", "team_user", "`team_user`.team_id = `team_repo`.team_id")), - // C. Public repositories in private organizations that we are member of - builder.And( - builder.Eq{"`repository`.is_private": false}, - builder.In("`repository`.owner_id", - builder.Select("`org_user`.org_id"). - From("org_user"). - Join("INNER", "`user`", "`user`.id = `org_user`.org_id"). - Where(builder.Eq{ - "`org_user`.uid": opts.OwnerID, - "`user`.type": user_model.UserTypeOrganization, - "`user`.visibility": structs.VisibleTypePrivate, - })))), + userOrgTeamRepoCond("`repository`.id", opts.OwnerID), + // C. Public repositories in organizations that we are member of + userOrgPublicRepoCond(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)) @@ -538,24 +537,49 @@ func accessibleRepositoryCondition(user *user_model.User) builder.Cond { if user != nil { cond = cond.Or( // 2. Be able to see all repositories that we have access to - builder.In("`repository`.id", builder.Select("repo_id"). - From("`access`"). - Where(builder.And( - builder.Eq{"user_id": user.ID}, - builder.Gt{"mode": int(perm.AccessModeNone)}))), + userCollaborationRepoCond("`repository`.id", user.ID), + // 3. 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), + ) + } + + return cond +} + +func accessibleRepoUnitCond(user *user_mdoel.User, unitType unit.Type) builder.Cond { + cond := builder.NewCond() + + if user == nil || !user.IsRestricted || user.ID <= 0 { + orgVisibilityLimit := []structs.VisibleType{structs.VisibleTypePrivate} + if user == nil || user.ID <= 0 { + orgVisibilityLimit = append(orgVisibilityLimit, structs.VisibleTypeLimited) + } + // 1. Be able to see all non-private repositories that either: + cond = cond.Or(builder.And( + builder.Eq{"`repository`.is_private": false}, + // 2. Aren't in an private organisation or limited organisation if we're not logged in + builder.NotIn("`repository`.owner_id", builder.Select("id").From("`user`").Where( + builder.And( + builder.Eq{"type": user_model.UserTypeOrganization}, + builder.In("visibility", orgVisibilityLimit)), + )))) + } + + if user != nil { + 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 builder.Eq{"`repository`.owner_id": user.ID}, // 4. Be able to see all repositories that we are in a team - builder.In("`repository`.id", builder.Select("`team_repo`.repo_id"). - From("team_repo"). - Where(builder.Eq{"`team_user`.uid": user.ID}). - Join("INNER", "team_user", "`team_user`.team_id = `team_repo`.team_id")), + userOrgTeamRepoCond("`repository`.id", user.ID), // 5. Be able to see all public repos in private organizations that we are an org_user of - builder.And(builder.Eq{"`repository`.is_private": false}, - builder.In("`repository`.owner_id", - builder.Select("`org_user`.org_id"). - From("org_user"). - Where(builder.Eq{"`org_user`.uid": user.ID})))) + userOrgPublicRepoCond(user.ID), + ) } return cond From fa54bb3c199f1f2f2b6c38ddbb2eb2a9a04139ef Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 15 Oct 2021 10:28:19 +0800 Subject: [PATCH 08/17] Fix build --- models/repo_list.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/models/repo_list.go b/models/repo_list.go index 62c7820adb764..2bd8a445317ec 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -637,7 +637,6 @@ func FindUserAccessibleRepoIDs(user *user_model.User) ([]int64, error) { return repoIDs, nil } -<<<<<<< HEAD // GetUserRepositories returns a list of repositories of given user. func GetUserRepositories(opts *SearchRepoOptions) ([]*repo_model.Repository, int64, error) { if len(opts.OrderBy) == 0 { @@ -664,9 +663,9 @@ func GetUserRepositories(opts *SearchRepoOptions) ([]*repo_model.Repository, int sess = sess.Where(cond).OrderBy(opts.OrderBy.String()) repos := make([]*repo_model.Repository, 0, opts.PageSize) return repos, count, db.SetSessionPagination(sess, opts).Find(&repos) -======= +} + // FindReposMapByIDs find repos as map func FindReposMapByIDs(repoIDs []int64, res map[int64]*Repository) error { - return x.In("id", repoIDs).Find(&res) ->>>>>>> f6ddec83b (Improve the performance of pulls/issue) + return db.GetEngine(db.DefaultContext).In("id", repoIDs).Find(&res) } From 74342a17dfec02d07d8068c96adaad70f96f2acd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 15 Oct 2021 13:45:44 +0800 Subject: [PATCH 09/17] Fix lint --- models/repo_list.go | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/models/repo_list.go b/models/repo_list.go index 2bd8a445317ec..672f1a9286788 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -281,10 +281,6 @@ func userOrgTeamUnitRepoBuilder(userID int64, unitType UnitType) *builder.Builde Where(builder.Eq{"`team_unit`.`type`": unitType}) } -func userOrgTeamUnitRepoCond(idStr string, userID int64, unitType UnitType) builder.Cond { - return builder.In(idStr, userOrgTeamUnitRepoBuilder(userID, unitType)) -} - func userOrgUnitRepoCond(idStr string, userID, orgID int64, unitType UnitType) builder.Cond { return builder.In(idStr, userOrgTeamUnitRepoBuilder(userID, unitType). @@ -550,41 +546,6 @@ func accessibleRepositoryCondition(user *user_model.User) builder.Cond { return cond } -func accessibleRepoUnitCond(user *user_mdoel.User, unitType unit.Type) builder.Cond { - cond := builder.NewCond() - - if user == nil || !user.IsRestricted || user.ID <= 0 { - orgVisibilityLimit := []structs.VisibleType{structs.VisibleTypePrivate} - if user == nil || user.ID <= 0 { - orgVisibilityLimit = append(orgVisibilityLimit, structs.VisibleTypeLimited) - } - // 1. Be able to see all non-private repositories that either: - cond = cond.Or(builder.And( - builder.Eq{"`repository`.is_private": false}, - // 2. Aren't in an private organisation or limited organisation if we're not logged in - builder.NotIn("`repository`.owner_id", builder.Select("id").From("`user`").Where( - builder.And( - builder.Eq{"type": user_model.UserTypeOrganization}, - builder.In("visibility", orgVisibilityLimit)), - )))) - } - - if user != nil { - 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 - 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), - ) - } - - return cond -} - // 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) (RepositoryList, int64, error) { From 5486f0e4f958137d1b6ff2749f256139bc40eaee Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 16 Nov 2021 17:30:58 +0800 Subject: [PATCH 10/17] Fix test --- models/issue.go | 4 ++-- models/repo_list.go | 7 +++++-- routers/web/user/home.go | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/models/issue.go b/models/issue.go index 4e9ceefa54dec..1c525bb0ab42a 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1343,9 +1343,9 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { // issuePullAccessibleRepoCond userID must not be zero, this condition require join repository table func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *User, team *Team, isPull bool) builder.Cond { var cond = builder.NewCond() - var unitType = UnitTypeIssues + var unitType = unit.TypeIssues if isPull { - unitType = UnitTypePullRequests + unitType = unit.TypePullRequests } if org != nil { if team != nil { diff --git a/models/repo_list.go b/models/repo_list.go index 672f1a9286788..443d4554f51fe 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -12,7 +12,10 @@ import ( "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" +<<<<<<< HEAD user_model "code.gitea.io/gitea/models/user" +======= +>>>>>>> f548ff512 (Fix test) "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" @@ -275,13 +278,13 @@ func userOrgTeamRepoBuilder(userID int64) *builder.Builder { Where(builder.Eq{"`team_user`.uid": userID}) } -func userOrgTeamUnitRepoBuilder(userID int64, unitType UnitType) *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}) } -func userOrgUnitRepoCond(idStr string, userID, orgID int64, unitType UnitType) builder.Cond { +func userOrgUnitRepoCond(idStr string, userID, orgID int64, unitType unit.Type) builder.Cond { return builder.In(idStr, userOrgTeamUnitRepoBuilder(userID, unitType). And(builder.Eq{"org_id": orgID}), diff --git a/routers/web/user/home.go b/routers/web/user/home.go index b6e338ab641d8..b8635c278901c 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -411,7 +411,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { team = ctx.Org.Team } - isPullList := unitType == models.UnitTypePullRequests + isPullList := unitType == unit.TypePullRequests opts := &models.IssuesOptions{ IsPull: util.OptionalBoolOf(isPullList), SortType: sortType, From f9501751ab3ac5b1651f45d4588d116d1512a1f4 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 20 Nov 2021 19:47:27 +0800 Subject: [PATCH 11/17] Fix build --- models/issue.go | 10 +++++----- routers/web/user/home.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/models/issue.go b/models/issue.go index 1c525bb0ab42a..3a54b4c33b313 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1186,9 +1186,9 @@ type IssuesOptions struct { // prioritize issues from this repo PriorityRepoID int64 IsArchived util.OptionalBool - Org *User // issues permission scope - Team *Team // issues permission scope - User *User // issues permission scope + Org *Organization // issues permission scope + Team *Team // issues permission scope + User *User // issues permission scope } // sortIssuesSession sort an issues-related session based on the provided @@ -1341,7 +1341,7 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } // issuePullAccessibleRepoCond userID must not be zero, this condition require join repository table -func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *User, team *Team, isPull bool) builder.Cond { +func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *Organization, team *Team, isPull bool) builder.Cond { var cond = builder.NewCond() var unitType = unit.TypeIssues if isPull { @@ -1687,7 +1687,7 @@ type UserIssueStatsOptions struct { IssueIDs []int64 IsArchived util.OptionalBool LabelIDs []int64 - Org *User + Org *Organization Team *Team } diff --git a/routers/web/user/home.go b/routers/web/user/home.go index b8635c278901c..bf089375504e5 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -405,7 +405,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { // Get repository IDs where User/Org/Team has access. var team *models.Team - var org *models.User + var org *models.Organization if ctx.Org != nil { org = ctx.Org.Organization team = ctx.Org.Team From 7129e28bc873e530e6600f97511315d5e9126c74 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 21 Nov 2021 16:26:17 +0800 Subject: [PATCH 12/17] Adjust the logic --- models/repo_list.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/models/repo_list.go b/models/repo_list.go index 443d4554f51fe..d263fc96340f3 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -302,6 +302,22 @@ func userOrgPublicRepoCond(userID int64) builder.Cond { ) } +func userOrgPublicRepoCondPrivate(userID int64) builder.Cond { + return builder.And( + builder.Eq{"`repository`.is_private": false}, + builder.In("`repository`.owner_id", + builder.Select("`org_user`.org_id"). + From("org_user"). + Join("INNER", "`user`", "`user`.id = `org_user`.org_id"). + Where(builder.Eq{ + "`org_user`.uid": userID, + "`user`.type": UserTypeOrganization, + "`user`.visibility": structs.VisibleTypePrivate, + }), + ), + ) +} + func userOrgPublicUnitRepoCond(userID, orgID int64) builder.Cond { return userOrgPublicRepoCond(userID). And(builder.Eq{"`repository`.owner_id": orgID}) @@ -364,7 +380,7 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { // B. We are in a team for userOrgTeamRepoCond("`repository`.id", opts.OwnerID), // C. Public repositories in organizations that we are member of - userOrgPublicRepoCond(opts.OwnerID), + userOrgPublicRepoCondPrivate(opts.OwnerID), ), ) if !opts.Private { From cb340f49a4e1a886239128ac392e39547e181729 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 25 Nov 2021 09:06:04 +0800 Subject: [PATCH 13/17] Merge --- models/issue.go | 6 +-- models/repo_list.go | 5 +-- models/user.go | 104 -------------------------------------------- 3 files changed, 4 insertions(+), 111 deletions(-) diff --git a/models/issue.go b/models/issue.go index 3a54b4c33b313..d815bc40101ea 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1186,9 +1186,9 @@ type IssuesOptions struct { // prioritize issues from this repo PriorityRepoID int64 IsArchived util.OptionalBool - Org *Organization // issues permission scope - Team *Team // issues permission scope - User *User // issues permission scope + Org *Organization // issues permission scope + Team *Team // issues permission scope + User *user_model.User // issues permission scope } // sortIssuesSession sort an issues-related session based on the provided diff --git a/models/repo_list.go b/models/repo_list.go index d263fc96340f3..fbbecca811769 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -12,10 +12,7 @@ import ( "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" -<<<<<<< HEAD user_model "code.gitea.io/gitea/models/user" -======= ->>>>>>> f548ff512 (Fix test) "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" @@ -311,7 +308,7 @@ func userOrgPublicRepoCondPrivate(userID int64) builder.Cond { Join("INNER", "`user`", "`user`.id = `org_user`.org_id"). Where(builder.Eq{ "`org_user`.uid": userID, - "`user`.type": UserTypeOrganization, + "`user`.type": user_model.UserTypeOrganization, "`user`.visibility": structs.VisibleTypePrivate, }), ), diff --git a/models/user.go b/models/user.go index 2e7a84273fae9..5f7bedd36da20 100644 --- a/models/user.go +++ b/models/user.go @@ -15,7 +15,6 @@ import ( asymkey_model "code.gitea.io/gitea/models/asymkey" "code.gitea.io/gitea/models/db" 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/setting" "code.gitea.io/gitea/modules/structs" @@ -30,109 +29,6 @@ func GetOrganizationCount(ctx context.Context, u *user_model.User) (int64, error Count(new(OrgUser)) } -// GetRepositoryIDs returns repositories IDs where user owned and has unittypes -// Caller shall check that units is not globally disabled -func GetRepositoryIDs(u *user_model.User, units ...unit.Type) ([]int64, error) { - var ids []int64 - - sess := db.GetEngine(db.DefaultContext).Table("repository").Cols("repository.id") - - if len(units) > 0 { - sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id") - sess = sess.In("repo_unit.type", units) - } - - return ids, sess.Where("owner_id = ?", u.ID).Find(&ids) -} - -// GetActiveRepositoryIDs returns non-archived repositories IDs where user owned and has unittypes -// Caller shall check that units is not globally disabled -func GetActiveRepositoryIDs(u *user_model.User, units ...unit.Type) ([]int64, error) { - var ids []int64 - - sess := db.GetEngine(db.DefaultContext).Table("repository").Cols("repository.id") - - if len(units) > 0 { - sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id") - sess = sess.In("repo_unit.type", units) - } - - sess.Where(builder.Eq{"is_archived": false}) - - return ids, sess.Where("owner_id = ?", u.ID).GroupBy("repository.id").Find(&ids) -} - -// GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes -// Caller shall check that units is not globally disabled -func GetOrgRepositoryIDs(u *user_model.User, units ...unit.Type) ([]int64, error) { - var ids []int64 - - if err := db.GetEngine(db.DefaultContext).Table("repository"). - Cols("repository.id"). - Join("INNER", "team_user", "repository.owner_id = team_user.org_id"). - Join("INNER", "team_repo", "(? != ? and repository.is_private != ?) OR (team_user.team_id = team_repo.team_id AND repository.id = team_repo.repo_id)", true, u.IsRestricted, true). - Where("team_user.uid = ?", u.ID). - GroupBy("repository.id").Find(&ids); err != nil { - return nil, err - } - - if len(units) > 0 { - return FilterOutRepoIdsWithoutUnitAccess(u, ids, units...) - } - - return ids, nil -} - -// GetActiveOrgRepositoryIDs returns non-archived repositories IDs where user's team owned and has unittypes -// Caller shall check that units is not globally disabled -func GetActiveOrgRepositoryIDs(u *user_model.User, units ...unit.Type) ([]int64, error) { - var ids []int64 - - if err := db.GetEngine(db.DefaultContext).Table("repository"). - Cols("repository.id"). - Join("INNER", "team_user", "repository.owner_id = team_user.org_id"). - Join("INNER", "team_repo", "(? != ? and repository.is_private != ?) OR (team_user.team_id = team_repo.team_id AND repository.id = team_repo.repo_id)", true, u.IsRestricted, true). - Where("team_user.uid = ?", u.ID). - Where(builder.Eq{"is_archived": false}). - GroupBy("repository.id").Find(&ids); err != nil { - return nil, err - } - - if len(units) > 0 { - return FilterOutRepoIdsWithoutUnitAccess(u, ids, units...) - } - - return ids, nil -} - -// GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations -// Caller shall check that units is not globally disabled -func GetAccessRepoIDs(u *user_model.User, units ...unit.Type) ([]int64, error) { - ids, err := GetRepositoryIDs(u, units...) - if err != nil { - return nil, err - } - ids2, err := GetOrgRepositoryIDs(u, units...) - if err != nil { - return nil, err - } - return append(ids, ids2...), nil -} - -// GetActiveAccessRepoIDs returns all non-archived repositories IDs where user's or user is a team member organizations -// Caller shall check that units is not globally disabled -func GetActiveAccessRepoIDs(u *user_model.User, units ...unit.Type) ([]int64, error) { - ids, err := GetActiveRepositoryIDs(u, units...) - if err != nil { - return nil, err - } - ids2, err := GetActiveOrgRepositoryIDs(u, units...) - if err != nil { - return nil, err - } - return append(ids, ids2...), nil -} - // deleteBeans deletes all given beans, beans should contain delete conditions. func deleteBeans(e db.Engine, beans ...interface{}) (err error) { for i := range beans { From 3b92f9b1972c3010af3e825881174ff329c8732c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 13 Dec 2021 09:36:25 +0800 Subject: [PATCH 14/17] Fix conflicts --- models/repo/repo.go | 31 --------------------------- models/repo/repo_list.go | 46 ++++++++++++++++++++++++++++++++++++++++ models/repo_list.go | 5 ----- models/user_test.go | 22 ------------------- routers/web/user/home.go | 4 ++-- 5 files changed, 48 insertions(+), 60 deletions(-) create mode 100644 models/repo/repo_list.go diff --git a/models/repo/repo.go b/models/repo/repo.go index d0136e9c66c4a..5108231cd8d03 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -731,15 +731,6 @@ func CountUserRepositories(userID int64, private bool) int64 { return countRepositories(userID, private) } -// GetUserMirrorRepositories returns a list of mirror repositories of given user. -func GetUserMirrorRepositories(userID int64) ([]*Repository, error) { - repos := make([]*Repository, 0, 10) - return repos, db.GetEngine(db.DefaultContext). - Where("owner_id = ?", userID). - And("is_mirror = ?", true). - Find(&repos) -} - func getRepositoryCount(e db.Engine, ownerID int64) (int64, error) { return e.Count(&Repository{OwnerID: ownerID}) } @@ -766,25 +757,3 @@ func GetPublicRepositoryCount(u *user_model.User) (int64, error) { func GetPrivateRepositoryCount(u *user_model.User) (int64, error) { return getPrivateRepositoryCount(db.GetEngine(db.DefaultContext), u) } - -// IterateRepository iterate repositories -func IterateRepository(f func(repo *Repository) error) error { - var start int - batchSize := setting.Database.IterateBufferSize - for { - repos := make([]*Repository, 0, batchSize) - if err := db.GetEngine(db.DefaultContext).Limit(batchSize, start).Find(&repos); err != nil { - return err - } - if len(repos) == 0 { - return nil - } - start += len(repos) - - for _, repo := range repos { - if err := f(repo); err != nil { - return err - } - } - } -} diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go new file mode 100644 index 0000000000000..571604a2c2a74 --- /dev/null +++ b/models/repo/repo_list.go @@ -0,0 +1,46 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repo + +import ( + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/setting" +) + +// GetUserMirrorRepositories returns a list of mirror repositories of given user. +func GetUserMirrorRepositories(userID int64) ([]*Repository, error) { + repos := make([]*Repository, 0, 10) + return repos, db.GetEngine(db.DefaultContext). + Where("owner_id = ?", userID). + And("is_mirror = ?", true). + Find(&repos) +} + +// IterateRepository iterate repositories +func IterateRepository(f func(repo *Repository) error) error { + var start int + batchSize := setting.Database.IterateBufferSize + for { + repos := make([]*Repository, 0, batchSize) + if err := db.GetEngine(db.DefaultContext).Limit(batchSize, start).Find(&repos); err != nil { + return err + } + if len(repos) == 0 { + return nil + } + start += len(repos) + + for _, repo := range repos { + if err := f(repo); err != nil { + return err + } + } + } +} + +// FindReposMapByIDs find repos as map +func FindReposMapByIDs(repoIDs []int64, res map[int64]*Repository) error { + return db.GetEngine(db.DefaultContext).In("id", repoIDs).Find(&res) +} diff --git a/models/repo_list.go b/models/repo_list.go index fbbecca811769..9f5da22d957cc 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -641,8 +641,3 @@ func GetUserRepositories(opts *SearchRepoOptions) ([]*repo_model.Repository, int repos := make([]*repo_model.Repository, 0, opts.PageSize) return repos, count, db.SetSessionPagination(sess, opts).Find(&repos) } - -// FindReposMapByIDs find repos as map -func FindReposMapByIDs(repoIDs []int64, res map[int64]*Repository) error { - return db.GetEngine(db.DefaultContext).In("id", repoIDs).Find(&res) -} diff --git a/models/user_test.go b/models/user_test.go index 4749a3af73983..83201ff4cba4e 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -98,25 +98,3 @@ func testIsUserOrgOwner(t *testing.T, uid, orgID int64, expected bool) { assert.NoError(t, err) assert.Equal(t, expected, is) } - -func TestGetOrgRepositoryIDs(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}).(*user_model.User) - user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}).(*user_model.User) - user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}).(*user_model.User) - - accessibleRepos, err := GetOrgRepositoryIDs(user2) - assert.NoError(t, err) - // User 2's team has access to private repos 3, 5, repo 32 is a public repo of the organization - assert.Equal(t, []int64{3, 5, 23, 24, 32}, accessibleRepos) - - accessibleRepos, err = GetOrgRepositoryIDs(user4) - assert.NoError(t, err) - // User 4's team has access to private repo 3, repo 32 is a public repo of the organization - assert.Equal(t, []int64{3, 32}, accessibleRepos) - - accessibleRepos, err = GetOrgRepositoryIDs(user5) - assert.NoError(t, err) - // User 5's team has no access to any repo - assert.Len(t, accessibleRepos, 0) -} diff --git a/routers/web/user/home.go b/routers/web/user/home.go index bf089375504e5..12f8bfecd6586 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -701,14 +701,14 @@ func loadRepoByIDs(ctxUser *user_model.User, issueCountByRepo map[int64]int64, u } repoIDs = append(repoIDs, id) if len(repoIDs) == 500 { - if err := models.FindReposMapByIDs(repoIDs, totalRes); err != nil { + if err := repo_model.FindReposMapByIDs(repoIDs, totalRes); err != nil { return nil, err } repoIDs = make([]int64, 0, 500) } } if len(repoIDs) > 0 { - if err := models.FindReposMapByIDs(repoIDs, totalRes); err != nil { + if err := repo_model.FindReposMapByIDs(repoIDs, totalRes); err != nil { return nil, err } } From 9bb54b29dcbf883dc48833dda3ad60588b96fea1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 21 Dec 2021 11:17:55 +0800 Subject: [PATCH 15/17] improve the performance --- routers/web/user/home.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 12f8bfecd6586..8bbc6dd0c3691 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -704,7 +704,7 @@ func loadRepoByIDs(ctxUser *user_model.User, issueCountByRepo map[int64]int64, u if err := repo_model.FindReposMapByIDs(repoIDs, totalRes); err != nil { return nil, err } - repoIDs = make([]int64, 0, 500) + repoIDs = repoIDs[:0] } } if len(repoIDs) > 0 { From 7bd82208d76d2a43af9a5849aefb0b2171a0c03f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 25 Dec 2021 16:20:54 +0800 Subject: [PATCH 16/17] Add comments for the query conditions functions --- models/repo_list.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/models/repo_list.go b/models/repo_list.go index 9f5da22d957cc..0296efe587868 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -227,6 +227,7 @@ func userMentionedRepoCond(id string, userID int64) builder.Cond { ) } +// teamUnitsRepoCond returns query condition for those repo id in the special org team with special units access func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Type) builder.Cond { return builder.In(id, builder.Select("repo_id").From("team_repo").Where( @@ -253,7 +254,7 @@ func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Typ )) } -// userCollaborationRepoCond return user as collabrators repositories list +// userCollaborationRepoCond returns user as collabrators repositories list func userCollaborationRepoCond(idStr string, userID int64) builder.Cond { return builder.In(idStr, builder.Select("repo_id"). From("`access`"). @@ -264,10 +265,12 @@ 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 { return builder.In(idStr, userOrgTeamRepoBuilder(userID)) } +// userOrgTeamRepoBuilder returns repo ids where user's teams can access. func userOrgTeamRepoBuilder(userID int64) *builder.Builder { return builder.Select("`team_repo`.repo_id"). From("team_repo"). @@ -275,12 +278,14 @@ func userOrgTeamRepoBuilder(userID int64) *builder.Builder { Where(builder.Eq{"`team_user`.uid": userID}) } +// userOrgTeamUnitRepoBuilder returns repo ids where user's teams can access the special unit. 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}) } +// userOrgUnitRepoCond selects repos that the given user has access to through org and the special unit func userOrgUnitRepoCond(idStr string, userID, orgID int64, unitType unit.Type) builder.Cond { return builder.In(idStr, userOrgTeamUnitRepoBuilder(userID, unitType). @@ -288,6 +293,7 @@ func userOrgUnitRepoCond(idStr string, userID, orgID int64, unitType unit.Type) ) } +// userOrgPublicRepoCond returns the condition that one user could access all public repositories in organizations func userOrgPublicRepoCond(userID int64) builder.Cond { return builder.And( builder.Eq{"`repository`.is_private": false}, @@ -299,6 +305,7 @@ func userOrgPublicRepoCond(userID int64) builder.Cond { ) } +// userOrgPublicRepoCondPrivate returns the condition that one user could access all public repositories in private organizations func userOrgPublicRepoCondPrivate(userID int64) builder.Cond { return builder.And( builder.Eq{"`repository`.is_private": false}, @@ -315,6 +322,7 @@ func userOrgPublicRepoCondPrivate(userID int64) builder.Cond { ) } +// userOrgPublicUnitRepoCond returns the condition that one user could access all public repositories in the special organization func userOrgPublicUnitRepoCond(userID, orgID int64) builder.Cond { return userOrgPublicRepoCond(userID). And(builder.Eq{"`repository`.owner_id": orgID}) From c6d37f1fb86afb5bc31175dc86a331c2d3b1c968 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 28 Dec 2021 23:22:18 +0800 Subject: [PATCH 17/17] Some improvements --- models/issue.go | 8 +++++--- models/repo_list.go | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/models/issue.go b/models/issue.go index d815bc40101ea..4fc3da25d67b8 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1711,13 +1711,15 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) { sess := func(cond builder.Cond) *xorm.Session { s := db.GetEngine(db.DefaultContext).Where(cond) - s.Join("INNER", "repository", "issue.repo_id = repository.id") if len(opts.LabelIDs) > 0 { s.Join("INNER", "issue_label", "issue_label.issue_id = issue.id"). In("issue_label.label_id", opts.LabelIDs) } - if opts.IsArchived != util.OptionalBoolNone { - s.And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()}) + if opts.UserID > 0 || opts.IsArchived != util.OptionalBoolNone { + s.Join("INNER", "repository", "issue.repo_id = repository.id") + if opts.IsArchived != util.OptionalBoolNone { + s.And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()}) + } } return s } diff --git a/models/repo_list.go b/models/repo_list.go index 0296efe587868..9bda0d5a374ab 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -315,7 +315,7 @@ func userOrgPublicRepoCondPrivate(userID int64) builder.Cond { Join("INNER", "`user`", "`user`.id = `org_user`.org_id"). Where(builder.Eq{ "`org_user`.uid": userID, - "`user`.type": user_model.UserTypeOrganization, + "`user`.`type`": user_model.UserTypeOrganization, "`user`.visibility": structs.VisibleTypePrivate, }), ),