Unexpected behavior of CValidator::safe

Yii 1.1 General Discussion
1

Since Yii 1.1.x the declaration of safe attributes has changed (discussed here). Unfortunately i find the new way very tedious if you have many scenarios and multiple rules per attribute, which is rather often the case in my models. Let me show you why.

Take these example rules for a username:

  • must only contain specific characters

  • must be unique

  • must (for some reason) not be an existing email address

  • is required in some scenarios

And we could have different scenarios like login, register, invite, edit-profile, admin. Username should not be safe in all scenarios, only in login, register, invite and admin. To achieve this we need 3 rules and specify all of the 4 safe scenarios on each of them. This is really tedious:


array('username', 'match', 'pattern'=>'/[a-z]+/', 'on'=>'login,register,invite,admin'),

array('username', 'unique', 'on'=>'login,register,invite,admin'),

array('username', 'unique', 'attributeName'=>'email', 'on'=>'login,register,invite,admin');

array('username', 'required', 'on'=>'login,register,invite,admin'),

You see that most of this code is the same scenarios repeated again and again. Situation gets even worse with complex models with many attributes that should be used in different scenarios: We have to declare the safe scenarios with every rule.

Now i noticed that CValidator has the safe property and thought it might solve my problem. My expectation how this works was like this:




array('username', 'match', 'pattern'=>'/[a-z]+/', 'safe'=>false),

array('username', 'unique', 'safe'=>false),

array('username', 'unique', 'attributeName'=>'email', 'safe'=>false),

array('username', 'required', 'on'=>'login,register,invite,admin'),

Only the last rule is used to define, where an attribute is considered safe. Less to type and much cleaner. Also easier to add/remove scenarios. This resembles the 1.0.x behavior: Rules are separated from the declaration of safety. I would prefer this in some cases for the reasons described above. But this doesn’t work as expected. With this setup username is never safe!

To make a long story short:

It seems like safe=>false translates to: This attribute is not safe, no matter what other rules define.

I would prefer if it translates to: This attribute is not safe, unless there is another rule that has safe=>true (which is the default value for any rule).

2

Wow, so many opinions, thanks …

So either no one uses models with such a complex rule setup in different scenarios, no one cares about the complex rules or i’m just dumb and missing something.

3

Yii seems to punish you each time try to do something is not 100% standard.

You can:

Set all rules like "not safe", and then extend safe attributes for make it safe as you need (this is an horrible workaround, but is the simpliest solution).

Create the rule using a bit of coding:




$mainRules=array([...]);

if (($this->scenario=='login')&&[..])

   $mainRules= array_merge($mainRules, array(

array('username', 'match', 'pattern'=>'/[a-z]+/'),

array('username', 'unique'),

array('username', 'unique', 'attributeName'=>'email'),

array('username', 'required'),

));

Those are all workaround I usually use. About your request to change the behaviour of safe, I guess that will never taken in consideration, because will break BC.

And I guess that there is a lot of case in wich is better to use this other formulation.

4

Could the unsafe validator help you ?

In your example you put all validators for username without the scenarios and then say that username is unsafe on ‘edit-profile’

5

Thanks for your suggestions, guys.

@zaccaria:

Yeah, that would be a way to solve it, but i tried to avoid constructs like these. It doesn’t make things more readable in my opinion.

@mdomba:

I like your idea, this should make things much cleaner. Why do i always forget to think about "inverse logic"?

As soon as i find time i’ll try your approach on some concrete models to see if there are some pitfalls. The only concern i could see is that you could forget to unsafe some attributes. But still this is much cleaner than reading this messy mix of rules and scenarios above.

6

Tried now with a complex model (many attributes, many scenarios). I still end up with a lot of writing, because now for each scenario i have to list all the unsafe attributes. E.g. for login only 2 attributes are safe, so i have to list pretty much all attributes there.

Any other ideas are welcome…

7

This is an interesting real life topic… can you post your current validation rules… and your needs so we can analyze it and try to come with something useful

8

Well, just extend my above example with more attributes common for a user (email, address, phone, imageUrl, etc.), think through the example scenarios i gave above and you’ll soon end up with the same problem.

What i try now is to override getSafeAttributeNames(). This way i can mimic the behavior from in 1.0.x and split up the definition of validation rules and safe attributes again:





    public function getSafeAttributeNames()

    {

        switch($this->scenario) {

            case 'login':

                return array('login','password');

            case 'register':

                return array('login','email','firstname','lastname','address1',...

Even though it’s a little more to type this seems like the best compromise between readability and amount of code required.