From f60967df1f4d85e41fcf2a289131ab2e04c8ec88 Mon Sep 17 00:00:00 2001 From: Dmitrii Romanov Date: Tue, 16 May 2017 00:27:59 +0300 Subject: [PATCH] Added maxLength() validation rule for text columns when baking tables --- src/Shell/Task/ModelTask.php | 48 +++--- src/View/Helper/BakeHelper.php | 31 +++- tests/Fixture/BakeArticlesFixture.php | 2 +- tests/Fixture/NumberTreesFixture.php | 138 ++++++++++++++++++ tests/TestCase/Shell/Task/ModelTaskTest.php | 115 ++++++++++++--- tests/TestCase/View/Helper/BakeHelperTest.php | 21 +++ ...akeAssociationDetectionCategoriesTable.php | 1 + ...keAssociationDetectionOldProductsTable.php | 1 + ...tBakeAssociationDetectionProductsTable.php | 1 + .../Model/testBakeTableValidation.php | 3 +- 10 files changed, 317 insertions(+), 44 deletions(-) create mode 100644 tests/Fixture/NumberTreesFixture.php diff --git a/src/Shell/Task/ModelTask.php b/src/Shell/Task/ModelTask.php index e808c30bf..56c1dac9f 100644 --- a/src/Shell/Task/ModelTask.php +++ b/src/Shell/Task/ModelTask.php @@ -701,46 +701,52 @@ public function fieldValidation($schema, $fieldName, array $metaData, $primaryKe return false; } - $rule = false; + $rules = []; if ($fieldName === 'email') { - $rule = 'email'; + $rules['email'] = []; } elseif ($metaData['type'] === 'uuid') { - $rule = 'uuid'; + $rules['uuid'] = []; } elseif ($metaData['type'] === 'integer') { - $rule = 'integer'; + $rules['integer'] = []; } elseif ($metaData['type'] === 'float') { - $rule = 'numeric'; + $rules['numeric'] = []; } elseif ($metaData['type'] === 'decimal') { - $rule = 'decimal'; + $rules['decimal'] = []; } elseif ($metaData['type'] === 'boolean') { - $rule = 'boolean'; + $rules['boolean'] = []; } elseif ($metaData['type'] === 'date') { - $rule = 'date'; + $rules['date'] = []; } elseif ($metaData['type'] === 'time') { - $rule = 'time'; + $rules['time'] = []; } elseif ($metaData['type'] === 'datetime') { - $rule = 'dateTime'; + $rules['dateTime'] = []; } elseif ($metaData['type'] === 'timestamp') { - $rule = 'dateTime'; + $rules['dateTime'] = []; } elseif ($metaData['type'] === 'inet') { - $rule = 'ip'; + $rules['ip'] = []; } elseif ($metaData['type'] === 'string' || $metaData['type'] === 'text') { - $rule = 'scalar'; + $rules['scalar'] = []; + if ($metaData['length'] > 0) { + $rules['maxLength'] = [$metaData['length']]; + } } - $allowEmpty = false; if (in_array($fieldName, $primaryKey)) { - $allowEmpty = 'create'; + $rules['allowEmpty'] = ['create']; } elseif ($metaData['null'] === true) { - $allowEmpty = true; + $rules['allowEmpty'] = []; + } else { + $rules['requirePresence'] = ['create']; + $rules['notEmpty'] = []; } - $validation = [ - 'valid' => [ + $validation = []; + foreach ($rules as $rule => $args) { + $validation[$rule] = [ 'rule' => $rule, - 'allowEmpty' => $allowEmpty, - ] - ]; + 'args' => $args + ]; + } foreach ($schema->constraints() as $constraint) { $constraint = $schema->getConstraint($constraint); diff --git a/src/View/Helper/BakeHelper.php b/src/View/Helper/BakeHelper.php index 29c828d3a..d45131a49 100644 --- a/src/View/Helper/BakeHelper.php +++ b/src/View/Helper/BakeHelper.php @@ -292,8 +292,19 @@ public function getValidationMethods($field, $rules) $validationMethods = []; foreach ($rules as $ruleName => $rule) { - if ($rule['rule'] && !isset($rule['provider'])) { + if ($rule['rule'] && !isset($rule['provider']) && !isset($rule['args'])) { $validationMethods[] = sprintf("->%s('%s')", $rule['rule'], $field); + } elseif ($rule['rule'] && !isset($rule['provider'])) { + $formatTemplate = "->%s('%s')"; + if (!empty($rule['args'])) { + $formatTemplate = "->%s('%s', %s)"; + } + $validationMethods[] = sprintf( + $formatTemplate, + $rule['rule'], + $field, + implode(', ', $this->escapeArguments($rule['args'])) + ); } elseif ($rule['rule'] && isset($rule['provider'])) { $validationMethods[] = sprintf( "->add('%s', '%s', ['rule' => '%s', 'provider' => '%s'])", @@ -359,6 +370,24 @@ public function getFieldAccessibility($fields = null, $primaryKey = null) return $accessible; } + /** + * Wrap string arguments with quotes + * + * @param array $args array of arguments + * @return array + */ + public function escapeArguments($args) + { + return array_map(function ($v) { + if (is_string($v)) { + $v = strtr($v, ["'" => "\'"]); + $v = "'$v'"; + } + + return $v; + }, $args); + } + /** * To be mocked elsewhere... * diff --git a/tests/Fixture/BakeArticlesFixture.php b/tests/Fixture/BakeArticlesFixture.php index 6809d9626..07a5ebc4c 100644 --- a/tests/Fixture/BakeArticlesFixture.php +++ b/tests/Fixture/BakeArticlesFixture.php @@ -30,7 +30,7 @@ class BakeArticlesFixture extends TestFixture public $fields = [ 'id' => ['type' => 'integer'], 'bake_user_id' => ['type' => 'integer', 'null' => false], - 'title' => ['type' => 'string', 'null' => false], + 'title' => ['type' => 'string', 'length' => 50, 'null' => false], 'body' => 'text', 'published' => ['type' => 'boolean', 'length' => 1, 'default' => false], 'created' => 'datetime', diff --git a/tests/Fixture/NumberTreesFixture.php b/tests/Fixture/NumberTreesFixture.php new file mode 100644 index 000000000..af3814d9c --- /dev/null +++ b/tests/Fixture/NumberTreesFixture.php @@ -0,0 +1,138 @@ + ['type' => 'integer'], + 'name' => ['type' => 'string', 'length' => 50, 'null' => false], + 'parent_id' => 'integer', + 'lft' => ['type' => 'integer'], + 'rght' => ['type' => 'integer'], + 'depth' => ['type' => 'integer'], + '_constraints' => ['primary' => ['type' => 'primary', 'columns' => ['id']]] + ]; + + /** + * Records + * + * - electronics:1 + * - televisions:2 + * - tube:3 + * - lcd:4 + * - plasma:5 + * - portable:6 + * - mp3:7 + * - flash:8 + * - cd:9 + * - radios:10 + * - alien ware: 11 + * + * @var array + */ + public $records = [ + [ + 'name' => 'electronics', + 'parent_id' => null, + 'lft' => '1', + 'rght' => '20', + 'depth' => 0 + ], + [ + 'name' => 'televisions', + 'parent_id' => '1', + 'lft' => '2', + 'rght' => '9', + 'depth' => 1 + ], + [ + 'name' => 'tube', + 'parent_id' => '2', + 'lft' => '3', + 'rght' => '4', + 'depth' => 2 + ], + [ + 'name' => 'lcd', + 'parent_id' => '2', + 'lft' => '5', + 'rght' => '6', + 'depth' => 2 + ], + [ + 'name' => 'plasma', + 'parent_id' => '2', + 'lft' => '7', + 'rght' => '8', + 'depth' => 2 + ], + [ + 'name' => 'portable', + 'parent_id' => '1', + 'lft' => '10', + 'rght' => '19', + 'depth' => 1 + ], + [ + 'name' => 'mp3', + 'parent_id' => '6', + 'lft' => '11', + 'rght' => '14', + 'depth' => 2 + ], + [ + 'name' => 'flash', + 'parent_id' => '7', + 'lft' => '12', + 'rght' => '13', + 'depth' => 3 + ], + [ + 'name' => 'cd', + 'parent_id' => '6', + 'lft' => '15', + 'rght' => '16', + 'depth' => 2 + ], + [ + 'name' => 'radios', + 'parent_id' => '6', + 'lft' => '17', + 'rght' => '18', + 'depth' => 2 + ], + [ + 'name' => 'alien hardware', + 'parent_id' => null, + 'lft' => '21', + 'rght' => '22', + 'depth' => 0 + ] + ]; +} diff --git a/tests/TestCase/Shell/Task/ModelTaskTest.php b/tests/TestCase/Shell/Task/ModelTaskTest.php index 1610cf535..37960ca87 100644 --- a/tests/TestCase/Shell/Task/ModelTaskTest.php +++ b/tests/TestCase/Shell/Task/ModelTaskTest.php @@ -43,7 +43,6 @@ class ModelTaskTest extends TestCase */ public $fixtures = [ 'core.users', - 'core.number_trees', 'core.counter_cache_users', 'core.counter_cache_posts', 'core.tags', @@ -54,6 +53,7 @@ class ModelTaskTest extends TestCase 'plugin.bake.bake_tags', 'plugin.bake.category_threads', 'plugin.bake.invitations', + 'plugin.bake.number_trees', ]; /** @@ -782,22 +782,58 @@ public function testGetValidation() $model = TableRegistry::get('BakeArticles'); $result = $this->Task->getValidation($model); $expected = [ - 'bake_user_id' => ['valid' => ['rule' => 'integer', 'allowEmpty' => false]], - 'title' => ['valid' => ['rule' => 'scalar', 'allowEmpty' => false]], - 'body' => ['valid' => ['rule' => 'scalar', 'allowEmpty' => true]], - 'published' => ['valid' => ['rule' => 'boolean', 'allowEmpty' => true]], - 'id' => ['valid' => ['rule' => 'integer', 'allowEmpty' => 'create']] + 'bake_user_id' => [ + 'integer' => ['rule' => 'integer', 'args' => []], + 'requirePresence' => ['rule' => 'requirePresence', 'args' => ['create']], + 'notEmpty' => ['rule' => 'notEmpty', 'args' => []] + ], + 'title' => [ + 'scalar' => ['rule' => 'scalar', 'args' => []], + 'requirePresence' => ['rule' => 'requirePresence', 'args' => ['create']], + 'notEmpty' => ['rule' => 'notEmpty', 'args' => []], + 'maxLength' => ['rule' => 'maxLength', 'args' => [50]] + ], + 'body' => [ + 'scalar' => ['rule' => 'scalar', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => []] + ], + 'published' => [ + 'boolean' => ['rule' => 'boolean', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => []] + ], + 'id' => [ + 'integer' => ['rule' => 'integer', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => ['create']] + ] ]; $this->assertEquals($expected, $result); $model = TableRegistry::get('BakeComments'); $result = $this->Task->getValidation($model); $expected = [ - 'bake_article_id' => ['valid' => ['rule' => 'integer', 'allowEmpty' => false]], - 'bake_user_id' => ['valid' => ['rule' => 'integer', 'allowEmpty' => false]], - 'comment' => ['valid' => ['rule' => 'scalar', 'allowEmpty' => true]], - 'published' => ['valid' => ['rule' => 'scalar', 'allowEmpty' => true]], - 'otherid' => ['valid' => ['rule' => 'integer', 'allowEmpty' => 'create']] + 'bake_article_id' => [ + 'integer' => ['rule' => 'integer', 'args' => []], + 'requirePresence' => ['rule' => 'requirePresence', 'args' => ['create']], + 'notEmpty' => ['rule' => 'notEmpty', 'args' => []] + ], + 'bake_user_id' => [ + 'integer' => ['rule' => 'integer', 'args' => []], + 'requirePresence' => ['rule' => 'requirePresence', 'args' => ['create']], + 'notEmpty' => ['rule' => 'notEmpty', 'args' => []] + ], + 'comment' => [ + 'scalar' => ['rule' => 'scalar', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => []] + ], + 'published' => [ + 'scalar' => ['rule' => 'scalar', 'args' => []], + 'maxLength' => ['rule' => 'maxLength', 'args' => [1]], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => []] + ], + 'otherid' => [ + 'integer' => ['rule' => 'integer', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => ['create']] + ] ]; $this->assertEquals($expected, $result); } @@ -819,7 +855,11 @@ public function testGetValidationUniqueDateField() ]); $result = $this->Task->getValidation($model); $this->assertArrayHasKey('release_date', $result); - $expected = ['valid' => ['rule' => 'dateTime', 'allowEmpty' => false]]; + $expected = [ + 'dateTime' => ['rule' => 'dateTime', 'args' => []], + 'requirePresence' => ['rule' => 'requirePresence', 'args' => ['create']], + 'notEmpty' => ['rule' => 'notEmpty', 'args' => []] + ]; $this->assertEquals($expected, $result['release_date']); } @@ -833,10 +873,24 @@ public function testGetValidationTree() $model = TableRegistry::get('NumberTrees'); $result = $this->Task->getValidation($model); $expected = [ - 'id' => ['valid' => ['rule' => 'integer', 'allowEmpty' => 'create']], - 'name' => ['valid' => ['rule' => 'scalar', 'allowEmpty' => false]], - 'parent_id' => ['valid' => ['rule' => 'integer', 'allowEmpty' => true]], - 'depth' => ['valid' => ['rule' => 'integer', 'allowEmpty' => true]], + 'id' => [ + 'integer' => ['rule' => 'integer', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => ['create']] + ], + 'name' => [ + 'scalar' => ['rule' => 'scalar', 'args' => []], + 'requirePresence' => ['rule' => 'requirePresence', 'args' => ['create']], + 'notEmpty' => ['rule' => 'notEmpty', 'args' => []], + 'maxLength' => ['rule' => 'maxLength', 'args' => [50]] + ], + 'parent_id' => [ + 'integer' => ['rule' => 'integer', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => []] + ], + 'depth' => [ + 'integer' => ['rule' => 'integer', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => []] + ], ]; $this->assertEquals($expected, $result); } @@ -856,10 +910,24 @@ public function testGetValidationExcludeForeignKeys() ]; $result = $this->Task->getValidation($model, $associations); $expected = [ - 'title' => ['valid' => ['rule' => 'scalar', 'allowEmpty' => false]], - 'body' => ['valid' => ['rule' => 'scalar', 'allowEmpty' => true]], - 'published' => ['valid' => ['rule' => 'boolean', 'allowEmpty' => true]], - 'id' => ['valid' => ['rule' => 'integer', 'allowEmpty' => 'create']] + 'title' => [ + 'scalar' => ['rule' => 'scalar', 'args' => []], + 'requirePresence' => ['rule' => 'requirePresence', 'args' => ['create']], + 'notEmpty' => ['rule' => 'notEmpty', 'args' => []], + 'maxLength' => ['rule' => 'maxLength', 'args' => [50]] + ], + 'body' => [ + 'scalar' => ['rule' => 'scalar', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => []] + ], + 'published' => [ + 'boolean' => ['rule' => 'boolean', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => []] + ], + 'id' => [ + 'integer' => ['rule' => 'integer', 'args' => []], + 'allowEmpty' => ['rule' => 'allowEmpty', 'args' => ['create']] + ] ]; $this->assertEquals($expected, $result); } @@ -1076,6 +1144,13 @@ public function testBakeTableValidation() 'valid' => [ 'allowEmpty' => false, 'rule' => 'scalar', + ], + 'maxLength' => [ + 'rule' => 'maxLength', + 'args' => [ + 100, + 'Name must be shorter than 100 characters.' + ] ] ], 'email' => [ diff --git a/tests/TestCase/View/Helper/BakeHelperTest.php b/tests/TestCase/View/Helper/BakeHelperTest.php index 23dfac765..2090e8bf9 100644 --- a/tests/TestCase/View/Helper/BakeHelperTest.php +++ b/tests/TestCase/View/Helper/BakeHelperTest.php @@ -230,4 +230,25 @@ public function testStringifyListWithCommaAtEnd() $spaces . $spaces; $this->assertSame($expected, $result); } + + /** + * test escapeArgument with integers and strings + * + * @return void + */ + public function testEscapeArguments() + { + $arguments = [ + 100, + "foo 'bar'", + 'foo "bar"', + ]; + $result = $this->BakeHelper->escapeArguments($arguments); + $expected = [ + 100, + "'foo \'bar\''", + "'foo \"bar\"'", + ]; + $this->assertSame($expected, $result); + } } diff --git a/tests/comparisons/Model/testBakeAssociationDetectionCategoriesTable.php b/tests/comparisons/Model/testBakeAssociationDetectionCategoriesTable.php index cb2449226..02b8b8029 100644 --- a/tests/comparisons/Model/testBakeAssociationDetectionCategoriesTable.php +++ b/tests/comparisons/Model/testBakeAssociationDetectionCategoriesTable.php @@ -61,6 +61,7 @@ public function validationDefault(Validator $validator) $validator ->scalar('name') + ->maxLength('name', 100) ->requirePresence('name', 'create') ->notEmpty('name'); diff --git a/tests/comparisons/Model/testBakeAssociationDetectionOldProductsTable.php b/tests/comparisons/Model/testBakeAssociationDetectionOldProductsTable.php index d6d7700bc..80d23aedb 100644 --- a/tests/comparisons/Model/testBakeAssociationDetectionOldProductsTable.php +++ b/tests/comparisons/Model/testBakeAssociationDetectionOldProductsTable.php @@ -53,6 +53,7 @@ public function validationDefault(Validator $validator) $validator ->scalar('name') + ->maxLength('name', 100) ->requirePresence('name', 'create') ->notEmpty('name'); diff --git a/tests/comparisons/Model/testBakeAssociationDetectionProductsTable.php b/tests/comparisons/Model/testBakeAssociationDetectionProductsTable.php index 5459f921d..426095610 100644 --- a/tests/comparisons/Model/testBakeAssociationDetectionProductsTable.php +++ b/tests/comparisons/Model/testBakeAssociationDetectionProductsTable.php @@ -65,6 +65,7 @@ public function validationDefault(Validator $validator) $validator ->scalar('name') + ->maxLength('name', 100) ->requirePresence('name', 'create') ->notEmpty('name'); diff --git a/tests/comparisons/Model/testBakeTableValidation.php b/tests/comparisons/Model/testBakeTableValidation.php index 92d524136..1a3d6f9fa 100644 --- a/tests/comparisons/Model/testBakeTableValidation.php +++ b/tests/comparisons/Model/testBakeTableValidation.php @@ -48,7 +48,8 @@ public function validationDefault(Validator $validator) $validator ->scalar('name') ->requirePresence('name', 'create') - ->notEmpty('name'); + ->notEmpty('name') + ->maxLength('name', 100, 'Name must be shorter than 100 characters.'); $validator ->email('email')