diff --git a/models/issues/comment.go b/models/issues/comment.go index e73ac4cf095..1ffe1b650ef 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -1114,8 +1114,7 @@ func UpdateComment(ctx context.Context, c *Comment, contentVersion int, doer *us // DeleteComment deletes the comment func DeleteComment(ctx context.Context, comment *Comment) (*Comment, error) { - e := db.GetEngine(ctx) - if _, err := e.ID(comment.ID).NoAutoCondition().Delete(comment); err != nil { + if _, err := db.GetEngine(ctx).ID(comment.ID).NoAutoCondition().Delete(comment); err != nil { return nil, err } @@ -1130,7 +1129,7 @@ func DeleteComment(ctx context.Context, comment *Comment) (*Comment, error) { return nil, err } } - if _, err := e.Table("action"). + if _, err := db.GetEngine(ctx).Table("action"). Where("comment_id = ?", comment.ID). Update(map[string]any{ "is_deleted": true, diff --git a/services/org/org.go b/services/org/org.go index 3d30ae21a39..13221e28958 100644 --- a/services/org/org.go +++ b/services/org/org.go @@ -48,12 +48,11 @@ func deleteOrganization(ctx context.Context, org *org_model.Organization) error // DeleteOrganization completely and permanently deletes everything of organization. func DeleteOrganization(ctx context.Context, org *org_model.Organization, purge bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - + // The repositories deletion of the organization cannot be under a transaction, + // because it cannot be rolled back because the content in disk will be deleted + // in the DeleteOwnerRepositoriesDirectly function. + // Even not all repositories deleted successfully, we still delete the organization again. + // TODO: We should mark all the repositories as deleted and delete them in a background job. if purge { err := repo_service.DeleteOwnerRepositoriesDirectly(ctx, org.AsUser()) if err != nil { @@ -61,26 +60,28 @@ func DeleteOrganization(ctx context.Context, org *org_model.Organization, purge } } - // Check ownership of repository. - count, err := repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{OwnerID: org.ID}) + err := db.WithTx(ctx, func(ctx context.Context) error { + // Check ownership of repository. + count, err := repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{OwnerID: org.ID}) + if err != nil { + return fmt.Errorf("GetRepositoryCount: %w", err) + } else if count > 0 { + return repo_model.ErrUserOwnRepos{UID: org.ID} + } + + // Check ownership of packages. + if ownsPackages, err := packages_model.HasOwnerPackages(ctx, org.ID); err != nil { + return fmt.Errorf("HasOwnerPackages: %w", err) + } else if ownsPackages { + return packages_model.ErrUserOwnPackages{UID: org.ID} + } + + if err := deleteOrganization(ctx, org); err != nil { + return fmt.Errorf("DeleteOrganization: %w", err) + } + return nil + }) if err != nil { - return fmt.Errorf("GetRepositoryCount: %w", err) - } else if count > 0 { - return repo_model.ErrUserOwnRepos{UID: org.ID} - } - - // Check ownership of packages. - if ownsPackages, err := packages_model.HasOwnerPackages(ctx, org.ID); err != nil { - return fmt.Errorf("HasOwnerPackages: %w", err) - } else if ownsPackages { - return packages_model.ErrUserOwnPackages{UID: org.ID} - } - - if err := deleteOrganization(ctx, org); err != nil { - return fmt.Errorf("DeleteOrganization: %w", err) - } - - if err := committer.Commit(); err != nil { return err } diff --git a/services/repository/delete.go b/services/repository/delete.go index 9b3d1c07e85..5647843e4a7 100644 --- a/services/repository/delete.go +++ b/services/repository/delete.go @@ -51,15 +51,8 @@ func deleteDBRepository(ctx context.Context, repoID int64) error { // DeleteRepository deletes a repository for a user or organization. // make sure if you call this func to close open sessions (sqlite will otherwise get a deadlock) func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams ...bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) - repo := &repo_model.Repository{} - has, err := sess.ID(repoID).Get(repo) + has, err := db.GetEngine(ctx).ID(repoID).Get(repo) if err != nil { return err } else if !has { @@ -82,214 +75,215 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams return fmt.Errorf("list actions artifacts of repo %v: %w", repoID, err) } - // In case owner is a organization, we have to change repo specific teams - // if ignoreOrgTeams is not true - var org *user_model.User - if len(ignoreOrgTeams) == 0 || !ignoreOrgTeams[0] { - if org, err = user_model.GetUserByID(ctx, repo.OwnerID); err != nil { - return err - } - } + var needRewriteKeysFile bool + releaseAttachments := make([]*repo_model.Attachment, 0, 20) + var repoAttachments []*repo_model.Attachment + var archivePaths []string + var lfsPaths []string - // Delete Deploy Keys - deleted, err := asymkey_service.DeleteRepoDeployKeys(ctx, repoID) - if err != nil { - return err - } - needRewriteKeysFile := deleted > 0 - - if err := deleteDBRepository(ctx, repoID); err != nil { - return err - } - - if org != nil && org.IsOrganization() { - teams, err := organization.FindOrgTeams(ctx, org.ID) - if err != nil { - return err - } - for _, t := range teams { - if !organization.HasTeamRepo(ctx, t.OrgID, t.ID, repoID) { - continue - } else if err = removeRepositoryFromTeam(ctx, t, repo, false); err != nil { + err = db.WithTx(ctx, func(ctx context.Context) error { + // In case owner is a organization, we have to change repo specific teams + // if ignoreOrgTeams is not true + var org *user_model.User + if len(ignoreOrgTeams) == 0 || !ignoreOrgTeams[0] { + if org, err = user_model.GetUserByID(ctx, repo.OwnerID); err != nil { return err } } - } - // some attachments have release_id but repo_id = 0 - releaseAttachments := make([]*repo_model.Attachment, 0, 20) - if err = db.GetEngine(ctx).Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). - Where("`release`.repo_id = ?", repoID). - Find(&releaseAttachments); err != nil { - return err - } - - if _, err := repo_model.MarkAttachmentsDeleted(ctx, releaseAttachments); err != nil { - return fmt.Errorf("delete release attachments: %w", err) - } - - if _, err := db.Exec(ctx, "UPDATE `user` SET num_stars=num_stars-1 WHERE id IN (SELECT `uid` FROM `star` WHERE repo_id = ?)", repo.ID); err != nil { - return err - } - - if _, err := db.GetEngine(ctx).In("hook_id", builder.Select("id").From("webhook").Where(builder.Eq{"webhook.repo_id": repo.ID})). - Delete(&webhook.HookTask{}); err != nil { - return err - } - - // CleanupEphemeralRunnersByPickedTaskOfRepo deletes ephemeral global/org/user that have started any task of this repo - // The cannot pick a second task hardening for ephemeral runners expect that task objects remain available until runner deletion - // This method will delete affected ephemeral global/org/user runners - // &actions_model.ActionRunner{RepoID: repoID} does only handle ephemeral repository runners - if err := actions_service.CleanupEphemeralRunnersByPickedTaskOfRepo(ctx, repoID); err != nil { - return fmt.Errorf("cleanupEphemeralRunners: %w", err) - } - - if err := db.DeleteBeans(ctx, - &access_model.Access{RepoID: repo.ID}, - &activities_model.Action{RepoID: repo.ID}, - &repo_model.Collaboration{RepoID: repoID}, - &issues_model.Comment{RefRepoID: repoID}, - &git_model.CommitStatus{RepoID: repoID}, - &git_model.Branch{RepoID: repoID}, - &git_model.LFSLock{RepoID: repoID}, - &repo_model.LanguageStat{RepoID: repoID}, - &repo_model.RepoLicense{RepoID: repoID}, - &issues_model.Milestone{RepoID: repoID}, - &repo_model.Mirror{RepoID: repoID}, - &activities_model.Notification{RepoID: repoID}, - &git_model.ProtectedBranch{RepoID: repoID}, - &git_model.ProtectedTag{RepoID: repoID}, - &repo_model.PushMirror{RepoID: repoID}, - &repo_model.Release{RepoID: repoID}, - &repo_model.RepoIndexerStatus{RepoID: repoID}, - &repo_model.Redirect{RedirectRepoID: repoID}, - &repo_model.RepoUnit{RepoID: repoID}, - &repo_model.Star{RepoID: repoID}, - &admin_model.Task{RepoID: repoID}, - &repo_model.Watch{RepoID: repoID}, - &webhook.Webhook{RepoID: repoID}, - &secret_model.Secret{RepoID: repoID}, - &actions_model.ActionTaskStep{RepoID: repoID}, - &actions_model.ActionTask{RepoID: repoID}, - &actions_model.ActionRunJob{RepoID: repoID}, - &actions_model.ActionRun{RepoID: repoID}, - &actions_model.ActionRunner{RepoID: repoID}, - &actions_model.ActionScheduleSpec{RepoID: repoID}, - &actions_model.ActionSchedule{RepoID: repoID}, - &actions_model.ActionArtifact{RepoID: repoID}, - &actions_model.ActionRunnerToken{RepoID: repoID}, - &issues_model.IssuePin{RepoID: repoID}, - ); err != nil { - return fmt.Errorf("deleteBeans: %w", err) - } - - // Delete Labels and related objects - if err := issues_model.DeleteLabelsByRepoID(ctx, repoID); err != nil { - return err - } - - // Delete Pulls and related objects - if err := issues_model.DeletePullsByBaseRepoID(ctx, repoID); err != nil { - return err - } - - // Delete Issues and related objects - // attachments will be deleted later with repo_id, so we don't need to delete them here - if _, err := issue_service.DeleteIssuesByRepoID(ctx, repoID, false); err != nil { - return err - } - - // Delete issue index - if err := db.DeleteResourceIndex(ctx, "issue_index", repoID); err != nil { - return err - } - - if repo.IsFork { - if _, err := db.Exec(ctx, "UPDATE `repository` SET num_forks=num_forks-1 WHERE id=?", repo.ForkID); err != nil { - return fmt.Errorf("decrease fork count: %w", err) - } - } - - if _, err := db.Exec(ctx, "UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", repo.OwnerID); err != nil { - return err - } - - if len(repo.Topics) > 0 { - if err := repo_model.RemoveTopicsFromRepo(ctx, repo.ID); err != nil { - return err - } - } - - if err := project_model.DeleteProjectByRepoID(ctx, repoID); err != nil { - return fmt.Errorf("unable to delete projects for repo[%d]: %w", repoID, err) - } - - // Remove LFS objects - var lfsObjects []*git_model.LFSMetaObject - if err = sess.Where("repository_id=?", repoID).Find(&lfsObjects); err != nil { - return err - } - - lfsPaths := make([]string, 0, len(lfsObjects)) - for _, v := range lfsObjects { - count, err := db.CountByBean(ctx, &git_model.LFSMetaObject{Pointer: lfs.Pointer{Oid: v.Oid}}) + // Delete Deploy Keys + deleted, err := asymkey_service.DeleteRepoDeployKeys(ctx, repoID) if err != nil { return err } - if count > 1 { - continue + needRewriteKeysFile = deleted > 0 + + if err := deleteDBRepository(ctx, repoID); err != nil { + return err } - lfsPaths = append(lfsPaths, v.RelativePath()) - } - - if _, err := db.DeleteByBean(ctx, &git_model.LFSMetaObject{RepositoryID: repoID}); err != nil { - return err - } - - // Remove archives - var archives []*repo_model.RepoArchiver - if err = sess.Where("repo_id=?", repoID).Find(&archives); err != nil { - return err - } - - archivePaths := make([]string, 0, len(archives)) - for _, v := range archives { - archivePaths = append(archivePaths, v.RelativePath()) - } - - if _, err := db.DeleteByBean(ctx, &repo_model.RepoArchiver{RepoID: repoID}); err != nil { - return err - } - - if repo.NumForks > 0 { - if _, err = sess.Exec("UPDATE `repository` SET fork_id=0,is_fork=? WHERE fork_id=?", false, repo.ID); err != nil { - log.Error("reset 'fork_id' and 'is_fork': %v", err) + if org != nil && org.IsOrganization() { + teams, err := organization.FindOrgTeams(ctx, org.ID) + if err != nil { + return err + } + for _, t := range teams { + if !organization.HasTeamRepo(ctx, t.OrgID, t.ID, repoID) { + continue + } else if err = removeRepositoryFromTeam(ctx, t, repo, false); err != nil { + return err + } + } } - } - // Get all attachments with repo_id = repo.ID. some release attachments have repo_id = 0 should be deleted before - var repoAttachments []*repo_model.Attachment - if err := sess.Where(builder.Eq{ - "repo_id": repo.ID, - }).Find(&repoAttachments); err != nil { + // some attachments have release_id but repo_id = 0 + if err = db.GetEngine(ctx).Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). + Where("`release`.repo_id = ?", repoID). + Find(&releaseAttachments); err != nil { + return err + } + + if _, err := repo_model.MarkAttachmentsDeleted(ctx, releaseAttachments); err != nil { + return fmt.Errorf("delete release attachments: %w", err) + } + + if _, err := db.Exec(ctx, "UPDATE `user` SET num_stars=num_stars-1 WHERE id IN (SELECT `uid` FROM `star` WHERE repo_id = ?)", repo.ID); err != nil { + return err + } + + if _, err := db.GetEngine(ctx).In("hook_id", builder.Select("id").From("webhook").Where(builder.Eq{"webhook.repo_id": repo.ID})). + Delete(&webhook.HookTask{}); err != nil { + return err + } + + // CleanupEphemeralRunnersByPickedTaskOfRepo deletes ephemeral global/org/user that have started any task of this repo + // The cannot pick a second task hardening for ephemeral runners expect that task objects remain available until runner deletion + // This method will delete affected ephemeral global/org/user runners + // &actions_model.ActionRunner{RepoID: repoID} does only handle ephemeral repository runners + if err := actions_service.CleanupEphemeralRunnersByPickedTaskOfRepo(ctx, repoID); err != nil { + return fmt.Errorf("cleanupEphemeralRunners: %w", err) + } + + if err := db.DeleteBeans(ctx, + &access_model.Access{RepoID: repo.ID}, + &activities_model.Action{RepoID: repo.ID}, + &repo_model.Collaboration{RepoID: repoID}, + &issues_model.Comment{RefRepoID: repoID}, + &git_model.CommitStatus{RepoID: repoID}, + &git_model.Branch{RepoID: repoID}, + &git_model.LFSLock{RepoID: repoID}, + &repo_model.LanguageStat{RepoID: repoID}, + &repo_model.RepoLicense{RepoID: repoID}, + &issues_model.Milestone{RepoID: repoID}, + &repo_model.Mirror{RepoID: repoID}, + &activities_model.Notification{RepoID: repoID}, + &git_model.ProtectedBranch{RepoID: repoID}, + &git_model.ProtectedTag{RepoID: repoID}, + &repo_model.PushMirror{RepoID: repoID}, + &repo_model.Release{RepoID: repoID}, + &repo_model.RepoIndexerStatus{RepoID: repoID}, + &repo_model.Redirect{RedirectRepoID: repoID}, + &repo_model.RepoUnit{RepoID: repoID}, + &repo_model.Star{RepoID: repoID}, + &admin_model.Task{RepoID: repoID}, + &repo_model.Watch{RepoID: repoID}, + &webhook.Webhook{RepoID: repoID}, + &secret_model.Secret{RepoID: repoID}, + &actions_model.ActionTaskStep{RepoID: repoID}, + &actions_model.ActionTask{RepoID: repoID}, + &actions_model.ActionRunJob{RepoID: repoID}, + &actions_model.ActionRun{RepoID: repoID}, + &actions_model.ActionRunner{RepoID: repoID}, + &actions_model.ActionScheduleSpec{RepoID: repoID}, + &actions_model.ActionSchedule{RepoID: repoID}, + &actions_model.ActionArtifact{RepoID: repoID}, + &actions_model.ActionRunnerToken{RepoID: repoID}, + &issues_model.IssuePin{RepoID: repoID}, + ); err != nil { + return fmt.Errorf("deleteBeans: %w", err) + } + + // Delete Labels and related objects + if err := issues_model.DeleteLabelsByRepoID(ctx, repoID); err != nil { + return err + } + + // Delete Pulls and related objects + if err := issues_model.DeletePullsByBaseRepoID(ctx, repoID); err != nil { + return err + } + + // Delete Issues and related objects + // attachments will be deleted later with repo_id, so we don't need to delete them here + if _, err := issue_service.DeleteIssuesByRepoID(ctx, repoID, false); err != nil { + return err + } + + // Delete issue index + if err := db.DeleteResourceIndex(ctx, "issue_index", repoID); err != nil { + return err + } + + if repo.IsFork { + if _, err := db.Exec(ctx, "UPDATE `repository` SET num_forks=num_forks-1 WHERE id=?", repo.ForkID); err != nil { + return fmt.Errorf("decrease fork count: %w", err) + } + } + + if _, err := db.Exec(ctx, "UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", repo.OwnerID); err != nil { + return err + } + + if len(repo.Topics) > 0 { + if err := repo_model.RemoveTopicsFromRepo(ctx, repo.ID); err != nil { + return err + } + } + + if err := project_model.DeleteProjectByRepoID(ctx, repoID); err != nil { + return fmt.Errorf("unable to delete projects for repo[%d]: %w", repoID, err) + } + + // Remove LFS objects + var lfsObjects []*git_model.LFSMetaObject + if err = db.GetEngine(ctx).Where("repository_id=?", repoID).Find(&lfsObjects); err != nil { + return err + } + + lfsPaths = make([]string, 0, len(lfsObjects)) + for _, v := range lfsObjects { + count, err := db.CountByBean(ctx, &git_model.LFSMetaObject{Pointer: lfs.Pointer{Oid: v.Oid}}) + if err != nil { + return err + } + if count > 1 { + continue + } + + lfsPaths = append(lfsPaths, v.RelativePath()) + } + + if _, err := db.DeleteByBean(ctx, &git_model.LFSMetaObject{RepositoryID: repoID}); err != nil { + return err + } + + // Remove archives + var archives []*repo_model.RepoArchiver + if err = db.GetEngine(ctx).Where("repo_id=?", repoID).Find(&archives); err != nil { + return err + } + + archivePaths = make([]string, 0, len(archives)) + for _, v := range archives { + archivePaths = append(archivePaths, v.RelativePath()) + } + + if _, err := db.DeleteByBean(ctx, &repo_model.RepoArchiver{RepoID: repoID}); err != nil { + return err + } + + if repo.NumForks > 0 { + if _, err = db.GetEngine(ctx).Exec("UPDATE `repository` SET fork_id=0,is_fork=? WHERE fork_id=?", false, repo.ID); err != nil { + log.Error("reset 'fork_id' and 'is_fork': %v", err) + } + } + + // Get all attachments with repo_id = repo.ID. some release attachments have repo_id = 0 should be deleted before + if err := db.GetEngine(ctx).Where(builder.Eq{ + "repo_id": repo.ID, + }).Find(&repoAttachments); err != nil { + return err + } + if _, err := repo_model.MarkAttachmentsDeleted(ctx, repoAttachments); err != nil { + return err + } + + // unlink packages linked to this repository + return packages_model.UnlinkRepositoryFromAllPackages(ctx, repoID) + }) + if err != nil { return err } - if _, err := repo_model.MarkAttachmentsDeleted(ctx, repoAttachments); err != nil { - return err - } - - // unlink packages linked to this repository - if err = packages_model.UnlinkRepositoryFromAllPackages(ctx, repoID); err != nil { - return err - } - - if err = committer.Commit(); err != nil { - return err - } - - committer.Close() if needRewriteKeysFile { if err := asymkey_service.RewriteAllPublicKeys(ctx); err != nil { diff --git a/services/user/user.go b/services/user/user.go index ad17732df15..ef2d27fffd5 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -211,48 +211,43 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error { } } - ctx, committer, err := db.TxContext(ctx) + toBeCleanedAttachments, err := db.WithTx2(ctx, func(ctx context.Context) ([]*repo_model.Attachment, error) { + // Note: A user owns any repository or belongs to any organization + // cannot perform delete operation. This causes a race with the purge above + // however consistency requires that we ensure that this is the case + + // Check ownership of repository. + count, err := repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{OwnerID: u.ID}) + if err != nil { + return nil, fmt.Errorf("GetRepositoryCount: %w", err) + } else if count > 0 { + return nil, repo_model.ErrUserOwnRepos{UID: u.ID} + } + + // Check membership of organization. + count, err = organization.GetOrganizationCount(ctx, u) + if err != nil { + return nil, fmt.Errorf("GetOrganizationCount: %w", err) + } else if count > 0 { + return nil, organization.ErrUserHasOrgs{UID: u.ID} + } + + // Check ownership of packages. + if ownsPackages, err := packages_model.HasOwnerPackages(ctx, u.ID); err != nil { + return nil, fmt.Errorf("HasOwnerPackages: %w", err) + } else if ownsPackages { + return nil, packages_model.ErrUserOwnPackages{UID: u.ID} + } + + toBeCleanedAttachments, err := deleteUser(ctx, u, purge) + if err != nil { + return nil, fmt.Errorf("DeleteUser: %w", err) + } + return toBeCleanedAttachments, nil + }) if err != nil { return err } - defer committer.Close() - - // Note: A user owns any repository or belongs to any organization - // cannot perform delete operation. This causes a race with the purge above - // however consistency requires that we ensure that this is the case - - // Check ownership of repository. - count, err := repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{OwnerID: u.ID}) - if err != nil { - return fmt.Errorf("GetRepositoryCount: %w", err) - } else if count > 0 { - return repo_model.ErrUserOwnRepos{UID: u.ID} - } - - // Check membership of organization. - count, err = organization.GetOrganizationCount(ctx, u) - if err != nil { - return fmt.Errorf("GetOrganizationCount: %w", err) - } else if count > 0 { - return organization.ErrUserHasOrgs{UID: u.ID} - } - - // Check ownership of packages. - if ownsPackages, err := packages_model.HasOwnerPackages(ctx, u.ID); err != nil { - return fmt.Errorf("HasOwnerPackages: %w", err) - } else if ownsPackages { - return packages_model.ErrUserOwnPackages{UID: u.ID} - } - - toBeCleanedAttachments, err := deleteUser(ctx, u, purge) - if err != nil { - return fmt.Errorf("DeleteUser: %w", err) - } - - if err := committer.Commit(); err != nil { - return err - } - _ = committer.Close() attachment_service.AddAttachmentsToCleanQueue(ctx, toBeCleanedAttachments)