jonathangeiger / kohana-jelly

See the link below for the most up-to-date code
https://github.com/creatoro/jelly
MIT License
146 stars 34 forks source link

More custom validation behavior #126

Closed bmidget closed 14 years ago

bmidget commented 14 years ago

While you are at it mucking around with making Jelly validation work a bit better than Validate does, I'd like to throw out for consideration the idea that Jelly's validate class follows the following slightly different rules:

  1. A rule only passes value as the first parameter in the callback if no parameters were specified
  2. If any parameters are specified, value must be explicitly stated — probably with ':value'
  3. An optional key passes a pretty name for error messages

Let me show some examples.

The first is of value being assumed, and the second and third are explicitly stated:

'rules' => array
(
    // Calls Validate::not_empty($value)
    'not_empty' => NULL,

    // Calls preg_match('/regex/', $value)
    'preg_match' => array('/regex/', ':value'),

    // Cals Validate::min_length($value, 10)
    'min_length', array(':value', 10)
);

Here is an example of a pretty name for a message

// Calls preg_match('/[a-z]+/', $value)
'preg_match' => array('lowercase letters only' => '/[a-z]+/', ':value')

Then in the messages file

// Username must be lowercase letters only
'preg_match' => ':field must be :param1'

Of course, if translation is used, the pretty name would be translated.

Allowing the pretty name keys is so much better than the K3 Validate system that forces us to either return a generic statement like "field does not match the correct pattern" or create many Validate::regex() clones that simply pass different parameters for the message to say the correct pattern.

These minor changes give lots more power to a rule callbacks, and perhaps most importantly, they enable you not to create asinine helper functions like Validate::regex() that do the same frikkin' thing as preg_match.

I personally think the level of control gained is great, and the API change actually makes more logical sense for validation rules.

jonathangeiger commented 14 years ago

Hi Ben, all of this looks really great. I've already added a bit of support for contexts and I want to add the suggestions you've brought forth.

Seeing as there's quite a bit of integration going on between Formo and Jelly, we should probably be working on something that will work together rather than reinventing the wheel. I noticed an issue you closed on Formo about Jelly being too tied to the Validate class, and banks mentioned you had a bit of trouble integrating our validation into Formo.

What I'm interested in is cleaning up the rule/filter/callback declaration style so that all types of callbacks are allowed as well as parameters. Here is the basic syntax I'm thinking of:

array(callback $callback [, array $params])

And if you don't need params:

callback $callback

And the contexts that I want to implement:

// ...
'filters' => array(
    array(array(':field', 'foobar'), array(':model', ':field', ':validator', ':value'))
)

All of the contexts are converted to their actual object (or value, in the case of :value). So :model is the actual model being validated, :field is the actual field, etc. This means that people can call filters/rules/callbacks on the actual model instance being validated.

I've talked with banks about this a bit and what concerns me about going off to a different place than Kohana is that there may be plans to change things around in 3.1.0 anyway. See these issues:

I might ask shadowhand directly what the plans are for 3.1.0. I don't know if that's imprudent or not.

It also may be good to figure out what it is we all need from Validate post an issue, offer to write it and hope it'll be merged into 3.1.0.

jonathangeiger commented 14 years ago

Posted a feature request here:

http://dev.kohanaframework.org/issues/2966

jonathangeiger commented 14 years ago

Threw up some ideas here:

http://github.com/jonathangeiger/core/commit/24a851934bee0ea7baab22560edca20ca26fa351

This has pretty much all of the features you asked for Ben. It'd be nice to see this merged into core for 3.1 and for you to be able to work with this for Formo.

We'll wait and see what they say.

bmidget commented 14 years ago

Nice. With that syntax, the two validation styles will certainly work hand-in-hand. And it's funny because I've had to implement a "context" system (oddly enough I used :model for a Jelly model as the context, too) in formo.

This would be sufficient for me to work in tandem with any modules that use K3's built-in Validate library without any problems whatsoever.

Moving rules to individual objects is so much better it's not even funny.

I sure hope they seriously consider this approach.

bmidget commented 14 years ago

One little side note, though, is I believe your implementation breaks the param1, param2, etc... syntax for messages as ":value" can technically be any parameter now. Also the current implementation always shifts ":value" from the parameters.

bmidget commented 14 years ago

I added to the issue you posted a link to my blog post about the requirements for an adequate validation system. I believe your push takes care of most of them.

http://bmidget.tumblr.com/post/703195192/requirements-for-an-adequate-validation-library

jonathangeiger commented 14 years ago

I'm going to keep working on this then and see what they say. I realize that I can get it completely backwards compatible with the old Validate class, which would be ideal.

I'll be going off of your blog post as well.

bmidget commented 14 years ago

Backwards compatibility. Pretty sweet.

I've been thinking about one thing. And I think this certainly applies to Jelly as well as Formo that are both field-centric systems.

In my perfect validation world, each field would actually hold a mini validation object full of rules, filters, callbacks, whatever. And a validation object could hold other validation objects.

Thus running $model->username->validate() would pull the individual object and run it's validation stuff. But perhaps $model->validate() would create a validate object on the fly and fetch each validation object located in each of the fields and run validation on those?

I may be way off here, but I wanted to throw the idea out there.

This is almost what I have done in Formo except each field holds arrays of rule, filter objects that are gathered at validate() time. But individual objects inside a single is so much simpler to work with.

Thoughts?

jonathangeiger commented 14 years ago

I don't really see the need for 1, 2, and 3 of your blog post in Jelly or in Kohana's Validate system. It seems like a lot of complexity for very little gain. Can you expand upon why it's necessary to be able to validate a single field, or copy and remove rules?

I can see how Formo might need rules to be copyable, but it seems like something that's very much specific to your use-case. If it needs rules to be copyable, however, the originals are always going to be available on the ORM model, or the Jelly's meta object. I don't really see why they should be removable either. It seems like conditions would be more useful than dynamically removable rules.

bmidget commented 14 years ago

Your point is well taken. I think what you're right for general use scenario, which is what the Kohana-packaged library is for.

Certainly from just a few minor changes, then, this will be sufficient for all my use, and will make working hand-in-hand with all Jelly users a seamless experience.

jonathangeiger commented 14 years ago

I'm going to give it a bit of time to see what the core devs think of it and to see if there are any alternate solutions that don't require hoping to get these changes into 3.1.0. I'm really torn because the features are—in my opinion—necessary to move forward with Jelly, but I hate to break from Kohana's validate system.

I'll keep you up to date.

jonathangeiger commented 14 years ago

Good news.

bmidget commented 14 years ago

Nice work man. This is indeed very good news.

jonathangeiger commented 14 years ago

Closing, since this is all done.

banks commented 14 years ago

Great work Jon