Skip to content

Commit 4ffc30c

Browse files
authored
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
1 parent 7046065 commit 4ffc30c

File tree

1 file changed

+48
-48
lines changed

1 file changed

+48
-48
lines changed

models/org_team.go

+48-48
Original file line numberDiff line numberDiff line change
@@ -366,69 +366,69 @@ func AddTeamMember(ctx context.Context, team *organization.Team, userID int64) e
366366
return err
367367
}
368368

369-
ctx, committer, err := db.TxContext(ctx)
370-
if err != nil {
371-
return err
372-
}
373-
defer committer.Close()
374-
375-
// check in transaction
376-
isAlreadyMember, err = organization.IsTeamMember(ctx, team.OrgID, team.ID, userID)
377-
if err != nil || isAlreadyMember {
378-
return err
379-
}
369+
err = db.WithTx(ctx, func(ctx context.Context) error {
370+
// check in transaction
371+
isAlreadyMember, err = organization.IsTeamMember(ctx, team.OrgID, team.ID, userID)
372+
if err != nil || isAlreadyMember {
373+
return err
374+
}
380375

381-
sess := db.GetEngine(ctx)
376+
sess := db.GetEngine(ctx)
382377

383-
if err := db.Insert(ctx, &organization.TeamUser{
384-
UID: userID,
385-
OrgID: team.OrgID,
386-
TeamID: team.ID,
387-
}); err != nil {
388-
return err
389-
} else if _, err := sess.Incr("num_members").ID(team.ID).Update(new(organization.Team)); err != nil {
390-
return err
391-
}
378+
if err := db.Insert(ctx, &organization.TeamUser{
379+
UID: userID,
380+
OrgID: team.OrgID,
381+
TeamID: team.ID,
382+
}); err != nil {
383+
return err
384+
} else if _, err := sess.Incr("num_members").ID(team.ID).Update(new(organization.Team)); err != nil {
385+
return err
386+
}
392387

393-
team.NumMembers++
388+
team.NumMembers++
394389

395-
// Give access to team repositories.
396-
// update exist access if mode become bigger
397-
subQuery := builder.Select("repo_id").From("team_repo").
398-
Where(builder.Eq{"team_id": team.ID})
390+
// Give access to team repositories.
391+
// update exist access if mode become bigger
392+
subQuery := builder.Select("repo_id").From("team_repo").
393+
Where(builder.Eq{"team_id": team.ID})
399394

400-
if _, err := sess.Where("user_id=?", userID).
401-
In("repo_id", subQuery).
402-
And("mode < ?", team.AccessMode).
403-
SetExpr("mode", team.AccessMode).
404-
Update(new(access_model.Access)); err != nil {
405-
return fmt.Errorf("update user accesses: %w", err)
406-
}
395+
if _, err := sess.Where("user_id=?", userID).
396+
In("repo_id", subQuery).
397+
And("mode < ?", team.AccessMode).
398+
SetExpr("mode", team.AccessMode).
399+
Update(new(access_model.Access)); err != nil {
400+
return fmt.Errorf("update user accesses: %w", err)
401+
}
407402

408-
// for not exist access
409-
var repoIDs []int64
410-
accessSubQuery := builder.Select("repo_id").From("access").Where(builder.Eq{"user_id": userID})
411-
if err := sess.SQL(subQuery.And(builder.NotIn("repo_id", accessSubQuery))).Find(&repoIDs); err != nil {
412-
return fmt.Errorf("select id accesses: %w", err)
413-
}
403+
// for not exist access
404+
var repoIDs []int64
405+
accessSubQuery := builder.Select("repo_id").From("access").Where(builder.Eq{"user_id": userID})
406+
if err := sess.SQL(subQuery.And(builder.NotIn("repo_id", accessSubQuery))).Find(&repoIDs); err != nil {
407+
return fmt.Errorf("select id accesses: %w", err)
408+
}
414409

415-
accesses := make([]*access_model.Access, 0, 100)
416-
for i, repoID := range repoIDs {
417-
accesses = append(accesses, &access_model.Access{RepoID: repoID, UserID: userID, Mode: team.AccessMode})
418-
if (i%100 == 0 || i == len(repoIDs)-1) && len(accesses) > 0 {
419-
if err = db.Insert(ctx, accesses); err != nil {
420-
return fmt.Errorf("insert new user accesses: %w", err)
410+
accesses := make([]*access_model.Access, 0, 100)
411+
for i, repoID := range repoIDs {
412+
accesses = append(accesses, &access_model.Access{RepoID: repoID, UserID: userID, Mode: team.AccessMode})
413+
if (i%100 == 0 || i == len(repoIDs)-1) && len(accesses) > 0 {
414+
if err = db.Insert(ctx, accesses); err != nil {
415+
return fmt.Errorf("insert new user accesses: %w", err)
416+
}
417+
accesses = accesses[:0]
421418
}
422-
accesses = accesses[:0]
423419
}
420+
return nil
421+
})
422+
if err != nil {
423+
return err
424424
}
425425

426426
// this behaviour may spend much time so run it in a goroutine
427427
// FIXME: Update watch repos batchly
428428
if setting.Service.AutoWatchNewRepos {
429429
// Get team and its repositories.
430430
if err := team.LoadRepositories(ctx); err != nil {
431-
log.Error("getRepositories failed: %v", err)
431+
log.Error("team.LoadRepositories failed: %v", err)
432432
}
433433
// FIXME: in the goroutine, it can't access the "ctx", it could only use db.DefaultContext at the moment
434434
go func(repos []*repo_model.Repository) {
@@ -440,7 +440,7 @@ func AddTeamMember(ctx context.Context, team *organization.Team, userID int64) e
440440
}(team.Repos)
441441
}
442442

443-
return committer.Commit()
443+
return nil
444444
}
445445

446446
func removeTeamMember(ctx context.Context, team *organization.Team, userID int64) error {

0 commit comments

Comments
 (0)