Update method rules() in next versions.

[Auramel]

Hello! So, I offer to change syntax in \yii\base\Model::rules()

For example, now we have

public function rules()
{
    return [
        [['login', 'pass'], 'required']
    ];
}

I offer try to code something else, for example:

public function rules()
{
    return [
        $this->login => Validator()->required()->string()->min(255)->max(1000)
    ];
}

I like https://github.com/Respect/Validation approach. What do you think? UPD: I don't know why tabs and spaces are not working... UPD2: Someone fixed it. Thank you 👍

Russian: хотелось бы писать правила, как в ссылке выше. :) UPD: Хз почему все переносы и табы слетели... Писал в Notepad++...

UPD2: Кто-то пофиксил. Спасибо 👍

[samdark]

That's quite similar to schema builder used in migrations. I like the idea.

[samdark]

Would you like to implement it?

[mgrechanik]

What is login in $this->login ? New object variable needed to be defined? How would it look like with two or more identical attributes per rule?

[Auramel]

@mgrechanik So, $this->login it's a SomeModel::$login property. When you call method validate(), you use current values of your class. So, it depends, how it will be implemented. For example:

class SomeModel
{
    public $property1;

    public $property2;

    # variant #1
    public function rules()
    {
        return [
            $this->property1 => Validator::required()->string()->min(6)->max(255),
            $this->property2 => Validator::required()->string()->min(6)->max(255)
        ];
    }

    # variant #2
    public function rules()
    {
        [$this->property1, $this->property2] => Validator::required()->string()->min(6)->max(255)
    }
}

[Auramel]

@samdark If you ready to see my shit-code - no problem :)

[samdark]

I am.

[bizley]

I like the idea but don't you think about something like:

public function rules()
{
    return [
        'property1' => Validator::required()->string()->min(6)->max(255),
        'property2' => Validator::required()->string()->min(6)->max(255),
    ];
}

[Auramel]

@bizley can be and so )

[ddinchev]

I do like the idea to use builder pattern to configure validators - the current strings array is error prone and devs have to always check documentation of the correct validator shortcut (was it exist or exists, was it integer or number validator).

However, I wouldn't like to have to define same rule for multiple fields like this:

    # variant #1
    public function rules()
    {
        return [
            $this->property1 => Validator::required()->string()->min(6)->max(255),
            $this->property2 => Validator::required()->string()->min(6)->max(255)
        ];
    }

And it's a bit hard to imagine how the implementation of Variant #2 would look like, besides the fact that this seems like a confusing way to combine required and string validator.

In the simplest form, we could just preserve the field definition and opt for builder pattern for the validators. Eg:

public function rules()
{
    return [
        [['property1', 'property2'], Validator::required()->build()],
        ['property2' => Validator::string()->min(6)->max(255)->build()],
    ];
}

With this, we could allow combining validators for a field in a more obvious manner:

public function rules()
{
    return [
        ['property2' => Validator::composite()
            ->add(Validator::required())
            ->add(Validator::string()->min(6)->max(255)->when(function (SomeModel $model) {
                return $model->property3 === "awesome";
            }))
            ->build()
        ],
    ];
}

What do you think?

[Auramel]

@ddinchev oh shit, it's crazy. Cool. It's will be difficult to write something this (for me). And has a one question - what do we do with previous versions? More projects use current rules. I think it not good to crash them, when they updated. I think we can save current syntax and add something new. Maybe, new method? Or add some 100+ if() to code ?) coreTeam, what do you think?

UPD: for difficult conditions, I think, we can use custom class that will be extends like a \yii\validators\Validator

[ddinchev]

Well, Yii 2.1 is going to have many breaking changes, this is up to @samdark and the team behind Yii2 to decide. We could have detection if the new pattern is used or the old definition, and let the framework handle both, and maybe remove the old syntax in v2.2.

[Auramel]

@ddinchev and it will we awesome decision!

[rob006]

It looks like overkill to me. You're trying to create sophisticated syntax to configure object (validator). Why not create object directly?

public function rules()
{
    return [
        [['login', 'pass'], new StringValidator(['min' => 0, 'max' => 100])],
        // with fluent setters
        [['login', 'pass'], (new StringValidator())->min(6)->max(255)],
    ];
}

It should be more intuitive, more flexible (works with custom validators), easier to implement and does not conflict with current syntax, so we could keep BC and support both syntax without much effort.

[Auramel]

@rob006 not a bad thought 👍 But we will have to rewrite all *Validator classes... It's difficult, no?

[ddinchev]

@rob006 it's maybe confusing to me because right now RequiredValidator class does not have min and max.

