diff --git a/docs/mutations.rst b/docs/mutations.rst index 4341f72..f080fca 100644 --- a/docs/mutations.rst +++ b/docs/mutations.rst @@ -255,3 +255,95 @@ rolls back the transaction. 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/graphene_django/settings.py b/graphene_django/settings.py index 71b791c..8a990dc 100644 --- a/graphene_django/settings.py +++ b/graphene_django/settings.py @@ -45,6 +45,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/test_views.py b/graphene_django/tests/test_views.py index 5a7df04..669f60f 100644 --- a/graphene_django/tests/test_views.py +++ b/graphene_django/tests/test_views.py @@ -6,6 +6,8 @@ from mock import patch from django.db import connection +from graphene_django.settings import graphene_settings + from .models import Pet try: @@ -567,9 +569,13 @@ def test_passes_request_into_context_request(client): def test_form_mutation_multiple_creation_invalid_atomic_request(client): + old_atomic_mutations = connection.settings_dict.get("ATOMIC_MUTATIONS", False) old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + old_graphene_atomic_mutations = graphene_settings.ATOMIC_MUTATIONS try: + connection.settings_dict["ATOMIC_MUTATIONS"] = False connection.settings_dict["ATOMIC_REQUESTS"] = True + graphene_settings.ATOMIC_MUTATIONS = False query = """ mutation PetMutations { @@ -602,13 +608,109 @@ def test_form_mutation_multiple_creation_invalid_atomic_request(client): assert Pet.objects.count() == 0 finally: + connection.settings_dict["ATOMIC_MUTATIONS"] = old_atomic_mutations connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + graphene_settings.ATOMIC_MUTATIONS = old_graphene_atomic_mutations -def test_form_mutation_multiple_creation_invalid_non_atomic_request(client): +def test_form_mutation_multiple_creation_invalid_atomic_mutation_1(client): + old_atomic_mutations = connection.settings_dict.get("ATOMIC_MUTATIONS", False) old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + old_graphene_atomic_mutations = graphene_settings.ATOMIC_MUTATIONS try: + connection.settings_dict["ATOMIC_MUTATIONS"] = True connection.settings_dict["ATOMIC_REQUESTS"] = False + graphene_settings.ATOMIC_MUTATIONS = False + + 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 + + finally: + connection.settings_dict["ATOMIC_MUTATIONS"] = old_atomic_mutations + connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + graphene_settings.ATOMIC_MUTATIONS = old_graphene_atomic_mutations + + +def test_form_mutation_multiple_creation_invalid_atomic_mutation_2(client): + old_atomic_mutations = connection.settings_dict.get("ATOMIC_MUTATIONS", False) + old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + old_graphene_atomic_mutations = graphene_settings.ATOMIC_MUTATIONS + try: + connection.settings_dict["ATOMIC_MUTATIONS"] = False + connection.settings_dict["ATOMIC_REQUESTS"] = False + graphene_settings.ATOMIC_MUTATIONS = True + + 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 + + finally: + connection.settings_dict["ATOMIC_MUTATIONS"] = old_atomic_mutations + connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + graphene_settings.ATOMIC_MUTATIONS = old_graphene_atomic_mutations + + +def test_form_mutation_multiple_creation_invalid_non_atomic(client): + old_atomic_mutations = connection.settings_dict.get("ATOMIC_MUTATIONS", False) + old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + old_graphene_atomic_mutations = graphene_settings.ATOMIC_MUTATIONS + try: + connection.settings_dict["ATOMIC_MUTATIONS"] = False + connection.settings_dict["ATOMIC_REQUESTS"] = False + graphene_settings.ATOMIC_MUTATIONS = False query = """ mutation PetMutations { @@ -645,13 +747,19 @@ def test_form_mutation_multiple_creation_invalid_non_atomic_request(client): assert pet.age == 0 finally: + connection.settings_dict["ATOMIC_MUTATIONS"] = old_atomic_mutations connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + graphene_settings.ATOMIC_MUTATIONS = old_graphene_atomic_mutations def test_model_form_mutation_multiple_creation_invalid_atomic_request(client): + old_atomic_mutations = connection.settings_dict.get("ATOMIC_MUTATIONS", False) old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + old_graphene_atomic_mutations = graphene_settings.ATOMIC_MUTATIONS try: + connection.settings_dict["ATOMIC_MUTATIONS"] = False connection.settings_dict["ATOMIC_REQUESTS"] = True + graphene_settings.ATOMIC_MUTATIONS = False query = """ mutation PetMutations { @@ -693,13 +801,19 @@ def test_model_form_mutation_multiple_creation_invalid_atomic_request(client): assert Pet.objects.count() == 0 finally: + connection.settings_dict["ATOMIC_MUTATIONS"] = old_atomic_mutations connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + graphene_settings.ATOMIC_MUTATIONS = old_graphene_atomic_mutations -def test_model_form_mutation_multiple_creation_invalid_non_atomic_request(client): +def test_model_form_mutation_multiple_creation_invalid_non_atomic(client): + old_atomic_mutations = connection.settings_dict.get("ATOMIC_MUTATIONS", False) old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + old_graphene_atomic_mutations = graphene_settings.ATOMIC_MUTATIONS try: + connection.settings_dict["ATOMIC_MUTATIONS"] = False connection.settings_dict["ATOMIC_REQUESTS"] = False + graphene_settings.ATOMIC_MUTATIONS = False query = """ mutation PetMutations { @@ -745,30 +859,44 @@ def test_model_form_mutation_multiple_creation_invalid_non_atomic_request(client assert pet.age == 0 finally: + connection.settings_dict["ATOMIC_MUTATIONS"] = old_atomic_mutations connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + graphene_settings.ATOMIC_MUTATIONS = old_graphene_atomic_mutations @patch("rest_framework.views.transaction.set_rollback") def test_query_errors_atomic_request(set_rollback_mock, client): + old_atomic_mutations = connection.settings_dict.get("ATOMIC_MUTATIONS", False) old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + old_graphene_atomic_mutations = graphene_settings.ATOMIC_MUTATIONS try: + connection.settings_dict["ATOMIC_MUTATIONS"] = False connection.settings_dict["ATOMIC_REQUESTS"] = True + graphene_settings.ATOMIC_MUTATIONS = False client.get(url_string(query="force error")) set_rollback_mock.assert_called_once_with(True) finally: + connection.settings_dict["ATOMIC_MUTATIONS"] = old_atomic_mutations connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + graphene_settings.ATOMIC_MUTATIONS = old_graphene_atomic_mutations @patch("rest_framework.views.transaction.set_rollback") -def test_query_errors_non_atomic_request(set_rollback_mock, client): +def test_query_errors_non_atomic(set_rollback_mock, client): + old_atomic_mutations = connection.settings_dict.get("ATOMIC_MUTATIONS", False) old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + old_graphene_atomic_mutations = graphene_settings.ATOMIC_MUTATIONS try: + connection.settings_dict["ATOMIC_MUTATIONS"] = False connection.settings_dict["ATOMIC_REQUESTS"] = False + graphene_settings.ATOMIC_MUTATIONS = False client.get(url_string(query="force error")) set_rollback_mock.assert_not_called() finally: + connection.settings_dict["ATOMIC_MUTATIONS"] = old_atomic_mutations connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + graphene_settings.ATOMIC_MUTATIONS = old_graphene_atomic_mutations diff --git a/graphene_django/views.py b/graphene_django/views.py index 04fe1bc..129f310 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -3,6 +3,7 @@ import json import re import six +from django.db import connection, transaction from django.http import HttpResponse, HttpResponseNotAllowed from django.http.response import HttpResponseBadRequest from django.shortcuts import render @@ -320,14 +321,27 @@ class GraphQLView(View): # executor is not a valid argument in all backends extra_options["executor"] = self.executor - return document.execute( - 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), - **extra_options - ) + operation_type = document.get_operation_type(operation_name) + options = { + "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), + **extra_options, + } + + if operation_type == "mutation" and ( + graphene_settings.ATOMIC_MUTATIONS is True + or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True + ): + with transaction.atomic(): + result = document.execute(**options) + if getattr(request, MUTATION_ERRORS_FLAG, False) is True: + transaction.set_rollback(True) + return result + + return document.execute(**options) except Exception as e: return ExecutionResult(errors=[e], invalid=True)