Merge pull request #8908 from ionas/fixExistsIn-New

Add allowPartialNulls-flag to ExistsIn() that matches SQLs behavior of composite foreign keys having some nullable nulls

src/ORM/Rule/ExistsIn.php

@@ -40,14 +40,28 @@ 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;
     }
@@ -99,6 +113,16 @@ class ExistsIn
             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
@@ -107,11 +131,12 @@ class ExistsIn
             $primary,
             $entity->extract($this->_fields)
         );
+
         return $target->exists($conditions);
     }
 
     /**
-     * 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.

src/ORM/RulesChecker.php

@@ -78,14 +78,26 @@ class RulesChecker extends BaseRulesChecker
      * $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 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, $message = null)
     {
+        $options = [];
+        if (is_array($message)) {
+            $options = $message + ['message' => null];
+            $message = $options['message'];
+            unset($options['message']);
+        }
+
         if (!$message) {
             if ($this->_useI18n) {
                 $message = __d('cake', 'This value does not exist');
@@ -95,7 +107,7 @@ class RulesChecker extends BaseRulesChecker
         }
 
         $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'));
     }
 
     /**

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'
     ];
 
     /**
@@ -884,6 +884,305 @@ class RulesCheckerIntegrationTest extends TestCase
     }
 
     /**
+     * Tests new allowPartialNulls flag with author id set to null
+     *
+     * @return
+     */
+    public function testExistsInAllowPartialNullsWithAuthorIdNullA()
+    {
+        $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($entity));
+    }
+
+    /**
+     * Tests new allowPartialNulls flag with author id set to null
+     *
+     * @return
+     */
+    public function testExistsInAllowPartialNullsWithAuthorIdNullB()
+    {
+        $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 testExistsInAllowPartialNullsWithAuthorIdNullC()
+    {
+        $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($entity));
+    }
+
+    /**
+     * Tests new allowPartialNulls flag with author id set to null
+     *
+     * @return
+     */
+    public function testExistsInAllowPartialNullsWithAuthorIdNullD()
+    {
+        $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 testExistsInAllowPartialNullsWithAuthorIdNullE()
+    {
+        $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));
+    }
+
+    /**
+     * Tests new allowPartialNulls flag with author id set to 1
+     *
+     * @return
+     */
+    public function testExistsInAllowPartialNullsWithAuthorId1A()
+    {
+        $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($entity));
+    }
+
+    /**
+     * Tests new allowPartialNulls flag with author id set to 1
+     *
+     * @return
+     */
+    public function testExistsInAllowPartialNullsWithAuthorIdB()
+    {
+        $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($entity));
+    }
+
+    /**
+     * Tests new allowPartialNulls flag with author id set to 1
+     *
+     * @return
+     */
+    public function testExistsInAllowPartialNullsWithAuthorId1C()
+    {
+        $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($entity));
+    }
+
+    /**
+     * Tests new allowPartialNulls flag with author id set to 1
+     *
+     * @return
+     */
+    public function testExistsInAllowPartialNullsWithAuthorId1E()
+    {
+        $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 error']));
+        $this->assertInstanceOf('Cake\ORM\Entity', $table->save($entity));
+    }
+
+    /**
+     * Tests new allowPartialNulls flag with author id set to 1
+     *
+     * @return
+     */
+    public function testExistsInAllowPartialNullsWithAuthorId1F()
+    {
+        $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 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
      *
      * @group delete