If that is just a copy/paste error and you don't intend to combine string and required validators -I don't see much of a difference between (new StringValidator())->min(6)->max(255) and Validator::string()->min(6)->max(255) - I'd be happy with both!

[Auramel]

I have an idea to add some ValidatorHelper || ValidationBuilder that replaced your builder syntax to current syntax. Good? Bad?

How example:

<?php

class ExperementalValidator 
{
    public static function required()
    {
        return 'required';
    }

    public static function min(int $min)
    {
        return ['min' => $min];
    }
}

[rob006]

@ddinchev I fixed my example - it was dump copy-paste :)

[Auramel]

Hey, guys, I made some wrapper on current rules. What do you think? https://github.com/Auramel/yii2-validation-rules.

See ValidationRule.php PHPDoc. Example in it.

It has a new syntax and not crash old. Perfect?

UPD: Has idea to call method build() in \yii\base\Model::validate() method. It cut some letters from your code 🤣

[viktorprogger]

@samdark will this be implemented? And in which form?

I like this variant the most.

[samdark]

Me too.

[dicrtarasov]

Will this chained calls slower then simple array of rules data? Even if a lot of objects?

[samdark]

Should not.

[viktorprogger]

I can make simple tests to show performance of both the variants. But first we need to finish our discussion about the final syntax in #108.

[rob006]

AFAIK validator object will be created from array anyway, so there should be no difference in performance. Especially on PHP7.

[viktorprogger]

I think it's a good idea to make a separate module for this functionality, e.g. yiisoft/validation and require it in yii-core. It will have a RequireValidation interface (requires getValidationAttribute, rules and validate methods) and a CanBeValidated trait (with default implementation of validate method).

Minuses:

• User should manually call validate method (only if we wan't require this module in yii-core)

Pluses:

• We will have an equal validation way for everything: models, controllers, lots of middleware, etc.

WDYT?

I think I'll make some free time for this in a month.

[samdark]

I agree. Validation could be separate library.

[samdark]

@miolae let's design it first though before implementing it. Would you please make a draft document explaining concepts about how to use it, where to use it, what are dependencies etc.?

[viktorprogger]

Yes, I'll do this, but after some higher priority things are completed. I'm going to show this draft in late march or april.

[viktorprogger]

Here is a sample of using the new package in an unusual way (inside a controller). Please provide any critics you can. I'll make a Validator class sample after this.

[samdark]

@viktorprogger controller example doesn't look right:

1. There are many actions possible and validating whole controller doesn't make sense there.
2. Each action context is different so trying to define rules() for all of them at once is asking for trouble.
3. Probably keeping validation closer to where it's used is better i.e. try defining rules in the action itself.

[viktorprogger]

I think validation should be applied to the controller because it collects the whole context. We can divide rules for each action (see the gist, rules method: there are 2 validations for login action and no validations for some action) but I don't see a way to take validation off a controller.

[viktorprogger]

@rob006 any comments?

[rob006]

@viktorprogger "Validation" of request and permissions is middlewares job, not validators. Decoupling validators from models may be good move, but using it in controller in this way is terrible example.

[viktorprogger]

It's reasonable

[viktorprogger]

I'll make another example later :)

[viktorprogger]

Here is a new draft of the library itself. It's not finished yet, it's just to show the way I see it. Feel free to ask anything or to give an advise.

[viktorprogger]

@miolae let's design it first though before implementing it. Would you please make a draft document explaining concepts about how to use it, where to use it, what are dependencies etc.?

I've added a readme file with concept describing and usage examples. If anybody needs it to be complemented - feel free to say me what to write there.

I've found a couple of bad places in this draft during creating the readme.

1. A bit earlier (here and here) we decided to add ability to pass instantiated and configured validators. On the other hand is another decision: @samdark said (can't find it now) objects mustn't know how they are configured, DI container must achieve that. So, validation rules in my implementation can't be configured through their __constructor. And now cool-looking variant [['login', 'pass'], new StringValidator(['min' => 0, 'max' => 100])] turns into [['login', 'pass'], $this->validator->getFactory()->get(StringValidator::class, ['min' => 0, 'max' => 100])]. I don't see an ideal way to get out of the problem. What can you suggest about this?
2. The only dependency I used is yiisoft/di to use \yii\di\Factory::create() method. I think it's an overhead to use the whole library to use just a single method but I didn't find a better way. Please make suggestions how to exclude this overhead.
3. [known issue] The result of validation and error existence in a rule are not coupled. You can return false during validation and it doesn't mean there will be an error in a rule instance. The reverse is true too.

[dicrtarasov]

To implement the rules of validation of business models with a large number of fields, the old syntax is much shorter, efficient and self-desciptive:

            ['views', 'trim'],
            ['views', 'default', 'value' => 0],
            ['views', 'integer', 'min' => 0, 'max' => 65535],
            ['views', 'filter', 'filter' => 'intval'],

then

new StringValidator(['min' => 0, 'max' => 100])]
$this->validator->getFactory()->get(StringValidator::class, ['min' => 0, 'max' => 100])]

