laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 28 forks source link

[Proposal] Custom Validators instances #1949

Open rogervila opened 4 years ago

rogervila commented 4 years ago

Laravel offers a useful way to generate Validation Rules, but not for creating custom Validator instances.

By encapsulating all the validation logic for a specific resource inside a class, it would be more easy to share the same validator for multiple data inputs.

Example:

Let's imagine an application that lets you create blog posts with artisan commands and HTTP requests. You could create a BlogPostValidator and instantiate it on your HTTP controller and your artisan command.

// Before
$validator = Validator::make($request->all(), [
        'title' => 'required|unique:posts|max:255',
        'body' => 'required',
    ]);

// After
$validator = BlogPostValidator::make($request->all());

Custom validator classes would be very similar to FormRequest classes, but focusing only on the validation process.

class BlogPostValidator {
    public function rules() {
        return [
            'title' => 'required|unique:posts|max:255',
            'body' => 'required',
        ]
    }

    public function messages() {
        //
    }

    // Other useful methods
}

What do you think?

Thank you.

martinbean commented 4 years ago

@rogervila I’m a big fan of this. I’m currently working in an organisation where we’re building a large API and would like to use the same validation logic in multiple places. Extracting rules from form requests to dedicated classes is something I’ve considered.

rogervila commented 4 years ago

Thank you @martinbean

I am considering to move this feature into a package that would bring some artisan commands to easily create custom validators.

This would be an example:

php artisan make:validator BlogPostValidator --model BlogPost

Then, based on the table columns and their types it could provide a default rules array.

Let's see if this idea gets more support :)

mpyw commented 4 years ago

We can easily implement this on userland.

<?php

namespace App\Validation;

use Illuminate\Contracts\Validation\Factory as FactoryContract;
use Illuminate\Contracts\Validation\Validator as ValidatorContract;

abstract class Validator
{
    public static function make(array $inputs): ValidatorContract
    {
        $def = app(static::class);

        return app(FactoryContract::class)->make(
            $inputs,
            $def->rules(),
            $def->messages(),
            $def->customAttributes()
        )->setCustomValues($this->customValues());
    }

    abstract public function rules(): array;

    public function messages(): array
    {
        return [];
    }

    public function customAttributes(): array
    {
        return [];
    }

    public function customValues(): array
    {
        return [];
    }
}
<?php

namespace App\Validation;

class BlogPostValidator extends Validator
{
    public function rules(): array
    {
        return [
            'title' => 'required|unique:posts|max:255',
            'body' => 'required',
        ];
    }
}

Advanced usage using dependency injection:

<?php

namespace App\Validation;

use Illuminate\Contracts\Auth\Guard;

class BlogPostValidator extends Validator
{
    public function __construct(Guard $guard)
    {
        $this->guard = $guard;
    }

    public function rules(): array
    {
        return [
            'title' => 'required|unique:posts|max:255',
            'body' => 'required',
            'secret' => $this->shouldRequireSecret() ? ['required'] : [],
        ];
    }

    protected function shouldRequireSecret(): bool
    {
        return !$this->guard->hasUser() || !$this->guard->user()->isAdministrator();
    }
}
rogervila commented 4 years ago

Thank you @mpyw

I agree that Custom Validators can be created by developers, but I wanted to discuss the idea to add the functionality on the Core, including the artisan command that I mentioned on my previous comment.

If the idea has support I will work on a PR for that. Otherwise, maybe I will create a package for it.

jlem commented 4 years ago

@rogervila, Laravel seems to have a "theme" of being able to define custom classes for things such as policies, observers etc, so I would say this proposal is very consistent with the rest of Laravel's features.

One thing to consider is that we have a kind of pseudo-implementation of this with Laravel's existing Form Requests feature:

https://laravel.com/docs/6.x/validation#form-request-validation

That said, I think your proposal is better than that feature because it just deals purely with validators, and isn't coupled to the notion of requests or authorization. This makes it more flexible.

DarkGhostHunter commented 4 years ago

What's the difference between this and Form Request? To put in another way, Why should I prefer this method instead of the Form Request? These actually can deal with Authorization too.

mpyw commented 4 years ago

@DarkGhostHunter It is always not a good idea that tightly coupling validation and HTTP. For example, Clean Architecture often requires domain level validation completely separated from HTTP layer.

But unfortunately Laravel doesn't seem to support such a philosophy. Laravel core team always pursues "Easy". I like this idea but I don't feel like bundling into the framework core.

simonhamp commented 4 years ago

