Skip to content

Commit 5d6d4be

Browse files
kenjiskenjis
authored
Merge pull request #7565 from kenjis/refactor-validation-processRules
refactor: Validation::processRules()
2 parents c603406 + 39230fe commit 5d6d4be

File tree

4 files changed

+187
-81
lines changed

4 files changed

+187
-81
lines changed

‎system/Validation/Rules.php‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,6 @@ public function required_with($str = null, ?string $fields = null, array $data =
263263
// If the field is present we can safely assume that
264264
// the field is here, no matter whether the corresponding
265265
// search field is present or not.
266-
$fields = explode(',', $fields);
267266
$present = $this->required($str ?? '');
268267

269268
if ($present) {
@@ -272,11 +271,14 @@ public function required_with($str = null, ?string $fields = null, array $data =
272271

273272
// Still here? Then we fail this test if
274273
// any of the fields are present in $data
275-
// as $fields is the lis
274+
// as $fields is the list
276275
$requiredFields = [];
277276

278-
foreach ($fields as $field) {
279-
if ((array_key_exists($field, $data) && ! empty($data[$field])) || (strpos($field, '.') !== false && ! empty(dot_array_search($field, $data)))) {
277+
foreach (explode(',', $fields) as $field) {
278+
if (
279+
(array_key_exists($field, $data) && ! empty($data[$field]))
280+
|| (strpos($field, '.') !== false && ! empty(dot_array_search($field, $data)))
281+
) {
280282
$requiredFields[] = $field;
281283
}
282284
}
@@ -285,7 +287,7 @@ public function required_with($str = null, ?string $fields = null, array $data =
285287
}
286288

287289
/**
288-
* The field is required when all of the other fields are present
290+
* The field is required when all the other fields are present
289291
* in the data but not required.
290292
*
291293
* Example (field is required when the id or email field is missing):
@@ -296,28 +298,36 @@ public function required_with($str = null, ?string $fields = null, array $data =
296298
* @param string|null $otherFields The param fields of required_without[].
297299
* @param string|null $field This rule param fields aren't present, this field is required.
298300
*/
299-
public function required_without($str = null, ?string $otherFields = null, array $data = [], ?string $error = null, ?string $field = null): bool
300-
{
301+
public function required_without(
302+
$str = null,
303+
?string $otherFields = null,
304+
array $data = [],
305+
?string $error = null,
306+
?string $field = null
307+
): bool {
301308
if ($otherFields === null || empty($data)) {
302309
throw new InvalidArgumentException('You must supply the parameters: otherFields, data.');
303310
}
304311

305312
// If the field is present we can safely assume that
306313
// the field is here, no matter whether the corresponding
307314
// search field is present or not.
308-
$otherFields = explode(',', $otherFields);
309-
$present = $this->required($str ?? '');
315+
$present = $this->required($str ?? '');
310316

311317
if ($present) {
312318
return true;
313319
}
314320

315321
// Still here? Then we fail this test if
316322
// any of the fields are not present in $data
317-
foreach ($otherFields as $otherField) {
318-
if ((strpos($otherField, '.') === false) && (! array_key_exists($otherField, $data) || empty($data[$otherField]))) {
323+
foreach (explode(',', $otherFields) as $otherField) {
324+
if (
325+
(strpos($otherField, '.') === false)
326+
&& (! array_key_exists($otherField, $data) || empty($data[$otherField]))
327+
) {
319328
return false;
320329
}
330+
321331
if (strpos($otherField, '.') !== false) {
322332
if ($field === null) {
323333
throw new InvalidArgumentException('You must supply the parameters: field.');

‎system/Validation/StrictRules/Rules.php‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ public function required_with($str = null, ?string $fields = null, array $data =
347347
}
348348

349349
/**
350-
* The field is required when all of the other fields are present
350+
* The field is required when all the other fields are present
351351
* in the data but not required.
352352
*
353353
* Example (field is required when the id or email field is missing):
@@ -358,8 +358,13 @@ public function required_with($str = null, ?string $fields = null, array $data =
358358
* @param string|null $otherFields The param fields of required_without[].
359359
* @param string|null $field This rule param fields aren't present, this field is required.
360360
*/
361-
public function required_without($str = null, ?string $otherFields = null, array $data = [], ?string $error = null, ?string $field = null): bool
362-
{
361+
public function required_without(
362+
$str = null,
363+
?string $otherFields = null,
364+
array $data = [],
365+
?string $error = null,
366+
?string $field = null
367+
): bool {
363368
return $this->nonStrictRules->required_without($str, $otherFields, $data, $error, $field);
364369
}
365370
}

‎system/Validation/Validation.php‎

Lines changed: 96 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -218,87 +218,30 @@ public function check($value, $rules, array $errors = [], $dbGroup = null): bool
218218
* so that we can collect all of the first errors.
219219
*
220220
* @param array|string $value
221-
* @param array|null $rules
222-
* @param array|null $data The array of data to validate, with `DBGroup`.
221+
* @param array $rules
222+
* @param array $data The array of data to validate, with `DBGroup`.
223223
* @param string|null $originalField The original asterisk field name like "foo.*.bar".
224224
*/
225225
protected function processRules(
226226
string $field,
227227
?string $label,
228228
$value,
229-
$rules = null,
230-
?array $data = null,
229+
$rules = null, // @TODO remove `= null`
230+
?array $data = null, // @TODO remove `= null`
231231
?string $originalField = null
232232
): bool {
233233
if ($data === null) {
234234
throw new InvalidArgumentException('You must supply the parameter: data.');
235235
}
236236

237-
if (in_array('if_exist', $rules, true)) {
238-
$flattenedData = array_flatten_with_dots($data);
239-
$ifExistField = $field;
240-
241-
if (strpos($field, '.*') !== false) {
242-
// We'll change the dot notation into a PCRE pattern that can be used later
243-
$ifExistField = str_replace('\.\*', '\.(?:[^\.]+)', preg_quote($field, '/'));
244-
$dataIsExisting = false;
245-
$pattern = sprintf('/%s/u', $ifExistField);
246-
247-
foreach (array_keys($flattenedData) as $item) {
248-
if (preg_match($pattern, $item) === 1) {
249-
$dataIsExisting = true;
250-
break;
251-
}
252-
}
253-
} else {
254-
$dataIsExisting = array_key_exists($ifExistField, $flattenedData);
255-
}
256-
257-
unset($ifExistField, $flattenedData);
258-
259-
if (! $dataIsExisting) {
260-
// we return early if `if_exist` is not satisfied. we have nothing to do here.
261-
return true;
262-
}
263-
264-
// Otherwise remove the if_exist rule and continue the process
265-
$rules = array_filter($rules, static fn ($rule) => $rule instanceof Closure || $rule !== 'if_exist');
237+
$rules = $this->processIfExist($field, $rules, $data);
238+
if ($rules === true) {
239+
return true;
266240
}
267241

268-
if (in_array('permit_empty', $rules, true)) {
269-
if (
270-
! in_array('required', $rules, true)
271-
&& (is_array($value) ? $value === [] : trim((string) $value) === '')
272-
) {
273-
$passed = true;
274-
275-
foreach ($rules as $rule) {
276-
if (! $this->isClosure($rule) && preg_match('/(.*?)\[(.*)\]/', $rule, $match)) {
277-
$rule = $match[1];
278-
$param = $match[2];
279-
280-
if (! in_array($rule, ['required_with', 'required_without'], true)) {
281-
continue;
282-
}
283-
284-
// Check in our rulesets
285-
foreach ($this->ruleSetInstances as $set) {
286-
if (! method_exists($set, $rule)) {
287-
continue;
288-
}
289-
290-
$passed = $passed && $set->{$rule}($value, $param, $data);
291-
break;
292-
}
293-
}
294-
}
295-
296-
if ($passed === true) {
297-
return true;
298-
}
299-
}
300-
301-
$rules = array_filter($rules, static fn ($rule) => $rule instanceof Closure || $rule !== 'permit_empty');
242+
$rules = $this->processPermitEmpty($value, $rules, $data);
243+
if ($rules === true) {
244+
return true;
302245
}
303246

304247
foreach ($rules as $i => $rule) {
@@ -374,6 +317,92 @@ protected function processRules(
374317
return true;
375318
}
376319

320+
/**
321+
* @param array $data The array of data to validate, with `DBGroup`.
322+
*
323+
* @return array|true The modified rules or true if we return early
324+
*/
325+
private function processIfExist(string $field, array $rules, array $data)
326+
{
327+
if (in_array('if_exist', $rules, true)) {
328+
$flattenedData = array_flatten_with_dots($data);
329+
$ifExistField = $field;
330+
331+
if (strpos($field, '.*') !== false) {
332+
// We'll change the dot notation into a PCRE pattern that can be used later
333+
$ifExistField = str_replace('\.\*', '\.(?:[^\.]+)', preg_quote($field, '/'));
334+
$dataIsExisting = false;
335+
$pattern = sprintf('/%s/u', $ifExistField);
336+
337+
foreach (array_keys($flattenedData) as $item) {
338+
if (preg_match($pattern, $item) === 1) {
339+
$dataIsExisting = true;
340+
break;
341+
}
342+
}
343+
} else {
344+
$dataIsExisting = array_key_exists($ifExistField, $flattenedData);
345+
}
346+
347+
if (! $dataIsExisting) {
348+
// we return early if `if_exist` is not satisfied. we have nothing to do here.
349+
return true;
350+
}
351+
352+
// Otherwise remove the if_exist rule and continue the process
353+
$rules = array_filter($rules, static fn ($rule) => $rule instanceof Closure || $rule !== 'if_exist');
354+
}
355+
356+
return $rules;
357+
}
358+
359+
/**
360+
* @param array|string $value
361+
* @param array $data The array of data to validate, with `DBGroup`.
362+
*
363+
* @return array|true The modified rules or true if we return early
364+
*/
365+
private function processPermitEmpty($value, array $rules, array $data)
366+
{
367+
if (in_array('permit_empty', $rules, true)) {
368+
if (
369+
! in_array('required', $rules, true)
370+
&& (is_array($value) ? $value === [] : trim((string) $value) === '')
371+
) {
372+
$passed = true;
373+
374+
foreach ($rules as $rule) {
375+
if (! $this->isClosure($rule) && preg_match('/(.*?)\[(.*)\]/', $rule, $match)) {
376+
$rule = $match[1];
377+
$param = $match[2];
378+
379+
if (! in_array($rule, ['required_with', 'required_without'], true)) {
380+
continue;
381+
}
382+
383+
// Check in our rulesets
384+
foreach ($this->ruleSetInstances as $set) {
385+
if (! method_exists($set, $rule)) {
386+
continue;
387+
}
388+
389+
$passed = $passed && $set->{$rule}($value, $param, $data);
390+
break;
391+
}
392+
}
393+
}
394+
395+
if ($passed === true) {
396+
return true;
397+
}
398+
}
399+
400+
$rules = array_filter($rules, static fn ($rule) => $rule instanceof Closure || $rule !== 'permit_empty');
401+
}
402+
403+
return $rules;
404+
}
405+
377406
/**
378407
* @param Closure|string $rule
379408
*/

‎tests/system/Validation/RulesTest.php‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,68 @@ public function requiredWithProvider(): Generator
614614
];
615615
}
616616

617+
/**
618+
* @see https://github.com/codeigniter4/CodeIgniter4/issues/7557
619+
*
620+
* @dataProvider RequiredWithAndOtherRulesProvider
621+
*/
622+
public function testRequiredWithAndOtherRules(bool $expected, array $data): void
623+
{
624+
$this->validation->setRules([
625+
'mustBeADate' => 'required_with[otherField]|permit_empty|valid_date',
626+
]);
627+
628+
$result = $this->validation->run($data);
629+
630+
$this->assertSame($expected, $result);
631+
}
632+
633+
public function RequiredWithAndOtherRulesProvider(): Generator
634+
{
635+
yield from [
636+
// `otherField` and `mustBeADate` do not exist
637+
[true, []],
638+
// `mustBeADate` does not exist
639+
[false, ['otherField' => 'exists']],
640+
// ``otherField` does not exist
641+
[true, ['mustBeADate' => '2023-06-12']],
642+
[true, ['mustBeADate' => '']],
643+
[true, ['mustBeADate' => null]],
644+
[true, ['mustBeADate' => []]],
645+
// `otherField` and `mustBeADate` exist
646+
[true, ['mustBeADate' => '', 'otherField' => '']],
647+
[true, ['mustBeADate' => '2023-06-12', 'otherField' => 'exists']],
648+
[true, ['mustBeADate' => '2023-06-12', 'otherField' => '']],
649+
[false, ['mustBeADate' => '', 'otherField' => 'exists']],
650+
[false, ['mustBeADate' => [], 'otherField' => 'exists']],
651+
[false, ['mustBeADate' => null, 'otherField' => 'exists']],
652+
];
653+
}
654+
655+
/**
656+
* @dataProvider RequiredWithAndOtherRuleWithValueZeroProvider
657+
*/
658+
public function testRequiredWithAndOtherRuleWithValueZero(bool $expected, array $data): void
659+
{
660+
$this->validation->setRules([
661+
'married' => ['rules' => ['in_list[0,1]']],
662+
'partner_name' => ['rules' => ['permit_empty', 'required_with[married]', 'alpha_space']],
663+
]);
664+
665+
$result = $this->validation->run($data);
666+
667+
$this->assertSame($expected, $result);
668+
}
669+
670+
public function RequiredWithAndOtherRuleWithValueZeroProvider(): Generator
671+
{
672+
yield from [
673+
[true, ['married' => '0', 'partner_name' => '']],
674+
[true, ['married' => '1', 'partner_name' => 'Foo']],
675+
[false, ['married' => '1', 'partner_name' => '']],
676+
];
677+
}
678+
617679
/**
618680
* @dataProvider requiredWithoutProvider
619681
*/

0 commit comments

Comments
 (0)