Handle database transactions

[iorlandini]

Motivation

I was faced with an issue similar to the one described in #835 and I decided to solve both use cases.

I updated the project documentation and I also put it below to explain the PR.

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:

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:

DATABASES = {
    "default": {
        # ...
        "ATOMIC_MUTATIONS": True,
    },
    # ...
}

Now, given the following example mutation:

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:

{
    "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.

[zbyte64]

Great work, definitely accomplishes something we couldn't simply do with a transaction decorator (the grouping of mutations). This appears to be backwards compatible and I am convinced enough of the use case.

It's less clear where ATOMIC_REQUESTS gets set, perhaps show a quick example of that.

Anyone else like this feature or have feedback on how it can be improved? Please feel free to add your own review to help.

[zbyte64]

We can use mock.patch.dict to temporarily set these values and drop the finally: https://docs.python.org/3/library/unittest.mock.html#unittest.mock.patch.dict

[iorlandini]

Amazing @zbyte64, I didn't know the feature, thank you very much! I removed all try-finally in favor of mock.path.dict.

On the other hand, I always perform a self code review when I am creating a PR. Anyway, I just reviewed it again and the only part I still don't love is the one I commented on the code:

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)

So, if you have an idea to do it in a better way, I will improve it.

[zbyte64]

I took note of that as well but I couldn't purpose any better solution.

[iorlandini]

Maybe @jkimbo, @ulgens, @ace-han, or @brucedesa can add their opinion because of the discussed issue #835.

[ace-han]

I had a hard time to figure out But the balance will remain the same.

It'd better to be

Assuming prev balance is 0, after the example mutation the balance is 10.

for a much clearer description