Reduce usage of allcols on update (#2596)

* reduce usage of allcols on update

* fix bug and tests

diff --git a/models/issue_milestone.go b/models/issue_milestone.go
index d12c309b8777b1d09b32b43b63be6e59ea2bee4d..0001da90ef691446275b35fee05941bfc99409e9 100644 (file)
--- 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 11d47dd0a666093ffe14383d24c0023a094b5121..e0110e6dc51b0e9798e1e71d782299a4a7830f23 100644 (file)
--- 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 eaf7ba602c2bbfd835bbdc530ec72730d8de6867..82a56cac18331febef2eecc2d2968ff690703491 100644 (file)
--- 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 acddc70b5857bd71e71b00a59a6ee0118092fc1f..a9b4fad05bc345c0a15e05db3427b5fea7dc1ec1 100644 (file)
--- 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 8754c119f10629a3fc33d61d0390d73be26f560a..31e1af7e8c293d5ec812ebde2a2b3ef0bb4dc9d4 100644 (file)
--- 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 cdaf9439442780947e43cf3cd50477767201d6bd..22a3a83229d11010e0dfd66ca66d76d15be0aab1 100644 (file)
--- 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 791f022a3f50444e1a2e3c52fa8667342c8144e1..0448149e6a52ba745c29934b73e02769e94be2de 100644 (file)
--- 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 539c60e696a54eb2538e17157f2522a92a338bd8..c8f65ef0d178d393063ed3b2976acacc32beaa3b 100644 (file)
--- 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 9adc5bd4e1e9746cdd17d52537bb23466b874eb6..dbc15ae68bda940b3f630f80725a597e8c941bb3 100644 (file)
--- 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 285ba74f61b2f1968eb4a0c2a7dcdde68cb6a347..ba69d9a470954e076974af9b80e5a367bf9c6135 100644 (file)
--- 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
        }