From ec5677b7a2f56e124eaaa53291e21d6260354c12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B5=B5=E6=99=BA=E8=B6=85?= <1012112796@qq.com> Date: Wed, 16 Sep 2020 07:49:34 +0800 Subject: [PATCH] Simplify CheckUnitUser logic (#12854) if check one user's unit in different repos, it's not necessary to get user data every time. Signed-off-by: a1012112796 <1012112796@qq.com> Co-authored-by: techknowlogick --- models/notification.go | 12 ++++++++++-- models/repo.go | 13 +++++-------- routers/home.go | 4 +--- routers/repo/repo.go | 2 +- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/models/notification.go b/models/notification.go index 9258b68f22..80d837078a 100644 --- a/models/notification.go +++ b/models/notification.go @@ -199,10 +199,18 @@ func createOrUpdateIssueNotifications(e Engine, issueID, commentID, notification // notify for userID := range toNotify { issue.Repo.Units = nil - if issue.IsPull && !issue.Repo.checkUnitUser(e, userID, false, UnitTypePullRequests) { + user, err := getUserByID(e, userID) + if err != nil { + if IsErrUserNotExist(err) { + continue + } + + return err + } + if issue.IsPull && !issue.Repo.checkUnitUser(e, user, UnitTypePullRequests) { continue } - if !issue.IsPull && !issue.Repo.checkUnitUser(e, userID, false, UnitTypeIssues) { + if !issue.IsPull && !issue.Repo.checkUnitUser(e, user, UnitTypeIssues) { continue } diff --git a/models/repo.go b/models/repo.go index d9b6576976..2bbc74f439 100644 --- a/models/repo.go +++ b/models/repo.go @@ -425,20 +425,17 @@ func (repo *Repository) getUnits(e Engine) (err error) { } // CheckUnitUser check whether user could visit the unit of this repository -func (repo *Repository) CheckUnitUser(userID int64, isAdmin bool, unitType UnitType) bool { - return repo.checkUnitUser(x, userID, isAdmin, unitType) +func (repo *Repository) CheckUnitUser(user *User, unitType UnitType) bool { + return repo.checkUnitUser(x, user, unitType) } -func (repo *Repository) checkUnitUser(e Engine, userID int64, isAdmin bool, unitType UnitType) bool { - if isAdmin { +func (repo *Repository) checkUnitUser(e Engine, user *User, unitType UnitType) bool { + if user.IsAdmin { return true } - user, err := getUserByID(e, userID) - if err != nil { - return false - } perm, err := getUserRepoPermission(e, repo, user) if err != nil { + log.Error("getUserRepoPermission(): %v", err) return false } diff --git a/routers/home.go b/routers/home.go index 5425670878..d37bf8e31b 100644 --- a/routers/home.go +++ b/routers/home.go @@ -303,10 +303,8 @@ func ExploreCode(ctx *context.Context) { repoIDs []int64 err error isAdmin bool - userID int64 ) if ctx.User != nil { - userID = ctx.User.ID isAdmin = ctx.User.IsAdmin } @@ -336,7 +334,7 @@ func ExploreCode(ctx *context.Context) { var rightRepoMap = make(map[int64]*models.Repository, len(repoMaps)) repoIDs = make([]int64, 0, len(repoMaps)) for id, repo := range repoMaps { - if repo.CheckUnitUser(userID, isAdmin, models.UnitTypeCode) { + if repo.CheckUnitUser(ctx.User, models.UnitTypeCode) { rightRepoMap[id] = repo repoIDs = append(repoIDs, id) } diff --git a/routers/repo/repo.go b/routers/repo/repo.go index 9a4fbfa130..d12640dd6d 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -140,7 +140,7 @@ func Create(ctx *context.Context) { templateID := ctx.QueryInt64("template_id") if templateID > 0 { templateRepo, err := models.GetRepositoryByID(templateID) - if err == nil && templateRepo.CheckUnitUser(ctxUser.ID, ctxUser.IsAdmin, models.UnitTypeCode) { + if err == nil && templateRepo.CheckUnitUser(ctxUser, models.UnitTypeCode) { ctx.Data["repo_template"] = templateID ctx.Data["repo_template_name"] = templateRepo.Name }