Handle database transactions (#1039)

* Handle Django database atomic requests

* Create and handle database atomic mutations

* Make code compatible with Python 2.7

* Code style

* Define set_rollback instead of using the one in rest_framework.views because of backward compatibility

* Implement mock.patch.dict
This commit is contained in:
Ignacio Orlandini 2020-12-23 01:18:14 -03:00 committed by GitHub
parent a51c2bffd9
commit 8f63199a63
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 613 additions and 70 deletions

View File

@ -230,3 +230,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.

View File

@ -0,0 +1 @@
MUTATION_ERRORS_FLAG = "graphene_mutation_has_errors"

View File

@ -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)

View File

@ -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

View File

@ -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:

View File

@ -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

View File

@ -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

View File

@ -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):

View File

@ -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:
@ -558,3 +566,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()

View File

@ -0,0 +1,9 @@
from graphene_django.types import DjangoObjectType
from .models import Pet
class PetType(DjangoObjectType):
class Meta:
model = Pet
fields = "__all__"

View File

@ -1,7 +1,7 @@
import inspect
import six
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_text
from django.utils.functional import Promise
@ -100,3 +100,9 @@ def import_single_dispatch():
)
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)

View File

@ -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
@ -17,6 +18,9 @@ from graphql.execution import ExecutionResult
from graphql.type.schema import GraphQLSchema
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
@ -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
]
@ -312,14 +320,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
)
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),
}
options.update(extra_options)
operation_type = document.get_operation_type(operation_name)
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)