From 3dc0c962bf3a74188bed02c01b4d34636112805a Mon Sep 17 00:00:00 2001 From: JakobDev Date: Thu, 19 Oct 2023 15:16:11 +0200 Subject: [PATCH] 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. --- routers/api/v1/org/org.go | 2 +- routers/web/org/setting.go | 4 ++-- services/org/org.go | 16 +++++++++------- services/org/org_test.go | 6 +++--- services/repository/delete.go | 27 +++++++++++++++++++++++++++ services/repository/delete_test.go | 16 +++++++++++++--- services/repository/lfs_test.go | 7 ++++--- services/user/user.go | 30 ++++++++---------------------- 8 files changed, 67 insertions(+), 41 deletions(-) diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index 4579f3b085..6fb8ecd403 100644 --- 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 2f2f7c14bd..fac83b3612 100644 --- 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 4ecafc93a6..dca7794b47 100644 --- 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 763c708c14..e7d2a18ea9 100644 --- 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 b7fe4282b3..9b0a988ea3 100644 --- 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 ae5fd53962..869b8af11d 100644 --- 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 e88befdfef..61348143d0 100644 --- 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 2806969638..4a4908fe8e 100644 --- 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)