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

Fixes https://codeberg.org/forgejo/forgejo/issues/1514

I had to remove `RenameOrganization` to avoid circular import.

We should really add some foreign keys to the database.

diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go
index 4579f3b0855664352b01b2ac70c314eeed57b0d7..6fb8ecd403a063e65d4300f1a55452a8a3a0e779 100644 (file)
--- 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, 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/web/org/setting.go b/routers/web/org/setting.go
index 2f2f7c14bdb68c9142d0b635b412757a58a40c37..fac83b361285ba65a8c5b29555b5a4a9bb0033f7 100644 (file)
--- 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
@@ -180,7 +180,7 @@ func SettingsDelete(ctx *context.Context) {
                        return
                }
 
-               if err := org_service.DeleteOrganization(ctx, 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/services/org/org.go b/services/org/org.go
index 4ecafc93a6f05aed79a3cd82b01e49c99ad54e0c..dca7794b47150e2f7a2ec65b00c41a067fb8ecc9 100644 (file)
--- 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(ctx context.Context, org *org_model.Organization) error {
+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(ctx context.Context, 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
index 763c708c143bf0b2a0e59ee434bc4f362be0385d..e7d2a18ea968808638f53fa913b51545b9672983 100644 (file)
--- a/services/org/org_test.go
+++ b/services/org/org_test.go
@@ -22,17 +22,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(db.DefaultContext, 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(db.DefaultContext, 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(db.DefaultContext, user))
+       assert.Error(t, DeleteOrganization(db.DefaultContext, user, false))
        unittest.CheckConsistencyFor(t, &user_model.User{}, &organization.Team{})
 }

diff --git a/services/repository/delete.go b/services/repository/delete.go
index b7fe4282b35898644cc29e2d204bd11781f52693..9b0a988ea3e2d8793fb1cdd3fb3b72fa5f66624b 100644 (file)
--- a/services/repository/delete.go
+++ b/services/repository/delete.go
@@ -420,3 +420,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, 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
index ae5fd5396210218be1546ee71e73433776cc656f..869b8af11d57d1115961acbc2373b84ec3cae9cd 100644 (file)
--- 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,7 +37,7 @@ 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})
        }
@@ -43,3 +45,11 @@ func TestTeam_RemoveRepository(t *testing.T) {
        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
index e88befdfefe96c66083e6851ca6c4f37aa4af526..61348143d00bf0673738117b1ef540554e52f061 100644 (file)
--- 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
index 2806969638fae548ab0c8c763a50833f4d8375a1..4a4908fe8e523fd6b51811b4e88a31b4304ec686 100644 (file)
--- 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(ctx, &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, 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(ctx, 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)