Skip to content

Commit

Permalink
Merge pull request #5545 from paulbalandan/fix-validation-placeholders
Browse files Browse the repository at this point in the history 
Validation: support placeholders for anything
  • Loading branch information
@paulbalandan
paulbalandan authored Jan 15, 2022
2 parents 034a965 + 84ac2ea commit 9c0aa1a
Show file tree
Hide file tree
Showing 4 changed files with 263 additions and 33 deletions.
71 changes: 45 additions & 26 deletions system/Validation/Validation.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use CodeIgniter\View\RendererInterface;
use Config\Validation as ValidationConfig;
use InvalidArgumentException;
use TypeError;

/**
* Validator
Expand Down Expand Up @@ -362,18 +363,30 @@ public function withRequest(RequestInterface $request): ValidationInterface
* 'rule' => 'message'
* ]
*
* @param array|string $rules
*
* @throws TypeError
*
* @return $this
*/
public function setRule(string $field, ?string $label, string $rules, array $errors = [])
public function setRule(string $field, ?string $label, $rules, array $errors = [])
{
$this->rules[$field] = [
'label' => $label,
'rules' => $rules,
if (! is_array($rules) && ! is_string($rules)) {
throw new TypeError('$rules must be of type string|array');
}

$ruleSet = [
$field => [
'label' => $label,
'rules' => $rules,
],
];

$this->customErrors = array_merge($this->customErrors, [
$field => $errors,
]);
if ($errors) {
$ruleSet[$field]['errors'] = $errors;
}

$this->setRules($ruleSet + $this->getRules());

return $this;
}
Expand Down Expand Up @@ -401,16 +414,18 @@ public function setRules(array $rules, array $errors = []): ValidationInterface
$this->customErrors = $errors;

foreach ($rules as $field => &$rule) {
if (! is_array($rule)) {
continue;
}
if (is_array($rule)) {
if (array_key_exists('errors', $rule)) {
$this->customErrors[$field] = $rule['errors'];
unset($rule['errors']);
}

if (! array_key_exists('errors', $rule)) {
continue;
// if $rule is already a rule collection, just move it to "rules"
// transforming [foo => [required, foobar]] to [foo => [rules => [required, foobar]]]
if (! array_key_exists('rules', $rule)) {
$rule = ['rules' => $rule];
}
}

$this->customErrors[$field] = $rule['errors'];
unset($rule['errors']);
}

$this->rules = $rules;
Expand Down Expand Up @@ -581,23 +596,27 @@ protected function fillPlaceholders(array $rules, array $data): array
$replacements["{{$key}}"] = $value;
}

if (! empty($replacements)) {
if ($replacements !== []) {
foreach ($rules as &$rule) {
if (is_array($rule)) {
foreach ($rule as &$row) {
// Should only be an `errors` array
// which doesn't take placeholders.
if (is_array($row)) {
continue;
}
$ruleSet = $rule['rules'] ?? $rule;

$row = strtr($row ?? '', $replacements);
if (is_array($ruleSet)) {
foreach ($ruleSet as &$row) {
if (is_string($row)) {
$row = strtr($row, $replacements);
}
}
}

continue;
if (is_string($ruleSet)) {
$ruleSet = strtr($ruleSet, $replacements);
}

$rule = strtr($rule ?? '', $replacements);
if (isset($rule['rules'])) {
$rule['rules'] = $ruleSet;
} else {
$rule = $ruleSet;
}
}
}

Expand Down
204 changes: 203 additions & 1 deletion tests/system/Validation/ValidationTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
use Config\App;
use Config\Services;
use Generator;
use PHPUnit\Framework\ExpectationFailedException;
use Tests\Support\Validation\TestRules;
use TypeError;

/**
* @internal
Expand Down Expand Up @@ -83,7 +85,109 @@ public function testSetRulesStoresRules(): void
$this->assertSame($rules, $this->validation->getRules());
}

public function testRunReturnsFalseWithNothingToDo(): void
public function testSetRuleStoresRule()
{
$this->validation->setRules([]);
$this->validation->setRule('foo', null, 'bar|baz');

$this->assertSame([
'foo' => [
'label' => null,
'rules' => 'bar|baz',
],
], $this->validation->getRules());
}

public function testSetRuleAddsRule()
{
$this->validation->setRules([
'bar' => [
'label' => null,
'rules' => 'bar|baz',
],
]);
$this->validation->setRule('foo', null, 'foo|foz');

$this->assertSame([
'foo' => [
'label' => null,
'rules' => 'foo|foz',
],
'bar' => [
'label' => null,
'rules' => 'bar|baz',
],
], $this->validation->getRules());
}

public function testSetRuleOverwritesRule()
{
$this->validation->setRules([
'foo' => [
'label' => null,
'rules' => 'bar|baz',
],
]);
$this->validation->setRule('foo', null, 'foo|foz');

$this->assertSame([
'foo' => [
'label' => null,
'rules' => 'foo|foz',
],
], $this->validation->getRules());
}

/**
* @dataProvider setRuleRulesFormatCaseProvider
*
* @param mixed $rules
*/
public function testSetRuleRulesFormat(bool $expected, $rules): void
{
if (! $expected) {
$this->expectException(TypeError::class);
$this->expectExceptionMessage('$rules must be of type string|array');
}

$this->validation->setRule('foo', null, $rules);
$this->addToAssertionCount(1);
}

public function setRuleRulesFormatCaseProvider(): iterable
{
yield 'fail-simple-object' => [
false,
(object) ['required'],
];

yield 'pass-single-string' => [
true,
'required',
];

yield 'pass-single-array' => [
true,
['required'],
];

yield 'fail-deep-object' => [
false,
new Validation((object) $this->config, Services::renderer()),
];

yield 'pass-multiple-string' => [
true,
'required|alpha',
];

yield 'pass-multiple-array' => [
true,
['required', 'alpha'],
];
}

public function testRunReturnsFalseWithNothingToDo()
{
$this->validation->setRules([]);
$this->assertFalse($this->validation->run([]));
Expand Down Expand Up @@ -1008,4 +1112,102 @@ public function provideStringRulesCases(): iterable
['required', 'regex_match[/^(01|2689|09)[0-9]{8}$/]', 'numeric'],
];
}

/**
* internal method to simplify placeholder replacement test
* REQUIRES THE RULES TO BE SET FOR THE FIELD "foo"
*
* @param array|null $data optional POST data, needs to contain the key $placeholderField to pass
*
* @source https://github.com/codeigniter4/CodeIgniter4/pull/3910#issuecomment-784922913
*/
private function placeholderReplacementResultDetermination(string $placeholder = 'id', ?array $data = null)
{
if ($data === null) {
$data = [$placeholder => 'placeholder-value'];
}

$validationRules = $this->getPrivateMethodInvoker($this->validation, 'fillPlaceholders')($this->validation->getRules(), $data);
$fieldRules = $validationRules['foo']['rules'] ?? $validationRules['foo'];
if (is_string($fieldRules)) {
$fieldRules = $this->getPrivateMethodInvoker($this->validation, 'splitRules')($fieldRules);
}

// loop all rules for this field
foreach ($fieldRules as $rule) {
// only string type rules are supported
if (is_string($rule)) {
$this->assertStringNotContainsString('{' . $placeholder . '}', $rule);
}
}
}

/**
* @see ValidationTest::placeholderReplacementResultDetermination()
*/
public function testPlaceholderReplacementTestFails()
{
// to test if placeholderReplacementResultDetermination() works we provoke and expect an exception
$this->expectException(ExpectationFailedException::class);
$this->expectExceptionMessage('Failed asserting that \'filter[{id}]\' does not contain "{id}".');

$this->validation->setRule('foo', 'foo-label', 'required|filter[{id}]');

// calling with empty $data should produce an exception since {id} can't be replaced
$this->placeholderReplacementResultDetermination('id', []);
}

public function testPlaceholderReplacementSetSingleRuleString()
{
$this->validation->setRule('foo', null, 'required|filter[{id}]');

$this->placeholderReplacementResultDetermination();
}

public function testPlaceholderReplacementSetSingleRuleArray()
{
$this->validation->setRule('foo', null, ['required', 'filter[{id}]']);

$this->placeholderReplacementResultDetermination();
}

public function testPlaceholderReplacementSetMultipleRulesSimpleString()
{
$this->validation->setRules([
'foo' => 'required|filter[{id}]',
]);

$this->placeholderReplacementResultDetermination();
}

public function testPlaceholderReplacementSetMultipleRulesSimpleArray()
{
$this->validation->setRules([
'foo' => ['required', 'filter[{id}]'],
]);

$this->placeholderReplacementResultDetermination();
}

public function testPlaceholderReplacementSetMultipleRulesComplexString()
{
$this->validation->setRules([
'foo' => [
'rules' => 'required|filter[{id}]',
],
]);

$this->placeholderReplacementResultDetermination();
}

public function testPlaceholderReplacementSetMultipleRulesComplexArray()
{
$this->validation->setRules([
'foo' => [
'rules' => ['required', 'filter[{id}]'],
],
]);

$this->placeholderReplacementResultDetermination();
}
}
1 change: 1 addition & 0 deletions user_guide_src/source/changelogs/v4.2.0.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Release Date: Not Released
BREAKING
********

- The method signature of ``Validation::setRule()`` has been changed. The ``string`` typehint on the ``$rules`` parameter was removed. Extending classes should likewise remove the parameter so as not to break LSP.


Enhancements
Expand Down
20 changes: 14 additions & 6 deletions user_guide_src/source/libraries/validation.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,20 +261,28 @@ methods.
setRule()
=========

This method sets a single rule. It takes the name of the field as
the first parameter, an optional label and a string with a pipe-delimited list of rules
that should be applied::
This method sets a single rule. It has the method signature

$validation->setRule('username', 'Username', 'required');
setRule(string $field, ?string $label, array|string $rules[, array $errors = []])

The **field name** must match the key of any data array that is sent in. If
The ``$rules`` either takes in a pipe-delimited list of rules or an array collection of rules.

$validation->setRule('username', 'Username', 'required|min_length[3]');
$validation->setRule('password', 'Password', ['required', 'min_length[8]', 'alpha_numeric_punct']);

The value you pass to ``$field`` must match the key of any data array that is sent in. If
the data is taken directly from ``$_POST``, then it must be an exact match for
the form input name.

.. warning:: Prior to v4.2.0, this method's third parameter, ``$rules``, was typehinted to accept
``string``. In v4.2.0 and after, the typehint was removed to allow arrays, too. To avoid LSP being
broken in extending classes overriding this method, the child class's method should also be modified
to remove the typehint.

setRules()
==========

Like, ``setRule()``, but accepts an array of field names and their rules::
Like ``setRule()``, but accepts an array of field names and their rules::

$validation->setRules([
'username' => 'required',
Expand Down

0 comments on commit 9c0aa1a

Please sign in to comment.