Изменёна кодировка столбца username в usernames_history для организации бинарного поиска

Из Account вынесена логика аутентификации в дочерний AccountIdentity
Исправлена логика валидации при вызове на неизменённом нике для формы смены ника

api/config/main.php

@@ -13,7 +13,7 @@ return [
     'controllerNamespace' => 'api\controllers',
     'components' => [
         'user' => [
-            'identityClass' => \common\models\Account::class,
+            'identityClass' => \api\models\AccountIdentity::class,
             'enableSession' => false,
             'loginUrl' => null,
         ],
@@ -21,7 +21,7 @@ return [
             'traceLevel' => YII_DEBUG ? 3 : 0,
             'targets' => [
                 [
-                    'class' => 'yii\log\FileTarget',
+                    'class' => \yii\log\FileTarget::class,
                     'levels' => ['error', 'warning'],
                 ],
             ],

api/models/AccountIdentity.php

@@ -0,0 +1,37 @@
+<?php
+namespace api\models;
+
+use common\models\Account;
+use yii\base\NotSupportedException;
+use yii\web\IdentityInterface;
+
+/**
+ * @method static findIdentityByAccessToken($token, $type = null) этот метод реализуется в UserTrait, который
+ * подключён в родительском Account и позволяет выполнить условия интерфейса
+ * @method string getId() метод реализован в родительском классе, т.к. UserTrait требует, чтобы этот метод
+ * присутствовал обязательно, но при этом не навязывает его как абстрактный
+ */
+class AccountIdentity extends Account implements IdentityInterface {
+
+    /**
+     * @inheritdoc
+     */
+    public static function findIdentity($id) {
+        return static::findOne($id);
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function getAuthKey() {
+        throw new NotSupportedException('This method used for cookie auth, except we using JWT tokens');
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function validateAuthKey($authKey) {
+        return $this->getAuthKey() === $authKey;
+    }
+
+}

api/models/ChangeUsernameForm.php

@@ -3,7 +3,6 @@ namespace api\models;
 
 use api\models\base\PasswordProtectedForm;
 use common\helpers\Amqp;
-use common\models\Account;
 use common\models\amqp\UsernameChanged;
 use common\models\UsernameHistory;
 use PhpAmqpLib\Message\AMQPMessage;
@@ -23,7 +22,7 @@ class ChangeUsernameForm extends PasswordProtectedForm {
     }
 
     public function validateUsername($attribute) {
-        $account = new Account();
+        $account = $this->getAccount();
         $account->username = $this->$attribute;
         if (!$account->validate(['username'])) {
             $this->addErrors($account->getErrors());

common/models/Account.php

@@ -2,14 +2,12 @@
 namespace common\models;
 
 use common\components\UserPass;
-use damirka\JWT\UserTrait;
+use damirka\JWT\UserTrait as UserJWTTrait;
 use Ely\Yii2\TempmailValidator;
 use Yii;
 use yii\base\InvalidConfigException;
-use yii\base\NotSupportedException;
 use yii\behaviors\TimestampBehavior;
 use yii\db\ActiveRecord;
-use yii\web\IdentityInterface;
 
 /**
  * Поля модели:
@@ -36,8 +34,8 @@ use yii\web\IdentityInterface;
  * Поведения:
  * @mixin TimestampBehavior
  */
-class Account extends ActiveRecord implements IdentityInterface {
-    use UserTrait;
+class Account extends ActiveRecord {
+    use UserJWTTrait;
 
     const STATUS_DELETED = -10;
     const STATUS_REGISTERED = 0;
@@ -52,7 +50,7 @@ class Account extends ActiveRecord implements IdentityInterface {
 
     public function behaviors() {
         return [
-            TimestampBehavior::className(),
+            TimestampBehavior::class,
         ];
     }
 
@@ -78,74 +76,6 @@ class Account extends ActiveRecord implements IdentityInterface {
         ];
     }
 
-    /**
-     * @inheritdoc
-     */
-    public static function findIdentity($id) {
-        return static::findOne(['id' => $id]);
-    }
-
-    /**
-     * Finds user by password reset token
-     *
-     * @param string $token password reset token
-     *
-     * @return static|null
-     *
-     * TODO: этот метод нужно убрать из базовой модели
-     */
-    public static function findByPasswordResetToken($token) {
-        if (!static::isPasswordResetTokenValid($token)) {
-            return null;
-        }
-
-        return static::findOne([
-            'password_reset_token' => $token,
-            'status' => self::STATUS_ACTIVE,
-        ]);
-    }
-
-    /**
-     * Finds out if password reset token is valid
-     *
-     * @param string $token password reset token
-     *
-     * @return boolean
-     *
-     * TODO: этот метод нужно убрать из базовой модели
-     */
-    public static function isPasswordResetTokenValid($token) {
-        if (empty($token)) {
-            return false;
-        }
-
-        $timestamp = (int) substr($token, strrpos($token, '_') + 1);
-        $expire = Yii::$app->params['user.passwordResetTokenExpire'];
-
-        return $timestamp + $expire >= time();
-    }
-
-    /**
-     * @inheritdoc
-     */
-    public function getId() {
-        return $this->getPrimaryKey();
-    }
-
-    /**
-     * @inheritdoc
-     */
-    public function getAuthKey() {
-        throw new NotSupportedException('This method used for cookie auth, except we using JWT tokens');
-    }
-
-    /**
-     * @inheritdoc
-     */
-    public function validateAuthKey($authKey) {
-        return $this->getAuthKey() === $authKey;
-    }
-
     /**
      * Validates password
      *
@@ -183,24 +113,6 @@ class Account extends ActiveRecord implements IdentityInterface {
         $this->password_changed_at = time();
     }
 
-    /**
-     * Generates new password reset token
-     *
-     * TODO: этот метод нужно отсюда убрать
-     */
-    public function generatePasswordResetToken() {
-        $this->password_reset_token = Yii::$app->security->generateRandomString() . '_' . time();
-    }
-
-    /**
-     * Removes password reset token
-     *
-     * TODO: этот метод нужно отсюда убрать
-     */
-    public function removePasswordResetToken() {
-        $this->password_reset_token = null;
-    }
-
     public function getEmailActivations() {
         return $this->hasMany(EmailActivation::class, ['account_id' => 'id']);
     }
@@ -266,4 +178,13 @@ class Account extends ActiveRecord implements IdentityInterface {
             ->exists();
     }
 
+    /**
+     * TODO: нужно создать PR в UserTrait репо, чтобы этот метод сделали абстрактным
+     *
+     * @return int
+     */
+    public function getId() {
+        return $this->getPrimaryKey();
+    }
+
 }

console/migrations/m160512_080955_usernames_history_encoding.php

@@ -0,0 +1,18 @@
+<?php
+
+use console\db\Migration;
+
+class m160512_080955_usernames_history_encoding extends Migration {
+
+    public function safeUp() {
+        $this->getDb()->createCommand('
+            ALTER TABLE {{%usernames_history}}
+            MODIFY username VARCHAR(255) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL
+        ')->execute();
+    }
+
+    public function safeDown() {
+        $this->alterColumn('{{%usernames_history}}', 'username', $this->string()->notNull());
+    }
+
+}

tests/codeception/api/unit/models/ChangeUsernameFormTest.php

@@ -10,7 +10,7 @@ use tests\codeception\common\fixtures\AccountFixture;
 use Yii;
 
 /**
- * @property array $accounts
+ * @property AccountFixture $accounts
  */
 class ChangeUsernameFormTest extends DbTestCase {
     use Specify;
@@ -26,52 +26,94 @@ class ChangeUsernameFormTest extends DbTestCase {
 
     public function testChange() {
         $this->specify('successfully change username to new one', function() {
-            $model = new DummyChangeUsernameForm([
+            $model = $this->createModel([
                 'password' => 'password_0',
                 'username' => 'my_new_nickname',
             ]);
             expect($model->change())->true();
-            expect(Account::findOne(1)->username)->equals('my_new_nickname');
+            expect(Account::findOne($this->getAccountId())->username)->equals('my_new_nickname');
             expect(UsernameHistory::findOne(['username' => 'my_new_nickname']))->isInstanceOf(UsernameHistory::class);
         });
     }
 
-    public function testUsernameUnavailable() {
+    public function testChangeWithoutChange() {
+        $this->scenario->incomplete('This test is written invalid');
+        return;
+
+        // TODO: этот тест написан неправильно - запись всё равно добавляется в базу данных, но тест не замечает
+        /** @noinspection PhpUnreachableStatementInspection */
+        $this->specify('no new UsernameHistory record, if we don\'t change nickname', function() {
+            $model = $this->createModel([
+                'password' => 'password_0',
+                'username' => $this->accounts['admin']['username'],
+            ]);
+            $callTime = time();
+            expect($model->change())->true();
+            expect(UsernameHistory::findOne([
+                'AND',
+                'username' => $this->accounts['admin']['username'],
+                ['>=', 'applied_in', $callTime - 5],
+            ]))->null();
+        });
+    }
+
+    public function testChangeCase() {
+        $this->specify('username should change, if we change case of some letters', function() {
+            $newUsername = mb_strtoupper($this->accounts['admin']['username']);
+            $model = $this->createModel([
+                'password' => 'password_0',
+                'username' => $newUsername,
+            ]);
+            expect($model->change())->true();
+            expect(Account::findOne($this->getAccountId())->username)->equals($newUsername);
+            expect(UsernameHistory::findOne(['username' => $newUsername]))->isInstanceOf(UsernameHistory::class);
+        });
+    }
+
+    public function testValidateUsername() {
         $this->specify('error.username_not_available expected if username is already taken', function() {
-            $model = new DummyChangeUsernameForm([
+            $model = $this->createModel([
                 'password' => 'password_0',
                 'username' => 'Jon',
             ]);
-            $model->validate();
+            $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 DummyChangeUsernameForm([
+            $model = $this->createModel([
                 'password' => 'password_0',
-                'username' => 'Admin',
+                'username' => $this->accounts['admin']['username'],
             ]);
-            $model->validate();
-            expect($model->getErrors('username'))->equals([]);
+            $model->validateUsername('username');
+            expect($model->getErrors('username'))->isEmpty();
         });
     }
 
     public function testCreateTask() {
-        $model = new DummyChangeUsernameForm();
+        $model = $this->createModel();
         $model->createTask('1', 'test1', 'test');
         // TODO: у меня пока нет идей о том, чтобы это как-то успешно протестировать, увы
         // но по крайней мере можно убедиться, что оно не падает где-то на этом шаге
     }
 
-}
+    private function createModel(array $params = []) : ChangeUsernameForm {
+        /** @noinspection PhpUnusedLocalVariableInspection */
+        $params = array_merge($params, [
+            'accountId' => $this->getAccountId(),
+        ]);
 
-// TODO: тут образуется магическая переменная 1, что не круто. После перехода на php7 можно заюзать анонимный класс
-// и создавать модель прямо внутри теста, где доступен объект фикстур с именами переменных
-
-class DummyChangeUsernameForm extends ChangeUsernameForm {
+        return new class($params) extends ChangeUsernameForm {
+            public $accountId;
 
             protected function getAccount() {
-        return Account::findOne(1);
+                return Account::findOne($this->accountId);
+            }
+        };
+    }
+
+    private function getAccountId() {
+        return $this->accounts['admin']['id'];
     }
 
 }