Member save method don't save in database if LDAP fail

Write test for this behavior

coin/members/models.py

@@ -8,6 +8,7 @@ import unicodedata
 import string
 import datetime
 from django.db import models
+from django.db import transaction
 from django.db.models import Q
 from django.db.models.signals import post_save, pre_save, post_delete
 from django.dispatch import receiver
@@ -84,20 +85,74 @@ class Member(models.Model):
             Q(subscription_date__lte=date),
             Q(resign_date__isnull=True) | Q(resign_date__gte=date))
 
+    def get_automatic_ldap_cn(self):
+        """
+        Calcul le login / ldap_cn automatiquement en fonction du nom et du prénom
+        """
+        # Première lettre de chaque partie du prénom
+        first_name_letters = ''.join(
+            [c[0] for c in self.first_name.split('-')]
+        )
+        # Concaténer avec nom de famille
+        ldap_cn = ('%s%s' % (first_name_letters, self.last_name))
+        # Remplacer ou enlever les caractères non ascii
+        ldap_cn = unicodedata.normalize('NFD', ldap_cn)\
+            .encode('ascii', 'ignore')
+        # Enlever ponctuation et espace
+        ldap_cn = ldap_cn.translate(None, string.punctuation + ' ')
+        # En minuscule
+        ldap_cn = ldap_cn.lower()
+
+        return ldap_cn
+
+    def sync_with_ldap(self, creation):
+        """
+        Update LDAP data when a member is saved
+        """
+        if not creation:
+            ldap_user = LdapUser.objects.get(pk=self.ldap_cn)
+
+        if creation:
+            max_uidNumber = LdapUser.objects.order_by('-uidNumber')[0].uidNumber
+
+            ldap_user = LdapUser()
+            ldap_user.pk = self.ldap_cn
+            ldap_user.uid = self.ldap_cn
+            ldap_user.nick_name = self.ldap_cn
+            ldap_user.uidNumber = max_uidNumber + 1
+
+        ldap_user.last_name = self.last_name
+        ldap_user.first_name = self.first_name
+        ldap_user.save()
+
+        if creation:
+            ldap_group = LdapGroup.objects.get(pk='coin')
+            ldap_group.members.append(ldap_user.pk)
+            ldap_group.save()
+
+    @transaction.commit_manually
     def save(self, *args, **kwargs):
-        create = (self.pk == None)
+        # Détermine si on est dans une création ou une sauvegarde
+        creation = (self.pk == None)
 
-        # If not ldap_cn defined and creation
-        if not self.ldap_cn and create:
-            self.ldap_cn = get_ldap_cn(self.first_name, self.last_name)
+        # Si pas de ldap_cn défint et création de l'objet, alors génère un
+        # ldap_cn depuis le nom et prénom
+        if not self.ldap_cn and creation:
+            self.ldap_cn = self.get_automatic_ldap_cn()
 
-        # Save in ldap and in database
+        # Sauvegarde en base de donnée (mais sans commit, cf decorator)
+        super(Member, self).save(*args, **kwargs)
+
+        # Sauvegarde dans le LDAP
+        # Si la sauvegarde LDAP échoue, Rollback la sauvegarde en base, sinon
+        # commit
         try:
-            sync_member_with_ldap_user(self, create)
-            super(Member, self).save(*args, **kwargs)
+            self.sync_with_ldap(creation)
         except:
-            pass
-
+            transaction.rollback()
+            raise
+        else:
+            transaction.commit()
 
     class Meta:
         verbose_name = 'membre'
@@ -217,53 +272,6 @@ def define_display_name(sender, instance, **kwargs):
                                            instance.last_name)
 
 
-def get_ldap_cn(first_name, last_name):
-    """
-    Calcul le login / ldap_cn automatiquement en fonction du nom et du prénom
-    """
-    # Première lettre de chaque partie du prénom
-    first_name_letters = ''.join(
-        [c[0] for c in first_name.split('-')]
-    )
-    # Concaténer avec nom de famille
-    ldap_cn = ('%s%s' % (first_name_letters, last_name))
-    # Remplacer ou enlever les caractères non ascii
-    ldap_cn = unicodedata.normalize('NFD', ldap_cn)\
-        .encode('ascii', 'ignore')
-    # Enlever ponctuation et espace
-    ldap_cn = ldap_cn.translate(None, string.punctuation + ' ')
-    # En minuscule
-    ldap_cn = ldap_cn.lower()
-
-    return ldap_cn
-
-
-def sync_member_with_ldap_user(member, created):
-    """
-    Update LDAP data when a member is saved
-    """
-    if not created:
-        ldap_user = LdapUser.objects.get(pk=member.ldap_cn)
-
-    if created:
-        max_uidNumber = LdapUser.objects.order_by('-uidNumber')[0].uidNumber
-
-        ldap_user = LdapUser()
-        ldap_user.pk = member.ldap_cn
-        ldap_user.uid = member.ldap_cn
-        ldap_user.nick_name = member.ldap_cn
-        ldap_user.uidNumber = max_uidNumber + 1
-
-    ldap_user.last_name = member.last_name
-    ldap_user.first_name = member.first_name
-    ldap_user.save()
-
-    if created:
-        ldap_group = LdapGroup.objects.get(pk='coin')
-        ldap_group.members.append(ldap_user.pk)
-        ldap_group.save()
-
-
 @receiver(post_delete, sender=Member)
 def remove_ldap_user_from_coin_group_when_deleting_member(sender,
                                                                  instance,

coin/members/tests.py

@@ -1,9 +1,12 @@
 # -*- coding: utf-8 -*-
 import os
+from django import db
 from django.test import TestCase, Client
 from django.contrib.auth.models import User
 from coin.members.models import Member, LdapUser, LdapGroup
 import logging
+import ldapdb
+from pprint import pprint
 
 
 class MemberTests(TestCase):
@@ -159,14 +162,27 @@ class MemberTests(TestCase):
     def test_when_saving_member_and_ldap_fail_dont_save(self):
         """
         Test que lors de la sauvegarde d'un membre et que la sauvegarde en LDAP
-        échoue, rien n'est sauvegardé en base
+        échoue (ici mauvais mot de passe), rien n'est sauvegardé en base
         """
-        first_name = u'Gérard'
-        last_name = u'Majax'
+        # Fait échouer le LDAP en définissant un mauvais mot de passe
+        for dbconnection in db.connections.all():
+            if (type(dbconnection) is
+                ldapdb.backends.ldap.base.DatabaseWrapper):
+                dbconnection.settings_dict['PASSWORD'] = 'wrong password test'
+
+        # Créé un membre
+        first_name = u'Du'
+        last_name = u'Pont'
         ldap_cn = MemberTestsUtils.get_random_ldap_cn()
         member = Member(first_name = first_name, last_name = last_name, ldap_cn = ldap_cn)
-        member.save()
         
+        # Le sauvegarde en base de donnée
+        # Le save devrait renvoyer une exception parceque le LDAP échoue
+        self.assertRaises(Exception, member.save)
+        
+        # On s'assure, malgré l'exception, que le membre n'est pas en base
+        with self.assertRaises(Member.DoesNotExist):
+            Member.objects.get(ldap_cn=ldap_cn)
 
 
 class MemberAdminTests(TestCase):