Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class based after validation rules #46757

Merged
merged 2 commits into from
Apr 13, 2023
Merged

Conversation

timacdonald
Copy link
Member

@timacdonald timacdonald commented Apr 13, 2023

This PR proposes improvements to "after" validation rules.

Currently, "after" validation rules must be handled with a callable. I find they generally take the following shape.

<?php

Validator::make(/* .. */)->after(function ($validator) use (/* ... */): void {
    if (/* ... */) {
        $validator->errors()->add(/* ... */);
    }

    if (/* ... */) {
        $validator->errors()->add(/* ... */);
    }

    if (/* ... */) {
        $validator->errors()->add(/* ... */);
    }
});

It would also be possible to extract this out to a single callable class:

<?php

class AfterRule
{
    public function __construct(/* ... */)
    {
        //
    }

    public function __invoke($validator): void
    {
        if (/* ... */) {
            $validator->errors()->add(/* ... */);
        }

        if (/* ... */) {
            $validator->errors()->add(/* ... */);
        }

        if (/* ... */) {
            $validator->errors()->add(/* ... */);
        }
    }
}

Validator::make(/* .. */)->after(new AfterRule(/* ... */));

This is well and good, however it doesn't lend itself to rule composition and rule reuse.

If I want to reuse one of these validation checks I end up with clunky to use abstractions.

<?php

class AfterRuleOne
{
    public function __construct(/* ... */) 
    {
        //
    }

    public function __invoke($validator): void
    {
        if (/* ... */) {
            $validator->errors()->add(/* ... */);
        }
    }
}

class AfterRuleOne
{
    public function __construct(/* ... */) 
    {
        //
    }

    public function __invoke($validator): void
    {
        if (/* ... */) {
            $validator->errors()->add(/* ... */);
        }

        if (/* ... */) {
            $validator->errors()->add(/* ... */);
        }
    }
}

Validator::make(/* .. */)->after(function ($validator) use (/* ... */): void {
    (new AfterRuleOne(/* ... */))($validator);
    (new AfterRuleTwo(/* ... */))($validator);
});

Or a little bit better, I could use static methods on the extracted classes:

<?php

Validator::make(/* .. */)->after(function ($validator) use (/* ... */): void {
    AfterRuleOne($validator, /* ... */);
    AfterRuleTwo($validator, /* ... */);
});

I'd like to propose that introduce a new after rule concept by allowing an array of rules to be passed to the "after" method.

<?php

Validator::make(/* .. */)->after([
    new AfterRuleOne(/* ... */),
    new AfterRuleTwo(/* ... */),
    function ($validator) use (/* ... */): void => {
        // ...
    },
});

This is based on how normal class based validation rules work. The "after rule" just needs to implement an after($validator) method. Callables are also accepted, so you may pass a Closure or an invokable class.

If the class is invokable and has an after method, the after method takes precedence.

Any state, or services, required by the rule should be passed into the constructor, again matching the way normal class based validation rules work.

Form Requests

As form requests are how a large part of the community handle validation rules, I've made some potential improvements there as well.

In my opinion, adding "after" rules in a form request already feels a little out of place with other Laravel APIs.

<?php

class UserRequest
{
    public function rules(): array
    {
        return [
            //
        ];
    }

    public function withValidator($validator): void
    {
        $service = app(MyService::class);

        $validator->after([
            new AfterRule($this->value, $service),
        ]);
    }
}

I understand that withValidator gives you total access to the validator, but in my personal experience I only ever use this method for adding "after" rules to the validator. Additionally, the rules method is called by the container, but the withValidator method does not give the same affordance. Dependencies need to be resolved manually.

This is where the after method comes into play.

<?php

class UserRequest
{
    protected function rules(): array
    {
        return [
            //
        ];
    }

    protected function after(MyService $service): array
    {
        return [
            new AfterRule($this->value, $service),
            // ...
        ];
    }
}

@timacdonald timacdonald marked this pull request as ready for review April 13, 2023 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants