🐛 Fix transaction update concurency

This commit is contained in:
Yohann D'ANELLO 2020-08-05 19:42:44 +02:00
parent b0398e59b8
commit c205219d47
4 changed files with 67 additions and 57 deletions

View File

@ -4,7 +4,7 @@
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from permission.backends import PermissionBackend from permission.backends import PermissionBackend
from rest_framework import viewsets from rest_framework import viewsets
from note_kfet.middlewares import get_current_authenticated_user from note_kfet.middlewares import get_current_session
class ReadProtectedModelViewSet(viewsets.ModelViewSet): class ReadProtectedModelViewSet(viewsets.ModelViewSet):
@ -17,7 +17,8 @@ class ReadProtectedModelViewSet(viewsets.ModelViewSet):
self.model = ContentType.objects.get_for_model(self.serializer_class.Meta.model).model_class() self.model = ContentType.objects.get_for_model(self.serializer_class.Meta.model).model_class()
def get_queryset(self): def get_queryset(self):
user = get_current_authenticated_user() user = self.request.user
get_current_session().setdefault("permission_mask", 42)
return self.model.objects.filter(PermissionBackend.filter_queryset(user, self.model, "view")).distinct() return self.model.objects.filter(PermissionBackend.filter_queryset(user, self.model, "view")).distinct()
@ -31,5 +32,6 @@ class ReadOnlyProtectedModelViewSet(viewsets.ReadOnlyModelViewSet):
self.model = ContentType.objects.get_for_model(self.serializer_class.Meta.model).model_class() self.model = ContentType.objects.get_for_model(self.serializer_class.Meta.model).model_class()
def get_queryset(self): def get_queryset(self):
user = get_current_authenticated_user() user = self.request.user
get_current_session().setdefault("permission_mask", 42)
return self.model.objects.filter(PermissionBackend.filter_queryset(user, self.model, "view")).distinct() return self.model.objects.filter(PermissionBackend.filter_queryset(user, self.model, "view")).distinct()

View File

@ -9,7 +9,7 @@ from rest_framework import viewsets
from rest_framework.response import Response from rest_framework.response import Response
from rest_framework import status from rest_framework import status
from api.viewsets import ReadProtectedModelViewSet, ReadOnlyProtectedModelViewSet from api.viewsets import ReadProtectedModelViewSet, ReadOnlyProtectedModelViewSet
from note_kfet.middlewares import get_current_authenticated_user from note_kfet.middlewares import get_current_session
from permission.backends import PermissionBackend from permission.backends import PermissionBackend
from .serializers import NotePolymorphicSerializer, AliasSerializer, ConsumerSerializer,\ from .serializers import NotePolymorphicSerializer, AliasSerializer, ConsumerSerializer,\
@ -154,5 +154,7 @@ class TransactionViewSet(ReadProtectedModelViewSet):
search_fields = ['$reason', ] search_fields = ['$reason', ]
def get_queryset(self): def get_queryset(self):
user = get_current_authenticated_user() user = self.request.user
return self.model.objects.filter(PermissionBackend.filter_queryset(user, self.model, "view")) get_current_session().setdefault("permission_mask", 42)
return self.model.objects.filter(PermissionBackend.filter_queryset(user, self.model, "view"))\
.order_by("created_at", "id")

View File

