From dd55534b82ca9527b68fddf0f63ebb13c105c466 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 25 Sep 2017 12:59:27 +0800 Subject: [PATCH] Reduce usage of allcols on update (#2596) * reduce usage of allcols on update * fix bug and tests --- models/issue_milestone.go | 4 ++-- models/issue_user.go | 2 +- models/org.go | 2 +- models/org_team.go | 10 +++++----- models/pull.go | 2 +- models/repo.go | 2 +- models/repo_collaboration.go | 2 +- models/ssh_key.go | 10 +--------- models/user.go | 6 +++++- models/user_mail.go | 6 +++--- 10 files changed, 21 insertions(+), 25 deletions(-) diff --git a/models/issue_milestone.go b/models/issue_milestone.go index d12c309b8..0001da90e 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -227,7 +227,7 @@ func ChangeMilestoneStatus(m *Milestone, isClosed bool) (err error) { repo.NumMilestones = int(countRepoMilestones(sess, repo.ID)) repo.NumClosedMilestones = int(countRepoClosedMilestones(sess, repo.ID)) - if _, err = sess.Id(repo.ID).AllCols().Update(repo); err != nil { + if _, err = sess.Id(repo.ID).Cols("num_milestones, num_closed_milestones").Update(repo); err != nil { return err } return sess.Commit() @@ -341,7 +341,7 @@ func DeleteMilestoneByRepoID(repoID, id int64) error { repo.NumMilestones = int(countRepoMilestones(sess, repo.ID)) repo.NumClosedMilestones = int(countRepoClosedMilestones(sess, repo.ID)) - if _, err = sess.Id(repo.ID).AllCols().Update(repo); err != nil { + if _, err = sess.Id(repo.ID).Cols("num_milestones, num_closed_milestones").Update(repo); err != nil { return err } diff --git a/models/issue_user.go b/models/issue_user.go index 11d47dd0a..e0110e6dc 100644 --- a/models/issue_user.go +++ b/models/issue_user.go @@ -101,7 +101,7 @@ func UpdateIssueUsersByMentions(e Engine, issueID int64, uids []int64) error { iu.IsMentioned = true if has { - _, err = e.Id(iu.ID).AllCols().Update(iu) + _, err = e.Id(iu.ID).Cols("is_mentioned").Update(iu) } else { _, err = e.Insert(iu) } diff --git a/models/org.go b/models/org.go index eaf7ba602..82a56cac1 100644 --- a/models/org.go +++ b/models/org.go @@ -412,7 +412,7 @@ func ChangeOrgUserStatus(orgID, uid int64, public bool) error { } ou.IsPublic = public - _, err = x.Id(ou.ID).AllCols().Update(ou) + _, err = x.Id(ou.ID).Cols("is_public").Update(ou) return err } diff --git a/models/org_team.go b/models/org_team.go index acddc70b5..a9b4fad05 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -96,7 +96,7 @@ func (t *Team) addRepository(e Engine, repo *Repository) (err error) { } t.NumRepos++ - if _, err = e.Id(t.ID).AllCols().Update(t); err != nil { + if _, err = e.Id(t.ID).Cols("num_repos").Update(t); err != nil { return fmt.Errorf("update team: %v", err) } @@ -142,7 +142,7 @@ func (t *Team) removeRepository(e Engine, repo *Repository, recalculate bool) (e } t.NumRepos-- - if _, err = e.Id(t.ID).AllCols().Update(t); err != nil { + if _, err = e.Id(t.ID).Cols("num_repos").Update(t); err != nil { return err } @@ -521,7 +521,7 @@ func AddTeamMember(team *Team, userID int64) error { if team.IsOwnerTeam() { ou.IsOwner = true } - if _, err := sess.Id(ou.ID).AllCols().Update(ou); err != nil { + if _, err := sess.Id(ou.ID).Cols("num_teams, is_owner").Update(ou); err != nil { return err } @@ -552,7 +552,7 @@ func removeTeamMember(e Engine, team *Team, userID int64) error { return err } else if _, err = e. Id(team.ID). - AllCols(). + Cols("num_members"). Update(team); err != nil { return err } @@ -579,7 +579,7 @@ func removeTeamMember(e Engine, team *Team, userID int64) error { } if _, err = e. Id(ou.ID). - AllCols(). + Cols("num_teams"). Update(ou); err != nil { return err } diff --git a/models/pull.go b/models/pull.go index 8754c119f..31e1af7e8 100644 --- a/models/pull.go +++ b/models/pull.go @@ -425,7 +425,7 @@ func (pr *PullRequest) setMerged() (err error) { if err = pr.Issue.changeStatus(sess, pr.Merger, pr.Issue.Repo, true); err != nil { return fmt.Errorf("Issue.changeStatus: %v", err) } - if _, err = sess.Id(pr.ID).AllCols().Update(pr); err != nil { + if _, err = sess.Id(pr.ID).Cols("has_merged").Update(pr); err != nil { return fmt.Errorf("update pull request: %v", err) } diff --git a/models/repo.go b/models/repo.go index cdaf94394..22a3a8322 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1458,7 +1458,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error } t.NumRepos-- - if _, err := sess.Id(t.ID).AllCols().Update(t); err != nil { + if _, err := sess.Id(t.ID).Cols("num_repos").Update(t); err != nil { return fmt.Errorf("decrease team repository count '%d': %v", t.ID, err) } } diff --git a/models/repo_collaboration.go b/models/repo_collaboration.go index 791f022a3..0448149e6 100644 --- a/models/repo_collaboration.go +++ b/models/repo_collaboration.go @@ -143,7 +143,7 @@ func (repo *Repository) ChangeCollaborationAccessMode(uid int64, mode AccessMode if _, err = sess. Id(collaboration.ID). - AllCols(). + Cols("mode"). Update(collaboration); err != nil { return fmt.Errorf("update collaboration: %v", err) } else if _, err = sess.Exec("UPDATE access SET mode = ? WHERE user_id = ? AND repo_id = ?", mode, uid, repo.ID); err != nil { diff --git a/models/ssh_key.go b/models/ssh_key.go index 539c60e69..c8f65ef0d 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -477,15 +477,8 @@ func ListPublicKeys(uid int64) ([]*PublicKey, error) { Find(&keys) } -// UpdatePublicKey updates given public key. -func UpdatePublicKey(key *PublicKey) error { - _, err := x.Id(key.ID).AllCols().Update(key) - return err -} - // UpdatePublicKeyUpdated updates public key use time. func UpdatePublicKeyUpdated(id int64) error { - now := time.Now() // Check if key exists before update as affected rows count is unreliable // and will return 0 affected rows if two updates are made at the same time if cnt, err := x.ID(id).Count(&PublicKey{}); err != nil { @@ -495,8 +488,7 @@ func UpdatePublicKeyUpdated(id int64) error { } _, err := x.ID(id).Cols("updated_unix").Update(&PublicKey{ - Updated: now, - UpdatedUnix: now.Unix(), + UpdatedUnix: time.Now().Unix(), }) if err != nil { return err diff --git a/models/user.go b/models/user.go index 9adc5bd4e..dbc15ae68 100644 --- a/models/user.go +++ b/models/user.go @@ -874,6 +874,10 @@ func UpdateUser(u *User) error { // UpdateUserCols update user according special columns func UpdateUserCols(u *User, cols ...string) error { + return updateUserCols(x, u, cols...) +} + +func updateUserCols(e Engine, u *User, cols ...string) error { // Organization does not need email u.Email = strings.ToLower(u.Email) if !u.IsOrganization() { @@ -890,7 +894,7 @@ func UpdateUserCols(u *User, cols ...string) error { u.Website = base.TruncateString(u.Website, 255) u.Description = base.TruncateString(u.Description, 255) - _, err := x.Id(u.ID).Cols(cols...).Update(u) + _, err := e.Id(u.ID).Cols(cols...).Update(u) return err } diff --git a/models/user_mail.go b/models/user_mail.go index 285ba74f6..ba69d9a47 100644 --- a/models/user_mail.go +++ b/models/user_mail.go @@ -135,10 +135,10 @@ func (email *EmailAddress) Activate() error { email.IsActivated = true if _, err := sess. Id(email.ID). - AllCols(). + Cols("is_activated"). Update(email); err != nil { return err - } else if err = updateUser(sess, user); err != nil { + } else if err = updateUserCols(sess, user, "rands"); err != nil { return err } @@ -222,7 +222,7 @@ func MakeEmailPrimary(email *EmailAddress) error { } user.Email = email.Email - if _, err = sess.Id(user.ID).AllCols().Update(user); err != nil { + if _, err = sess.Id(user.ID).Cols("email").Update(user); err != nil { return err }