Django: Transactions and how to avoid wrong counting?

Question

I am currently struggling with a topic connected to transactions. I implemented a discount functionality. Whenever a sale is made with a discount code, the counter redeemed_quantity is increased by + 1.

Now I thought about the case. What if one or more users redeem a discount at the same time? Assuming redeemed_quantity is 10. User 1 buys the product and redeemed_quantity increases by +1 = 11. Now User 2 clicked on 'Pay' at the same time and again redeemed_quantity increases by +1 = 11. Even so, it should be 12. I learned about @transaction.atomic but I think the way I implemented them here will not help me with what I am actually trying to prevent. Can anyone help me with that?

view.py

class IndexView(TemplateView):
    template_name = 'website/index.html'
    initial_price_of_course = 100000  # TODO: Move to settings

    def check_discount_and_get_price(self):
        discount_code_get = self.request.GET.get('discount')
        discount_code = Discount.objects.filter(code=discount_code_get).first()
        if discount_code:
            discount_available = discount_code.available()
            if not discount_available:
                messages.add_message(
                    self.request,
                    messages.WARNING,
                    'Discount not available anymore.'
                )
        if discount_code and discount_available:
            return discount_code, self.initial_price_of_course - discount_code.value
        else:
            return discount_code, self.initial_price_of_course

    def get_context_data(self, **kwargs):
        context = super().get_context_data(**kwargs)
        context['stripe_pub_key'] = settings.STRIPE_PUB_KEY
        discount_object, course_price = self.check_discount_and_get_price()
        context['course_price'] = course_price
        return context

    @transaction.atomic
    def post(self, request, *args, **kwargs):
        stripe.api_key = settings.STRIPE_SECRET_KEY
        token = request.POST.get('stripeToken')
        email = request.POST.get('stripeEmail')
        discount_object, course_price = self.check_discount_and_get_price()
        charge = stripe.Charge.create(
            amount=course_price,
            currency='EUR',
            description='My Description',
            source=token,
            receipt_email=email,
        )

        if charge.paid:
            if discount_object:
                discount_object.redeemed_quantity += 1
                discount_object.save()
            order = Order(
                total_gross=course_price,
                discount=discount_object
            )
            order.save()

        return redirect('website:index')

models.py

class Discount(TimeStampedModel):
    code = models.CharField(max_length=20)
    value = models.IntegerField()  # Smallest currency unit, as amount charged
    max_quantity = models.IntegerField()
    redeemed_quantity = models.IntegerField(default=0)

    def available(self):
            available_quantity = self.max_quantity - self.redeemed_quantity
            if available_quantity > 0:
                return True


class Order(TimeStampedModel):
    total_gross = models.IntegerField()
    discount = models.ForeignKey(
        Discount,
        on_delete=models.PROTECT,  # Can't delete discount if used.
        related_name='orders',
        null=True,

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

1

You can pass the handling of the incrementation to the database in order to avoid the race condition in your code by using django's F expression:

from django.db.models import F

# ...
discount_object.redeemed_quantity = F('redeemed_quantity') + 1 
discount_object.save()

From the docs with a completely analogous example:

Although reporter.stories_filed = F('stories_filed') + 1 looks like a normal Python assignment of value to an instance attribute, in fact it’s an SQL construct describing an operation on the database.

When Django encounters an instance of F(), it overrides the standard Python operators to create an encapsulated SQL expression; in this case, one which instructs the database to increment the database field represented by reporter.stories_filed.

edited Jun 20 '20 at 09:12

Community

1
1

answered Aug 24 '18 at 14:17

user2390182

72,016
6
67
89

Oh wow, that's just perfect. Never heard of this, but exactly what I was looking for. Than you very much! Do I even need `@transaction.atomic` in that case? I read a lot about it, but still not sure if I should keep it. – Aug 24 '18 at 14:25
You don't need it for this case. I feel it is generally a decent Idea to activate the `atomic_requests` setting for your database backend, so that all db changes during a request causing a server error are rolled back. As the other answer points out, atomicity of a code block does not avoid race conditions, but ensures that a series of db queries is never just partially executed. – user2390182 Aug 24 '18 at 14:30

score 0 · Answer 2 · answered Aug 24 '18 at 14:25

Django is a piece of a synchronous code. It means that every request you make to the server is processed individually. This problem could arise, when there are multiple server-workers (for example uwsgi workers), but again - it's practically impossible to do this. We run a webshop application with multiple workers and something like this never happend.

But back to the question - if you want to query the database to increase a value by one, see schwobaseggl's answer.

The last thing is that I think you misunderstand what transaction.atomic() does. Simply put it rolls back any queries made to the database in a function if function exits with an error to the state when function was called. See this answer and this piece of documentation. Maybe it will clear some things up.

Django: Transactions and how to avoid wrong counting?

2 Answers2