Delete repos of org when purge delete user (#27273) (#27728)

6543 · JakobDev · web-flow · commit 6637bbf5100a · 2023-11-01T10:03:12.000+08:00
Fixes https://codeberg.org/forgejo/forgejo/issues/1514 Backports #27273 --------- Co-authored-by: JakobDev <jakobdev@gmx.de>
diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go
@@ -716,7 +716,7 @@ func FindUserCodeAccessibleOwnerRepoIDs(ctx context.Context, ownerID int64, user
 }
 
 // GetUserRepositories returns a list of repositories of given user.
-func GetUserRepositories(opts *SearchRepoOptions) (RepositoryList, int64, error) {
+func GetUserRepositories(ctx context.Context, opts *SearchRepoOptions) (RepositoryList, int64, error) {
 	if len(opts.OrderBy) == 0 {
 		opts.OrderBy = "updated_unix DESC"
 	}
@@ -734,7 +734,7 @@ func GetUserRepositories(opts *SearchRepoOptions) (RepositoryList, int64, error)
 		cond = cond.And(builder.In("lower_name", opts.LowerNames))
 	}
 
-	sess := db.GetEngine(db.DefaultContext)
+	sess := db.GetEngine(ctx)
 
 	count, err := sess.Where(cond).Count(new(Repository))
 	if err != nil {
diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go
@@ -385,7 +385,7 @@ func Delete(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	if err := org.DeleteOrganization(ctx.Org.Organization); err != nil {
+	if err := org.DeleteOrganization(ctx, ctx.Org.Organization, false); err != nil {
 		ctx.Error(http.StatusInternalServerError, "DeleteOrganization", err)
 		return
 	}
diff --git a/routers/api/v1/user/repo.go b/routers/api/v1/user/repo.go
@@ -21,7 +21,7 @@ import (
 func listUserRepos(ctx *context.APIContext, u *user_model.User, private bool) {
 	opts := utils.GetListOptions(ctx)
 
-	repos, count, err := repo_model.GetUserRepositories(&repo_model.SearchRepoOptions{
+	repos, count, err := repo_model.GetUserRepositories(ctx, &repo_model.SearchRepoOptions{
 		Actor:       u,
 		Private:     private,
 		ListOptions: opts,
diff --git a/routers/web/org/setting.go b/routers/web/org/setting.go
@@ -75,7 +75,7 @@ func SettingsPost(ctx *context.Context) {
 
 	// Check if organization name has been changed.
 	if nameChanged {
-		err := org_service.RenameOrganization(ctx, org, form.Name)
+		err := user_service.RenameUser(ctx, org.AsUser(), form.Name)
 		switch {
 		case user_model.IsErrUserAlreadyExist(err):
 			ctx.Data["OrgName"] = true
@@ -124,7 +124,7 @@ func SettingsPost(ctx *context.Context) {
 
 	// update forks visibility
 	if visibilityChanged {
-		repos, _, err := repo_model.GetUserRepositories(&repo_model.SearchRepoOptions{
+		repos, _, err := repo_model.GetUserRepositories(ctx, &repo_model.SearchRepoOptions{
 			Actor: org.AsUser(), Private: true, ListOptions: db.ListOptions{Page: 1, PageSize: org.NumRepos},
 		})
 		if err != nil {
@@ -180,7 +180,7 @@ func SettingsDelete(ctx *context.Context) {
 			return
 		}
 
-		if err := org_service.DeleteOrganization(ctx.Org.Organization); err != nil {
+		if err := org_service.DeleteOrganization(ctx, ctx.Org.Organization, false); err != nil {
 			if models.IsErrUserOwnRepos(err) {
 				ctx.Flash.Error(ctx.Tr("form.org_still_own_repo"))
 				ctx.Redirect(ctx.Org.OrgLink + "/settings/delete")
diff --git a/routers/web/user/home_test.go b/routers/web/user/home_test.go
@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	"code.gitea.io/gitea/modules/contexttest"
@@ -25,7 +26,7 @@ func TestArchivedIssues(t *testing.T) {
 	ctx.Req.Form.Set("state", "open")
 
 	// Assume: User 30 has access to two Repos with Issues, one of the Repos being archived.
-	repos, _, _ := repo_model.GetUserRepositories(&repo_model.SearchRepoOptions{Actor: ctx.Doer})
+	repos, _, _ := repo_model.GetUserRepositories(db.DefaultContext, &repo_model.SearchRepoOptions{Actor: ctx.Doer})
 	assert.Len(t, repos, 3)
 	IsArchived := make(map[int64]bool)
 	NumIssues := make(map[int64]int)
diff --git a/routers/web/user/package.go b/routers/web/user/package.go
@@ -383,7 +383,7 @@ func PackageSettings(ctx *context.Context) {
 	ctx.Data["IsPackagesPage"] = true
 	ctx.Data["PackageDescriptor"] = pd
 
-	repos, _, _ := repo_model.GetUserRepositories(&repo_model.SearchRepoOptions{
+	repos, _, _ := repo_model.GetUserRepositories(ctx, &repo_model.SearchRepoOptions{
 		Actor:   pd.Owner,
 		Private: true,
 	})
diff --git a/routers/web/user/setting/profile.go b/routers/web/user/setting/profile.go
@@ -287,7 +287,7 @@ func Repos(ctx *context.Context) {
 			return
 		}
 
-		userRepos, _, err := repo_model.GetUserRepositories(&repo_model.SearchRepoOptions{
+		userRepos, _, err := repo_model.GetUserRepositories(ctx, &repo_model.SearchRepoOptions{
 			Actor:   ctxUser,
 			Private: true,
 			ListOptions: db.ListOptions{
@@ -312,7 +312,7 @@ func Repos(ctx *context.Context) {
 		ctx.Data["Dirs"] = repoNames
 		ctx.Data["ReposMap"] = repos
 	} else {
-		repos, count64, err := repo_model.GetUserRepositories(&repo_model.SearchRepoOptions{Actor: ctxUser, Private: true, ListOptions: opts})
+		repos, count64, err := repo_model.GetUserRepositories(ctx, &repo_model.SearchRepoOptions{Actor: ctxUser, Private: true, ListOptions: opts})
 		if err != nil {
 			ctx.ServerError("GetUserRepositories", err)
 			return
diff --git a/services/org/org.go b/services/org/org.go
@@ -15,17 +15,24 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/util"
-	user_service "code.gitea.io/gitea/services/user"
+	repo_service "code.gitea.io/gitea/services/repository"
 )
 
 // DeleteOrganization completely and permanently deletes everything of organization.
-func DeleteOrganization(org *org_model.Organization) error {
-	ctx, commiter, err := db.TxContext(db.DefaultContext)
+func DeleteOrganization(ctx context.Context, org *org_model.Organization, purge bool) error {
+	ctx, commiter, err := db.TxContext(ctx)
 	if err != nil {
 		return err
 	}
 	defer commiter.Close()
 
+	if purge {
+		err := repo_service.DeleteOwnerRepositoriesDirectly(ctx, org.AsUser())
+		if err != nil {
+			return err
+		}
+	}
+
 	// Check ownership of repository.
 	count, err := repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{OwnerID: org.ID})
 	if err != nil {
@@ -67,8 +74,3 @@ func DeleteOrganization(org *org_model.Organization) error {
 
 	return nil
 }
-
-// RenameOrganization renames an organization.
-func RenameOrganization(ctx context.Context, org *org_model.Organization, newName string) error {
-	return user_service.RenameUser(ctx, org.AsUser(), newName)
-}
diff --git a/services/org/org_test.go b/services/org/org_test.go
@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/organization"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
@@ -24,17 +25,17 @@ func TestMain(m *testing.M) {
 func TestDeleteOrganization(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 6})
-	assert.NoError(t, DeleteOrganization(org))
+	assert.NoError(t, DeleteOrganization(db.DefaultContext, org, false))
 	unittest.AssertNotExistsBean(t, &organization.Organization{ID: 6})
 	unittest.AssertNotExistsBean(t, &organization.OrgUser{OrgID: 6})
 	unittest.AssertNotExistsBean(t, &organization.Team{OrgID: 6})
 
 	org = unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
-	err := DeleteOrganization(org)
+	err := DeleteOrganization(db.DefaultContext, org, false)
 	assert.Error(t, err)
 	assert.True(t, models.IsErrUserOwnRepos(err))
 
 	user := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 5})
-	assert.Error(t, DeleteOrganization(user))
+	assert.Error(t, DeleteOrganization(db.DefaultContext, user, false))
 	unittest.CheckConsistencyFor(t, &user_model.User{}, &organization.Team{})
 }
diff --git a/services/repository/adopt.go b/services/repository/adopt.go
@@ -259,7 +259,7 @@ func checkUnadoptedRepositories(ctx context.Context, userName string, repoNamesT
 		}
 		return err
 	}
-	repos, _, err := repo_model.GetUserRepositories(&repo_model.SearchRepoOptions{
+	repos, _, err := repo_model.GetUserRepositories(ctx, &repo_model.SearchRepoOptions{
 		Actor:   ctxUser,
 		Private: true,
 		ListOptions: db.ListOptions{
diff --git a/services/repository/delete.go b/services/repository/delete.go
@@ -425,3 +425,30 @@ func RemoveRepositoryFromTeam(ctx context.Context, t *organization.Team, repoID
 
 	return committer.Commit()
 }
+
+// DeleteOwnerRepositoriesDirectly calls DeleteRepositoryDirectly for all repos of the given owner
+func DeleteOwnerRepositoriesDirectly(ctx context.Context, owner *user_model.User) error {
+	for {
+		repos, _, err := repo_model.GetUserRepositories(ctx, &repo_model.SearchRepoOptions{
+			ListOptions: db.ListOptions{
+				PageSize: repo_model.RepositoryListDefaultPageSize,
+				Page:     1,
+			},
+			Private: true,
+			OwnerID: owner.ID,
+			Actor:   owner,
+		})
+		if err != nil {
+			return fmt.Errorf("GetUserRepositories: %w", err)
+		}
+		if len(repos) == 0 {
+			break
+		}
+		for _, repo := range repos {
+			if err := DeleteRepositoryDirectly(ctx, owner, owner.ID, repo.ID); err != nil {
+				return fmt.Errorf("unable to delete repository %s for %s[%d]. Error: %w", repo.Name, owner.Name, owner.ID, err)
+			}
+		}
+	}
+	return nil
+}
diff --git a/services/repository/delete_test.go b/services/repository/delete_test.go
@@ -1,7 +1,7 @@
 // Copyright 2017 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-package repository
+package repository_test
 
 import (
 	"testing"
@@ -10,6 +10,8 @@ import (
 	"code.gitea.io/gitea/models/organization"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	repo_service "code.gitea.io/gitea/services/repository"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -19,7 +21,7 @@ func TestTeam_HasRepository(t *testing.T) {
 
 	test := func(teamID, repoID int64, expected bool) {
 		team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: teamID})
-		assert.Equal(t, expected, HasRepository(db.DefaultContext, team, repoID))
+		assert.Equal(t, expected, repo_service.HasRepository(db.DefaultContext, team, repoID))
 	}
 	test(1, 1, false)
 	test(1, 3, true)
@@ -35,11 +37,19 @@ func TestTeam_RemoveRepository(t *testing.T) {
 
 	testSuccess := func(teamID, repoID int64) {
 		team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: teamID})
-		assert.NoError(t, RemoveRepositoryFromTeam(db.DefaultContext, team, repoID))
+		assert.NoError(t, repo_service.RemoveRepositoryFromTeam(db.DefaultContext, team, repoID))
 		unittest.AssertNotExistsBean(t, &organization.TeamRepo{TeamID: teamID, RepoID: repoID})
 		unittest.CheckConsistencyFor(t, &organization.Team{ID: teamID}, &repo_model.Repository{ID: repoID})
 	}
 	testSuccess(2, 3)
 	testSuccess(2, 5)
 	testSuccess(1, unittest.NonexistentID)
 }
+
+func TestDeleteOwnerRepositoriesDirectly(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+
+	assert.NoError(t, repo_service.DeleteOwnerRepositoriesDirectly(db.DefaultContext, user))
+}
diff --git a/services/repository/lfs_test.go b/services/repository/lfs_test.go
@@ -1,7 +1,7 @@
 // Copyright 2023 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-package repository
+package repository_test
 
 import (
 	"bytes"
@@ -16,12 +16,13 @@ import (
 	"code.gitea.io/gitea/modules/lfs"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/storage"
+	repo_service "code.gitea.io/gitea/services/repository"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestGarbageCollectLFSMetaObjects(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
+	unittest.PrepareTestEnv(t)
 
 	setting.LFS.StartServer = true
 	err := storage.Init()
@@ -35,7 +36,7 @@ func TestGarbageCollectLFSMetaObjects(t *testing.T) {
 	lfsOid := storeObjectInRepo(t, repo.ID, &lfsContent)
 
 	// gc
-	err = GarbageCollectLFSMetaObjects(context.Background(), GarbageCollectLFSMetaObjectsOptions{
+	err = repo_service.GarbageCollectLFSMetaObjects(context.Background(), repo_service.GarbageCollectLFSMetaObjectsOptions{
 		AutoFix:                 true,
 		OlderThan:               time.Now().Add(7 * 24 * time.Hour).Add(5 * 24 * time.Hour),
 		UpdatedLessRecentlyThan: time.Now().Add(7 * 24 * time.Hour).Add(3 * 24 * time.Hour),
diff --git a/services/user/user.go b/services/user/user.go
@@ -24,6 +24,7 @@ import (
 	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/services/agit"
+	org_service "code.gitea.io/gitea/services/org"
 	"code.gitea.io/gitea/services/packages"
 	container_service "code.gitea.io/gitea/services/packages/container"
 	repo_service "code.gitea.io/gitea/services/repository"
@@ -158,27 +159,9 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
 		//
 		// An alternative option here would be write a DeleteAllRepositoriesForUserID function which would delete all of the repos
 		// but such a function would likely get out of date
-		for {
-			repos, _, err := repo_model.GetUserRepositories(&repo_model.SearchRepoOptions{
-				ListOptions: db.ListOptions{
-					PageSize: repo_model.RepositoryListDefaultPageSize,
-					Page:     1,
-				},
-				Private: true,
-				OwnerID: u.ID,
-				Actor:   u,
-			})
-			if err != nil {
-				return fmt.Errorf("GetUserRepositories: %w", err)
-			}
-			if len(repos) == 0 {
-				break
-			}
-			for _, repo := range repos {
-				if err := repo_service.DeleteRepositoryDirectly(ctx, u, u.ID, repo.ID); err != nil {
-					return fmt.Errorf("unable to delete repository %s for %s[%d]. Error: %w", repo.Name, u.Name, u.ID, err)
-				}
-			}
+		err := repo_service.DeleteOwnerRepositoriesDirectly(ctx, u)
+		if err != nil {
+			return err
 		}
 
 		// Remove from Organizations and delete last owner organizations
@@ -206,7 +189,10 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
 			for _, org := range orgs {
 				if err := models.RemoveOrgUser(org.ID, u.ID); err != nil {
 					if organization.IsErrLastOrgOwner(err) {
-						err = organization.DeleteOrganization(ctx, org)
+						err = org_service.DeleteOrganization(ctx, org, true)
+						if err != nil {
+							return fmt.Errorf("unable to delete organization %d: %w", org.ID, err)
+						}
 					}
 					if err != nil {
 						return fmt.Errorf("unable to remove user %s[%d] from org %s[%d]. Error: %w", u.Name, u.ID, org.Name, org.ID, err)

Original file line number	Diff line number	Diff line change
`@@ -716,7 +716,7 @@ func FindUserCodeAccessibleOwnerRepoIDs(ctx context.Context, ownerID int64, user`
`716`	`716`	`}`
`717`	`717`
`718`	`718`	`// GetUserRepositories returns a list of repositories of given user.`
`719`		`-func GetUserRepositories(opts *SearchRepoOptions) (RepositoryList, int64, error) {`
	`719`	`+func GetUserRepositories(ctx context.Context, opts *SearchRepoOptions) (RepositoryList, int64, error) {`
`720`	`720`	`if len(opts.OrderBy) == 0 {`
`721`	`721`	`opts.OrderBy = "updated_unix DESC"`
`722`	`722`	`}`
`@@ -734,7 +734,7 @@ func GetUserRepositories(opts *SearchRepoOptions) (RepositoryList, int64, error)`
`734`	`734`	`cond = cond.And(builder.In("lower_name", opts.LowerNames))`
`735`	`735`	`}`
`736`	`736`
`737`		`- sess := db.GetEngine(db.DefaultContext)`
	`737`	`+ sess := db.GetEngine(ctx)`
`738`	`738`
`739`	`739`	`count, err := sess.Where(cond).Count(new(Repository))`
`740`	`740`	`if err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -385,7 +385,7 @@ func Delete(ctx *context.APIContext) {`
`385`	`385`	`// "404":`
`386`	`386`	`// "$ref": "#/responses/notFound"`
`387`	`387`
`388`		`- if err := org.DeleteOrganization(ctx.Org.Organization); err != nil {`
	`388`	`+ if err := org.DeleteOrganization(ctx, ctx.Org.Organization, false); err != nil {`
`389`	`389`	`ctx.Error(http.StatusInternalServerError, "DeleteOrganization", err)`
`390`	`390`	`return`
`391`	`391`	`}`
Original file line number	Diff line number	Diff line change
`@@ -259,7 +259,7 @@ func checkUnadoptedRepositories(ctx context.Context, userName string, repoNamesT`
`259`	`259`	`}`
`260`	`260`	`return err`
`261`	`261`	`}`
`262`		`- repos, _, err := repo_model.GetUserRepositories(&repo_model.SearchRepoOptions{`
	`262`	`+ repos, _, err := repo_model.GetUserRepositories(ctx, &repo_model.SearchRepoOptions{`
`263`	`263`	`Actor: ctxUser,`
`264`	`264`	`Private: true,`
`265`	`265`	`ListOptions: db.ListOptions{`