Basic usage discussion

[drgomesp]

This issue is aimed to allow us to discuss the basic usage of Respect\Validation rules inside of a Symfony application using the ValidationBundle.

As previously discussed in #112, we are probably going to need a Metadata class to delegate the validation calls to the right validation rules.

We could create this Metadata class as a Symfony custom constraint simple class.

I'm not sure how, but the usage would be something like this:

class User
{
    /**
     * @Respect\Validation\NotEmpty
     * @Respect\Validation\Between{min="6", max="24"}
     */
    protected $password;
}

Or we can talk about what @alganet proposed, which is a more complex integration using the power of Symfony's DI component to inject validators as services bind rules to class properties.

It would be something like this:

# app/config/config.yml
services:
    validator.user.password:
        class: Respect\Validation\Validator
        calls: 
            - [string]
            - [alnum, [_]]
            - [length, [6,24]]

And bind the created rules using a Bind annotation:

/**
 * NOT SURE WHAT TO IMPORT HERE
 */
class User
{
    /**
     * @Respect\ValidationBundle\Bind(service=validator.user.password)
     */
    protected $password;
}

Let there be discussion.

[filhodanuvem]

I think that we can work with the two purposes:

1) Create the constraint Bind like a façade is simple and to readability of code, I think that it is better.

<?php 

use Respect\ValidationBundle\Constraints as Respect;  

// into class
    /**
     * Right here, Symfony will search the Respect\ValidationBundle\Constraints\NotEmpty 
     * class and not will find.
     * @Respect\NotEmpty
     */
    protected $password;

  /**
  * This way, Symfony will use the one constraint class Bind (our metadata),
  * that would create the validator of the respect 
  * and call validate() method. Using 'rule' attribute we don't would use a service. 
  * @Respect\Bind(rule="notEmpty")
  */
   protected $password;

2) Custom services If a user want pre-configure a validator, he would write a service and he would use it with the constraint Bind, as you guys said.

<?php 

     /**
     * @Respect\Bind(service="validator.user.password" )
     */
    protected $password;

or in action of a controller

public function saveAction()
{
    // ... 
    $validator = $this->container->get("validator.user.password");
    $valitator->validate($user); 
} 

concluding, one thing does not exclude the other.

[drgomesp]

... one thing does not exclude the other.

Totally agreed.

I guess we can start working on a first experimental version and see how that goes.

[filhodanuvem]

Can you commit the bundle empty? I guess that, at first, we will create the Constraint.

[alganet]

Nice, so it's possible to create a single Constraint for all of our rules =D

I have a question: what namespace should we use for the code? "Validation" or "ValidationBundle"?

[drgomesp]

@cloudson

Can you commit the bundle empty?

Once I have a basic structure, I will.

@alganet

Nice, so it's possible to create a single Constraint for all of our rules =D

Yes, we will have one single metadata constraint class for each and every rule.

I have a question: what namespace should we use for the code? "Validation" or "ValidationBundle"?

We will need to use the ValidationBundle namespace, according to the Symfony best practices guide.

I don't think that will be a problem, though. o/

I'm already starting with the code.

[filhodanuvem]

We will need to use the ValidationBundle namespace ...

I agree!

One more question: Which name would have the constraint Metadata ? Bind is the better name for you guys?

[drgomesp]

Which name would have the constraint Metadata ? Bind is the better name for you guys?

I don't think Bind would represent a good constraint name, even known it will be a metadata class.

We should probably come up with a more verbose name.

What do you think, @alganet ?

[filhodanuvem]

Maybe, about names, we have that think about other feature: How we will write the rules :

• v::allOf()
• v::oneOf()
• v::noneOf()

If we write two rules in a attribute, this will be checked with AllOf, but sometimes, we want use OneOf (or maybe NoneOF).

Proposals:

1) Simple assert

/**
* "assert" is a proposal of name 
* @Respect\Assert(rule="string")
* @Respect\Assert(rule="alnum")
*/
protected $login; 

2) Using one constraint with many rules

/**
* AllOf is a alias to many Asserts 
* @Respect\AllOf(rules={"string", "alnum"})
*/
protected $login; 

and...

/**
* @Respect\OneOf(rules={"cpf", "empty"})
*/
protected $cpf; 

With this proposals, we don't have only one constraint (again), but I think is better decouple this problem instead of use only one class with many attributes that resolves everything.

If we will use services in everything complex cases, forget this comment.

[alganet]

I was doing some drafts of the best annotations I could write as an experiment despite the difficulty of implementation:

use Respect\ValidationBundle\Assert;
use Respect\ValidationBundle\Check;
use Respect\ValidationBundle\Validate;
/**
 * @Respect\Assert({
 *    {string},
 *    {alnum={_}},
 *    {length={1,15}}
 * })
 */
 protected $screenName;

This is similar to the Validation's own API and can handle complex things 'cause it's based on the same built structure. We can use a single Assert as well and all samples from our docs will make sense for annotation users as well.