[viktorprogger]

The old syntax is still possible, I've kept it.

[viktorprogger]

About option №1: I'm inclined to opinion to add magic configuration through constructor to the BaseRule, but not to the RuleInterface.

[samdark]

Interface should not care about configuration.

[rob006]

2. The only dependency I used is yiisoft/di to use \yii\di\Factory::create() method. I think it's an overhead to use the whole library to use just a single method but I didn't find a better way. Please make suggestions how to exclude this overhead.

IMO requiring library for this is not a problem, but using (new Factory())->create($config) directly is design smell, and really ugly solution. I don't really understand purpose of RuleFactory. Why not create validators instances directly in rules()?

[viktorprogger]

I don't really understand purpose of RuleFactory. Why not create validators instances directly in rules()?

It's decoupling of classes and dividing responsibility. Validator shouldn't care about how to create Rule instances, it's just adapter between Validatable and Rules. It's not about validating provided rule type and selecting a right instantiation way for that type. Its responsibility is to collect some information

• from Validatable to provide it to rules (before validation)
• from rules to provide it to Validatable (after validation)

Maybe it's a naming problem. If you have an idea about more clear naming - lets discuss it.

[rob006]

It's decoupling of classes and dividing responsibility.

Do we need it if we require to create ValidationRule object inside of rules()?

return [
    ['name', new RequireValidator()],
    ['email', new EmailValiadtor()],
];

[viktorprogger]

We allow to do so, not require. The preferred way is to create rules through DI container either using their string aliases (e.g. 'require' and 'email'), or instantiating them with DI container like in the last example in this comment.

[rob006]

The preferred way is to create rules through DI container either using their string aliases (e.g. 'require' and 'email'), or instantiating them with DI container like in the last example in this comment.

Why? If AR relies on specific implementation of validator, then changing it by DI container will change how AR works. IMO DI should not change business logic in this way.

Besides, you didn't answer my question. :P

[viktorprogger]

Why? If AR relies on specific implementation of validator, then changing it by DI container will change how AR works. IMO DI should not change business logic in this way.

If smthng relies on specific implementation, it must use this specific implementation, not some contract through DI. But if you has 10-20 places of email validation in your project (by standard yii validator), and suddenly a business task to change this validation appeared, it could be cool just to set your own email validator implementation through di (RuleFactory in this case). Or set some new rules to save uniformity of rule declarations (e.g. ['attribute', 'my-validation-rule']).

The factory is really useless in case when validation rule instances are created in-place. It's created to cover all other cases.

Besides, you didn't answer my question. :P

I answered as I see)) Could you please repeat it in other words?

[rob006]

If smthng relies on specific implementation, it must use this specific implementation, not some contract through DI.

That is my point - validation is internal logic of model, you should not rely on DI here.

But if you has 10-20 places of email validation in your project (by standard yii validator), and suddenly a business task to change this validation appeared, it could be cool just to set your own email validator implementation through di (RuleFactory in this case). Or set some new rules to save uniformity of rule declarations (e.g. ['attribute', 'my-validation-rule']).

And IMO this is great example why using DI for this this is dangerous. You're changing internal logic of models by configuring application, and you're doing it for all models and in implicit way. You don't even know which models may be affected (in some cases it may be undesired), and this is completely invisible from model perspective. It is convenient way to shoot yourself in the foot - creating custom validator (or some global config param) and using it in 20 places will take like ~15 minutes and may save you hours of debugging.

I answered as I see)) Could you please repeat it in other words?

I asked "do we need factory if we require instances from rules()?". You answered "we don't require". I did not asked "do we require?" but "what if we require?".

[viktorprogger]

IMO it is a good practice to allow user to use DI when he needs it and not to use DI when he does't need it. It's users' responsibility to choose if he need to use one way or another. IMHO if somebody uses DI he understands why does he use it.

Another point is that this library will not be used only with AR now. It could be used in any place where you need to have some data checking logic. Middleware, object factories, anywhere. So it is not just "internal logic of models" anymore. Another use cases are coming.

I asked "do we need factory if we require instances from rules()?". You answered "we don't require". I did not asked "do we require?" but "what if we require?".

No, we don't need factory if requiring instances from rules() is the only way of creating those instances.

[viktorprogger]

It could be great if we hear more opinions from YiiSoft team.