aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwxiaoguang <wxiaoguang@gmail.com>2023-09-16 20:54:23 +0800
committerGitHub <noreply@github.com>2023-09-16 12:54:23 +0000
commit4ffc30cb842d6631fad1e6b45c77ac1101b405a3 (patch)
treea7518f5ad02130c43bd7cb19b259a64f9a7d20d2
parent7046065c0ed321af156e84d2a10a25c8df047ddd (diff)
downloadgitea-4ffc30cb842d6631fad1e6b45c77ac1101b405a3.tar.gz
gitea-4ffc30cb842d6631fad1e6b45c77ac1101b405a3.zip
Use db.WithTx for AddTeamMember to avoid ctx abuse (#27095)
Compare with ignoring spaces: https://github.com/go-gitea/gitea/pull/27095/files?diff=split&w=1
-rw-r--r--models/org_team.go96
1 files changed, 48 insertions, 48 deletions
diff --git a/models/org_team.go b/models/org_team.go
index 7ddf986ce9..ea73918435 100644
--- a/models/org_team.go
+++ b/models/org_team.go
@@ -366,61 +366,61 @@ func AddTeamMember(ctx context.Context, team *organization.Team, userID int64) e
return err
}
- ctx, committer, err := db.TxContext(ctx)
- if err != nil {
- return err
- }
- defer committer.Close()
-
- // check in transaction
- isAlreadyMember, err = organization.IsTeamMember(ctx, team.OrgID, team.ID, userID)
- if err != nil || isAlreadyMember {
- return err
- }
+ err = db.WithTx(ctx, func(ctx context.Context) error {
+ // check in transaction
+ isAlreadyMember, err = organization.IsTeamMember(ctx, team.OrgID, team.ID, userID)
+ if err != nil || isAlreadyMember {
+ return err
+ }
- sess := db.GetEngine(ctx)
+ sess := db.GetEngine(ctx)
- if err := db.Insert(ctx, &organization.TeamUser{
- UID: userID,
- OrgID: team.OrgID,
- TeamID: team.ID,
- }); err != nil {
- return err
- } else if _, err := sess.Incr("num_members").ID(team.ID).Update(new(organization.Team)); err != nil {
- return err
- }
+ if err := db.Insert(ctx, &organization.TeamUser{
+ UID: userID,
+ OrgID: team.OrgID,
+ TeamID: team.ID,
+ }); err != nil {
+ return err
+ } else if _, err := sess.Incr("num_members").ID(team.ID).Update(new(organization.Team)); err != nil {
+ return err
+ }
- team.NumMembers++
+ team.NumMembers++
- // Give access to team repositories.
- // update exist access if mode become bigger
- subQuery := builder.Select("repo_id").From("team_repo").
- Where(builder.Eq{"team_id": team.ID})
+ // Give access to team repositories.
+ // 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_model.Access)); err != nil {
- return fmt.Errorf("update user accesses: %w", err)
- }
+ if _, err := sess.Where("user_id=?", userID).
+ In("repo_id", subQuery).
+ And("mode < ?", team.AccessMode).
+ SetExpr("mode", team.AccessMode).
+ Update(new(access_model.Access)); err != nil {
+ return fmt.Errorf("update user accesses: %w", 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: %w", 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: %w", err)
+ }
- accesses := make([]*access_model.Access, 0, 100)
- for i, repoID := range repoIDs {
- accesses = append(accesses, &access_model.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: %w", err)
+ accesses := make([]*access_model.Access, 0, 100)
+ for i, repoID := range repoIDs {
+ accesses = append(accesses, &access_model.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: %w", err)
+ }
+ accesses = accesses[:0]
}
- accesses = accesses[:0]
}
+ return nil
+ })
+ if err != nil {
+ return err
}
// this behaviour may spend much time so run it in a goroutine
@@ -428,7 +428,7 @@ func AddTeamMember(ctx context.Context, team *organization.Team, userID int64) e
if setting.Service.AutoWatchNewRepos {
// Get team and its repositories.
if err := team.LoadRepositories(ctx); err != nil {
- log.Error("getRepositories failed: %v", err)
+ log.Error("team.LoadRepositories failed: %v", err)
}
// FIXME: in the goroutine, it can't access the "ctx", it could only use db.DefaultContext at the moment
go func(repos []*repo_model.Repository) {
@@ -440,7 +440,7 @@ func AddTeamMember(ctx context.Context, team *organization.Team, userID int64) e
}(team.Repos)
}
- return committer.Commit()
+ return nil
}
func removeTeamMember(ctx context.Context, team *organization.Team, userID int64) error {