Skip to content

Commit

Permalink
One less SQL query for login sequence in basic and advanced apps
Browse files Browse the repository at this point in the history
  • Loading branch information
@samdark
samdark committed Nov 16, 2013
1 parent b06caa2 commit 5b36503
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
19 changes: 17 additions & 2 deletions apps/advanced/common/models/LoginForm.php
View file
Expand Up @@ -13,6 +13,7 @@ class LoginForm extends Model
public $username;
public $password;
public $rememberMe = true;
private $_user;

/**
* @return array the validation rules.
Expand All @@ -35,7 +36,7 @@ public function rules()
*/
public function validatePassword()
{
$user = User::findByUsername($this->username);
$user = $this->getUserByUsername($this->username);
if (!$user || !$user->validatePassword($this->password)) {
$this->addError('password', 'Incorrect username or password.');
}
Expand All @@ -48,11 +49,25 @@ public function validatePassword()
public function login()
{
if ($this->validate()) {
$user = User::findByUsername($this->username);
$user = $this->getUserByUsername($this->username);
Yii::$app->user->login($user, $this->rememberMe ? 3600*24*30 : 0);
return true;
} else {
return false;
}
}

/**
* Finds user by username
*
* @param string $username
* @return User|null
*/
private function getUserByUsername($username)
{
if ($this->_user === null) {
$this->_user = User::findByUsername($username);
}
return $this->_user;
}
}
19 changes: 17 additions & 2 deletions apps/basic/models/LoginForm.php
View file
Expand Up @@ -13,6 +13,7 @@ class LoginForm extends Model
public $username;
public $password;
public $rememberMe = true;
private $_user;

/**
* @return array the validation rules.
Expand All @@ -35,7 +36,7 @@ public function rules()
*/
public function validatePassword()
{
$user = User::findByUsername($this->username);
$user = $this->getUserByUsername($this->username);
if (!$user || !$user->validatePassword($this->password)) {
$this->addError('password', 'Incorrect username or password.');
}
Expand All @@ -48,11 +49,25 @@ public function validatePassword()
public function login()
{
if ($this->validate()) {
$user = User::findByUsername($this->username);
$user = $this->getUserByUsername($this->username);
Yii::$app->user->login($user, $this->rememberMe ? 3600*24*30 : 0);
return true;
} else {
return false;
}
}

/**
* Finds user by username
*
* @param string $username
* @return User|null
*/
private function getUserByUsername($username)
{
if ($this->_user === null) {
$this->_user = User::findByUsername($username);
}
return $this->_user;
}
}

2 comments on commit 5b36503

@qiangxue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change is not needed:

  • logically speaking, the implementation of LoginForm::getUserByUsername() is incorrect because it returns the same user even if $username is different.
  • The performance of login() is not important. We even want to slow down login() for security reason.

@samdark
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed logic: ad6411c

bcrypt only consumes CPU performance of web frontend while extra query consumes DBMS performance.

Please sign in to comment.