Browse Source

Don't mutate ExistsIn fields.

We should not mutate fields when applying an ExistsIn rule as it causes
problems with saveMany().

Thanks to @Nic0tiN for the test case

Refs #11880
Mark Story 8 years ago
parent
commit
3990241bd3
2 changed files with 42 additions and 3 deletions
  1. 4 3
      src/ORM/Rule/ExistsIn.php
  2. 38 0
      tests/TestCase/ORM/RulesCheckerIntegrationTest.php

+ 4 - 3
src/ORM/Rule/ExistsIn.php

@@ -92,6 +92,7 @@ class ExistsIn
             $this->_repository = $repository;
         }
 
+        $fields = $this->_fields;
         $source = $target = $this->_repository;
         $isAssociation = $target instanceof Association;
         $bindingKey = $isAssociation ? (array)$target->getBindingKey() : (array)$target->getPrimaryKey();
@@ -118,9 +119,9 @@ class ExistsIn
 
         if ($this->_options['allowNullableNulls']) {
             $schema = $source->getSchema();
-            foreach ($this->_fields as $i => $field) {
+            foreach ($fields as $i => $field) {
                 if ($schema->getColumn($field) && $schema->isNullable($field) && $entity->get($field) === null) {
-                    unset($bindingKey[$i], $this->_fields[$i]);
+                    unset($bindingKey[$i], $fields[$i]);
                 }
             }
         }
@@ -131,7 +132,7 @@ class ExistsIn
         );
         $conditions = array_combine(
             $primary,
-            $entity->extract($this->_fields)
+            $entity->extract($fields)
         );
 
         return $target->exists($conditions);

+ 38 - 0
tests/TestCase/ORM/RulesCheckerIntegrationTest.php

@@ -1120,6 +1120,44 @@ class RulesCheckerIntegrationTest extends TestCase
     }
 
     /**
+     * Tests new allowNullableNulls with saveMany
+     *
+     * @return
+     */
+    public function testExistsInAllowNullableNullsSaveMany()
+    {
+        $entities = [
+            new Entity([
+                'id' => 1,
+                'author_id' => null,
+                'site_id' => 1,
+                'name' => 'New Site Article without Author',
+            ]),
+            new Entity([
+                'id' => 2,
+                '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', [
+            'allowNullableNulls' => true,
+            'message' => 'will error with array_combine warning']));
+        $result = $table->saveMany($entities);
+        $this->assertCount(2, $result);
+
+        $this->assertInstanceOf(Entity::class, $result[0]);
+        $this->assertEmpty($result[0]->getErrors());
+
+        $this->assertInstanceOf(Entity::class, $result[1]);
+        $this->assertEmpty($result[1]->getErrors());
+    }
+
+    /**
      * Tests using rules to prevent delete operations
      *
      * @group delete