[9.x] Patch nested array validation rule regression bug by stevebauman · Pull Request #43897 · laravel/framework

stevebauman · 2022-08-28T05:59:39Z

Closes #43779
Closes #43751
Related #43192
Related #40924

Important: Please look at the first PR above for full context.

This PR resolves a regression bug that I introduced in 9.x in the Rule::forEach() validation feature, where nested array validation rules can no longer be used, due to flattening them via an Arr::flatten($rules).

For example, this can no longer be done, whereas it used to work in prior versions of Laravel:

$data = [
    'items' => [
        ['|name' => 'foo'],
    ]
];

$rules = [
    'items.*' => ['array', ['required_array_keys', '|name']] // <-- Nested array rule with an argument.
];

// BadMethodCallException: Method Illuminate\Validation\Validator::validateName does not exist
Validator::make($data, $rules)->passes();

Arr::flatten() was used to be able to determine if a ruleset contains a NestedRules instance, and allowed validation rules to be infinitely nested, as they would be flattened before being parsed:

framework/src/Illuminate/Validation/ValidationRuleParser.php

Lines 154 to 155 in 2a1a55c

    
           foreach (Arr::flatten((array) $rules) as $rule) { 
        
               if ($rule instanceof NestedRules) {

This PR will break the below (however unlikely) implementation for developers -- but will resolve the regression bug, while keeping the Rule::forEach feature:

$rules = [
    'users.*.name' => [
        Rule::forEach(function ($value, $attribute, $data) {
            // ...
        }),
        Rule::forEach(function ($value, $attribute, $data) {
            // ...
        }),
    ],
];

Developers will have instead have to use a single Rule::forEach() (non-nested), as this will no longer be supported:

$rules = [
    'users.*.name' => Rule::forEach(function ($value, $attribute, $data) {
        // ...
    }),
];

I've added test to ensure we have some coverage over the nested array rule feature so regression will not occur again in the future.

Thanks so much for your time, and I apologize for this issue that has caused grief for devs upgrading to 9.x ❤️

garygreen · 2022-08-28T13:26:01Z

Awesome Steve. Thanks so much for adding the additional tests as well! 😀

siarheipashkevich · 2022-08-31T17:53:57Z

Hi @stevebauman.

I've updated to 9.27 version and getting error for this rule.

My rule:

$promotionItemIdRule = Rule::forEach(function (mixed $value, string $attribute, array $data) use (
    $conditionItemIds,
) {
    $type = Arr::get($data, Str::replaceLast('.item_id', '.item_type', $attribute));

    return [Rule::in($conditionItemIds->get($type, []))];
});

// and using

'conditions.*.items.*.item_id' => ['required', 'integer', $promotionItemIdRule],
'conditions.*.items.*.item_type' => ['required', 'string', Rule::in(PromotionItemType::all())],

Method Illuminate\Validation\Validator::validateIn: does not exist. {"userId":93,"exception":"[object] (BadMethodCallException(code: 0): Method Illuminate\\Validation\\Validator::validateIn: does not exist. at /var/www/html/vendor/laravel/framework/src/Illuminate/Validation/Validator.php:1534)
[stacktrace]
#0 /var/www/html/vendor/laravel/framework/src/Illuminate/Validation/Validator.php(609): Illuminate\\Validation\\Validator->__call()
#1 /var/www/html/vendor/laravel/framework/src/Illuminate/Validation/Validator.php(415): Illuminate\\Validation\\Validator->validateAttribute()
#2 /var/www/html/vendor/laravel/framework/src/Illuminate/Validation/Validator.php(446): Illuminate\\Validation\\Validator->passes()
#3 /var/www/html/vendor/laravel/framework/src/Illuminate/Validation/ValidatesWhenResolvedTrait.php(25): Illuminate\\Validation\\Validator->fails()
#4 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Providers/FormRequestServiceProvider.php(30): Illuminate\\Foundation\\Http\\FormRequest->validateResolved()
#5 /var/www/html/vendor/laravel/framework/src/Illuminate/Container/Container.php(1263): Illuminate\\Foundation\\Providers\\FormRequestServiceProvider->Illuminate\\Foundation\\Providers\\{closure}()
#6 /var/www/html/vendor/laravel/framework/src/Illuminate/Container/Container.php(1228): Illuminate\\Container\\Container->fireCallbackArray()
#7 /var/www/html/vendor/laravel/framework/src/Illuminate/Container/Container.php(1213): Illuminate\\Container\\Container->fireAfterResolvingCallbacks()
#8 /var/www/html/vendor/laravel/framework/src/Illuminate/Container/Container.php(776): Illuminate\\Container\\Container->fireResolvingCallbacks()
#9 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(857): Illuminate\\Container\\Container->resolve()
#10 /var/www/html/vendor/laravel/framework/src/Illuminate/Container/Container.php(692): Illuminate\\Foundation\\Application->resolve()
#11 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(842): Illuminate\\Container\\Container->make()
#12 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/RouteDependencyResolverTrait.php(80): Illuminate\\Foundation\\Application->make()
#13 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/RouteDependencyResolverTrait.php(49): Illuminate\\Routing\\ControllerDispatcher->transformDependency()
#14 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/RouteDependencyResolverTrait.php(29): Illuminate\\Routing\\ControllerDispatcher->resolveMethodDependencies()
#15 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(41): Illuminate\\Routing\\ControllerDispatcher->resolveClassMethodDependencies()
#16 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Route.php(261): Illuminate\\Routing\\ControllerDispatcher->dispatch()
#17 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Route.php(204): Illuminate\\Routing\\Route->runController()
#18 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Router.php(725): Illuminate\\Routing\\Route->run()
#19 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(141): Illuminate\\Routing\\Router->Illuminate\\Routing\\{closure}()
#20 /var/www/html/app/Http/Middleware/VerifyUserHasTeam.php(33): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#21 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): App\\Http\\Middleware\\VerifyUserHasTeam->handle()
#22 /var/www/html/app/Http/Middleware/AuthorizeMiddleware.php(39): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#23 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): App\\Http\\Middleware\\AuthorizeMiddleware->handle()
#24 /var/www/html/app/Http/Middleware/LogLastUserActivity.php(25): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#25 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): App\\Http\\Middleware\\LogLastUserActivity->handle()
#26 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Middleware/SubstituteBindings.php(50): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#27 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Routing\\Middleware\\SubstituteBindings->handle()
#28 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Middleware/ThrottleRequestsWithRedis.php(65): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#29 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Middleware/ThrottleRequests.php(102): Illuminate\\Routing\\Middleware\\ThrottleRequestsWithRedis->handleRequest()
#30 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Middleware/ThrottleRequests.php(54): Illuminate\\Routing\\Middleware\\ThrottleRequests->handleRequestUsingNamedLimiter()
#31 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Routing\\Middleware\\ThrottleRequests->handle()
#32 /var/www/html/vendor/laravel/framework/src/Illuminate/Auth/Middleware/Authenticate.php(44): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#33 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Auth\\Middleware\\Authenticate->handle()
#34 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(116): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#35 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Router.php(726): Illuminate\\Pipeline\\Pipeline->then()
#36 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Router.php(703): Illuminate\\Routing\\Router->runRouteWithinStack()
#37 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Router.php(667): Illuminate\\Routing\\Router->runRoute()
#38 /var/www/html/vendor/laravel/framework/src/Illuminate/Routing/Router.php(656): Illuminate\\Routing\\Router->dispatchToRoute()
#39 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(167): Illuminate\\Routing\\Router->dispatch()
#40 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(141): Illuminate\\Foundation\\Http\\Kernel->Illuminate\\Foundation\\Http\\{closure}()
#41 /var/www/html/app/Http/Middleware/SetLocale.php(36): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#42 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): App\\Http\\Middleware\\SetLocale->handle()
#43 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#44 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ConvertEmptyStringsToNull.php(31): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle()
#45 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Foundation\\Http\\Middleware\\ConvertEmptyStringsToNull->handle()
#46 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#47 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle()
#48 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#49 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TrimStrings.php(40): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle()
#50 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Foundation\\Http\\Middleware\\TrimStrings->handle()
#51 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php(27): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#52 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Foundation\\Http\\Middleware\\ValidatePostSize->handle()
#53 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/PreventRequestsDuringMaintenance.php(86): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#54 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Foundation\\Http\\Middleware\\PreventRequestsDuringMaintenance->handle()
#55 /var/www/html/vendor/laravel/framework/src/Illuminate/Http/Middleware/HandleCors.php(62): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#56 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(180): Illuminate\\Http\\Middleware\\HandleCors->handle()
#57 /var/www/html/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(116): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}()
#58 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(142): Illuminate\\Pipeline\\Pipeline->then()
#59 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(111): Illuminate\\Foundation\\Http\\Kernel->sendRequestThroughRouter()
#60 /var/www/html/public/index.php(52): Illuminate\\Foundation\\Http\\Kernel->handle()
#61 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/resources/server.php(16): require_once('...')
#62 {main}
"}

siarheipashkevich · 2022-08-31T17:57:34Z

$attribute does not contain full path for the current item.

stevebauman · 2022-08-31T20:25:47Z

Hi @siarheipashkevich -- as shown in the above PR, you can no longer nest one or many Rule::forEach() rules inside of a validation rule array:

$rules = [
    'conditions.*.items.*.item_id' => ['required', 'integer', Rule::forEach('...')], // <-- Will not work.
];

You must refactor the validation rule like so by returning all your validation rules inside of the Rule::forEach() callback:

$promotionItemIdRule = Rule::forEach(function (mixed $value, string $attribute, array $data) use (
    $conditionItemIds,
) {
    $type = Arr::get($data, Str::replaceLast('.item_id', '.item_type', $attribute));

    return ['required', 'integer', Rule::in($conditionItemIds->get($type, []))]; // <-- All rules returned here.
});

// ...

$rules = [
    'conditions.*.items.*.item_id' => $promotionItemIdRule,
];

siarheipashkevich · 2022-08-31T20:33:20Z

@stevebauman thank you so much.

stevebauman · 2022-08-31T20:37:25Z

Happy to help @siarheipashkevich. Also you may be able to use something like this to clean it up a bit (correct me if I'm wrong):

$rules = [
    'conditions.*.items' => ['required', 'array'],
    'conditions.*.items.*.item_id' => ['required', 'integer'],
    'conditions.*.items.*.item_type' => ['required', 'string'],
    'conditions.*.items.*' => Rule::forEach(function ($value) use ($conditionItemIds) {
        return [
            'item_id' => Rule::in($conditionItemIds->get($value['item_type'], [])),
            'item_type' => Rule::in(PromotionItemType::all()),
        ];
    }),
];

siarheipashkevich · 2022-09-01T14:13:28Z

@stevebauman in your example the rules will be combined?

stevebauman · 2022-09-01T20:13:29Z

@siarheipashkevich I'm not sure what you mean, can you elaborate?

siarheipashkevich · 2022-09-02T08:52:43Z

@stevebauman in your example we will have the next rules:

'conditions.*.items.*.item_id' => ['required', 'integer', Rule::in(...)],
'conditions.*.items.*.item_type' => ['required', 'string', Rule::in(...)],

because Rule::forEach will add additional rules for already declared rules with the same keys, is it?

stevebauman · 2022-09-02T10:27:32Z

@siarheipashkevich The rules will not be merged. They will be evaluated individually.

stevebauman added 5 commits August 19, 2022 13:22

Remove Arr::flatten

b49e47b

Update tests and remove no-longer relevant tests

35c0874

Add test for nested validation rules

3df2336

Rename and move test

0de8df9

Add failing validation run

92b5402

stevebauman mentioned this pull request Aug 28, 2022

[9.x] Add validation Rule::raw($rule, $parameters) #43779

Closed

CS fix

694b5e7

taylorotwell merged commit 5bba757 into laravel:9.x Aug 29, 2022

stevebauman deleted the fix-nested-array-validation-rules branch September 2, 2022 10:27

aihowes mentioned this pull request Nov 10, 2022

Invokeable Rules don't work for Rule::forEach() under nested array property validation. #44756

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[9.x] Patch nested array validation rule regression bug#43897

[9.x] Patch nested array validation rule regression bug#43897
taylorotwell merged 6 commits into
laravel:9.xfrom
stevebauman:fix-nested-array-validation-rules

stevebauman commented Aug 28, 2022

Uh oh!

garygreen commented Aug 28, 2022

Uh oh!

siarheipashkevich commented Aug 31, 2022

Uh oh!

siarheipashkevich commented Aug 31, 2022 •

edited

Loading

Uh oh!

stevebauman commented Aug 31, 2022 •

edited

Loading

Uh oh!

siarheipashkevich commented Aug 31, 2022

Uh oh!

stevebauman commented Aug 31, 2022 •

edited

Loading

Uh oh!

siarheipashkevich commented Sep 1, 2022

Uh oh!

stevebauman commented Sep 1, 2022

Uh oh!

siarheipashkevich commented Sep 2, 2022 •

edited

Loading

Uh oh!

stevebauman commented Sep 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	foreach (Arr::flatten((array) $rules) as $rule) {
	if ($rule instanceof NestedRules) {

Conversation

stevebauman commented Aug 28, 2022

Uh oh!

garygreen commented Aug 28, 2022

Uh oh!

siarheipashkevich commented Aug 31, 2022

Uh oh!

siarheipashkevich commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stevebauman commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

siarheipashkevich commented Aug 31, 2022

Uh oh!

stevebauman commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

siarheipashkevich commented Sep 1, 2022

Uh oh!

stevebauman commented Sep 1, 2022

Uh oh!

siarheipashkevich commented Sep 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stevebauman commented Sep 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

siarheipashkevich commented Aug 31, 2022 •

edited

Loading

stevebauman commented Aug 31, 2022 •

edited

Loading

stevebauman commented Aug 31, 2022 •

edited

Loading

siarheipashkevich commented Sep 2, 2022 •

edited

Loading