fixed bug #151 caused Find should be Get (#153)

4 files changed, 29 insertions, 21 deletions

diff --git a/models/error.go b/models/error.go
index 182a944a6f..459954545a 100644
--- a/models/error.go
+++ b/models/error.go
@@ -55,8 +55,9 @@ func (err ErrUserAlreadyExist) Error() string {
 }
 
 type ErrUserNotExist struct {
-	UID  int64
-	Name string
+	UID   int64
+	Name  string
+	KeyID int64
 }
 
 func IsErrUserNotExist(err error) bool {
@@ -65,7 +66,7 @@ func IsErrUserNotExist(err error) bool {
 }
 
 func (err ErrUserNotExist) Error() string {
-	return fmt.Sprintf("user does not exist [uid: %d, name: %s]", err.UID, err.Name)
+	return fmt.Sprintf("user does not exist [uid: %d, name: %s, keyid: %d]", err.UID, err.Name, err.KeyID)
 }
 
 type ErrEmailAlreadyUsed struct {
diff --git a/models/login_source.go b/models/login_source.go
index 0d9226a583..774cb7f4db 100644
--- a/models/login_source.go
+++ b/models/login_source.go
@@ -296,7 +296,7 @@ func LoginViaLDAP(user *User, login, passowrd string, source *LoginSource, autoR
 	username, fn, sn, mail, isAdmin, succeed := source.Cfg.(*LDAPConfig).SearchEntry(login, passowrd, source.Type == LoginDLDAP)
 	if !succeed {
 		// User not in LDAP, do nothing
-		return nil, ErrUserNotExist{0, login}
+		return nil, ErrUserNotExist{0, login, 0}
 	}
 
 	if !autoRegister {
@@ -404,9 +404,9 @@ func LoginViaSMTP(user *User, login, password string, sourceID int64, cfg *SMTPC
 	if len(cfg.AllowedDomains) > 0 {
 		idx := strings.Index(login, "@")
 		if idx == -1 {
-			return nil, ErrUserNotExist{0, login}
+			return nil, ErrUserNotExist{0, login, 0}
 		} else if !com.IsSliceContainsStr(strings.Split(cfg.AllowedDomains, ","), login[idx+1:]) {
-			return nil, ErrUserNotExist{0, login}
+			return nil, ErrUserNotExist{0, login, 0}
 		}
 	}
 
@@ -425,7 +425,7 @@ func LoginViaSMTP(user *User, login, password string, sourceID int64, cfg *SMTPC
 		tperr, ok := err.(*textproto.Error)
 		if (ok && tperr.Code == 535) ||
 			strings.Contains(err.Error(), "Username and Password not accepted") {
-			return nil, ErrUserNotExist{0, login}
+			return nil, ErrUserNotExist{0, login, 0}
 		}
 		return nil, err
 	}
@@ -465,7 +465,7 @@ func LoginViaSMTP(user *User, login, password string, sourceID int64, cfg *SMTPC
 func LoginViaPAM(user *User, login, password string, sourceID int64, cfg *PAMConfig, autoRegister bool) (*User, error) {
 	if err := pam.PAMAuth(cfg.ServiceName, login, password); err != nil {
 		if strings.Contains(err.Error(), "Authentication failure") {
-			return nil, ErrUserNotExist{0, login}
+			return nil, ErrUserNotExist{0, login, 0}
 		}
 		return nil, err
 	}
@@ -525,7 +525,7 @@ func UserSignIn(username, passowrd string) (*User, error) {
 				return user, nil
 			}
 
-			return nil, ErrUserNotExist{user.ID, user.Name}
+			return nil, ErrUserNotExist{user.ID, user.Name, 0}
 
 		default:
 			var source LoginSource
@@ -554,5 +554,5 @@ func UserSignIn(username, passowrd string) (*User, error) {
 		log.Warn("Failed to login '%s' via '%s': %v", username, source.Name, err)
 	}
 
-	return nil, ErrUserNotExist{user.ID, user.Name}
+	return nil, ErrUserNotExist{user.ID, user.Name, 0}
 }
diff --git a/models/user.go b/models/user.go
index 15d8eba0a5..9437d40a52 100644
--- a/models/user.go
+++ b/models/user.go
@@ -890,12 +890,19 @@ func UserPath(userName string) string {
 	return filepath.Join(setting.RepoRootPath, strings.ToLower(userName))
 }
 
+// GetUserByKeyID get user information by user's public key id
 func GetUserByKeyID(keyID int64) (*User, error) {
-	user := new(User)
-	return user, x.
-		Join("INNER", "public_key", "`public_key`.owner_id = `user`.id").
+	var user User
+	has, err := x.Join("INNER", "public_key", "`public_key`.owner_id = `user`.id").
 		Where("`public_key`.id=?", keyID).
-		Find(user)
+		Get(user)
+	if err != nil {
+		return nil, err
+	}
+	if !has {
+		return nil, ErrUserNotExist{0, "", keyID}
+	}
+	return &user, nil
 }
 
 func getUserByID(e Engine, id int64) (*User, error) {
@@ -904,7 +911,7 @@ func getUserByID(e Engine, id int64) (*User, error) {
 	if err != nil {
 		return nil, err
 	} else if !has {
-		return nil, ErrUserNotExist{id, ""}
+		return nil, ErrUserNotExist{id, "", 0}
 	}
 	return u, nil
 }
@@ -920,7 +927,7 @@ func GetAssigneeByID(repo *Repository, userID int64) (*User, error) {
 	if err != nil {
 		return nil, err
 	} else if !has {
-		return nil, ErrUserNotExist{userID, ""}
+		return nil, ErrUserNotExist{userID, "", 0}
 	}
 	return GetUserByID(userID)
 }
@@ -928,14 +935,14 @@ func GetAssigneeByID(repo *Repository, userID int64) (*User, error) {
 // GetUserByName returns user by given name.
 func GetUserByName(name string) (*User, error) {
 	if len(name) == 0 {
-		return nil, ErrUserNotExist{0, name}
+		return nil, ErrUserNotExist{0, name, 0}
 	}
 	u := &User{LowerName: strings.ToLower(name)}
 	has, err := x.Get(u)
 	if err != nil {
 		return nil, err
 	} else if !has {
-		return nil, ErrUserNotExist{0, name}
+		return nil, ErrUserNotExist{0, name, 0}
 	}
 	return u, nil
 }
@@ -1021,7 +1028,7 @@ func ValidateCommitsWithEmails(oldCommits *list.List) *list.List {
 // GetUserByEmail returns the user object by given e-mail if exists.
 func GetUserByEmail(email string) (*User, error) {
 	if len(email) == 0 {
-		return nil, ErrUserNotExist{0, "email"}
+		return nil, ErrUserNotExist{0, email, 0}
 	}
 
 	email = strings.ToLower(email)
@@ -1045,7 +1052,7 @@ func GetUserByEmail(email string) (*User, error) {
 		return GetUserByID(emailAddress.UID)
 	}
 
-	return nil, ErrUserNotExist{0, email}
+	return nil, ErrUserNotExist{0, email, 0}
 }
 
 type SearchUserOptions struct {
diff --git a/models/user_mail.go b/models/user_mail.go
index 9dc3d624b4..c6ade3fa43 100644
--- a/models/user_mail.go
+++ b/models/user_mail.go
@@ -172,7 +172,7 @@ func MakeEmailPrimary(email *EmailAddress) error {
 	if err != nil {
 		return err
 	} else if !has {
-		return ErrUserNotExist{email.UID, ""}
+		return ErrUserNotExist{email.UID, "", 0}
 	}
 
 	// Make sure the former primary email doesn't disappear.