From 9852c92e9a633fdd318fde5014a12ef299b990ef Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 18 Oct 2023 23:03:10 +0800 Subject: [PATCH] Remove unnecessary parameter (#27671) --- routers/api/v1/repo/migrate.go | 2 +- services/repository/check.go | 2 +- services/repository/create.go | 2 +- services/repository/create_test.go | 4 ++-- services/repository/delete.go | 32 ++++++++++++++---------------- services/repository/repository.go | 2 +- services/user/user.go | 2 +- tests/integration/api_repo_test.go | 2 +- 8 files changed, 23 insertions(+), 25 deletions(-) diff --git a/routers/api/v1/repo/migrate.go b/routers/api/v1/repo/migrate.go index 14ff04e7b..839fbfe8a 100644 --- a/routers/api/v1/repo/migrate.go +++ b/routers/api/v1/repo/migrate.go @@ -200,7 +200,7 @@ func Migrate(ctx *context.APIContext) { } if repo != nil { - if errDelete := repo_service.DeleteRepositoryDirectly(ctx, ctx.Doer, repoOwner.ID, repo.ID); errDelete != nil { + if errDelete := repo_service.DeleteRepositoryDirectly(ctx, ctx.Doer, repo.ID); errDelete != nil { log.Error("DeleteRepository: %v", errDelete) } } diff --git a/services/repository/check.go b/services/repository/check.go index 6ad644561..2f26d030c 100644 --- a/services/repository/check.go +++ b/services/repository/check.go @@ -164,7 +164,7 @@ func DeleteMissingRepositories(ctx context.Context, doer *user_model.User) error default: } log.Trace("Deleting %d/%d...", repo.OwnerID, repo.ID) - if err := DeleteRepositoryDirectly(ctx, doer, repo.OwnerID, repo.ID); err != nil { + if err := DeleteRepositoryDirectly(ctx, doer, repo.ID); err != nil { log.Error("Failed to DeleteRepository %-v: Error: %v", repo, err) if err2 := system_model.CreateRepositoryNotice("Failed to DeleteRepository %s [%d]: Error: %v", repo.FullName(), repo.ID, err); err2 != nil { log.Error("CreateRepositoryNotice: %v", err) diff --git a/services/repository/create.go b/services/repository/create.go index 09956b74d..b6b6454c4 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -302,7 +302,7 @@ func CreateRepositoryDirectly(ctx context.Context, doer, u *user_model.User, opt return nil }); err != nil { if rollbackRepo != nil { - if errDelete := DeleteRepositoryDirectly(ctx, doer, rollbackRepo.OwnerID, rollbackRepo.ID); errDelete != nil { + if errDelete := DeleteRepositoryDirectly(ctx, doer, rollbackRepo.ID); errDelete != nil { log.Error("Rollback deleteRepository: %v", errDelete) } } diff --git a/services/repository/create_test.go b/services/repository/create_test.go index a9827c688..b3e1f0550 100644 --- a/services/repository/create_test.go +++ b/services/repository/create_test.go @@ -129,7 +129,7 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) { } // Remove repo and check teams repositories. - assert.NoError(t, DeleteRepositoryDirectly(db.DefaultContext, user, org.ID, repoIds[0]), "DeleteRepository") + assert.NoError(t, DeleteRepositoryDirectly(db.DefaultContext, user, repoIds[0]), "DeleteRepository") teamRepos[0] = repoIds[1:] teamRepos[1] = repoIds[1:] teamRepos[3] = repoIds[1:3] @@ -141,7 +141,7 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) { // Wipe created items. for i, rid := range repoIds { if i > 0 { // first repo already deleted. - assert.NoError(t, DeleteRepositoryDirectly(db.DefaultContext, user, org.ID, rid), "DeleteRepository %d", i) + assert.NoError(t, DeleteRepositoryDirectly(db.DefaultContext, user, rid), "DeleteRepository %d", i) } } assert.NoError(t, organization.DeleteOrganization(db.DefaultContext, org), "DeleteOrganization") diff --git a/services/repository/delete.go b/services/repository/delete.go index f3bf91af4..b7fe4282b 100644 --- a/services/repository/delete.go +++ b/services/repository/delete.go @@ -33,7 +33,7 @@ import ( // DeleteRepository deletes a repository for a user or organization. // make sure if you call this func to close open sessions (sqlite will otherwise get a deadlock) -func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, uid, repoID int64) error { +func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID int64) error { ctx, committer, err := db.TxContext(ctx) if err != nil { return err @@ -41,6 +41,18 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, uid, r defer committer.Close() sess := db.GetEngine(ctx) + repo := &repo_model.Repository{} + has, err := sess.ID(repoID).Get(repo) + if err != nil { + return err + } else if !has { + return repo_model.ErrRepoNotExist{ + ID: repoID, + OwnerName: "", + Name: "", + } + } + // Query the action tasks of this repo, they will be needed after they have been deleted to remove the logs tasks, err := actions_model.FindTasks(ctx, actions_model.FindTaskOptions{RepoID: repoID}) if err != nil { @@ -54,24 +66,11 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, uid, r } // In case is a organization. - org, err := user_model.GetUserByID(ctx, uid) + org, err := user_model.GetUserByID(ctx, repo.OwnerID) if err != nil { return err } - repo := &repo_model.Repository{OwnerID: uid} - has, err := sess.ID(repoID).Get(repo) - if err != nil { - return err - } else if !has { - return repo_model.ErrRepoNotExist{ - ID: repoID, - UID: uid, - OwnerName: "", - Name: "", - } - } - // Delete Deploy Keys deployKeys, err := asymkey_model.ListDeployKeys(ctx, &asymkey_model.ListDeployKeysOptions{RepoID: repoID}) if err != nil { @@ -89,7 +88,6 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, uid, r } else if cnt != 1 { return repo_model.ErrRepoNotExist{ ID: repoID, - UID: uid, OwnerName: "", Name: "", } @@ -192,7 +190,7 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, uid, r } } - if _, err := db.Exec(ctx, "UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", uid); err != nil { + if _, err := db.Exec(ctx, "UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", repo.OwnerID); err != nil { return err } diff --git a/services/repository/repository.go b/services/repository/repository.go index fb52980bb..d28200c0a 100644 --- a/services/repository/repository.go +++ b/services/repository/repository.go @@ -62,7 +62,7 @@ func DeleteRepository(ctx context.Context, doer *user_model.User, repo *repo_mod notify_service.DeleteRepository(ctx, doer, repo) } - if err := DeleteRepositoryDirectly(ctx, doer, repo.OwnerID, repo.ID); err != nil { + if err := DeleteRepositoryDirectly(ctx, doer, repo.ID); err != nil { return err } diff --git a/services/user/user.go b/services/user/user.go index b754b0aac..280696963 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -175,7 +175,7 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error { break } for _, repo := range repos { - if err := repo_service.DeleteRepositoryDirectly(ctx, u, u.ID, repo.ID); err != nil { + 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) } } diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index d1d669616..fa159c6c5 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -541,7 +541,7 @@ func TestAPIRepoTransfer(t *testing.T) { // cleanup repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID}) - _ = repo_service.DeleteRepositoryDirectly(db.DefaultContext, user, repo.OwnerID, repo.ID) + _ = repo_service.DeleteRepositoryDirectly(db.DefaultContext, user, repo.ID) } func transfer(t *testing.T) *repo_model.Repository {