From 2decc4eb16d28aac5696a9a50068caee96cc9783 Mon Sep 17 00:00:00 2001 From: Jonas Date: Mon, 30 May 2016 08:13:36 +0000 Subject: [PATCH 01/10] Add allowPartialNulls flag to existsIn that matches SQLs behavior of composite foreign keys with nullable nulls - set 'allowPartialNulls' true to accept composite foreign keys where one or more nullable columns are null. Ths Retargets #8903 to 3.next - in a clean way --- src/ORM/Rule/ExistsIn.php | 39 ++++++++++++- src/ORM/RulesChecker.php | 18 +++++- .../ORM/RulesCheckerIntegrationTest.php | 56 ++++++++++++++++++- 3 files changed, 108 insertions(+), 5 deletions(-) diff --git a/src/ORM/Rule/ExistsIn.php b/src/ORM/Rule/ExistsIn.php index 286075be79a..267a6b12380 100644 --- a/src/ORM/Rule/ExistsIn.php +++ b/src/ORM/Rule/ExistsIn.php @@ -39,15 +39,29 @@ class ExistsIn */ protected $_repository; + /** + * Options for the constructor + * + * @var array + */ + protected $_options = []; + /** * Constructor. * + * Available option for $options is 'allowPartialNulls' flag. + * Set to true to accept composite foreign keys where one or more nullable columns are null.' + * * @param string|array $fields The field or fields to check existence as primary key. * @param object|string $repository The repository where the field will be looked for, * or the association name for the repository. + * @param array $options The options that modify the rules behavior. */ - public function __construct($fields, $repository) + public function __construct($fields, $repository, array $options = []) { + $options += ['allowPartialNulls' => false]; + $this->_options = $options; + $this->_fields = (array)$fields; $this->_repository = $repository; } @@ -96,6 +110,11 @@ public function __invoke(EntityInterface $entity, array $options) return true; } + if ($this->_options['allowPartialNulls'] === true + && $this->_checkPartialSchemaNulls($entity, $source) === true + ) { + return true; + } if ($this->_fieldsAreNull($entity, $source)) { return true; } @@ -129,4 +148,22 @@ protected function _fieldsAreNull($entity, $source) } return $nulls === count($this->_fields); } + + /** + * Check whether there are nullable nulls in at least one part of the foreign key. + * + * @param \Cake\Datasource\EntityInterface $entity The entity to check. + * @param \Cake\ORM\Table $source The table to use schema from. + * @return bool + */ + protected function _checkPartialSchemaNulls($entity, $source) + { + $schema = $source->schema(); + foreach ($this->_fields as $field) { + if ($schema->isNullable($field) === true && $entity->get($field) === null) { + return true; + } + } + return false; + } } diff --git a/src/ORM/RulesChecker.php b/src/ORM/RulesChecker.php index e46c8f07887..2eadba6e93d 100644 --- a/src/ORM/RulesChecker.php +++ b/src/ORM/RulesChecker.php @@ -71,14 +71,26 @@ public function isUnique(array $fields, $message = null) * $rules->add($rules->existsIn('site_id', new SitesTable(), 'Invalid Site')); * ``` * + * Available $options are error 'message' and 'allowPartialNulls' flag. + * 'message' sets a custom error message. + * Set 'allowPartialNulls' to true to accept composite foreign keys where one or more nullable columns are null.' + * * @param string|array $field The field or list of fields to check for existence by * primary key lookup in the other table. * @param object|string $table The table name where the fields existence will be checked. - * @param string|null $message The error message to show in case the rule does not pass. + * @param array|string|null $options List of options or error message string to show in case the rule does not pass. * @return callable */ - public function existsIn($field, $table, $message = null) + public function existsIn($field, $table, $options = null) { + if (is_string($options)) { + $options = ['message' => $options]; + } + + $options = (array)$options + ['message' => null]; + $message = $options['message']; + unset($options['message']); + if (!$message) { if ($this->_useI18n) { $message = __d('cake', 'This value does not exist'); @@ -88,7 +100,7 @@ public function existsIn($field, $table, $message = null) } $errorField = is_string($field) ? $field : current($field); - return $this->_addError(new ExistsIn($field, $table), '_existsIn', compact('errorField', 'message')); + return $this->_addError(new ExistsIn($field, $table, $options), '_existsIn', compact('errorField', 'message')); } /** diff --git a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php index 5467df72d6b..6cd908ff000 100644 --- a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php +++ b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php @@ -32,7 +32,7 @@ class RulesCheckerIntegrationTest extends TestCase */ public $fixtures = [ 'core.articles', 'core.articles_tags', 'core.authors', 'core.tags', - 'core.special_tags', 'core.categories' + 'core.special_tags', 'core.categories', 'core.site_articles', 'core.site_authors' ]; /** @@ -827,6 +827,60 @@ public function testExistsInErrorWithArrayField() $this->assertEquals(['_existsIn' => 'This value does not exist'], $entity->errors('author_id')); } + /** + * Tests new allowPartialNulls flag with author id set to null + * + * @return + */ + public function testExistsInAllowSqlNullsWithParentIdNull() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => null, + 'site_id' => 1, + 'name' => 'New Site Article without Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', ['allowPartialNulls' => true])); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save(clone $entity)); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', ['allowPartialNulls' => false])); + $this->assertFalse($table->save(clone $entity)); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors')); + $this->assertFalse($table->save(clone $entity)); + } + + /** + * Tests new allowPartialNulls flag with author id set to 1 + * + * @return + */ + public function testExistsInAllowSqlNullsWithParentId1() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => 1, + 'site_id' => 1, + 'name' => 'New Site Article with Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', ['allowPartialNulls' => true])); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save(clone $entity)); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', ['allowPartialNulls' => false])); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save(clone $entity)); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors')); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save(clone $entity)); + } + /** * Tests using rules to prevent delete operations * From 644a7422f044c36566361a19d2554b7885af3e7a Mon Sep 17 00:00:00 2001 From: Jonas Date: Mon, 30 May 2016 08:16:51 +0000 Subject: [PATCH 02/10] remove typo --- src/ORM/Rule/ExistsIn.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ORM/Rule/ExistsIn.php b/src/ORM/Rule/ExistsIn.php index 267a6b12380..8a2bc9141ce 100644 --- a/src/ORM/Rule/ExistsIn.php +++ b/src/ORM/Rule/ExistsIn.php @@ -50,7 +50,7 @@ class ExistsIn * Constructor. * * Available option for $options is 'allowPartialNulls' flag. - * Set to true to accept composite foreign keys where one or more nullable columns are null.' + * Set to true to accept composite foreign keys where one or more nullable columns are null. * * @param string|array $fields The field or fields to check existence as primary key. * @param object|string $repository The repository where the field will be looked for, From 363ff8e238d816dbd7a8541c18add1ba207085f3 Mon Sep 17 00:00:00 2001 From: Jonas Date: Mon, 30 May 2016 08:41:00 +0000 Subject: [PATCH 03/10] shorter boolean check --- src/ORM/Rule/ExistsIn.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ORM/Rule/ExistsIn.php b/src/ORM/Rule/ExistsIn.php index 8a2bc9141ce..cbcdfeb6a0e 100644 --- a/src/ORM/Rule/ExistsIn.php +++ b/src/ORM/Rule/ExistsIn.php @@ -160,7 +160,7 @@ protected function _checkPartialSchemaNulls($entity, $source) { $schema = $source->schema(); foreach ($this->_fields as $field) { - if ($schema->isNullable($field) === true && $entity->get($field) === null) { + if ($schema->isNullable($field) && $entity->get($field) === null) { return true; } } From a248513cdf8f237dbe0db5c44ee1a9df24fcd6ce Mon Sep 17 00:00:00 2001 From: Jonas Date: Mon, 30 May 2016 08:49:56 +0000 Subject: [PATCH 04/10] shorter boolean checks --- src/ORM/Rule/ExistsIn.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ORM/Rule/ExistsIn.php b/src/ORM/Rule/ExistsIn.php index cbcdfeb6a0e..17fd9e5f74a 100644 --- a/src/ORM/Rule/ExistsIn.php +++ b/src/ORM/Rule/ExistsIn.php @@ -110,9 +110,7 @@ public function __invoke(EntityInterface $entity, array $options) return true; } - if ($this->_options['allowPartialNulls'] === true - && $this->_checkPartialSchemaNulls($entity, $source) === true - ) { + if ($this->_options['allowPartialNulls'] && $this->_checkPartialSchemaNulls($entity, $source)) { return true; } if ($this->_fieldsAreNull($entity, $source)) { From 7f480afd72dfaee0920d24491f3934db76b19568 Mon Sep 17 00:00:00 2001 From: Jonas Date: Fri, 3 Jun 2016 19:03:03 +0000 Subject: [PATCH 05/10] make sure error messages are tested, make sure that object mutation does not screw up tests --- src/ORM/RulesChecker.php | 16 +- .../ORM/RulesCheckerIntegrationTest.php | 191 +++++++++++++++++- 2 files changed, 189 insertions(+), 18 deletions(-) diff --git a/src/ORM/RulesChecker.php b/src/ORM/RulesChecker.php index 2eadba6e93d..c5802ef1905 100644 --- a/src/ORM/RulesChecker.php +++ b/src/ORM/RulesChecker.php @@ -78,19 +78,19 @@ public function isUnique(array $fields, $message = null) * @param string|array $field The field or list of fields to check for existence by * primary key lookup in the other table. * @param object|string $table The table name where the fields existence will be checked. - * @param array|string|null $options List of options or error message string to show in case the rule does not pass. + * @param @param string|array|null $message The error message to show in case the rule does not pass. Can + * also be an array of options. When an array, the 'message' key can be used to provide a message. * @return callable */ - public function existsIn($field, $table, $options = null) + public function existsIn($field, $table, $message = null) { - if (is_string($options)) { - $options = ['message' => $options]; + $options = []; + if (is_array($message)) { + $options = $message + ['message' => null]; + $message = $options['message']; + unset($options['message']); } - $options = (array)$options + ['message' => null]; - $message = $options['message']; - unset($options['message']); - if (!$message) { if ($this->_useI18n) { $message = __d('cake', 'This value does not exist'); diff --git a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php index 6cd908ff000..c2a22cd3616 100644 --- a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php +++ b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php @@ -832,7 +832,7 @@ public function testExistsInErrorWithArrayField() * * @return */ - public function testExistsInAllowSqlNullsWithParentIdNull() + public function testExistsInAllowSqlNullsWithParentIdNullA() { $entity = new Entity([ 'id' => 10, @@ -844,14 +844,103 @@ public function testExistsInAllowSqlNullsWithParentIdNull() $table->belongsTo('SiteAuthors'); $rules = $table->rulesChecker(); - $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', ['allowPartialNulls' => true])); - $this->assertInstanceOf('Cake\ORM\Entity', $table->save(clone $entity)); + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ + 'allowPartialNulls' => true + ])); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save($entity)); + } - $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', ['allowPartialNulls' => false])); - $this->assertFalse($table->save(clone $entity)); + /** + * Tests new allowPartialNulls flag with author id set to null + * + * @return + */ + public function testExistsInAllowSqlNullsWithParentIdNullB() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => null, + 'site_id' => 1, + 'name' => 'New Site Article without Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ + 'allowPartialNulls' => false + ])); + $this->assertFalse($table->save($entity)); + } + + /** + * Tests new allowPartialNulls flag with author id set to null + * + * @return + */ + public function testExistsInAllowSqlNullsWithParentIdNullC() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => null, + 'site_id' => 1, + 'name' => 'New Site Article without Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors')); - $this->assertFalse($table->save(clone $entity)); + $this->assertFalse($table->save($entity)); + } + + /** + * Tests new allowPartialNulls flag with author id set to null + * + * @return + */ + public function testExistsInAllowSqlNullsWithParentIdNullD() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => null, + 'site_id' => 1, + 'name' => 'New Site Article without Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ + 'allowPartialNulls' => false, + 'message' => 'Niente' + ])); + $this->assertFalse($table->save($entity)); + $this->assertEquals(['author_id' => ['_existsIn' => 'Niente']], $entity->errors()); + } + + /** + * Tests new allowPartialNulls flag with author id set to null + * + * @return + */ + public function testExistsInAllowSqlNullsWithParentIdNullE() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => null, + 'site_id' => 1, + 'name' => 'New Site Article without Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ + 'allowPartialNulls' => true, + 'message' => 'Niente' + ])); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save($entity)); } /** @@ -859,7 +948,7 @@ public function testExistsInAllowSqlNullsWithParentIdNull() * * @return */ - public function testExistsInAllowSqlNullsWithParentId1() + public function testExistsInAllowSqlNullsWithParentId1A() { $entity = new Entity([ 'id' => 10, @@ -872,13 +961,95 @@ public function testExistsInAllowSqlNullsWithParentId1() $rules = $table->rulesChecker(); $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', ['allowPartialNulls' => true])); - $this->assertInstanceOf('Cake\ORM\Entity', $table->save(clone $entity)); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save($entity)); + } + + /** + * Tests new allowPartialNulls flag with author id set to 1 + * + * @return + */ + public function testExistsInAllowSqlNullsWithParentIdB() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => 1, + 'site_id' => 1, + 'name' => 'New Site Article with Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', ['allowPartialNulls' => false])); - $this->assertInstanceOf('Cake\ORM\Entity', $table->save(clone $entity)); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save($entity)); + } + + /** + * Tests new allowPartialNulls flag with author id set to 1 + * + * @return + */ + public function testExistsInAllowSqlNullsWithParentId1C() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => 1, + 'site_id' => 1, + 'name' => 'New Site Article with Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors')); - $this->assertInstanceOf('Cake\ORM\Entity', $table->save(clone $entity)); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save($entity)); + } + + /** + * Tests new allowPartialNulls flag with author id set to 1 + * + * @return + */ + public function testExistsInAllowSqlNullsWithParentId1E() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => 1, + 'site_id' => 1, + 'name' => 'New Site Article with Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ + 'allowPartialNulls' => true, + 'message' => 'will not occur'])); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save($entity)); + } + + /** + * Tests new allowPartialNulls flag with author id set to 1 + * + * @return + */ + public function testExistsInAllowSqlNullsWithParentId1G() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => 1, + 'site_id' => 1, + 'name' => 'New Site Article with Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ + 'allowPartialNulls' => false, + 'message' => 'will not occur'])); + $this->assertInstanceOf('Cake\ORM\Entity', $table->save($entity)); } /** From 98447aa90634c712fc3daf43cb43f9d9f99e6804 Mon Sep 17 00:00:00 2001 From: Jonas Date: Fri, 3 Jun 2016 21:43:58 +0000 Subject: [PATCH 06/10] fix unit test names --- .../ORM/RulesCheckerIntegrationTest.php | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php index c2a22cd3616..33600788700 100644 --- a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php +++ b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php @@ -832,7 +832,7 @@ public function testExistsInErrorWithArrayField() * * @return */ - public function testExistsInAllowSqlNullsWithParentIdNullA() + public function testExistsInAllowPartialNullsWithAuthorIdNullA() { $entity = new Entity([ 'id' => 10, @@ -855,7 +855,7 @@ public function testExistsInAllowSqlNullsWithParentIdNullA() * * @return */ - public function testExistsInAllowSqlNullsWithParentIdNullB() + public function testExistsInAllowPartialNullsWithAuthorIdNullB() { $entity = new Entity([ 'id' => 10, @@ -878,7 +878,7 @@ public function testExistsInAllowSqlNullsWithParentIdNullB() * * @return */ - public function testExistsInAllowSqlNullsWithParentIdNullC() + public function testExistsInAllowPartialNullsWithAuthorIdNullC() { $entity = new Entity([ 'id' => 10, @@ -899,7 +899,7 @@ public function testExistsInAllowSqlNullsWithParentIdNullC() * * @return */ - public function testExistsInAllowSqlNullsWithParentIdNullD() + public function testExistsInAllowPartialNullsWithAuthorIdNullD() { $entity = new Entity([ 'id' => 10, @@ -924,7 +924,7 @@ public function testExistsInAllowSqlNullsWithParentIdNullD() * * @return */ - public function testExistsInAllowSqlNullsWithParentIdNullE() + public function testExistsInAllowPartialNullsWithAuthorIdNullE() { $entity = new Entity([ 'id' => 10, @@ -948,7 +948,7 @@ public function testExistsInAllowSqlNullsWithParentIdNullE() * * @return */ - public function testExistsInAllowSqlNullsWithParentId1A() + public function testExistsInAllowPartialNullsWithAuthorId1A() { $entity = new Entity([ 'id' => 10, @@ -969,7 +969,7 @@ public function testExistsInAllowSqlNullsWithParentId1A() * * @return */ - public function testExistsInAllowSqlNullsWithParentIdB() + public function testExistsInAllowPartialNullsWithAuthorIdB() { $entity = new Entity([ 'id' => 10, @@ -990,7 +990,7 @@ public function testExistsInAllowSqlNullsWithParentIdB() * * @return */ - public function testExistsInAllowSqlNullsWithParentId1C() + public function testExistsInAllowPartialNullsWithAuthorId1C() { $entity = new Entity([ 'id' => 10, @@ -1011,7 +1011,7 @@ public function testExistsInAllowSqlNullsWithParentId1C() * * @return */ - public function testExistsInAllowSqlNullsWithParentId1E() + public function testExistsInAllowPartialNullsWithAuthorId1E() { $entity = new Entity([ 'id' => 10, @@ -1034,7 +1034,7 @@ public function testExistsInAllowSqlNullsWithParentId1E() * * @return */ - public function testExistsInAllowSqlNullsWithParentId1G() + public function testExistsInAllowPartialNullsWithAuthorId1G() { $entity = new Entity([ 'id' => 10, From 15bbb994f0372924b95ac4906b22e656b31efa2e Mon Sep 17 00:00:00 2001 From: Jonas Date: Sat, 11 Jun 2016 13:48:15 +0000 Subject: [PATCH 07/10] CS:remove whitespaces --- tests/TestCase/ORM/RulesCheckerIntegrationTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php index 33600788700..4f872157eb9 100644 --- a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php +++ b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php @@ -32,7 +32,7 @@ class RulesCheckerIntegrationTest extends TestCase */ public $fixtures = [ 'core.articles', 'core.articles_tags', 'core.authors', 'core.tags', - 'core.special_tags', 'core.categories', 'core.site_articles', 'core.site_authors' + 'core.special_tags', 'core.categories', 'core.site_articles', 'core.site_authors' ]; /** From acbacd175284654dda0f44aa458147cf957443aa Mon Sep 17 00:00:00 2001 From: Jonas Date: Fri, 17 Jun 2016 12:50:30 +0000 Subject: [PATCH 08/10] add convenient checks for not-nullable nulls for "allowPartialNulls" flag --- src/ORM/Rule/ExistsIn.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/ORM/Rule/ExistsIn.php b/src/ORM/Rule/ExistsIn.php index 17fd9e5f74a..6e2fb6cef6f 100644 --- a/src/ORM/Rule/ExistsIn.php +++ b/src/ORM/Rule/ExistsIn.php @@ -129,7 +129,7 @@ public function __invoke(EntityInterface $entity, array $options) } /** - * Check whether or not the entity fields are nullable and null. + * Checks whether or not the given entity fields are nullable and null. * * @param \Cake\Datasource\EntityInterface $entity The entity to check. * @param \Cake\ORM\Table $source The table to use schema from. @@ -148,7 +148,8 @@ protected function _fieldsAreNull($entity, $source) } /** - * Check whether there are nullable nulls in at least one part of the foreign key. + * Checks whether or not the give entity fields are null and map to schema NULL + * or are not null and map to schema NOT NULL. * * @param \Cake\Datasource\EntityInterface $entity The entity to check. * @param \Cake\ORM\Table $source The table to use schema from. @@ -158,10 +159,15 @@ protected function _checkPartialSchemaNulls($entity, $source) { $schema = $source->schema(); foreach ($this->_fields as $field) { - if ($schema->isNullable($field) && $entity->get($field) === null) { - return true; + $isNullable = $schema->isNullable($field); + $value = $entity->get($field); + if (!$isNullable && $value === null) { + return false; + } + if ($isNullable && $value !== null) { + return false; } } - return false; + return true; } } From 1ec0ab73c0e9dd10893b38a374717fba5e2c8ef1 Mon Sep 17 00:00:00 2001 From: Jonas Date: Fri, 17 Jun 2016 12:54:36 +0000 Subject: [PATCH 09/10] fix docblocks --- src/ORM/RulesChecker.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ORM/RulesChecker.php b/src/ORM/RulesChecker.php index c5802ef1905..6aa70ec4891 100644 --- a/src/ORM/RulesChecker.php +++ b/src/ORM/RulesChecker.php @@ -73,12 +73,12 @@ public function isUnique(array $fields, $message = null) * * Available $options are error 'message' and 'allowPartialNulls' flag. * 'message' sets a custom error message. - * Set 'allowPartialNulls' to true to accept composite foreign keys where one or more nullable columns are null.' + * Set 'allowPartialNulls' to true to accept composite foreign keys where one or more nullable columns are null. * * @param string|array $field The field or list of fields to check for existence by * primary key lookup in the other table. * @param object|string $table The table name where the fields existence will be checked. - * @param @param string|array|null $message The error message to show in case the rule does not pass. Can + * @param string|array|null $message The error message to show in case the rule does not pass. Can * also be an array of options. When an array, the 'message' key can be used to provide a message. * @return callable */ From 230e06fca7bc171757f12e8738a3698966c34e09 Mon Sep 17 00:00:00 2001 From: Jonas Date: Tue, 28 Jun 2016 15:06:23 +0000 Subject: [PATCH 10/10] 3rd allowPartialNulls implementation --- src/ORM/Rule/ExistsIn.php | 38 +++------ .../ORM/RulesCheckerIntegrationTest.php | 80 ++++++++++++++++++- 2 files changed, 88 insertions(+), 30 deletions(-) diff --git a/src/ORM/Rule/ExistsIn.php b/src/ORM/Rule/ExistsIn.php index 6e2fb6cef6f..8440e48cd6e 100644 --- a/src/ORM/Rule/ExistsIn.php +++ b/src/ORM/Rule/ExistsIn.php @@ -110,13 +110,20 @@ public function __invoke(EntityInterface $entity, array $options) return true; } - if ($this->_options['allowPartialNulls'] && $this->_checkPartialSchemaNulls($entity, $source)) { - return true; - } if ($this->_fieldsAreNull($entity, $source)) { return true; } + if ($this->_options['allowPartialNulls']) { + $schema = $source->schema(); + foreach ($this->_fields as $i => $field) { + if ($schema->column($field) && $schema->isNullable($field) && $entity->get($field) === null) { + unset($bindingKey[$i]); + unset($this->_fields[$i]); + } + } + } + $primary = array_map( [$target, 'aliasField'], $bindingKey @@ -125,6 +132,7 @@ public function __invoke(EntityInterface $entity, array $options) $primary, $entity->extract($this->_fields) ); + return $target->exists($conditions); } @@ -146,28 +154,4 @@ protected function _fieldsAreNull($entity, $source) } return $nulls === count($this->_fields); } - - /** - * Checks whether or not the give entity fields are null and map to schema NULL - * or are not null and map to schema NOT NULL. - * - * @param \Cake\Datasource\EntityInterface $entity The entity to check. - * @param \Cake\ORM\Table $source The table to use schema from. - * @return bool - */ - protected function _checkPartialSchemaNulls($entity, $source) - { - $schema = $source->schema(); - foreach ($this->_fields as $field) { - $isNullable = $schema->isNullable($field); - $value = $entity->get($field); - if (!$isNullable && $value === null) { - return false; - } - if ($isNullable && $value !== null) { - return false; - } - } - return true; - } } diff --git a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php index 4f872157eb9..48b9a7dc5b3 100644 --- a/tests/TestCase/ORM/RulesCheckerIntegrationTest.php +++ b/tests/TestCase/ORM/RulesCheckerIntegrationTest.php @@ -1025,7 +1025,7 @@ public function testExistsInAllowPartialNullsWithAuthorId1E() $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ 'allowPartialNulls' => true, - 'message' => 'will not occur'])); + 'message' => 'will not error'])); $this->assertInstanceOf('Cake\ORM\Entity', $table->save($entity)); } @@ -1034,7 +1034,7 @@ public function testExistsInAllowPartialNullsWithAuthorId1E() * * @return */ - public function testExistsInAllowPartialNullsWithAuthorId1G() + public function testExistsInAllowPartialNullsWithAuthorId1F() { $entity = new Entity([ 'id' => 10, @@ -1048,10 +1048,84 @@ public function testExistsInAllowPartialNullsWithAuthorId1G() $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ 'allowPartialNulls' => false, - 'message' => 'will not occur'])); + 'message' => 'will not error'])); $this->assertInstanceOf('Cake\ORM\Entity', $table->save($entity)); } + /** + * Tests new allowPartialNulls flag with author id set to 99999999 (does not exist) + * + * @return + */ + public function testExistsInAllowPartialNullsWithAuthorId1G() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => 99999999, + 'site_id' => 1, + 'name' => 'New Site Article with Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ + 'allowPartialNulls' => true, + 'message' => 'will error'])); + $this->assertFalse($table->save($entity)); + $this->assertEquals(['author_id' => ['_existsIn' => 'will error']], $entity->errors()); + } + + /** + * Tests new allowPartialNulls flag with author id set to 99999999 (does not exist) + * and site_id set to 99999999 (does not exist) + * + * @return + */ + public function testExistsInAllowPartialNullsWithAuthorId1H() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => 99999999, + 'site_id' => 99999999, + 'name' => 'New Site Article with Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ + 'allowPartialNulls' => true, + 'message' => 'will error'])); + $this->assertFalse($table->save($entity)); + $this->assertEquals(['author_id' => ['_existsIn' => 'will error']], $entity->errors()); + } + + /** + * Tests new allowPartialNulls flag with author id set to 1 (does exist) + * and site_id set to 99999999 (does not exist) + * + * @return + */ + public function testExistsInAllowPartialNullsWithAuthorId1I() + { + $entity = new Entity([ + 'id' => 10, + 'author_id' => 1, + 'site_id' => 99999999, + 'name' => 'New Site Article with Author', + ]); + $table = TableRegistry::get('SiteArticles'); + $table->belongsTo('SiteAuthors'); + $rules = $table->rulesChecker(); + + $rules->add($rules->existsIn(['author_id', 'site_id'], 'SiteAuthors', [ + 'allowPartialNulls' => true, + 'message' => 'will error'])); + $this->assertFalse($table->save($entity)); + $this->assertEquals(['author_id' => ['_existsIn' => 'will error']], $entity->errors()); + } + /** * Tests using rules to prevent delete operations *