From bda7281fcb65fd12359f1234bad6200705fd0108 Mon Sep 17 00:00:00 2001 From: Mohit25022005 Date: Thu, 7 May 2026 14:03:44 +0000 Subject: [PATCH] fix: remove retry helper, modernize loops and waitgroup usage Signed-off-by: Mohit25022005 --- .../source/oauth2/source_group_sync_test.go | 13 +++---- services/org/team.go | 36 ++----------------- services/org/team_test.go | 16 ++++----- 3 files changed, 13 insertions(+), 52 deletions(-) diff --git a/services/auth/source/oauth2/source_group_sync_test.go b/services/auth/source/oauth2/source_group_sync_test.go index a637eed373..77430b53e9 100644 --- a/services/auth/source/oauth2/source_group_sync_test.go +++ b/services/auth/source/oauth2/source_group_sync_test.go @@ -32,16 +32,13 @@ func TestSyncGroupsToTeamsConcurrentRuns(t *testing.T) { } var wg sync.WaitGroup - for i := 0; i < 10; i++ { - wg.Add(2) - go func() { - defer wg.Done() + for range 10 { + wg.Go(func() { assert.NoError(t, auth_source.SyncGroupsToTeams(ctx, user, container.SetOf("ldap-group"), sourceGroupTeamMapping, true)) - }() - go func() { - defer wg.Done() + }) + wg.Go(func() { assert.NoError(t, auth_source.SyncGroupsToTeams(ctx, user, container.Set[string]{}, sourceGroupTeamMapping, true)) - }() + }) } wg.Wait() diff --git a/services/org/team.go b/services/org/team.go index c595f17e1f..3ccef4ae8e 100644 --- a/services/org/team.go +++ b/services/org/team.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "strings" - "time" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" @@ -225,7 +224,7 @@ func AddTeamMember(ctx context.Context, team *organization.Team, user *user_mode return err } - err = withTeamMembershipTxRetry(ctx, func(ctx context.Context) error { + err = db.WithTx(ctx, func(ctx context.Context) error { // check in transaction isAlreadyMember, err = organization.IsTeamMember(ctx, team.OrgID, team.ID, user.ID) if err != nil || isAlreadyMember { @@ -359,38 +358,7 @@ func removeInvalidOrgUser(ctx context.Context, orgID int64, user *user_model.Use // RemoveTeamMember removes member from given team of given organization. func RemoveTeamMember(ctx context.Context, team *organization.Team, user *user_model.User) error { - return withTeamMembershipTxRetry(ctx, func(ctx context.Context) error { + return db.WithTx(ctx, func(ctx context.Context) error { return removeTeamMember(ctx, team, user) }) } - -func withTeamMembershipTxRetry(parentCtx context.Context, f func(ctx context.Context) error) error { - const maxAttempts = 3 - var err error - for i := 0; i < maxAttempts; i++ { - err = db.WithTx(parentCtx, f) - if err == nil || !isRetriableTeamMembershipTxError(err) || i == maxAttempts-1 { - return err - } - time.Sleep(time.Duration(i+1) * 10 * time.Millisecond) - } - return err -} - -func isRetriableTeamMembershipTxError(err error) bool { - msg := strings.ToLower(err.Error()) - if strings.Contains(msg, "deadlock") || strings.Contains(msg, "serialization failure") { - return true - } - // SQLSTATE 40P01 - if strings.Contains(msg, "40p01") { - return true - } - // MySQL ER_LOCK_DEADLOCK and MSSQL deadlock victim are frequently surfaced as numeric codes in error text. - for _, code := range []string{"1213", "1205"} { - if strings.Contains(msg, " "+code+" ") || strings.Contains(msg, ":"+code) || strings.Contains(msg, "("+code+")") { - return true - } - } - return false -} diff --git a/services/org/team_test.go b/services/org/team_test.go index 3784f9be6d..e03ab1ea2d 100644 --- a/services/org/team_test.go +++ b/services/org/team_test.go @@ -339,12 +339,10 @@ func TestTeamMemberConcurrentAddRemoveIdempotent(t *testing.T) { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) var wg sync.WaitGroup - for i := 0; i < 8; i++ { - wg.Add(1) - go func() { - defer wg.Done() + for range 8 { + wg.Go(func() { assert.NoError(t, AddTeamMember(ctx, team, user)) - }() + }) } wg.Wait() @@ -352,12 +350,10 @@ func TestTeamMemberConcurrentAddRemoveIdempotent(t *testing.T) { assert.NoError(t, err) assert.EqualValues(t, 1, count) - for i := 0; i < 8; i++ { - wg.Add(1) - go func() { - defer wg.Done() + for range 8 { + wg.Go(func() { assert.NoError(t, RemoveTeamMember(ctx, team, user)) - }() + }) } wg.Wait()