From 01cacced43175461d7f814a7f9379bfb2b85acbc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 27 Mar 2022 11:19:27 +0800 Subject: [PATCH 1/3] performance improvement for add team user when org has more than 1000 repositories --- models/org_team.go | 55 +++++++++++++++++++++++++++++++---------- models/repo_transfer.go | 2 +- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/models/org_team.go b/models/org_team.go index cf810cad337e6..1a565d6e73dca 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/models/organization" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -488,18 +489,13 @@ func AddTeamMember(team *organization.Team, userID int64) error { return err } - if err := organization.AddOrgUser(team.OrgID, userID); err != nil { - return err - } - ctx, committer, err := db.TxContext() if err != nil { return err } defer committer.Close() - // Get team and its repositories. - if err := team.GetRepositoriesCtx(ctx); err != nil { + if err := AddOrgUser(ctx, team.OrgID, userID); err != nil { return err } @@ -518,17 +514,50 @@ func AddTeamMember(team *organization.Team, userID int64) error { team.NumMembers++ // Give access to team repositories. - for _, repo := range team.Repos { - if err := recalculateUserAccess(ctx, repo, userID); err != nil { - return err - } - if setting.Service.AutoWatchNewRepos { - if err = repo_model.WatchRepoCtx(ctx, userID, repo.ID, true); err != nil { - return err + // update exist access if mode become bigger + subQuery := builder.Select("repo_id").From("team_repo"). + Where(builder.Eq{"team_id": team.ID}) + + if _, err := sess.Where("user_id=?", userID). + In("repo_id", subQuery). + And("mode < ?", team.AccessMode). + SetExpr("mode", team.AccessMode). + Update(new(Access)); err != nil { + return fmt.Errorf("update user accesses: %v", err) + } + + // for not exist access + var repoIDs []int64 + accessSubQuery := builder.Select("repo_id").From("access").Where(builder.Eq{"user_id": userID}) + if err := sess.SQL(subQuery.And(builder.NotIn("repo_id", accessSubQuery))).Find(&repoIDs); err != nil { + return fmt.Errorf("select id accesses: %v", err) + } + + accesses := make([]*Access, 0, 100) + for i, repoID := range repoIDs { + accesses = append(accesses, &Access{RepoID: repoID, UserID: userID}) + if (i%100 == 0 || i == len(repoIDs)-1) && len(accesses) > 0 { + if err = db.Insert(ctx, accesses); err != nil { + return fmt.Errorf("insert new user accesses: %v", err) } + accesses = accesses[:0] } } + // watch could be failed, so run it in a goroutine + if setting.Service.AutoWatchNewRepos { + if err := team.getRepositories(db.GetEngine(db.DefaultContext)); err != nil { + log.Error("getRepositories failed: %v", err) + } + go func(repos []*repo_model.Repository) { + for _, repo := range repos { + if err = repo_model.WatchRepoCtx(db.DefaultContext, userID, repo.ID, true); err != nil { + log.Error("watch repo failed: %v", err) + } + } + }(team.Repos) + } + return committer.Commit() } diff --git a/models/repo_transfer.go b/models/repo_transfer.go index f9a758a20ba7b..e1316d4646723 100644 --- a/models/repo_transfer.go +++ b/models/repo_transfer.go @@ -297,7 +297,7 @@ func TransferOwnership(doer *user_model.User, newOwnerName string, repo *repo_mo } if c.ID != newOwner.ID { - isMember, err := organization.IsOrganizationMember(ctx, newOwner.ID, c.ID) + isMember, err := organization.IsOrganizationMember(ctx, newOwner.ID, c.ID)000 repositories) if err != nil { return fmt.Errorf("IsOrgMember: %v", err) } else if !isMember { From cf67cb1323ffbc9413c663dad9724a54301e7b8f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 1 Apr 2022 09:22:01 +0800 Subject: [PATCH 2/3] Fix bug --- models/org_team.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/org_team.go b/models/org_team.go index 1a565d6e73dca..610c0fc0ecbcc 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -535,7 +535,7 @@ func AddTeamMember(team *organization.Team, userID int64) error { accesses := make([]*Access, 0, 100) for i, repoID := range repoIDs { - accesses = append(accesses, &Access{RepoID: repoID, UserID: userID}) + accesses = append(accesses, &Access{RepoID: repoID, UserID: userID, Mode: team.AccessMode}) if (i%100 == 0 || i == len(repoIDs)-1) && len(accesses) > 0 { if err = db.Insert(ctx, accesses); err != nil { return fmt.Errorf("insert new user accesses: %v", err) From 2cc688f3c5fd8ef33da2bba580c76d6a24c5670a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 1 Apr 2022 09:28:43 +0800 Subject: [PATCH 3/3] Fix bug --- models/org_team.go | 11 ++++++----- models/repo_transfer.go | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/models/org_team.go b/models/org_team.go index 610c0fc0ecbcc..695f803dbfdbe 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -489,15 +489,15 @@ func AddTeamMember(team *organization.Team, userID int64) error { return err } - ctx, committer, err := db.TxContext() - if err != nil { + if err := organization.AddOrgUser(team.OrgID, userID); err != nil { return err } - defer committer.Close() - if err := AddOrgUser(ctx, team.OrgID, userID); err != nil { + ctx, committer, err := db.TxContext() + if err != nil { return err } + defer committer.Close() sess := db.GetEngine(ctx) @@ -546,7 +546,8 @@ func AddTeamMember(team *organization.Team, userID int64) error { // watch could be failed, so run it in a goroutine if setting.Service.AutoWatchNewRepos { - if err := team.getRepositories(db.GetEngine(db.DefaultContext)); err != nil { + // Get team and its repositories. + if err := team.GetRepositoriesCtx(db.DefaultContext); err != nil { log.Error("getRepositories failed: %v", err) } go func(repos []*repo_model.Repository) { diff --git a/models/repo_transfer.go b/models/repo_transfer.go index e1316d4646723..f9a758a20ba7b 100644 --- a/models/repo_transfer.go +++ b/models/repo_transfer.go @@ -297,7 +297,7 @@ func TransferOwnership(doer *user_model.User, newOwnerName string, repo *repo_mo } if c.ID != newOwner.ID { - isMember, err := organization.IsOrganizationMember(ctx, newOwner.ID, c.ID)000 repositories) + isMember, err := organization.IsOrganizationMember(ctx, newOwner.ID, c.ID) if err != nil { return fmt.Errorf("IsOrgMember: %v", err) } else if !isMember {