Skip to content

Commit

Permalink
Remove automagic pk-based sequence setup
Browse files Browse the repository at this point in the history
Related to issues #78, #92, #103, #111, #153, #170

The default value of all sequences is now 0; the automagic
``_setup_next_sequence`` behavior of Django/SQLAlchemy has been removed.

This feature's only goal was to allow the following scenario:

1. Run a Python script that uses MyFactory.create() a couple of times
   (with a unique field based on the sequence counter)
2. Run the same Python script a second time

Without the magical ``_setup_next_sequence``, the Sequence counter would be set
to 0 at the beginning of each script run, so both runs would generate objects
with the same values for the unique field ; thus conflicting and crashing.

The above behavior having only a very limited use and bringing various
issues (hitting the database on ``build()``, problems with non-integer
or composite primary key columns, ...), it has been removed.

It could still be emulated through custom ``_setup_next_sequence``
methods, or by calling ``MyFactory.reset_sequence()``.
  • Loading branch information
rbarrois committed Nov 16, 2014
1 parent 827af8f commit 13d310f
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 74 deletions.
13 changes: 12 additions & 1 deletion docs/changelog.rst
Expand Up @@ -2,6 +2,17 @@ ChangeLog
========= =========




.. _v2.5.0:

2.5.0 (master)
--------------

*Deprecation:*

- Remove deprecated features from :ref:`v2.4.0`
- Remove the auto-magical sequence setup (based on the latest primary key value in the database) for Django and SQLAlchemy;
this relates to issues :issue:`170`, :issue:`153`, :issue:`111`, :issue:`103`, :issue:`92`, :issue:`78`.

.. _v2.4.1: .. _v2.4.1:


2.4.1 (2014-06-23) 2.4.1 (2014-06-23)
Expand All @@ -19,7 +30,7 @@ ChangeLog
*New:* *New:*


- Add support for :attr:`factory.fuzzy.FuzzyInteger.step`, thanks to `ilya-pirogov <https://github.com/ilya-pirogov>`_ (:issue:`120`) - Add support for :attr:`factory.fuzzy.FuzzyInteger.step`, thanks to `ilya-pirogov <https://github.com/ilya-pirogov>`_ (:issue:`120`)
- Add :meth:`~factory.django.mute_signals` decorator to temporarily disable some signals, thanks to `ilya-pirogov <https://github.com>`_ (:issue:`122`) - Add :meth:`~factory.django.mute_signals` decorator to temporarily disable some signals, thanks to `ilya-pirogov <https://github.com/ilya-pirogov>`_ (:issue:`122`)
- Add :class:`~factory.fuzzy.FuzzyFloat` (:issue:`124`) - Add :class:`~factory.fuzzy.FuzzyFloat` (:issue:`124`)
- Declare target model and other non-declaration fields in a ``class Meta`` section. - Declare target model and other non-declaration fields in a ``class Meta`` section.


Expand Down
2 changes: 0 additions & 2 deletions docs/orms.rst
Expand Up @@ -35,7 +35,6 @@ All factories for a Django :class:`~django.db.models.Model` should use the
* The :attr:`~factory.FactoryOptions.model` attribute also supports the ``'app.Model'`` * The :attr:`~factory.FactoryOptions.model` attribute also supports the ``'app.Model'``
syntax syntax
* :func:`~factory.Factory.create()` uses :meth:`Model.objects.create() <django.db.models.query.QuerySet.create>` * :func:`~factory.Factory.create()` uses :meth:`Model.objects.create() <django.db.models.query.QuerySet.create>`
* :func:`~factory.Factory._setup_next_sequence()` selects the next unused primary key value
* When using :class:`~factory.RelatedFactory` or :class:`~factory.PostGeneration` * When using :class:`~factory.RelatedFactory` or :class:`~factory.PostGeneration`
attributes, the base object will be :meth:`saved <django.db.models.Model.save>` attributes, the base object will be :meth:`saved <django.db.models.Model.save>`
once all post-generation hooks have run. once all post-generation hooks have run.
Expand Down Expand Up @@ -284,7 +283,6 @@ To work, this class needs an `SQLAlchemy`_ session object affected to the :attr:
This class provides the following features: This class provides the following features:


* :func:`~factory.Factory.create()` uses :meth:`sqlalchemy.orm.session.Session.add` * :func:`~factory.Factory.create()` uses :meth:`sqlalchemy.orm.session.Session.add`
* :func:`~factory.Factory._setup_next_sequence()` selects the next unused primary key value


.. attribute:: FACTORY_SESSION .. attribute:: FACTORY_SESSION


