strawberry-graphql / strawberry

A GraphQL library for Python that leverages type annotations 🍓
https://strawberry.rocks
MIT License
4.01k stars 533 forks source link

Validate resolver types against field types #1149

Open patrick91 opened 3 years ago

patrick91 commented 3 years ago

Current we don't throw errors in cases like this:

def example() -> str:
    ...

@strawberry.type
class Q:
    x: int = strawberry.field(resolver=example)

We should raise an error when this happens. We should also maybe allow to change the error to a warning, to make it easier to upgrade.

We could also prevent this (maybe as opt-in/out behaviour):

def example():  # missing type
    ...

@strawberry.type
class Q:
    x: int = strawberry.field(resolver=example)

Upvote & Fund

Fund with Polar

estyxx commented 3 years ago

I was looking at this a bit. I think I found the place where introduce the change but looking at this question think can be complicated, especially for complex type annotation... as far I can see there isn't a built-in way to do it.

Did we already have an idea about how to check this? Or there are any Strawberry's objects that can help on this?

BryceBeagle commented 3 years ago

The good news is that this is a bit easier because we don't need to interact with raw Python types. You can make use of StrawberryType.__eq__. I think the types should exactly match, (i.e. -> int should not satisfy -> float), so __eq__ should be enough.

estyxx commented 3 years ago

That would be a nice strategy, but I was trying to put the check here as the TODO suggest, something like: if field_.type != field_.base_resolver.type: but these two types are not StrawbarryType always, because StrawberryField.type returns both StrawberryType and Type... so the check can fail even if when theoretically they are the same... :( Any idea? :D

BryceBeagle commented 3 years ago

because StrawberryField.type returns both StrawberryType and Type

Can you give an example of when this occurs where this should pass? I would have thought that there'd be no time where a StrawberryType should be equivalent to a Type?

bikeshedder commented 2 years ago

I'm generating some of my resolvers dynamically. I don't define the return type or use Any in this case. If you implement this feature please don't forget this case as a resolver might return the right type at runtime but is unknown at the time it's created.

estyxx commented 2 years ago

Maybe can you post here a little example? So we can include in the tests :)

bikeshedder commented 2 years ago

In my code it looks somewhat like that:

def create_resolver(**filter):
    def resolver():
        # XXX In the actual code some DB query or similar is issued.
        return []
    return resolver

@strawberry.type
class Foo:
    bars: List['Bar'] = strawberry.field(resolver=create_resolver(type='bar'))

@strawberry.type
class Bar:
   name: str

In #1449 I added a unit test for that. The PR is about resolvers in different modules, but the test also includes the part of a generic resolver (no type annotation).