From 0bc02180323c3202641885a787a3366d5ee5fc52 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Thu, 8 Oct 2015 23:24:21 -0700 Subject: [PATCH] Improved fields ordering. Thanks @leebyron for pointing this! Not everything yet fixed. Have to fix too in graphql-core/relay --- graphene/contrib/django/types.py | 8 ++++---- graphene/core/fields.py | 20 ++++++++++++++++++++ graphene/core/options.py | 9 +++------ graphene/core/types.py | 12 +++++------- graphene/relay/fields.py | 6 +++--- tests/contrib_django/test_types.py | 10 +++++----- 6 files changed, 40 insertions(+), 25 deletions(-) diff --git a/graphene/contrib/django/types.py b/graphene/contrib/django/types.py index 0c660217..87af3cae 100644 --- a/graphene/contrib/django/types.py +++ b/graphene/contrib/django/types.py @@ -25,10 +25,10 @@ class DjangoObjectTypeMeta(ObjectTypeMeta): if not cls._meta.model: return only_fields = cls._meta.only_fields - reverse_fields = tuple(get_reverse_fields(cls._meta.model)) - all_fields = (list(cls._meta.model._meta.fields) + - list(reverse_fields) + - list(cls._meta.model._meta.local_many_to_many)) + reverse_fields = get_reverse_fields(cls._meta.model) + all_fields = sorted(list(cls._meta.model._meta.fields) + + list(reverse_fields) + + list(cls._meta.model._meta.local_many_to_many)) for field in all_fields: is_not_in_only = only_fields and field.name not in only_fields diff --git a/graphene/core/fields.py b/graphene/core/fields.py index 69e831ff..862e2b31 100644 --- a/graphene/core/fields.py +++ b/graphene/core/fields.py @@ -1,5 +1,6 @@ import inspect import six +from functools import total_ordering from graphql.core.type import ( GraphQLField, GraphQLList, @@ -16,8 +17,10 @@ from graphene.core.types import BaseObjectType from graphene.core.scalars import GraphQLSkipField +@total_ordering class Field(object): SKIP = GraphQLSkipField + creation_counter = 0 def __init__(self, field_type, name=None, resolve=None, required=False, args=None, description='', **extra_args): self.field_type = field_type @@ -29,6 +32,8 @@ class Field(object): self.name = name self.description = description or self.__doc__ self.object_type = None + self.creation_counter = Field.creation_counter + Field.creation_counter += 1 def contribute_to_class(self, cls, name): if not self.name: @@ -122,6 +127,21 @@ class Field(object): return '<%s: %s>' % (path, name) return '<%s>' % path + def __eq__(self, other): + # Needed for @total_ordering + if isinstance(other, Field): + return self.creation_counter == other.creation_counter + return NotImplemented + + def __lt__(self, other): + # This is needed because bisect does not take a comparison function. + if isinstance(other, Field): + return self.creation_counter < other.creation_counter + return NotImplemented + + def __hash__(self): + return hash(self.creation_counter) + class NativeField(Field): diff --git a/graphene/core/options.py b/graphene/core/options.py index 6ff03b3d..1d052f04 100644 --- a/graphene/core/options.py +++ b/graphene/core/options.py @@ -1,4 +1,5 @@ from graphene.utils import cached_property +from collections import OrderedDict DEFAULT_NAMES = ('description', 'name', 'interface', 'type_name', 'interfaces', 'proxy') @@ -66,12 +67,8 @@ class Options(object): fields = [] for parent in self.parents: fields.extend(parent._meta.fields) - return self.local_fields + fields + return sorted(self.local_fields + fields) @cached_property def fields_map(self): - return {f.field_name: f for f in self.fields} - - @cached_property - def internal_fields_map(self): - return {f.name: f for f in self.fields} + return OrderedDict([(f.field_name, f) for f in self.fields]) diff --git a/graphene/core/types.py b/graphene/core/types.py index 02173706..fed216c4 100644 --- a/graphene/core/types.py +++ b/graphene/core/types.py @@ -1,5 +1,6 @@ import inspect import six +from collections import OrderedDict from graphql.core.type import ( GraphQLObjectType, @@ -49,10 +50,9 @@ class ObjectTypeMeta(type): new_class.add_extra_fields() new_fields = new_class._meta.local_fields - field_names = {f.name:f for f in new_fields} + field_names = {f.name: f for f in new_fields} for base in parents: - original_base = base if not hasattr(base, '_meta'): # Things without _meta aren't functional models, so they're # uninteresting parents. @@ -132,11 +132,9 @@ class BaseObjectType(object): @memoize @register_internal_type def internal_type(cls, schema): - fields_map = cls._meta.internal_fields_map - fields = lambda: { - name: field.internal_field(schema) - for name, field in fields_map.items() - } + fields_list = cls._meta.fields + fields = lambda: OrderedDict([(f.name, f.internal_field(schema)) + for f in fields_list]) if cls._meta.interface: return GraphQLInterfaceType( cls._meta.type_name, diff --git a/graphene/relay/fields.py b/graphene/relay/fields.py index c9eb87e0..4542d558 100644 --- a/graphene/relay/fields.py +++ b/graphene/relay/fields.py @@ -1,4 +1,4 @@ -import collections +from collections import Iterable, OrderedDict from graphql_relay.connection.arrayconnection import ( connectionFromArray @@ -30,7 +30,7 @@ class ConnectionField(Field): if resolved: resolved = self.wrap_resolved(resolved, instance, args, info) assert isinstance( - resolved, collections.Iterable), 'Resolved value from the connection field have to be iterable' + resolved, Iterable), 'Resolved value from the connection field have to be iterable' return connectionFromArray(resolved, args) @memoize @@ -71,7 +71,7 @@ class NodeTypeField(LazyField): if resolved_global_id.type == self.field_object_type._meta.type_name: return node_field.resolver(instance, args, info) - args = {a.name: a for a in node_field.args} + args = OrderedDict([(a.name, a) for a in node_field.args]) field = Field(self.field_object_type, id=args['id'], resolve=resolver) field.contribute_to_class(self.object_type, self.field_name) diff --git a/tests/contrib_django/test_types.py b/tests/contrib_django/test_types.py index 7308da22..4efabc6b 100644 --- a/tests/contrib_django/test_types.py +++ b/tests/contrib_django/test_types.py @@ -63,13 +63,13 @@ def test_interface_resolve_type(): def test_object_type(): object_type = Human.internal_type(schema) - internal_fields_map = Human._meta.internal_fields_map + fields_map = Human._meta.fields_map assert Human._meta.interface is False assert isinstance(object_type, GraphQLObjectType) assert object_type.get_fields() == { - 'headline': internal_fields_map['headline'].internal_field(schema), - 'id': internal_fields_map['id'].internal_field(schema), - 'reporter': internal_fields_map['reporter'].internal_field(schema), - 'pubDate': internal_fields_map['pubDate'].internal_field(schema), + 'headline': fields_map['headline'].internal_field(schema), + 'id': fields_map['id'].internal_field(schema), + 'reporter': fields_map['reporter'].internal_field(schema), + 'pubDate': fields_map['pub_date'].internal_field(schema), } assert object_type.get_interfaces() == [DjangoNode.internal_type(schema)]