From 9e85358777ea4bd345f832d08b19915a72cc128b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 26 Oct 2019 14:54:11 +0800 Subject: [PATCH] Move some repositories' operations to a standalone service package (#8557) * Move some repositories' operations to a standalone service package * improve code * remove unused codes * add rollback when fork failed * add repo when return --- models/repo.go | 91 ++++++++++--------------- modules/notification/indexer/indexer.go | 1 + modules/notification/webhook/webhook.go | 64 +++++++++++++++++ routers/admin/repos.go | 3 +- routers/api/v1/repo/fork.go | 5 +- routers/api/v1/repo/repo.go | 30 +++----- routers/repo/pull.go | 3 +- routers/repo/repo.go | 12 +--- routers/repo/setting.go | 3 +- services/repository/repository.go | 56 +++++++++++++++ 10 files changed, 179 insertions(+), 89 deletions(-) create mode 100644 services/repository/repository.go diff --git a/models/repo.go b/models/repo.go index 3b5395ce164..ac3443f8953 100644 --- a/models/repo.go +++ b/models/repo.go @@ -742,6 +742,28 @@ func (repo *Repository) CanUserFork(user *User) (bool, error) { return false, nil } +// CanUserDelete returns true if user could delete the repository +func (repo *Repository) CanUserDelete(user *User) (bool, error) { + if user.IsAdmin || user.ID == repo.OwnerID { + return true, nil + } + + if err := repo.GetOwner(); err != nil { + return false, err + } + + if repo.Owner.IsOrganization() { + isOwner, err := repo.Owner.IsOwnedBy(user.ID) + if err != nil { + return false, err + } else if isOwner { + return true, nil + } + } + + return false, nil +} + // CanEnablePulls returns true if repository meets the requirements of accepting pulls. func (repo *Repository) CanEnablePulls() bool { return !repo.IsMirror && !repo.IsEmpty @@ -1430,15 +1452,9 @@ func createRepository(e *xorm.Session, doer, u *User, repo *Repository) (err err t, err := u.getOwnerTeam(e) if err != nil { return fmt.Errorf("getOwnerTeam: %v", err) - } else if err = t.addRepository(e, repo); err != nil { + } + if err = t.addRepository(e, repo); err != nil { return fmt.Errorf("addRepository: %v", err) - } else if err = prepareWebhooks(e, repo, HookEventRepository, &api.RepositoryPayload{ - Action: api.HookRepoCreated, - Repository: repo.innerAPIFormat(e, AccessModeOwner, false), - Organization: u.APIFormat(), - Sender: doer.APIFormat(), - }); err != nil { - return fmt.Errorf("prepareWebhooks: %v", err) } } else if err = repo.recalculateAccesses(e); err != nil { // Organization automatically called this in addRepository method. @@ -1522,11 +1538,6 @@ func CreateRepository(doer, u *User, opts CreateRepoOptions) (_ *Repository, err return nil, err } - // Add to hook queue for created repo after session commit. - if u.IsOrganization() { - go HookQueue.Add(repo.ID) - } - return repo, err } @@ -2044,18 +2055,6 @@ func DeleteRepository(doer *User, uid, repoID int64) error { return fmt.Errorf("Commit: %v", err) } - if org.IsOrganization() { - if err = PrepareWebhooks(repo, HookEventRepository, &api.RepositoryPayload{ - Action: api.HookRepoDeleted, - Repository: repo.APIFormat(AccessModeOwner), - Organization: org.APIFormat(), - Sender: doer.APIFormat(), - }); err != nil { - return err - } - go HookQueue.Add(repo.ID) - } - if len(repo.Avatar) > 0 { avatarPath := repo.CustomAvatarPath() if com.IsExist(avatarPath) { @@ -2065,7 +2064,6 @@ func DeleteRepository(doer *User, uid, repoID int64) error { } } - DeleteRepoFromIndexer(repo) return nil } @@ -2521,22 +2519,22 @@ func HasForkedRepo(ownerID, repoID int64) (*Repository, bool) { } // ForkRepository forks a repository -func ForkRepository(doer, u *User, oldRepo *Repository, name, desc string) (_ *Repository, err error) { - forkedRepo, err := oldRepo.GetUserFork(u.ID) +func ForkRepository(doer, owner *User, oldRepo *Repository, name, desc string) (_ *Repository, err error) { + forkedRepo, err := oldRepo.GetUserFork(owner.ID) if err != nil { return nil, err } if forkedRepo != nil { return nil, ErrForkAlreadyExist{ - Uname: u.Name, + Uname: owner.Name, RepoName: oldRepo.FullName(), ForkName: forkedRepo.FullName(), } } repo := &Repository{ - OwnerID: u.ID, - Owner: u, + OwnerID: owner.ID, + Owner: owner, Name: name, LowerName: strings.ToLower(name), Description: desc, @@ -2553,7 +2551,7 @@ func ForkRepository(doer, u *User, oldRepo *Repository, name, desc string) (_ *R return nil, err } - if err = createRepository(sess, doer, u, repo); err != nil { + if err = createRepository(sess, doer, owner, repo); err != nil { return nil, err } @@ -2561,9 +2559,9 @@ func ForkRepository(doer, u *User, oldRepo *Repository, name, desc string) (_ *R return nil, err } - repoPath := RepoPath(u.Name, repo.Name) + repoPath := RepoPath(owner.Name, repo.Name) _, stderr, err := process.GetManager().ExecTimeout(10*time.Minute, - fmt.Sprintf("ForkRepository(git clone): %s/%s", u.Name, repo.Name), + fmt.Sprintf("ForkRepository(git clone): %s/%s", owner.Name, repo.Name), git.GitExecutable, "clone", "--bare", oldRepo.repoPath(sess), repoPath) if err != nil { return nil, fmt.Errorf("git clone: %v", stderr) @@ -2586,24 +2584,6 @@ func ForkRepository(doer, u *User, oldRepo *Repository, name, desc string) (_ *R return nil, err } - oldMode, _ := AccessLevel(doer, oldRepo) - mode, _ := AccessLevel(doer, repo) - - if err = PrepareWebhooks(oldRepo, HookEventFork, &api.ForkPayload{ - Forkee: oldRepo.APIFormat(oldMode), - Repo: repo.APIFormat(mode), - Sender: doer.APIFormat(), - }); err != nil { - log.Error("PrepareWebhooks [repo_id: %d]: %v", oldRepo.ID, err) - } else { - go HookQueue.Add(oldRepo.ID) - } - - // Add to hook queue for created repo after session commit. - if u.IsOrganization() { - go HookQueue.Add(repo.ID) - } - if err = repo.UpdateSize(); err != nil { log.Error("Failed to update size for repository: %v", err) } @@ -2612,20 +2592,19 @@ func ForkRepository(doer, u *User, oldRepo *Repository, name, desc string) (_ *R sess2 := x.NewSession() defer sess2.Close() if err = sess2.Begin(); err != nil { - return nil, err + return repo, err } var lfsObjects []*LFSMetaObject - if err = sess2.Where("repository_id=?", oldRepo.ID).Find(&lfsObjects); err != nil { - return nil, err + return repo, err } for _, v := range lfsObjects { v.ID = 0 v.RepositoryID = repo.ID if _, err = sess2.Insert(v); err != nil { - return nil, err + return repo, err } } diff --git a/modules/notification/indexer/indexer.go b/modules/notification/indexer/indexer.go index 66614b2c20c..453eb0c295e 100644 --- a/modules/notification/indexer/indexer.go +++ b/modules/notification/indexer/indexer.go @@ -98,6 +98,7 @@ func (r *indexerNotifier) NotifyDeleteComment(doer *models.User, comment *models func (r *indexerNotifier) NotifyDeleteRepository(doer *models.User, repo *models.Repository) { issue_indexer.DeleteRepoIssueIndexer(repo) + models.DeleteRepoFromIndexer(repo) } func (r *indexerNotifier) NotifyIssueChangeContent(doer *models.User, issue *models.Issue, oldContent string) { diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 33adfaa739b..13f2f4486a5 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -65,3 +65,67 @@ func (m *webhookNotifier) NotifyIssueClearLabels(doer *models.User, issue *model go models.HookQueue.Add(issue.RepoID) } } + +func (m *webhookNotifier) NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) { + oldMode, _ := models.AccessLevel(doer, oldRepo) + mode, _ := models.AccessLevel(doer, repo) + + // forked webhook + if err := models.PrepareWebhooks(oldRepo, models.HookEventFork, &api.ForkPayload{ + Forkee: oldRepo.APIFormat(oldMode), + Repo: repo.APIFormat(mode), + Sender: doer.APIFormat(), + }); err != nil { + log.Error("PrepareWebhooks [repo_id: %d]: %v", oldRepo.ID, err) + } else { + go models.HookQueue.Add(oldRepo.ID) + } + + u := repo.MustOwner() + + // Add to hook queue for created repo after session commit. + if u.IsOrganization() { + if err := models.PrepareWebhooks(repo, models.HookEventRepository, &api.RepositoryPayload{ + Action: api.HookRepoCreated, + Repository: repo.APIFormat(models.AccessModeOwner), + Organization: u.APIFormat(), + Sender: doer.APIFormat(), + }); err != nil { + log.Error("PrepareWebhooks [repo_id: %d]: %v", repo.ID, err) + } else { + go models.HookQueue.Add(repo.ID) + } + } +} + +func (m *webhookNotifier) NotifyCreateRepository(doer *models.User, u *models.User, repo *models.Repository) { + // Add to hook queue for created repo after session commit. + if u.IsOrganization() { + if err := models.PrepareWebhooks(repo, models.HookEventRepository, &api.RepositoryPayload{ + Action: api.HookRepoCreated, + Repository: repo.APIFormat(models.AccessModeOwner), + Organization: u.APIFormat(), + Sender: doer.APIFormat(), + }); err != nil { + log.Error("PrepareWebhooks [repo_id: %d]: %v", repo.ID, err) + } else { + go models.HookQueue.Add(repo.ID) + } + } +} + +func (m *webhookNotifier) NotifyDeleteRepository(doer *models.User, repo *models.Repository) { + u := repo.MustOwner() + + if u.IsOrganization() { + if err := models.PrepareWebhooks(repo, models.HookEventRepository, &api.RepositoryPayload{ + Action: api.HookRepoDeleted, + Repository: repo.APIFormat(models.AccessModeOwner), + Organization: u.APIFormat(), + Sender: doer.APIFormat(), + }); err != nil { + log.Error("PrepareWebhooks [repo_id: %d]: %v", repo.ID, err) + } + go models.HookQueue.Add(repo.ID) + } +} diff --git a/routers/admin/repos.go b/routers/admin/repos.go index d345c8e76ab..73aa66807b3 100644 --- a/routers/admin/repos.go +++ b/routers/admin/repos.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/routers" + repo_service "code.gitea.io/gitea/services/repository" ) const ( @@ -38,7 +39,7 @@ func DeleteRepo(ctx *context.Context) { return } - if err := models.DeleteRepository(ctx.User, repo.MustOwner().ID, repo.ID); err != nil { + if err := repo_service.DeleteRepository(ctx.User, repo); err != nil { ctx.ServerError("DeleteRepository", err) return } diff --git a/routers/api/v1/repo/fork.go b/routers/api/v1/repo/fork.go index 7231daab292..31f2389478c 100644 --- a/routers/api/v1/repo/fork.go +++ b/routers/api/v1/repo/fork.go @@ -8,6 +8,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" api "code.gitea.io/gitea/modules/structs" + repo_service "code.gitea.io/gitea/services/repository" ) // ListForks list a repository's forks @@ -97,10 +98,12 @@ func CreateFork(ctx *context.APIContext, form api.CreateForkOption) { } forker = org } - fork, err := models.ForkRepository(ctx.User, forker, repo, repo.Name, repo.Description) + + fork, err := repo_service.ForkRepository(ctx.User, forker, repo, repo.Name, repo.Description) if err != nil { ctx.Error(500, "ForkRepository", err) return } + ctx.JSON(202, fork.APIFormat(models.AccessModeOwner)) } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index a4417107ee3..422bb0ac3a2 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -24,6 +24,7 @@ import ( "code.gitea.io/gitea/modules/validation" "code.gitea.io/gitea/routers/api/v1/convert" mirror_service "code.gitea.io/gitea/services/mirror" + repo_service "code.gitea.io/gitea/services/repository" ) var searchOrderByMap = map[string]map[string]models.SearchOrderBy{ @@ -207,7 +208,7 @@ func CreateUserRepo(ctx *context.APIContext, owner *models.User, opt api.CreateR if opt.AutoInit && opt.Readme == "" { opt.Readme = "Default" } - repo, err := models.CreateRepository(ctx.User, owner, models.CreateRepoOptions{ + repo, err := repo_service.CreateRepository(ctx.User, owner, models.CreateRepoOptions{ Name: opt.Name, Description: opt.Description, IssueLabels: opt.IssueLabels, @@ -224,18 +225,11 @@ func CreateUserRepo(ctx *context.APIContext, owner *models.User, opt api.CreateR models.IsErrNamePatternNotAllowed(err) { ctx.Error(422, "", err) } else { - if repo != nil { - if err = models.DeleteRepository(ctx.User, ctx.User.ID, repo.ID); err != nil { - log.Error("DeleteRepository: %v", err) - } - } ctx.Error(500, "CreateRepository", err) } return } - notification.NotifyCreateRepository(ctx.User, owner, repo) - ctx.JSON(201, repo.APIFormat(models.AccessModeOwner)) } @@ -433,7 +427,7 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { repo, err := migrations.MigrateRepository(ctx.User, ctxUser.Name, opts) if err == nil { - notification.NotifyCreateRepository(ctx.User, ctxUser, repo) + notification.NotifyMigrateRepository(ctx.User, ctxUser, repo) log.Trace("Repository migrated: %s/%s", ctxUser.Name, form.RepoName) ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin)) @@ -876,18 +870,16 @@ func Delete(ctx *context.APIContext) { owner := ctx.Repo.Owner repo := ctx.Repo.Repository - if owner.IsOrganization() && !ctx.User.IsAdmin { - isOwner, err := owner.IsOwnedBy(ctx.User.ID) - if err != nil { - ctx.Error(500, "IsOwnedBy", err) - return - } else if !isOwner { - ctx.Error(403, "", "Given user is not owner of organization.") - return - } + canDelete, err := repo.CanUserDelete(ctx.User) + if err != nil { + ctx.Error(500, "CanUserDelete", err) + return + } else if !canDelete { + ctx.Error(403, "", "Given user is not owner of organization.") + return } - if err := models.DeleteRepository(ctx.User, owner.ID, repo.ID); err != nil { + if err := repo_service.DeleteRepository(ctx.User, repo); err != nil { ctx.Error(500, "DeleteRepository", err) return } diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 23d97e7b7e2..f3a9aca2a43 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -26,6 +26,7 @@ import ( "code.gitea.io/gitea/services/gitdiff" issue_service "code.gitea.io/gitea/services/issue" pull_service "code.gitea.io/gitea/services/pull" + repo_service "code.gitea.io/gitea/services/repository" "github.com/unknwon/com" ) @@ -209,7 +210,7 @@ func ForkPost(ctx *context.Context, form auth.CreateRepoForm) { } } - repo, err := models.ForkRepository(ctx.User, ctxUser, forkRepo, form.RepoName, form.Description) + repo, err := repo_service.ForkRepository(ctx.User, ctxUser, forkRepo, form.RepoName, form.Description) if err != nil { ctx.Data["Err_RepoName"] = true switch { diff --git a/routers/repo/repo.go b/routers/repo/repo.go index bfd0c771b05..cf1845a727b 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -17,10 +17,10 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/migrations" - "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/task" "code.gitea.io/gitea/modules/util" + repo_service "code.gitea.io/gitea/services/repository" "github.com/unknwon/com" ) @@ -170,7 +170,7 @@ func CreatePost(ctx *context.Context, form auth.CreateRepoForm) { return } - repo, err := models.CreateRepository(ctx.User, ctxUser, models.CreateRepoOptions{ + repo, err := repo_service.CreateRepository(ctx.User, ctxUser, models.CreateRepoOptions{ Name: form.RepoName, Description: form.Description, Gitignores: form.Gitignores, @@ -181,19 +181,11 @@ func CreatePost(ctx *context.Context, form auth.CreateRepoForm) { AutoInit: form.AutoInit, }) if err == nil { - notification.NotifyCreateRepository(ctx.User, ctxUser, repo) - log.Trace("Repository created [%d]: %s/%s", repo.ID, ctxUser.Name, repo.Name) ctx.Redirect(setting.AppSubURL + "/" + ctxUser.Name + "/" + repo.Name) return } - if repo != nil { - if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil { - log.Error("DeleteRepository: %v", errDelete) - } - } - handleCreateError(ctx, ctxUser, err, "CreatePost", tplCreate, &form) } diff --git a/routers/repo/setting.go b/routers/repo/setting.go index c74b273420d..663394fe3db 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -26,6 +26,7 @@ import ( "code.gitea.io/gitea/routers/utils" "code.gitea.io/gitea/services/mailer" mirror_service "code.gitea.io/gitea/services/mirror" + repo_service "code.gitea.io/gitea/services/repository" "github.com/unknwon/com" "mvdan.cc/xurls/v2" @@ -407,7 +408,7 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { return } - if err := models.DeleteRepository(ctx.User, ctx.Repo.Owner.ID, repo.ID); err != nil { + if err := repo_service.DeleteRepository(ctx.User, ctx.Repo.Repository); err != nil { ctx.ServerError("DeleteRepository", err) return } diff --git a/services/repository/repository.go b/services/repository/repository.go new file mode 100644 index 00000000000..5135435c781 --- /dev/null +++ b/services/repository/repository.go @@ -0,0 +1,56 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package repository + +import ( + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/notification" +) + +// CreateRepository creates a repository for the user/organization. +func CreateRepository(doer, owner *models.User, opts models.CreateRepoOptions) (*models.Repository, error) { + repo, err := models.CreateRepository(doer, owner, opts) + if err != nil { + if repo != nil { + if errDelete := models.DeleteRepository(doer, owner.ID, repo.ID); errDelete != nil { + log.Error("Rollback deleteRepository: %v", errDelete) + } + } + return nil, err + } + + notification.NotifyCreateRepository(doer, owner, repo) + + return repo, nil +} + +// ForkRepository forks a repository +func ForkRepository(doer, u *models.User, oldRepo *models.Repository, name, desc string) (*models.Repository, error) { + repo, err := models.ForkRepository(doer, u, oldRepo, name, desc) + if err != nil { + if repo != nil { + if errDelete := models.DeleteRepository(doer, u.ID, repo.ID); errDelete != nil { + log.Error("Rollback deleteRepository: %v", errDelete) + } + } + return nil, err + } + + notification.NotifyForkRepository(doer, oldRepo, repo) + + return repo, nil +} + +// DeleteRepository deletes a repository for a user or organization. +func DeleteRepository(doer *models.User, repo *models.Repository) error { + if err := models.DeleteRepository(doer, repo.OwnerID, repo.ID); err != nil { + return err + } + + notification.NotifyDeleteRepository(doer, repo) + + return nil +}