Having one class in the bundle for each rule in the Validation is really bad for maintenance :( I would like to avoid that as much as possible.

[drgomesp]

@alganet

Having one class in the bundle for each rule in the Validation is really bad for maintenance :( I would like to avoid that as much as possible.

You should probably not worry about that. In the worst case scenario, we would have to have two constraints, one for general rules, other for AllOf and OneOf cases (and related).

Still, I think the last usage you proposed is achievable.

I'm already close to that with what I'm doing.

Soon I will commit an experimental version and we'll se how it goes.

Do you have an idea on how to use the AllOf or OneOf cases with this last proposal?

@cloudson

I like the proposals, but I still think we can reach some deeper level of abstraction.

Check my answer to @alganet, I think we can couple your proposals with his last one and reach something pretty solid.

I'm already on it.

[filhodanuvem]

@drgomesp

You should probably not worry about that. In the worst case scenario, we would have to have two constraints, one for general rules, other for AllOf and OneOf cases (and related).

Exactly!

I think we can couple your proposals with his last one and reach something pretty solid.

I agree but I guess that use Assert Constraint to every case makes this api confused. For now, I'm thinking about how write rules with params of a simple way. :confused:

[alganet]

Like in the v:: chain, every validator using annotations is an AllOf. Even the Validation class extends AllOf, so the following annotation. I've changed the syntax a little bit:

use Respect\ValidationBundle\Assert;
class User {
/**
 * @Respect\Assert({ string={}, alnum={_}, length={1,15} })
 */
 protected $screenName;
}

Is equivalent to the following validation:

v::string()->alnum('_')->length(1,15)->assert($userInstance);

Since we can use ->validate(), ->assert() and ->check() on validator instances, we can use them in annotations as well:

use Respect\ValidationBundle\Check;
class User {
/**
 * @Respect\Check({ string={}, alnum={_}, length={1,15} })
 */
 protected $screenName;
}

This is similar to the previous, but uses the Validation check mechanism, equivalent to:

v::string()->alnum('_')->length(1,15)->check($userInstance);

The same goes for ->validate() and @Respect\Validate. This is an important feature of Respect\Validation and I would like to provide that for our annotation users.

The OneOf rule can be expressed as such:

/**
 * @Respect\Assert({ oneOf={ nullValue={}, date={Y-m-d}, date={Y-m-d H:i:s} } })
 */
 protected $created;

It represents that this property can be either a null value, a simple date with Y-m-d or a simple date with Y-m-d H:i:s. Equivalent to v::oneOf(v::nullValue(), v::date('Y-m-d'), v::date('Y-m-d H:i:s'))->assert($obj);

Rules can be beautifully nested that way in the same way as the v::something or new Rules\Something API (which Validation already supports).

Also, this API allows really expressive constructions such as this:

/**
 * @Respect\Assert({ 
 *    call={
 *        parse_url, {
 *            arr={},
 *            key={scheme, startsWith={http}},
 *            key={host, domain={}},
 *            key={path, string={}},
 *            key={query, notEmpty={}},
 *        }
 *    }
 *})
 */
 protected $uri;

Which is equivalent to this sample from our docs:

v::call(
    'parse_url',
     v::arr()->key('scheme', v::startsWith('http'))
        ->key('host',   v::domain())
        ->key('path',   v::string())
        ->key('query',  v::notEmpty())
)->validate($url);

[nickl-]

+0 on @alganet's approach which is the only one that made sense to me so far.

@drgomesp perhaps only do one or two for now and commit to save having to redo lots. I'm still not completely sure I follow what we're trying to do here. Code should place that in context I'm sure...

[nickl-]

There are 3 methods for validation if I recall

• assert - Respect\Assert
• check - Respect\Check
• validate - Respect\Validate

[drgomesp]

Guys,

I totally agree with the last proposal, especially with the differentiation between the assert, check and validate methods.

@nickl-

My intent is to commit as much as possible. It's just that this start has still been kinda worky.

@alganet

About your proposition:

/**
 * @Respect\Assert({ string={}, alnum={_}, length={1,15} })
 */
protected $screenName;

I'm affraid we would have to write it a little different to work with the annotations support from Symfony and Doctrine, something like:

/**
 * @Respect\Assert(rules = { "string"={""}, "alnum"={"_"}, "length"={1, 15} })
 */
protected $screenName;

I don't think that would be a huge problem, though.

I'm changing the code to support the 3 methods we talked about.

[filhodanuvem]

I believe that we don't need of the 3 methods. To Doctrine, the goal is ever generate a Violation and this is done returning true or false.

<?php
class CpfValidator extends \Symfony\Component\Validator\ConstraintValidator
{
    // this method defines if attribute $cpf is valid, we don't want a exception. 
    // i.e. maybe only validate() method will to work.
    public function isValid($value, \Symfony\Component\Validator\Constraint $constraint)
    {
        $isValid = v::oneOf(v::cpf(), v::nullValue())->validate($value);

        if(!$isValid) {
            // $constraint->message is a param that the user injects by annotation
            $this->setMessage($constraint->message);
        }
        return $isValid;
    }
}

[drgomesp]

@cloudson

The isValid method is deprecated since 2.1 version.

We need to use the validate method and add context violations. There is no return value anymore.

Check this:

try {
    v::oneOf(v::cpf(), v::nullValue())->validate($value);
} catch(\InvalidArgumentException $e) {
    $messages = $e->findMessages(array('cpf', 'nullValue'));

    foreach($messages as $message) {
        $this->context->addViolation($message, array('{{ value }}' => $value));
    }
}    

But you got a point there saying we might not need to have the 3 methods.

I think all of them generate the expected exception, right @alganet and @nickl- ?

[filhodanuvem]

@drgomesp

The isValid method is deprecated since 2.1 version.

You are right. :+1:

Only be carefull with messages, normally a user will write the message in the annotation (or xml) instead of use the default of the Respect. What you think about is?

[drgomesp]

@cloudson

Only be carefull with messages, normally a user will write the message in the annotation (or xml) instead of use the default of the Respect. What you think about is?

I think we could ignore this at this point and just use the default rules messages.

Maybe later we can implement the custom message feature.

[drgomesp]

Ok guys,

with what we have for now it's possible to do some basic stuff, such as:

/**
  * @Respect\Assert(options = { "string"={""}, "alnum"={"_"}, "length"={1, 15} })
  */
protected $name;

@alganet

About the last sample you've presented, I had to change it a little bit to fit the annotations pattern from Symfony and Doctrine. It would be something like this:

/**
  * @Respect\Assert(options = { 
  *    "call"={
  *        "parse_url", {
  *            "arr"={""},
  *            "key"={
  *                {"scheme", "startsWith"={"http"}},
  *                {"host", "domain"={""}},
  *                {"path", "string"={""}},
  *                {"query", "notEmpty"={""}}
  *            }
  *        }
  *    }
  *})
  */
protected $uri;

To represent:

v::call(
    'parse_url',
     v::arr()->key('scheme', v::startsWith('http'))
        ->key('host',   v::domain())
        ->key('path',   v::string())
        ->key('query',  v::notEmpty())
)->validate($url);

This is not functional yet, I'm still working on a way to resolve all of this chain calls (recursively, maybe?) inside the ConstraintValidator.

I would really love some feedback from each one of you about everything we have so far.

@alganet , @cloudson , @nickl- ?

[filhodanuvem]

@drgomesp are you tried uses this bundle with doctrine? Right here, the tests are ok but using with doctrine its break. I have some questions:

1) I guess that we can't use Depedency injection in constructor of ConstraintValidator. Symfony creates that object without arguments (and I guess too that isn't possible configure it).

