From bfba0aa711869f2eed3ecd5d1b672c55a72a4554 Mon Sep 17 00:00:00 2001 From: Elvira Sheina Date: Thu, 15 Dec 2016 16:23:16 +0500 Subject: [PATCH] Refactor validateAttribute method in UniqueValidator (#13202) * Refactor validateAttribute method in UniqueValidator Extract prepareParams and prepareQuery from validateAttribute, so they can be tested separately. * Added issue number to changelog * Eliminated unneeded variable * Renamed methods and parameters, update PHPDocs --- framework/CHANGELOG.md | 1 + framework/validators/UniqueValidator.php | 110 +++++++++++++----- .../validators/UniqueValidatorTest.php | 59 +++++++++- 3 files changed, 142 insertions(+), 28 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index b28380cc59..e321921b17 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -74,6 +74,7 @@ Yii Framework 2 Change Log - Enh: Added constants for specifying `yii\validators\CompareValidator::$type` (cebe) - Enh #12854: Added `RangeNotSatisfiableHttpException` to cover HTTP error 416 file request exceptions (zalatov) - Enh #13122: Optimized query for information about foreign keys in `yii\db\oci` (zlakomanoff) +- Enh #13202: Refactor validateAttribute method in UniqueValidator (developeruz) 2.0.10 October 20, 2016 ----------------------- diff --git a/framework/validators/UniqueValidator.php b/framework/validators/UniqueValidator.php index 5fd5be1d3c..03c29c07d9 100644 --- a/framework/validators/UniqueValidator.php +++ b/framework/validators/UniqueValidator.php @@ -8,7 +8,11 @@ namespace yii\validators; use Yii; +use yii\base\Model; +use yii\db\ActiveQuery; +use yii\db\ActiveQueryInterface; use yii\db\ActiveRecordInterface; +use yii\db\Query; use yii\helpers\Inflector; /** @@ -44,7 +48,7 @@ class UniqueValidator extends Validator */ public $targetClass; /** - * @var string|array the name of the ActiveRecord attribute that should be used to + * @var string|array the name of the [[\yii\db\ActiveRecord|ActiveRecord]] attribute that should be used to * validate the uniqueness of the current attribute value. If not set, it will use the name * of the attribute currently being validated. You may use an array to validate the uniqueness * of multiple columns at the same time. The array values are the attributes that will be @@ -111,32 +115,38 @@ class UniqueValidator extends Validator /* @var $targetClass ActiveRecordInterface */ $targetClass = $this->targetClass === null ? get_class($model) : $this->targetClass; $targetAttribute = $this->targetAttribute === null ? $attribute : $this->targetAttribute; + $conditions = $this->prepareConditions($targetAttribute, $model, $attribute); - if (is_array($targetAttribute)) { - $params = []; - foreach ($targetAttribute as $k => $v) { - $params[$v] = is_int($k) ? $model->$v : $model->$k; - } - } else { - $params = [$targetAttribute => $model->$attribute]; - } - - foreach ($params as $value) { + foreach ($conditions as $value) { if (is_array($value)) { $this->addError($model, $attribute, Yii::t('yii', '{attribute} is invalid.')); - return; } } - $query = $targetClass::find(); - $query->andWhere($params); - - if ($this->filter instanceof \Closure) { - call_user_func($this->filter, $query); - } elseif ($this->filter !== null) { - $query->andWhere($this->filter); + if ($this->modelExists($targetClass, $conditions, $model)) { + if (count($targetAttribute) > 1) { + $this->addComboNotUniqueError($model, $attribute); + } else { + $this->addError($model, $attribute, $this->message); + } } + } + + /** + * Checks whether the $model exists in the database. + * + * @param string $targetClass the name of the ActiveRecord class that should be used to validate the uniqueness + * of the current attribute value. + * @param array $conditions conditions, compatible with [[\yii\db\Query::where()|Query::where()]] key-value format. + * @param Model $model the data model to be validated + * + * @return bool whether the model already exists + */ + private function modelExists($targetClass, $conditions, $model) + { + /** @var ActiveRecordInterface $targetClass $query */ + $query = $this->prepareQuery($targetClass, $conditions); if (!$model instanceof ActiveRecordInterface || $model->getIsNewRecord() || $model->className() !== $targetClass::className()) { // if current $model isn't in the database yet then it's OK just to call exists() @@ -144,11 +154,11 @@ class UniqueValidator extends Validator $exists = $query->exists(); } else { // if current $model is in the database already we can't use exists() - /* @var $models ActiveRecordInterface[] */ + /** @var $models ActiveRecordInterface[] */ $models = $query->select($targetClass::primaryKey())->limit(2)->all(); $n = count($models); if ($n === 1) { - $keys = array_keys($params); + $keys = array_keys($conditions); $pks = $targetClass::primaryKey(); sort($keys); sort($pks); @@ -164,13 +174,59 @@ class UniqueValidator extends Validator } } - if ($exists) { - if (count($targetAttribute) > 1) { - $this->addComboNotUniqueError($model, $attribute); - } else { - $this->addError($model, $attribute, $this->message); - } + return $exists; + } + + /** + * Prepares a query by applying filtering conditions defined in $conditions method property + * and [[filter]] class property. + * + * @param ActiveRecordInterface $targetClass the name of the ActiveRecord class that should be used to validate + * the uniqueness of the current attribute value. + * @param array $conditions conditions, compatible with [[\yii\db\Query::where()|Query::where()]] key-value format + * + * @return ActiveQueryInterface|ActiveQuery + */ + private function prepareQuery($targetClass, $conditions) + { + $query = $targetClass::find(); + $query->andWhere($conditions); + + if ($this->filter instanceof \Closure) { + call_user_func($this->filter, $query); + } elseif ($this->filter !== null) { + $query->andWhere($this->filter); } + + return $query; + } + + /** + * Processes attributes' relations described in $targetAttribute parameter into conditions, compatible with + * [[\yii\db\Query::where()|Query::where()]] key-value format. + * + * @param string|array $targetAttribute the name of the [[\yii\db\ActiveRecord|ActiveRecord]] attribute that + * should be used to validate the uniqueness of the current attribute value. You may use an array to validate + * the uniqueness of multiple columns at the same time. The array values are the attributes that will be + * used to validate the uniqueness, while the array keys are the attributes whose values are to be validated. + * If the key and the value are the same, you can just specify the value. + * @param Model $model the data model to be validated + * @param string $attribute the name of the attribute to be validated in the $model + + * @return array conditions, compatible with [[\yii\db\Query::where()|Query::where()]] key-value format. + */ + private function prepareConditions($targetAttribute, $model, $attribute) + { + if (is_array($targetAttribute)) { + $conditions = []; + foreach ($targetAttribute as $k => $v) { + $conditions[$v] = is_int($k) ? $model->$v : $model->$k; + } + } else { + $conditions = [$targetAttribute => $model->$attribute]; + } + + return $conditions; } /** diff --git a/tests/framework/validators/UniqueValidatorTest.php b/tests/framework/validators/UniqueValidatorTest.php index 9305118864..02a3940a72 100644 --- a/tests/framework/validators/UniqueValidatorTest.php +++ b/tests/framework/validators/UniqueValidatorTest.php @@ -303,4 +303,61 @@ abstract class UniqueValidatorTest extends DatabaseTestCase $val->validateAttribute($m, 'ref'); $this->assertTrue($m->hasErrors('ref')); } -} \ No newline at end of file + + public function testPrepareParams() + { + $model = new FakedValidationModel(); + $model->val_attr_a = 'test value a'; + $model->val_attr_b = 'test value b'; + $model->val_attr_c = 'test value c'; + $attribute = 'val_attr_a'; + + $targetAttribute = 'val_attr_b'; + $result = $this->invokeMethod(new UniqueValidator(), 'prepareConditions', [$targetAttribute, $model, $attribute]); + $expected = ['val_attr_b' => 'test value a']; + $this->assertEquals($expected, $result); + + $targetAttribute = ['val_attr_b', 'val_attr_c']; + $result = $this->invokeMethod(new UniqueValidator(), 'prepareConditions', [$targetAttribute, $model, $attribute]); + $expected = ['val_attr_b' => 'test value b', 'val_attr_c' => 'test value c']; + $this->assertEquals($expected, $result); + + $targetAttribute = ['val_attr_a' => 'val_attr_b']; + $result = $this->invokeMethod(new UniqueValidator(), 'prepareConditions', [$targetAttribute, $model, $attribute]); + $expected = ['val_attr_b' => 'test value a']; + $this->assertEquals($expected, $result); + + + $targetAttribute = ['val_attr_b', 'val_attr_a' => 'val_attr_c']; + $result = $this->invokeMethod(new UniqueValidator(), 'prepareConditions', [$targetAttribute, $model, $attribute]); + $expected = ['val_attr_b' => 'test value b', 'val_attr_c' => 'test value a']; + $this->assertEquals($expected, $result); + } + + public function testPrepareQuery() + { + $schema = $this->getConnection()->schema; + + $model = new ValidatorTestMainModel(); + $query = $this->invokeMethod(new UniqueValidator(), 'prepareQuery', [$model,['val_attr_b' => 'test value a']]); + $expected = "SELECT * FROM {$schema->quoteTableName('validator_main')} WHERE {$schema->quoteColumnName('val_attr_b')}=:qp0"; + $this->assertEquals($expected, $query->createCommand()->getSql()); + + $params = ['val_attr_b' => 'test value b', 'val_attr_c' => 'test value a']; + $query = $this->invokeMethod(new UniqueValidator(), 'prepareQuery', [$model, $params]); + $expected = "SELECT * FROM {$schema->quoteTableName('validator_main')} WHERE ({$schema->quoteColumnName('val_attr_b')}=:qp0) AND ({$schema->quoteColumnName('val_attr_c')}=:qp1)"; + $this->assertEquals($expected, $query->createCommand()->getSql()); + + $params = ['val_attr_b' => 'test value b']; + $query = $this->invokeMethod(new UniqueValidator(['filter' => 'val_attr_a > 0']), 'prepareQuery', [$model, $params]); + $expected = "SELECT * FROM {$schema->quoteTableName('validator_main')} WHERE ({$schema->quoteColumnName('val_attr_b')}=:qp0) AND (val_attr_a > 0)"; + $this->assertEquals($expected, $query->createCommand()->getSql()); + + $params = ['val_attr_b' => 'test value b']; + $query = $this->invokeMethod(new UniqueValidator(['filter' => function($query) { + $query->orWhere('val_attr_a > 0'); + }]), 'prepareQuery', [$model, $params]); + $expected = "SELECT * FROM {$schema->quoteTableName('validator_main')} WHERE ({$schema->quoteColumnName('val_attr_b')}=:qp0) OR (val_attr_a > 0)"; + $this->assertEquals($expected, $query->createCommand()->getSql()); + } +}