Expand Down
12 changes: 0 additions & 12 deletions factory/alchemy.py
Expand Up @@ -43,18 +43,6 @@ class Meta:
'FACTORY_SESSION': 'sqlalchemy_session', 'FACTORY_SESSION': 'sqlalchemy_session',
}) })


@classmethod
def _setup_next_sequence(cls, *args, **kwargs):
"""Compute the next available PK, based on the 'pk' database field."""
session = cls._meta.sqlalchemy_session
model = cls._meta.model
pk = getattr(model, model.__mapper__.primary_key[0].name)

This comment has been minimized.

Copy link
@lazytype

lazytype Jan 5, 2015

The import of max from sqlalchemy.sql.functions in this file is no longer necessary after this deletion :)

max_pk = session.query(max(pk)).one()[0]
if isinstance(max_pk, int):
return max_pk + 1 if max_pk else 1
else:
return 1

@classmethod @classmethod
def _create(cls, model_class, *args, **kwargs): def _create(cls, model_class, *args, **kwargs):
"""Create an instance of the model, and save it to the database.""" """Create an instance of the model, and save it to the database."""
Expand Down
15 changes: 0 additions & 15 deletions factory/django.py
Expand Up @@ -109,21 +109,6 @@ def _get_manager(cls, model_class):
except AttributeError: except AttributeError:
return model_class.objects return model_class.objects


@classmethod
def _setup_next_sequence(cls):
"""Compute the next available PK, based on the 'pk' database field."""

model = cls._get_model_class() # pylint: disable=E1101
manager = cls._get_manager(model)

try:
return 1 + manager.values_list('pk', flat=True
).order_by('-pk')[0]
except (IndexError, TypeError):
# IndexError: No instance exist yet
# TypeError: pk isn't an integer type
return 1

@classmethod @classmethod
def _get_or_create(cls, model_class, *args, **kwargs): def _get_or_create(cls, model_class, *args, **kwargs):
"""Create an instance of the model through objects.get_or_create.""" """Create an instance of the model through objects.get_or_create."""
Expand Down
22 changes: 11 additions & 11 deletions tests/test_alchemy.py
Expand Up @@ -88,18 +88,18 @@ def test_pk_creation(self):


StandardFactory.reset_sequence() StandardFactory.reset_sequence()
std2 = StandardFactory.create() std2 = StandardFactory.create()
self.assertEqual('foo2', std2.foo) self.assertEqual('foo0', std2.foo)
self.assertEqual(2, std2.id) self.assertEqual(0, std2.id)


def test_pk_force_value(self): def test_pk_force_value(self):
std1 = StandardFactory.create(id=10) std1 = StandardFactory.create(id=10)
self.assertEqual('foo1', std1.foo) # sequence was set before pk self.assertEqual('foo1', std1.foo) # sequence and pk are unrelated
self.assertEqual(10, std1.id) self.assertEqual(10, std1.id)


StandardFactory.reset_sequence() StandardFactory.reset_sequence()
std2 = StandardFactory.create() std2 = StandardFactory.create()
self.assertEqual('foo11', std2.foo) self.assertEqual('foo0', std2.foo) # Sequence doesn't care about pk
self.assertEqual(11, std2.id) self.assertEqual(0, std2.id)




@unittest.skipIf(sqlalchemy is None, "SQLalchemy not installed.") @unittest.skipIf(sqlalchemy is None, "SQLalchemy not installed.")
Expand All @@ -111,27 +111,27 @@ def setUp(self):


def test_first(self): def test_first(self):
nonint = NonIntegerPkFactory.build() nonint = NonIntegerPkFactory.build()
self.assertEqual('foo1', nonint.id) self.assertEqual('foo0', nonint.id)


def test_many(self): def test_many(self):
nonint1 = NonIntegerPkFactory.build() nonint1 = NonIntegerPkFactory.build()
nonint2 = NonIntegerPkFactory.build() nonint2 = NonIntegerPkFactory.build()


self.assertEqual('foo1', nonint1.id) self.assertEqual('foo0', nonint1.id)
self.assertEqual('foo2', nonint2.id) self.assertEqual('foo1', nonint2.id)


def test_creation(self): def test_creation(self):
nonint1 = NonIntegerPkFactory.create() nonint1 = NonIntegerPkFactory.create()
self.assertEqual('foo1', nonint1.id) self.assertEqual('foo0', nonint1.id)


NonIntegerPkFactory.reset_sequence() NonIntegerPkFactory.reset_sequence()
nonint2 = NonIntegerPkFactory.build() nonint2 = NonIntegerPkFactory.build()
self.assertEqual('foo1', nonint2.id) self.assertEqual('foo0', nonint2.id)


