Move user/org deletion to services (#17673)

Author: KN4CK3R

cmd/admin.go

@@ -26,6 +26,7 @@ import (
 	auth_service "code.gitea.io/gitea/services/auth"
 	"code.gitea.io/gitea/services/auth/source/oauth2"
 	repo_service "code.gitea.io/gitea/services/repository"
+	user_service "code.gitea.io/gitea/services/user"
 
 	"github.com/urfave/cli"
 )
@@ -534,7 +535,7 @@ func runDeleteUser(c *cli.Context) error {
 		return fmt.Errorf("The user %s does not match the provided id %d", user.Name, c.Int64("id"))
 	}
 
-	return models.DeleteUser(user)
+	return user_service.DeleteUser(user)
 }
 
 func runRepoSyncReleases(_ *cli.Context) error {

models/admin/notice.go

@@ -43,12 +43,7 @@ func (n *Notice) TrStr() string {
 }
 
 // CreateNotice creates new system notice.
-func CreateNotice(tp NoticeType, desc string, args ...interface{}) error {
-	return CreateNoticeCtx(db.DefaultContext, tp, desc, args...)
-}
-
-// CreateNoticeCtx creates new system notice.
-func CreateNoticeCtx(ctx context.Context, tp NoticeType, desc string, args ...interface{}) error {
+func CreateNotice(ctx context.Context, tp NoticeType, desc string, args ...interface{}) error {
 	if len(args) > 0 {
 		desc = fmt.Sprintf(desc, args...)
 	}
@@ -61,38 +56,28 @@ func CreateNoticeCtx(ctx context.Context, tp NoticeType, desc string, args ...in
 
 // CreateRepositoryNotice creates new system notice with type NoticeRepository.
 func CreateRepositoryNotice(desc string, args ...interface{}) error {
-	return CreateNoticeCtx(db.DefaultContext, NoticeRepository, desc, args...)
+	return CreateNotice(db.DefaultContext, NoticeRepository, desc, args...)
 }
 
 // RemoveAllWithNotice removes all directories in given path and
 // creates a system notice when error occurs.
-func RemoveAllWithNotice(title, path string) {
-	RemoveAllWithNoticeCtx(db.DefaultContext, title, path)
-}
-
-// RemoveStorageWithNotice removes a file from the storage and
-// creates a system notice when error occurs.
-func RemoveStorageWithNotice(bucket storage.ObjectStorage, title, path string) {
-	removeStorageWithNotice(db.DefaultContext, bucket, title, path)
-}
-
-func removeStorageWithNotice(ctx context.Context, bucket storage.ObjectStorage, title, path string) {
-	if err := bucket.Delete(path); err != nil {
+func RemoveAllWithNotice(ctx context.Context, title, path string) {
+	if err := util.RemoveAll(path); err != nil {
 		desc := fmt.Sprintf("%s [%s]: %v", title, path, err)
 		log.Warn(title+" [%s]: %v", path, err)
-		if err = CreateNoticeCtx(ctx, NoticeRepository, desc); err != nil {
+		if err = CreateNotice(ctx, NoticeRepository, desc); err != nil {
 			log.Error("CreateRepositoryNotice: %v", err)
 		}
 	}
 }
 
-// RemoveAllWithNoticeCtx removes all directories in given path and
+// RemoveStorageWithNotice removes a file from the storage and
 // creates a system notice when error occurs.
-func RemoveAllWithNoticeCtx(ctx context.Context, title, path string) {
-	if err := util.RemoveAll(path); err != nil {
+func RemoveStorageWithNotice(ctx context.Context, bucket storage.ObjectStorage, title, path string) {
+	if err := bucket.Delete(path); err != nil {
 		desc := fmt.Sprintf("%s [%s]: %v", title, path, err)
 		log.Warn(title+" [%s]: %v", path, err)
-		if err = CreateNoticeCtx(ctx, NoticeRepository, desc); err != nil {
+		if err = CreateNotice(ctx, NoticeRepository, desc); err != nil {
 			log.Error("CreateRepositoryNotice: %v", err)
 		}
 	}

models/admin/notice_test.go

@@ -7,6 +7,7 @@ package admin
 import (
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/unittest"
 
 	"github.com/stretchr/testify/assert"
@@ -28,7 +29,7 @@ func TestCreateNotice(t *testing.T) {
 		Description: "test description",
 	}
 	unittest.AssertNotExistsBean(t, noticeBean)
-	assert.NoError(t, CreateNotice(noticeBean.Type, noticeBean.Description))
+	assert.NoError(t, CreateNotice(db.DefaultContext, noticeBean.Type, noticeBean.Description))
 	unittest.AssertExistsAndLoadBean(t, noticeBean)
 }
 

models/consistency.go

@@ -128,7 +128,7 @@ func DeleteOrphanedIssues() error {
 
 	// Remove issue attachment files.
 	for i := range attachmentPaths {
-		admin_model.RemoveAllWithNoticeCtx(db.DefaultContext, "Delete issue attachment", attachmentPaths[i])
+		admin_model.RemoveAllWithNotice(db.DefaultContext, "Delete issue attachment", attachmentPaths[i])
 	}
 	return nil
 }

models/org.go

@@ -6,6 +6,7 @@
 package models
 
 import (
+	"context"
 	"fmt"
 	"strings"
 
@@ -14,9 +15,7 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/structs"
-	"code.gitea.io/gitea/modules/util"
 
 	"xorm.io/builder"
 	"xorm.io/xorm"
@@ -254,68 +253,23 @@ func CountOrganizations() int64 {
 	return count
 }
 
-// DeleteOrganization completely and permanently deletes everything of organization.
-func DeleteOrganization(org *User) (err error) {
-	if !org.IsOrganization() {
-		return fmt.Errorf("%s is a user not an organization", org.Name)
-	}
-
-	sess := db.NewSession(db.DefaultContext)
-	defer sess.Close()
-
-	if err = sess.Begin(); err != nil {
-		return err
-	}
-
-	if err = deleteOrg(sess, org); err != nil {
-		if IsErrUserOwnRepos(err) {
-			return err
-		} else if err != nil {
-			return fmt.Errorf("deleteOrg: %v", err)
-		}
-	}
-
-	return sess.Commit()
-}
-
-func deleteOrg(e *xorm.Session, u *User) error {
-	// Check ownership of repository.
-	count, err := getRepositoryCount(e, u)
-	if err != nil {
-		return fmt.Errorf("GetRepositoryCount: %v", err)
-	} else if count > 0 {
-		return ErrUserOwnRepos{UID: u.ID}
-	}
+// DeleteOrganization deletes models associated to an organization.
+func DeleteOrganization(ctx context.Context, org *User) error {
+	e := db.GetEngine(ctx)
 
 	if err := deleteBeans(e,
-		&Team{OrgID: u.ID},
-		&OrgUser{OrgID: u.ID},
-		&TeamUser{OrgID: u.ID},
-		&TeamUnit{OrgID: u.ID},
+		&Team{OrgID: org.ID},
+		&OrgUser{OrgID: org.ID},
+		&TeamUser{OrgID: org.ID},
+		&TeamUnit{OrgID: org.ID},
 	); err != nil {
 		return fmt.Errorf("deleteBeans: %v", err)
 	}
 
-	if _, err = e.ID(u.ID).Delete(new(User)); err != nil {
+	if _, err := e.ID(org.ID).Delete(new(User)); err != nil {
 		return fmt.Errorf("Delete: %v", err)
 	}
 
-	// FIXME: system notice
-	// Note: There are something just cannot be roll back,
-	//	so just keep error logs of those operations.
-	path := UserPath(u.Name)
-
-	if err := util.RemoveAll(path); err != nil {
-		return fmt.Errorf("Failed to RemoveAll %s: %v", path, err)
-	}
-
-	if len(u.Avatar) > 0 {
-		avatarPath := u.CustomAvatarRelativePath()
-		if err := storage.Avatars.Delete(avatarPath); err != nil {
-			return fmt.Errorf("Failed to remove %s: %v", avatarPath, err)
-		}
-	}
-
 	return nil
 }
 

models/org_test.go

@@ -261,24 +261,6 @@ func TestCountOrganizations(t *testing.T) {
 	assert.Equal(t, expected, CountOrganizations())
 }
 
-func TestDeleteOrganization(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-	org := unittest.AssertExistsAndLoadBean(t, &User{ID: 6}).(*User)
-	assert.NoError(t, DeleteOrganization(org))
-	unittest.AssertNotExistsBean(t, &User{ID: 6})
-	unittest.AssertNotExistsBean(t, &OrgUser{OrgID: 6})
-	unittest.AssertNotExistsBean(t, &Team{OrgID: 6})
-
-	org = unittest.AssertExistsAndLoadBean(t, &User{ID: 3}).(*User)
-	err := DeleteOrganization(org)
-	assert.Error(t, err)
-	assert.True(t, IsErrUserOwnRepos(err))
-
-	user := unittest.AssertExistsAndLoadBean(t, &User{ID: 5}).(*User)
-	assert.Error(t, DeleteOrganization(user))
-	unittest.CheckConsistencyFor(t, &User{}, &Team{})
-}
-
 func TestIsOrganizationOwner(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	test := func(orgID, userID int64, expected bool) {

models/repo.go

@@ -134,7 +134,7 @@ func NewRepoContext() {
 	loadRepoConfig()
 	unit.LoadUnitConfig()
 
-	admin_model.RemoveAllWithNotice("Clean up repository temporary data", filepath.Join(setting.AppDataPath, "tmp"))
+	admin_model.RemoveAllWithNotice(db.DefaultContext, "Clean up repository temporary data", filepath.Join(setting.AppDataPath, "tmp"))
 }
 
 // RepositoryStatus defines the status of repository
@@ -1649,36 +1649,36 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
 
 	// Remove repository files.
 	repoPath := repo.RepoPath()
-	admin_model.RemoveAllWithNotice("Delete repository files", repoPath)
+	admin_model.RemoveAllWithNotice(db.DefaultContext, "Delete repository files", repoPath)
 
 	// Remove wiki files
 	if repo.HasWiki() {
-		admin_model.RemoveAllWithNotice("Delete repository wiki", repo.WikiPath())
+		admin_model.RemoveAllWithNotice(db.DefaultContext, "Delete repository wiki", repo.WikiPath())
 	}
 
 	// Remove archives
 	for i := range archivePaths {
-		admin_model.RemoveStorageWithNotice(storage.RepoArchives, "Delete repo archive file", archivePaths[i])
+		admin_model.RemoveStorageWithNotice(db.DefaultContext, storage.RepoArchives, "Delete repo archive file", archivePaths[i])
 	}
 
 	// Remove lfs objects
 	for i := range lfsPaths {
-		admin_model.RemoveStorageWithNotice(storage.LFS, "Delete orphaned LFS file", lfsPaths[i])
+		admin_model.RemoveStorageWithNotice(db.DefaultContext, storage.LFS, "Delete orphaned LFS file", lfsPaths[i])
 	}
 
 	// Remove issue attachment files.
 	for i := range attachmentPaths {
-		admin_model.RemoveStorageWithNotice(storage.Attachments, "Delete issue attachment", attachmentPaths[i])
+		admin_model.RemoveStorageWithNotice(db.DefaultContext, storage.Attachments, "Delete issue attachment", attachmentPaths[i])
 	}
 
 	// Remove release attachment files.
 	for i := range releaseAttachments {
-		admin_model.RemoveStorageWithNotice(storage.Attachments, "Delete release attachment", releaseAttachments[i])
+		admin_model.RemoveStorageWithNotice(db.DefaultContext, storage.Attachments, "Delete release attachment", releaseAttachments[i])
 	}
 
 	// Remove attachment with no issue_id and release_id.
 	for i := range newAttachmentPaths {
-		admin_model.RemoveStorageWithNotice(storage.Attachments, "Delete issue attachment", attachmentPaths[i])
+		admin_model.RemoveStorageWithNotice(db.DefaultContext, storage.Attachments, "Delete issue attachment", attachmentPaths[i])
 	}
 
 	if len(repo.Avatar) > 0 {
@@ -1803,8 +1803,8 @@ func getPrivateRepositoryCount(e db.Engine, u *User) (int64, error) {
 }
 
 // GetRepositoryCount returns the total number of repositories of user.
-func GetRepositoryCount(u *User) (int64, error) {
-	return getRepositoryCount(db.GetEngine(db.DefaultContext), u)
+func GetRepositoryCount(ctx context.Context, u *User) (int64, error) {
+	return getRepositoryCount(db.GetEngine(ctx), u)
 }
 
 // GetPublicRepositoryCount returns the total number of public repositories of user.

models/repo_test.go

@@ -71,7 +71,7 @@ func TestMetas(t *testing.T) {
 func TestGetRepositoryCount(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 
-	count, err1 := GetRepositoryCount(&User{ID: int64(10)})
+	count, err1 := GetRepositoryCount(db.DefaultContext, &User{ID: int64(10)})
 	privateCount, err2 := GetPrivateRepositoryCount(&User{ID: int64(10)})
 	publicCount, err3 := GetPublicRepositoryCount(&User{ID: int64(10)})
 	assert.NoError(t, err1)