I like the concept. Not sure if it offers more than Form Requests.

Personally, I’d love to see something like Form Requests make its way into Lumen. As Lumen lacks views (and thereby, forms) validation for each request is a little trickier to extract from controllers in a way that’s supported by the framework requiring a little extra plumbing.

Perhaps this approach could be the concept that enables this for Laravel and Lumen, which would be really nice.

adiachenko commented 4 years ago

I don't like how form requests mix authorization logic with validation and that they're idiomatically limited to HTTP requests. Sometimes you may need to validate CLI arguments or a message payload, or whatever. We need an abstraction that can consistently handle all of such validation needs. Ideally, form request would be a subclass that derives from such an abstraction. In fact, I already use custom validators that loosely resemble form requests in all of my apps.

martinbean commented 4 years ago

What's the difference between this and Form Request? To put in another way, Why should I prefer this method instead of the Form Request?

@DarkGhostHunter The validation can be re-used in other contexts (such as a console command, queued job, etc) if it is in its own separate class.

I also had a case recently where we wanted to add a “bulk” endpoint to our RESTful API and to validate multiple instances of a particular model. We can’t easily re-use the rules we’ve specified in the form request for the singular version of the endpoint; it would have been easier if the validation rules were separate from the HTTP request.

[Form requests can] actually can deal with Authorization too.

That doesn’t necessarily make them a good thing.

donnysim commented 4 years ago

I also dislike form requests having authorization and for that matter - validation ¯_(ツ)_/¯ . Often it means 2x+ model queries if you use model injection in controller, because you often need to retrieve the model for authorization and sometimes for validation.

In couple recent projects I just ditched the form requests as they just get in the way than actually help - I just do the validation and authorization checks in the controller, allows me to reuse the injected model or fetch it manually without having to store it in the form request etc. and also tweak some rules based on the request and model values. In all honesty, I did like them, but as the projects grow it's just a nuisance.

I do prefer having separate validators, but sometimes they need to be connected together (data with relationships etc.) and I'm not sure what the better way would be to go about them, but for now I just keep separate class(es) for specific validation rules/sets. Also, if we were to go with custom validator classes, it does require a way to pass custom models/data that "tweak" the rules depending on some fields - having them in constructor with dependency injection would be an inconvenience as you would not be able to inject it in a controller or so.

DarkGhostHunter commented 4 years ago

What's the difference between this and Form Request? To put in another way, Why should I prefer this method instead of the Form Request?

@DarkGhostHunter The validation can be re-used in other contexts (such as a console command, queued job, etc) if it is in its own separate class.

Can't you just create a new Validator then? What does the Validator Factory does wrong that you can't use it there?

I also had a case recently where we wanted to add a “bulk” endpoint to our RESTful API and to validate multiple instances of a particular model. We can’t easily re-use the rules we’ve specified in the form request for the singular version of the endpoint; it would have been easier if the validation rules were separate from the HTTP request.

[Form requests can] actually can deal with Authorization too.

That doesn’t necessarily make them a good thing.

From my point of view, a Form Request is basically a logic before hitting the controller that allows to not clog up the controller method itself with authorization and validation. Also, If I do the same thing on other parts of the application (like user panel and admin panel) I can just use the same Request.

I'm trying to get the idea of having Validators instances and I can understand the need to having a class that receives something and automatically validates that thing.

Personally I think the custom Validators as done here is the solution. Even better, a Validates trait that could automatically validate a class object that could be bound to the class itself you want to validate:

<?php

namespace Illuminate\Validation;

trait Validates
{
    /**
     * Validates the attributes of the current object
     *
     * @return array
     * @throws \Illuminate\Contracts\Container\BindingResolutionException
     * @throws \Illuminate\Validation\ValidationException
     */
    public function validate() : array
    {
        return app(Factory::class)->make(
            $this->input(),
            $this->messages(),
            $this->customAttributes(),
        )->validate();
    }

    /**
     * The input array to validate
     *
     * @return array
     */
    protected function input(): array
    {
        return [];
    }

    /**
     * Custom messages
     *
     * @return array
     */
    protected function messages(): array
    {
        return [];
    }

    /**
     * Custom validation messages
     *
     * @return array
     */
    protected function customAttributes(): array
    {
        return [];
    }
}

The other way is to add a Validatable trait, and step into some parts of the application to automatically validate the object after instantiating it, but that could be kind of troublesome.

I think we could start we having a list of useful things that could be validated after instancing, to make this approach more automatic than just creating an custom validator and call it a day: