Browse Source

Fix context array being passed for optional argument of validation method.

Prior to this patch validation providers had to be used through the RulesProvider
class to handle properly passing the context array.
Fixes #17631
ADmad 1 year ago
parent
commit
c7fe5971de

+ 1 - 1
src/ORM/Table.php

@@ -3132,7 +3132,7 @@ class Table implements RepositoryInterface, EventListenerInterface, EventDispatc
      * @param array|null $context Either the validation context or null.
      * @return bool True if the value is unique, or false if a non-scalar, non-unique value was given.
      */
-    public function validateUnique(mixed $value, array $options, ?array $context = null): bool
+    public function validateUnique(mixed $value, array $options = [], ?array $context = null): bool
     {
         if ($context === null) {
             $context = $options;

+ 12 - 0
src/Validation/RulesProvider.php

@@ -17,12 +17,15 @@ declare(strict_types=1);
 namespace Cake\Validation;
 
 use ReflectionClass;
+use function Cake\Core\deprecationWarning;
 
 /**
  * A Proxy class used to remove any extra arguments when the user intended to call
  * a method in another class that is not aware of validation providers signature
  *
  * @method bool extension(mixed $check, array $extensions, array $context = [])
+ * @deprecated 5.2.0 This class is no longer used. Cake\Validation\Validation
+ *  is now directly used as a provider in Cake\Validation\Validator.
  */
 class RulesProvider
 {
@@ -49,6 +52,15 @@ class RulesProvider
      */
     public function __construct(object|string $class = Validation::class)
     {
+        deprecationWarning(
+            '5.2.0',
+            sprintf(
+                'The class Cake\Validation\RulesProvider is deprecated. '
+                . 'Directly set %s as a validation provider.',
+                (is_string($class) ? $class : get_class($class))
+            )
+        );
+
         $this->_class = $class;
         $this->_reflection = new ReflectionClass($class);
     }

+ 17 - 20
src/Validation/ValidationRule.php

@@ -20,7 +20,8 @@ declare(strict_types=1);
  */
 namespace Cake\Validation;
 
-use InvalidArgumentException;
+use Closure;
+use ReflectionFunction;
 
 /**
  * ValidationRule object. Represents a validation method, error message and
@@ -120,33 +121,29 @@ class ValidationRule
 
         if (is_string($this->_rule)) {
             $provider = $providers[$this->_provider];
-            /** @var callable $callable */
-            $callable = [$provider, $this->_rule];
-            $isCallable = is_callable($callable);
+            /** @phpstan-ignore-next-line */
+            $callable = [$provider, $this->_rule](...);
         } else {
             $callable = $this->_rule;
-            $isCallable = true;
+            if (!$callable instanceof Closure) {
+                $callable = $callable(...);
+            }
         }
 
-        if (!$isCallable) {
-            /** @var string $method */
-            $method = $this->_rule;
-            $message = sprintf(
-                'Unable to call method `%s` in `%s` provider for field `%s`',
-                $method,
-                $this->_provider,
-                $context['field']
-            );
-            throw new InvalidArgumentException($message);
-        }
+        $args = [$value];
 
         if ($this->_pass) {
-            $args = array_values(array_merge([$value], $this->_pass, [$context]));
-            $result = $callable(...$args);
-        } else {
-            $result = $callable($value, $context);
+            $args = array_merge([$value], array_values($this->_pass));
         }
 
+        $params = (new ReflectionFunction($callable))->getParameters();
+        $lastParm = array_pop($params);
+        if ($lastParm && $lastParm->getName() === 'context') {
+            $args['context'] = $context;
+        }
+
+        $result = $callable(...$args);
+
         if ($result === false) {
             return $this->_message ?: false;
         }

+ 1 - 1
src/Validation/Validator.php

@@ -330,7 +330,7 @@ class Validator implements ArrayAccess, IteratorAggregate, Countable
             return null;
         }
 
-        $this->_providers[$name] = new RulesProvider();
+        $this->_providers[$name] = Validation::class;
 
         return $this->_providers[$name];
     }

+ 3 - 3
tests/TestCase/Validation/ValidationRuleTest.php

@@ -20,7 +20,7 @@ use Cake\Core\Exception\CakeException;
 use Cake\TestSuite\TestCase;
 use Cake\Validation\ValidationRule;
 use Cake\Validation\ValidationSet;
-use InvalidArgumentException;
+use Error;
 
 /**
  * ValidationRuleTest
@@ -93,8 +93,8 @@ class ValidationRuleTest extends TestCase
      */
     public function testCustomMethodMissingError(): void
     {
-        $this->expectException(InvalidArgumentException::class);
-        $this->expectExceptionMessage('Unable to call method `totallyMissing` in `default` provider for field `test`');
+        $this->expectException(Error::class);
+        $this->expectExceptionMessage('Call to undefined method Cake\Test\TestCase\Validation\ValidationRuleTest::totallyMissing()');
         $def = ['rule' => ['totallyMissing']];
         $data = 'some data';
         $providers = ['default' => $this];

+ 76 - 43
tests/TestCase/Validation/ValidatorTest.php

@@ -17,7 +17,6 @@ declare(strict_types=1);
 namespace Cake\Test\TestCase\Validation;
 
 use Cake\TestSuite\TestCase;
-use Cake\Validation\RulesProvider;
 use Cake\Validation\Validation;
 use Cake\Validation\ValidationRule;
 use Cake\Validation\ValidationSet;
@@ -1480,7 +1479,7 @@ class ValidatorTest extends TestCase
         $this->assertSame($validator, $validator->setProvider('bar', $another));
         $this->assertSame($another, $validator->getProvider('bar'));
 
-        $this->assertEquals(new RulesProvider(), $validator->getProvider('default'));
+        $this->assertEquals(Validation::class, $validator->getProvider('default'));
     }
 
     /**
@@ -1523,27 +1522,16 @@ class ValidatorTest extends TestCase
             ->add('email', 'alpha', ['rule' => 'alphanumeric'])
             ->add('title', 'cool', ['rule' => 'isCool', 'provider' => 'thing']);
 
-        $thing = $this->getMockBuilder(stdMock::class)->getMock();
-        $thing->expects($this->once())->method('isCool')
-            ->willReturnCallback(function ($data, $context) use ($thing) {
-                $this->assertSame('bar', $data);
-                $expected = [
-                    'default' => new RulesProvider(),
-                    'thing' => $thing,
-                ];
-                $expected = [
-                    'newRecord' => true,
-                    'providers' => $expected,
-                    'data' => [
-                        'email' => '!',
-                        'title' => 'bar',
-                    ],
-                    'field' => 'title',
-                ];
-                $this->assertEquals($expected, $context);
+        $thing = new class {
+            public $args = [];
+
+            public function isCool($data, $context)
+            {
+                $this->args = [$data, $context];
 
                 return "That ain't cool, yo";
-            });
+            }
+        };
 
         $validator->setProvider('thing', $thing);
         $errors = $validator->validate(['email' => '!', 'title' => 'bar']);
@@ -1552,6 +1540,26 @@ class ValidatorTest extends TestCase
             'title' => ['cool' => "That ain't cool, yo"],
         ];
         $this->assertEquals($expected, $errors);
+
+        $this->assertSame('bar', $thing->args[0]);
+
+        $context = $thing->args[1];
+        $provider = $context['providers']['thing'];
+        $this->assertSame($thing, $provider);
+
+        unset($context['providers']['thing']);
+        $expected = [
+            'newRecord' => true,
+            'providers' => [
+                'default' => 'Cake\Validation\Validation',
+            ],
+            'data' => [
+                'email' => '!',
+                'title' => 'bar',
+            ],
+            'field' => 'title',
+        ];
+        $this->assertEquals($expected, $context);
     }
 
     /**
@@ -1565,35 +1573,45 @@ class ValidatorTest extends TestCase
             'rule' => ['isCool', 'and', 'awesome'],
             'provider' => 'thing',
         ]);
-        $thing = $this->getMockBuilder(stdMock::class)->getMock();
-        $thing->expects($this->once())->method('isCool')
-            ->willReturnCallback(function ($data, $a, $b, $context) use ($thing) {
-                $this->assertSame('bar', $data);
-                $this->assertSame('and', $a);
-                $this->assertSame('awesome', $b);
-                $expected = [
-                    'default' => new RulesProvider(),
-                    'thing' => $thing,
-                ];
-                $expected = [
-                    'newRecord' => true,
-                    'providers' => $expected,
-                    'data' => [
-                        'email' => '!',
-                        'title' => 'bar',
-                    ],
-                    'field' => 'title',
-                ];
-                $this->assertEquals($expected, $context);
+        $thing = new class {
+            public $args = [];
+
+            public function isCool($data, $a, $b, $context)
+            {
+                $this->args = [$data, $a, $b, $context];
 
                 return "That ain't cool, yo";
-            });
+            }
+        };
+
         $validator->setProvider('thing', $thing);
         $errors = $validator->validate(['email' => '!', 'title' => 'bar']);
         $expected = [
             'title' => ['cool' => "That ain't cool, yo"],
         ];
         $this->assertEquals($expected, $errors);
+
+        $this->assertSame('bar', $thing->args[0]);
+        $this->assertSame('and', $thing->args[1]);
+        $this->assertSame('awesome', $thing->args[2]);
+
+        $context = $thing->args[3];
+        $provider = $context['providers']['thing'];
+        $this->assertSame($thing, $provider);
+
+        unset($context['providers']['thing']);
+        $expected = [
+            'newRecord' => true,
+            'providers' => [
+                'default' => 'Cake\Validation\Validation',
+            ],
+            'data' => [
+                'email' => '!',
+                'title' => 'bar',
+            ],
+            'field' => 'title',
+        ];
+        $this->assertEquals($expected, $context);
     }
 
     /**
@@ -1603,7 +1621,7 @@ class ValidatorTest extends TestCase
     {
         $validator = new Validator();
         $validator->add('name', 'myRule', [
-            'rule' => function ($data, $provider) {
+            'rule' => function ($data) {
                 $this->assertSame('foo', $data);
 
                 return 'You fail';
@@ -2990,6 +3008,21 @@ class ValidatorTest extends TestCase
     }
 
     /**
+     * Test ensuring that context array doesn't get passed for an optional validation method argument.
+     * Validation::decimal() has 2 arguments and only 1 is being passed here.
+     *
+     * @return void
+     */
+    public function testWithoutPassingAllArguments(): void
+    {
+        $validator = new Validator();
+        $validator->setProvider('default', Validation::class);
+
+        $validator->decimal('field', 2);
+        $this->assertEmpty($validator->validate(['field' => 1.23]));
+    }
+
+    /**
      * Assert for the data validation message for a given field's rule for a I18n-enabled & a I18n-disabled validator
      *
      * @param string $fieldName The field name.