diff --git a/docs/mutations.rst b/docs/mutations.rst index aef32eb..4341f72 100644 --- a/docs/mutations.rst +++ b/docs/mutations.rst @@ -229,3 +229,29 @@ 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. 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/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_mutation.py b/graphene_django/forms/tests/test_mutation.py index a455a0a..ed92863 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/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_views.py b/graphene_django/tests/test_views.py index db6cc4e..5a7df04 100644 --- a/graphene_django/tests/test_views.py +++ b/graphene_django/tests/test_views.py @@ -2,6 +2,12 @@ import json import pytest +from mock import patch + +from django.db import connection + +from .models import Pet + try: from urllib import urlencode except ImportError: @@ -558,3 +564,211 @@ def test_passes_request_into_context_request(client): assert response.status_code == 200 assert response_json(response) == {"data": {"request": "testing"}} + + +def test_form_mutation_multiple_creation_invalid_atomic_request(client): + old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + try: + connection.settings_dict["ATOMIC_REQUESTS"] = 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_REQUESTS"] = old_atomic_requests + + +def test_form_mutation_multiple_creation_invalid_non_atomic_request(client): + old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + try: + connection.settings_dict["ATOMIC_REQUESTS"] = 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() == 1 + + pet = Pet.objects.get() + assert pet.name == "Enzo" + assert pet.age == 0 + + finally: + connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + + +def test_model_form_mutation_multiple_creation_invalid_atomic_request(client): + old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + try: + connection.settings_dict["ATOMIC_REQUESTS"] = True + + 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 + + finally: + connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + + +def test_model_form_mutation_multiple_creation_invalid_non_atomic_request(client): + old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + try: + connection.settings_dict["ATOMIC_REQUESTS"] = False + + 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 + + finally: + connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + + +@patch("rest_framework.views.transaction.set_rollback") +def test_query_errors_atomic_request(set_rollback_mock, client): + old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + try: + connection.settings_dict["ATOMIC_REQUESTS"] = True + + client.get(url_string(query="force error")) + set_rollback_mock.assert_called_once_with(True) + + finally: + connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests + + +@patch("rest_framework.views.transaction.set_rollback") +def test_query_errors_non_atomic_request(set_rollback_mock, client): + old_atomic_requests = connection.settings_dict["ATOMIC_REQUESTS"] + try: + connection.settings_dict["ATOMIC_REQUESTS"] = False + + client.get(url_string(query="force error")) + set_rollback_mock.assert_not_called() + + finally: + connection.settings_dict["ATOMIC_REQUESTS"] = old_atomic_requests 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/views.py b/graphene_django/views.py index 5ee0297..04fe1bc 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -17,6 +17,10 @@ from graphql.execution import ExecutionResult from graphql.type.schema import GraphQLSchema from graphql.execution.middleware import MiddlewareManager +from rest_framework.views import set_rollback + +from graphene_django.constants import MUTATION_ERRORS_FLAG + from .settings import graphene_settings @@ -203,11 +207,15 @@ 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 ]