graphql-python / graphene

GraphQL framework for Python
http://graphene-python.org/
MIT License
8.09k stars 826 forks source link

Having custom validation over scalars #1070

Open mxamin opened 5 years ago

mxamin commented 5 years ago

I need to pass some validation check when defining Arguments for Mutation. I know I can check it inside the mutate function but I need this check before that (similar to what we have in marshmallow.validate module: Range and Length).

I just want to know if there is any future plan to implement it inside the graphene or not.

KaySackey commented 5 years ago

You can create a custom scalar that does validation. It’s what I do anyways, when accepting input strings to validate that user data doesn’t contain HTML.

mxamin commented 5 years ago

But the scalars won't accept inputs, suppose I want to define a new scalar which accepts strings with specific lengths, in that case, I want to pass the length value to my custom scalar but as far as I found out, scalars won't accept input parameters and you have to hardcode the length. Correct me if I'm wrong :)

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

KaySackey commented 4 years ago

Not sure what you are doing in particular, but Scalars can accept kwargs. You just have to access them via self.kwargs

chpmrc commented 4 years ago

Sure, you can pass kwargs but where exactly would the validation logic run and be called? parse_value, serialize? Also there's no access to the supertype so what if I wanted to show a hierarchy/path to the wrong value (e.g. items[0].email is invalid)?

jkimbo commented 4 years ago

@chpmrc do you need to validate before the mutate function? (just trying to understand the use case better)

chpmrc commented 4 years ago

Yes, similarly to how Django validates input in the forms, before calling the various create, update, etc. I threw together a decorator for pre mutation validation for graphene, I'll see if I can open a PR once it's polished.

jkimbo commented 4 years ago

I was going to suggest using a decorator! I think that is the easiest approach to add validation.

I'd be interested in seeing a PR if you get it working.

chpmrc commented 4 years ago

@jkimbo so I came up with this PoC and I was hoping you could take a look to maybe integrate it in Graphene (after some refinements): https://github.com/chpmrc/graphene-validator . Let me know what you think! (I put the repo together quickly but we're already using it in production)

chpmrc commented 4 years ago

@jkimbo did you have a chance to look at the repo I linked above? Cheers!

vancouverwill commented 4 years ago

Sure, you can pass kwargs but where exactly would the validation logic run and be called? parse_value, serialize? Also there's no access to the supertype so what if I wanted to show a hierarchy/path to the wrong value (e.g. items[0].email is invalid)?

@chpmrc I got the validation we needed working by using a custom Scalar and adding the validation on parse_value() as that does get called by the Graphene library before input is passed to either query or mutations. Adding an example here as the docs are super sparse and might of help to you or someone else in the future

class Email(String):

    @staticmethod
    def parse_value(value: Text) -> Text:
        if is_valid_email(value):
            return value
        raise GraphQLError(
                f"Invalid Graph ID {value}, should be hexadecimal format 0x1f1f"
            )

def is_valid_email(email):
    # do some regex or other validation
stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

chpmrc commented 4 years ago

@vancouverwill thanks for that example! Does it return the path to the invalid field in the error? E.g. if you send an object {user: {name, email}} will path be [user, email] in case of invalid email?

hypnotic-frog commented 3 years ago
class Email(String):

    @staticmethod
    def parse_value(value: Text) -> Text:
        if is_valid_email(value):
            return value
        raise GraphQLError(
                f"Invalid Graph ID {value}, should be hexadecimal format 0x1f1f"
            )

def is_valid_email(email):
    # do some regex or other validation

This doesn't work for me. I used exactly the same example as above and the parse_value function never gets called. How is this possible? Anyone has found a reliable way to add custom validator to a custom scalar type?

Thanks!!

meysam81 commented 3 years ago

@hypnotic-frog

Turn parse_value to parse_literal and it'll work. Tested and got the result.

adamsanghera commented 3 years ago

Putting up this interface for consideration, which would mirror interfaces from similar libraries with validation hooks.

def is_valid_email(value: str) -> bool:
    """Validate an email.

    :raises: ValidationError, if email is invalid.
    """
    # whatever
    return True

class Foo(graphene.Mutation):
    class Arguments:
        email_address = graphene.String(validators=[is_valid_email])

I'm not very familiar with graphene's internals, but I think you could add a function, Field.validate, and add validators: [Callable -> bool] to __init__.

def validate(self, value):
    for validator in self.validators:
        validator(value)
    return value

which would then be called, after the value is resolved (this part I'm less familiar with).

This would be a fairly large extension to the surface area of Field which is, at present, quite narrow.

However, there are many lines of code spilled over more-magical implementations of validate. IMO, graphene deserves to have validation as a first-party feature, and for it to be a "declarative" implementation, so you can understand the input space of a variable in one closure.

If a maintainer validates (zing) that this would be a desirable interface, I'd be happy to take a stab at it.

aryadovoy commented 2 years ago

Is it possible to pass info in parse_value()?

And how can I raise multiple errors after validations from all inputs?