diff --git a/models/actions/run.go b/models/actions/run.go index f0ab61b200..f5ccba06c2 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -282,77 +282,72 @@ func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID strin // InsertRun inserts a run // The title will be cut off at 255 characters if it's longer than 255 characters. func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWorkflow) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - index, err := db.GetNextResourceIndex(ctx, "action_run_index", run.RepoID) - if err != nil { - return err - } - run.Index = index - run.Title = util.EllipsisDisplayString(run.Title, 255) - - if err := db.Insert(ctx, run); err != nil { - return err - } - - if run.Repo == nil { - repo, err := repo_model.GetRepositoryByID(ctx, run.RepoID) + return db.WithTx(ctx, func(ctx context.Context) error { + index, err := db.GetNextResourceIndex(ctx, "action_run_index", run.RepoID) if err != nil { return err } - run.Repo = repo - } + run.Index = index + run.Title = util.EllipsisDisplayString(run.Title, 255) - if err := updateRepoRunsNumbers(ctx, run.Repo); err != nil { - return err - } - - runJobs := make([]*ActionRunJob, 0, len(jobs)) - var hasWaiting bool - for _, v := range jobs { - id, job := v.Job() - needs := job.Needs() - if err := v.SetJob(id, job.EraseNeeds()); err != nil { + if err := db.Insert(ctx, run); err != nil { return err } - payload, _ := v.Marshal() - status := StatusWaiting - if len(needs) > 0 || run.NeedApproval { - status = StatusBlocked - } else { - hasWaiting = true - } - job.Name = util.EllipsisDisplayString(job.Name, 255) - runJobs = append(runJobs, &ActionRunJob{ - RunID: run.ID, - RepoID: run.RepoID, - OwnerID: run.OwnerID, - CommitSHA: run.CommitSHA, - IsForkPullRequest: run.IsForkPullRequest, - Name: job.Name, - WorkflowPayload: payload, - JobID: id, - Needs: needs, - RunsOn: job.RunsOn(), - Status: status, - }) - } - if err := db.Insert(ctx, runJobs); err != nil { - return err - } - // if there is a job in the waiting status, increase tasks version. - if hasWaiting { - if err := IncreaseTaskVersion(ctx, run.OwnerID, run.RepoID); err != nil { + if run.Repo == nil { + repo, err := repo_model.GetRepositoryByID(ctx, run.RepoID) + if err != nil { + return err + } + run.Repo = repo + } + + if err := updateRepoRunsNumbers(ctx, run.Repo); err != nil { return err } - } - return committer.Commit() + runJobs := make([]*ActionRunJob, 0, len(jobs)) + var hasWaiting bool + for _, v := range jobs { + id, job := v.Job() + needs := job.Needs() + if err := v.SetJob(id, job.EraseNeeds()); err != nil { + return err + } + payload, _ := v.Marshal() + status := StatusWaiting + if len(needs) > 0 || run.NeedApproval { + status = StatusBlocked + } else { + hasWaiting = true + } + job.Name = util.EllipsisDisplayString(job.Name, 255) + runJobs = append(runJobs, &ActionRunJob{ + RunID: run.ID, + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: run.CommitSHA, + IsForkPullRequest: run.IsForkPullRequest, + Name: job.Name, + WorkflowPayload: payload, + JobID: id, + Needs: needs, + RunsOn: job.RunsOn(), + Status: status, + }) + } + if err := db.Insert(ctx, runJobs); err != nil { + return err + } + + // if there is a job in the waiting status, increase tasks version. + if hasWaiting { + if err := IncreaseTaskVersion(ctx, run.OwnerID, run.RepoID); err != nil { + return err + } + } + return nil + }) } func GetRunByRepoAndID(ctx context.Context, repoID, runID int64) (*ActionRun, error) { diff --git a/models/actions/schedule.go b/models/actions/schedule.go index 2edf483fe0..ffde5092e0 100644 --- a/models/actions/schedule.go +++ b/models/actions/schedule.go @@ -56,65 +56,54 @@ func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error { return nil } - // Begin transaction - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - // Loop through each schedule row - for _, row := range rows { - row.Title = util.EllipsisDisplayString(row.Title, 255) - // Create new schedule row - if err = db.Insert(ctx, row); err != nil { - return err - } - - // Loop through each schedule spec and create a new spec row - now := time.Now() - - for _, spec := range row.Specs { - specRow := &ActionScheduleSpec{ - RepoID: row.RepoID, - ScheduleID: row.ID, - Spec: spec, - } - // Parse the spec and check for errors - schedule, err := specRow.Parse() - if err != nil { - continue // skip to the next spec if there's an error - } - - specRow.Next = timeutil.TimeStamp(schedule.Next(now).Unix()) - - // Insert the new schedule spec row - if err = db.Insert(ctx, specRow); err != nil { + return db.WithTx(ctx, func(ctx context.Context) error { + // Loop through each schedule row + for _, row := range rows { + row.Title = util.EllipsisDisplayString(row.Title, 255) + // Create new schedule row + if err := db.Insert(ctx, row); err != nil { return err } - } - } - // Commit transaction - return committer.Commit() + // Loop through each schedule spec and create a new spec row + now := time.Now() + + for _, spec := range row.Specs { + specRow := &ActionScheduleSpec{ + RepoID: row.RepoID, + ScheduleID: row.ID, + Spec: spec, + } + // Parse the spec and check for errors + schedule, err := specRow.Parse() + if err != nil { + continue // skip to the next spec if there's an error + } + + specRow.Next = timeutil.TimeStamp(schedule.Next(now).Unix()) + + // Insert the new schedule spec row + if err = db.Insert(ctx, specRow); err != nil { + return err + } + } + } + return nil + }) } func DeleteScheduleTaskByRepo(ctx context.Context, id int64) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err := db.GetEngine(ctx).Delete(&ActionSchedule{RepoID: id}); err != nil { + return err + } - if _, err := db.GetEngine(ctx).Delete(&ActionSchedule{RepoID: id}); err != nil { - return err - } + if _, err := db.GetEngine(ctx).Delete(&ActionScheduleSpec{RepoID: id}); err != nil { + return err + } - if _, err := db.GetEngine(ctx).Delete(&ActionScheduleSpec{RepoID: id}); err != nil { - return err - } - - return committer.Commit() + return nil + }) } func CleanRepoScheduleTasks(ctx context.Context, repo *repo_model.Repository) ([]*ActionRunJob, error) { diff --git a/models/actions/task.go b/models/actions/task.go index e0756b10c2..c1306a8418 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -352,78 +352,70 @@ func UpdateTaskByState(ctx context.Context, runnerID int64, state *runnerv1.Task stepStates[v.Id] = v } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() + return db.WithTx2(ctx, func(ctx context.Context) (*ActionTask, error) { + e := db.GetEngine(ctx) - e := db.GetEngine(ctx) + task := &ActionTask{} + if has, err := e.ID(state.Id).Get(task); err != nil { + return nil, err + } else if !has { + return nil, util.ErrNotExist + } else if runnerID != task.RunnerID { + return nil, errors.New("invalid runner for task") + } - task := &ActionTask{} - if has, err := e.ID(state.Id).Get(task); err != nil { - return nil, err - } else if !has { - return nil, util.ErrNotExist - } else if runnerID != task.RunnerID { - return nil, errors.New("invalid runner for task") - } + if task.Status.IsDone() { + // the state is final, do nothing + return task, nil + } + + // state.Result is not unspecified means the task is finished + if state.Result != runnerv1.Result_RESULT_UNSPECIFIED { + task.Status = Status(state.Result) + task.Stopped = timeutil.TimeStamp(state.StoppedAt.AsTime().Unix()) + if err := UpdateTask(ctx, task, "status", "stopped"); err != nil { + return nil, err + } + if _, err := UpdateRunJob(ctx, &ActionRunJob{ + ID: task.JobID, + Status: task.Status, + Stopped: task.Stopped, + }, nil); err != nil { + return nil, err + } + } else { + // Force update ActionTask.Updated to avoid the task being judged as a zombie task + task.Updated = timeutil.TimeStampNow() + if err := UpdateTask(ctx, task, "updated"); err != nil { + return nil, err + } + } + + if err := task.LoadAttributes(ctx); err != nil { + return nil, err + } + + for _, step := range task.Steps { + var result runnerv1.Result + if v, ok := stepStates[step.Index]; ok { + result = v.Result + step.LogIndex = v.LogIndex + step.LogLength = v.LogLength + step.Started = convertTimestamp(v.StartedAt) + step.Stopped = convertTimestamp(v.StoppedAt) + } + if result != runnerv1.Result_RESULT_UNSPECIFIED { + step.Status = Status(result) + } else if step.Started != 0 { + step.Status = StatusRunning + } + if _, err := e.ID(step.ID).Update(step); err != nil { + return nil, err + } + } - if task.Status.IsDone() { - // the state is final, do nothing return task, nil - } - - // state.Result is not unspecified means the task is finished - if state.Result != runnerv1.Result_RESULT_UNSPECIFIED { - task.Status = Status(state.Result) - task.Stopped = timeutil.TimeStamp(state.StoppedAt.AsTime().Unix()) - if err := UpdateTask(ctx, task, "status", "stopped"); err != nil { - return nil, err - } - if _, err := UpdateRunJob(ctx, &ActionRunJob{ - ID: task.JobID, - Status: task.Status, - Stopped: task.Stopped, - }, nil); err != nil { - return nil, err - } - } else { - // Force update ActionTask.Updated to avoid the task being judged as a zombie task - task.Updated = timeutil.TimeStampNow() - if err := UpdateTask(ctx, task, "updated"); err != nil { - return nil, err - } - } - - if err := task.LoadAttributes(ctx); err != nil { - return nil, err - } - - for _, step := range task.Steps { - var result runnerv1.Result - if v, ok := stepStates[step.Index]; ok { - result = v.Result - step.LogIndex = v.LogIndex - step.LogLength = v.LogLength - step.Started = convertTimestamp(v.StartedAt) - step.Stopped = convertTimestamp(v.StoppedAt) - } - if result != runnerv1.Result_RESULT_UNSPECIFIED { - step.Status = Status(result) - } else if step.Started != 0 { - step.Status = StatusRunning - } - if _, err := e.ID(step.ID).Update(step); err != nil { - return nil, err - } - } - - if err := committer.Commit(); err != nil { - return nil, err - } - - return task, nil + }) } func StopTask(ctx context.Context, taskID int64, status Status) error { diff --git a/models/actions/tasks_version.go b/models/actions/tasks_version.go index 96c5468c1a..b686ce2443 100644 --- a/models/actions/tasks_version.go +++ b/models/actions/tasks_version.go @@ -73,33 +73,29 @@ func increaseTasksVersionByScope(ctx context.Context, ownerID, repoID int64) err } func IncreaseTaskVersion(ctx context.Context, ownerID, repoID int64) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - // 1. increase global - if err := increaseTasksVersionByScope(ctx, 0, 0); err != nil { - log.Error("IncreaseTasksVersionByScope(Global): %v", err) - return err - } - - // 2. increase owner - if ownerID > 0 { - if err := increaseTasksVersionByScope(ctx, ownerID, 0); err != nil { - log.Error("IncreaseTasksVersionByScope(Owner): %v", err) + return db.WithTx(ctx, func(ctx context.Context) error { + // 1. increase global + if err := increaseTasksVersionByScope(ctx, 0, 0); err != nil { + log.Error("IncreaseTasksVersionByScope(Global): %v", err) return err } - } - // 3. increase repo - if repoID > 0 { - if err := increaseTasksVersionByScope(ctx, 0, repoID); err != nil { - log.Error("IncreaseTasksVersionByScope(Repo): %v", err) - return err + // 2. increase owner + if ownerID > 0 { + if err := increaseTasksVersionByScope(ctx, ownerID, 0); err != nil { + log.Error("IncreaseTasksVersionByScope(Owner): %v", err) + return err + } } - } - return committer.Commit() + // 3. increase repo + if repoID > 0 { + if err := increaseTasksVersionByScope(ctx, 0, repoID); err != nil { + log.Error("IncreaseTasksVersionByScope(Repo): %v", err) + return err + } + } + + return nil + }) } diff --git a/models/activities/notification_list.go b/models/activities/notification_list.go index b47f5dc404..6539e14ea2 100644 --- a/models/activities/notification_list.go +++ b/models/activities/notification_list.go @@ -70,17 +70,9 @@ func (opts FindNotificationOptions) ToOrders() string { // for each watcher, or updates it if already exists // receiverID > 0 just send to receiver, else send to all watcher func CreateOrUpdateIssueNotifications(ctx context.Context, issueID, commentID, notificationAuthorID, receiverID int64) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err := createOrUpdateIssueNotifications(ctx, issueID, commentID, notificationAuthorID, receiverID); err != nil { - return err - } - - return committer.Commit() + return db.WithTx(ctx, func(ctx context.Context) error { + return createOrUpdateIssueNotifications(ctx, issueID, commentID, notificationAuthorID, receiverID) + }) } func createOrUpdateIssueNotifications(ctx context.Context, issueID, commentID, notificationAuthorID, receiverID int64) error { diff --git a/models/asymkey/gpg_key.go b/models/asymkey/gpg_key.go index 220f46ad1d..38de7cbda6 100644 --- a/models/asymkey/gpg_key.go +++ b/models/asymkey/gpg_key.go @@ -228,17 +228,10 @@ func DeleteGPGKey(ctx context.Context, doer *user_model.User, id int64) (err err return fmt.Errorf("GetPublicKeyByID: %w", err) } - ctx, committer, err := db.TxContext(ctx) - if err != nil { + return db.WithTx(ctx, func(ctx context.Context) error { + _, err = deleteGPGKey(ctx, key.KeyID) return err - } - defer committer.Close() - - if _, err = deleteGPGKey(ctx, key.KeyID); err != nil { - return err - } - - return committer.Commit() + }) } func FindGPGKeyWithSubKeys(ctx context.Context, keyID string) ([]*GPGKey, error) { diff --git a/models/asymkey/gpg_key_verify.go b/models/asymkey/gpg_key_verify.go index 5ab2fd8081..55c64973b4 100644 --- a/models/asymkey/gpg_key_verify.go +++ b/models/asymkey/gpg_key_verify.go @@ -14,97 +14,76 @@ import ( "code.gitea.io/gitea/modules/log" ) -// __________________ ________ ____ __. -// / _____/\______ \/ _____/ | |/ _|____ ___.__. -// / \ ___ | ___/ \ ___ | <_/ __ < | | -// \ \_\ \| | \ \_\ \ | | \ ___/\___ | -// \______ /|____| \______ / |____|__ \___ > ____| -// \/ \/ \/ \/\/ -// ____ ____ .__ _____ -// \ \ / /___________|__|/ ____\__.__. -// \ Y // __ \_ __ \ \ __< | | -// \ /\ ___/| | \/ || | \___ | -// \___/ \___ >__| |__||__| / ____| -// \/ \/ - // This file provides functions relating verifying gpg keys // VerifyGPGKey marks a GPG key as verified func VerifyGPGKey(ctx context.Context, ownerID int64, keyID, token, signature string) (string, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return "", err - } - defer committer.Close() + return db.WithTx2(ctx, func(ctx context.Context) (string, error) { + key := new(GPGKey) - key := new(GPGKey) - - has, err := db.GetEngine(ctx).Where("owner_id = ? AND key_id = ?", ownerID, keyID).Get(key) - if err != nil { - return "", err - } else if !has { - return "", ErrGPGKeyNotExist{} - } - - if err := key.LoadSubKeys(ctx); err != nil { - return "", err - } - - sig, err := ExtractSignature(signature) - if err != nil { - return "", ErrGPGInvalidTokenSignature{ - ID: key.KeyID, - Wrapped: err, + has, err := db.GetEngine(ctx).Where("owner_id = ? AND key_id = ?", ownerID, keyID).Get(key) + if err != nil { + return "", err + } else if !has { + return "", ErrGPGKeyNotExist{} } - } - signer, err := hashAndVerifyWithSubKeys(sig, token, key) - if err != nil { - return "", ErrGPGInvalidTokenSignature{ - ID: key.KeyID, - Wrapped: err, + if err := key.LoadSubKeys(ctx); err != nil { + return "", err } - } - if signer == nil { - signer, err = hashAndVerifyWithSubKeys(sig, token+"\n", key) + + sig, err := ExtractSignature(signature) if err != nil { return "", ErrGPGInvalidTokenSignature{ ID: key.KeyID, Wrapped: err, } } - } - if signer == nil { - signer, err = hashAndVerifyWithSubKeys(sig, token+"\n\n", key) + + signer, err := hashAndVerifyWithSubKeys(sig, token, key) if err != nil { return "", ErrGPGInvalidTokenSignature{ ID: key.KeyID, Wrapped: err, } } - } - - if signer == nil { - log.Debug("VerifyGPGKey failed: no signer") - return "", ErrGPGInvalidTokenSignature{ - ID: key.KeyID, + if signer == nil { + signer, err = hashAndVerifyWithSubKeys(sig, token+"\n", key) + if err != nil { + return "", ErrGPGInvalidTokenSignature{ + ID: key.KeyID, + Wrapped: err, + } + } + } + if signer == nil { + signer, err = hashAndVerifyWithSubKeys(sig, token+"\n\n", key) + if err != nil { + return "", ErrGPGInvalidTokenSignature{ + ID: key.KeyID, + Wrapped: err, + } + } } - } - if signer.PrimaryKeyID != key.KeyID && signer.KeyID != key.KeyID { - return "", ErrGPGKeyNotExist{} - } + if signer == nil { + log.Debug("VerifyGPGKey failed: no signer") + return "", ErrGPGInvalidTokenSignature{ + ID: key.KeyID, + } + } - key.Verified = true - if _, err := db.GetEngine(ctx).ID(key.ID).SetExpr("verified", true).Update(new(GPGKey)); err != nil { - return "", err - } + if signer.PrimaryKeyID != key.KeyID && signer.KeyID != key.KeyID { + return "", ErrGPGKeyNotExist{} + } - if err := committer.Commit(); err != nil { - return "", err - } + key.Verified = true + if _, err := db.GetEngine(ctx).ID(key.ID).SetExpr("verified", true).Update(new(GPGKey)); err != nil { + return "", err + } - return key.KeyID, nil + return key.KeyID, nil + }) } // VerificationToken returns token for the user that will be valid in minutes (time) diff --git a/models/asymkey/ssh_key.go b/models/asymkey/ssh_key.go index dd94070fb9..87205f0651 100644 --- a/models/asymkey/ssh_key.go +++ b/models/asymkey/ssh_key.go @@ -99,40 +99,36 @@ func AddPublicKey(ctx context.Context, ownerID int64, name, content string, auth return nil, err } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() + return db.WithTx2(ctx, func(ctx context.Context) (*PublicKey, error) { + if err := checkKeyFingerprint(ctx, fingerprint); err != nil { + return nil, err + } - if err := checkKeyFingerprint(ctx, fingerprint); err != nil { - return nil, err - } + // Key name of same user cannot be duplicated. + has, err := db.GetEngine(ctx). + Where("owner_id = ? AND name = ?", ownerID, name). + Get(new(PublicKey)) + if err != nil { + return nil, err + } else if has { + return nil, ErrKeyNameAlreadyUsed{ownerID, name} + } - // Key name of same user cannot be duplicated. - has, err := db.GetEngine(ctx). - Where("owner_id = ? AND name = ?", ownerID, name). - Get(new(PublicKey)) - if err != nil { - return nil, err - } else if has { - return nil, ErrKeyNameAlreadyUsed{ownerID, name} - } + key := &PublicKey{ + OwnerID: ownerID, + Name: name, + Fingerprint: fingerprint, + Content: content, + Mode: perm.AccessModeWrite, + Type: KeyTypeUser, + LoginSourceID: authSourceID, + } + if err = addKey(ctx, key); err != nil { + return nil, fmt.Errorf("addKey: %w", err) + } - key := &PublicKey{ - OwnerID: ownerID, - Name: name, - Fingerprint: fingerprint, - Content: content, - Mode: perm.AccessModeWrite, - Type: KeyTypeUser, - LoginSourceID: authSourceID, - } - if err = addKey(ctx, key); err != nil { - return nil, fmt.Errorf("addKey: %w", err) - } - - return key, committer.Commit() + return key, nil + }) } // GetPublicKeyByID returns public key by given ID. @@ -288,33 +284,24 @@ func PublicKeyIsExternallyManaged(ctx context.Context, id int64) (bool, error) { // deleteKeysMarkedForDeletion returns true if ssh keys needs update func deleteKeysMarkedForDeletion(ctx context.Context, keys []string) (bool, error) { - // Start session - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return false, err - } - defer committer.Close() - - // Delete keys marked for deletion - var sshKeysNeedUpdate bool - for _, KeyToDelete := range keys { - key, err := SearchPublicKeyByContent(ctx, KeyToDelete) - if err != nil { - log.Error("SearchPublicKeyByContent: %v", err) - continue + return db.WithTx2(ctx, func(ctx context.Context) (bool, error) { + // Delete keys marked for deletion + var sshKeysNeedUpdate bool + for _, KeyToDelete := range keys { + key, err := SearchPublicKeyByContent(ctx, KeyToDelete) + if err != nil { + log.Error("SearchPublicKeyByContent: %v", err) + continue + } + if _, err = db.DeleteByID[PublicKey](ctx, key.ID); err != nil { + log.Error("DeleteByID[PublicKey]: %v", err) + continue + } + sshKeysNeedUpdate = true } - if _, err = db.DeleteByID[PublicKey](ctx, key.ID); err != nil { - log.Error("DeleteByID[PublicKey]: %v", err) - continue - } - sshKeysNeedUpdate = true - } - if err := committer.Commit(); err != nil { - return false, err - } - - return sshKeysNeedUpdate, nil + return sshKeysNeedUpdate, nil + }) } // AddPublicKeysBySource add a users public keys. Returns true if there are changes. diff --git a/models/asymkey/ssh_key_deploy.go b/models/asymkey/ssh_key_deploy.go index 923c5020ed..4ab84eabcf 100644 --- a/models/asymkey/ssh_key_deploy.go +++ b/models/asymkey/ssh_key_deploy.go @@ -125,39 +125,35 @@ func AddDeployKey(ctx context.Context, repoID int64, name, content string, readO accessMode = perm.AccessModeWrite } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - pkey, exist, err := db.Get[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint}) - if err != nil { - return nil, err - } else if exist { - if pkey.Type != KeyTypeDeploy { - return nil, ErrKeyAlreadyExist{0, fingerprint, ""} + return db.WithTx2(ctx, func(ctx context.Context) (*DeployKey, error) { + pkey, exist, err := db.Get[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint}) + if err != nil { + return nil, err + } else if exist { + if pkey.Type != KeyTypeDeploy { + return nil, ErrKeyAlreadyExist{0, fingerprint, ""} + } + } else { + // First time use this deploy key. + pkey = &PublicKey{ + Fingerprint: fingerprint, + Mode: accessMode, + Type: KeyTypeDeploy, + Content: content, + Name: name, + } + if err = addKey(ctx, pkey); err != nil { + return nil, fmt.Errorf("addKey: %w", err) + } } - } else { - // First time use this deploy key. - pkey = &PublicKey{ - Fingerprint: fingerprint, - Mode: accessMode, - Type: KeyTypeDeploy, - Content: content, - Name: name, - } - if err = addKey(ctx, pkey); err != nil { - return nil, fmt.Errorf("addKey: %w", err) - } - } - key, err := addDeployKey(ctx, pkey.ID, repoID, name, pkey.Fingerprint, accessMode) - if err != nil { - return nil, err - } + key, err := addDeployKey(ctx, pkey.ID, repoID, name, pkey.Fingerprint, accessMode) + if err != nil { + return nil, err + } - return key, committer.Commit() + return key, nil + }) } // GetDeployKeyByID returns deploy key by given ID. diff --git a/models/asymkey/ssh_key_verify.go b/models/asymkey/ssh_key_verify.go index 0cf29ca9f1..04917239ee 100644 --- a/models/asymkey/ssh_key_verify.go +++ b/models/asymkey/ssh_key_verify.go @@ -15,41 +15,33 @@ import ( // VerifySSHKey marks a SSH key as verified func VerifySSHKey(ctx context.Context, ownerID int64, fingerprint, token, signature string) (string, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return "", err - } - defer committer.Close() + return db.WithTx2(ctx, func(ctx context.Context) (string, error) { + key := new(PublicKey) - key := new(PublicKey) + has, err := db.GetEngine(ctx).Where("owner_id = ? AND fingerprint = ?", ownerID, fingerprint).Get(key) + if err != nil { + return "", err + } else if !has { + return "", ErrKeyNotExist{} + } - has, err := db.GetEngine(ctx).Where("owner_id = ? AND fingerprint = ?", ownerID, fingerprint).Get(key) - if err != nil { - return "", err - } else if !has { - return "", ErrKeyNotExist{} - } - - err = sshsig.Verify(strings.NewReader(token), []byte(signature), []byte(key.Content), "gitea") - if err != nil { - // edge case for Windows based shells that will add CR LF if piped to ssh-keygen command - // see https://github.com/PowerShell/PowerShell/issues/5974 - if sshsig.Verify(strings.NewReader(token+"\r\n"), []byte(signature), []byte(key.Content), "gitea") != nil { - log.Debug("VerifySSHKey sshsig.Verify failed: %v", err) - return "", ErrSSHInvalidTokenSignature{ - Fingerprint: key.Fingerprint, + err = sshsig.Verify(strings.NewReader(token), []byte(signature), []byte(key.Content), "gitea") + if err != nil { + // edge case for Windows based shells that will add CR LF if piped to ssh-keygen command + // see https://github.com/PowerShell/PowerShell/issues/5974 + if sshsig.Verify(strings.NewReader(token+"\r\n"), []byte(signature), []byte(key.Content), "gitea") != nil { + log.Debug("VerifySSHKey sshsig.Verify failed: %v", err) + return "", ErrSSHInvalidTokenSignature{ + Fingerprint: key.Fingerprint, + } } } - } - key.Verified = true - if _, err := db.GetEngine(ctx).ID(key.ID).Cols("verified").Update(key); err != nil { - return "", err - } + key.Verified = true + if _, err := db.GetEngine(ctx).ID(key.ID).Cols("verified").Update(key); err != nil { + return "", err + } - if err := committer.Commit(); err != nil { - return "", err - } - - return key.Fingerprint, nil + return key.Fingerprint, nil + }) } diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 55af4e9036..d664841306 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -289,35 +289,31 @@ type UpdateOAuth2ApplicationOptions struct { // UpdateOAuth2Application updates an oauth2 application func UpdateOAuth2Application(ctx context.Context, opts UpdateOAuth2ApplicationOptions) (*OAuth2Application, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() + return db.WithTx2(ctx, func(ctx context.Context) (*OAuth2Application, error) { + app, err := GetOAuth2ApplicationByID(ctx, opts.ID) + if err != nil { + return nil, err + } + if app.UID != opts.UserID { + return nil, errors.New("UID mismatch") + } + builtinApps := BuiltinApplications() + if _, builtin := builtinApps[app.ClientID]; builtin { + return nil, fmt.Errorf("failed to edit OAuth2 application: application is locked: %s", app.ClientID) + } - app, err := GetOAuth2ApplicationByID(ctx, opts.ID) - if err != nil { - return nil, err - } - if app.UID != opts.UserID { - return nil, errors.New("UID mismatch") - } - builtinApps := BuiltinApplications() - if _, builtin := builtinApps[app.ClientID]; builtin { - return nil, fmt.Errorf("failed to edit OAuth2 application: application is locked: %s", app.ClientID) - } + app.Name = opts.Name + app.RedirectURIs = opts.RedirectURIs + app.ConfidentialClient = opts.ConfidentialClient + app.SkipSecondaryAuthorization = opts.SkipSecondaryAuthorization - app.Name = opts.Name - app.RedirectURIs = opts.RedirectURIs - app.ConfidentialClient = opts.ConfidentialClient - app.SkipSecondaryAuthorization = opts.SkipSecondaryAuthorization + if err = updateOAuth2Application(ctx, app); err != nil { + return nil, err + } + app.ClientSecret = "" - if err = updateOAuth2Application(ctx, app); err != nil { - return nil, err - } - app.ClientSecret = "" - - return app, committer.Commit() + return app, nil + }) } func updateOAuth2Application(ctx context.Context, app *OAuth2Application) error { @@ -358,23 +354,17 @@ func deleteOAuth2Application(ctx context.Context, id, userid int64) error { // DeleteOAuth2Application deletes the application with the given id and the grants and auth codes related to it. It checks if the userid was the creator of the app. func DeleteOAuth2Application(ctx context.Context, id, userid int64) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - app, err := GetOAuth2ApplicationByID(ctx, id) - if err != nil { - return err - } - builtinApps := BuiltinApplications() - if _, builtin := builtinApps[app.ClientID]; builtin { - return fmt.Errorf("failed to delete OAuth2 application: application is locked: %s", app.ClientID) - } - if err := deleteOAuth2Application(ctx, id, userid); err != nil { - return err - } - return committer.Commit() + return db.WithTx(ctx, func(ctx context.Context) error { + app, err := GetOAuth2ApplicationByID(ctx, id) + if err != nil { + return err + } + builtinApps := BuiltinApplications() + if _, builtin := builtinApps[app.ClientID]; builtin { + return fmt.Errorf("failed to delete OAuth2 application: application is locked: %s", app.ClientID) + } + return deleteOAuth2Application(ctx, id, userid) + }) } ////////////////////////////////////////////////////// diff --git a/models/auth/session.go b/models/auth/session.go index 75a205f702..0378d0ec6f 100644 --- a/models/auth/session.go +++ b/models/auth/session.go @@ -35,26 +35,22 @@ func UpdateSession(ctx context.Context, key string, data []byte) error { // ReadSession reads the data for the provided session func ReadSession(ctx context.Context, key string) (*Session, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - session, exist, err := db.Get[Session](ctx, builder.Eq{"`key`": key}) - if err != nil { - return nil, err - } else if !exist { - session = &Session{ - Key: key, - Expiry: timeutil.TimeStampNow(), - } - if err := db.Insert(ctx, session); err != nil { + return db.WithTx2(ctx, func(ctx context.Context) (*Session, error) { + session, exist, err := db.Get[Session](ctx, builder.Eq{"`key`": key}) + if err != nil { return nil, err + } else if !exist { + session = &Session{ + Key: key, + Expiry: timeutil.TimeStampNow(), + } + if err := db.Insert(ctx, session); err != nil { + return nil, err + } } - } - return session, committer.Commit() + return session, nil + }) } // ExistSession checks if a session exists @@ -72,40 +68,36 @@ func DestroySession(ctx context.Context, key string) error { // RegenerateSession regenerates a session from the old id func RegenerateSession(ctx context.Context, oldKey, newKey string) (*Session, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() + return db.WithTx2(ctx, func(ctx context.Context) (*Session, error) { + if has, err := db.Exist[Session](ctx, builder.Eq{"`key`": newKey}); err != nil { + return nil, err + } else if has { + return nil, fmt.Errorf("session Key: %s already exists", newKey) + } - if has, err := db.Exist[Session](ctx, builder.Eq{"`key`": newKey}); err != nil { - return nil, err - } else if has { - return nil, fmt.Errorf("session Key: %s already exists", newKey) - } + if has, err := db.Exist[Session](ctx, builder.Eq{"`key`": oldKey}); err != nil { + return nil, err + } else if !has { + if err := db.Insert(ctx, &Session{ + Key: oldKey, + Expiry: timeutil.TimeStampNow(), + }); err != nil { + return nil, err + } + } - if has, err := db.Exist[Session](ctx, builder.Eq{"`key`": oldKey}); err != nil { - return nil, err - } else if !has { - if err := db.Insert(ctx, &Session{ - Key: oldKey, - Expiry: timeutil.TimeStampNow(), - }); err != nil { + if _, err := db.Exec(ctx, "UPDATE "+db.TableName(&Session{})+" SET `key` = ? WHERE `key`=?", newKey, oldKey); err != nil { return nil, err } - } - if _, err := db.Exec(ctx, "UPDATE "+db.TableName(&Session{})+" SET `key` = ? WHERE `key`=?", newKey, oldKey); err != nil { - return nil, err - } + s, _, err := db.Get[Session](ctx, builder.Eq{"`key`": newKey}) + if err != nil { + // is not exist, it should be impossible + return nil, err + } - s, _, err := db.Get[Session](ctx, builder.Eq{"`key`": newKey}) - if err != nil { - // is not exist, it should be impossible - return nil, err - } - - return s, committer.Commit() + return s, nil + }) } // CountSessions returns the number of sessions diff --git a/models/git/commit_status.go b/models/git/commit_status.go index f85e1b15e5..e255bca5d0 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -470,35 +470,31 @@ func NewCommitStatus(ctx context.Context, opts NewCommitStatusOptions) error { return fmt.Errorf("NewCommitStatus[%s, %s]: no user specified", opts.Repo.FullName(), opts.SHA) } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %w", opts.Repo.ID, opts.Creator.ID, opts.SHA, err) - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + // Get the next Status Index + idx, err := GetNextCommitStatusIndex(ctx, opts.Repo.ID, opts.SHA.String()) + if err != nil { + return fmt.Errorf("generate commit status index failed: %w", err) + } - // Get the next Status Index - idx, err := GetNextCommitStatusIndex(ctx, opts.Repo.ID, opts.SHA.String()) - if err != nil { - return fmt.Errorf("generate commit status index failed: %w", err) - } + opts.CommitStatus.Description = strings.TrimSpace(opts.CommitStatus.Description) + opts.CommitStatus.Context = strings.TrimSpace(opts.CommitStatus.Context) + opts.CommitStatus.TargetURL = strings.TrimSpace(opts.CommitStatus.TargetURL) + opts.CommitStatus.SHA = opts.SHA.String() + opts.CommitStatus.CreatorID = opts.Creator.ID + opts.CommitStatus.RepoID = opts.Repo.ID + opts.CommitStatus.Index = idx + log.Debug("NewCommitStatus[%s, %s]: %d", opts.Repo.FullName(), opts.SHA, opts.CommitStatus.Index) - opts.CommitStatus.Description = strings.TrimSpace(opts.CommitStatus.Description) - opts.CommitStatus.Context = strings.TrimSpace(opts.CommitStatus.Context) - opts.CommitStatus.TargetURL = strings.TrimSpace(opts.CommitStatus.TargetURL) - opts.CommitStatus.SHA = opts.SHA.String() - opts.CommitStatus.CreatorID = opts.Creator.ID - opts.CommitStatus.RepoID = opts.Repo.ID - opts.CommitStatus.Index = idx - log.Debug("NewCommitStatus[%s, %s]: %d", opts.Repo.FullName(), opts.SHA, opts.CommitStatus.Index) + opts.CommitStatus.ContextHash = hashCommitStatusContext(opts.CommitStatus.Context) - opts.CommitStatus.ContextHash = hashCommitStatusContext(opts.CommitStatus.Context) + // Insert new CommitStatus + if err = db.Insert(ctx, opts.CommitStatus); err != nil { + return fmt.Errorf("insert CommitStatus[%s, %s]: %w", opts.Repo.FullName(), opts.SHA, err) + } - // Insert new CommitStatus - if _, err = db.GetEngine(ctx).Insert(opts.CommitStatus); err != nil { - return fmt.Errorf("insert CommitStatus[%s, %s]: %w", opts.Repo.FullName(), opts.SHA, err) - } - - return committer.Commit() + return nil + }) } // SignCommitWithStatuses represents a commit with validation of signature and status state. diff --git a/models/git/lfs.go b/models/git/lfs.go index e4fa2b446a..c471baf588 100644 --- a/models/git/lfs.go +++ b/models/git/lfs.go @@ -135,25 +135,18 @@ var ErrLFSObjectNotExist = db.ErrNotExist{Resource: "LFS Meta object"} // NewLFSMetaObject stores a given populated LFSMetaObject structure in the database // if it is not already present. func NewLFSMetaObject(ctx context.Context, repoID int64, p lfs.Pointer) (*LFSMetaObject, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - m, exist, err := db.Get[LFSMetaObject](ctx, builder.Eq{"repository_id": repoID, "oid": p.Oid}) if err != nil { return nil, err } else if exist { - return m, committer.Commit() + return m, nil } m = &LFSMetaObject{Pointer: p, RepositoryID: repoID} if err = db.Insert(ctx, m); err != nil { return nil, err } - - return m, committer.Commit() + return m, nil } // GetLFSMetaObjectByOid selects a LFSMetaObject entry from database by its OID. diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index 07ce7d4abf..c5f9a4e6de 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -70,32 +70,28 @@ func (l *LFSLock) LoadOwner(ctx context.Context) error { // CreateLFSLock creates a new lock. func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLock) (*LFSLock, error) { - dbCtx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() + return db.WithTx2(ctx, func(ctx context.Context) (*LFSLock, error) { + if err := CheckLFSAccessForRepo(ctx, lock.OwnerID, repo, perm.AccessModeWrite); err != nil { + return nil, err + } - if err := CheckLFSAccessForRepo(dbCtx, lock.OwnerID, repo, perm.AccessModeWrite); err != nil { - return nil, err - } + lock.Path = util.PathJoinRel(lock.Path) + lock.RepoID = repo.ID - lock.Path = util.PathJoinRel(lock.Path) - lock.RepoID = repo.ID + l, err := GetLFSLock(ctx, repo, lock.Path) + if err == nil { + return l, ErrLFSLockAlreadyExist{lock.RepoID, lock.Path} + } + if !IsErrLFSLockNotExist(err) { + return nil, err + } - l, err := GetLFSLock(dbCtx, repo, lock.Path) - if err == nil { - return l, ErrLFSLockAlreadyExist{lock.RepoID, lock.Path} - } - if !IsErrLFSLockNotExist(err) { - return nil, err - } + if err := db.Insert(ctx, lock); err != nil { + return nil, err + } - if err := db.Insert(dbCtx, lock); err != nil { - return nil, err - } - - return lock, committer.Commit() + return lock, nil + }) } // GetLFSLock returns release by given path. @@ -163,30 +159,26 @@ func CountLFSLockByRepoID(ctx context.Context, repoID int64) (int64, error) { // DeleteLFSLockByID deletes a lock by given ID. func DeleteLFSLockByID(ctx context.Context, id int64, repo *repo_model.Repository, u *user_model.User, force bool) (*LFSLock, error) { - dbCtx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() + return db.WithTx2(ctx, func(ctx context.Context) (*LFSLock, error) { + lock, err := GetLFSLockByID(ctx, id) + if err != nil { + return nil, err + } - lock, err := GetLFSLockByID(dbCtx, id) - if err != nil { - return nil, err - } + if err := CheckLFSAccessForRepo(ctx, u.ID, repo, perm.AccessModeWrite); err != nil { + return nil, err + } - if err := CheckLFSAccessForRepo(dbCtx, u.ID, repo, perm.AccessModeWrite); err != nil { - return nil, err - } + if !force && u.ID != lock.OwnerID { + return nil, errors.New("user doesn't own lock and force flag is not set") + } - if !force && u.ID != lock.OwnerID { - return nil, errors.New("user doesn't own lock and force flag is not set") - } + if _, err := db.GetEngine(ctx).ID(id).Delete(new(LFSLock)); err != nil { + return nil, err + } - if _, err := db.GetEngine(dbCtx).ID(id).Delete(new(LFSLock)); err != nil { - return nil, err - } - - return lock, committer.Commit() + return lock, nil + }) } // CheckLFSAccessForRepo check needed access mode base on action diff --git a/models/issues/comment.go b/models/issues/comment.go index 4fdb0c1808..d22f08fa87 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -766,81 +766,73 @@ func (c *Comment) CodeCommentLink(ctx context.Context) string { // CreateComment creates comment with context func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment, err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - e := db.GetEngine(ctx) - var LabelID int64 - if opts.Label != nil { - LabelID = opts.Label.ID - } - - var commentMetaData *CommentMetaData - if opts.ProjectColumnTitle != "" { - commentMetaData = &CommentMetaData{ - ProjectColumnID: opts.ProjectColumnID, - ProjectColumnTitle: opts.ProjectColumnTitle, - ProjectTitle: opts.ProjectTitle, + return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { + var LabelID int64 + if opts.Label != nil { + LabelID = opts.Label.ID } - } - comment := &Comment{ - Type: opts.Type, - PosterID: opts.Doer.ID, - Poster: opts.Doer, - IssueID: opts.Issue.ID, - LabelID: LabelID, - OldMilestoneID: opts.OldMilestoneID, - MilestoneID: opts.MilestoneID, - OldProjectID: opts.OldProjectID, - ProjectID: opts.ProjectID, - TimeID: opts.TimeID, - RemovedAssignee: opts.RemovedAssignee, - AssigneeID: opts.AssigneeID, - AssigneeTeamID: opts.AssigneeTeamID, - CommitID: opts.CommitID, - CommitSHA: opts.CommitSHA, - Line: opts.LineNum, - Content: opts.Content, - OldTitle: opts.OldTitle, - NewTitle: opts.NewTitle, - OldRef: opts.OldRef, - NewRef: opts.NewRef, - DependentIssueID: opts.DependentIssueID, - TreePath: opts.TreePath, - ReviewID: opts.ReviewID, - Patch: opts.Patch, - RefRepoID: opts.RefRepoID, - RefIssueID: opts.RefIssueID, - RefCommentID: opts.RefCommentID, - RefAction: opts.RefAction, - RefIsPull: opts.RefIsPull, - IsForcePush: opts.IsForcePush, - Invalidated: opts.Invalidated, - CommentMetaData: commentMetaData, - } - if _, err = e.Insert(comment); err != nil { - return nil, err - } + var commentMetaData *CommentMetaData + if opts.ProjectColumnTitle != "" { + commentMetaData = &CommentMetaData{ + ProjectColumnID: opts.ProjectColumnID, + ProjectColumnTitle: opts.ProjectColumnTitle, + ProjectTitle: opts.ProjectTitle, + } + } - if err = opts.Repo.LoadOwner(ctx); err != nil { - return nil, err - } + comment := &Comment{ + Type: opts.Type, + PosterID: opts.Doer.ID, + Poster: opts.Doer, + IssueID: opts.Issue.ID, + LabelID: LabelID, + OldMilestoneID: opts.OldMilestoneID, + MilestoneID: opts.MilestoneID, + OldProjectID: opts.OldProjectID, + ProjectID: opts.ProjectID, + TimeID: opts.TimeID, + RemovedAssignee: opts.RemovedAssignee, + AssigneeID: opts.AssigneeID, + AssigneeTeamID: opts.AssigneeTeamID, + CommitID: opts.CommitID, + CommitSHA: opts.CommitSHA, + Line: opts.LineNum, + Content: opts.Content, + OldTitle: opts.OldTitle, + NewTitle: opts.NewTitle, + OldRef: opts.OldRef, + NewRef: opts.NewRef, + DependentIssueID: opts.DependentIssueID, + TreePath: opts.TreePath, + ReviewID: opts.ReviewID, + Patch: opts.Patch, + RefRepoID: opts.RefRepoID, + RefIssueID: opts.RefIssueID, + RefCommentID: opts.RefCommentID, + RefAction: opts.RefAction, + RefIsPull: opts.RefIsPull, + IsForcePush: opts.IsForcePush, + Invalidated: opts.Invalidated, + CommentMetaData: commentMetaData, + } + if err = db.Insert(ctx, comment); err != nil { + return nil, err + } - if err = updateCommentInfos(ctx, opts, comment); err != nil { - return nil, err - } + if err = opts.Repo.LoadOwner(ctx); err != nil { + return nil, err + } - if err = comment.AddCrossReferences(ctx, opts.Doer, false); err != nil { - return nil, err - } - if err = committer.Commit(); err != nil { - return nil, err - } - return comment, nil + if err = updateCommentInfos(ctx, opts, comment); err != nil { + return nil, err + } + + if err = comment.AddCrossReferences(ctx, opts.Doer, false); err != nil { + return nil, err + } + return comment, nil + }) } func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment *Comment) (err error) { @@ -1092,33 +1084,21 @@ func UpdateCommentInvalidate(ctx context.Context, c *Comment) error { // UpdateComment updates information of comment. func UpdateComment(ctx context.Context, c *Comment, contentVersion int, doer *user_model.User) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) + return db.WithTx(ctx, func(ctx context.Context) error { + c.ContentVersion = contentVersion + 1 - c.ContentVersion = contentVersion + 1 - - affected, err := sess.ID(c.ID).AllCols().Where("content_version = ?", contentVersion).Update(c) - if err != nil { - return err - } - if affected == 0 { - return ErrCommentAlreadyChanged - } - if err := c.LoadIssue(ctx); err != nil { - return err - } - if err := c.AddCrossReferences(ctx, doer, true); err != nil { - return err - } - if err := committer.Commit(); err != nil { - return fmt.Errorf("Commit: %w", err) - } - - return nil + affected, err := db.GetEngine(ctx).ID(c.ID).AllCols().Where("content_version = ?", contentVersion).Update(c) + if err != nil { + return err + } + if affected == 0 { + return ErrCommentAlreadyChanged + } + if err := c.LoadIssue(ctx); err != nil { + return err + } + return c.AddCrossReferences(ctx, doer, true) + }) } // DeleteComment deletes the comment @@ -1277,31 +1257,28 @@ func InsertIssueComments(ctx context.Context, comments []*Comment) error { return comment.IssueID, true }) - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - for _, comment := range comments { - if _, err := db.GetEngine(ctx).NoAutoTime().Insert(comment); err != nil { - return err + return db.WithTx(ctx, func(ctx context.Context) error { + for _, comment := range comments { + if _, err := db.GetEngine(ctx).NoAutoTime().Insert(comment); err != nil { + return err + } + + for _, reaction := range comment.Reactions { + reaction.IssueID = comment.IssueID + reaction.CommentID = comment.ID + } + if len(comment.Reactions) > 0 { + if err := db.Insert(ctx, comment.Reactions); err != nil { + return err + } + } } - for _, reaction := range comment.Reactions { - reaction.IssueID = comment.IssueID - reaction.CommentID = comment.ID - } - if len(comment.Reactions) > 0 { - if err := db.Insert(ctx, comment.Reactions); err != nil { + for _, issueID := range issueIDs { + if err := UpdateIssueNumComments(ctx, issueID); err != nil { return err } } - } - - for _, issueID := range issueIDs { - if err := UpdateIssueNumComments(ctx, issueID); err != nil { - return err - } - } - return committer.Commit() + return nil + }) } diff --git a/models/issues/dependency.go b/models/issues/dependency.go index 146dd1887d..0eaa47e359 100644 --- a/models/issues/dependency.go +++ b/models/issues/dependency.go @@ -128,79 +128,64 @@ const ( // CreateIssueDependency creates a new dependency for an issue func CreateIssueDependency(ctx context.Context, user *user_model.User, issue, dep *Issue) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + // Check if it already exists + exists, err := issueDepExists(ctx, issue.ID, dep.ID) + if err != nil { + return err + } + if exists { + return ErrDependencyExists{issue.ID, dep.ID} + } + // And if it would be circular + circular, err := issueDepExists(ctx, dep.ID, issue.ID) + if err != nil { + return err + } + if circular { + return ErrCircularDependency{issue.ID, dep.ID} + } - // Check if it already exists - exists, err := issueDepExists(ctx, issue.ID, dep.ID) - if err != nil { - return err - } - if exists { - return ErrDependencyExists{issue.ID, dep.ID} - } - // And if it would be circular - circular, err := issueDepExists(ctx, dep.ID, issue.ID) - if err != nil { - return err - } - if circular { - return ErrCircularDependency{issue.ID, dep.ID} - } + if err := db.Insert(ctx, &IssueDependency{ + UserID: user.ID, + IssueID: issue.ID, + DependencyID: dep.ID, + }); err != nil { + return err + } - if err := db.Insert(ctx, &IssueDependency{ - UserID: user.ID, - IssueID: issue.ID, - DependencyID: dep.ID, - }); err != nil { - return err - } - - // Add comment referencing the new dependency - if err = createIssueDependencyComment(ctx, user, issue, dep, true); err != nil { - return err - } - - return committer.Commit() + // Add comment referencing the new dependency + return createIssueDependencyComment(ctx, user, issue, dep, true) + }) } // RemoveIssueDependency removes a dependency from an issue func RemoveIssueDependency(ctx context.Context, user *user_model.User, issue, dep *Issue, depType DependencyType) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + var issueDepToDelete IssueDependency - var issueDepToDelete IssueDependency + switch depType { + case DependencyTypeBlockedBy: + issueDepToDelete = IssueDependency{IssueID: issue.ID, DependencyID: dep.ID} + case DependencyTypeBlocking: + issueDepToDelete = IssueDependency{IssueID: dep.ID, DependencyID: issue.ID} + default: + return ErrUnknownDependencyType{depType} + } - switch depType { - case DependencyTypeBlockedBy: - issueDepToDelete = IssueDependency{IssueID: issue.ID, DependencyID: dep.ID} - case DependencyTypeBlocking: - issueDepToDelete = IssueDependency{IssueID: dep.ID, DependencyID: issue.ID} - default: - return ErrUnknownDependencyType{depType} - } + affected, err := db.GetEngine(ctx).Delete(&issueDepToDelete) + if err != nil { + return err + } - affected, err := db.GetEngine(ctx).Delete(&issueDepToDelete) - if err != nil { - return err - } + // If we deleted nothing, the dependency did not exist + if affected <= 0 { + return ErrDependencyNotExists{issue.ID, dep.ID} + } - // If we deleted nothing, the dependency did not exist - if affected <= 0 { - return ErrDependencyNotExists{issue.ID, dep.ID} - } - - // Add comment referencing the removed dependency - if err = createIssueDependencyComment(ctx, user, issue, dep, false); err != nil { - return err - } - return committer.Commit() + // Add comment referencing the removed dependency + return createIssueDependencyComment(ctx, user, issue, dep, false) + }) } // Check if the dependency already exists diff --git a/models/issues/issue.go b/models/issues/issue.go index a86d50ca9d..ef651359ab 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -755,18 +755,14 @@ func (issue *Issue) HasOriginalAuthor() bool { // InsertIssues insert issues to database func InsertIssues(ctx context.Context, issues ...*Issue) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - for _, issue := range issues { - if err := insertIssue(ctx, issue); err != nil { - return err + return db.WithTx(ctx, func(ctx context.Context) error { + for _, issue := range issues { + if err := insertIssue(ctx, issue); err != nil { + return err + } } - } - return committer.Commit() + return nil + }) } func insertIssue(ctx context.Context, issue *Issue) error { diff --git a/models/issues/issue_index.go b/models/issues/issue_index.go index 2eb61858bf..1fe4a08a09 100644 --- a/models/issues/issue_index.go +++ b/models/issues/issue_index.go @@ -12,20 +12,12 @@ import ( // RecalculateIssueIndexForRepo create issue_index for repo if not exist and // update it based on highest index of existing issues assigned to a repo func RecalculateIssueIndexForRepo(ctx context.Context, repoID int64) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + var maxIndex int64 + if _, err := db.GetEngine(ctx).Select(" MAX(`index`)").Table("issue").Where("repo_id=?", repoID).Get(&maxIndex); err != nil { + return err + } - var maxIndex int64 - if _, err = db.GetEngine(ctx).Select(" MAX(`index`)").Table("issue").Where("repo_id=?", repoID).Get(&maxIndex); err != nil { - return err - } - - if err = db.SyncMaxResourceIndex(ctx, "issue_index", repoID, maxIndex); err != nil { - return err - } - - return committer.Commit() + return db.SyncMaxResourceIndex(ctx, "issue_index", repoID, maxIndex) + }) } diff --git a/models/issues/issue_label.go b/models/issues/issue_label.go index f082079e07..151469a9b8 100644 --- a/models/issues/issue_label.go +++ b/models/issues/issue_label.go @@ -88,36 +88,28 @@ func NewIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *user_m return nil } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if err = issue.LoadRepo(ctx); err != nil { + return err + } - if err = issue.LoadRepo(ctx); err != nil { - return err - } + // Do NOT add invalid labels + if issue.RepoID != label.RepoID && issue.Repo.OwnerID != label.OrgID { + return nil + } - // Do NOT add invalid labels - if issue.RepoID != label.RepoID && issue.Repo.OwnerID != label.OrgID { - return nil - } + if err = RemoveDuplicateExclusiveIssueLabels(ctx, issue, label, doer); err != nil { + return nil + } - if err = RemoveDuplicateExclusiveIssueLabels(ctx, issue, label, doer); err != nil { - return nil - } + if err = newIssueLabel(ctx, issue, label, doer); err != nil { + return err + } - if err = newIssueLabel(ctx, issue, label, doer); err != nil { - return err - } - - issue.isLabelsLoaded = false - issue.Labels = nil - if err = issue.LoadLabels(ctx); err != nil { - return err - } - - return committer.Commit() + issue.isLabelsLoaded = false + issue.Labels = nil + return issue.LoadLabels(ctx) + }) } // newIssueLabels add labels to an issue. It will check if the labels are valid for the issue @@ -151,24 +143,16 @@ func newIssueLabels(ctx context.Context, issue *Issue, labels []*Label, doer *us // NewIssueLabels creates a list of issue-label relations. func NewIssueLabels(ctx context.Context, issue *Issue, labels []*Label, doer *user_model.User) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if err = newIssueLabels(ctx, issue, labels, doer); err != nil { + return err + } - if err = newIssueLabels(ctx, issue, labels, doer); err != nil { - return err - } - - // reload all labels - issue.isLabelsLoaded = false - issue.Labels = nil - if err = issue.LoadLabels(ctx); err != nil { - return err - } - - return committer.Commit() + // reload all labels + issue.isLabelsLoaded = false + issue.Labels = nil + return issue.LoadLabels(ctx) + }) } func deleteIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *user_model.User) (err error) { @@ -365,35 +349,23 @@ func clearIssueLabels(ctx context.Context, issue *Issue, doer *user_model.User) // ClearIssueLabels removes all issue labels as the given user. // Triggers appropriate WebHooks, if any. func ClearIssueLabels(ctx context.Context, issue *Issue, doer *user_model.User) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if err := issue.LoadRepo(ctx); err != nil { + return err + } else if err = issue.LoadPullRequest(ctx); err != nil { + return err + } - if err := issue.LoadRepo(ctx); err != nil { - return err - } else if err = issue.LoadPullRequest(ctx); err != nil { - return err - } + perm, err := access_model.GetUserRepoPermission(ctx, issue.Repo, doer) + if err != nil { + return err + } + if !perm.CanWriteIssuesOrPulls(issue.IsPull) { + return ErrRepoLabelNotExist{} + } - perm, err := access_model.GetUserRepoPermission(ctx, issue.Repo, doer) - if err != nil { - return err - } - if !perm.CanWriteIssuesOrPulls(issue.IsPull) { - return ErrRepoLabelNotExist{} - } - - if err = clearIssueLabels(ctx, issue, doer); err != nil { - return err - } - - if err = committer.Commit(); err != nil { - return fmt.Errorf("Commit: %w", err) - } - - return nil + return clearIssueLabels(ctx, issue, doer) + }) } type labelSorter []*Label @@ -438,69 +410,61 @@ func RemoveDuplicateExclusiveLabels(labels []*Label) []*Label { // ReplaceIssueLabels removes all current labels and add new labels to the issue. // Triggers appropriate WebHooks, if any. func ReplaceIssueLabels(ctx context.Context, issue *Issue, labels []*Label, doer *user_model.User) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if err = issue.LoadRepo(ctx); err != nil { + return err + } - if err = issue.LoadRepo(ctx); err != nil { - return err - } + if err = issue.LoadLabels(ctx); err != nil { + return err + } - if err = issue.LoadLabels(ctx); err != nil { - return err - } + labels = RemoveDuplicateExclusiveLabels(labels) - labels = RemoveDuplicateExclusiveLabels(labels) + sort.Sort(labelSorter(labels)) + sort.Sort(labelSorter(issue.Labels)) - sort.Sort(labelSorter(labels)) - sort.Sort(labelSorter(issue.Labels)) + var toAdd, toRemove []*Label - var toAdd, toRemove []*Label + addIndex, removeIndex := 0, 0 + for addIndex < len(labels) && removeIndex < len(issue.Labels) { + addLabel := labels[addIndex] + removeLabel := issue.Labels[removeIndex] + if addLabel.ID == removeLabel.ID { + // Silently drop invalid labels + if removeLabel.RepoID != issue.RepoID && removeLabel.OrgID != issue.Repo.OwnerID { + toRemove = append(toRemove, removeLabel) + } - addIndex, removeIndex := 0, 0 - for addIndex < len(labels) && removeIndex < len(issue.Labels) { - addLabel := labels[addIndex] - removeLabel := issue.Labels[removeIndex] - if addLabel.ID == removeLabel.ID { - // Silently drop invalid labels - if removeLabel.RepoID != issue.RepoID && removeLabel.OrgID != issue.Repo.OwnerID { + addIndex++ + removeIndex++ + } else if addLabel.ID < removeLabel.ID { + // Only add if the label is valid + if addLabel.RepoID == issue.RepoID || addLabel.OrgID == issue.Repo.OwnerID { + toAdd = append(toAdd, addLabel) + } + addIndex++ + } else { toRemove = append(toRemove, removeLabel) + removeIndex++ } + } + toAdd = append(toAdd, labels[addIndex:]...) + toRemove = append(toRemove, issue.Labels[removeIndex:]...) - addIndex++ - removeIndex++ - } else if addLabel.ID < removeLabel.ID { - // Only add if the label is valid - if addLabel.RepoID == issue.RepoID || addLabel.OrgID == issue.Repo.OwnerID { - toAdd = append(toAdd, addLabel) + if len(toAdd) > 0 { + if err = newIssueLabels(ctx, issue, toAdd, doer); err != nil { + return fmt.Errorf("addLabels: %w", err) } - addIndex++ - } else { - toRemove = append(toRemove, removeLabel) - removeIndex++ } - } - toAdd = append(toAdd, labels[addIndex:]...) - toRemove = append(toRemove, issue.Labels[removeIndex:]...) - if len(toAdd) > 0 { - if err = newIssueLabels(ctx, issue, toAdd, doer); err != nil { - return fmt.Errorf("addLabels: %w", err) + for _, l := range toRemove { + if err = deleteIssueLabel(ctx, issue, l, doer); err != nil { + return fmt.Errorf("removeLabel: %w", err) + } } - } - for _, l := range toRemove { - if err = deleteIssueLabel(ctx, issue, l, doer); err != nil { - return fmt.Errorf("removeLabel: %w", err) - } - } - - issue.Labels = nil - if err = issue.LoadLabels(ctx); err != nil { - return err - } - - return committer.Commit() + issue.Labels = nil + return issue.LoadLabels(ctx) + }) } diff --git a/models/issues/issue_lock.go b/models/issues/issue_lock.go index fa0d128f74..2e5bf64cc6 100644 --- a/models/issues/issue_lock.go +++ b/models/issues/issue_lock.go @@ -47,26 +47,19 @@ func updateIssueLock(ctx context.Context, opts *IssueLockOptions, lock bool) err commentType = CommentTypeUnlock } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if err := UpdateIssueCols(ctx, opts.Issue, "is_locked"); err != nil { + return err + } - if err := UpdateIssueCols(ctx, opts.Issue, "is_locked"); err != nil { + opt := &CreateCommentOptions{ + Doer: opts.Doer, + Issue: opts.Issue, + Repo: opts.Issue.Repo, + Type: commentType, + Content: opts.Reason, + } + _, err := CreateComment(ctx, opt) return err - } - - opt := &CreateCommentOptions{ - Doer: opts.Doer, - Issue: opts.Issue, - Repo: opts.Issue.Repo, - Type: commentType, - Content: opts.Reason, - } - if _, err := CreateComment(ctx, opt); err != nil { - return err - } - - return committer.Commit() + }) } diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 9b99787e3b..1c16817491 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -167,20 +167,9 @@ func CloseIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comm return nil, err } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - comment, err := SetIssueAsClosed(ctx, issue, doer, false) - if err != nil { - return nil, err - } - if err := committer.Commit(); err != nil { - return nil, err - } - return comment, nil + return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { + return SetIssueAsClosed(ctx, issue, doer, false) + }) } // ReopenIssue changes issue status to open. @@ -192,88 +181,64 @@ func ReopenIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Com return nil, err } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - comment, err := setIssueAsReopen(ctx, issue, doer) - if err != nil { - return nil, err - } - if err := committer.Commit(); err != nil { - return nil, err - } - return comment, nil + return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { + return setIssueAsReopen(ctx, issue, doer) + }) } // ChangeIssueTitle changes the title of this issue, as the given user. func ChangeIssueTitle(ctx context.Context, issue *Issue, doer *user_model.User, oldTitle string) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + issue.Title = util.EllipsisDisplayString(issue.Title, 255) + if err = UpdateIssueCols(ctx, issue, "name"); err != nil { + return fmt.Errorf("updateIssueCols: %w", err) + } - issue.Title = util.EllipsisDisplayString(issue.Title, 255) - if err = UpdateIssueCols(ctx, issue, "name"); err != nil { - return fmt.Errorf("updateIssueCols: %w", err) - } + if err = issue.LoadRepo(ctx); err != nil { + return fmt.Errorf("loadRepo: %w", err) + } - if err = issue.LoadRepo(ctx); err != nil { - return fmt.Errorf("loadRepo: %w", err) - } - - opts := &CreateCommentOptions{ - Type: CommentTypeChangeTitle, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - OldTitle: oldTitle, - NewTitle: issue.Title, - } - if _, err = CreateComment(ctx, opts); err != nil { - return fmt.Errorf("createComment: %w", err) - } - if err = issue.AddCrossReferences(ctx, doer, true); err != nil { - return err - } - - return committer.Commit() + opts := &CreateCommentOptions{ + Type: CommentTypeChangeTitle, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + OldTitle: oldTitle, + NewTitle: issue.Title, + } + if _, err = CreateComment(ctx, opts); err != nil { + return fmt.Errorf("createComment: %w", err) + } + return issue.AddCrossReferences(ctx, doer, true) + }) } // ChangeIssueRef changes the branch of this issue, as the given user. func ChangeIssueRef(ctx context.Context, issue *Issue, doer *user_model.User, oldRef string) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if err = UpdateIssueCols(ctx, issue, "ref"); err != nil { + return fmt.Errorf("updateIssueCols: %w", err) + } - if err = UpdateIssueCols(ctx, issue, "ref"); err != nil { - return fmt.Errorf("updateIssueCols: %w", err) - } + if err = issue.LoadRepo(ctx); err != nil { + return fmt.Errorf("loadRepo: %w", err) + } + oldRefFriendly := strings.TrimPrefix(oldRef, git.BranchPrefix) + newRefFriendly := strings.TrimPrefix(issue.Ref, git.BranchPrefix) - if err = issue.LoadRepo(ctx); err != nil { - return fmt.Errorf("loadRepo: %w", err) - } - oldRefFriendly := strings.TrimPrefix(oldRef, git.BranchPrefix) - newRefFriendly := strings.TrimPrefix(issue.Ref, git.BranchPrefix) - - opts := &CreateCommentOptions{ - Type: CommentTypeChangeIssueRef, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - OldRef: oldRefFriendly, - NewRef: newRefFriendly, - } - if _, err = CreateComment(ctx, opts); err != nil { - return fmt.Errorf("createComment: %w", err) - } - - return committer.Commit() + opts := &CreateCommentOptions{ + Type: CommentTypeChangeIssueRef, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + OldRef: oldRefFriendly, + NewRef: newRefFriendly, + } + if _, err = CreateComment(ctx, opts); err != nil { + return fmt.Errorf("createComment: %w", err) + } + return nil + }) } // AddDeletePRBranchComment adds delete branch comment for pull request issue @@ -295,64 +260,56 @@ func AddDeletePRBranchComment(ctx context.Context, doer *user_model.User, repo * // UpdateIssueAttachments update attachments by UUIDs for the issue func UpdateIssueAttachments(ctx context.Context, issueID int64, uuids []string) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids) - if err != nil { - return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) - } - for i := range attachments { - attachments[i].IssueID = issueID - if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { - return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) + return db.WithTx(ctx, func(ctx context.Context) error { + attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids) + if err != nil { + return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) } - } - return committer.Commit() + for i := range attachments { + attachments[i].IssueID = issueID + if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { + return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err) + } + } + return nil + }) } // ChangeIssueContent changes issue content, as the given user. func ChangeIssueContent(ctx context.Context, issue *Issue, doer *user_model.User, content string, contentVersion int) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + hasContentHistory, err := HasIssueContentHistory(ctx, issue.ID, 0) + if err != nil { + return fmt.Errorf("HasIssueContentHistory: %w", err) + } + if !hasContentHistory { + if err = SaveIssueContentHistory(ctx, issue.PosterID, issue.ID, 0, + issue.CreatedUnix, issue.Content, true); err != nil { + return fmt.Errorf("SaveIssueContentHistory: %w", err) + } + } - hasContentHistory, err := HasIssueContentHistory(ctx, issue.ID, 0) - if err != nil { - return fmt.Errorf("HasIssueContentHistory: %w", err) - } - if !hasContentHistory { - if err = SaveIssueContentHistory(ctx, issue.PosterID, issue.ID, 0, - issue.CreatedUnix, issue.Content, true); err != nil { + issue.Content = content + issue.ContentVersion = contentVersion + 1 + + affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content", "content_version").Where("content_version = ?", contentVersion).Update(issue) + if err != nil { + return err + } + if affected == 0 { + return ErrIssueAlreadyChanged + } + + if err = SaveIssueContentHistory(ctx, doer.ID, issue.ID, 0, + timeutil.TimeStampNow(), issue.Content, false); err != nil { return fmt.Errorf("SaveIssueContentHistory: %w", err) } - } - issue.Content = content - issue.ContentVersion = contentVersion + 1 - - affected, err := db.GetEngine(ctx).ID(issue.ID).Cols("content", "content_version").Where("content_version = ?", contentVersion).Update(issue) - if err != nil { - return err - } - if affected == 0 { - return ErrIssueAlreadyChanged - } - - if err = SaveIssueContentHistory(ctx, doer.ID, issue.ID, 0, - timeutil.TimeStampNow(), issue.Content, false); err != nil { - return fmt.Errorf("SaveIssueContentHistory: %w", err) - } - - if err = issue.AddCrossReferences(ctx, doer, true); err != nil { - return fmt.Errorf("addCrossReferences: %w", err) - } - - return committer.Commit() + if err = issue.AddCrossReferences(ctx, doer, true); err != nil { + return fmt.Errorf("addCrossReferences: %w", err) + } + return nil + }) } // NewIssueOptions represents the options of a new issue. @@ -512,23 +469,19 @@ func UpdateIssueDeadline(ctx context.Context, issue *Issue, deadlineUnix timeuti if issue.DeadlineUnix == deadlineUnix { return nil } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - // Update the deadline - if err = UpdateIssueCols(ctx, &Issue{ID: issue.ID, DeadlineUnix: deadlineUnix}, "deadline_unix"); err != nil { - return err - } + return db.WithTx(ctx, func(ctx context.Context) error { + // Update the deadline + if err = UpdateIssueCols(ctx, &Issue{ID: issue.ID, DeadlineUnix: deadlineUnix}, "deadline_unix"); err != nil { + return err + } - // Make the comment - if _, err = createDeadlineComment(ctx, doer, issue, deadlineUnix); err != nil { - return fmt.Errorf("createRemovedDueDateComment: %w", err) - } - - return committer.Commit() + // Make the comment + if _, err = createDeadlineComment(ctx, doer, issue, deadlineUnix); err != nil { + return fmt.Errorf("createRemovedDueDateComment: %w", err) + } + return nil + }) } // FindAndUpdateIssueMentions finds users mentioned in the given content string, and saves them in the database. diff --git a/models/issues/label.go b/models/issues/label.go index cfbe100926..25d6f1303e 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -209,24 +209,20 @@ func NewLabel(ctx context.Context, l *Label) error { // NewLabels creates new labels func NewLabels(ctx context.Context, labels ...*Label) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + for _, l := range labels { + color, err := label.NormalizeColor(l.Color) + if err != nil { + return err + } + l.Color = color - for _, l := range labels { - color, err := label.NormalizeColor(l.Color) - if err != nil { - return err + if err := db.Insert(ctx, l); err != nil { + return err + } } - l.Color = color - - if err := db.Insert(ctx, l); err != nil { - return err - } - } - return committer.Commit() + return nil + }) } // UpdateLabel updates label information. @@ -250,35 +246,26 @@ func DeleteLabel(ctx context.Context, id, labelID int64) error { return err } - ctx, committer, err := db.TxContext(ctx) - if err != nil { + return db.WithTx(ctx, func(ctx context.Context) error { + if l.BelongsToOrg() && l.OrgID != id { + return nil + } + if l.BelongsToRepo() && l.RepoID != id { + return nil + } + + if _, err = db.DeleteByID[Label](ctx, labelID); err != nil { + return err + } else if _, err = db.GetEngine(ctx). + Where("label_id = ?", labelID). + Delete(new(IssueLabel)); err != nil { + return err + } + + // delete comments about now deleted label_id + _, err = db.GetEngine(ctx).Where("label_id = ?", labelID).Cols("label_id").Delete(&Comment{}) return err - } - defer committer.Close() - - sess := db.GetEngine(ctx) - - if l.BelongsToOrg() && l.OrgID != id { - return nil - } - if l.BelongsToRepo() && l.RepoID != id { - return nil - } - - if _, err = db.DeleteByID[Label](ctx, labelID); err != nil { - return err - } else if _, err = sess. - Where("label_id = ?", labelID). - Delete(new(IssueLabel)); err != nil { - return err - } - - // delete comments about now deleted label_id - if _, err = sess.Where("label_id = ?", labelID).Cols("label_id").Delete(&Comment{}); err != nil { - return err - } - - return committer.Commit() + }) } // GetLabelByID returns a label by given ID. diff --git a/models/issues/milestone.go b/models/issues/milestone.go index 4c9bae58f7..373f39f4ff 100644 --- a/models/issues/milestone.go +++ b/models/issues/milestone.go @@ -105,22 +105,16 @@ func (m *Milestone) State() api.StateType { // NewMilestone creates new milestone of repository. func NewMilestone(ctx context.Context, m *Milestone) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + m.Name = strings.TrimSpace(m.Name) - m.Name = strings.TrimSpace(m.Name) + if err = db.Insert(ctx, m); err != nil { + return err + } - if err = db.Insert(ctx, m); err != nil { + _, err = db.Exec(ctx, "UPDATE `repository` SET num_milestones = num_milestones + 1 WHERE id = ?", m.RepoID) return err - } - - if _, err = db.Exec(ctx, "UPDATE `repository` SET num_milestones = num_milestones + 1 WHERE id = ?", m.RepoID); err != nil { - return err - } - return committer.Commit() + }) } // HasMilestoneByRepoID returns if the milestone exists in the repository. @@ -155,28 +149,23 @@ func GetMilestoneByRepoIDANDName(ctx context.Context, repoID int64, name string) // UpdateMilestone updates information of given milestone. func UpdateMilestone(ctx context.Context, m *Milestone, oldIsClosed bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if m.IsClosed && !oldIsClosed { + m.ClosedDateUnix = timeutil.TimeStampNow() + } - if m.IsClosed && !oldIsClosed { - m.ClosedDateUnix = timeutil.TimeStampNow() - } - - if err := updateMilestone(ctx, m); err != nil { - return err - } - - // if IsClosed changed, update milestone numbers of repository - if oldIsClosed != m.IsClosed { - if err := updateRepoMilestoneNum(ctx, m.RepoID); err != nil { + if err := updateMilestone(ctx, m); err != nil { return err } - } - return committer.Commit() + // if IsClosed changed, update milestone numbers of repository + if oldIsClosed != m.IsClosed { + if err := updateRepoMilestoneNum(ctx, m.RepoID); err != nil { + return err + } + } + return nil + }) } func updateMilestone(ctx context.Context, m *Milestone) error { @@ -213,44 +202,28 @@ func UpdateMilestoneCounters(ctx context.Context, id int64) error { // ChangeMilestoneStatusByRepoIDAndID changes a milestone open/closed status if the milestone ID is in the repo. func ChangeMilestoneStatusByRepoIDAndID(ctx context.Context, repoID, milestoneID int64, isClosed bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + m := &Milestone{ + ID: milestoneID, + RepoID: repoID, + } - m := &Milestone{ - ID: milestoneID, - RepoID: repoID, - } + has, err := db.GetEngine(ctx).ID(milestoneID).Where("repo_id = ?", repoID).Get(m) + if err != nil { + return err + } else if !has { + return ErrMilestoneNotExist{ID: milestoneID, RepoID: repoID} + } - has, err := db.GetEngine(ctx).ID(milestoneID).Where("repo_id = ?", repoID).Get(m) - if err != nil { - return err - } else if !has { - return ErrMilestoneNotExist{ID: milestoneID, RepoID: repoID} - } - - if err := changeMilestoneStatus(ctx, m, isClosed); err != nil { - return err - } - - return committer.Commit() + return changeMilestoneStatus(ctx, m, isClosed) + }) } // ChangeMilestoneStatus changes the milestone open/closed status. func ChangeMilestoneStatus(ctx context.Context, m *Milestone, isClosed bool) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err := changeMilestoneStatus(ctx, m, isClosed); err != nil { - return err - } - - return committer.Commit() + return db.WithTx(ctx, func(ctx context.Context) error { + return changeMilestoneStatus(ctx, m, isClosed) + }) } func changeMilestoneStatus(ctx context.Context, m *Milestone, isClosed bool) error { @@ -284,40 +257,34 @@ func DeleteMilestoneByRepoID(ctx context.Context, repoID, id int64) error { return err } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err = db.DeleteByID[Milestone](ctx, m.ID); err != nil { + return err + } - if _, err = db.DeleteByID[Milestone](ctx, m.ID); err != nil { - return err - } + numMilestones, err := db.Count[Milestone](ctx, FindMilestoneOptions{ + RepoID: repo.ID, + }) + if err != nil { + return err + } + numClosedMilestones, err := db.Count[Milestone](ctx, FindMilestoneOptions{ + RepoID: repo.ID, + IsClosed: optional.Some(true), + }) + if err != nil { + return err + } + repo.NumMilestones = int(numMilestones) + repo.NumClosedMilestones = int(numClosedMilestones) - numMilestones, err := db.Count[Milestone](ctx, FindMilestoneOptions{ - RepoID: repo.ID, + if _, err = db.GetEngine(ctx).ID(repo.ID).Cols("num_milestones, num_closed_milestones").Update(repo); err != nil { + return err + } + + _, err = db.Exec(ctx, "UPDATE `issue` SET milestone_id = 0 WHERE milestone_id = ?", m.ID) + return err }) - if err != nil { - return err - } - numClosedMilestones, err := db.Count[Milestone](ctx, FindMilestoneOptions{ - RepoID: repo.ID, - IsClosed: optional.Some(true), - }) - if err != nil { - return err - } - repo.NumMilestones = int(numMilestones) - repo.NumClosedMilestones = int(numClosedMilestones) - - if _, err = db.GetEngine(ctx).ID(repo.ID).Cols("num_milestones, num_closed_milestones").Update(repo); err != nil { - return err - } - - if _, err = db.Exec(ctx, "UPDATE `issue` SET milestone_id = 0 WHERE milestone_id = ?", m.ID); err != nil { - return err - } - return committer.Commit() } func updateRepoMilestoneNum(ctx context.Context, repoID int64) error { @@ -360,22 +327,15 @@ func InsertMilestones(ctx context.Context, ms ...*Milestone) (err error) { return nil } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) - - // to return the id, so we should not use batch insert - for _, m := range ms { - if _, err = sess.NoAutoTime().Insert(m); err != nil { - return err + return db.WithTx(ctx, func(ctx context.Context) error { + // to return the id, so we should not use batch insert + for _, m := range ms { + if _, err = db.GetEngine(ctx).NoAutoTime().Insert(m); err != nil { + return err + } } - } - if _, err = db.Exec(ctx, "UPDATE `repository` SET num_milestones = num_milestones + ? WHERE id = ?", len(ms), ms[0].RepoID); err != nil { + _, err = db.Exec(ctx, "UPDATE `repository` SET num_milestones = num_milestones + ? WHERE id = ?", len(ms), ms[0].RepoID) return err - } - return committer.Commit() + }) } diff --git a/models/issues/pull.go b/models/issues/pull.go index 4c25b6f0c8..00d7bfe1ca 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -364,17 +364,10 @@ func (pr *PullRequest) GetApprovers(ctx context.Context) string { func (pr *PullRequest) getReviewedByLines(ctx context.Context, writer io.Writer) error { maxReviewers := setting.Repository.PullRequest.DefaultMergeMessageMaxApprovers - if maxReviewers == 0 { return nil } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - // Note: This doesn't page as we only expect a very limited number of reviews reviews, err := FindLatestReviews(ctx, FindReviewOptions{ Types: []ReviewType{ReviewTypeApprove}, @@ -410,7 +403,7 @@ func (pr *PullRequest) getReviewedByLines(ctx context.Context, writer io.Writer) } reviewersWritten++ } - return committer.Commit() + return nil } // GetGitHeadRefName returns git ref for hidden pull request branch @@ -464,45 +457,36 @@ func (pr *PullRequest) IsFromFork() bool { // NewPullRequest creates new pull request with labels for repository. func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids []string, pr *PullRequest) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - idx, err := db.GetNextResourceIndex(ctx, "issue_index", repo.ID) - if err != nil { - return fmt.Errorf("generate pull request index failed: %w", err) - } - - issue.Index = idx - issue.Title = util.EllipsisDisplayString(issue.Title, 255) - - if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{ - Repo: repo, - Issue: issue, - LabelIDs: labelIDs, - Attachments: uuids, - IsPull: true, - }); err != nil { - if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) { - return err + return db.WithTx(ctx, func(ctx context.Context) error { + idx, err := db.GetNextResourceIndex(ctx, "issue_index", repo.ID) + if err != nil { + return fmt.Errorf("generate pull request index failed: %w", err) } - return fmt.Errorf("newIssue: %w", err) - } - pr.Index = issue.Index - pr.BaseRepo = repo - pr.IssueID = issue.ID - if err = db.Insert(ctx, pr); err != nil { - return fmt.Errorf("insert pull repo: %w", err) - } + issue.Index = idx + issue.Title = util.EllipsisDisplayString(issue.Title, 255) - if err = committer.Commit(); err != nil { - return fmt.Errorf("Commit: %w", err) - } + if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{ + Repo: repo, + Issue: issue, + LabelIDs: labelIDs, + Attachments: uuids, + IsPull: true, + }); err != nil { + if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) { + return err + } + return fmt.Errorf("newIssue: %w", err) + } - return nil + pr.Index = issue.Index + pr.BaseRepo = repo + pr.IssueID = issue.ID + if err = db.Insert(ctx, pr); err != nil { + return fmt.Errorf("insert pull repo: %w", err) + } + return nil + }) } // ErrUserMustCollaborator represents an error that the user must be a collaborator to a given repo. @@ -977,22 +961,18 @@ func TokenizeCodeOwnersLine(line string) []string { // InsertPullRequests inserted pull requests func InsertPullRequests(ctx context.Context, prs ...*PullRequest) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) - for _, pr := range prs { - if err := insertIssue(ctx, pr.Issue); err != nil { - return err + return db.WithTx(ctx, func(ctx context.Context) error { + for _, pr := range prs { + if err := insertIssue(ctx, pr.Issue); err != nil { + return err + } + pr.IssueID = pr.Issue.ID + if _, err := db.GetEngine(ctx).NoAutoTime().Insert(pr); err != nil { + return err + } } - pr.IssueID = pr.Issue.ID - if _, err := sess.NoAutoTime().Insert(pr); err != nil { - return err - } - } - return committer.Commit() + return nil + }) } // GetPullRequestByMergedCommit returns a merged pull request by the given commit diff --git a/models/issues/reaction.go b/models/issues/reaction.go index f24001fd23..3b5ad6d7ab 100644 --- a/models/issues/reaction.go +++ b/models/issues/reaction.go @@ -224,21 +224,9 @@ func CreateReaction(ctx context.Context, opts *ReactionOptions) (*Reaction, erro return nil, ErrForbiddenIssueReaction{opts.Type} } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - reaction, err := createReaction(ctx, opts) - if err != nil { - return reaction, err - } - - if err := committer.Commit(); err != nil { - return nil, err - } - return reaction, nil + return db.WithTx2(ctx, func(ctx context.Context) (*Reaction, error) { + return createReaction(ctx, opts) + }) } // DeleteReaction deletes reaction for issue or comment. diff --git a/models/issues/review.go b/models/issues/review.go index 71fdb7456f..b758fa5ffa 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -334,54 +334,51 @@ func IsOfficialReviewerTeam(ctx context.Context, issue *Issue, team *organizatio // CreateReview creates a new review based on opts func CreateReview(ctx context.Context, opts CreateReviewOptions) (*Review, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - sess := db.GetEngine(ctx) + return db.WithTx2(ctx, func(ctx context.Context) (*Review, error) { + sess := db.GetEngine(ctx) - review := &Review{ - Issue: opts.Issue, - IssueID: opts.Issue.ID, - Reviewer: opts.Reviewer, - ReviewerTeam: opts.ReviewerTeam, - Content: opts.Content, - Official: opts.Official, - CommitID: opts.CommitID, - Stale: opts.Stale, - } - - if opts.Reviewer != nil { - review.Type = opts.Type - review.ReviewerID = opts.Reviewer.ID - - reviewCond := builder.Eq{"reviewer_id": opts.Reviewer.ID, "issue_id": opts.Issue.ID} - // make sure user review requests are cleared - if opts.Type != ReviewTypePending { - if _, err := sess.Where(reviewCond.And(builder.Eq{"type": ReviewTypeRequest})).Delete(new(Review)); err != nil { - return nil, err - } + review := &Review{ + Issue: opts.Issue, + IssueID: opts.Issue.ID, + Reviewer: opts.Reviewer, + ReviewerTeam: opts.ReviewerTeam, + Content: opts.Content, + Official: opts.Official, + CommitID: opts.CommitID, + Stale: opts.Stale, } - // make sure if the created review gets dismissed no old review surface - // other types can be ignored, as they don't affect branch protection - if opts.Type == ReviewTypeApprove || opts.Type == ReviewTypeReject { - if _, err := sess.Where(reviewCond.And(builder.In("type", ReviewTypeApprove, ReviewTypeReject))). - Cols("dismissed").Update(&Review{Dismissed: true}); err != nil { - return nil, err - } - } - } else if opts.ReviewerTeam != nil { - review.Type = ReviewTypeRequest - review.ReviewerTeamID = opts.ReviewerTeam.ID - } else { - return nil, errors.New("provide either reviewer or reviewer team") - } - if _, err := sess.Insert(review); err != nil { - return nil, err - } - return review, committer.Commit() + if opts.Reviewer != nil { + review.Type = opts.Type + review.ReviewerID = opts.Reviewer.ID + + reviewCond := builder.Eq{"reviewer_id": opts.Reviewer.ID, "issue_id": opts.Issue.ID} + // make sure user review requests are cleared + if opts.Type != ReviewTypePending { + if _, err := sess.Where(reviewCond.And(builder.Eq{"type": ReviewTypeRequest})).Delete(new(Review)); err != nil { + return nil, err + } + } + // make sure if the created review gets dismissed no old review surface + // other types can be ignored, as they don't affect branch protection + if opts.Type == ReviewTypeApprove || opts.Type == ReviewTypeReject { + if _, err := sess.Where(reviewCond.And(builder.In("type", ReviewTypeApprove, ReviewTypeReject))). + Cols("dismissed").Update(&Review{Dismissed: true}); err != nil { + return nil, err + } + } + } else if opts.ReviewerTeam != nil { + review.Type = ReviewTypeRequest + review.ReviewerTeamID = opts.ReviewerTeam.ID + } else { + return nil, errors.New("provide either reviewer or reviewer team") + } + + if _, err := sess.Insert(review); err != nil { + return nil, err + } + return review, nil + }) } // GetCurrentReview returns the current pending review of reviewer for given issue @@ -605,168 +602,152 @@ func DismissReview(ctx context.Context, review *Review, isDismiss bool) (err err // InsertReviews inserts review and review comments func InsertReviews(ctx context.Context, reviews []*Review) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) + return db.WithTx(ctx, func(ctx context.Context) error { + sess := db.GetEngine(ctx) - for _, review := range reviews { - if _, err := sess.NoAutoTime().Insert(review); err != nil { - return err - } + for _, review := range reviews { + if _, err := sess.NoAutoTime().Insert(review); err != nil { + return err + } - if _, err := sess.NoAutoTime().Insert(&Comment{ - Type: CommentTypeReview, - Content: review.Content, - PosterID: review.ReviewerID, - OriginalAuthor: review.OriginalAuthor, - OriginalAuthorID: review.OriginalAuthorID, - IssueID: review.IssueID, - ReviewID: review.ID, - CreatedUnix: review.CreatedUnix, - UpdatedUnix: review.UpdatedUnix, - }); err != nil { - return err - } + if _, err := sess.NoAutoTime().Insert(&Comment{ + Type: CommentTypeReview, + Content: review.Content, + PosterID: review.ReviewerID, + OriginalAuthor: review.OriginalAuthor, + OriginalAuthorID: review.OriginalAuthorID, + IssueID: review.IssueID, + ReviewID: review.ID, + CreatedUnix: review.CreatedUnix, + UpdatedUnix: review.UpdatedUnix, + }); err != nil { + return err + } - for _, c := range review.Comments { - c.ReviewID = review.ID - } + for _, c := range review.Comments { + c.ReviewID = review.ID + } - if len(review.Comments) > 0 { - if _, err := sess.NoAutoTime().Insert(review.Comments); err != nil { + if len(review.Comments) > 0 { + if _, err := sess.NoAutoTime().Insert(review.Comments); err != nil { + return err + } + } + + if err := UpdateIssueNumComments(ctx, review.IssueID); err != nil { return err } } - - if err := UpdateIssueNumComments(ctx, review.IssueID); err != nil { - return err - } - } - - return committer.Commit() + return nil + }) } // AddReviewRequest add a review request from one reviewer func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_model.User) (*Comment, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - sess := db.GetEngine(ctx) + return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { + sess := db.GetEngine(ctx) - review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID) - if err != nil && !IsErrReviewNotExist(err) { - return nil, err - } - - if review != nil { - // skip it when reviewer has been request to review - if review.Type == ReviewTypeRequest { - return nil, committer.Commit() // still commit the transaction, or committer.Close() will rollback it, even if it's a reused transaction. - } - - if issue.IsClosed { - return nil, ErrReviewRequestOnClosedPR{} - } - - if issue.IsPull { - if err := issue.LoadPullRequest(ctx); err != nil { - return nil, err - } - if issue.PullRequest.HasMerged { - return nil, ErrReviewRequestOnClosedPR{} - } - } - } - - // if the reviewer is an official reviewer, - // remove the official flag in the all previous reviews - official, err := IsOfficialReviewer(ctx, issue, reviewer) - if err != nil { - return nil, err - } else if official { - if _, err := sess.Exec("UPDATE `review` SET official=? WHERE issue_id=? AND reviewer_id=?", false, issue.ID, reviewer.ID); err != nil { + review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID) + if err != nil && !IsErrReviewNotExist(err) { return nil, err } - } - review, err = CreateReview(ctx, CreateReviewOptions{ - Type: ReviewTypeRequest, - Issue: issue, - Reviewer: reviewer, - Official: official, - Stale: false, + if review != nil { + // skip it when reviewer has been request to review + if review.Type == ReviewTypeRequest { + return nil, nil // still commit the transaction, or committer.Close() will rollback it, even if it's a reused transaction. + } + + if issue.IsClosed { + return nil, ErrReviewRequestOnClosedPR{} + } + + if issue.IsPull { + if err := issue.LoadPullRequest(ctx); err != nil { + return nil, err + } + if issue.PullRequest.HasMerged { + return nil, ErrReviewRequestOnClosedPR{} + } + } + } + + // if the reviewer is an official reviewer, + // remove the official flag in the all previous reviews + official, err := IsOfficialReviewer(ctx, issue, reviewer) + if err != nil { + return nil, err + } else if official { + if _, err := sess.Exec("UPDATE `review` SET official=? WHERE issue_id=? AND reviewer_id=?", false, issue.ID, reviewer.ID); err != nil { + return nil, err + } + } + + review, err = CreateReview(ctx, CreateReviewOptions{ + Type: ReviewTypeRequest, + Issue: issue, + Reviewer: reviewer, + Official: official, + Stale: false, + }) + if err != nil { + return nil, err + } + + comment, err := CreateComment(ctx, &CreateCommentOptions{ + Type: CommentTypeReviewRequest, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + RemovedAssignee: false, // Use RemovedAssignee as !isRequest + AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID + ReviewID: review.ID, + }) + if err != nil { + return nil, err + } + + // func caller use the created comment to retrieve created review too. + comment.Review = review + + return comment, nil }) - if err != nil { - return nil, err - } - - comment, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeReviewRequest, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - RemovedAssignee: false, // Use RemovedAssignee as !isRequest - AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID - ReviewID: review.ID, - }) - if err != nil { - return nil, err - } - - // func caller use the created comment to retrieve created review too. - comment.Review = review - - return comment, committer.Commit() } // RemoveReviewRequest remove a review request from one reviewer func RemoveReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_model.User) (*Comment, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID) - if err != nil && !IsErrReviewNotExist(err) { - return nil, err - } - - if review == nil || review.Type != ReviewTypeRequest { - return nil, nil - } - - if _, err = db.DeleteByBean(ctx, review); err != nil { - return nil, err - } - - official, err := IsOfficialReviewer(ctx, issue, reviewer) - if err != nil { - return nil, err - } else if official { - if err := restoreLatestOfficialReview(ctx, issue.ID, reviewer.ID); err != nil { + return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { + review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID) + if err != nil && !IsErrReviewNotExist(err) { return nil, err } - } - comment, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeReviewRequest, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - RemovedAssignee: true, // Use RemovedAssignee as !isRequest - AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID + if review == nil || review.Type != ReviewTypeRequest { + return nil, nil + } + + if _, err = db.DeleteByBean(ctx, review); err != nil { + return nil, err + } + + official, err := IsOfficialReviewer(ctx, issue, reviewer) + if err != nil { + return nil, err + } else if official { + if err := restoreLatestOfficialReview(ctx, issue.ID, reviewer.ID); err != nil { + return nil, err + } + } + + return CreateComment(ctx, &CreateCommentOptions{ + Type: CommentTypeReviewRequest, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + RemovedAssignee: true, // Use RemovedAssignee as !isRequest + AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID + }) }) - if err != nil { - return nil, err - } - - return comment, committer.Commit() } // Recalculate the latest official review for reviewer @@ -787,120 +768,112 @@ func restoreLatestOfficialReview(ctx context.Context, issueID, reviewerID int64) // AddTeamReviewRequest add a review request from one team func AddTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organization.Team, doer *user_model.User) (*Comment, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - review, err := GetTeamReviewerByIssueIDAndTeamID(ctx, issue.ID, reviewer.ID) - if err != nil && !IsErrReviewNotExist(err) { - return nil, err - } - - // This team already has been requested to review - therefore skip this. - if review != nil { - return nil, nil - } - - official, err := IsOfficialReviewerTeam(ctx, issue, reviewer) - if err != nil { - return nil, fmt.Errorf("isOfficialReviewerTeam(): %w", err) - } else if !official { - if official, err = IsOfficialReviewer(ctx, issue, doer); err != nil { - return nil, fmt.Errorf("isOfficialReviewer(): %w", err) - } - } - - if review, err = CreateReview(ctx, CreateReviewOptions{ - Type: ReviewTypeRequest, - Issue: issue, - ReviewerTeam: reviewer, - Official: official, - Stale: false, - }); err != nil { - return nil, err - } - - if official { - if _, err := db.Exec(ctx, "UPDATE `review` SET official=? WHERE issue_id=? AND reviewer_team_id=?", false, issue.ID, reviewer.ID); err != nil { - return nil, err - } - } - - comment, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeReviewRequest, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - RemovedAssignee: false, // Use RemovedAssignee as !isRequest - AssigneeTeamID: reviewer.ID, // Use AssigneeTeamID as reviewer team ID - ReviewID: review.ID, - }) - if err != nil { - return nil, fmt.Errorf("CreateComment(): %w", err) - } - - return comment, committer.Commit() -} - -// RemoveTeamReviewRequest remove a review request from one team -func RemoveTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organization.Team, doer *user_model.User) (*Comment, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - review, err := GetTeamReviewerByIssueIDAndTeamID(ctx, issue.ID, reviewer.ID) - if err != nil && !IsErrReviewNotExist(err) { - return nil, err - } - - if review == nil { - return nil, nil - } - - if _, err = db.DeleteByBean(ctx, review); err != nil { - return nil, err - } - - official, err := IsOfficialReviewerTeam(ctx, issue, reviewer) - if err != nil { - return nil, fmt.Errorf("isOfficialReviewerTeam(): %w", err) - } - - if official { - // recalculate which is the latest official review from that team - review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, -reviewer.ID) + return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { + review, err := GetTeamReviewerByIssueIDAndTeamID(ctx, issue.ID, reviewer.ID) if err != nil && !IsErrReviewNotExist(err) { return nil, err } + // This team already has been requested to review - therefore skip this. if review != nil { - if _, err := db.Exec(ctx, "UPDATE `review` SET official=? WHERE id=?", true, review.ID); err != nil { + return nil, nil + } + + official, err := IsOfficialReviewerTeam(ctx, issue, reviewer) + if err != nil { + return nil, fmt.Errorf("isOfficialReviewerTeam(): %w", err) + } else if !official { + if official, err = IsOfficialReviewer(ctx, issue, doer); err != nil { + return nil, fmt.Errorf("isOfficialReviewer(): %w", err) + } + } + + if review, err = CreateReview(ctx, CreateReviewOptions{ + Type: ReviewTypeRequest, + Issue: issue, + ReviewerTeam: reviewer, + Official: official, + Stale: false, + }); err != nil { + return nil, err + } + + if official { + if _, err := db.Exec(ctx, "UPDATE `review` SET official=? WHERE issue_id=? AND reviewer_team_id=?", false, issue.ID, reviewer.ID); err != nil { return nil, err } } - } - if doer == nil { - return nil, committer.Commit() - } + comment, err := CreateComment(ctx, &CreateCommentOptions{ + Type: CommentTypeReviewRequest, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + RemovedAssignee: false, // Use RemovedAssignee as !isRequest + AssigneeTeamID: reviewer.ID, // Use AssigneeTeamID as reviewer team ID + ReviewID: review.ID, + }) + if err != nil { + return nil, fmt.Errorf("CreateComment(): %w", err) + } - comment, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeReviewRequest, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - RemovedAssignee: true, // Use RemovedAssignee as !isRequest - AssigneeTeamID: reviewer.ID, // Use AssigneeTeamID as reviewer team ID + return comment, nil }) - if err != nil { - return nil, fmt.Errorf("CreateComment(): %w", err) - } +} - return comment, committer.Commit() +// RemoveTeamReviewRequest remove a review request from one team +func RemoveTeamReviewRequest(ctx context.Context, issue *Issue, reviewer *organization.Team, doer *user_model.User) (*Comment, error) { + return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { + review, err := GetTeamReviewerByIssueIDAndTeamID(ctx, issue.ID, reviewer.ID) + if err != nil && !IsErrReviewNotExist(err) { + return nil, err + } + + if review == nil { + return nil, nil + } + + if _, err = db.DeleteByBean(ctx, review); err != nil { + return nil, err + } + + official, err := IsOfficialReviewerTeam(ctx, issue, reviewer) + if err != nil { + return nil, fmt.Errorf("isOfficialReviewerTeam(): %w", err) + } + + if official { + // recalculate which is the latest official review from that team + review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, -reviewer.ID) + if err != nil && !IsErrReviewNotExist(err) { + return nil, err + } + + if review != nil { + if _, err := db.Exec(ctx, "UPDATE `review` SET official=? WHERE id=?", true, review.ID); err != nil { + return nil, err + } + } + } + + if doer == nil { + return nil, nil + } + + comment, err := CreateComment(ctx, &CreateCommentOptions{ + Type: CommentTypeReviewRequest, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + RemovedAssignee: true, // Use RemovedAssignee as !isRequest + AssigneeTeamID: reviewer.ID, // Use AssigneeTeamID as reviewer team ID + }) + if err != nil { + return nil, fmt.Errorf("CreateComment(): %w", err) + } + + return comment, nil + }) } // MarkConversation Add or remove Conversation mark for a code comment @@ -966,61 +939,56 @@ func CanMarkConversation(ctx context.Context, issue *Issue, doer *user_model.Use // DeleteReview delete a review and it's code comments func DeleteReview(ctx context.Context, r *Review) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if r.ID == 0 { + return errors.New("review is not allowed to be 0") + } - if r.ID == 0 { - return errors.New("review is not allowed to be 0") - } + if r.Type == ReviewTypeRequest { + return errors.New("review request can not be deleted using this method") + } - if r.Type == ReviewTypeRequest { - return errors.New("review request can not be deleted using this method") - } + opts := FindCommentsOptions{ + Type: CommentTypeCode, + IssueID: r.IssueID, + ReviewID: r.ID, + } - opts := FindCommentsOptions{ - Type: CommentTypeCode, - IssueID: r.IssueID, - ReviewID: r.ID, - } - - if _, err := db.Delete[Comment](ctx, opts); err != nil { - return err - } - - opts = FindCommentsOptions{ - Type: CommentTypeReview, - IssueID: r.IssueID, - ReviewID: r.ID, - } - - if _, err := db.Delete[Comment](ctx, opts); err != nil { - return err - } - - opts = FindCommentsOptions{ - Type: CommentTypeDismissReview, - IssueID: r.IssueID, - ReviewID: r.ID, - } - - if _, err := db.Delete[Comment](ctx, opts); err != nil { - return err - } - - if _, err := db.DeleteByID[Review](ctx, r.ID); err != nil { - return err - } - - if r.Official { - if err := restoreLatestOfficialReview(ctx, r.IssueID, r.ReviewerID); err != nil { + if _, err := db.Delete[Comment](ctx, opts); err != nil { return err } - } - return committer.Commit() + opts = FindCommentsOptions{ + Type: CommentTypeReview, + IssueID: r.IssueID, + ReviewID: r.ID, + } + + if _, err := db.Delete[Comment](ctx, opts); err != nil { + return err + } + + opts = FindCommentsOptions{ + Type: CommentTypeDismissReview, + IssueID: r.IssueID, + ReviewID: r.ID, + } + + if _, err := db.Delete[Comment](ctx, opts); err != nil { + return err + } + + if _, err := db.DeleteByID[Review](ctx, r.ID); err != nil { + return err + } + + if r.Official { + if err := restoreLatestOfficialReview(ctx, r.IssueID, r.ReviewerID); err != nil { + return err + } + } + return nil + }) } // GetCodeCommentsCount return count of CodeComments a Review has diff --git a/models/issues/tracked_time.go b/models/issues/tracked_time.go index 2afbe272ed..9c11881e44 100644 --- a/models/issues/tracked_time.go +++ b/models/issues/tracked_time.go @@ -168,35 +168,31 @@ func GetTrackedSeconds(ctx context.Context, opts FindTrackedTimesOptions) (track // AddTime will add the given time (in seconds) to the issue func AddTime(ctx context.Context, user *user_model.User, issue *Issue, amount int64, created time.Time) (*TrackedTime, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() + return db.WithTx2(ctx, func(ctx context.Context) (*TrackedTime, error) { + t, err := addTime(ctx, user, issue, amount, created) + if err != nil { + return nil, err + } - t, err := addTime(ctx, user, issue, amount, created) - if err != nil { - return nil, err - } + if err := issue.LoadRepo(ctx); err != nil { + return nil, err + } - if err := issue.LoadRepo(ctx); err != nil { - return nil, err - } + if _, err := CreateComment(ctx, &CreateCommentOptions{ + Issue: issue, + Repo: issue.Repo, + Doer: user, + // Content before v1.21 did store the formatted string instead of seconds, + // so use "|" as delimiter to mark the new format + Content: fmt.Sprintf("|%d", amount), + Type: CommentTypeAddTimeManual, + TimeID: t.ID, + }); err != nil { + return nil, err + } - if _, err := CreateComment(ctx, &CreateCommentOptions{ - Issue: issue, - Repo: issue.Repo, - Doer: user, - // Content before v1.21 did store the formatted string instead of seconds, - // so use "|" as delimiter to mark the new format - Content: fmt.Sprintf("|%d", amount), - Type: CommentTypeAddTimeManual, - TimeID: t.ID, - }); err != nil { - return nil, err - } - - return t, committer.Commit() + return t, nil + }) } func addTime(ctx context.Context, user *user_model.User, issue *Issue, amount int64, created time.Time) (*TrackedTime, error) { @@ -241,72 +237,58 @@ func TotalTimesForEachUser(ctx context.Context, options *FindTrackedTimesOptions // DeleteIssueUserTimes deletes times for issue func DeleteIssueUserTimes(ctx context.Context, issue *Issue, user *user_model.User) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + opts := FindTrackedTimesOptions{ + IssueID: issue.ID, + UserID: user.ID, + } - opts := FindTrackedTimesOptions{ - IssueID: issue.ID, - UserID: user.ID, - } + removedTime, err := deleteTimes(ctx, opts) + if err != nil { + return err + } + if removedTime == 0 { + return db.ErrNotExist{Resource: "tracked_time"} + } - removedTime, err := deleteTimes(ctx, opts) - if err != nil { + if err := issue.LoadRepo(ctx); err != nil { + return err + } + _, err = CreateComment(ctx, &CreateCommentOptions{ + Issue: issue, + Repo: issue.Repo, + Doer: user, + // Content before v1.21 did store the formatted string instead of seconds, + // so use "|" as delimiter to mark the new format + Content: fmt.Sprintf("|%d", removedTime), + Type: CommentTypeDeleteTimeManual, + }) return err - } - if removedTime == 0 { - return db.ErrNotExist{Resource: "tracked_time"} - } - - if err := issue.LoadRepo(ctx); err != nil { - return err - } - if _, err := CreateComment(ctx, &CreateCommentOptions{ - Issue: issue, - Repo: issue.Repo, - Doer: user, - // Content before v1.21 did store the formatted string instead of seconds, - // so use "|" as delimiter to mark the new format - Content: fmt.Sprintf("|%d", removedTime), - Type: CommentTypeDeleteTimeManual, - }); err != nil { - return err - } - - return committer.Commit() + }) } // DeleteTime delete a specific Time func DeleteTime(ctx context.Context, t *TrackedTime) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if err := t.LoadAttributes(ctx); err != nil { + return err + } - if err := t.LoadAttributes(ctx); err != nil { - return err - } + if err := deleteTime(ctx, t); err != nil { + return err + } - if err := deleteTime(ctx, t); err != nil { + _, err := CreateComment(ctx, &CreateCommentOptions{ + Issue: t.Issue, + Repo: t.Issue.Repo, + Doer: t.User, + // Content before v1.21 did store the formatted string instead of seconds, + // so use "|" as delimiter to mark the new format + Content: fmt.Sprintf("|%d", t.Time), + Type: CommentTypeDeleteTimeManual, + }) return err - } - - if _, err := CreateComment(ctx, &CreateCommentOptions{ - Issue: t.Issue, - Repo: t.Issue.Repo, - Doer: t.User, - // Content before v1.21 did store the formatted string instead of seconds, - // so use "|" as delimiter to mark the new format - Content: fmt.Sprintf("|%d", t.Time), - Type: CommentTypeDeleteTimeManual, - }); err != nil { - return err - } - - return committer.Commit() + }) } func deleteTimes(ctx context.Context, opts FindTrackedTimesOptions) (removedTime int64, err error) { diff --git a/models/organization/org.go b/models/organization/org.go index 0f3aef146c..5eba004d69 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -310,74 +310,69 @@ func CreateOrganization(ctx context.Context, org *Organization, owner *user_mode org.NumMembers = 1 org.Type = user_model.UserTypeOrganization - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err = user_model.DeleteUserRedirect(ctx, org.Name); err != nil { - return err - } - - if err = db.Insert(ctx, org); err != nil { - return fmt.Errorf("insert organization: %w", err) - } - if err = user_model.GenerateRandomAvatar(ctx, org.AsUser()); err != nil { - return fmt.Errorf("generate random avatar: %w", err) - } - - // Add initial creator to organization and owner team. - if err = db.Insert(ctx, &OrgUser{ - UID: owner.ID, - OrgID: org.ID, - IsPublic: setting.Service.DefaultOrgMemberVisible, - }); err != nil { - return fmt.Errorf("insert org-user relation: %w", err) - } - - // Create default owner team. - t := &Team{ - OrgID: org.ID, - LowerName: strings.ToLower(OwnerTeamName), - Name: OwnerTeamName, - AccessMode: perm.AccessModeOwner, - NumMembers: 1, - IncludesAllRepositories: true, - CanCreateOrgRepo: true, - } - if err = db.Insert(ctx, t); err != nil { - return fmt.Errorf("insert owner team: %w", err) - } - - // insert units for team - units := make([]TeamUnit, 0, len(unit.AllRepoUnitTypes)) - for _, tp := range unit.AllRepoUnitTypes { - up := perm.AccessModeOwner - if tp == unit.TypeExternalTracker || tp == unit.TypeExternalWiki { - up = perm.AccessModeRead + return db.WithTx(ctx, func(ctx context.Context) error { + if err = user_model.DeleteUserRedirect(ctx, org.Name); err != nil { + return err } - units = append(units, TeamUnit{ - OrgID: org.ID, - TeamID: t.ID, - Type: tp, - AccessMode: up, - }) - } - if err = db.Insert(ctx, &units); err != nil { - return err - } + if err = db.Insert(ctx, org); err != nil { + return fmt.Errorf("insert organization: %w", err) + } + if err = user_model.GenerateRandomAvatar(ctx, org.AsUser()); err != nil { + return fmt.Errorf("generate random avatar: %w", err) + } - if err = db.Insert(ctx, &TeamUser{ - UID: owner.ID, - OrgID: org.ID, - TeamID: t.ID, - }); err != nil { - return fmt.Errorf("insert team-user relation: %w", err) - } + // Add initial creator to organization and owner team. + if err = db.Insert(ctx, &OrgUser{ + UID: owner.ID, + OrgID: org.ID, + IsPublic: setting.Service.DefaultOrgMemberVisible, + }); err != nil { + return fmt.Errorf("insert org-user relation: %w", err) + } - return committer.Commit() + // Create default owner team. + t := &Team{ + OrgID: org.ID, + LowerName: strings.ToLower(OwnerTeamName), + Name: OwnerTeamName, + AccessMode: perm.AccessModeOwner, + NumMembers: 1, + IncludesAllRepositories: true, + CanCreateOrgRepo: true, + } + if err = db.Insert(ctx, t); err != nil { + return fmt.Errorf("insert owner team: %w", err) + } + + // insert units for team + units := make([]TeamUnit, 0, len(unit.AllRepoUnitTypes)) + for _, tp := range unit.AllRepoUnitTypes { + up := perm.AccessModeOwner + if tp == unit.TypeExternalTracker || tp == unit.TypeExternalWiki { + up = perm.AccessModeRead + } + units = append(units, TeamUnit{ + OrgID: org.ID, + TeamID: t.ID, + Type: tp, + AccessMode: up, + }) + } + + if err = db.Insert(ctx, &units); err != nil { + return err + } + + if err = db.Insert(ctx, &TeamUser{ + UID: owner.ID, + OrgID: org.ID, + TeamID: t.ID, + }); err != nil { + return fmt.Errorf("insert team-user relation: %w", err) + } + return nil + }) } // GetOrgByName returns organization by given name. @@ -499,31 +494,26 @@ func AddOrgUser(ctx context.Context, orgID, uid int64) error { return err } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + // check in transaction + isAlreadyMember, err = IsOrganizationMember(ctx, orgID, uid) + if err != nil || isAlreadyMember { + return err + } - // check in transaction - isAlreadyMember, err = IsOrganizationMember(ctx, orgID, uid) - if err != nil || isAlreadyMember { - return err - } + ou := &OrgUser{ + UID: uid, + OrgID: orgID, + IsPublic: setting.Service.DefaultOrgMemberVisible, + } - ou := &OrgUser{ - UID: uid, - OrgID: orgID, - IsPublic: setting.Service.DefaultOrgMemberVisible, - } - - if err := db.Insert(ctx, ou); err != nil { - return err - } else if _, err = db.Exec(ctx, "UPDATE `user` SET num_members = num_members + 1 WHERE id = ?", orgID); err != nil { - return err - } - - return committer.Commit() + if err := db.Insert(ctx, ou); err != nil { + return err + } else if _, err = db.Exec(ctx, "UPDATE `user` SET num_members = num_members + 1 WHERE id = ?", orgID); err != nil { + return err + } + return nil + }) } // GetOrgByID returns the user object by given ID if exists. diff --git a/models/organization/team_unit.go b/models/organization/team_unit.go index 3087b70770..c6ec6b39b2 100644 --- a/models/organization/team_unit.go +++ b/models/organization/team_unit.go @@ -31,21 +31,16 @@ func getUnitsByTeamID(ctx context.Context, teamID int64) (units []*TeamUnit, err // UpdateTeamUnits updates a teams's units func UpdateTeamUnits(ctx context.Context, team *Team, units []TeamUnit) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if _, err = db.GetEngine(ctx).Where("team_id = ?", team.ID).Delete(new(TeamUnit)); err != nil { - return err - } - - if len(units) > 0 { - if err = db.Insert(ctx, units); err != nil { + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err = db.GetEngine(ctx).Where("team_id = ?", team.ID).Delete(new(TeamUnit)); err != nil { return err } - } - return committer.Commit() + if len(units) > 0 { + if err = db.Insert(ctx, units); err != nil { + return err + } + } + return nil + }) } diff --git a/models/project/project.go b/models/project/project.go index f516466854..c003664fa3 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -359,41 +359,25 @@ func updateRepositoryProjectCount(ctx context.Context, repoID int64) error { // ChangeProjectStatusByRepoIDAndID toggles a project between opened and closed func ChangeProjectStatusByRepoIDAndID(ctx context.Context, repoID, projectID int64, isClosed bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + p := new(Project) - p := new(Project) + has, err := db.GetEngine(ctx).ID(projectID).Where("repo_id = ?", repoID).Get(p) + if err != nil { + return err + } else if !has { + return ErrProjectNotExist{ID: projectID, RepoID: repoID} + } - has, err := db.GetEngine(ctx).ID(projectID).Where("repo_id = ?", repoID).Get(p) - if err != nil { - return err - } else if !has { - return ErrProjectNotExist{ID: projectID, RepoID: repoID} - } - - if err := changeProjectStatus(ctx, p, isClosed); err != nil { - return err - } - - return committer.Commit() + return changeProjectStatus(ctx, p, isClosed) + }) } // ChangeProjectStatus toggle a project between opened and closed func ChangeProjectStatus(ctx context.Context, p *Project, isClosed bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err := changeProjectStatus(ctx, p, isClosed); err != nil { - return err - } - - return committer.Commit() + return db.WithTx(ctx, func(ctx context.Context) error { + return changeProjectStatus(ctx, p, isClosed) + }) } func changeProjectStatus(ctx context.Context, p *Project, isClosed bool) error { diff --git a/models/repo.go b/models/repo.go index 9bc67079a9..522debb9fe 100644 --- a/models/repo.go +++ b/models/repo.go @@ -290,19 +290,14 @@ func UpdateRepoStats(ctx context.Context, id int64) error { } func updateUserStarNumbers(ctx context.Context, users []user_model.User) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - for _, user := range users { - if _, err = db.Exec(ctx, "UPDATE `user` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE uid=?) WHERE id=?", user.ID, user.ID); err != nil { - return err + return db.WithTx(ctx, func(ctx context.Context) error { + for _, user := range users { + if _, err := db.Exec(ctx, "UPDATE `user` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE uid=?) WHERE id=?", user.ID, user.ID); err != nil { + return err + } } - } - - return committer.Commit() + return nil + }) } // DoctorUserStarNum recalculate Stars number for all user diff --git a/models/repo/language_stats.go b/models/repo/language_stats.go index 7db8cd4dd2..1cddd25f1d 100644 --- a/models/repo/language_stats.go +++ b/models/repo/language_stats.go @@ -141,101 +141,90 @@ func GetTopLanguageStats(ctx context.Context, repo *Repository, limit int) (Lang // UpdateLanguageStats updates the language statistics for repository func UpdateLanguageStats(ctx context.Context, repo *Repository, commitID string, stats map[string]int64) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) + return db.WithTx(ctx, func(ctx context.Context) error { + sess := db.GetEngine(ctx) - oldstats, err := GetLanguageStats(ctx, repo) - if err != nil { - return err - } - var topLang string - var s int64 - for lang, size := range stats { - if size > s { - s = size - topLang = lang + oldstats, err := GetLanguageStats(ctx, repo) + if err != nil { + return err } - } - - for lang, size := range stats { - upd := false - for _, s := range oldstats { - // Update already existing language - if strings.EqualFold(s.Language, lang) { - s.CommitID = commitID - s.IsPrimary = lang == topLang - s.Size = size - if _, err := sess.ID(s.ID).Cols("`commit_id`", "`size`", "`is_primary`").Update(s); err != nil { - return err - } - upd = true - break + var topLang string + var s int64 + for lang, size := range stats { + if size > s { + s = size + topLang = lang } } - // Insert new language - if !upd { - if err := db.Insert(ctx, &LanguageStat{ - RepoID: repo.ID, - CommitID: commitID, - IsPrimary: lang == topLang, - Language: lang, - Size: size, - }); err != nil { + + for lang, size := range stats { + upd := false + for _, s := range oldstats { + // Update already existing language + if strings.EqualFold(s.Language, lang) { + s.CommitID = commitID + s.IsPrimary = lang == topLang + s.Size = size + if _, err := sess.ID(s.ID).Cols("`commit_id`", "`size`", "`is_primary`").Update(s); err != nil { + return err + } + upd = true + break + } + } + // Insert new language + if !upd { + if err := db.Insert(ctx, &LanguageStat{ + RepoID: repo.ID, + CommitID: commitID, + IsPrimary: lang == topLang, + Language: lang, + Size: size, + }); err != nil { + return err + } + } + } + // Delete old languages + statsToDelete := make([]int64, 0, len(oldstats)) + for _, s := range oldstats { + if s.CommitID != commitID { + statsToDelete = append(statsToDelete, s.ID) + } + } + if len(statsToDelete) > 0 { + if _, err := sess.In("`id`", statsToDelete).Delete(&LanguageStat{}); err != nil { return err } } - } - // Delete old languages - statsToDelete := make([]int64, 0, len(oldstats)) - for _, s := range oldstats { - if s.CommitID != commitID { - statsToDelete = append(statsToDelete, s.ID) - } - } - if len(statsToDelete) > 0 { - if _, err := sess.In("`id`", statsToDelete).Delete(&LanguageStat{}); err != nil { - return err - } - } - // Update indexer status - if err = UpdateIndexerStatus(ctx, repo, RepoIndexerTypeStats, commitID); err != nil { - return err - } - - return committer.Commit() + // Update indexer status + return UpdateIndexerStatus(ctx, repo, RepoIndexerTypeStats, commitID) + }) } // CopyLanguageStat Copy originalRepo language stat information to destRepo (use for forked repo) func CopyLanguageStat(ctx context.Context, originalRepo, destRepo *Repository) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - RepoLang := make(LanguageStatList, 0, 6) - if err := db.GetEngine(ctx).Where("`repo_id` = ?", originalRepo.ID).Desc("`size`").Find(&RepoLang); err != nil { - return err - } - if len(RepoLang) > 0 { - for i := range RepoLang { - RepoLang[i].ID = 0 - RepoLang[i].RepoID = destRepo.ID - RepoLang[i].CreatedUnix = timeutil.TimeStampNow() - } - // update destRepo's indexer status - tmpCommitID := RepoLang[0].CommitID - if err := UpdateIndexerStatus(ctx, destRepo, RepoIndexerTypeStats, tmpCommitID); err != nil { + return db.WithTx(ctx, func(ctx context.Context) error { + RepoLang := make(LanguageStatList, 0, 6) + if err := db.GetEngine(ctx).Where("`repo_id` = ?", originalRepo.ID).Desc("`size`").Find(&RepoLang); err != nil { return err } - if err := db.Insert(ctx, &RepoLang); err != nil { - return err + if len(RepoLang) > 0 { + for i := range RepoLang { + RepoLang[i].ID = 0 + RepoLang[i].RepoID = destRepo.ID + RepoLang[i].CreatedUnix = timeutil.TimeStampNow() + } + // update destRepo's indexer status + tmpCommitID := RepoLang[0].CommitID + if err := UpdateIndexerStatus(ctx, destRepo, RepoIndexerTypeStats, tmpCommitID); err != nil { + return err + } + if err := db.Insert(ctx, &RepoLang); err != nil { + return err + } } - } - return committer.Commit() + return nil + }) } diff --git a/models/repo/release.go b/models/repo/release.go index 59f4caf5aa..0db57503ce 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -472,30 +472,24 @@ func (r *Release) GetExternalID() int64 { return r.OriginalAuthorID } // InsertReleases migrates release func InsertReleases(ctx context.Context, rels ...*Release) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) - - for _, rel := range rels { - if _, err := sess.NoAutoTime().Insert(rel); err != nil { - return err - } - - if len(rel.Attachments) > 0 { - for i := range rel.Attachments { - rel.Attachments[i].ReleaseID = rel.ID - } - - if _, err := sess.NoAutoTime().Insert(rel.Attachments); err != nil { + return db.WithTx(ctx, func(ctx context.Context) error { + for _, rel := range rels { + if _, err := db.GetEngine(ctx).NoAutoTime().Insert(rel); err != nil { return err } - } - } - return committer.Commit() + if len(rel.Attachments) > 0 { + for i := range rel.Attachments { + rel.Attachments[i].ReleaseID = rel.ID + } + + if _, err := db.GetEngine(ctx).NoAutoTime().Insert(rel.Attachments); err != nil { + return err + } + } + } + return nil + }) } func FindTagsByCommitIDs(ctx context.Context, repoID int64, commitIDs ...string) (map[string][]*Release, error) { diff --git a/models/repo/star.go b/models/repo/star.go index 4c66855525..bc865f8373 100644 --- a/models/repo/star.go +++ b/models/repo/star.go @@ -25,48 +25,45 @@ func init() { // StarRepo or unstar repository. func StarRepo(ctx context.Context, doer *user_model.User, repo *Repository, star bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - staring := IsStaring(ctx, doer.ID, repo.ID) + return db.WithTx(ctx, func(ctx context.Context) error { + staring := IsStaring(ctx, doer.ID, repo.ID) - if star { - if user_model.IsUserBlockedBy(ctx, doer, repo.OwnerID) { - return user_model.ErrBlockedUser + if star { + if user_model.IsUserBlockedBy(ctx, doer, repo.OwnerID) { + return user_model.ErrBlockedUser + } + + if staring { + return nil + } + + if err := db.Insert(ctx, &Star{UID: doer.ID, RepoID: repo.ID}); err != nil { + return err + } + if _, err := db.Exec(ctx, "UPDATE `repository` SET num_stars = num_stars + 1 WHERE id = ?", repo.ID); err != nil { + return err + } + if _, err := db.Exec(ctx, "UPDATE `user` SET num_stars = num_stars + 1 WHERE id = ?", doer.ID); err != nil { + return err + } + } else { + if !staring { + return nil + } + + if _, err := db.DeleteByBean(ctx, &Star{UID: doer.ID, RepoID: repo.ID}); err != nil { + return err + } + if _, err := db.Exec(ctx, "UPDATE `repository` SET num_stars = num_stars - 1 WHERE id = ?", repo.ID); err != nil { + return err + } + if _, err := db.Exec(ctx, "UPDATE `user` SET num_stars = num_stars - 1 WHERE id = ?", doer.ID); err != nil { + return err + } } - if staring { - return nil - } - - if err := db.Insert(ctx, &Star{UID: doer.ID, RepoID: repo.ID}); err != nil { - return err - } - if _, err := db.Exec(ctx, "UPDATE `repository` SET num_stars = num_stars + 1 WHERE id = ?", repo.ID); err != nil { - return err - } - if _, err := db.Exec(ctx, "UPDATE `user` SET num_stars = num_stars + 1 WHERE id = ?", doer.ID); err != nil { - return err - } - } else { - if !staring { - return nil - } - - if _, err := db.DeleteByBean(ctx, &Star{UID: doer.ID, RepoID: repo.ID}); err != nil { - return err - } - if _, err := db.Exec(ctx, "UPDATE `repository` SET num_stars = num_stars - 1 WHERE id = ?", repo.ID); err != nil { - return err - } - if _, err := db.Exec(ctx, "UPDATE `user` SET num_stars = num_stars - 1 WHERE id = ?", doer.ID); err != nil { - return err - } - } - - return committer.Commit() + return nil + }) } // IsStaring checks if user has starred given repository. diff --git a/models/repo/topic.go b/models/repo/topic.go index 430a60f603..baeae01efa 100644 --- a/models/repo/topic.go +++ b/models/repo/topic.go @@ -227,32 +227,26 @@ func GetRepoTopicByName(ctx context.Context, repoID int64, topicName string) (*T // AddTopic adds a topic name to a repository (if it does not already have it) func AddTopic(ctx context.Context, repoID int64, topicName string) (*Topic, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - sess := db.GetEngine(ctx) + return db.WithTx2(ctx, func(ctx context.Context) (*Topic, error) { + topic, err := GetRepoTopicByName(ctx, repoID, topicName) + if err != nil { + return nil, err + } + if topic != nil { + // Repo already have topic + return topic, nil + } - topic, err := GetRepoTopicByName(ctx, repoID, topicName) - if err != nil { - return nil, err - } - if topic != nil { - // Repo already have topic + topic, err = addTopicByNameToRepo(ctx, repoID, topicName) + if err != nil { + return nil, err + } + + if err = syncTopicsInRepository(ctx, repoID); err != nil { + return nil, err + } return topic, nil - } - - topic, err = addTopicByNameToRepo(ctx, repoID, topicName) - if err != nil { - return nil, err - } - - if err = syncTopicsInRepository(sess, repoID); err != nil { - return nil, err - } - - return topic, committer.Commit() + }) } // DeleteTopic removes a topic name from a repository (if it has it) @@ -266,14 +260,15 @@ func DeleteTopic(ctx context.Context, repoID int64, topicName string) (*Topic, e return nil, nil } - err = removeTopicFromRepo(ctx, repoID, topic) - if err != nil { - return nil, err - } - - err = syncTopicsInRepository(db.GetEngine(ctx), repoID) - - return topic, err + return db.WithTx2(ctx, func(ctx context.Context) (*Topic, error) { + if err = removeTopicFromRepo(ctx, repoID, topic); err != nil { + return nil, err + } + if err = syncTopicsInRepository(ctx, repoID); err != nil { + return nil, err + } + return topic, nil + }) } // SaveTopics save topics to a repository @@ -285,64 +280,55 @@ func SaveTopics(ctx context.Context, repoID int64, topicNames ...string) error { return err } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) - - var addedTopicNames []string - for _, topicName := range topicNames { - if strings.TrimSpace(topicName) == "" { - continue - } - - var found bool - for _, t := range topics { - if strings.EqualFold(topicName, t.Name) { - found = true - break - } - } - if !found { - addedTopicNames = append(addedTopicNames, topicName) - } - } - - var removeTopics []*Topic - for _, t := range topics { - var found bool + return db.WithTx(ctx, func(ctx context.Context) error { + var addedTopicNames []string for _, topicName := range topicNames { - if strings.EqualFold(topicName, t.Name) { - found = true - break + if strings.TrimSpace(topicName) == "" { + continue + } + + var found bool + for _, t := range topics { + if strings.EqualFold(topicName, t.Name) { + found = true + break + } + } + if !found { + addedTopicNames = append(addedTopicNames, topicName) } } - if !found { - removeTopics = append(removeTopics, t) + + var removeTopics []*Topic + for _, t := range topics { + var found bool + for _, topicName := range topicNames { + if strings.EqualFold(topicName, t.Name) { + found = true + break + } + } + if !found { + removeTopics = append(removeTopics, t) + } } - } - for _, topicName := range addedTopicNames { - _, err := addTopicByNameToRepo(ctx, repoID, topicName) - if err != nil { - return err + for _, topicName := range addedTopicNames { + _, err := addTopicByNameToRepo(ctx, repoID, topicName) + if err != nil { + return err + } } - } - for _, topic := range removeTopics { - err := removeTopicFromRepo(ctx, repoID, topic) - if err != nil { - return err + for _, topic := range removeTopics { + err := removeTopicFromRepo(ctx, repoID, topic) + if err != nil { + return err + } } - } - if err := syncTopicsInRepository(sess, repoID); err != nil { - return err - } - - return committer.Commit() + return syncTopicsInRepository(ctx, repoID) + }) } // GenerateTopics generates topics from a template repository @@ -353,19 +339,19 @@ func GenerateTopics(ctx context.Context, templateRepo, generateRepo *Repository) } } - return syncTopicsInRepository(db.GetEngine(ctx), generateRepo.ID) + return syncTopicsInRepository(ctx, generateRepo.ID) } // syncTopicsInRepository makes sure topics in the topics table are copied into the topics field of the repository -func syncTopicsInRepository(sess db.Engine, repoID int64) error { +func syncTopicsInRepository(ctx context.Context, repoID int64) error { topicNames := make([]string, 0, 25) - if err := sess.Table("topic").Cols("name"). + if err := db.GetEngine(ctx).Table("topic").Cols("name"). Join("INNER", "repo_topic", "repo_topic.topic_id = topic.id"). Where("repo_topic.repo_id = ?", repoID).Asc("topic.name").Find(&topicNames); err != nil { return err } - if _, err := sess.ID(repoID).Cols("topics").Update(&Repository{ + if _, err := db.GetEngine(ctx).ID(repoID).Cols("topics").Update(&Repository{ Topics: topicNames, }); err != nil { return err diff --git a/models/repo/update.go b/models/repo/update.go index 64065f11c4..3228ae11a4 100644 --- a/models/repo/update.go +++ b/models/repo/update.go @@ -19,11 +19,6 @@ func UpdateRepositoryOwnerNames(ctx context.Context, ownerID int64, ownerName st if ownerID == 0 { return nil } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() if _, err := db.GetEngine(ctx).Where("owner_id = ?", ownerID).Cols("owner_name").NoAutoTime().Update(&Repository{ OwnerName: ownerName, @@ -31,7 +26,7 @@ func UpdateRepositoryOwnerNames(ctx context.Context, ownerID int64, ownerName st return err } - return committer.Commit() + return nil } // UpdateRepositoryUpdatedTime updates a repository's updated time diff --git a/models/repo/upload.go b/models/repo/upload.go index 20a8fa26fe..f7d4749842 100644 --- a/models/repo/upload.go +++ b/models/repo/upload.go @@ -117,12 +117,6 @@ func DeleteUploads(ctx context.Context, uploads ...*Upload) (err error) { return nil } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - ids := make([]int64, len(uploads)) for i := range uploads { ids[i] = uploads[i].ID @@ -131,10 +125,6 @@ func DeleteUploads(ctx context.Context, uploads ...*Upload) (err error) { return fmt.Errorf("delete uploads: %w", err) } - if err = committer.Commit(); err != nil { - return err - } - for _, upload := range uploads { localPath := upload.LocalPath() isFile, err := util.IsFile(localPath) diff --git a/models/user/email_address.go b/models/user/email_address.go index 2ba6a56450..cb423945f8 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -256,15 +256,9 @@ func IsEmailUsed(ctx context.Context, email string) (bool, error) { // ActivateEmail activates the email address to given user. func ActivateEmail(ctx context.Context, email *EmailAddress) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - if err := updateActivation(ctx, email, true); err != nil { - return err - } - return committer.Commit() + return db.WithTx(ctx, func(ctx context.Context) error { + return updateActivation(ctx, email, true) + }) } func updateActivation(ctx context.Context, email *EmailAddress, activate bool) error { @@ -305,33 +299,30 @@ func makeEmailPrimaryInternal(ctx context.Context, emailID int64, isActive bool) return ErrUserNotExist{UID: email.UID} } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) + return db.WithTx(ctx, func(ctx context.Context) error { + sess := db.GetEngine(ctx) - // 1. Update user table - user.Email = email.Email - if _, err = sess.ID(user.ID).Cols("email").Update(user); err != nil { - return err - } + // 1. Update user table + user.Email = email.Email + if _, err := sess.ID(user.ID).Cols("email").Update(user); err != nil { + return err + } - // 2. Update old primary email - if _, err = sess.Where("uid=? AND is_primary=?", email.UID, true).Cols("is_primary").Update(&EmailAddress{ - IsPrimary: false, - }); err != nil { - return err - } + // 2. Update old primary email + if _, err := sess.Where("uid=? AND is_primary=?", email.UID, true).Cols("is_primary").Update(&EmailAddress{ + IsPrimary: false, + }); err != nil { + return err + } - // 3. update new primary email - email.IsPrimary = true - if _, err = sess.ID(email.ID).Cols("is_primary").Update(email); err != nil { - return err - } + // 3. update new primary email + email.IsPrimary = true + if _, err := sess.ID(email.ID).Cols("is_primary").Update(email); err != nil { + return err + } - return committer.Commit() + return nil + }) } // ChangeInactivePrimaryEmail replaces the inactive primary email of a given user diff --git a/models/user/follow.go b/models/user/follow.go index cf9672109a..e098caab5b 100644 --- a/models/user/follow.go +++ b/models/user/follow.go @@ -38,24 +38,20 @@ func FollowUser(ctx context.Context, user, follow *User) (err error) { return ErrBlockedUser } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if err = db.Insert(ctx, &Follow{UserID: user.ID, FollowID: follow.ID}); err != nil { + return err + } - if err = db.Insert(ctx, &Follow{UserID: user.ID, FollowID: follow.ID}); err != nil { - return err - } + if _, err = db.Exec(ctx, "UPDATE `user` SET num_followers = num_followers + 1 WHERE id = ?", follow.ID); err != nil { + return err + } - if _, err = db.Exec(ctx, "UPDATE `user` SET num_followers = num_followers + 1 WHERE id = ?", follow.ID); err != nil { - return err - } - - if _, err = db.Exec(ctx, "UPDATE `user` SET num_following = num_following + 1 WHERE id = ?", user.ID); err != nil { - return err - } - return committer.Commit() + if _, err = db.Exec(ctx, "UPDATE `user` SET num_following = num_following + 1 WHERE id = ?", user.ID); err != nil { + return err + } + return nil + }) } // UnfollowUser unmarks someone as another's follower. @@ -64,22 +60,18 @@ func UnfollowUser(ctx context.Context, userID, followID int64) (err error) { return nil } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err = db.DeleteByBean(ctx, &Follow{UserID: userID, FollowID: followID}); err != nil { + return err + } - if _, err = db.DeleteByBean(ctx, &Follow{UserID: userID, FollowID: followID}); err != nil { - return err - } + if _, err = db.Exec(ctx, "UPDATE `user` SET num_followers = num_followers - 1 WHERE id = ?", followID); err != nil { + return err + } - if _, err = db.Exec(ctx, "UPDATE `user` SET num_followers = num_followers - 1 WHERE id = ?", followID); err != nil { - return err - } - - if _, err = db.Exec(ctx, "UPDATE `user` SET num_following = num_following - 1 WHERE id = ?", userID); err != nil { - return err - } - return committer.Commit() + if _, err = db.Exec(ctx, "UPDATE `user` SET num_following = num_following - 1 WHERE id = ?", userID); err != nil { + return err + } + return nil + }) } diff --git a/models/webhook/webhook.go b/models/webhook/webhook.go index b234d9ffee..7d4b2e2237 100644 --- a/models/webhook/webhook.go +++ b/models/webhook/webhook.go @@ -319,21 +319,16 @@ func UpdateWebhookLastStatus(ctx context.Context, w *Webhook) error { // DeleteWebhookByID uses argument bean as query condition, // ID must be specified and do not assign unnecessary fields. func DeleteWebhookByID(ctx context.Context, id int64) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if count, err := db.DeleteByID[Webhook](ctx, id); err != nil { - return err - } else if count == 0 { - return ErrWebhookNotExist{ID: id} - } else if _, err = db.DeleteByBean(ctx, &HookTask{HookID: id}); err != nil { - return err - } - - return committer.Commit() + return db.WithTx(ctx, func(ctx context.Context) error { + if count, err := db.DeleteByID[Webhook](ctx, id); err != nil { + return err + } else if count == 0 { + return ErrWebhookNotExist{ID: id} + } else if _, err = db.DeleteByBean(ctx, &HookTask{HookID: id}); err != nil { + return err + } + return nil + }) } // DeleteWebhookByRepoID deletes webhook of repository by given ID. diff --git a/services/asymkey/deploy_key.go b/services/asymkey/deploy_key.go index 9e5a6d6292..04023f9ffb 100644 --- a/services/asymkey/deploy_key.go +++ b/services/asymkey/deploy_key.go @@ -49,28 +49,21 @@ func deleteDeployKeyFromDB(ctx context.Context, key *asymkey_model.DeployKey) er // DeleteDeployKey deletes deploy key from its repository authorized_keys file if needed. // Permissions check should be done outside. func DeleteDeployKey(ctx context.Context, repo *repo_model.Repository, id int64) error { - dbCtx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - key, err := asymkey_model.GetDeployKeyByID(ctx, id) - if err != nil { - if asymkey_model.IsErrDeployKeyNotExist(err) { - return nil + if err := db.WithTx(ctx, func(ctx context.Context) error { + key, err := asymkey_model.GetDeployKeyByID(ctx, id) + if err != nil { + if asymkey_model.IsErrDeployKeyNotExist(err) { + return nil + } + return fmt.Errorf("GetDeployKeyByID: %w", err) } - return fmt.Errorf("GetDeployKeyByID: %w", err) - } - if key.RepoID != repo.ID { - return fmt.Errorf("deploy key %d does not belong to repository %d", id, repo.ID) - } + if key.RepoID != repo.ID { + return fmt.Errorf("deploy key %d does not belong to repository %d", id, repo.ID) + } - if err := deleteDeployKeyFromDB(dbCtx, key); err != nil { - return err - } - if err := committer.Commit(); err != nil { + return deleteDeployKeyFromDB(ctx, key) + }); err != nil { return err } diff --git a/services/asymkey/ssh_key.go b/services/asymkey/ssh_key.go index da57059d4b..01fa7ff15f 100644 --- a/services/asymkey/ssh_key.go +++ b/services/asymkey/ssh_key.go @@ -27,20 +27,9 @@ func DeletePublicKey(ctx context.Context, doer *user_model.User, id int64) (err } } - dbCtx, committer, err := db.TxContext(ctx) - if err != nil { + if _, err = db.DeleteByID[asymkey_model.PublicKey](ctx, id); err != nil { return err } - defer committer.Close() - - if _, err = db.DeleteByID[asymkey_model.PublicKey](dbCtx, id); err != nil { - return err - } - - if err = committer.Commit(); err != nil { - return err - } - committer.Close() if key.Type == asymkey_model.KeyTypePrincipal { return RewriteAllPrincipalKeys(ctx) diff --git a/services/asymkey/ssh_key_principals.go b/services/asymkey/ssh_key_principals.go index 5ed5cfa782..6493c1cc51 100644 --- a/services/asymkey/ssh_key_principals.go +++ b/services/asymkey/ssh_key_principals.go @@ -14,24 +14,6 @@ import ( // AddPrincipalKey adds new principal to database and authorized_principals file. func AddPrincipalKey(ctx context.Context, ownerID int64, content string, authSourceID int64) (*asymkey_model.PublicKey, error) { - dbCtx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - // Principals cannot be duplicated. - has, err := db.GetEngine(dbCtx). - Where("content = ? AND type = ?", content, asymkey_model.KeyTypePrincipal). - Get(new(asymkey_model.PublicKey)) - if err != nil { - return nil, err - } else if has { - return nil, asymkey_model.ErrKeyAlreadyExist{ - Content: content, - } - } - key := &asymkey_model.PublicKey{ OwnerID: ownerID, Name: content, @@ -40,15 +22,27 @@ func AddPrincipalKey(ctx context.Context, ownerID int64, content string, authSou Type: asymkey_model.KeyTypePrincipal, LoginSourceID: authSourceID, } - if err = db.Insert(dbCtx, key); err != nil { - return nil, fmt.Errorf("addKey: %w", err) - } - if err = committer.Commit(); err != nil { + if err := db.WithTx(ctx, func(ctx context.Context) error { + // Principals cannot be duplicated. + has, err := db.GetEngine(ctx). + Where("content = ? AND type = ?", content, asymkey_model.KeyTypePrincipal). + Get(new(asymkey_model.PublicKey)) + if err != nil { + return err + } else if has { + return asymkey_model.ErrKeyAlreadyExist{ + Content: content, + } + } + + if err = db.Insert(ctx, key); err != nil { + return fmt.Errorf("addKey: %w", err) + } + return nil + }); err != nil { return nil, err } - committer.Close() - return key, RewriteAllPrincipalKeys(ctx) } diff --git a/services/issue/label.go b/services/issue/label.go index 6b8070d8aa..e30983df37 100644 --- a/services/issue/label.go +++ b/services/issue/label.go @@ -46,32 +46,24 @@ func AddLabels(ctx context.Context, issue *issues_model.Issue, doer *user_model. // RemoveLabel removes a label from issue by given ID. func RemoveLabel(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, label *issues_model.Label) error { - dbCtx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err := issue.LoadRepo(dbCtx); err != nil { - return err - } - - perm, err := access_model.GetUserRepoPermission(dbCtx, issue.Repo, doer) - if err != nil { - return err - } - if !perm.CanWriteIssuesOrPulls(issue.IsPull) { - if label.OrgID > 0 { - return issues_model.ErrOrgLabelNotExist{} + if err := db.WithTx(ctx, func(ctx context.Context) error { + if err := issue.LoadRepo(ctx); err != nil { + return err } - return issues_model.ErrRepoLabelNotExist{} - } - if err := issues_model.DeleteIssueLabel(dbCtx, issue, label, doer); err != nil { - return err - } + perm, err := access_model.GetUserRepoPermission(ctx, issue.Repo, doer) + if err != nil { + return err + } + if !perm.CanWriteIssuesOrPulls(issue.IsPull) { + if label.OrgID > 0 { + return issues_model.ErrOrgLabelNotExist{} + } + return issues_model.ErrRepoLabelNotExist{} + } - if err := committer.Commit(); err != nil { + return issues_model.DeleteIssueLabel(ctx, issue, label, doer) + }); err != nil { return err } diff --git a/services/issue/milestone.go b/services/issue/milestone.go index afca70794d..05aefad752 100644 --- a/services/issue/milestone.go +++ b/services/issue/milestone.go @@ -69,21 +69,12 @@ func changeMilestoneAssign(ctx context.Context, doer *user_model.User, issue *is // ChangeMilestoneAssign changes assignment of milestone for issue. func ChangeMilestoneAssign(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, oldMilestoneID int64) (err error) { - dbCtx, committer, err := db.TxContext(ctx) - if err != nil { + if err := db.WithTx(ctx, func(dbCtx context.Context) error { + return changeMilestoneAssign(dbCtx, doer, issue, oldMilestoneID) + }); err != nil { return err } - defer committer.Close() - - if err = changeMilestoneAssign(dbCtx, doer, issue, oldMilestoneID); err != nil { - return err - } - - if err = committer.Commit(); err != nil { - return fmt.Errorf("Commit: %w", err) - } notify_service.IssueChangeMilestone(ctx, doer, issue, oldMilestoneID) - return nil } diff --git a/services/issue/status.go b/services/issue/status.go index f9d7dca841..fa59df93ba 100644 --- a/services/issue/status.go +++ b/services/issue/status.go @@ -15,31 +15,25 @@ import ( // CloseIssue close an issue. func CloseIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string) error { - dbCtx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - comment, err := issues_model.CloseIssue(dbCtx, issue, doer) - if err != nil { - if issues_model.IsErrDependenciesLeft(err) { - if _, err := issues_model.FinishIssueStopwatch(dbCtx, doer, issue); err != nil { - log.Error("Unable to stop stopwatch for issue[%d]#%d: %v", issue.ID, issue.Index, err) + var comment *issues_model.Comment + if err := db.WithTx(ctx, func(ctx context.Context) error { + var err error + comment, err = issues_model.CloseIssue(ctx, issue, doer) + if err != nil { + if issues_model.IsErrDependenciesLeft(err) { + if _, err := issues_model.FinishIssueStopwatch(ctx, doer, issue); err != nil { + log.Error("Unable to stop stopwatch for issue[%d]#%d: %v", issue.ID, issue.Index, err) + } } + return err } - return err - } - if _, err := issues_model.FinishIssueStopwatch(dbCtx, doer, issue); err != nil { + _, err = issues_model.FinishIssueStopwatch(ctx, doer, issue) + return err + }); err != nil { return err } - if err := committer.Commit(); err != nil { - return err - } - committer.Close() - notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, true) return nil diff --git a/services/org/team.go b/services/org/team.go index 6890dafd90..773bd11f49 100644 --- a/services/org/team.go +++ b/services/org/team.go @@ -54,39 +54,33 @@ func NewTeam(ctx context.Context, t *organization.Team) (err error) { return organization.ErrTeamAlreadyExist{OrgID: t.OrgID, Name: t.LowerName} } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err = db.Insert(ctx, t); err != nil { - return err - } - - // insert units for team - if len(t.Units) > 0 { - for _, unit := range t.Units { - unit.TeamID = t.ID - } - if err = db.Insert(ctx, &t.Units); err != nil { + return db.WithTx(ctx, func(ctx context.Context) error { + if err = db.Insert(ctx, t); err != nil { return err } - } - // Add all repositories to the team if it has access to all of them. - if t.IncludesAllRepositories { - err = repo_service.AddAllRepositoriesToTeam(ctx, t) - if err != nil { - return fmt.Errorf("addAllRepositories: %w", err) + // insert units for team + if len(t.Units) > 0 { + for _, unit := range t.Units { + unit.TeamID = t.ID + } + if err = db.Insert(ctx, &t.Units); err != nil { + return err + } } - } - // Update organization number of teams. - if _, err = db.Exec(ctx, "UPDATE `user` SET num_teams=num_teams+1 WHERE id = ?", t.OrgID); err != nil { + // Add all repositories to the team if it has access to all of them. + if t.IncludesAllRepositories { + err = repo_service.AddAllRepositoriesToTeam(ctx, t) + if err != nil { + return fmt.Errorf("addAllRepositories: %w", err) + } + } + + // Update organization number of teams. + _, err = db.Exec(ctx, "UPDATE `user` SET num_teams=num_teams+1 WHERE id = ?", t.OrgID) return err - } - return committer.Commit() + }) } // UpdateTeam updates information of team. @@ -99,128 +93,117 @@ func UpdateTeam(ctx context.Context, t *organization.Team, authChanged, includeA t.Description = t.Description[:255] } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - t.LowerName = strings.ToLower(t.Name) - has, err := db.Exist[organization.Team](ctx, builder.Eq{ - "org_id": t.OrgID, - "lower_name": t.LowerName, - }.And(builder.Neq{"id": t.ID}), - ) - if err != nil { - return err - } else if has { - return organization.ErrTeamAlreadyExist{OrgID: t.OrgID, Name: t.LowerName} - } - - sess := db.GetEngine(ctx) - if _, err = sess.ID(t.ID).Cols("name", "lower_name", "description", - "can_create_org_repo", "authorize", "includes_all_repositories").Update(t); err != nil { - return fmt.Errorf("update: %w", err) - } - - // update units for team - if len(t.Units) > 0 { - for _, unit := range t.Units { - unit.TeamID = t.ID - } - // Delete team-unit. - if _, err := sess. - Where("team_id=?", t.ID). - Delete(new(organization.TeamUnit)); err != nil { - return err - } - if _, err = sess.Cols("org_id", "team_id", "type", "access_mode").Insert(&t.Units); err != nil { - return err - } - } - - // Update access for team members if needed. - if authChanged { - repos, err := repo_model.GetTeamRepositories(ctx, &repo_model.SearchTeamRepoOptions{ - TeamID: t.ID, - }) + return db.WithTx(ctx, func(ctx context.Context) error { + t.LowerName = strings.ToLower(t.Name) + has, err := db.Exist[organization.Team](ctx, builder.Eq{ + "org_id": t.OrgID, + "lower_name": t.LowerName, + }.And(builder.Neq{"id": t.ID}), + ) if err != nil { - return fmt.Errorf("GetTeamRepositories: %w", err) + return err + } else if has { + return organization.ErrTeamAlreadyExist{OrgID: t.OrgID, Name: t.LowerName} } - for _, repo := range repos { - if err = access_model.RecalculateTeamAccesses(ctx, repo, 0); err != nil { - return fmt.Errorf("recalculateTeamAccesses: %w", err) + sess := db.GetEngine(ctx) + if _, err = sess.ID(t.ID).Cols("name", "lower_name", "description", + "can_create_org_repo", "authorize", "includes_all_repositories").Update(t); err != nil { + return fmt.Errorf("update: %w", err) + } + + // update units for team + if len(t.Units) > 0 { + for _, unit := range t.Units { + unit.TeamID = t.ID + } + // Delete team-unit. + if _, err := sess. + Where("team_id=?", t.ID). + Delete(new(organization.TeamUnit)); err != nil { + return err + } + if _, err = sess.Cols("org_id", "team_id", "type", "access_mode").Insert(&t.Units); err != nil { + return err } } - } - // Add all repositories to the team if it has access to all of them. - if includeAllChanged && t.IncludesAllRepositories { - err = repo_service.AddAllRepositoriesToTeam(ctx, t) - if err != nil { - return fmt.Errorf("addAllRepositories: %w", err) + // Update access for team members if needed. + if authChanged { + repos, err := repo_model.GetTeamRepositories(ctx, &repo_model.SearchTeamRepoOptions{ + TeamID: t.ID, + }) + if err != nil { + return fmt.Errorf("GetTeamRepositories: %w", err) + } + + for _, repo := range repos { + if err = access_model.RecalculateTeamAccesses(ctx, repo, 0); err != nil { + return fmt.Errorf("recalculateTeamAccesses: %w", err) + } + } } - } - return committer.Commit() + // Add all repositories to the team if it has access to all of them. + if includeAllChanged && t.IncludesAllRepositories { + err = repo_service.AddAllRepositoriesToTeam(ctx, t) + if err != nil { + return fmt.Errorf("addAllRepositories: %w", err) + } + } + + return nil + }) } // DeleteTeam deletes given team. // It's caller's responsibility to assign organization ID. func DeleteTeam(ctx context.Context, t *organization.Team) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err := t.LoadMembers(ctx); err != nil { - return err - } - - // update branch protections - { - protections := make([]*git_model.ProtectedBranch, 0, 10) - err := db.GetEngine(ctx).In("repo_id", - builder.Select("id").From("repository").Where(builder.Eq{"owner_id": t.OrgID})). - Find(&protections) - if err != nil { - return fmt.Errorf("findProtectedBranches: %w", err) + return db.WithTx(ctx, func(ctx context.Context) error { + if err := t.LoadMembers(ctx); err != nil { + return err } - for _, p := range protections { - if err := git_model.RemoveTeamIDFromProtectedBranch(ctx, p, t.ID); err != nil { + + // update branch protections + { + protections := make([]*git_model.ProtectedBranch, 0, 10) + err := db.GetEngine(ctx).In("repo_id", + builder.Select("id").From("repository").Where(builder.Eq{"owner_id": t.OrgID})). + Find(&protections) + if err != nil { + return fmt.Errorf("findProtectedBranches: %w", err) + } + for _, p := range protections { + if err := git_model.RemoveTeamIDFromProtectedBranch(ctx, p, t.ID); err != nil { + return err + } + } + } + + if err := repo_service.RemoveAllRepositoriesFromTeam(ctx, t); err != nil { + return err + } + + if err := db.DeleteBeans(ctx, + &organization.Team{ID: t.ID}, + &organization.TeamUser{OrgID: t.OrgID, TeamID: t.ID}, + &organization.TeamUnit{TeamID: t.ID}, + &organization.TeamInvite{TeamID: t.ID}, + &issues_model.Review{Type: issues_model.ReviewTypeRequest, ReviewerTeamID: t.ID}, // batch delete the binding relationship between team and PR (request review from team) + ); err != nil { + return err + } + + for _, tm := range t.Members { + if err := removeInvalidOrgUser(ctx, t.OrgID, tm); err != nil { return err } } - } - if err := repo_service.RemoveAllRepositoriesFromTeam(ctx, t); err != nil { + // Update organization number of teams. + _, err := db.Exec(ctx, "UPDATE `user` SET num_teams=num_teams-1 WHERE id=?", t.OrgID) return err - } - - if err := db.DeleteBeans(ctx, - &organization.Team{ID: t.ID}, - &organization.TeamUser{OrgID: t.OrgID, TeamID: t.ID}, - &organization.TeamUnit{TeamID: t.ID}, - &organization.TeamInvite{TeamID: t.ID}, - &issues_model.Review{Type: issues_model.ReviewTypeRequest, ReviewerTeamID: t.ID}, // batch delete the binding relationship between team and PR (request review from team) - ); err != nil { - return err - } - - for _, tm := range t.Members { - if err := removeInvalidOrgUser(ctx, t.OrgID, tm); err != nil { - return err - } - } - - // Update organization number of teams. - if _, err := db.Exec(ctx, "UPDATE `user` SET num_teams=num_teams-1 WHERE id=?", t.OrgID); err != nil { - return err - } - - return committer.Commit() + }) } // AddTeamMember adds new membership of given team to given organization, @@ -363,13 +346,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 { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - if err := removeTeamMember(ctx, team, user); err != nil { - return err - } - return committer.Commit() + return db.WithTx(ctx, func(ctx context.Context) error { + return removeTeamMember(ctx, team, user) + }) } diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index 959babe7cd..ec860db1bb 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -164,42 +164,38 @@ func ExecuteCleanupRules(ctx context.Context) error { }) } -func CleanupExpiredData(outerCtx context.Context, olderThan time.Duration) error { - ctx, committer, err := db.TxContext(outerCtx) - if err != nil { - return err - } - defer committer.Close() - - if err := container_service.Cleanup(ctx, olderThan); err != nil { - return err - } - - ps, err := packages_model.FindUnreferencedPackages(ctx) - if err != nil { - return err - } - for _, p := range ps { - if err := packages_model.DeleteAllProperties(ctx, packages_model.PropertyTypePackage, p.ID); err != nil { +func CleanupExpiredData(ctx context.Context, olderThan time.Duration) error { + pbs := make([]*packages_model.PackageBlob, 0, 100) + if err := db.WithTx(ctx, func(ctx context.Context) error { + if err := container_service.Cleanup(ctx, olderThan); err != nil { return err } - if err := packages_model.DeletePackageByID(ctx, p.ID); err != nil { + + ps, err := packages_model.FindUnreferencedPackages(ctx) + if err != nil { return err } - } + for _, p := range ps { + if err := packages_model.DeleteAllProperties(ctx, packages_model.PropertyTypePackage, p.ID); err != nil { + return err + } + if err := packages_model.DeletePackageByID(ctx, p.ID); err != nil { + return err + } + } - pbs, err := packages_model.FindExpiredUnreferencedBlobs(ctx, olderThan) - if err != nil { - return err - } - - for _, pb := range pbs { - if err := packages_model.DeleteBlobByID(ctx, pb.ID); err != nil { + pbs, err = packages_model.FindExpiredUnreferencedBlobs(ctx, olderThan) + if err != nil { return err } - } - if err := committer.Commit(); err != nil { + for _, pb := range pbs { + if err := packages_model.DeleteBlobByID(ctx, pb.ID); err != nil { + return err + } + } + return nil + }); err != nil { return err } diff --git a/services/packages/packages.go b/services/packages/packages.go index 4b16ee7285..22b26b6563 100644 --- a/services/packages/packages.go +++ b/services/packages/packages.go @@ -469,24 +469,15 @@ func RemovePackageVersionByNameAndVersion(ctx context.Context, doer *user_model. // RemovePackageVersion deletes the package version and all associated files func RemovePackageVersion(ctx context.Context, doer *user_model.User, pv *packages_model.PackageVersion) error { - dbCtx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - pd, err := packages_model.GetPackageDescriptor(dbCtx, pv) + pd, err := packages_model.GetPackageDescriptor(ctx, pv) if err != nil { return err } - log.Trace("Deleting package: %v", pv.ID) - - if err := DeletePackageVersionAndReferences(dbCtx, pv); err != nil { - return err - } - - if err := committer.Commit(); err != nil { + if err := db.WithTx(ctx, func(ctx context.Context) error { + log.Trace("Deleting package: %v", pv.ID) + return DeletePackageVersionAndReferences(ctx, pv) + }); err != nil { return err } diff --git a/services/pull/merge.go b/services/pull/merge.go index cd9aeb2ad1..a941c20435 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -687,48 +687,40 @@ func SetMerged(ctx context.Context, pr *issues_model.PullRequest, mergedCommitID return false, fmt.Errorf("unable to merge PullRequest[%d], some required fields are empty", pr.Index) } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return false, err - } - defer committer.Close() + return db.WithTx2(ctx, func(ctx context.Context) (bool, error) { + pr.Issue = nil + if err := pr.LoadIssue(ctx); err != nil { + return false, err + } - pr.Issue = nil - if err := pr.LoadIssue(ctx); err != nil { - return false, err - } + if err := pr.Issue.LoadRepo(ctx); err != nil { + return false, err + } - if err := pr.Issue.LoadRepo(ctx); err != nil { - return false, err - } + if err := pr.Issue.Repo.LoadOwner(ctx); err != nil { + return false, err + } - if err := pr.Issue.Repo.LoadOwner(ctx); err != nil { - return false, err - } + // Removing an auto merge pull and ignore if not exist + if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) { + return false, fmt.Errorf("DeleteScheduledAutoMerge[%d]: %v", pr.ID, err) + } - // Removing an auto merge pull and ignore if not exist - if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) { - return false, fmt.Errorf("DeleteScheduledAutoMerge[%d]: %v", pr.ID, err) - } + // Set issue as closed + if _, err := issues_model.SetIssueAsClosed(ctx, pr.Issue, pr.Merger, true); err != nil { + return false, fmt.Errorf("ChangeIssueStatus: %w", err) + } - // Set issue as closed - if _, err := issues_model.SetIssueAsClosed(ctx, pr.Issue, pr.Merger, true); err != nil { - return false, fmt.Errorf("ChangeIssueStatus: %w", err) - } + // We need to save all of the data used to compute this merge as it may have already been changed by testPullRequestBranchMergeable. FIXME: need to set some state to prevent testPullRequestBranchMergeable from running whilst we are merging. + if cnt, err := db.GetEngine(ctx).Where("id = ?", pr.ID). + And("has_merged = ?", false). + Cols("has_merged, status, merge_base, merged_commit_id, merger_id, merged_unix, conflicted_files"). + Update(pr); err != nil { + return false, fmt.Errorf("failed to update pr[%d]: %w", pr.ID, err) + } else if cnt != 1 { + return false, issues_model.ErrIssueAlreadyChanged + } - // We need to save all of the data used to compute this merge as it may have already been changed by testPullRequestBranchMergeable. FIXME: need to set some state to prevent testPullRequestBranchMergeable from running whilst we are merging. - if cnt, err := db.GetEngine(ctx).Where("id = ?", pr.ID). - And("has_merged = ?", false). - Cols("has_merged, status, merge_base, merged_commit_id, merger_id, merged_unix, conflicted_files"). - Update(pr); err != nil { - return false, fmt.Errorf("failed to update pr[%d]: %w", pr.ID, err) - } else if cnt != 1 { - return false, issues_model.ErrIssueAlreadyChanged - } - - if err := committer.Commit(); err != nil { - return false, err - } - - return true, nil + return true, nil + }) } diff --git a/services/repository/avatar.go b/services/repository/avatar.go index 26bf6da465..998ac42230 100644 --- a/services/repository/avatar.go +++ b/services/repository/avatar.go @@ -29,35 +29,30 @@ func UploadAvatar(ctx context.Context, repo *repo_model.Repository, data []byte) return nil } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + oldAvatarPath := repo.CustomAvatarRelativePath() - oldAvatarPath := repo.CustomAvatarRelativePath() - - // Users can upload the same image to other repo - prefix it with ID - // Then repo will be removed - only it avatar file will be removed - repo.Avatar = newAvatar - if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "avatar"); err != nil { - return fmt.Errorf("UploadAvatar: Update repository avatar: %w", err) - } - - if err := storage.SaveFrom(storage.RepoAvatars, repo.CustomAvatarRelativePath(), func(w io.Writer) error { - _, err := w.Write(avatarData) - return err - }); err != nil { - return fmt.Errorf("UploadAvatar %s failed: Failed to remove old repo avatar %s: %w", repo.RepoPath(), newAvatar, err) - } - - if len(oldAvatarPath) > 0 { - if err := storage.RepoAvatars.Delete(oldAvatarPath); err != nil { - return fmt.Errorf("UploadAvatar: Failed to remove old repo avatar %s: %w", oldAvatarPath, err) + // Users can upload the same image to other repo - prefix it with ID + // Then repo will be removed - only it avatar file will be removed + repo.Avatar = newAvatar + if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "avatar"); err != nil { + return fmt.Errorf("UploadAvatar: Update repository avatar: %w", err) } - } - return committer.Commit() + if err := storage.SaveFrom(storage.RepoAvatars, repo.CustomAvatarRelativePath(), func(w io.Writer) error { + _, err := w.Write(avatarData) + return err + }); err != nil { + return fmt.Errorf("UploadAvatar %s failed: Failed to remove old repo avatar %s: %w", repo.RepoPath(), newAvatar, err) + } + + if len(oldAvatarPath) > 0 { + if err := storage.RepoAvatars.Delete(oldAvatarPath); err != nil { + return fmt.Errorf("UploadAvatar: Failed to remove old repo avatar %s: %w", oldAvatarPath, err) + } + } + return nil + }) } // DeleteAvatar deletes the repos's custom avatar. @@ -70,22 +65,17 @@ func DeleteAvatar(ctx context.Context, repo *repo_model.Repository) error { avatarPath := repo.CustomAvatarRelativePath() log.Trace("DeleteAvatar[%d]: %s", repo.ID, avatarPath) - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + repo.Avatar = "" + if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "avatar"); err != nil { + return fmt.Errorf("DeleteAvatar: Update repository avatar: %w", err) + } - repo.Avatar = "" - if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "avatar"); err != nil { - return fmt.Errorf("DeleteAvatar: Update repository avatar: %w", err) - } - - if err := storage.RepoAvatars.Delete(avatarPath); err != nil { - return fmt.Errorf("DeleteAvatar: Failed to remove %s: %w", avatarPath, err) - } - - return committer.Commit() + if err := storage.RepoAvatars.Delete(avatarPath); err != nil { + return fmt.Errorf("DeleteAvatar: Failed to remove %s: %w", avatarPath, err) + } + return nil + }) } // RemoveRandomAvatars removes the randomly generated avatars that were created for repositories diff --git a/services/repository/collaboration.go b/services/repository/collaboration.go index b5fc523623..53b3c2e203 100644 --- a/services/repository/collaboration.go +++ b/services/repository/collaboration.go @@ -71,40 +71,32 @@ func DeleteCollaboration(ctx context.Context, repo *repo_model.Repository, colla UserID: collaborator.ID, } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + if has, err := db.GetEngine(ctx).Delete(collaboration); err != nil { + return err + } else if has == 0 { + return nil + } - if has, err := db.GetEngine(ctx).Delete(collaboration); err != nil { - return err - } else if has == 0 { - return committer.Commit() - } + if err := repo.LoadOwner(ctx); err != nil { + return err + } - if err := repo.LoadOwner(ctx); err != nil { - return err - } + if err = access_model.RecalculateAccesses(ctx, repo); err != nil { + return err + } - if err = access_model.RecalculateAccesses(ctx, repo); err != nil { - return err - } + if err = repo_model.WatchRepo(ctx, collaborator, repo, false); err != nil { + return err + } - if err = repo_model.WatchRepo(ctx, collaborator, repo, false); err != nil { - return err - } + if err = ReconsiderWatches(ctx, repo, collaborator); err != nil { + return err + } - if err = ReconsiderWatches(ctx, repo, collaborator); err != nil { - return err - } - - // Unassign a user from any issue (s)he has been assigned to in the repository - if err := ReconsiderRepoIssuesAssignee(ctx, repo, collaborator); err != nil { - return err - } - - return committer.Commit() + // Unassign a user from any issue (s)he has been assigned to in the repository + return ReconsiderRepoIssuesAssignee(ctx, repo, collaborator) + }) } func ReconsiderRepoIssuesAssignee(ctx context.Context, repo *repo_model.Repository, user *user_model.User) error { diff --git a/services/repository/repo_team.go b/services/repository/repo_team.go index 672ee49fea..8ea186f8cc 100644 --- a/services/repository/repo_team.go +++ b/services/repository/repo_team.go @@ -86,17 +86,9 @@ func RemoveAllRepositoriesFromTeam(ctx context.Context, t *organization.Team) (e return nil } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err = removeAllRepositoriesFromTeam(ctx, t); err != nil { - return err - } - - return committer.Commit() + return db.WithTx(ctx, func(ctx context.Context) error { + return removeAllRepositoriesFromTeam(ctx, t) + }) } // removeAllRepositoriesFromTeam removes all repositories from team and recalculates access @@ -167,17 +159,9 @@ func RemoveRepositoryFromTeam(ctx context.Context, t *organization.Team, repoID return err } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err = removeRepositoryFromTeam(ctx, t, repo, true); err != nil { - return err - } - - return committer.Commit() + return db.WithTx(ctx, func(ctx context.Context) error { + return removeRepositoryFromTeam(ctx, t, repo, true) + }) } // removeRepositoryFromTeam removes a repository from a team and recalculates access diff --git a/services/repository/setting.go b/services/repository/setting.go index e0c787dd2d..b6873691eb 100644 --- a/services/repository/setting.go +++ b/services/repository/setting.go @@ -16,41 +16,37 @@ import ( // UpdateRepositoryUnits updates a repository's units func UpdateRepositoryUnits(ctx context.Context, repo *repo_model.Repository, units []repo_model.RepoUnit, deleteUnitTypes []unit.Type) (err error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - // Delete existing settings of units before adding again - for _, u := range units { - deleteUnitTypes = append(deleteUnitTypes, u.Type) - } - - if slices.Contains(deleteUnitTypes, unit.TypeActions) { - if err := actions_service.CleanRepoScheduleTasks(ctx, repo); err != nil { - log.Error("CleanRepoScheduleTasks: %v", err) + return db.WithTx(ctx, func(ctx context.Context) error { + // Delete existing settings of units before adding again + for _, u := range units { + deleteUnitTypes = append(deleteUnitTypes, u.Type) } - } - for _, u := range units { - if u.Type == unit.TypeActions { - if err := actions_service.DetectAndHandleSchedules(ctx, repo); err != nil { - log.Error("DetectAndHandleSchedules: %v", err) + if slices.Contains(deleteUnitTypes, unit.TypeActions) { + if err := actions_service.CleanRepoScheduleTasks(ctx, repo); err != nil { + log.Error("CleanRepoScheduleTasks: %v", err) } - break } - } - if _, err = db.GetEngine(ctx).Where("repo_id = ?", repo.ID).In("type", deleteUnitTypes).Delete(new(repo_model.RepoUnit)); err != nil { - return err - } + for _, u := range units { + if u.Type == unit.TypeActions { + if err := actions_service.DetectAndHandleSchedules(ctx, repo); err != nil { + log.Error("DetectAndHandleSchedules: %v", err) + } + break + } + } - if len(units) > 0 { - if err = db.Insert(ctx, units); err != nil { + if _, err = db.GetEngine(ctx).Where("repo_id = ?", repo.ID).In("type", deleteUnitTypes).Delete(new(repo_model.RepoUnit)); err != nil { return err } - } - return committer.Commit() + if len(units) > 0 { + if err = db.Insert(ctx, units); err != nil { + return err + } + } + + return nil + }) } diff --git a/services/user/avatar.go b/services/user/avatar.go index 3f87466eaa..df188e5adc 100644 --- a/services/user/avatar.go +++ b/services/user/avatar.go @@ -24,26 +24,22 @@ func UploadAvatar(ctx context.Context, u *user_model.User, data []byte) error { return err } - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() + return db.WithTx(ctx, func(ctx context.Context) error { + u.UseCustomAvatar = true + u.Avatar = avatar.HashAvatar(u.ID, data) + if err = user_model.UpdateUserCols(ctx, u, "use_custom_avatar", "avatar"); err != nil { + return fmt.Errorf("updateUser: %w", err) + } - u.UseCustomAvatar = true - u.Avatar = avatar.HashAvatar(u.ID, data) - if err = user_model.UpdateUserCols(ctx, u, "use_custom_avatar", "avatar"); err != nil { - return fmt.Errorf("updateUser: %w", err) - } + if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error { + _, err := w.Write(avatarData) + return err + }); err != nil { + return fmt.Errorf("Failed to create dir %s: %w", u.CustomAvatarRelativePath(), err) + } - if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error { - _, err := w.Write(avatarData) - return err - }); err != nil { - return fmt.Errorf("Failed to create dir %s: %w", u.CustomAvatarRelativePath(), err) - } - - return committer.Commit() + return nil + }) } // DeleteAvatar deletes the user's custom avatar.