From 94a8e21f27aded4d82bc19bbfeed29f64643f304 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Tue, 1 Nov 2016 23:27:38 +0300 Subject: [PATCH] =?UTF-8?q?=D0=9E=D0=B1=D1=80=D0=B0=D0=B7=D0=BE=D0=B2?= =?UTF-8?q?=D0=B0=D0=BD=20=D0=B2=D0=B0=D0=BB=D0=B8=D0=B4=D0=B0=D1=82=D0=BE?= =?UTF-8?q?=D1=80=20UsernameValidator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../authentication/RegistrationForm.php | 11 +- api/models/profile/ChangeUsernameForm.php | 19 +--- common/models/Account.php | 11 -- common/validators/UsernameValidator.php | 59 ++++++++++ .../models/profile/ChangeUsernameFormTest.php | 20 ---- .../common/unit/models/AccountTest.php | 50 --------- .../unit/validators/UsernameValidatorTest.php | 106 ++++++++++++++++++ 7 files changed, 171 insertions(+), 105 deletions(-) create mode 100644 common/validators/UsernameValidator.php create mode 100644 tests/codeception/common/unit/validators/UsernameValidatorTest.php diff --git a/api/models/authentication/RegistrationForm.php b/api/models/authentication/RegistrationForm.php index fea1245..9d80b2d 100644 --- a/api/models/authentication/RegistrationForm.php +++ b/api/models/authentication/RegistrationForm.php @@ -11,6 +11,7 @@ use common\models\EmailActivation; use common\models\UsernameHistory; use common\validators\LanguageValidator; use common\validators\PasswordValidate; +use common\validators\UsernameValidator; use Exception; use Ramsey\Uuid\Uuid; use Yii; @@ -40,7 +41,7 @@ class RegistrationForm extends ApiForm { ['captcha', ReCaptchaValidator::class], ['rulesAgreement', 'required', 'message' => E::RULES_AGREEMENT_REQUIRED], - ['username', 'validateUsername', 'skipOnEmpty' => false], + ['username', UsernameValidator::class], ['email', 'validateEmail', 'skipOnEmpty' => false], ['password', 'required', 'message' => E::PASSWORD_REQUIRED], @@ -53,14 +54,6 @@ class RegistrationForm extends ApiForm { ]; } - public function validateUsername() { - $account = new Account(); - $account->username = $this->username; - if (!$account->validate(['username'])) { - $this->addErrors($account->getErrors()); - } - } - public function validateEmail() { $account = new Account(); $account->email = $this->email; diff --git a/api/models/profile/ChangeUsernameForm.php b/api/models/profile/ChangeUsernameForm.php index 81f7957..562ef73 100644 --- a/api/models/profile/ChangeUsernameForm.php +++ b/api/models/profile/ChangeUsernameForm.php @@ -4,11 +4,10 @@ namespace api\models\profile; use api\models\AccountIdentity; use api\models\base\ApiForm; use api\validators\PasswordRequiredValidator; -use common\helpers\Error; use common\helpers\Amqp; -use common\models\Account; use common\models\amqp\UsernameChanged; use common\models\UsernameHistory; +use common\validators\UsernameValidator; use Exception; use PhpAmqpLib\Message\AMQPMessage; use Yii; @@ -22,23 +21,13 @@ class ChangeUsernameForm extends ApiForm { public function rules() { return [ - ['username', 'required', 'message' => Error::USERNAME_REQUIRED], - ['username', 'validateUsername'], + ['username', UsernameValidator::class, 'accountCallback' => function() { + return $this->getAccount()->id; + }], ['password', PasswordRequiredValidator::class], ]; } - public function validateUsername($attribute) { - $account = new Account(); - $account->id = $this->getAccount()->id; - $account->username = $this->$attribute; - // Это чтобы unique validator учёл, что ник может быть забит на текущий аккаунт - $account->setIsNewRecord(false); - if (!$account->validate(['username'])) { - $this->addErrors($account->getErrors()); - } - } - public function change() { if (!$this->validate()) { return false; diff --git a/common/models/Account.php b/common/models/Account.php index 4c94f08..0a04312 100644 --- a/common/models/Account.php +++ b/common/models/Account.php @@ -61,17 +61,6 @@ class Account extends ActiveRecord { public function rules() { return [ - [['username'], 'filter', 'filter' => 'trim'], - [['username'], 'required', 'message' => E::USERNAME_REQUIRED], - [['username'], 'string', 'min' => 3, 'max' => 21, - 'tooShort' => E::USERNAME_TOO_SHORT, - 'tooLong' => E::USERNAME_TOO_LONG, - ], - [['username'], 'match', 'pattern' => '/^[\p{L}\d-_\.!?#$%^&*()\[\]:;]+$/u', - 'message' => E::USERNAME_INVALID, - ], - [['username'], 'unique', 'message' => E::USERNAME_NOT_AVAILABLE], - [['email'], 'filter', 'filter' => 'trim'], [['email'], 'required', 'message' => E::EMAIL_REQUIRED], [['email'], 'string', 'max' => 255, 'tooLong' => E::EMAIL_TOO_LONG], diff --git a/common/validators/UsernameValidator.php b/common/validators/UsernameValidator.php new file mode 100644 index 0000000..f5d8895 --- /dev/null +++ b/common/validators/UsernameValidator.php @@ -0,0 +1,59 @@ + 'trim']); + + $required = new validators\RequiredValidator(); + $required->message = E::USERNAME_REQUIRED; + + $length = new validators\StringValidator(); + $length->min = 3; + $length->max = 21; + $length->tooShort = E::USERNAME_TOO_SHORT; + $length->tooLong = E::USERNAME_TOO_LONG; + + $pattern = new validators\RegularExpressionValidator(['pattern' => '/^[\p{L}\d-_\.!?#$%^&*()\[\]:;]+$/u']); + $pattern->message = E::USERNAME_INVALID; + + $unique = new validators\UniqueValidator(); + $unique->message = E::USERNAME_NOT_AVAILABLE; + $unique->targetClass = Account::class; + $unique->targetAttribute = 'username'; + if ($this->accountCallback !== null) { + $unique->filter = function(QueryInterface $query) { + $query->andWhere(['NOT', ['id' => ($this->accountCallback)()]]); + }; + } + + $this->executeValidation($filter, $model, $attribute) && + $this->executeValidation($required, $model, $attribute) && + $this->executeValidation($length, $model, $attribute) && + $this->executeValidation($pattern, $model, $attribute) && + $this->executeValidation($unique, $model, $attribute); + } + + protected function executeValidation(Validator $validator, Model $model, string $attribute) { + $validator->validateAttribute($model, $attribute); + + return !$model->hasErrors($attribute); + } + +} diff --git a/tests/codeception/api/unit/models/profile/ChangeUsernameFormTest.php b/tests/codeception/api/unit/models/profile/ChangeUsernameFormTest.php index 75834aa..a41dd75 100644 --- a/tests/codeception/api/unit/models/profile/ChangeUsernameFormTest.php +++ b/tests/codeception/api/unit/models/profile/ChangeUsernameFormTest.php @@ -67,26 +67,6 @@ class ChangeUsernameFormTest extends TestCase { ); } - public function testValidateUsername() { - $this->specify('error.username_not_available expected if username is already taken', function() { - $model = new ChangeUsernameForm([ - 'password' => 'password_0', - 'username' => 'Jon', - ]); - $model->validateUsername('username'); - expect($model->getErrors('username'))->equals(['error.username_not_available']); - }); - - $this->specify('error.username_not_available is NOT expected if username is already taken by CURRENT user', function() { - $model = new ChangeUsernameForm([ - 'password' => 'password_0', - 'username' => $this->tester->grabFixture('accounts', 'admin')['username'], - ]); - $model->validateUsername('username'); - expect($model->getErrors('username'))->isEmpty(); - }); - } - public function testCreateTask() { $model = new ChangeUsernameForm(); $model->createEventTask('1', 'test1', 'test'); diff --git a/tests/codeception/common/unit/models/AccountTest.php b/tests/codeception/common/unit/models/AccountTest.php index a2078dc..9e5beb5 100644 --- a/tests/codeception/common/unit/models/AccountTest.php +++ b/tests/codeception/common/unit/models/AccountTest.php @@ -20,56 +20,6 @@ class AccountTest extends TestCase { ]; } - public function testValidateUsername() { - $this->specify('username required', function() { - $model = new Account(['username' => null]); - expect($model->validate(['username']))->false(); - expect($model->getErrors('username'))->equals(['error.username_required']); - }); - - $this->specify('username should be at least 3 symbols length', function() { - $model = new Account(['username' => 'at']); - expect($model->validate(['username']))->false(); - expect($model->getErrors('username'))->equals(['error.username_too_short']); - }); - - $this->specify('username should be not more than 21 symbols length', function() { - $model = new Account(['username' => 'erickskrauch_erickskrauch']); - expect($model->validate(['username']))->false(); - expect($model->getErrors('username'))->equals(['error.username_too_long']); - }); - - $this->specify('username can contain many cool symbols', function() { - $shouldBeValid = [ - 'русский_ник', 'русский_ник_на_грани!', 'numbers1132', '*__*-Stars-*__*', '1-_.!?#$%^&*()[]', - '[ESP]Эрик', 'Свят_помидор;', 'зроблена_ў_беларусі:)', - ]; - foreach($shouldBeValid as $nickname) { - $model = new Account(['username' => $nickname]); - expect($nickname . ' passed validation', $model->validate(['username']))->true(); - expect($model->getErrors('username'))->isEmpty(); - } - }); - - $this->specify('username cannot contain some symbols', function() { - $shouldBeInvalid = [ - 'nick@name', 'spaced nick', - ]; - foreach($shouldBeInvalid as $nickname) { - $model = new Account(['username' => $nickname]); - expect($nickname . ' fail validation', $model->validate('username'))->false(); - expect($model->getErrors('username'))->equals(['error.username_invalid']); - } - }); - - $this->specify('username should be unique', function() { - $model = new Account(); - $model->username = $this->tester->grabFixture('accounts', 'admin')['username']; - expect($model->validate('username'))->false(); - expect($model->getErrors('username'))->equals(['error.username_not_available']); - }); - } - public function testValidateEmail() { // TODO: пропускать этот тест, если падает ошибка с недостпуностью интернет соединения $this->specify('email required', function() { diff --git a/tests/codeception/common/unit/validators/UsernameValidatorTest.php b/tests/codeception/common/unit/validators/UsernameValidatorTest.php new file mode 100644 index 0000000..ecb8238 --- /dev/null +++ b/tests/codeception/common/unit/validators/UsernameValidatorTest.php @@ -0,0 +1,106 @@ +validator = new UsernameValidator(); + } + + public function testValidateAttributeRequired() { + $model = $this->createModel(''); + $this->validator->validateAttribute($model, 'field'); + $this->assertEquals(['error.username_required'], $model->getErrors('field')); + + $model = $this->createModel('username'); + $this->validator->validateAttribute($model, 'field'); + $this->assertNotEquals(['error.username_required'], $model->getErrors('field')); + } + + public function testValidateAttributeLength() { + $model = $this->createModel('at'); + $this->validator->validateAttribute($model, 'field'); + $this->assertEquals(['error.username_too_short'], $model->getErrors('field')); + + $model = $this->createModel('erickskrauch_erickskrauch'); + $this->validator->validateAttribute($model, 'field'); + $this->assertEquals(['error.username_too_long'], $model->getErrors('field')); + + $model = $this->createModel('username'); + $this->validator->validateAttribute($model, 'field'); + $this->assertNotEquals(['error.username_too_short'], $model->getErrors('field')); + $this->assertNotEquals(['error.username_too_long'], $model->getErrors('field')); + } + + public function testValidateAttributePattern() { + $shouldBeValid = [ + 'русский_ник', 'русский_ник_на_грани!', 'numbers1132', '*__*-Stars-*__*', '1-_.!?#$%^&*()[]', + '[ESP]Эрик', 'Свят_помидор;', 'зроблена_ў_беларусі:)', + ]; + foreach($shouldBeValid as $nickname) { + $model = $this->createModel($nickname); + $this->validator->validateAttribute($model, 'field'); + $this->assertNotEquals(['error.username_invalid'], $model->getErrors('field')); + } + + $shouldBeInvalid = [ + 'nick@name', 'spaced nick', + ]; + foreach($shouldBeInvalid as $nickname) { + $model = $this->createModel($nickname); + $this->validator->validateAttribute($model, 'field'); + $this->assertEquals(['error.username_invalid'], $model->getErrors('field')); + } + } + + public function testValidateAttributeUnique() { + $this->tester->haveFixtures([ + 'accounts' => AccountFixture::class, + ]); + + /** @var \common\models\Account $accountFixture */ + $accountFixture = $this->tester->grabFixture('accounts', 'admin'); + + $model = $this->createModel($accountFixture->username); + $this->validator->validateAttribute($model, 'field'); + $this->assertEquals(['error.username_not_available'], $model->getErrors('field')); + + $model = $this->createModel($accountFixture->username); + $this->validator->accountCallback = function() use ($accountFixture) { + return $accountFixture->id; + }; + $this->validator->validateAttribute($model, 'field'); + $this->assertNotEquals(['error.username_not_available'], $model->getErrors('field')); + $this->validator->accountCallback = null; + + $model = $this->createModel('some-unique-username'); + $this->validator->validateAttribute($model, 'field'); + $this->assertNotEquals(['error.username_not_available'], $model->getErrors('field')); + } + + /** + * @param string $fieldValue + * @return Model + */ + private function createModel(string $fieldValue) : Model { + $class = new class extends Model { + public $field; + }; + + $class->field = $fieldValue; + + return $class; + } + +}