From 4c8b23e999e2cfd6ef2c73dee58ce92b56836841 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Aur=C3=A9lien=20Delobelle?= <aurelien.delobelle@ens.fr>
Date: Mon, 8 Aug 2016 03:32:48 +0200
Subject: [PATCH] Correction de l'usage de transaction dans K-Psul

---
 kfet/views.py | 76 +++++++++++++++++++++++++++------------------------
 1 file changed, 40 insertions(+), 36 deletions(-)

diff --git a/kfet/views.py b/kfet/views.py
index e444b042..707e2ddb 100644
--- a/kfet/views.py
+++ b/kfet/views.py
@@ -418,6 +418,7 @@ def kpsul_perform_operations(request):
     cof_grant_divisor = 1 + cof_grant / 100
     is_addcost = (addcost_for and addcost_amount
                     and addcost_for != operationgroup.on_acc)
+    addcost_total = 0
 
     # 1. Calculating amount of each PURCHASE operations
     # 1.1 Standard price for n articles
@@ -437,7 +438,7 @@ def kpsul_perform_operations(request):
                 operation.addcost_amount = addcost_amount * operation.article_nb
                 operation.amount -= operation.addcost_amount
                 # 5
-                addcost_for.balance += operation.addcost_amount
+                addcost_total += operation.addcost_amount
                 operation.addcost_for = addcost_for
             # 1.3
             if operationgroup.on_acc.is_cof:
@@ -450,47 +451,40 @@ def kpsul_perform_operations(request):
         if operation.type == Operation.DEPOSIT:
             required_perms.append('kfet.can_perform_deposit')
 
-
-    # Adding required permissions to perform operation group
-    opegroup_perms = operationgroup.on_acc.perms_to_perform_operation(
-        amount = operationgroup.amount)
-    required_perms += opegroup_perms
-
-    # Checking authenticated user has all perms
-    if not request.user.has_perms(required_perms):
-        # Sending BAD_REQUEST with missing perms
-        missing_perms = \
-            [ Permission.objects.get(codename=codename).name for codename in (
-                (perm.split('.'))[1] for perm in
-                required_perms if not request.user.has_perm(perm)
-            )]
-        data['errors'].append({'missing_perms': missing_perms })
-        return JsonResponse(data, status=403)
-
-    # If 1 perm is required, saving who perform the operations
-    if len(required_perms) > 0:
-        operationgroup.valid_by = request.user.profile.account_kfet
-
-    # Filling cof status for statistics
-    operationgroup.is_cof = operationgroup.on_acc.is_cof
-
-    # Updating (no commit) account's balance
-    operationgroup.on_acc.balance += operationgroup.amount
-
-    # Saving in a transaction to ensure database integrity
+    # Starting transaction to ensure data consistency
+    # Using select_for_update where it is critical
     try:
         with transaction.atomic():
+            on_acc =  operationgroup.on_acc
+            on_acc = Account.objects.select_for_update().get(pk=on_acc.pk)
+            # Adding required permissions to perform operation group
+            opegroup_perms = on_acc.perms_to_perform_operation(
+                amount = operationgroup.amount)
+            required_perms += opegroup_perms
+
+            # Checking authenticated user has all perms
+            if not request.user.has_perms(required_perms):
+                raise PermissionDenied
+
+            # If 1 perm is required, saving who perform the operations
+            if len(required_perms) > 0:
+                operationgroup.valid_by = request.user.profile.account_kfet
+
+            # Filling cof status for statistics
+            operationgroup.is_cof = on_acc.is_cof
+
+            # Saving account's balance
+            on_acc.balance += operationgroup.amount
+            on_acc.save()
+            # Saving addcost_for with new balance if there is one
+            if is_addcost:
+                addcost_for += addcost_total
+                addcost_for.save()
+
             # Saving operation group
             operationgroup.save()
             data['operationgroup'] = operationgroup.pk
 
-            # Saving account with new balance
-            operationgroup.on_acc.save()
-
-            # Saving addcost_for with new balance if there is one
-            if is_addcost:
-                addcost_for.save()
-
             # Filling operationgroup id for each operations and saving
             # Saving articles with new stock
             for operation in operations:
@@ -499,7 +493,17 @@ def kpsul_perform_operations(request):
                 if operation.type == Operation.PURCHASE:
                     operation.article.save()
                 data['operations'].append(operation.pk)
+    except PermissionDenied:
+        # Sending BAD_REQUEST with missing perms
+        missing_perms = \
+            [ Permission.objects.get(codename=codename).name for codename in (
+                (perm.split('.'))[1] for perm in
+                required_perms if not request.user.has_perm(perm)
+            )]
+        data['errors'].append({'missing_perms': missing_perms })
+        return JsonResponse(data, status=403)
     except IntegrityError:
         data['errors'].append('DB error')
+        return JsonResponse(data, status=500)
 
     return JsonResponse(data)