Apply Feedback

Author: JakobDev

services/org/org.go

@@ -21,34 +21,9 @@ import (
 // DeleteOrganization completely and permanently deletes everything of organization.
 func DeleteOrganization(ctx context.Context, org *org_model.Organization, purge bool) error {
 	if purge {
-		// Delete all repos belonging to this organisation
-		// Now this is not within a transaction because there are internal transactions within the DeleteRepository
-		// BUT: the db will still be consistent even if a number of repos have already been deleted.
-		// And in fact we want to capture any repositories that are being created in other transactions in the meantime
-		//
-		// 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: org.ID,
-				Actor:   org.AsUser(),
-			})
-			if err != nil {
-				return fmt.Errorf("GetUserRepositories: %w", err)
-			}
-			if len(repos) == 0 {
-				break
-			}
-			for _, repo := range repos {
-				if err := repo_service.DeleteRepositoryDirectly(ctx, org.AsUser(), org.ID, repo.ID); err != nil {
-					return fmt.Errorf("unable to delete repository %s for %s[%d]. Error: %w", repo.Name, org.Name, org.ID, err)
-				}
-			}
+		err := repo_service.DeleteOwnerRepositoriesDirectly(ctx, org.AsUser())
+		if err != nil {
+			return err
 		}
 	}
 

services/repository/delete.go

@@ -422,3 +422,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(&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
+}

services/repository/delete_test.go

@@ -10,6 +10,7 @@ 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"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -43,3 +44,11 @@ func TestTeam_RemoveRepository(t *testing.T) {
 	testSuccess(2, 5)
 	testSuccess(1, unittest.NonexistentID)
 }
+
+func TestDeleteOwnerRepositoriesDirectly(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+
+	assert.NoError(t, DeleteOwnerRepositoriesDirectly(db.DefaultContext, user))
+}

services/user/user.go

@@ -159,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
@@ -209,7 +191,7 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
 					if organization.IsErrLastOrgOwner(err) {
 						err = org_service.DeleteOrganization(ctx, org, true)
 						if err != nil {
-							return fmt.Errorf("unable to delete organisation %d: %w", org.ID, err)
+							return fmt.Errorf("unable to delete organization %d: %w", org.ID, err)
 						}
 					}
 					if err != nil {