feature/validation

[murtukov]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no (maybe?)
Deprecations?	no
Tests pass?	yes
Documented?	yes

Documentation:

Please first read the documentation completely. If some sections are not clear let me know, I'll try to rewrite them in more clear words.

Think about the titles, maybe some of them should be changed. There is a section called How does it work?. Is the name ok or should we rename it to something like Internals? Any other suggestions?

Architechture:

When you call graphql:compile, the Generator creates definition classes. Currently all resolvers always look something like this:

'resolve' => function ($value, $args, $context, ResolveInfo $info) use ($globalVariable) {
    return $globalVariable->get('mutationResolver')->resolve(["createUser", [0 => $args]]);
},

there is no extra code, just calling $globalVariable->get('...')->resolve([...]).

I modified the generator classes so, that when you add validation to your configuration and call graphql:compile, the Generator creates an extra code in the resolver of your definition classes, something like this:

<?php
// ...
'resolve' => function ($value, $args, $context, ResolveInfo $info) use ($globalVariable) {
     $validator = new InputValidator(
         func_get_args(),
         $globalVariable->get('container')->get('validator'),
         $globalVariable->get('validatorFactory'),
         [
             'class' => [
                 'link' => null,
                 'constraints' => [
                     new Assert\Expression('arguments([], [])'),
                 ],
                 'cascade' => null
             ],
             'properties' => [
                 'username' => [
                     'link' => null,
                     'constraints' => [
                         new Assert\Length([
                             'min' => 6, 
                             'max' => 32, 
                             'minMessage' => 'createUser.username.length.min'
                         ]),
                         new Assert\NotBlank([
                             'message' => 'Something is empty'
                         ]),
                     ],
                     'cascade' => null
                 ],
                 'password' => [
                     'link' => null,
                     'constraints' => [
                         new Assert\Length([
                             'min' => 6, 
                             'max' => 32
                         ]),
                         new Assert\IdenticalTo([
                             'propertyPath' => 'passwordRepeat', 
                             'message' => 'Passwords are not equal!'
                         ]),
                     ],
                     'cascade' => null
                 ],
                 'passwordRepeat' => null,
                 'birthday' => [
                     'link' => null,
                     'constraints' => [
                         new Assert\Date(),
                         new Assert\Callback([
                             'callback' => [
                                 'App\Util\Validator', 
                                 'greaterThan'
                             ], 
                             'payload' => '-18 years'
                         ]),
                     ],
                     'cascade' => null
                 ],
             ]
      ]
     );

     return $globalVariable->get('mutationResolver')->resolve(["createUser", [0 => $args, 1 => $validator]]);
    },
//...

This will be done by the generateExtraCode() method declared in the lib\ folder as abstract, so that the code under the lib\ folder remains generic and it's implementation locates under the src\ directory.

The main reason why I decided to make it all by Generator is because of its performance:

1. The InputValidator class will be instantiated only when you call a resolver.
2. If you have for example 20 resolvers, it won't instantiate all validators, but only those, which are in the requested resolvers.

So it doesn't create any services and therefore creates no overhead.

The generator classes won't generate extra code if you dont declare the validator variable in the expression e.g.:

# This will trigger the generator to generate extra code in 
# the resolver callback
resolve: "@=res('my_resolver', [args, validator])"

# and this won't generate anything in the resolver callback
resolve: "@=res('my_resolver', [args])"

if you have any thoughts about this solution, let me know.

Formatting of validation errors

It uses the built in feature Error handling to return validation errors to the client. I created a class Overblog\GraphQLBundle\Validator\Formatter to change the response for exceptions of class ArgumentsValidationException.

Validators

All the validation logic is located under the src\Validator directory.

Annotations and ArgumentsTransformer class.

@Vincz I assume that the annotation feature is mainly your work. Currently my validator implementation works only for yaml configuration. I would like to also add support for annotations, but first I would like to discuss your implementation of the class ArgumentsTransformer with you , because it has it's own validation mechanism which conflicts with my implementation. I will create a separate issue for that, but for now I ask you to stop any further development of the ArgumentsTransformer. Maybe it will be removed or modified, to use this validation feature (depends on our discussion).

---

I will edit and add new stuff to this comment during the discussion.

[akomm]

@murtukov validator injection is good. Also the option not to throw. I had similar feature in mind for the ArgumentsTransformer.

Reason: There are few opinionated way handling user input errors. Not all of them are handled via error and/or extension. Currently ArgumentsTransformer throws and does not ask questions. I wanted to add a feature to either being able to inject errors (ConstraintViolations) or validator. Would be kind of obsolete with this PR :)

Would it be possible to directly inject errors via expression, so ->validate is called implicitly? Not that big of a deal if not.