From c044bea50a0c4bc06b990ce1e41432543a5f8610 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Tue, 24 May 2022 23:55:24 +0200 Subject: [PATCH] agent/authentic2: prevent error if instance is deleted during current transaction (#65553) --- hobo/agent/authentic2/provisionning.py | 33 +++++++++++++------------- tests_authentic/test_provisionning.py | 5 +++- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/hobo/agent/authentic2/provisionning.py b/hobo/agent/authentic2/provisionning.py index c7c404a..b54e36d 100644 --- a/hobo/agent/authentic2/provisionning.py +++ b/hobo/agent/authentic2/provisionning.py @@ -49,7 +49,6 @@ class Provisionning(threading.local): { 'saved': {}, 'deleted': {}, - 'in_atomic_block': connection.in_atomic_block, } ) @@ -61,7 +60,6 @@ class Provisionning(threading.local): return context = self.stack.pop() - context.pop('in_atomic_block') if provision: @@ -88,34 +86,35 @@ class Provisionning(threading.local): if not self.stack: return - in_atomic_block = self.stack[-1]['in_atomic_block'] + # prevent losing pk of the model instances between call to add_saved + # and its execution at the end of the current transaction + instances = [copy.copy(instance) for instance in args] + saved = self.saved def callback(): - for instance in args: + for instance in instances: klass = User if isinstance(instance, User) else Role - self.saved.setdefault(klass, set()).add(instance) + saved.setdefault(klass, set()).add(instance) - if in_atomic_block: - callback() - else: - transaction.on_commit(callback) + transaction.on_commit(callback) def add_deleted(self, *args): if not self.stack: return - in_atomic_block = self.stack[-1]['in_atomic_block'] + # prevent losing pk of the model instances between call to add_saved + # and its execution at the end of the current transaction + instances = [copy.copy(instance) for instance in args] + deleted = self.deleted + saved = self.saved def callback(): - for instance in args: + for instance in instances: klass = User if isinstance(instance, User) else Role - self.deleted.setdefault(klass, set()).add(instance) - self.saved.get(klass, set()).discard(instance) + deleted.setdefault(klass, set()).add(instance) + saved.get(klass, set()).discard(instance) - if in_atomic_block: - callback() - else: - transaction.on_commit(callback) + transaction.on_commit(callback) def resolve_ou(self, instances, ous): for instance in instances: diff --git a/tests_authentic/test_provisionning.py b/tests_authentic/test_provisionning.py index b28525c..c79580a 100644 --- a/tests_authentic/test_provisionning.py +++ b/tests_authentic/test_provisionning.py @@ -12,6 +12,7 @@ from authentic2.models import Attribute, AttributeValue from authentic2.saml.models import LibertyProvider from django.contrib.auth import get_user_model from django.core.management import call_command +from django.db import transaction from django_rbac.utils import get_ou_model from mock import ANY, call, patch from tenant_schemas.utils import tenant_context @@ -447,7 +448,9 @@ def test_provision_user(transactional_db, tenant, caplog): protocol_conformance=lasso.PROTOCOL_SAML_2_0, ) with provisionning: - user1.delete() + with transaction.atomic(): + user1.save() + user1.delete() user2.delete() assert notify_agents.call_count == 1 arg = notify_agents.call_args -- 2.35.1