From 80eaa0c8711f2c3ca82eb7953db49c7c61bd9ffa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Barrois?= <raphael.barrois@polytechnique.org>
Date: Sun, 18 May 2014 12:16:51 +0200
Subject: factory.django: Fix counter inheritance with abstract models.

---
 factory/base.py       | 19 ++++++++++++-------
 factory/django.py     | 16 ++++++++++++----
 tests/djapp/models.py | 11 ++++++++++-
 tests/test_django.py  | 17 +++++++++++++++--
 4 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/factory/base.py b/factory/base.py
index cc1fb57..d255dbf 100644
--- a/factory/base.py
+++ b/factory/base.py
@@ -218,13 +218,7 @@ class FactoryOptions(object):
         if self.target is None:
             self.abstract = True
 
-        if (self.target is not None
-                and self.base_factory is not None
-                and self.base_factory._meta.target is not None
-                and issubclass(self.target, base_factory._meta.target)):
-            self.counter_reference = self.base_factory
-        else:
-            self.counter_reference = self.factory
+        self.counter_reference = self._get_counter_reference()
 
         for parent in self.factory.__mro__[1:]:
             if not hasattr(parent, '_meta'):
@@ -238,6 +232,17 @@ class FactoryOptions(object):
             if self._is_postgen_declaration(k, v):
                 self.postgen_declarations[k] = v
 
+    def _get_counter_reference(self):
+        """Identify which factory should be used for a shared counter."""
+
+        if (self.target is not None
+                and self.base_factory is not None
+                and self.base_factory._meta.target is not None
+                and issubclass(self.target, self.base_factory._meta.target)):
+            return self.base_factory
+        else:
+            return self.factory
+
     def _is_declaration(self, name, value):
         """Determines if a class attribute is a field value declaration.
 
diff --git a/factory/django.py b/factory/django.py
index 9a4e07a..77afd8c 100644
--- a/factory/django.py
+++ b/factory/django.py
@@ -58,6 +58,18 @@ class DjangoOptions(base.FactoryOptions):
             base.OptionDefault('django_get_or_create', (), inherit=True),
         ]
 
+    def _get_counter_reference(self):
+        counter_reference = super(DjangoOptions, self)._get_counter_reference()
+        if (counter_reference == self.base_factory
+                and self.base_factory._meta.target is not None
+                and self.base_factory._meta.target._meta.abstract
+                and self.target is not None
+                and not self.target._meta.abstract):
+            # Target factory is for an abstract model, yet we're for another,
+            # concrete subclass => don't reuse the counter.
+            return self.factory
+        return counter_reference
+
 
 class DjangoModelFactory(base.Factory):
     """Factory for Django models.
@@ -72,10 +84,6 @@ class DjangoModelFactory(base.Factory):
     class Meta:
         abstract = True  # Optional, but explicit.
 
-    @classmethod
-    def _get_blank_options(cls):
-        return DjangoOptions()
-
     _OLDSTYLE_ATTRIBUTES = base.Factory._OLDSTYLE_ATTRIBUTES.copy()
     _OLDSTYLE_ATTRIBUTES.update({
         'FACTORY_DJANGO_GET_OR_CREATE': 'django_get_or_create',
diff --git a/tests/djapp/models.py b/tests/djapp/models.py
index a65b50a..9b21181 100644
--- a/tests/djapp/models.py
+++ b/tests/djapp/models.py
@@ -55,6 +55,15 @@ class ConcreteSon(AbstractBase):
     pass
 
 
+class AbstractSon(AbstractBase):
+    class Meta:
+        abstract = True
+
+
+class ConcreteGrandSon(AbstractSon):
+    pass
+
+
 class StandardSon(StandardModel):
     pass
 
@@ -77,4 +86,4 @@ else:
 
 
 class WithSignals(models.Model):
-    foo = models.CharField(max_length=20)
\ No newline at end of file
+    foo = models.CharField(max_length=20)
diff --git a/tests/test_django.py b/tests/test_django.py
index 29453e6..37bf7a5 100644
--- a/tests/test_django.py
+++ b/tests/test_django.py
@@ -130,6 +130,14 @@ class ConcreteSonFactory(AbstractBaseFactory):
     FACTORY_FOR = models.ConcreteSon
 
 
+class AbstractSonFactory(AbstractBaseFactory):
+    FACTORY_FOR = models.AbstractSon
+
+
+class ConcreteGrandSonFactory(AbstractBaseFactory):
+    FACTORY_FOR = models.ConcreteGrandSon
+
+
 class WithFileFactory(factory.django.DjangoModelFactory):
     FACTORY_FOR = models.WithFile
 
@@ -307,8 +315,13 @@ class DjangoNonIntegerPkTestCase(django_test.TestCase):
 @unittest.skipIf(django is None, "Django not installed.")
 class DjangoAbstractBaseSequenceTestCase(django_test.TestCase):
     def test_auto_sequence(self):
-        with factory.debug():
-            obj = ConcreteSonFactory()
+        """The sequence of the concrete son of an abstract model should be autonomous."""
+        obj = ConcreteSonFactory()
+        self.assertEqual(1, obj.pk)
+
+    def test_auto_sequence(self):
+        """The sequence of the concrete grandson of an abstract model should be autonomous."""
+        obj = ConcreteGrandSonFactory()
         self.assertEqual(1, obj.pk)
 
 
-- 
cgit v1.2.3