2) You overwrote the method validateBy for to return 'respect_validation' but this isn't a valid classname (for example, using Assert, symfony will search for AssertValidator instead of respect_validation. Removing this method, it's works.

If this problems happened in your workspace, I will commit this changes and others with a PR.

[drgomesp]

@cloudson

I've tested this with a symfony standard application, which comes with Doctrine bundled, and I got no errors at all.

Can you give some more information on the error?

1) I guess that we can't use Depedency injection in constructor of ConstraintValidator. Symfony creates that object without arguments (and I guess too that isn't possible configure it).

I've managed to get this working just the way it's currently implemented. I really see no problems with this. As you can see, the dependency is injected through the service container:

<service id="respect.validator" class="Respect\Validation\Validator" />

<service id="respect.validator.assert" class="Respect\ValidationBundle\Validator\Constraints\AssertValidator">
    <argument type="service" id="respect.validator" />
    <tag name="validator.constraint_validator" alias="respect_validation" />
</service>

2) You overwrote the method validateBy for to return 'respect_validation' but this isn't a valid classname (for example, using Assert, symfony will search for AssertValidator instead of respect_validation. Removing this method, it's works.

Again, as you can see in the service container, I'm using an alias for the class name, being respect_validation. That is set with the tag element:

<tag name="validator.constraint_validator" alias="respect_validation" />

About the assertion, the annotation can be whatever you want it to be, but the namespace should be imported as:

use Respect\ValidationBundle\Validator\Constraints as Respect;

I really see no problems with the current implementation.

Please provide some more information on the errors and we can try to solve them.

[filhodanuvem]

Probably I was using a old version of the symfony, I again create a standard project and everything works. :smile:

[filhodanuvem]

@drgomesp

Are you thinking about next steps ? I believe that is better solves the small problems at first like a "custom messages". Is common someone write

/**
* @Respect\Assert(options = { "string"={""} }, message="message one")
* @Respect\Assert(options = { "alnum"={"_"} }) // message default 
* @Respect\Assert(options = { "length"={1, 15} })
*/
protected $name;

instead of

/**
  * @Respect\Assert(options = { "string"={""}, "alnum"={"_"}, "length"={1, 15} })
  */
protected $name;

[drgomesp]

@cloudson want to start talking again about this bundle? It's been some time since I actually code anything here.

[filhodanuvem]

Hey @drgomesp !! Good see you! I'm yet interested on this project, let's go back to talk about it.