ObjectType.resolve_type classmethod makes it impossible to have a field named 'type'

[inklesspen]

The resolve_type method on ObjectType conflicts with actually resolving a field called type. Given the importance of resolve method names, I think great care should be taken to make sure graphene's codebase does not use these method names unless necessary.

The below test fails with GraphQLError("'Location' object has no attribute 'T'",); renaming the resolve_type method to _resolve_type solves the problem.

import graphene
from graphql.core.type import GraphQLEnumValue


LOCATION_TYPES = {
    'HQ': "Headquarters",
    'BRANCH': "Branch Location",
}


LOCATIONS = {
    '1': {
        'type': 'HQ',
        'address': '123 Main St',
        'city': 'Metropolis',
    },
    '2': {
        'type': 'BRANCH',
        'address': '55 Side St',
        'city': 'Smallville',
    },
}


LocationType = graphene.Enum(
    'LocationType',
    {k: GraphQLEnumValue(value=k, description=v) for k, v in LOCATION_TYPES.items()}
)


class Location(graphene.ObjectType):
    type = graphene.NonNull(LocationType)
    address = graphene.NonNull(graphene.String())
    city = graphene.NonNull(graphene.String())


class Query(graphene.ObjectType):
    location = graphene.Field(Location, id=graphene.ID(description='location id'))

    def resolve_location(self, args, info):
        loc_id = args.get('id')
        # error checking ommitted for brevity
        return Location(**LOCATIONS[loc_id])


schema = graphene.Schema(name="Sample", query=Query)

EXPECTED_SCHEMA = """
type Location {
  type: LocationType!
  address: String!
  city: String!
}

enum LocationType {
  BRANCH
  HQ
}

type Query {
  location(id: ID): Location
}
""".lstrip()

QUERY = """
fragment locFields on Location {
  type
  address
  city
}

query SomeQuery {
  hq: location(id: 1) {
    ...locFields
  }
  branch: location(id: 2) {
    ...locFields
  }

}
""".lstrip()

EXPECTED_RESPONSE = {
    'hq': {
        'type': 'HQ',
        'address': '123 Main St',
        'city': 'Metropolis'
    },
    'branch': {
        'type': 'BRANCH',
        'address': '55 Side St',
        'city': 'Smallville'
    },
}


def test_type():
    assert str(schema) == EXPECTED_SCHEMA
    result = schema.execute(QUERY)
    assert not result.errors
    assert result.data == EXPECTED_RESPONSE

[syrusakbary]

Thanks for such a detailed example @inklesspen.
Will fix this in the next commit.

[svilendobrev]

this is still a problem on graphene.Interface - resolve_type is a classmethod there..

[jkimbo]

Ha, that is a good point @svilendobrev !

@syrusakbary should probably rename the function to _resolve_type and do it soon since it's now documented. I guess it's technically a breaking change but I don't think it's extensively used so we can probably get away with releasing it in 2.2 version

[cherls]

Using type as a variable is an anti-pattern in Python as it overwrites a built-in function, not to mention the use of type in GraphQL Schemas. The better alternative, in my opinion, would be to rename the field as location_type or type_.

[inklesspen]

Using type as a variable in Python code and using type as a field in a schema to send data between several systems, many of which may not be written in Python, are two different scenarios.

[stale]

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.