graphql-python / graphene

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

Resolver Not Receiving Arguments when Nested #870

Closed getglad closed 5 years ago

getglad commented 5 years ago

I have a list of classes stored in memory that I am trying to parse through various types. It is referenced through the method get_inventory().

When I call the classes individually, they resolve as I would expect.

But when I try to nest one in the other, the value is returning null.

The code, followed by some examples:

class Account(graphene.ObjectType):

    account_name = graphene.String()
    account_id = graphene.String()

    def resolve_account(
        self, info,
        account_id=None,
        account_name=None
    ):

        inventory = get_inventory()

        result = [Account(
            account_id=i.account_id,
            account_name=i.account_name
        ) for i in inventory if (
            (i.account_id == account_id) or 
            (i.account_name == account_name)
        )]

        if len(result):
            return result[0]
        else:
            return Account()

account = graphene.Field(
    Account, 
    resolver=Account.resolve_account, 
    account_name=graphene.String(default_value=None),
    account_id=graphene.String(default_value=None)
)

class Item(graphene.ObjectType):

    item_name = graphene.String()
    region = graphene.String()
    account = account

    def resolve_item(
        self, info,
        item_name=None
    ):
        inventory = get_inventory()

        result = [Item(
            item_name=i.item_name,
            region=i.region,
            account=Account(
                account_id=i.account_id
            )
        ) for i in inventory if (
            (i.item_name == item_name)
        )]

        if len(result):
            return result[0]
        else:
            return Item()

item = graphene.Field(
    Item, 
    resolver=Item.resolve_item, 
    item_name=graphene.String(default_value=None)
)

class Query(graphene.ObjectType):
    account = account
    item = item

schema = graphene.Schema(query=Query)

Let's assume I have an account foo that has an item bar. The below queries return the fields correctly.

{
  account(accountName:"foo") {
    accountName
    accountId
  }
}

{
  item(itemName: "bar") {
    itemName
    region
  }
}

So if I wanted to find the account that has the item bar, I would think I could query bar and get foo. But it returns the account fields as null.

{
  item(itemName: "bar") {
    itemName
    region
    account {
      accountId
      accountName
    }
  }
}

Recall that as part of resolve_item, I am doing account=Account(account_id=i.account_id) - I would expect this to work.

If I alter the last return statement of resolve_account to the below, accountId always returns yo.

...
else:
    return Account(
        account_id='yo'
    )

So this tells me that my resolver is firing, but the invocation in resolve_item is not passing account_id properly.

Not sure if this is a bug or user error.

getglad commented 5 years ago

From what I am seeing, it looks like rather than being passed in as an argument, account_id is being stored already as part of account in the ObjectType.

So if I add the below, I can get to the result that I want.

account_id = account_id if getattr(self, "account", None) is None else self.account.account_id

Is this probably the best approach?

If so, I would be happy to try to add this into the docs for others.

jkimbo commented 5 years ago

@getglad I think the issue is that you're assuming that you only need to define one resolver for how to get Account regardless of where it is needed in the schema. That won't work because although the resolver will always return the same type (Account) it won't always get the instance of that type in the same way.

For example if you want to be able to access an Account type by it's name at the top level of your query then you set up the resolver that you have:

class Query(graphene.ObjectType):
    account = graphene.Field(
        Account,
        account_name=graphene.String(default_value=None),
        account_id=graphene.String(default_value=None)
    )

    def resolve_account(root, info, account_name=None, account_id=None):
        # get Account by account name or account id
        inventory = get_inventory()

        result = [i for i in inventory if (
            (i.account_id == account_id) or 
            (i.account_name == account_name)
        )]
        return result[0] if len(result) else Account()

Then you can query it like this:

{
  account(accountName:"foo") {
    accountName
    accountId
  }
}

However if you are getting an account that is associated with another type (like with Item) then the resolver needs to be different because the way that it finds the right account is fundamentally different.

For example:

class Item(graphene.ObjectType):
    item_name = graphene.String()
    account = graphene.Field(Account)  # <= Note I am not setting up any extra arguments here

    def resolve_account(
        item,  # <= the first argument is object that is returned from the parent resolver
        info,
        item_name=None
    ):
        account_id = item.account_id
        inventory = get_inventory()

        result = [i for i in inventory if (
            (i.account_id == account_id)
        )]
        return result[0] if len(result) else Account()

class Query(graphene.ObjectType):
    item = graphene.Field(
        Item,
        item_name=graphene.String(default_value=None)
    )

    def resolve_item(root, info, item_name=None):
        inventory = get_inventory()

        result = [i for i in inventory if (
            (i.item_name == item_name)
        )]
        return result[0] if len(result) else Item()

Then you can query it like this:

{
  item(itemName: "bar") {
    itemName
    account {
      accountId
      accountName
    }
  }
}

I hope that makes sense!

jkimbo commented 5 years ago

@getglad did my comment solve your issue?

getglad commented 5 years ago

@jkimbo - sorry for the radio silence. Have been working on this off and on.

I'm tracking w/ your explanation, and I maybe didn't do a good job of explaining my solution, but I think we arrived at a similar solution, only I was reusing the same resolver w/ some added fault tolerance.

def resolve_account(
        self, # <= the resolved parent type
        info,
        account_id=None,
        account_name=None
    ):

        inventory = get_inventory()

        #change implemented here
        account_id = account_id if getattr(self, "account", None) is None else self.account.account_id

        result = [Account(
            account_id=i.account_id,
            account_name=i.account_name
        ) for i in inventory if (
            (i.account_id == account_id) or 
            (i.account_name == account_name)
        )]
        ...

You comment re: "the way that it finds the right account is fundamentally different" did get me thinking. My solution works because my resolvers are actually extremely similar (they just access their inputs differently), but I could see a situation where a dissimilar enough logic exists that an unique resolver attached to the parent would be justified.

I get a little itchy at the idea of resolver propagation, especially when the underlying data sources remain the same.

A place where this strategy falls down, however, is when you want something from a parent's parent. I suppose this could be solved by adding parameters to your resolver method that you don't expose as an argument in the Field.

def resolve_account(
        self, # <= the resolved parent type
        info,
        account_id=None,
        account_name=None,
        parents_parent=None
    ):
        ...

But I could see that getting messy/noisy fast, so maybe not a great idea.

Thoughts?

stale[bot] commented 5 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.