Browse Source

Merge pull request #15226 from cakephp/unique-nulls

Add allowMultipleNulls to IsUnique rule check
Mark Story 5 years ago
parent
commit
e9c6b5ee46

+ 22 - 2
src/ORM/Rule/IsUnique.php

@@ -31,13 +31,28 @@ class IsUnique
     protected $_fields;
 
     /**
+     * The unique check options
+     *
+     * @var array
+     */
+    protected $_options = [
+        'allowMultipleNulls' => false,
+    ];
+
+    /**
      * Constructor.
      *
+     * ### Options
+     *
+     * - `allowMultipleNulls` Allows any field to have multiple null values. Defaults to false.
+     *
      * @param string[] $fields The list of fields to check uniqueness for
+     * @param array $options The options for unique checks.
      */
-    public function __construct(array $fields)
+    public function __construct(array $fields, array $options = [])
     {
         $this->_fields = $fields;
+        $this->_options = $options + $this->_options;
     }
 
     /**
@@ -54,8 +69,13 @@ class IsUnique
             return true;
         }
 
+        $fields = $entity->extract($this->_fields);
+        if ($this->_options['allowMultipleNulls'] && array_filter($fields, 'is_null')) {
+            return true;
+        }
+
         $alias = $options['repository']->getAlias();
-        $conditions = $this->_alias($alias, $entity->extract($this->_fields));
+        $conditions = $this->_alias($alias, $fields);
         if ($entity->isNew() === false) {
             $keys = (array)$options['repository']->getPrimaryKey();
             $keys = $this->_alias($alias, $entity->extract($keys));

+ 10 - 2
src/ORM/RulesChecker.php

@@ -37,12 +37,16 @@ class RulesChecker extends BaseRulesChecker
      * Returns a callable that can be used as a rule for checking the uniqueness of a value
      * in the table.
      *
-     * ### Example:
+     * ### Example
      *
      * ```
      * $rules->add($rules->isUnique(['email'], 'The email should be unique'));
      * ```
      *
+     * ### Options
+     *
+     * - `allowMultipleNulls` Allows any field to have multiple null values. Defaults to false.
+     *
      * @param string[] $fields The list of fields to check for uniqueness.
      * @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.
@@ -50,6 +54,10 @@ class RulesChecker extends BaseRulesChecker
      */
     public function isUnique(array $fields, $message = null): RuleInvoker
     {
+        $options = is_array($message) ? $message : ['message' => $message];
+        $message = $options['message'] ?? null;
+        unset($options['message']);
+
         if (!$message) {
             if ($this->_useI18n) {
                 $message = __d('cake', 'This value is already in use');
@@ -60,7 +68,7 @@ class RulesChecker extends BaseRulesChecker
 
         $errorField = current($fields);
 
-        return $this->_addError(new IsUnique($fields), '_isUnique', compact('errorField', 'message'));
+        return $this->_addError(new IsUnique($fields, $options), '_isUnique', compact('errorField', 'message'));
     }
 
     /**

+ 47 - 0
tests/Fixture/UniqueAuthorsFixture.php

@@ -0,0 +1,47 @@
+<?php
+/**
+ * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakephp.org CakePHP(tm) Project
+ * @since         4.2.0
+ * @license       https://opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Test\Fixture;
+
+use Cake\TestSuite\Fixture\TestFixture;
+
+/**
+ * Tables of unique author ids
+ */
+class UniqueAuthorsFixture extends TestFixture
+{
+    /**
+     * fields property
+     *
+     * @var array
+     */
+    public $fields = [
+        'id' => ['type' => 'integer'],
+        'first_author_id' => ['type' => 'integer', 'null' => true],
+        'second_author_id' => ['type' => 'integer', 'null' => false],
+        '_constraints' => [
+            'primary' => ['type' => 'primary', 'columns' => ['id']],
+            'nullable_non_nullable_unique' => ['type' => 'unique', 'columns' => ['first_author_id', 'second_author_id']],
+        ],
+    ];
+
+    /**
+     * records property
+     *
+     * @var array
+     */
+    public $records = [
+        ['first_author_id' => null, 'second_author_id' => 1],
+    ];
+}

+ 30 - 56
tests/TestCase/ORM/RulesCheckerIntegrationTest.php

@@ -16,6 +16,8 @@ declare(strict_types=1);
  */
 namespace Cake\Test\TestCase\ORM;
 
+use Cake\Database\Driver\Sqlserver;
+use Cake\Datasource\ConnectionManager;
 use Cake\Datasource\EntityInterface;
 use Cake\Event\EventInterface;
 use Cake\I18n\I18n;
@@ -37,7 +39,7 @@ class RulesCheckerIntegrationTest extends TestCase
     protected $fixtures = [
         'core.Articles', 'core.ArticlesTags', 'core.Authors', 'core.Comments', 'core.Tags',
         'core.SpecialTags', 'core.Categories', 'core.SiteArticles', 'core.SiteAuthors',
-        'core.Comments',
+        'core.Comments', 'core.UniqueAuthors',
     ];
 
     /**
@@ -394,86 +396,58 @@ class RulesCheckerIntegrationTest extends TestCase
     }
 
     /**
-     * Tests isUnique with allowMultipleNulls
+     * Tests isUnique with non-unique null values
      *
-     * @group save
      * @return void
      */
-    public function testIsUniqueAllowMultipleNulls()
+    public function testIsUniqueNonUniqueNulls()
     {
-        $entity = new Entity([
-            'article_id' => 11,
-            'tag_id' => 11,
-            'author_id' => null,
-        ]);
-
-        $table = $this->getTableLocator()->get('SpecialTags');
+        $table = $this->getTableLocator()->get('UniqueAuthors');
         $rules = $table->rulesChecker();
-        $rules->add($rules->isUnique(['author_id'], 'All fields are required'));
+        $rules->add($rules->isUnique(
+            ['first_author_id', 'second_author_id']
+        ));
 
+        $entity = new Entity([
+            'first_author_id' => null,
+            'second_author_id' => 1,
+        ]);
         $this->assertFalse($table->save($entity));
-        $this->assertEquals(['_isUnique' => 'All fields are required'], $entity->getError('author_id'));
-
-        $entity->author_id = 11;
-        $this->assertSame($entity, $table->save($entity));
-
-        $entity = $table->get(1);
-        $entity->setDirty('author_id', true);
-        $this->assertSame($entity, $table->save($entity));
+        $this->assertEquals(['first_author_id' => ['_isUnique' => 'This value is already in use']], $entity->getErrors());
     }
 
     /**
-     * Tests isUnique with multiple fields and allowMultipleNulls
+     * Tests isUnique with allowMultipleNulls
      *
      * @group save
      * @return void
      */
-    public function testIsUniqueMultipleFieldsAllowMultipleNulls()
+    public function testIsUniqueAllowMultipleNulls()
     {
-        $entity = new Entity([
-            'article_id' => 10,
-            'tag_id' => 12,
-            'author_id' => null,
-        ]);
+        $this->skipIf(ConnectionManager::get('test')->getDriver() instanceof Sqlserver);
 
-        $table = $this->getTableLocator()->get('SpecialTags');
+        $table = $this->getTableLocator()->get('UniqueAuthors');
         $rules = $table->rulesChecker();
-        $rules->add($rules->isUnique(['author_id', 'article_id'], 'Nope'));
+        $rules->add($rules->isUnique(
+            ['first_author_id', 'second_author_id'],
+            ['allowMultipleNulls' => true]
+        ));
 
-        $this->assertFalse($table->save($entity));
-        $this->assertEquals(['author_id' => ['_isUnique' => 'Nope']], $entity->getErrors());
-
-        $entity->clean();
-        $entity->article_id = 10;
-        $entity->tag_id = 12;
-        $entity->author_id = 12;
-        $this->assertSame($entity, $table->save($entity));
-    }
-
-    /**
-     * Tests isUnique with multiple fields emulates SQL UNIQUE keys
-     *
-     * @group save
-     * @return void
-     */
-    public function testIsUniqueMultipleFieldsOneIsNull()
-    {
         $entity = new Entity([
-            'author_id' => null,
-            'title' => 'First Article',
+            'first_author_id' => null,
+            'second_author_id' => 1,
         ]);
-        $table = $this->getTableLocator()->get('Articles');
-        $rules = $table->rulesChecker();
-        $rules->add($rules->isUnique(['title', 'author_id'], 'Nope'));
+        $this->assertNotEmpty($table->save($entity));
 
+        $entity->first_author_id = 2;
         $this->assertSame($entity, $table->save($entity));
 
-        // Make a matching record
         $entity = new Entity([
-            'author_id' => null,
-            'title' => 'New Article',
+            'first_author_id' => 2,
+            'second_author_id' => 1,
         ]);
-        $this->assertSame($entity, $table->save($entity));
+        $this->assertFalse($table->save($entity));
+        $this->assertEquals(['first_author_id' => ['_isUnique' => 'This value is already in use']], $entity->getErrors());
     }
 
     /**