@ -1,7 +1,7 @@
# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay # Copyright (C) 2018-2020 by BDE ENS Paris-Saclay
# SPDX-License-Identifier: GPL-3.0-or-later # SPDX-License-Identifier: GPL-3.0-or-later
from django.core.exceptions import ValidationError from django.core.exceptions import ValidationError
from django.db import models from django.db import models, transaction
from django.urls import reverse from django.urls import reverse
from django.utils import timezone from django.utils import timezone
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
@ -196,38 +196,51 @@ class Transaction(PolymorphicModel):
or dest_balance > 2147483647 or dest_balance < -2147483648: or dest_balance > 2147483647 or dest_balance < -2147483648:
raise ValidationError(_("The note balances must be between - 21 474 836.47 € and 21 474 836.47 €.")) raise ValidationError(_("The note balances must be between - 21 474 836.47 € and 21 474 836.47 €."))
@transaction.atomic
def save(self, *args, **kwargs): def save(self, *args, **kwargs):
""" """
When saving, also transfer money between two notes When saving, also transfer money between two notes
""" """
self.validate(False) with transaction.atomic():
self.refresh_from_db()
self.source.refresh_from_db()
self.destination.refresh_from_db()
self.validate(False)
if not self.source.is_active or not self.destination.is_active: if not self.source.is_active or not self.destination.is_active:
if 'force_insert' not in kwargs or not kwargs['force_insert']: if 'force_insert' not in kwargs or not kwargs['force_insert']:
if 'force_update' not in kwargs or not kwargs['force_update']: if 'force_update' not in kwargs or not kwargs['force_update']:
raise ValidationError(_("The transaction can't be saved since the source note " raise ValidationError(_("The transaction can't be saved since the source note "
"or the destination note is not active.")) "or the destination note is not active."))
# If the aliases are not entered, we assume that the used alias is the name of the note # If the aliases are not entered, we assume that the used alias is the name of the note
if not self.source_alias: if not self.source_alias:
self.source_alias = str(self.source) self.source_alias = str(self.source)
if not self.destination_alias: if not self.destination_alias:
self.destination_alias = str(self.destination) self.destination_alias = str(self.destination)
if self.source.pk == self.destination.pk: if self.source.pk == self.destination.pk:
# When source == destination, no money is transferred # When source == destination, no money is transferred
super().save(*args, **kwargs)
return
self.log("Saving")
# We save first the transaction, in case of the user has no right to transfer money
super().save(*args, **kwargs) super().save(*args, **kwargs)
return self.log("Saved")
# We save first the transaction, in case of the user has no right to transfer money # Save notes
super().save(*args, **kwargs) self.source._force_save = True
self.source.save()
self.log("Source saved")
self.destination._force_save = True
self.destination.save()
self.log("Destination saved")
# Save notes def log(self, msg):
self.source._force_save = True with open("/tmp/log", "a") as f:
self.source.save() f.write(msg + "\n")
self.destination._force_save = True
self.destination.save()
def delete(self, **kwargs): def delete(self, **kwargs):
""" """

View File

@ -10,7 +10,7 @@ from time import sleep
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.core.exceptions import ValidationError from django.core.exceptions import ValidationError
from django.core.mail import mail_admins from django.core.mail import mail_admins
from django.db import models from django.db import models, transaction
from django.db.models import F, Q, Model from django.db.models import F, Q, Model
from django.forms import model_to_dict from django.forms import model_to_dict
from django.utils.translation import gettext_lazy as _ from django.utils.translation import gettext_lazy as _
@ -43,35 +43,28 @@ class InstancedPermission:
obj = copy(obj) obj = copy(obj)
obj.pk = 0 obj.pk = 0
# Ensure previous models are deleted with transaction.atomic():
for ignored in range(1000): for o in self.model.model_class().objects.filter(pk=0).all():
if self.model.model_class().objects.filter(pk=0).exists(): o._force_delete = True
# If the object exists, that means that one permission is currently checked. Model.delete(o)
# We wait before the other permission, at most 1 second. # An object with pk 0 wouldn't deleted. That's not normal, we alert admins.
sleep(0.001) msg = "Lors de la vérification d'une permission d'ajout, un objet de clé primaire nulle était "\
continue "encore présent.\n"\
break "Type de permission : " + self.type + "\n"\
for o in self.model.model_class().objects.filter(pk=0).all(): "Modèle : " + str(self.model) + "\n"\
o._force_delete = True "Objet trouvé : " + str(model_to_dict(o)) + "\n\n"\
Model.delete(o) "--\nLe BDE"
# An object with pk 0 wouldn't deleted. That's not normal, we alert admins. mail_admins("[Note Kfet] Un objet a été supprimé de force", msg)
msg = "Lors de la vérification d'une permission d'ajout, un objet de clé primaire nulle était "\
"encore présent.\n"\
"Type de permission : " + self.type + "\n"\
"Modèle : " + str(self.model) + "\n"\
"Objet trouvé : " + str(model_to_dict(o)) + "\n\n"\
"--\nLe BDE"
mail_admins("[Note Kfet] Un objet a été supprimé de force", msg)
# Force insertion, no data verification, no trigger # Force insertion, no data verification, no trigger
obj._force_save = True obj._force_save = True
Model.save(obj, force_insert=True) Model.save(obj, force_insert=True)
# We don't want log anything # We don't want log anything
obj._no_log = True obj._no_log = True
ret = self.model.model_class().objects.filter(self.query & Q(pk=0)).exists() ret = self.model.model_class().objects.filter(self.query & Q(pk=0)).exists()
# Delete testing object # Delete testing object
obj._force_delete = True obj._force_delete = True
Model.delete(obj) Model.delete(obj)
with open("/tmp/log", "w") as f: with open("/tmp/log", "w") as f:
f.write(str(obj) + ", " + str(obj.pk) + ", " + str(self.model.model_class().objects.filter(pk=0).exists())) f.write(str(obj) + ", " + str(obj.pk) + ", " + str(self.model.model_class().objects.filter(pk=0).exists()))