WIP: field data processors and validators

[la4de]

The purpose of this pull request is to demonstrate the functionality of validators. This new feature has been proposed and discussed in #788.

Description

New validators list is added to StrawberryField structure which contains list of callable validators. User defined validator function takes value as an parameter and returns validated value. Validator function is called after resolver functions with output types and before resolver with input types. See discussion in #788.

Both output and input types are supported.

def permission_validator(value, info):
    if not info.context.request.user.has_permission():
        raise Exception('Permission denied')
    return value

@strawberry.type
class User:
    # option 1, define validator as a field parameter
    name: str = strawberry.field(validators=[permission_validator])
    age: int = strawberry.field()

    # option 2, define validator with field decorator    
    @age.validator
    def age(value, info):
        if not info.context.request.user.has_permission():
            raise Exception('Permission denied')
        return value

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Documentation

Issues Fixed or Closed by This PR

• Field level data validators and pre/post processors #788

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[codecov]

Codecov Report

Merging #809 (31680fc) into main (e6b005c) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head 31680fc differs from pull request most recent head af7c825. Consider uploading reports for the commit af7c825 to get more accurate results

@@            Coverage Diff             @@
##             main     #809      +/-   ##
==========================================
- Coverage   96.83%   96.80%   -0.04%     
==========================================
  Files          70       71       +1     
  Lines        2370     2409      +39     
  Branches      333      339       +6     
==========================================
+ Hits         2295     2332      +37     
- Misses         48       49       +1     
- Partials       27       28       +1     

[patrick91]

I haven't seen the implementation yet, but I'm a bit confused on the terms, we are passing validators, but we are not doing any validation here 🤔

[la4de]

I'll add some data validation.

As discussed in #788, this same method can be used for data pre/post processing and validation. Could we have more generic name for this? Or do we want to separate these so that there would be separate validator and processor methods? Any opinions?

[la4de]

Hello team,

Are you happy with chosen approach? I'm happy to contribute and to start working on final implementation but before that I'd like to hear your feedback. Is this the way to go?

[patrick91]

I have a few comments, is this going to (also) replace our permission_classes? What about decorators, we wanted to support them too, see #473?

Also, do you think we can make it easier to support this pattern for handling errors with this approach: https://sachee.medium.com/200-ok-error-handling-in-graphql-7ec869aec9bc ?

[la4de]

I have a few comments, is this going to (also) replace our permission_classes? What about decorators, we wanted to support them too, see #473?

Validators can be used for permission validation as well. This can replace permission_classes if we decide so.

Decorators are nice but they can be used only with those fields which have resolver function. Validators can be used with both, fields with and without resolvers. I'd support both, decorators and validators. Decorators are more flexible, but as said they cannot be always used. @jkimbo, any comments?

Also, do you think we can make it easier to support this pattern for handling errors with this approach: https://sachee.medium.com/200-ok-error-handling-in-graphql-7ec869aec9bc ?

This is very good question. I haven't thought about that earlier. :)

I think we could interpret exception as errors. Alternative results could be provided by validators. For example:

@strawberry.type
class User:
    groups: List[Group] = strawberry.field()

    @groups.validator
    def groups(value, info):
        if not info.context.user.is_active:
            # inactive user, return error
            raise Exception('Invalid User')
        if not info.context.user.has_permission():
            # no permissions, return alternative response
            return []
        # validation passed, return original value
        return value

This works already!

[jkimbo]

I've been thinking about the proposal a lot and I think there is some really interesting ideas here but I'm not sure if it's the right approach. I also feel like the feature has expanded in scope quite a bit and I'd like to focus just on the validation part first. Some thoughts (numbered for easy replying):

1. I find it strange that the validators can modify the return value of the field. If we look at the example from attrs: the return value of the validator functions is ignored https://www.attrs.org/en/stable/examples.html#validators That makes more sense to me.

2. I think validators make the most sense for input types since they deal with user input. Object types are constructed by the developer so I would imagine that adding validation there is mostly unnecessary since you have full control over the values already before they get passed to the object type.

3. As @patrick91 points out this API doesn't allow you to model errors as part of the return type very easily. For example take this mutation which can either return a success state or an error:

@strawberry.input
class UserInput:
	email: str = strawberry.field(validators=[valid_email])

@strawberry.type
class InvalidEmail:
	message: str

@strawberry.type
class Success:
	user: User

Response = strawberry.union("Response", [InvalidEmail, Success])

@strawberry.mutation
def create_user(user_input: UserInput) -> Response:
	# validation has already happened by this point

If the input contains an invalid email the error will be raised before the mutation function body gets called and so there is no opportunity to return the InvalidEmail type.

Aside

A way to support this would be to add an explicit validate function to the input types so that you could run the validation in the resolver body. (or an is_valid function like in Django forms)

@strawberry.mutation
def create_user(user_input: UserInput) -> Response:
	try:
		user_input.validate()
	except ValidationError as error:
		return InvalidEmail(message="Invalid email address")

4. The above points then lead me to the question: are validators part of the GraphQL request/response or are they just concerned with the strawberry.type/input construction? I can't think of a use case where a validator would need access to the GraphQL context (other than permissions but I think that is better done elsewhere) and so I would lean to just supporting validation when creating the strawberry types. That would leave decorators to deal with any logic that does require the GraphQL context (e.g. permissions). This is closer to what Pydantic and attrs do and I think would be a valuable feature for strawberry.

Aside

It would actually be possible to support this form of validation in user land using the __post_init__ method:

@strawberry.input
class UserInput:
	email: str

	def __post_init__(self):
		if not valid_email(self.email):
			raise ValidationError("Invalid email")

I think there is still value in adding first class support for validators though FYI.

---

Sorry for such a long response @la4de but this proposal has been very thought provoking for me! Looking forward to hearing what you think. Also happy to jump on a call in Discord if that helps.

[patrick91]

3. As @patrick91 points out this API doesn't allow you to model errors as part of the return type very easily. For example take this mutation which can either return a success state or an error [...]

Oh, by the way, if we find a way to manage this and validation we could use the same approach for Pydantic (https://strawberry.rocks/docs/integrations/pydantic#input-types).

At the moment we do exactly what @jkimbo proposed:

@strawberry.mutation
def create_user(user_input: UserInput) -> Response:
	try:
		user_input.to_pydantic()
	except ValidationError as error:
		return InvalidEmail(message="Invalid email address")

which isn't that bad to be honest, but I wonder if we could simplify things a bit (if that's actually useful)

[la4de]

1. I find it strange that the validators can modify the return value of the field. If we look at the example from attrs: the return value of the validator functions is ignored https://www.attrs.org/en/stable/examples.html#validators That makes more sense to me.

I'm still preparing better sample code but meanwhile I want give another example: https://pydantic-docs.helpmanual.io/usage/validators/

Pydantic validators return value, which can be used for data cleaning and normalization.

[jkimbo]

1. I find it strange that the validators can modify the return value of the field. If we look at the example from attrs: the return value of the validator functions is ignored https://www.attrs.org/en/stable/examples.html#validators That makes more sense to me.

I'm still preparing better sample code but meanwhile I want give another example: https://pydantic-docs.helpmanual.io/usage/validators/

Pydantic validators return value, which can be used for data cleaning and normalization.

@la4de that is a fair point. I still find it a bit odd because the naming doesn't really match the functionality. I guess this is why Django forms have the clean_* methods rather than calling them validators.

[patrick91]

This is also a similar approach here: https://docs.nestjs.com/graphql/field-middleware

[patrick91]

Closing as we have extensions now 😊