From 1645d4a5d8def3cc5451e068aa0a321e028a889b Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Thu, 18 Jun 2020 01:50:11 +0800
Subject: [PATCH] Use ID or Where to instead directly use Get when load object
 from database (#11925)

* Use ID or Where to instead directly use Get when load object from database

* Apply suggestions from code review

Co-authored-by: 6543 <6543@obermui.de>

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: zeripath <art27@cantab.net>
---
 models/attachment.go   |  9 ++++-----
 models/branches.go     | 10 +++++-----
 models/issue_label.go  |  6 ++----
 models/login_source.go |  2 +-
 models/twofactor.go    |  4 ++--
 models/upload.go       |  4 ++--
 models/user.go         |  4 ++--
 models/user_mail.go    | 10 +++++-----
 models/user_openid.go  |  4 ++--
 9 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/models/attachment.go b/models/attachment.go
index 7e59c7eef4..a5f264b32b 100644
--- a/models/attachment.go
+++ b/models/attachment.go
@@ -136,9 +136,8 @@ func GetAttachmentByID(id int64) (*Attachment, error) {
 }
 
 func getAttachmentByID(e Engine, id int64) (*Attachment, error) {
-	attach := &Attachment{ID: id}
-
-	if has, err := e.Get(attach); err != nil {
+	attach := &Attachment{}
+	if has, err := e.ID(id).Get(attach); err != nil {
 		return nil, err
 	} else if !has {
 		return nil, ErrAttachmentNotExist{ID: id, UUID: ""}
@@ -147,8 +146,8 @@ func getAttachmentByID(e Engine, id int64) (*Attachment, error) {
 }
 
 func getAttachmentByUUID(e Engine, uuid string) (*Attachment, error) {
-	attach := &Attachment{UUID: uuid}
-	has, err := e.Get(attach)
+	attach := &Attachment{}
+	has, err := e.Where("uuid=?", uuid).Get(attach)
 	if err != nil {
 		return nil, err
 	} else if !has {
diff --git a/models/branches.go b/models/branches.go
index 688e2d7f51..fc3c783b3a 100644
--- a/models/branches.go
+++ b/models/branches.go
@@ -240,8 +240,8 @@ func getProtectedBranchBy(e Engine, repoID int64, branchName string) (*Protected
 
 // GetProtectedBranchByID getting protected branch by ID
 func GetProtectedBranchByID(id int64) (*ProtectedBranch, error) {
-	rel := &ProtectedBranch{ID: id}
-	has, err := x.Get(rel)
+	rel := &ProtectedBranch{}
+	has, err := x.ID(id).Get(rel)
 	if err != nil {
 		return nil, err
 	}
@@ -509,9 +509,9 @@ func (repo *Repository) GetDeletedBranches() ([]*DeletedBranch, error) {
 }
 
 // GetDeletedBranchByID get a deleted branch by its ID
-func (repo *Repository) GetDeletedBranchByID(ID int64) (*DeletedBranch, error) {
-	deletedBranch := &DeletedBranch{ID: ID}
-	has, err := x.Get(deletedBranch)
+func (repo *Repository) GetDeletedBranchByID(id int64) (*DeletedBranch, error) {
+	deletedBranch := &DeletedBranch{}
+	has, err := x.ID(id).Get(deletedBranch)
 	if err != nil {
 		return nil, err
 	}
diff --git a/models/issue_label.go b/models/issue_label.go
index aa2c8de009..2b519ee71d 100644
--- a/models/issue_label.go
+++ b/models/issue_label.go
@@ -295,10 +295,8 @@ func getLabelByID(e Engine, labelID int64) (*Label, error) {
 		return nil, ErrLabelNotExist{labelID}
 	}
 
-	l := &Label{
-		ID: labelID,
-	}
-	has, err := e.Get(l)
+	l := &Label{}
+	has, err := e.ID(labelID).Get(l)
 	if err != nil {
 		return nil, err
 	} else if !has {
diff --git a/models/login_source.go b/models/login_source.go
index 5350446233..1de24a9cf7 100644
--- a/models/login_source.go
+++ b/models/login_source.go
@@ -300,7 +300,7 @@ func (source *LoginSource) SSPI() *SSPIConfig {
 // CreateLoginSource inserts a LoginSource in the DB if not already
 // existing with the given name.
 func CreateLoginSource(source *LoginSource) error {
-	has, err := x.Get(&LoginSource{Name: source.Name})
+	has, err := x.Where("name=?", source.Name).Exist(new(LoginSource))
 	if err != nil {
 		return err
 	} else if has {
diff --git a/models/twofactor.go b/models/twofactor.go
index 00a465353a..888c910b94 100644
--- a/models/twofactor.go
+++ b/models/twofactor.go
@@ -142,8 +142,8 @@ func UpdateTwoFactor(t *TwoFactor) error {
 // GetTwoFactorByUID returns the two-factor authentication token associated with
 // the user, if any.
 func GetTwoFactorByUID(uid int64) (*TwoFactor, error) {
-	twofa := &TwoFactor{UID: uid}
-	has, err := x.Get(twofa)
+	twofa := &TwoFactor{}
+	has, err := x.Where("uid=?", uid).Get(twofa)
 	if err != nil {
 		return nil, err
 	} else if !has {
diff --git a/models/upload.go b/models/upload.go
index 65b3220c12..c3719368f3 100644
--- a/models/upload.go
+++ b/models/upload.go
@@ -76,8 +76,8 @@ func NewUpload(name string, buf []byte, file multipart.File) (_ *Upload, err err
 
 // GetUploadByUUID returns the Upload by UUID
 func GetUploadByUUID(uuid string) (*Upload, error) {
-	upload := &Upload{UUID: uuid}
-	has, err := x.Get(upload)
+	upload := &Upload{}
+	has, err := x.Where("uuid=?", uuid).Get(upload)
 	if err != nil {
 		return nil, err
 	} else if !has {
diff --git a/models/user.go b/models/user.go
index 8056e30cea..4597508fb1 100644
--- a/models/user.go
+++ b/models/user.go
@@ -1559,8 +1559,8 @@ func GetUserByEmailContext(ctx DBContext, email string) (*User, error) {
 	// Finally, if email address is the protected email address:
 	if strings.HasSuffix(email, fmt.Sprintf("@%s", setting.Service.NoReplyAddress)) {
 		username := strings.TrimSuffix(email, fmt.Sprintf("@%s", setting.Service.NoReplyAddress))
-		user := &User{LowerName: username}
-		has, err := ctx.e.Get(user)
+		user := &User{}
+		has, err := ctx.e.Where("lower_name=?", username).Get(user)
 		if err != nil {
 			return nil, err
 		}
diff --git a/models/user_mail.go b/models/user_mail.go
index af9602e714..60354e23ff 100644
--- a/models/user_mail.go
+++ b/models/user_mail.go
@@ -71,8 +71,8 @@ func GetEmailAddresses(uid int64) ([]*EmailAddress, error) {
 // GetEmailAddressByID gets a user's email address by ID
 func GetEmailAddressByID(uid, id int64) (*EmailAddress, error) {
 	// User ID is required for security reasons
-	email := &EmailAddress{ID: id, UID: uid}
-	if has, err := x.Get(email); err != nil {
+	email := &EmailAddress{UID: uid}
+	if has, err := x.ID(id).Get(email); err != nil {
 		return nil, err
 	} else if !has {
 		return nil, nil
@@ -126,7 +126,7 @@ func isEmailUsed(e Engine, email string) (bool, error) {
 		return true, nil
 	}
 
-	return e.Get(&EmailAddress{Email: email})
+	return e.Where("email=?", email).Get(&EmailAddress{})
 }
 
 // IsEmailUsed returns true if the email has been used.
@@ -251,8 +251,8 @@ func MakeEmailPrimary(email *EmailAddress) error {
 		return ErrEmailNotActivated
 	}
 
-	user := &User{ID: email.UID}
-	has, err = x.Get(user)
+	user := &User{}
+	has, err = x.ID(email.UID).Get(user)
 	if err != nil {
 		return err
 	} else if !has {
diff --git a/models/user_openid.go b/models/user_openid.go
index 49edc1db21..503c2a52d1 100644
--- a/models/user_openid.go
+++ b/models/user_openid.go
@@ -111,8 +111,8 @@ func GetUserByOpenID(uri string) (*User, error) {
 	log.Trace("Normalized OpenID URI: " + uri)
 
 	// Otherwise, check in openid table
-	oid := &UserOpenID{URI: uri}
-	has, err := x.Get(oid)
+	oid := &UserOpenID{}
+	has, err := x.Where("uri=?", uri).Get(oid)
 	if err != nil {
 		return nil, err
 	}