Skip to content

Commit

Permalink
Deprecation plan
Browse files Browse the repository at this point in the history
This change is sure to cause disruption for users. Smooth the transition
with an option.
  • Loading branch information
francoisfreitag committed Dec 9, 2020
1 parent 1535959 commit 880d9e7
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 0 deletions.
16 changes: 16 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,22 @@ retain the previous behavior.

:func:`~factory.use_strategy()` will be removed in the next major version.

- :class:`factory.django.DjangoModelFactory._after_postgeneration` raises a :class:`DeprecationWarning` when
calling :meth:`~django.db.models.Model.save`. :meth:`~django.db.models.Model.save` will no longer be called in
the next major version of Factory Boy.

Inspect your :class:`~factory.django.DjangoModelFactory` subclasses:

- If the :meth:`~django.db.models.Model.save` call is not needed after :class:`~factory.PostGeneration`, set
:attr:`~factory.django.DjangoOptions.skip_postgeneration_save` to ``True``.
- Otherwise, the instance has been modified by :class:`~factory.PostGeneration` hooks and needs to be
:meth:`~django.db.models.Model.save`\ d. Either:

- call :meth:`django.db.models.Model.save` in the :class:`~factory.PostGeneration` hook that modifies the
instance, or
- override :class:`~factory.django.DjangoModelFactory._after_postgeneration` to
:meth:`~django.db.models.Model.save` the instance.


3.1.1 (unreleased)
------------------
Expand Down
8 changes: 8 additions & 0 deletions docs/orms.rst
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,14 @@ All factories for a Django :class:`~django.db.models.Model` should use the
>>> john.email # The email value was not updated
"john@example.com"
.. attribute:: skip_postgeneration_save

Transitional option to prevent
:meth:`~factory.django.DjangoModelFactory._after_postgeneration` from
issuing a duplicate call to :meth:`~django.db.models.Model.save` on the
created instance when :class:`factory.PostGeneration` hooks return a
value.


Extra fields
""""""""""""
Expand Down
19 changes: 19 additions & 0 deletions factory/django.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io
import logging
import os
import warnings

from django.contrib.auth.hashers import make_password
from django.core import files as django_files
Expand Down Expand Up @@ -48,6 +49,7 @@ def _build_default_options(self):
return super()._build_default_options() + [
base.OptionDefault('django_get_or_create', (), inherit=True),
base.OptionDefault('database', DEFAULT_DB_ALIAS, inherit=True),
base.OptionDefault('skip_postgeneration_save', False, inherit=True),
]

def _get_counter_reference(self):
Expand Down Expand Up @@ -166,6 +168,23 @@ def _create(cls, model_class, *args, **kwargs):
manager = cls._get_manager(model_class)
return manager.create(*args, **kwargs)

# DEPRECATED. Remove this override with the next major release.
@classmethod
def _after_postgeneration(cls, instance, create, results=None):
"""Save again the instance if creating and at least one hook ran."""
if create and results and not cls._meta.skip_postgeneration_save:
warnings.warn(
f"{cls.__name__}._after_postgeneration will stop saving the instance "
"after postgeneration hooks in the next major release.\n"
"If the save call is extraneous, set skip_postgeneration_save=True "
f"in the {cls.__name__}.Meta.\n"
"To keep saving the instance, move the save call to your "
"postgeneration hooks or override _after_postgeneration.",
DeprecationWarning,
)
# Some post-generation hooks ran, and may have modified us.
instance.save()


class Password(declarations.Transformer):
def __init__(self, password, *args, **kwargs):
Expand Down
33 changes: 33 additions & 0 deletions tests/test_django.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,9 @@ class PointedRelatedFactory(PointedFactory):
factory_related_name='pointed',
)

class Meta:
skip_postgeneration_save = True

class PointerExtraFactory(PointerFactory):
pointed__foo = 'extra_new_foo'

Expand All @@ -441,6 +444,9 @@ class Params:
)
)

class Meta:
skip_postgeneration_save = True

cls.PointedFactory = PointedFactory
cls.PointerFactory = PointerFactory
cls.PointedRelatedFactory = PointedRelatedFactory
Expand Down Expand Up @@ -935,6 +941,7 @@ def test_class_decorator_with_subfactory(self):
class WithSignalsDecoratedFactory(factory.django.DjangoModelFactory):
class Meta:
model = models.WithSignals
skip_postgeneration_save = True

@factory.post_generation
def post(obj, create, extracted, **kwargs):
Expand Down Expand Up @@ -1021,6 +1028,7 @@ def test_class_decorator_with_muted_related_factory(self):
class UndecoratedFactory(factory.django.DjangoModelFactory):
class Meta:
model = models.PointerModel
skip_postgeneration_save = True
pointed = factory.RelatedFactory(self.WithSignalsDecoratedFactory)

UndecoratedFactory()
Expand All @@ -1041,3 +1049,28 @@ class Meta:
# Our CustomManager will remove the 'arg=' argument,
# invalid for the actual model.
ObjFactory.create(arg='invalid')


class DjangoModelFactoryDuplicateSaveDeprecationTest(django_test.TestCase):
class StandardFactoryWithPost(StandardFactory):
@factory.post_generation
def post_action(obj, create, extracted, **kwargs):
return 3

def test_create_warning(self):
with self.assertWarns(DeprecationWarning) as cm:
self.StandardFactoryWithPost.create()

[msg] = cm.warning.args
self.assertEqual(
msg,
"StandardFactoryWithPost._after_postgeneration will stop saving the "
"instance after postgeneration hooks in the next major release.\n"
"If the save call is extraneous, set skip_postgeneration_save=True in the "
"StandardFactoryWithPost.Meta.\n"
"To keep saving the instance, move the save call to your postgeneration "
"hooks or override _after_postgeneration.",
)

def test_build_no_warning(self):
self.StandardFactoryWithPost.build()

0 comments on commit 880d9e7

Please sign in to comment.