def test_force_pk(self): def test_force_pk(self):
nonint1 = NonIntegerPkFactory.create(id='foo10') nonint1 = NonIntegerPkFactory.create(id='foo10')
self.assertEqual('foo10', nonint1.id) self.assertEqual('foo10', nonint1.id)


NonIntegerPkFactory.reset_sequence() NonIntegerPkFactory.reset_sequence()
nonint2 = NonIntegerPkFactory.create() nonint2 = NonIntegerPkFactory.create()
self.assertEqual('foo1', nonint2.id) self.assertEqual('foo0', nonint2.id)
40 changes: 20 additions & 20 deletions tests/test_django.py
Expand Up @@ -174,32 +174,32 @@ def setUp(self):


def test_pk_first(self): def test_pk_first(self):
std = StandardFactory.build() std = StandardFactory.build()
self.assertEqual('foo1', std.foo) self.assertEqual('foo0', std.foo)


def test_pk_many(self): def test_pk_many(self):
std1 = StandardFactory.build() std1 = StandardFactory.build()
std2 = StandardFactory.build() std2 = StandardFactory.build()
self.assertEqual('foo1', std1.foo) self.assertEqual('foo0', std1.foo)
self.assertEqual('foo2', std2.foo) self.assertEqual('foo1', std2.foo)


def test_pk_creation(self): def test_pk_creation(self):
std1 = StandardFactory.create() std1 = StandardFactory.create()
self.assertEqual('foo1', std1.foo) self.assertEqual('foo0', std1.foo)
self.assertEqual(1, std1.pk) self.assertEqual(1, std1.pk)


StandardFactory.reset_sequence() StandardFactory.reset_sequence()
std2 = StandardFactory.create() std2 = StandardFactory.create()
self.assertEqual('foo2', std2.foo) self.assertEqual('foo0', std2.foo)
self.assertEqual(2, std2.pk) self.assertEqual(2, std2.pk)


def test_pk_force_value(self): def test_pk_force_value(self):
std1 = StandardFactory.create(pk=10) std1 = StandardFactory.create(pk=10)
self.assertEqual('foo1', std1.foo) # sequence was set before pk self.assertEqual('foo0', std1.foo) # sequence is unrelated to pk
self.assertEqual(10, std1.pk) self.assertEqual(10, std1.pk)


StandardFactory.reset_sequence() StandardFactory.reset_sequence()
std2 = StandardFactory.create() std2 = StandardFactory.create()
self.assertEqual('foo11', std2.foo) self.assertEqual('foo0', std2.foo)
self.assertEqual(11, std2.pk) self.assertEqual(11, std2.pk)




Expand All @@ -212,12 +212,12 @@ def setUp(self):
def test_no_pk(self): def test_no_pk(self):
std = StandardFactoryWithPKField() std = StandardFactoryWithPKField()
self.assertIsNotNone(std.pk) self.assertIsNotNone(std.pk)
self.assertEqual('foo1', std.foo) self.assertEqual('foo0', std.foo)


def test_force_pk(self): def test_force_pk(self):
std = StandardFactoryWithPKField(pk=42) std = StandardFactoryWithPKField(pk=42)
self.assertIsNotNone(std.pk) self.assertIsNotNone(std.pk)
self.assertEqual('foo1', std.foo) self.assertEqual('foo0', std.foo)


def test_reuse_pk(self): def test_reuse_pk(self):
std1 = StandardFactoryWithPKField(foo='bar') std1 = StandardFactoryWithPKField(foo='bar')
Expand Down Expand Up @@ -286,9 +286,9 @@ class Meta:
self.assertEqual(models.StandardModel, e1.__class__) self.assertEqual(models.StandardModel, e1.__class__)
self.assertEqual(models.StandardSon, e2.__class__) self.assertEqual(models.StandardSon, e2.__class__)
self.assertEqual(models.StandardModel, e3.__class__) self.assertEqual(models.StandardModel, e3.__class__)
self.assertEqual(1, e1.foo) self.assertEqual(0, e1.foo)
self.assertEqual(2, e2.foo) self.assertEqual(1, e2.foo)
self.assertEqual(3, e3.foo) self.assertEqual(2, e3.foo)




@unittest.skipIf(django is None, "Django not installed.") @unittest.skipIf(django is None, "Django not installed.")
Expand All @@ -299,23 +299,23 @@ def setUp(self):


def test_first(self): def test_first(self):
nonint = NonIntegerPkFactory.build() nonint = NonIntegerPkFactory.build()
self.assertEqual('foo1', nonint.foo) self.assertEqual('foo0', nonint.foo)


def test_many(self): def test_many(self):
nonint1 = NonIntegerPkFactory.build() nonint1 = NonIntegerPkFactory.build()
nonint2 = NonIntegerPkFactory.build() nonint2 = NonIntegerPkFactory.build()


self.assertEqual('foo1', nonint1.foo) self.assertEqual('foo0', nonint1.foo)
self.assertEqual('foo2', nonint2.foo) self.assertEqual('foo1', nonint2.foo)


def test_creation(self): def test_creation(self):
nonint1 = NonIntegerPkFactory.create() nonint1 = NonIntegerPkFactory.create()
self.assertEqual('foo1', nonint1.foo) self.assertEqual('foo0', nonint1.foo)
self.assertEqual('foo1', nonint1.pk) self.assertEqual('foo0', nonint1.pk)


NonIntegerPkFactory.reset_sequence() NonIntegerPkFactory.reset_sequence()
nonint2 = NonIntegerPkFactory.build() nonint2 = NonIntegerPkFactory.build()
self.assertEqual('foo1', nonint2.foo) self.assertEqual('foo0', nonint2.foo)


def test_force_pk(self): def test_force_pk(self):
nonint1 = NonIntegerPkFactory.create(pk='foo10') nonint1 = NonIntegerPkFactory.create(pk='foo10')
Expand All @@ -324,8 +324,8 @@ def test_force_pk(self):


NonIntegerPkFactory.reset_sequence() NonIntegerPkFactory.reset_sequence()
nonint2 = NonIntegerPkFactory.create() nonint2 = NonIntegerPkFactory.create()
self.assertEqual('foo1', nonint2.foo) self.assertEqual('foo0', nonint2.foo)
self.assertEqual('foo1', nonint2.pk) self.assertEqual('foo0', nonint2.pk)




@unittest.skipIf(django is None, "Django not installed.") @unittest.skipIf(django is None, "Django not installed.")
Expand Down
20 changes: 7 additions & 13 deletions tests/test_using.py
Expand Up @@ -1490,12 +1490,6 @@ def get_or_create(self, **kwargs):
instance.id = 2 instance.id = 2
return instance, True return instance, True


def values_list(self, *args, **kwargs):
return self

def order_by(self, *args, **kwargs):
return [1]



class BetterFakeModel(object): class BetterFakeModel(object):
@classmethod @classmethod
Expand Down Expand Up @@ -1618,14 +1612,14 @@ class Meta:
o1 = TestModelFactory() o1 = TestModelFactory()
o2 = TestModelFactory() o2 = TestModelFactory()


self.assertEqual('foo_2', o1.a) self.assertEqual('foo_0', o1.a)
self.assertEqual('foo_3', o2.a) self.assertEqual('foo_1', o2.a)


o3 = TestModelFactory.build() o3 = TestModelFactory.build()
o4 = TestModelFactory.build() o4 = TestModelFactory.build()


self.assertEqual('foo_4', o3.a) self.assertEqual('foo_2', o3.a)
self.assertEqual('foo_5', o4.a) self.assertEqual('foo_3', o4.a)


def test_no_get_or_create(self): def test_no_get_or_create(self):
class TestModelFactory(factory.django.DjangoModelFactory): class TestModelFactory(factory.django.DjangoModelFactory):
Expand All @@ -1636,7 +1630,7 @@ class Meta:


o = TestModelFactory() o = TestModelFactory()
self.assertEqual(None, o._defaults) self.assertEqual(None, o._defaults)
self.assertEqual('foo_2', o.a) self.assertEqual('foo_0', o.a)
self.assertEqual(2, o.id) self.assertEqual(2, o.id)


def test_get_or_create(self): def test_get_or_create(self):
Expand All @@ -1652,7 +1646,7 @@ class Meta:


o = TestModelFactory() o = TestModelFactory()
self.assertEqual({'c': 3, 'd': 4}, o._defaults) self.assertEqual({'c': 3, 'd': 4}, o._defaults)
self.assertEqual('foo_2', o.a) self.assertEqual('foo_0', o.a)
self.assertEqual(2, o.b) self.assertEqual(2, o.b)
self.assertEqual(3, o.c) self.assertEqual(3, o.c)
self.assertEqual(4, o.d) self.assertEqual(4, o.d)
Expand All @@ -1672,7 +1666,7 @@ class Meta:


o = TestModelFactory() o = TestModelFactory()
self.assertEqual({}, o._defaults) self.assertEqual({}, o._defaults)
self.assertEqual('foo_2', o.a) self.assertEqual('foo_0', o.a)
self.assertEqual(2, o.b) self.assertEqual(2, o.b)
self.assertEqual(3, o.c) self.assertEqual(3, o.c)
self.assertEqual(4, o.d) self.assertEqual(4, o.d)
Expand Down

0 comments on commit 13d310f

Please sign in to comment.