From ac8aaf8525e5779c8508b0d812dce0161c4259de Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Wed, 30 Dec 2020 10:07:13 -0800 Subject: [PATCH] merge master into v3 --- .github/stale.yml | 13 +- MANIFEST.in | 3 + README.md | 11 +- README.rst | 4 +- django_test_settings.py | 35 --- docs/debug.rst | 4 +- docs/filtering.rst | 2 +- docs/installation.rst | 2 +- docs/mutations.rst | 121 +++++++- docs/queries.rst | 2 +- docs/testing.rst | 7 +- examples/__init__.py | 0 examples/cookbook-plain/__init__.py | 0 examples/cookbook/__init__.py | 0 examples/django_test_settings.py | 30 ++ graphene_django/__init__.py | 2 +- graphene_django/constants.py | 1 + graphene_django/converter.py | 5 + graphene_django/fields.py | 48 ++-- graphene_django/filter/fields.py | 21 +- graphene_django/filter/tests/filters.py | 2 +- graphene_django/filter/tests/test_fields.py | 106 +++++-- .../filter/tests/test_in_filter.py | 139 +++++++++ graphene_django/filter/utils.py | 68 ++++- graphene_django/forms/converter.py | 5 + graphene_django/forms/mutation.py | 13 + graphene_django/forms/tests/test_converter.py | 9 +- graphene_django/forms/tests/test_mutation.py | 188 ++++++++---- .../management/commands/graphql_schema.py | 4 +- graphene_django/rest_framework/mutation.py | 11 +- .../rest_framework/tests/test_mutation.py | 5 +- graphene_django/settings.py | 1 + graphene_django/tests/forms.py | 16 ++ graphene_django/tests/mutations.py | 18 ++ graphene_django/tests/schema_view.py | 4 + graphene_django/tests/test_converter.py | 4 + graphene_django/tests/test_fields.py | 39 ++- graphene_django/tests/test_query.py | 238 +++++++++++++++ graphene_django/tests/test_types.py | 2 +- graphene_django/tests/test_views.py | 270 ++++++++++++++++++ graphene_django/tests/types.py | 9 + graphene_django/utils/str_converters.py | 2 +- graphene_django/utils/testing.py | 12 +- graphene_django/utils/utils.py | 31 +- graphene_django/views.py | 53 +++- setup.py | 4 +- tox.ini | 3 +- 47 files changed, 1370 insertions(+), 197 deletions(-) delete mode 100644 django_test_settings.py create mode 100644 examples/__init__.py create mode 100644 examples/cookbook-plain/__init__.py create mode 100644 examples/cookbook/__init__.py create mode 100644 examples/django_test_settings.py create mode 100644 graphene_django/constants.py create mode 100644 graphene_django/filter/tests/test_in_filter.py create mode 100644 graphene_django/tests/forms.py create mode 100644 graphene_django/tests/mutations.py create mode 100644 graphene_django/tests/types.py diff --git a/.github/stale.yml b/.github/stale.yml index d066ca6..b9356c8 100644 --- a/.github/stale.yml +++ b/.github/stale.yml @@ -1,7 +1,7 @@ # Number of days of inactivity before an issue becomes stale -daysUntilStale: 120 +daysUntilStale: false # Number of days of inactivity before a stale issue is closed -daysUntilClose: 30 +daysUntilClose: false # Issues with these labels will never be considered stale exemptLabels: - pinned @@ -13,9 +13,10 @@ exemptLabels: # Label to use when marking an issue as stale staleLabel: wontfix # Comment to post when marking an issue as stale. Set to `false` to disable -markComment: > - 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. +markComment: false +# markComment: > + # 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. # Comment to post when closing a stale issue. Set to `false` to disable closeComment: false diff --git a/MANIFEST.in b/MANIFEST.in index 4677330..045af08 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,3 +1,6 @@ include README.md LICENSE recursive-include graphene_django/templates * recursive-include graphene_django/static * + +include examples/cookbook/cookbook/ingredients/fixtures/ingredients.json +include examples/cookbook-plain/cookbook/ingredients/fixtures/ingredients.json \ No newline at end of file diff --git a/README.md b/README.md index 2490209..5045e78 100644 --- a/README.md +++ b/README.md @@ -3,13 +3,13 @@ A [Django](https://www.djangoproject.com/) integration for [Graphene](http://graphene-python.org/). -[![travis][travis-image]][travis-url] +[![build][build-image]][build-url] [![pypi][pypi-image]][pypi-url] [![Anaconda-Server Badge][conda-image]][conda-url] [![coveralls][coveralls-image]][coveralls-url] -[travis-image]: https://travis-ci.org/graphql-python/graphene-django.svg?branch=master&style=flat -[travis-url]: https://travis-ci.org/graphql-python/graphene-django +[build-image]: https://github.com/graphql-python/graphene-django/workflows/Tests/badge.svg +[build-url]: https://github.com/graphql-python/graphene-django/actions [pypi-image]: https://img.shields.io/pypi/v/graphene-django.svg?style=flat [pypi-url]: https://pypi.org/project/graphene-django/ [coveralls-image]: https://coveralls.io/repos/github/graphql-python/graphene-django/badge.svg?branch=master @@ -110,6 +110,11 @@ To learn more check out the following [examples](examples/): * **Relay Schema**: [Starwars Relay example](examples/starwars) +## GraphQL testing clients + - [Firecamp](https://firecamp.io/graphql) + - [GraphiQL](https://github.com/graphql/graphiql) + + ## Contributing See [CONTRIBUTING.md](CONTRIBUTING.md) diff --git a/README.rst b/README.rst index 4ac7dda..f8aa205 100644 --- a/README.rst +++ b/README.rst @@ -114,8 +114,8 @@ Contributing See `CONTRIBUTING.md `__. .. |Graphene Logo| image:: http://graphene-python.org/favicon.png -.. |Build Status| image:: https://travis-ci.org/graphql-python/graphene-django.svg?branch=master - :target: https://travis-ci.org/graphql-python/graphene-django +.. |Build Status| image:: https://github.com/graphql-python/graphene-django/workflows/Tests/badge.svg + :target: https://github.com/graphql-python/graphene-django/actions .. |PyPI version| image:: https://badge.fury.io/py/graphene-django.svg :target: https://badge.fury.io/py/graphene-django .. |Coverage Status| image:: https://coveralls.io/repos/graphql-python/graphene-django/badge.svg?branch=master&service=github diff --git a/django_test_settings.py b/django_test_settings.py deleted file mode 100644 index 9279a73..0000000 --- a/django_test_settings.py +++ /dev/null @@ -1,35 +0,0 @@ -import sys -import os - -ROOT_PATH = os.path.dirname(os.path.abspath(__file__)) -sys.path.insert(0, ROOT_PATH + '/examples/') - -SECRET_KEY = 1 - -INSTALLED_APPS = [ - 'graphene_django', - 'graphene_django.rest_framework', - 'graphene_django.tests', - 'starwars', -] - -DATABASES = { - 'default': { - 'ENGINE': 'django.db.backends.sqlite3', - 'NAME': 'django_test.sqlite', - } -} - -TEMPLATES = [ - { - 'BACKEND': 'django.template.backends.django.DjangoTemplates', - 'DIRS': [], - 'APP_DIRS': True, - }, -] - -GRAPHENE = { - 'SCHEMA': 'graphene_django.tests.schema_view.schema' -} - -ROOT_URLCONF = 'graphene_django.tests.urls' diff --git a/docs/debug.rst b/docs/debug.rst index d1cbb21..2286519 100644 --- a/docs/debug.rst +++ b/docs/debug.rst @@ -3,7 +3,7 @@ Django Debug Middleware You can debug your GraphQL queries in a similar way to `django-debug-toolbar `__, -but outputing in the results in GraphQL response as fields, instead of +but outputting in the results in GraphQL response as fields, instead of the graphical HTML interface. For that, you will need to add the plugin in your graphene schema. @@ -43,7 +43,7 @@ And in your ``settings.py``: Querying -------- -You can query it for outputing all the sql transactions that happened in +You can query it for outputting all the sql transactions that happened in the GraphQL request, like: .. code:: diff --git a/docs/filtering.rst b/docs/filtering.rst index 9fff8eb..6a57bf9 100644 --- a/docs/filtering.rst +++ b/docs/filtering.rst @@ -230,7 +230,7 @@ Extend the tuple of fields if you want to order by more than one field. order_by = OrderingFilter( fields=( - ('created_at', 'created_at'), + ('name', 'created_at'), ) ) diff --git a/docs/installation.rst b/docs/installation.rst index cf7e798..88b7922 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -73,7 +73,7 @@ To learn how to extend the schema object for your project, read the basic tutori CSRF exempt ----------- -If have enabled `CSRF protection `_ in your Django app +If you have enabled `CSRF protection `_ in your Django app you will find that it prevents your API clients from POSTing to the ``graphql`` endpoint. You can either update your API client to pass the CSRF token with each request (the Django docs have a guide on how to do that: https://docs.djangoproject.com/en/3.0/ref/csrf/#ajax) or you can exempt your Graphql endpoint from CSRF protection by wrapping the ``GraphQLView`` with the ``csrf_exempt`` decorator: diff --git a/docs/mutations.rst b/docs/mutations.rst index 3069322..2ce8f16 100644 --- a/docs/mutations.rst +++ b/docs/mutations.rst @@ -37,7 +37,8 @@ Simple example # The class attributes define the response of the mutation question = graphene.Field(QuestionType) - def mutate(self, info, text, id): + @classmethod + def mutate(cls, root, info, text, id): question = Question.objects.get(pk=id) question.text = text question.save() @@ -231,3 +232,121 @@ This argument is also sent back to the client with the mutation result (you do not have to do anything). For services that manage a pool of many GraphQL requests in bulk, the ``clientIDMutation`` allows you to match up a specific mutation with the response. + + + +Django Database Transactions +---------------------------- + +Django gives you a few ways to control how database transactions are managed. + +Tying transactions to HTTP requests +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A common way to handle transactions in Django is to wrap each request in a transaction. +Set ``ATOMIC_REQUESTS`` settings to ``True`` in the configuration of each database for +which you want to enable this behavior. + +It works like this. Before calling ``GraphQLView`` Django starts a transaction. If the +response is produced without problems, Django commits the transaction. If the view, a +``DjangoFormMutation`` or a ``DjangoModelFormMutation`` produces an exception, Django +rolls back the transaction. + +.. warning:: + + While the simplicity of this transaction model is appealing, it also makes it + inefficient when traffic increases. Opening a transaction for every request has some + overhead. The impact on performance depends on the query patterns of your application + and on how well your database handles locking. + +Check the next section for a better solution. + +Tying transactions to mutations +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A mutation can contain multiple fields, just like a query. There's one important +distinction between queries and mutations, other than the name: + +.. + + `While query fields are executed in parallel, mutation fields run in series, one + after the other.` + +This means that if we send two ``incrementCredits`` mutations in one request, the first +is guaranteed to finish before the second begins, ensuring that we don't end up with a +race condition with ourselves. + +On the other hand, if the first ``incrementCredits`` runs successfully but the second +one does not, the operation cannot be retried as it is. That's why is a good idea to +run all mutation fields in a transaction, to guarantee all occur or nothing occurs. + +To enable this behavior for all databases set the graphene ``ATOMIC_MUTATIONS`` settings +to ``True`` in your settings file: + +.. code:: python + + GRAPHENE = { + # ... + "ATOMIC_MUTATIONS": True, + } + +On the contrary, if you want to enable this behavior for a specific database, set +``ATOMIC_MUTATIONS`` to ``True`` in your database settings: + +.. code:: python + + DATABASES = { + "default": { + # ... + "ATOMIC_MUTATIONS": True, + }, + # ... + } + +Now, given the following example mutation: + +.. code:: + + mutation IncreaseCreditsTwice { + + increaseCredits1: increaseCredits(input: { amount: 10 }) { + balance + errors { + field + messages + } + } + + increaseCredits2: increaseCredits(input: { amount: -1 }) { + balance + errors { + field + messages + } + } + + } + +The server is going to return something like: + +.. code:: json + + { + "data": { + "increaseCredits1": { + "balance": 10.0, + "errors": [] + }, + "increaseCredits2": { + "balance": null, + "errors": [ + { + "field": "amount", + "message": "Amount should be a positive number" + } + ] + }, + } + } + +But the balance will remain the same. diff --git a/docs/queries.rst b/docs/queries.rst index 78633b8..1e1ba82 100644 --- a/docs/queries.rst +++ b/docs/queries.rst @@ -291,7 +291,7 @@ Where "foo" is the name of the field declared in the ``Query`` object. class Query(graphene.ObjectType): foo = graphene.List(QuestionType) - def resolve_foo(root, info): + def resolve_foo(root, info, **kwargs): id = kwargs.get("id") return Question.objects.get(id) diff --git a/docs/testing.rst b/docs/testing.rst index 23acef2..c24f7c4 100644 --- a/docs/testing.rst +++ b/docs/testing.rst @@ -90,11 +90,12 @@ To use pytest define a simple fixture using the query helper below .. code:: python # Create a fixture using the graphql_query helper and `client` fixture from `pytest-django`. + import json import pytest from graphene_django.utils.testing import graphql_query @pytest.fixture - def client_query(client) + def client_query(client): def func(*args, **kwargs): return graphql_query(*args, **kwargs, client=client) @@ -102,7 +103,7 @@ To use pytest define a simple fixture using the query helper below # Test you query using the client_query fixture def test_some_query(client_query): - response = graphql_query( + response = client_query( ''' query { myModel { @@ -115,4 +116,4 @@ To use pytest define a simple fixture using the query helper below ) content = json.loads(response.content) - assert 'errors' not in content \ No newline at end of file + assert 'errors' not in content diff --git a/examples/__init__.py b/examples/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/examples/cookbook-plain/__init__.py b/examples/cookbook-plain/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/examples/cookbook/__init__.py b/examples/cookbook/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/examples/django_test_settings.py b/examples/django_test_settings.py new file mode 100644 index 0000000..7b98861 --- /dev/null +++ b/examples/django_test_settings.py @@ -0,0 +1,30 @@ +import sys +import os + +ROOT_PATH = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, ROOT_PATH + "/examples/") + +SECRET_KEY = 1 + +INSTALLED_APPS = [ + "graphene_django", + "graphene_django.rest_framework", + "graphene_django.tests", + "examples.starwars", +] + +DATABASES = { + "default": {"ENGINE": "django.db.backends.sqlite3", "NAME": "django_test.sqlite"} +} + +TEMPLATES = [ + { + "BACKEND": "django.template.backends.django.DjangoTemplates", + "DIRS": [], + "APP_DIRS": True, + } +] + +GRAPHENE = {"SCHEMA": "graphene_django.tests.schema_view.schema"} + +ROOT_URLCONF = "graphene_django.tests.urls" diff --git a/graphene_django/__init__.py b/graphene_django/__init__.py index 6bac245..999f3de 100644 --- a/graphene_django/__init__.py +++ b/graphene_django/__init__.py @@ -1,7 +1,7 @@ from .fields import DjangoConnectionField, DjangoListField from .types import DjangoObjectType -__version__ = "3.0.0b6" +__version__ = "3.0.0b7" __all__ = [ "__version__", diff --git a/graphene_django/constants.py b/graphene_django/constants.py new file mode 100644 index 0000000..e4d7ea9 --- /dev/null +++ b/graphene_django/constants.py @@ -0,0 +1 @@ +MUTATION_ERRORS_FLAG = "graphene_mutation_has_errors" diff --git a/graphene_django/converter.py b/graphene_django/converter.py index ddeae51..11df638 100644 --- a/graphene_django/converter.py +++ b/graphene_django/converter.py @@ -20,6 +20,7 @@ from graphene import ( NonNull, String, Time, + Decimal, ) from graphene.types.json import JSONString from graphene.utils.str_converters import to_camel_case @@ -174,6 +175,10 @@ def convert_field_to_boolean(field, registry=None): @convert_django_field.register(models.DecimalField) +def convert_field_to_decimal(field, registry=None): + return Decimal(description=field.help_text, required=not field.null) + + @convert_django_field.register(models.FloatField) @convert_django_field.register(models.DurationField) def convert_field_to_float(field, registry=None): diff --git a/graphene_django/fields.py b/graphene_django/fields.py index 75dc718..8d6e995 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -3,11 +3,13 @@ from functools import partial from django.db.models.query import QuerySet from graphql_relay.connection.arrayconnection import ( connection_from_array_slice, + cursor_to_offset, get_offset_with_default, + offset_to_cursor, ) from promise import Promise -from graphene import NonNull +from graphene import Int, NonNull from graphene.relay import ConnectionField from graphene.relay.connection import connection_adapter, page_info_adapter from graphene.types import Field, List @@ -41,16 +43,16 @@ class DjangoListField(Field): def model(self): return self._underlying_type._meta.model - def get_default_queryset(self): - return self.model._default_manager.get_queryset() + def get_manager(self): + return self.model._default_manager @staticmethod def list_resolver( - django_object_type, resolver, default_queryset, root, info, **args + django_object_type, resolver, default_manager, root, info, **args ): queryset = maybe_queryset(resolver(root, info, **args)) if queryset is None: - queryset = default_queryset + queryset = maybe_queryset(default_manager) if isinstance(queryset, QuerySet): # Pass queryset to the DjangoObjectType get_queryset method @@ -64,10 +66,7 @@ class DjangoListField(Field): _type = _type.of_type django_object_type = _type.of_type.of_type return partial( - self.list_resolver, - django_object_type, - parent_resolver, - self.get_default_queryset(), + self.list_resolver, django_object_type, parent_resolver, self.get_manager(), ) @@ -81,6 +80,7 @@ class DjangoConnectionField(ConnectionField): "enforce_first_or_last", graphene_settings.RELAY_CONNECTION_ENFORCE_FIRST_OR_LAST, ) + kwargs.setdefault("offset", Int()) super(DjangoConnectionField, self).__init__(*args, **kwargs) @property @@ -131,18 +131,24 @@ class DjangoConnectionField(ConnectionField): @classmethod def resolve_connection(cls, connection, args, iterable, max_limit=None): + # Remove the offset parameter and convert it to an after cursor. + offset = args.pop("offset", None) + after = args.get("after") + if offset: + if after: + offset += cursor_to_offset(after) + 1 + # input offset starts at 1 while the graphene offset starts at 0 + args["after"] = offset_to_cursor(offset - 1) + iterable = maybe_queryset(iterable) if isinstance(iterable, QuerySet): list_length = iterable.count() - list_slice_length = ( - min(max_limit, list_length) if max_limit is not None else list_length - ) else: list_length = len(iterable) - list_slice_length = ( - min(max_limit, list_length) if max_limit is not None else list_length - ) + list_slice_length = ( + min(max_limit, list_length) if max_limit is not None else list_length + ) # If after is higher than list_length, connection_from_list_slice # would try to do a negative slicing which makes django throw an @@ -150,7 +156,10 @@ class DjangoConnectionField(ConnectionField): after = min(get_offset_with_default(args.get("after"), -1) + 1, list_length) if max_limit is not None and args.get("first", None) == None: - args["first"] = max_limit + if args.get("last", None) != None: + after = list_length - args["last"] + else: + args["first"] = max_limit connection = connection_from_array_slice( iterable[after:], @@ -181,6 +190,8 @@ class DjangoConnectionField(ConnectionField): ): first = args.get("first") last = args.get("last") + offset = args.get("offset") + before = args.get("before") if enforce_first_or_last: assert first or last, ( @@ -200,6 +211,11 @@ class DjangoConnectionField(ConnectionField): ).format(last, info.field_name, max_limit) args["last"] = min(last, max_limit) + if offset is not None: + assert before is None, ( + "You can't provide a `before` value at the same time as an `offset` value to properly paginate the `{}` connection." + ).format(info.field_name) + # eventually leads to DjangoObjectType's get_queryset (accepts queryset) # or a resolve_foo (does not accept queryset) iterable = resolver(root, info, **args) diff --git a/graphene_django/filter/fields.py b/graphene_django/filter/fields.py index 3a98e8d..7d8d2d8 100644 --- a/graphene_django/filter/fields.py +++ b/graphene_django/filter/fields.py @@ -3,6 +3,7 @@ from functools import partial from django.core.exceptions import ValidationError from graphene.types.argument import to_arguments +from graphene.utils.str_converters import to_snake_case from ..fields import DjangoConnectionField from .utils import get_filtering_args_from_filterset, get_filterset_class @@ -21,6 +22,7 @@ class DjangoFilterConnectionField(DjangoConnectionField): self._fields = fields self._provided_filterset_class = filterset_class self._filterset_class = None + self._filtering_args = None self._extra_filter_meta = extra_filter_meta self._base_args = None super(DjangoFilterConnectionField, self).__init__(type, *args, **kwargs) @@ -50,18 +52,31 @@ class DjangoFilterConnectionField(DjangoConnectionField): @property def filtering_args(self): - return get_filtering_args_from_filterset(self.filterset_class, self.node_type) + if not self._filtering_args: + self._filtering_args = get_filtering_args_from_filterset( + self.filterset_class, self.node_type + ) + return self._filtering_args @classmethod def resolve_queryset( cls, connection, iterable, info, args, filtering_args, filterset_class ): + def filter_kwargs(): + kwargs = {} + for k, v in args.items(): + if k in filtering_args: + if k == "order_by": + v = to_snake_case(v) + kwargs[k] = v + return kwargs + qs = super(DjangoFilterConnectionField, cls).resolve_queryset( connection, iterable, info, args ) - filter_kwargs = {k: v for k, v in args.items() if k in filtering_args} + filterset = filterset_class( - data=filter_kwargs, queryset=qs, request=info.context + data=filter_kwargs(), queryset=qs, request=info.context ) if filterset.form.is_valid(): return filterset.qs diff --git a/graphene_django/filter/tests/filters.py b/graphene_django/filter/tests/filters.py index 359d2ba..43b6a87 100644 --- a/graphene_django/filter/tests/filters.py +++ b/graphene_django/filter/tests/filters.py @@ -21,7 +21,7 @@ class ReporterFilter(django_filters.FilterSet): model = Reporter fields = ["first_name", "last_name", "email", "pets"] - order_by = OrderingFilter(fields=("pub_date",)) + order_by = OrderingFilter(fields=("first_name",)) class PetFilter(django_filters.FilterSet): diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index 331527a..fa8060b 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -62,7 +62,7 @@ def get_args(field): def assert_arguments(field, *arguments): - ignore = ("after", "before", "first", "last", "order_by") + ignore = ("offset", "after", "before", "first", "last", "order_by") args = get_args(field) actual = [name for name in args if name not in ignore and not name.startswith("_")] assert set(arguments) == set( @@ -733,6 +733,73 @@ def test_should_query_filter_node_limit(): assert result.data == expected +def test_order_by(): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(ObjectType): + all_reporters = DjangoFilterConnectionField( + ReporterType, filterset_class=ReporterFilter + ) + + Reporter.objects.create(first_name="b") + Reporter.objects.create(first_name="a") + + schema = Schema(query=Query) + query = """ + query NodeFilteringQuery { + allReporters(orderBy: "-firstName") { + edges { + node { + firstName + } + } + } + } + """ + expected = { + "allReporters": { + "edges": [{"node": {"firstName": "b"}}, {"node": {"firstName": "a"}}] + } + } + + result = schema.execute(query) + assert not result.errors + assert result.data == expected + + query = """ + query NodeFilteringQuery { + allReporters(orderBy: "-first_name") { + edges { + node { + firstName + } + } + } + } + """ + + result = schema.execute(query) + assert not result.errors + assert result.data == expected + + query = """ + query NodeFilteringQuery { + allReporters(orderBy: "-firtsnaMe") { + edges { + node { + firstName + } + } + } + } + """ + result = schema.execute(query) + assert result.errors + + def test_order_by_is_perserved(): class ReporterType(DjangoObjectType): class Meta: @@ -939,7 +1006,7 @@ def test_integer_field_filter_type(): assert str(schema) == dedent( """\ type Query { - pets(before: String = null, after: String = null, first: Int = null, last: Int = null, age: Int = null): PetTypeConnection + pets(offset: Int = null, before: String = null, after: String = null, first: Int = null, last: Int = null, age: Int = null): PetTypeConnection } type PetTypeConnection { @@ -956,33 +1023,33 @@ def test_integer_field_filter_type(): type PageInfo { \"""When paginating forwards, are there more items?\""" hasNextPage: Boolean! - + \"""When paginating backwards, are there more items?\""" hasPreviousPage: Boolean! - + \"""When paginating backwards, the cursor to continue.\""" startCursor: String - + \"""When paginating forwards, the cursor to continue.\""" endCursor: String } - + \"""A Relay edge containing a `PetType` and its cursor.\""" type PetTypeEdge { \"""The item at the end of the edge\""" node: PetType - + \"""A cursor for use in pagination\""" cursor: String! } - + type PetType implements Node { age: Int! - + \"""The ID of the object\""" id: ID! } - + \"""An object with an ID\""" interface Node { \"""The ID of the object\""" @@ -1008,13 +1075,13 @@ def test_other_filter_types(): assert str(schema) == dedent( """\ type Query { - pets(before: String = null, after: String = null, first: Int = null, last: Int = null, age: Int = null, age_Isnull: Boolean = null, age_Lt: Int = null): PetTypeConnection + pets(offset: Int = null, before: String = null, after: String = null, first: Int = null, last: Int = null, age: Int = null, age_Isnull: Boolean = null, age_Lt: Int = null): PetTypeConnection } type PetTypeConnection { \"""Pagination data for this connection.\""" pageInfo: PageInfo! - + \"""Contains the nodes in this connection.\""" edges: [PetTypeEdge]! } @@ -1040,14 +1107,14 @@ def test_other_filter_types(): type PetTypeEdge { \"""The item at the end of the edge\""" node: PetType - + \"""A cursor for use in pagination\""" cursor: String! } type PetType implements Node { age: Int! - + \"""The ID of the object\""" id: ID! } @@ -1129,10 +1196,9 @@ def test_filter_filterset_based_on_mixin(): schema = Schema(query=Query) - query = ( - """ - query NodeFilteringQuery { - allArticles(viewer_Email_In: "%s") { + query = """ + query NodeFilteringQuery ($email: String!) { + allArticles(viewer_Email_In: $email) { edges { node { headline @@ -1144,8 +1210,6 @@ def test_filter_filterset_based_on_mixin(): } } """ - % reporter_1.email - ) expected = { "allArticles": { @@ -1160,7 +1224,7 @@ def test_filter_filterset_based_on_mixin(): } } - result = schema.execute(query) + result = schema.execute(query, variable_values={"email": reporter_1.email}) assert not result.errors assert result.data == expected diff --git a/graphene_django/filter/tests/test_in_filter.py b/graphene_django/filter/tests/test_in_filter.py new file mode 100644 index 0000000..3d4034e --- /dev/null +++ b/graphene_django/filter/tests/test_in_filter.py @@ -0,0 +1,139 @@ +import pytest + +from graphene import ObjectType, Schema +from graphene.relay import Node +from graphene_django import DjangoObjectType +from graphene_django.tests.models import Pet +from graphene_django.utils import DJANGO_FILTER_INSTALLED + +pytestmark = [] + +if DJANGO_FILTER_INSTALLED: + from graphene_django.filter import DjangoFilterConnectionField +else: + pytestmark.append( + pytest.mark.skipif( + True, reason="django_filters not installed or not compatible" + ) + ) + + +class PetNode(DjangoObjectType): + class Meta: + model = Pet + interfaces = (Node,) + filter_fields = { + "name": ["exact", "in"], + "age": ["exact", "in", "range"], + } + + +class Query(ObjectType): + pets = DjangoFilterConnectionField(PetNode) + + +def test_string_in_filter(): + """ + Test in filter on a string field. + """ + Pet.objects.create(name="Brutus", age=12) + Pet.objects.create(name="Mimi", age=3) + Pet.objects.create(name="Jojo, the rabbit", age=3) + + schema = Schema(query=Query) + + query = """ + query { + pets (name_In: ["Brutus", "Jojo, the rabbit"]) { + edges { + node { + name + } + } + } + } + """ + result = schema.execute(query) + assert not result.errors + assert result.data["pets"]["edges"] == [ + {"node": {"name": "Brutus"}}, + {"node": {"name": "Jojo, the rabbit"}}, + ] + + +def test_int_in_filter(): + """ + Test in filter on an integer field. + """ + Pet.objects.create(name="Brutus", age=12) + Pet.objects.create(name="Mimi", age=3) + Pet.objects.create(name="Jojo, the rabbit", age=3) + + schema = Schema(query=Query) + + query = """ + query { + pets (age_In: [3]) { + edges { + node { + name + } + } + } + } + """ + result = schema.execute(query) + assert not result.errors + assert result.data["pets"]["edges"] == [ + {"node": {"name": "Mimi"}}, + {"node": {"name": "Jojo, the rabbit"}}, + ] + + query = """ + query { + pets (age_In: [3, 12]) { + edges { + node { + name + } + } + } + } + """ + result = schema.execute(query) + assert not result.errors + assert result.data["pets"]["edges"] == [ + {"node": {"name": "Brutus"}}, + {"node": {"name": "Mimi"}}, + {"node": {"name": "Jojo, the rabbit"}}, + ] + + +def test_int_range_filter(): + """ + Test in filter on an integer field. + """ + Pet.objects.create(name="Brutus", age=12) + Pet.objects.create(name="Mimi", age=8) + Pet.objects.create(name="Jojo, the rabbit", age=3) + Pet.objects.create(name="Picotin", age=5) + + schema = Schema(query=Query) + + query = """ + query { + pets (age_Range: [4, 9]) { + edges { + node { + name + } + } + } + } + """ + result = schema.execute(query) + assert not result.errors + assert result.data["pets"]["edges"] == [ + {"node": {"name": "Mimi"}}, + {"node": {"name": "Picotin"}}, + ] diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index f234d6a..0245f16 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -1,4 +1,8 @@ +from graphene import List + from django_filters.utils import get_model_field +from django_filters.filters import Filter, BaseCSVFilter + from .filterset import custom_filterset_factory, setup_filterset @@ -15,8 +19,11 @@ def get_filtering_args_from_filterset(filterset_class, type): form_field = None if name in filterset_class.declared_filters: + # Get the filter field from the explicitly declared filter form_field = filter_field.field + field = convert_form_field(form_field) else: + # Get the filter field with no explicit type declaration model_field = get_model_field(model, filter_field.field_name) filter_type = filter_field.lookup_expr if filter_type != "isnull" and hasattr(model_field, "formfield"): @@ -24,22 +31,63 @@ def get_filtering_args_from_filterset(filterset_class, type): required=filter_field.extra.get("required", False) ) - # Fallback to field defined on filter if we can't get it from the - # model field - if not form_field: - form_field = filter_field.field + # Fallback to field defined on filter if we can't get it from the + # model field + if not form_field: + form_field = filter_field.field - field_type = convert_form_field(form_field).Argument() + field = convert_form_field(form_field) + + if filter_type in ["in", "range"]: + # Replace CSV filters (`in`, `range`) argument type to be a list of the same type as the field. + # See comments in `replace_csv_filters` method for more details. + field = List(field.get_type()) + + field_type = field.Argument() field_type.description = str(filter_field.label) if filter_field.label else None + args[name] = field_type return args def get_filterset_class(filterset_class, **meta): - """Get the class to be used as the FilterSet""" + """ + Get the class to be used as the FilterSet. + """ if filterset_class: - # If were given a FilterSet class, then set it up and - # return it - return setup_filterset(filterset_class) - return custom_filterset_factory(**meta) + # If were given a FilterSet class, then set it up. + graphene_filterset_class = setup_filterset(filterset_class) + else: + # Otherwise create one. + graphene_filterset_class = custom_filterset_factory(**meta) + + replace_csv_filters(graphene_filterset_class) + return graphene_filterset_class + + +def replace_csv_filters(filterset_class): + """ + Replace the "in" and "range" filters (that are not explicitly declared) to not be BaseCSVFilter (BaseInFilter, BaseRangeFilter) objects anymore + but regular Filter objects that simply use the input value as filter argument on the queryset. + + This is because those BaseCSVFilter are expecting a string as input with comma separated value but with GraphQl we + can actually have a list as input and have a proper type verification of each value in the list. + + See issue https://github.com/graphql-python/graphene-django/issues/1068. + """ + for name, filter_field in list(filterset_class.base_filters.items()): + filter_type = filter_field.lookup_expr + if ( + filter_type in ["in", "range"] + and name not in filterset_class.declared_filters + ): + assert isinstance(filter_field, BaseCSVFilter) + filterset_class.base_filters[name] = Filter( + field_name=filter_field.field_name, + lookup_expr=filter_field.lookup_expr, + label=filter_field.label, + method=filter_field.method, + exclude=filter_field.exclude, + **filter_field.extra + ) diff --git a/graphene_django/forms/converter.py b/graphene_django/forms/converter.py index b64e478..a75da4e 100644 --- a/graphene_django/forms/converter.py +++ b/graphene_django/forms/converter.py @@ -76,6 +76,11 @@ def convert_form_field_to_id_list(field): return List(ID, required=field.required) +@convert_form_field.register(forms.MultipleChoiceField) +def convert_form_field_to_string_list(field): + return List(String, required=field.required) + + @convert_form_field.register(forms.DateField) def convert_form_field_to_date(field): return Date(description=get_form_field_description(field), required=field.required) diff --git a/graphene_django/forms/mutation.py b/graphene_django/forms/mutation.py index 692f8d5..cc7d647 100644 --- a/graphene_django/forms/mutation.py +++ b/graphene_django/forms/mutation.py @@ -11,8 +11,13 @@ from graphene.types.mutation import MutationOptions # InputObjectType, # ) from graphene.types.utils import yank_fields_from_attrs +from graphene_django.constants import MUTATION_ERRORS_FLAG from graphene_django.registry import get_global_registry + +from django.core.exceptions import ValidationError +from django.db import connection + from ..types import ErrorType from .converter import convert_form_field @@ -46,6 +51,7 @@ class BaseDjangoFormMutation(ClientIDMutation): return cls.perform_mutate(form, info) else: errors = ErrorType.from_errors(form.errors) + _set_errors_flag_to_context(info) return cls(errors=errors, **form.data) @@ -170,6 +176,7 @@ class DjangoModelFormMutation(BaseDjangoFormMutation): return cls.perform_mutate(form, info) else: errors = ErrorType.from_errors(form.errors) + _set_errors_flag_to_context(info) return cls(errors=errors) @@ -178,3 +185,9 @@ class DjangoModelFormMutation(BaseDjangoFormMutation): obj = form.save() kwargs = {cls._meta.return_field_name: obj} return cls(errors=[], **kwargs) + + +def _set_errors_flag_to_context(info): + # This is not ideal but necessary to keep the response errors empty + if info and info.context: + setattr(info.context, MUTATION_ERRORS_FLAG, True) diff --git a/graphene_django/forms/tests/test_converter.py b/graphene_django/forms/tests/test_converter.py index 29a4419..04e8d9a 100644 --- a/graphene_django/forms/tests/test_converter.py +++ b/graphene_django/forms/tests/test_converter.py @@ -105,7 +105,14 @@ def test_should_decimal_convert_float(): assert_conversion(forms.DecimalField, Float) -def test_should_multiple_choice_convert_connectionorlist(): +def test_should_multiple_choice_convert_list(): + field = forms.MultipleChoiceField() + graphene_type = convert_form_field(field) + assert isinstance(graphene_type, List) + assert graphene_type.of_type == String + + +def test_should_model_multiple_choice_convert_connectionorlist(): field = forms.ModelMultipleChoiceField(queryset=None) graphene_type = convert_form_field(field) assert isinstance(graphene_type, List) diff --git a/graphene_django/forms/tests/test_mutation.py b/graphene_django/forms/tests/test_mutation.py index cab7d42..0770acb 100644 --- a/graphene_django/forms/tests/test_mutation.py +++ b/graphene_django/forms/tests/test_mutation.py @@ -5,21 +5,13 @@ from py.test import raises from graphene import Field, ObjectType, Schema, String from graphene_django import DjangoObjectType +from graphene_django.tests.forms import PetForm from graphene_django.tests.models import Pet +from graphene_django.tests.mutations import PetMutation from ..mutation import DjangoFormMutation, DjangoModelFormMutation -@pytest.fixture() -def pet_type(): - class PetType(DjangoObjectType): - class Meta: - model = Pet - fields = "__all__" - - return PetType - - class MyForm(forms.Form): text = forms.CharField() @@ -33,18 +25,6 @@ class MyForm(forms.Form): pass -class PetForm(forms.ModelForm): - class Meta: - model = Pet - fields = "__all__" - - def clean_age(self): - age = self.cleaned_data["age"] - if age >= 99: - raise ValidationError("Too old") - return age - - def test_needs_form_class(): with raises(Exception) as exc: @@ -70,11 +50,18 @@ def test_has_input_fields(): assert "text" in MyMutation.Input._meta.fields -def test_mutation_error_camelcased(pet_type, graphene_settings): +def test_mutation_error_camelcased(graphene_settings): class ExtraPetForm(PetForm): test_field = forms.CharField(required=True) + class PetType(DjangoObjectType): + class Meta: + model = Pet + fields = "__all__" + class PetMutation(DjangoModelFormMutation): + pet = Field(PetType) + class Meta: form_class = ExtraPetForm @@ -146,21 +133,13 @@ def test_form_valid_input(): assert result.data["myMutation"]["text"] == "VALID_INPUT" -def test_default_meta_fields(pet_type): - class PetMutation(DjangoModelFormMutation): - class Meta: - form_class = PetForm - +def test_default_meta_fields(): assert PetMutation._meta.model is Pet assert PetMutation._meta.return_field_name == "pet" assert "pet" in PetMutation._meta.fields -def test_default_input_meta_fields(pet_type): - class PetMutation(DjangoModelFormMutation): - class Meta: - form_class = PetForm - +def test_default_input_meta_fields(): assert PetMutation._meta.model is Pet assert PetMutation._meta.return_field_name == "pet" assert "name" in PetMutation.Input._meta.fields @@ -168,8 +147,15 @@ def test_default_input_meta_fields(pet_type): assert "id" in PetMutation.Input._meta.fields -def test_exclude_fields_input_meta_fields(pet_type): +def test_exclude_fields_input_meta_fields(): + class PetType(DjangoObjectType): + class Meta: + model = Pet + fields = "__all__" + class PetMutation(DjangoModelFormMutation): + pet = Field(PetType) + class Meta: form_class = PetForm exclude_fields = ["id"] @@ -182,8 +168,15 @@ def test_exclude_fields_input_meta_fields(pet_type): assert "id" not in PetMutation.Input._meta.fields -def test_custom_return_field_name(pet_type): +def test_custom_return_field_name(): + class PetType(DjangoObjectType): + class Meta: + model = Pet + fields = "__all__" + class PetMutation(DjangoModelFormMutation): + pet = Field(PetType) + class Meta: form_class = PetForm model = Pet @@ -194,13 +187,7 @@ def test_custom_return_field_name(pet_type): assert "animal" in PetMutation._meta.fields -def test_model_form_mutation_mutate_existing(pet_type): - class PetMutation(DjangoModelFormMutation): - pet = Field(pet_type) - - class Meta: - form_class = PetForm - +def test_model_form_mutation_mutate_existing(): class Mutation(ObjectType): pet_mutation = PetMutation.Field() @@ -229,13 +216,7 @@ def test_model_form_mutation_mutate_existing(pet_type): assert pet.name == "Mia" -def test_model_form_mutation_creates_new(pet_type): - class PetMutation(DjangoModelFormMutation): - pet = Field(pet_type) - - class Meta: - form_class = PetForm - +def test_model_form_mutation_creates_new(): class Mutation(ObjectType): pet_mutation = PetMutation.Field() @@ -265,13 +246,7 @@ def test_model_form_mutation_creates_new(pet_type): assert pet.age == 10 -def test_model_form_mutation_invalid_input(pet_type): - class PetMutation(DjangoModelFormMutation): - pet = Field(pet_type) - - class Meta: - form_class = PetForm - +def test_model_form_mutation_invalid_input(): class Mutation(ObjectType): pet_mutation = PetMutation.Field() @@ -301,11 +276,7 @@ def test_model_form_mutation_invalid_input(pet_type): assert Pet.objects.count() == 0 -def test_model_form_mutation_mutate_invalid_form(pet_type): - class PetMutation(DjangoModelFormMutation): - class Meta: - form_class = PetForm - +def test_model_form_mutation_mutate_invalid_form(): result = PetMutation.mutate_and_get_payload(None, None) # A pet was not created @@ -317,3 +288,98 @@ def test_model_form_mutation_mutate_invalid_form(pet_type): assert result.errors[1].messages == ["This field is required."] assert "age" in fields_w_error assert "name" in fields_w_error + + +def test_model_form_mutation_multiple_creation_valid(): + class Mutation(ObjectType): + pet_mutation = PetMutation.Field() + + schema = Schema(query=MockQuery, mutation=Mutation) + + result = schema.execute( + """ + mutation PetMutations { + petMutation1: petMutation(input: { name: "Mia", age: 10 }) { + pet { + name + age + } + errors { + field + messages + } + } + petMutation2: petMutation(input: { name: "Enzo", age: 0 }) { + pet { + name + age + } + errors { + field + messages + } + } + } + """ + ) + assert result.errors is None + assert result.data["petMutation1"]["pet"] == {"name": "Mia", "age": 10} + assert result.data["petMutation2"]["pet"] == {"name": "Enzo", "age": 0} + + assert Pet.objects.count() == 2 + + pet1 = Pet.objects.first() + assert pet1.name == "Mia" + assert pet1.age == 10 + + pet2 = Pet.objects.last() + assert pet2.name == "Enzo" + assert pet2.age == 0 + + +def test_model_form_mutation_multiple_creation_invalid(): + class Mutation(ObjectType): + pet_mutation = PetMutation.Field() + + schema = Schema(query=MockQuery, mutation=Mutation) + + result = schema.execute( + """ + mutation PetMutations { + petMutation1: petMutation(input: { name: "Mia", age: 99 }) { + pet { + name + age + } + errors { + field + messages + } + } + petMutation2: petMutation(input: { name: "Enzo", age: 0 }) { + pet { + name + age + } + errors { + field + messages + } + } + } + """ + ) + assert result.errors is None + + assert result.data["petMutation1"]["pet"] is None + assert result.data["petMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert result.data["petMutation2"]["pet"] == {"name": "Enzo", "age": 0} + + assert Pet.objects.count() == 1 + + pet = Pet.objects.get() + assert pet.name == "Enzo" + assert pet.age == 0 diff --git a/graphene_django/management/commands/graphql_schema.py b/graphene_django/management/commands/graphql_schema.py index 9cf55ca..babbf05 100644 --- a/graphene_django/management/commands/graphql_schema.py +++ b/graphene_django/management/commands/graphql_schema.py @@ -60,8 +60,10 @@ class Command(CommandArguments): def get_schema(self, schema, out, indent): schema_dict = {"data": schema.introspect()} - if out == "-": + if out == "-" or out == "-.json": self.stdout.write(json.dumps(schema_dict, indent=indent, sort_keys=True)) + elif out == "-.graphql": + self.stdout.write(print_schema(schema)) else: # Determine format _, file_extension = os.path.splitext(out) diff --git a/graphene_django/rest_framework/mutation.py b/graphene_django/rest_framework/mutation.py index 592f8b3..000b21e 100644 --- a/graphene_django/rest_framework/mutation.py +++ b/graphene_django/rest_framework/mutation.py @@ -26,6 +26,7 @@ def fields_for_serializer( exclude_fields, is_input=False, convert_choices_to_enum=True, + lookup_field=None, ): fields = OrderedDict() for name, field in serializer.fields.items(): @@ -35,7 +36,9 @@ def fields_for_serializer( name in exclude_fields, field.write_only and not is_input, # don't show write_only fields in Query - field.read_only and is_input, # don't show read_only fields in Input + field.read_only + and is_input + and lookup_field != name, # don't show read_only fields in Input ] ) @@ -66,6 +69,7 @@ class SerializerMutation(ClientIDMutation): only_fields=(), exclude_fields=(), convert_choices_to_enum=True, + _meta=None, **options ): @@ -90,6 +94,7 @@ class SerializerMutation(ClientIDMutation): exclude_fields, is_input=True, convert_choices_to_enum=convert_choices_to_enum, + lookup_field=lookup_field, ) output_fields = fields_for_serializer( serializer, @@ -97,9 +102,11 @@ class SerializerMutation(ClientIDMutation): exclude_fields, is_input=False, convert_choices_to_enum=convert_choices_to_enum, + lookup_field=lookup_field, ) - _meta = SerializerMutationOptions(cls) + if not _meta: + _meta = SerializerMutationOptions(cls) _meta.lookup_field = lookup_field _meta.model_operations = model_operations _meta.serializer_class = serializer_class diff --git a/graphene_django/rest_framework/tests/test_mutation.py b/graphene_django/rest_framework/tests/test_mutation.py index 63285b2..e0e5602 100644 --- a/graphene_django/rest_framework/tests/test_mutation.py +++ b/graphene_django/rest_framework/tests/test_mutation.py @@ -145,17 +145,20 @@ def test_write_only_field_using_extra_kwargs(): def test_read_only_fields(): class ReadOnlyFieldModelSerializer(serializers.ModelSerializer): + id = serializers.CharField(read_only=True) cool_name = serializers.CharField(read_only=True) class Meta: model = MyFakeModelWithPassword - fields = ["cool_name", "password"] + lookup_field = "id" + fields = ["id", "cool_name", "password"] class MyMutation(SerializerMutation): class Meta: serializer_class = ReadOnlyFieldModelSerializer assert "password" in MyMutation.Input._meta.fields + assert "id" in MyMutation.Input._meta.fields assert ( "cool_name" not in MyMutation.Input._meta.fields ), "'cool_name' is read_only field and shouldn't be on arguments" diff --git a/graphene_django/settings.py b/graphene_django/settings.py index 296879d..c487123 100644 --- a/graphene_django/settings.py +++ b/graphene_django/settings.py @@ -44,6 +44,7 @@ DEFAULTS = { # This sets headerEditorEnabled GraphiQL option, for details go to # https://github.com/graphql/graphiql/tree/main/packages/graphiql#options "GRAPHIQL_HEADER_EDITOR_ENABLED": True, + "ATOMIC_MUTATIONS": False, } if settings.DEBUG: diff --git a/graphene_django/tests/forms.py b/graphene_django/tests/forms.py new file mode 100644 index 0000000..95e3534 --- /dev/null +++ b/graphene_django/tests/forms.py @@ -0,0 +1,16 @@ +from django import forms +from django.core.exceptions import ValidationError + +from .models import Pet + + +class PetForm(forms.ModelForm): + class Meta: + model = Pet + fields = "__all__" + + def clean_age(self): + age = self.cleaned_data["age"] + if age >= 99: + raise ValidationError("Too old") + return age diff --git a/graphene_django/tests/mutations.py b/graphene_django/tests/mutations.py new file mode 100644 index 0000000..3aa8bfc --- /dev/null +++ b/graphene_django/tests/mutations.py @@ -0,0 +1,18 @@ +from graphene import Field + +from graphene_django.forms.mutation import DjangoFormMutation, DjangoModelFormMutation + +from .forms import PetForm +from .types import PetType + + +class PetFormMutation(DjangoFormMutation): + class Meta: + form_class = PetForm + + +class PetMutation(DjangoModelFormMutation): + pet = Field(PetType) + + class Meta: + form_class = PetForm diff --git a/graphene_django/tests/schema_view.py b/graphene_django/tests/schema_view.py index 9b3bd1e..8ed2ecf 100644 --- a/graphene_django/tests/schema_view.py +++ b/graphene_django/tests/schema_view.py @@ -1,6 +1,8 @@ import graphene from graphene import ObjectType, Schema +from .mutations import PetFormMutation, PetMutation + class QueryRoot(ObjectType): @@ -19,6 +21,8 @@ class QueryRoot(ObjectType): class MutationRoot(ObjectType): + pet_form_mutation = PetFormMutation.Field() + pet_mutation = PetMutation.Field() write_test = graphene.Field(QueryRoot) def resolve_write_test(self, info): diff --git a/graphene_django/tests/test_converter.py b/graphene_django/tests/test_converter.py index 8c61167..fd94a24 100644 --- a/graphene_django/tests/test_converter.py +++ b/graphene_django/tests/test_converter.py @@ -242,6 +242,10 @@ def test_should_float_convert_float(): assert_conversion(models.FloatField, graphene.Float) +def test_should_float_convert_decimal(): + assert_conversion(models.DecimalField, graphene.Decimal) + + def test_should_manytomany_convert_connectionorlist(): registry = Registry() dynamic_field = convert_django_field(Reporter._meta.local_many_to_many[0], registry) diff --git a/graphene_django/tests/test_fields.py b/graphene_django/tests/test_fields.py index cd5bd1b..f68470e 100644 --- a/graphene_django/tests/test_fields.py +++ b/graphene_django/tests/test_fields.py @@ -75,6 +75,39 @@ class TestDjangoListField: "reporters": [{"firstName": "Tara"}, {"firstName": "Debra"}] } + def test_list_field_queryset_is_not_cached(self): + class Reporter(DjangoObjectType): + class Meta: + model = ReporterModel + fields = ("first_name",) + + class Query(ObjectType): + reporters = DjangoListField(Reporter) + + schema = Schema(query=Query) + + query = """ + query { + reporters { + firstName + } + } + """ + + result = schema.execute(query) + assert not result.errors + assert result.data == {"reporters": []} + + ReporterModel.objects.create(first_name="Tara", last_name="West") + ReporterModel.objects.create(first_name="Debra", last_name="Payne") + + result = schema.execute(query) + + assert not result.errors + assert result.data == { + "reporters": [{"firstName": "Tara"}, {"firstName": "Debra"}] + } + def test_override_resolver(self): class Reporter(DjangoObjectType): class Meta: @@ -267,7 +300,7 @@ class TestDjangoListField: result = schema.execute(query) assert not result.errors - assert result.data == {"reporters": [{"firstName": "Tara"},]} + assert result.data == {"reporters": [{"firstName": "Tara"}]} def test_resolve_list(self): """Resolving a plain list should work (and not call get_queryset)""" @@ -314,7 +347,7 @@ class TestDjangoListField: result = schema.execute(query) assert not result.errors - assert result.data == {"reporters": [{"firstName": "Debra"},]} + assert result.data == {"reporters": [{"firstName": "Debra"}]} def test_get_queryset_foreign_key(self): class Article(DjangoObjectType): @@ -371,7 +404,7 @@ class TestDjangoListField: assert not result.errors assert result.data == { "reporters": [ - {"firstName": "Tara", "articles": [{"headline": "Amazing news"},],}, + {"firstName": "Tara", "articles": [{"headline": "Amazing news"}]}, {"firstName": "Debra", "articles": []}, ] } diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 44a2333..2ff81a3 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1241,6 +1241,103 @@ def test_should_have_next_page(graphene_settings): } +class TestBackwardPagination: + def setup_schema(self, graphene_settings, max_limit): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = max_limit + reporters = [Reporter(**kwargs) for kwargs in REPORTERS] + Reporter.objects.bulk_create(reporters) + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + return schema + + def do_queries(self, schema): + # Simply last 3 + query_last = """ + query { + allReporters(last: 3) { + edges { + node { + firstName + } + } + } + } + """ + + result = schema.execute(query_last) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 3 + assert [ + e["node"]["firstName"] for e in result.data["allReporters"]["edges"] + ] == ["First 3", "First 4", "First 5"] + + # Use a combination of first and last + query_first_and_last = """ + query { + allReporters(first: 4, last: 3) { + edges { + node { + firstName + } + } + } + } + """ + + result = schema.execute(query_first_and_last) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 3 + assert [ + e["node"]["firstName"] for e in result.data["allReporters"]["edges"] + ] == ["First 1", "First 2", "First 3"] + + # Use a combination of first and last and after + query_first_last_and_after = """ + query queryAfter($after: String) { + allReporters(first: 4, last: 3, after: $after) { + edges { + node { + firstName + } + } + } + } + """ + + after = base64.b64encode(b"arrayconnection:0").decode() + result = schema.execute( + query_first_last_and_after, variable_values=dict(after=after) + ) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 3 + assert [ + e["node"]["firstName"] for e in result.data["allReporters"]["edges"] + ] == ["First 2", "First 3", "First 4"] + + def test_should_query(self, graphene_settings): + """ + Backward pagination should work as expected + """ + schema = self.setup_schema(graphene_settings, max_limit=100) + self.do_queries(schema) + + def test_should_query_with_low_max_limit(self, graphene_settings): + """ + When doing backward pagination (using last) in combination with a max limit higher than the number of objects + we should really retrieve the last ones. + """ + schema = self.setup_schema(graphene_settings, max_limit=4) + self.do_queries(schema) + + def test_should_preserve_prefetch_related(django_assert_num_queries): class ReporterType(DjangoObjectType): class Meta: @@ -1348,3 +1445,144 @@ def test_should_preserve_annotations(): } assert result.data == expected, str(result.data) assert not result.errors + + +def test_connection_should_enable_offset_filtering(): + Reporter.objects.create(first_name="John", last_name="Doe") + Reporter.objects.create(first_name="Some", last_name="Guy") + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query { + allReporters(first: 1, offset: 1) { + edges { + node { + firstName + lastName + } + } + } + } + """ + + result = schema.execute(query) + assert not result.errors + expected = { + "allReporters": {"edges": [{"node": {"firstName": "Some", "lastName": "Guy"}},]} + } + assert result.data == expected + + +def test_connection_should_enable_offset_filtering_higher_than_max_limit( + graphene_settings, +): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = 2 + Reporter.objects.create(first_name="John", last_name="Doe") + Reporter.objects.create(first_name="Some", last_name="Guy") + Reporter.objects.create(first_name="Jane", last_name="Roe") + Reporter.objects.create(first_name="Some", last_name="Lady") + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query { + allReporters(first: 1, offset: 3) { + edges { + node { + firstName + lastName + } + } + } + } + """ + + result = schema.execute(query) + assert not result.errors + expected = { + "allReporters": { + "edges": [{"node": {"firstName": "Some", "lastName": "Lady"}},] + } + } + assert result.data == expected + + +def test_connection_should_forbid_offset_filtering_with_before(): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query ReporterPromiseConnectionQuery ($before: String) { + allReporters(first: 1, before: $before, offset: 1) { + edges { + node { + firstName + lastName + } + } + } + } + """ + before = base64.b64encode(b"arrayconnection:2").decode() + result = schema.execute(query, variable_values=dict(before=before)) + expected_error = "You can't provide a `before` value at the same time as an `offset` value to properly paginate the `allReporters` connection." + assert len(result.errors) == 1 + assert result.errors[0].message == expected_error + + +def test_connection_should_allow_offset_filtering_with_after(): + Reporter.objects.create(first_name="John", last_name="Doe") + Reporter.objects.create(first_name="Some", last_name="Guy") + Reporter.objects.create(first_name="Jane", last_name="Roe") + Reporter.objects.create(first_name="Some", last_name="Lady") + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query ReporterPromiseConnectionQuery ($after: String) { + allReporters(first: 1, after: $after, offset: 1) { + edges { + node { + firstName + lastName + } + } + } + } + """ + + after = base64.b64encode(b"arrayconnection:0").decode() + result = schema.execute(query, variable_values=dict(after=after)) + assert not result.errors + expected = { + "allReporters": {"edges": [{"node": {"firstName": "Jane", "lastName": "Roe"}},]} + } + assert result.data == expected diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py index e970588..cb653e1 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -183,7 +183,7 @@ def test_schema_representation(): pets: [Reporter!]! aChoice: TestsReporterAChoiceChoices reporterType: TestsReporterReporterTypeChoices - articles(before: String = null, after: String = null, first: Int = null, last: Int = null): ArticleConnection! + articles(offset: Int = null, before: String = null, after: String = null, first: Int = null, last: Int = null): ArticleConnection! } \"""An enumeration.\""" diff --git a/graphene_django/tests/test_views.py b/graphene_django/tests/test_views.py index 1c027d9..945fa87 100644 --- a/graphene_django/tests/test_views.py +++ b/graphene_django/tests/test_views.py @@ -2,6 +2,14 @@ import json import pytest +from mock import patch + +from django.db import connection + +from graphene_django.settings import graphene_settings + +from .models import Pet + try: from urllib import urlencode except ImportError: @@ -562,3 +570,265 @@ def test_passes_request_into_context_request(client): assert response.status_code == 200 assert response_json(response) == {"data": {"request": "testing"}} + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": True} +) +def test_form_mutation_multiple_creation_invalid_atomic_request(client): + query = """ + mutation PetMutations { + petFormMutation1: petFormMutation(input: { name: "Mia", age: 99 }) { + errors { + field + messages + } + } + petFormMutation2: petFormMutation(input: { name: "Enzo", age: 0 }) { + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petFormMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petFormMutation2"]["errors"] == [] + + assert Pet.objects.count() == 0 + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": True, "ATOMIC_REQUESTS": False} +) +def test_form_mutation_multiple_creation_invalid_atomic_mutation_1(client): + query = """ + mutation PetMutations { + petFormMutation1: petFormMutation(input: { name: "Mia", age: 99 }) { + errors { + field + messages + } + } + petFormMutation2: petFormMutation(input: { name: "Enzo", age: 0 }) { + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petFormMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petFormMutation2"]["errors"] == [] + + assert Pet.objects.count() == 0 + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", True) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": False} +) +def test_form_mutation_multiple_creation_invalid_atomic_mutation_2(client): + query = """ + mutation PetMutations { + petFormMutation1: petFormMutation(input: { name: "Mia", age: 99 }) { + errors { + field + messages + } + } + petFormMutation2: petFormMutation(input: { name: "Enzo", age: 0 }) { + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petFormMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petFormMutation2"]["errors"] == [] + + assert Pet.objects.count() == 0 + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": False} +) +def test_form_mutation_multiple_creation_invalid_non_atomic(client): + query = """ + mutation PetMutations { + petFormMutation1: petFormMutation(input: { name: "Mia", age: 99 }) { + errors { + field + messages + } + } + petFormMutation2: petFormMutation(input: { name: "Enzo", age: 0 }) { + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petFormMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petFormMutation2"]["errors"] == [] + + assert Pet.objects.count() == 1 + + pet = Pet.objects.get() + assert pet.name == "Enzo" + assert pet.age == 0 + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": True} +) +def test_model_form_mutation_multiple_creation_invalid_atomic_request(client): + query = """ + mutation PetMutations { + petMutation1: petMutation(input: { name: "Mia", age: 99 }) { + pet { + name + age + } + errors { + field + messages + } + } + petMutation2: petMutation(input: { name: "Enzo", age: 0 }) { + pet { + name + age + } + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petMutation1"]["pet"] is None + assert content["data"]["petMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petMutation2"]["pet"] == {"name": "Enzo", "age": 0} + + assert Pet.objects.count() == 0 + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": False} +) +def test_model_form_mutation_multiple_creation_invalid_non_atomic(client): + query = """ + mutation PetMutations { + petMutation1: petMutation(input: { name: "Mia", age: 99 }) { + pet { + name + age + } + errors { + field + messages + } + } + petMutation2: petMutation(input: { name: "Enzo", age: 0 }) { + pet { + name + age + } + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petMutation1"]["pet"] is None + assert content["data"]["petMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petMutation2"]["pet"] == {"name": "Enzo", "age": 0} + + assert Pet.objects.count() == 1 + + pet = Pet.objects.get() + assert pet.name == "Enzo" + assert pet.age == 0 + + +@patch("graphene_django.utils.utils.transaction.set_rollback") +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": True} +) +def test_query_errors_atomic_request(set_rollback_mock, client): + client.get(url_string(query="force error")) + set_rollback_mock.assert_called_once_with(True) + + +@patch("graphene_django.utils.utils.transaction.set_rollback") +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": False} +) +def test_query_errors_non_atomic(set_rollback_mock, client): + client.get(url_string(query="force error")) + set_rollback_mock.assert_not_called() diff --git a/graphene_django/tests/types.py b/graphene_django/tests/types.py new file mode 100644 index 0000000..56fe51d --- /dev/null +++ b/graphene_django/tests/types.py @@ -0,0 +1,9 @@ +from graphene_django.types import DjangoObjectType + +from .models import Pet + + +class PetType(DjangoObjectType): + class Meta: + model = Pet + fields = "__all__" diff --git a/graphene_django/utils/str_converters.py b/graphene_django/utils/str_converters.py index f41e87a..77a0f37 100644 --- a/graphene_django/utils/str_converters.py +++ b/graphene_django/utils/str_converters.py @@ -1,5 +1,5 @@ import re -from unidecode import unidecode +from text_unidecode import unidecode def to_const(string): diff --git a/graphene_django/utils/testing.py b/graphene_django/utils/testing.py index 97dd1c0..22efd8b 100644 --- a/graphene_django/utils/testing.py +++ b/graphene_django/utils/testing.py @@ -45,7 +45,7 @@ def graphql_query( if variables: body["variables"] = variables if input_data: - if variables in body: + if "variables" in body: body["variables"]["input"] = input_data else: body["variables"] = {"input": input_data} @@ -105,20 +105,20 @@ class GraphQLTestCase(TestCase): graphql_url=self.GRAPHQL_URL, ) - def assertResponseNoErrors(self, resp): + def assertResponseNoErrors(self, resp, msg=None): """ Assert that the call went through correctly. 200 means the syntax is ok, if there are no `errors`, the call was fine. :resp HttpResponse: Response """ - self.assertEqual(resp.status_code, 200) content = json.loads(resp.content) - self.assertNotIn("errors", list(content.keys())) + self.assertEqual(resp.status_code, 200, msg or content) + self.assertNotIn("errors", list(content.keys()), msg or content) - def assertResponseHasErrors(self, resp): + def assertResponseHasErrors(self, resp, msg=None): """ Assert that the call was failing. Take care: Even with errors, GraphQL returns status 200! :resp HttpResponse: Response """ content = json.loads(resp.content) - self.assertIn("errors", list(content.keys())) + self.assertIn("errors", list(content.keys()), msg or content) diff --git a/graphene_django/utils/utils.py b/graphene_django/utils/utils.py index c5ea85b..343a3a7 100644 --- a/graphene_django/utils/utils.py +++ b/graphene_django/utils/utils.py @@ -1,6 +1,6 @@ import inspect -from django.db import models +from django.db import connection, models, transaction from django.db.models.manager import Manager from django.utils.encoding import force_str from django.utils.functional import Promise @@ -76,3 +76,32 @@ def get_model_fields(model): def is_valid_django_model(model): return inspect.isclass(model) and issubclass(model, models.Model) + + +def import_single_dispatch(): + try: + from functools import singledispatch + except ImportError: + singledispatch = None + + if not singledispatch: + try: + from singledispatch import singledispatch + except ImportError: + pass + + if not singledispatch: + raise Exception( + "It seems your python version does not include " + "functools.singledispatch. Please install the 'singledispatch' " + "package. More information here: " + "https://pypi.python.org/pypi/singledispatch" + ) + + return singledispatch + + +def set_rollback(): + atomic_requests = connection.settings_dict.get("ATOMIC_REQUESTS", False) + if atomic_requests and connection.in_atomic_block: + transaction.set_rollback(True) diff --git a/graphene_django/views.py b/graphene_django/views.py index 1316214..3123118 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -2,6 +2,7 @@ import inspect import json import re +from django.db import connection, transaction from django.http import HttpResponse, HttpResponseNotAllowed from django.http.response import HttpResponseBadRequest from django.shortcuts import render @@ -16,6 +17,9 @@ from graphql.execution import ExecutionResult from graphene import Schema from graphql.execution.middleware import MiddlewareManager +from graphene_django.constants import MUTATION_ERRORS_FLAG +from graphene_django.utils.utils import set_rollback + from .settings import graphene_settings @@ -190,17 +194,21 @@ class GraphQLView(View): request, data, query, variables, operation_name, show_graphiql ) + if getattr(request, MUTATION_ERRORS_FLAG, False) is True: + set_rollback() + status_code = 200 if execution_result: response = {} if execution_result.errors: + set_rollback() response["errors"] = [ self.format_error(e) for e in execution_result.errors ] if execution_result.errors and any( - not e.path for e in execution_result.errors + not getattr(e, "path", None) for e in execution_result.errors ): status_code = 400 else: @@ -297,14 +305,41 @@ class GraphQLView(View): if validation_errors: return ExecutionResult(data=None, errors=validation_errors) - return self.schema.execute( - source=query, - root_value=self.get_root_value(request), - variable_values=variables, - operation_name=operation_name, - context_value=self.get_context(request), - middleware=self.get_middleware(request), - ) + try: + extra_options = {} + if getattr(self, "executor", None): + # We only include it optionally since + # executor is not a valid argument in all backends + extra_options["executor"] = self.executor + + options = { + "source": query, + "root_value": self.get_root_value(request), + "variable_values": variables, + "operation_name": operation_name, + "context_value": self.get_context(request), + "middleware": self.get_middleware(request), + } + options.update(extra_options) + + operation_ast = get_operation_ast(document, operation_name) + if ( + operation_ast + and operation_ast.operation == OperationType.MUTATION + and ( + graphene_settings.ATOMIC_MUTATIONS is True + or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True + ) + ): + with transaction.atomic(): + result = self.schema.execute(**options) + if getattr(request, MUTATION_ERRORS_FLAG, False) is True: + transaction.set_rollback(True) + return result + + return self.schema.execute(**options) + except Exception as e: + return ExecutionResult(errors=[e]) @classmethod def can_display_graphiql(cls, request, data): diff --git a/setup.py b/setup.py index 01c438b..4750671 100644 --- a/setup.py +++ b/setup.py @@ -56,13 +56,13 @@ setup( "Framework :: Django :: 3.0", ], keywords="api graphql protocol rest relay graphene", - packages=find_packages(exclude=["tests"]), + packages=find_packages(exclude=["tests", "examples", "examples.*"]), install_requires=[ "graphene>=3.0.0b5,<4", "graphql-core>=3.1.0,<4", "Django>=2.2", "promise>=2.1", - "unidecode>=1.1.1,<2", + "text-unidecode", ], setup_requires=["pytest-runner"], tests_require=tests_require, diff --git a/tox.ini b/tox.ini index 3592042..4fd5a1b 100644 --- a/tox.ini +++ b/tox.ini @@ -20,11 +20,12 @@ DJANGO = passenv = * usedevelop = True setenv = - DJANGO_SETTINGS_MODULE=django_test_settings + DJANGO_SETTINGS_MODULE=examples.django_test_settings deps = -e.[test] psycopg2-binary django111: Django>=1.11,<2.0 + django111: djangorestframework<3.12 django20: Django>=2.0,<2.1 django21: Django>=2.1,<2.2 django22: Django>=2.2,<3.0