Alias only_fields as fields and exclude_fields as exclude (#691)

* Create new fields and exclude options that are aliased to exclude_fields and only_fields

* Update docs

* Add some checking around fields and exclude definitions

* Add all fields option

* Update docs to include `__all__` option

* Actual order of fields is not stable

* Update docs/queries.rst

Co-Authored-By: Semyon Pupkov <[email protected]>

* Fix example code

* Format code

* Start raising PendingDeprecationWarnings for using only_fields and exclude_fields

* Update tests

Author: jkimbo

docs/queries.rst

@@ -41,14 +41,18 @@ Full example
             return Question.objects.get(pk=question_id)
 
 
-Fields
-------
+Specifying which fields to include
+----------------------------------
 
 By default, ``DjangoObjectType`` will present all fields on a Model through GraphQL.
-If you don't want to do this you can change this by setting either ``only_fields`` and ``exclude_fields``.
+If you only want a subset of fields to be present, you can do so using
+``fields`` or ``exclude``. It is strongly recommended that you explicitly set
+all fields that should be exposed using the fields attribute.
+This will make it less likely to result in unintentionally exposing data when
+your models change.
 
-only_fields
-~~~~~~~~~~~
+``fields``
+~~~~~~~~~~
 
 Show **only** these fields on the model:
 
@@ -57,24 +61,35 @@ Show **only** these fields on the model:
     class QuestionType(DjangoObjectType):
         class Meta:
             model = Question
-            only_fields = ('question_text')
+            fields = ('id', 'question_text')
+
+You can also set the ``fields`` attribute to the special value ``'__all__'`` to indicate that all fields in the model should be used.
+
+For example:
+
+.. code:: python
+
+    class QuestionType(DjangoObjectType):
+        class Meta:
+            model = Question
+            fields = '__all__'
 
 
-exclude_fields
-~~~~~~~~~~~~~~
+``exclude``
+~~~~~~~~~~~
 
-Show all fields **except** those in ``exclude_fields``:
+Show all fields **except** those in ``exclude``:
 
 .. code:: python
 
     class QuestionType(DjangoObjectType):
         class Meta:
             model = Question
-            exclude_fields = ('question_text')
+            exclude = ('question_text',)
 
 
-Customised fields
-~~~~~~~~~~~~~~~~~
+Customising fields
+------------------
 
 You can completely overwrite a field, or add new fields, to a ``DjangoObjectType`` using a Resolver:
 
@@ -84,7 +99,7 @@ You can completely overwrite a field, or add new fields, to a ``DjangoObjectType
 
         class Meta:
             model = Question
-            exclude_fields = ('question_text')
+            fields = ('id', 'question_text')
 
         extra_field = graphene.String()
 
@@ -178,7 +193,7 @@ When ``Question`` is published as a ``DjangoObjectType`` and you want to add ``C
     class QuestionType(DjangoObjectType):
         class Meta:
             model = Question
-            only_fields = ('category',)
+            fields = ('category',)
 
 Then all query-able related models must be defined as DjangoObjectType subclass,
 or they will fail to show if you are trying to query those relation fields. You only

graphene_django/filter/tests/test_fields.py

@@ -774,7 +774,7 @@ class Meta:
             model = Pet
             interfaces = (Node,)
             filter_fields = {"age": ["exact"]}
-            only_fields = ["age"]
+            fields = ("age",)
 
     class Query(ObjectType):
         pets = DjangoFilterConnectionField(PetType)

graphene_django/rest_framework/serializer_converter.py

@@ -57,7 +57,9 @@ def convert_serializer_field(field, is_input=True):
 
 
 def convert_serializer_to_input_type(serializer_class):
-    cached_type = convert_serializer_to_input_type.cache.get(serializer_class.__name__, None)
+    cached_type = convert_serializer_to_input_type.cache.get(
+        serializer_class.__name__, None
+    )
     if cached_type:
         return cached_type
     serializer = serializer_class()

graphene_django/rest_framework/tests/test_multiple_model_serializers.py

@@ -18,8 +18,12 @@ class MyFakeChildModel(models.Model):
 class MyFakeParentModel(models.Model):
     name = models.CharField(max_length=50)
     created = models.DateTimeField(auto_now_add=True)
-    child1 = models.OneToOneField(MyFakeChildModel, related_name='parent1', on_delete=models.CASCADE)
-    child2 = models.OneToOneField(MyFakeChildModel, related_name='parent2', on_delete=models.CASCADE)
+    child1 = models.OneToOneField(
+        MyFakeChildModel, related_name="parent1", on_delete=models.CASCADE
+    )
+    child2 = models.OneToOneField(
+        MyFakeChildModel, related_name="parent2", on_delete=models.CASCADE
+    )
 
 
 class ParentType(DjangoObjectType):

graphene_django/tests/test_query.py

@@ -28,7 +28,7 @@ def test_should_query_only_fields():
         class ReporterType(DjangoObjectType):
             class Meta:
                 model = Reporter
-                only_fields = ("articles",)
+                fields = ("articles",)
 
         schema = graphene.Schema(query=ReporterType)
         query = """
@@ -44,7 +44,7 @@ def test_should_query_simplelazy_objects():
     class ReporterType(DjangoObjectType):
         class Meta:
             model = Reporter
-            only_fields = ("id",)
+            fields = ("id",)
 
     class Query(graphene.ObjectType):
         reporter = graphene.Field(ReporterType)
@@ -289,7 +289,7 @@ class ReporterType(DjangoObjectType):
         class Meta:
             model = Reporter
             interfaces = (Node,)
-            only_fields = ("articles",)
+            fields = ("articles",)
 
     class Query(graphene.ObjectType):
         all_reporters = DjangoConnectionField(ReporterType)
@@ -329,7 +329,7 @@ class ReporterType(DjangoObjectType):
         class Meta:
             model = Reporter
             interfaces = (Node,)
-            only_fields = ("articles",)
+            fields = ("articles",)
 
     class ArticleType(DjangoObjectType):
         class Meta:

graphene_django/tests/test_schema.py

@@ -48,6 +48,6 @@ def test_should_map_only_few_fields():
     class Reporter2(DjangoObjectType):
         class Meta:
             model = Reporter
-            only_fields = ("id", "email")
+            fields = ("id", "email")
 
     assert list(Reporter2._meta.fields.keys()) == ["id", "email"]

graphene_django/tests/test_types.py

@@ -211,26 +211,113 @@ def inner(*args, **kwargs):
 
 @with_local_registry
 def test_django_objecttype_only_fields():
+    with pytest.warns(PendingDeprecationWarning):
+
+        class Reporter(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                only_fields = ("id", "email", "films")
+
+    fields = list(Reporter._meta.fields.keys())
+    assert fields == ["id", "email", "films"]
+
+
+@with_local_registry
+def test_django_objecttype_fields():
     class Reporter(DjangoObjectType):
         class Meta:
             model = ReporterModel
-            only_fields = ("id", "email", "films")
+            fields = ("id", "email", "films")
 
     fields = list(Reporter._meta.fields.keys())
     assert fields == ["id", "email", "films"]
 
 
+@with_local_registry
+def test_django_objecttype_only_fields_and_fields():
+    with pytest.raises(Exception):
+
+        class Reporter(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                only_fields = ("id", "email", "films")
+                fields = ("id", "email", "films")
+
+
+@with_local_registry
+def test_django_objecttype_all_fields():
+    class Reporter(DjangoObjectType):
+        class Meta:
+            model = ReporterModel
+            fields = "__all__"
+
+    fields = list(Reporter._meta.fields.keys())
+    assert len(fields) == len(ReporterModel._meta.get_fields())
+
+
 @with_local_registry
 def test_django_objecttype_exclude_fields():
+    with pytest.warns(PendingDeprecationWarning):
+
+        class Reporter(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                exclude_fields = ["email"]
+
+    fields = list(Reporter._meta.fields.keys())
+    assert "email" not in fields
+
+
+@with_local_registry
+def test_django_objecttype_exclude():
     class Reporter(DjangoObjectType):
         class Meta:
             model = ReporterModel
-            exclude_fields = "email"
+            exclude = ["email"]
 
     fields = list(Reporter._meta.fields.keys())
     assert "email" not in fields
 
 
+@with_local_registry
+def test_django_objecttype_exclude_fields_and_exclude():
+    with pytest.raises(Exception):
+
+        class Reporter(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                exclude = ["email"]
+                exclude_fields = ["email"]
+
+
+@with_local_registry
+def test_django_objecttype_exclude_and_only():
+    with pytest.raises(AssertionError):
+
+        class Reporter(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                exclude = ["email"]
+                fields = ["id"]
+
+
+@with_local_registry
+def test_django_objecttype_fields_exclude_type_checking():
+    with pytest.raises(TypeError):
+
+        class Reporter(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                fields = "foo"
+
+    with pytest.raises(TypeError):
+
+        class Reporter2(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                fields = "foo"
+
+
 class TestDjangoObjectType:
     @pytest.fixture
     def PetModel(self):

graphene_django/types.py

@@ -1,3 +1,4 @@
+import warnings
 from collections import OrderedDict
 
 import six
@@ -24,6 +25,9 @@
     from typing import Type
 
 
+ALL_FIELDS = "__all__"
+
+
 def construct_fields(
     model, registry, only_fields, exclude_fields, convert_choices_to_enum
 ):
@@ -74,8 +78,10 @@ def __init_subclass_with_meta__(
         model=None,
         registry=None,
         skip_registry=False,
-        only_fields=(),
-        exclude_fields=(),
+        only_fields=(),  # deprecated in favour of `fields`
+        fields=(),
+        exclude_fields=(),  # deprecated in favour of `exclude`
+        exclude=(),
         filter_fields=None,
         filterset_class=None,
         connection=None,
@@ -109,10 +115,48 @@ def __init_subclass_with_meta__(
                 )
             )
 
+        assert not (fields and exclude), (
+            "Cannot set both 'fields' and 'exclude' options on "
+            "DjangoObjectType {class_name}.".format(class_name=cls.__name__)
+        )
+
+        # Alias only_fields -> fields
+        if only_fields and fields:
+            raise Exception("Can't set both only_fields and fields")
+        if only_fields:
+            warnings.warn(
+                "Defining `only_fields` is deprecated in favour of `fields`.",
+                PendingDeprecationWarning,
+                stacklevel=2,
+            )
+            fields = only_fields
+        if fields and fields != ALL_FIELDS and not isinstance(fields, (list, tuple)):
+            raise TypeError(
+                'The `fields` option must be a list or tuple or "__all__". '
+                "Got %s." % type(fields).__name__
+            )
+
+        if fields == ALL_FIELDS:
+            fields = None
+
+        # Alias exclude_fields -> exclude
+        if exclude_fields and exclude:
+            raise Exception("Can't set both exclude_fields and exclude")
+        if exclude_fields:
+            warnings.warn(
+                "Defining `exclude_fields` is deprecated in favour of `exclude`.",
+                PendingDeprecationWarning,
+                stacklevel=2,
+            )
+            exclude = exclude_fields
+        if exclude and not isinstance(exclude, (list, tuple)):
+            raise TypeError(
+                "The `exclude` option must be a list or tuple. Got %s."
+                % type(exclude).__name__
+            )
+
         django_fields = yank_fields_from_attrs(
-            construct_fields(
-                model, registry, only_fields, exclude_fields, convert_choices_to_enum
-            ),
+            construct_fields(model, registry, fields, exclude, convert_choices_to_enum),
             _as=Field,
         )