graphql-python / graphql-core

A Python 3.6+ port of the GraphQL.js reference implementation of GraphQL.
MIT License
512 stars 136 forks source link

`exclude_unset` fields flag for removing fields that are not passed from the client to the server #170

Open dima-dmytruk23 opened 2 years ago

dima-dmytruk23 commented 2 years ago

PR - https://github.com/graphql-python/graphql-core/pull/168

I use graphene and graphene-pydantic libraries. Code example. You can run this for testing. https://gist.github.com/dima-dmytruk23/aaeba0fbc7a539c1f8bf3d0914fce580

The client does not pass the name field, but it is still present in the mutation as None. Input query turns into a UserUpdateInput, which is when the default values are filled in to the dictionary. So then when code passes the dictionary in to build the UserUpdate, it sets all the fields -- so exclude_unset doesn't exclude anything, since all the fields were in fact set.

I am fairly sure it's not in graphene-pydantic, though, since that is only responsible for converting to the GrapheneInputObjectType.

I propose to resolve this issue by adding the exclude_unset flag to the GraphQLSchema class and use it in the coerce_input_value function.

Cito commented 2 years ago

As explained in the PR, I prefer examples that use Graphql-Core directly. Your example involves the two additional libs Graphene and Graphene-Pydantic which could both cause the problem - this makes it very hard and time-consuming for me to see if there is really an issue with Graphql-Core that needs to be fixed. Note again, that the goal of Graphql-Core is to be a Python port of GraphQL.js. Changes or additions to the API to support other libraries are only added in rare cases if you can argue that there is no other solution for these other libraries to solve a certain use case.

In this case, I don't think there is a problem in GraphQL-Core. It behaves exactly like you expect, by ignoring the unset values.

Here is your example code, expressed in GraphQL-Core only:

from graphql import *

current_user = {"age": 16, "name": "Morty"}

user_type = GraphQLObjectType(
    "User",
    {
        "age": GraphQLField(GraphQLInt),
        "name": GraphQLField(GraphQLString),
    },
)

user_update = GraphQLInputObjectType(
    "UserUpdate",
    {
        "age": GraphQLInputField(GraphQLInt),
        "name": GraphQLInputField(GraphQLString),
    },
)

def get_user(_obj, _info):
    return current_user

def update_user(_obj, _info, user):
    print("User input:", user)
    current_user.update(**user)
    return current_user

query = GraphQLObjectType(
    "Query", {"currentUser": GraphQLField(user_type, resolve=get_user)}
)

mutation = GraphQLObjectType(
    "Mutation",
    {
        "updateUser": GraphQLField(
            user_type, {"user": GraphQLArgument(user_update)}, resolve=update_user
        )
    },
)

schema = GraphQLSchema(query, mutation)

def print_result(q):
    print("Running", q)
    print("Result: ", graphql_sync(schema, q).data)

print_result("{currentUser {name age}}")
print_result("mutation {updateUser(user: {age: 17}) {name age}}")
print_result("{currentUser {name age}}")

When I run this with the current version 3.2.1 of GraphQL-Core, I get:

Running {currentUser {name age}}
Result:  {'currentUser': {'name': 'Morty', 'age': 16}}
Running mutation {updateUser(user: {age: 17}) {name age}}
User input: {'age': 17}
Result:  {'updateUser': {'name': 'Morty', 'age': 17}}
Running {currentUser {name age}}
Result:  {'currentUser': {'name': 'Morty', 'age': 17}}

As you see, the name is not passed to the mutation resolver.

To me it looks like your issue is caused by Graphene or Graphene-Pydantic. Note that they may also use an older version of GraphQL-Core, which may behave differently.