tfoxy / graphene-django-optimizer

Optimize database access inside graphene queries
MIT License
427 stars 86 forks source link

Optimizer error 'str' object has no attribute 'name' on `get_field_def` #93

Closed lautarodapin-magoya closed 5 months ago

lautarodapin-magoya commented 1 year ago

This is the info context

GraphQLResolveInfo(field_name='childOrganizations', field_nodes=[FieldNode at 39:70], return_type=<GraphQLNonNull <GraphQLList <GraphQLNonNull <GrapheneObjectType 'OrganizationType'>>>>, parent_type=<GrapheneObjectType 'Query'>, path=Path(prev=None, key='childOrganizations', typename='Query'), schema=<graphql.type.schema.GraphQLSchema object at 0x7ff2eebc0650>, fragments={}, root_value=None, operation=OperationDefinitionNode at 0:72, variable_values={}, context=<WSGIRequest: POST '/graphql/'>, is_awaitable=<function assume_not_awaitable at 0x7ff2f18f22a0>)
Using selector: EpollSelector

And i'm getting the error when the optimizer tries to get the field def


field_def = get_field_def(info.schema, info.parent_type, info.field_name)

and it gets the error in the first linne of the function due to field_node is now a string

def get_field_def(
        schema: GraphQLSchema, parent_type: GraphQLObjectType, field_node: FieldNode
) -> GraphQLField:
        """Get field definition.

    This method looks up the field on the given type definition. It has special casing
    for the three introspection fields, ``__schema``, ``__type`, and ``__typename``.
    ``__typename`` is special because it can always be queried as a field, even in
    situations where no other fields are allowed, like on a Union. ``__schema`` and
    ``__type`` could get automatically added to the query type, but that would require
    mutating type definitions, which would cause issues.

    For internal use only.
    """
    field_name = field_node.name.value

    if field_name == "__schema" and schema.query_type == parent_type:
            return SchemaMetaFieldDef
    elif field_name == "__type" and schema.query_type == parent_type:
            return TypeMetaFieldDef
    elif field_name == "__typename":
            return TypeNameMetaFieldDef
    return parent_type.fields.get(field_name)
ipdb> info.field_name.name.value
*** AttributeError: 'str' object has no attribute 'name'
Using selector: EpollSelector
[tool.poetry]
version = "0.1.0"
description = ""
authors = []

[tool.poetry.dependencies]
python = "^3.11"
django = "^4.2.3"
graphene = "^3.2.2"
graphene-django = "^3.1.2"
graphene-file-upload = "^1.3.0"
pillow = "^10.0.0"
sentry-sdk = "^1.27.0"
rq = "^1.15.1"
django-admin-interface = "^0.26.0"
django-modeladmin-reorder = "^0.3.1"
django-anymail = "^10.0"
django-extensions = "^3.2.3"
django-rq = "^2.8.1"
django-import-export = "^3.2.0"
django-mptt = "^0.14.0"
django-solo = "^2.1.0"
django-simple-history = "^3.3.0"
django-tinymce = "^3.6.1"
psycopg2-binary = "^2.9.6"
django-model-utils = "^4.3.1"
django-redis = "^5.3.0"
hiredis = "^2.2.3"
graphene-django-optimizer = "^0.9.1"

[tool.poetry.dev-dependencies]
ipython = "^8.14.0"
ipdb = "^0.13.13"

[build-system]
requires = ["poetry>=1.5.1"]
build-backend = "poetry.masonry.api"
tfoxy commented 1 year ago

Hi @lautarodapin-magoya ! If you can create a PR with a test reproducing the error and the fix in the code (I guess it would be a ternary checking if field_node.name is a string), then I would be more than welcome to merge that PR and release a new version. Not sure why name is a string in this case, maybe because of a change in graphene versions. Let me know if you need any help with this.

lautarodapin commented 5 months ago

@tfoxy sorry for the late resopnse i got switch from the project and now i got it back

# models.py
class Account(UUIDTimeStampedModel):
  pass

class Contact(UUIDTimeStampedModel):
    account = models.ForeignKey(Account, models.CASCADE, related_name='contacts')

class Opportunity(UUIDTimeStampedModel):
    contact = models.ForeignKey(Contact, models.CASCADE, related_name='opportunities')

# schema.py 
class OpportunityType(DjangoObjectType):
    class Meta:
        model = Opportunity

class ContactType(DjangoObjectType):
    class Meta:
        model = Contact

class AccountType(DjangoObjectType):
    opportunities = graphene.List(OpportunityType, required=True)

    class Meta:
        model = Account

    def resolve_opportunities(self, info):
        qs = OpportunityModel.objects.filter(contact__in=self.contacts.all())
        return gql_optimizer.query(qs, info)

this is the minimal code this is the info field at the resolver point

GraphQLResolveInfo(field_name='opportunities', field_nodes=[FieldNode at 495:540], return_type=<GraphQLNonNull <GraphQLList <GrapheneObjectType 'OpportunityType'>>>, parent_type=<GrapheneObjectType 'AccountType'>, path=Path(prev=Path(prev=Path(prev=Path(prev=Path(prev=Path(prev=Path(prev=None, key='supplyOrders', typename='Query'), key='items', typename='SupplyOrderListType'), key=0, typename=None), key='quote', typename='SupplyOrderType'), key='contact', typename='QuoteType'), key='account', typename='ContactType'), key='opportunities', typename='AccountType'), schema=<graphql.type.schema.GraphQLSchema object at 0xffff5b8ab990>, fragments={}, root_value=None, operation=OperationDefinitionNode at 0:749, variable_values={}, context=<WSGIRequest: POST '/graphql/'>, is_awaitable=<function is_awaitable at 0xffff82620fe0>)

I guess doing this should be almost the same


@gql_optimizer.resolver_hints(prefetch_related="opportunities")
def resolve_opportunities(...): ...
martinlehoux commented 5 months ago

We are also interested in this change! (I can see we pinned GraphQL Core 19 months ago)

But I think it is already fixed by https://github.com/tfoxy/graphene-django-optimizer/commit/618f8190e804cbb6552bbbe779ce9d7b90669c20

Until there is a proper release, you can target the commit for installation I guess

lautarodapin commented 5 months ago

@martinlehoux thanks, i will check that!

lautarodapin commented 5 months ago

@martinlehoux now it works! thank you!