_original_params do not get cleared out when the factory is used as a SubFactory

[mrwbarg]

Description

Using a factory to create an object (using .create()) will set args passed in onto the _original_params class attribute.
If a second factory uses that same factory as a SubFactory in another test case, the _original_params will still be set
to the values from the first test case because the SubFactory call does not reset the _original_params attribute.

To Reproduce

There's a working example below.

Model / Factory code

Check the code below for the factories and models.

The issue

This causes an issue when overriding the _create method in the factory to grab the _original_params since those will not
be valid/stale (especially because the SubFactory passes no params). When one of the _original_params is a model instance, on the subfactory call (in the separate test case) and is used on the _create override to set something in the object an integrity error
will be raised since the object referenced in the _original_params no longer exists in the database (it was deleted after the first
test was done).

  from datetime import timedelta
  from unittest import skip

  import factory
  import pytz
  from django.test import TestCase
  from factory.django import DjangoModelFactory

  from appointments.models import Appointment, AppointmentAttendance, AppointmentType
  from people.models import Person
  from utils.testing_utils import CustomSubFactory


  class PersonFactory(DjangoModelFactory):
      class Meta:
          model = Person


  class AppointmentTypeFactory(DjangoModelFactory):
      name = factory.Faker("company")
      custom_id = factory.Faker("numerify", text="########")

      class Meta:
          model = AppointmentType


  class AppointmentFactory(DjangoModelFactory):
      appointment_type = factory.SubFactory(AppointmentTypeFactory)
      start_at = factory.Faker("future_datetime", tzinfo=pytz.timezone("UTC"))
      end_at = factory.LazyAttribute(lambda d: d.start_at + timedelta(hours=1))

      @classmethod
      def _create(cls, model_class, *args, **kwargs):
          # NOTE:
          # Without the CustomSubFactory this params variable is still reflecting
          # values from the first use of the AppointmentFactory class. Notice that the
          # values are bound to the class and not to the instance.

          params = getattr(cls, "_original_params", {}) or {}
          kwargs["appointment_type"] = cls._get_appointment_type(params)
          return super()._create(model_class, *args, **kwargs)

      @classmethod
      def _get_appointment_type(cls, params):
          explicit = params.get("appointment_type", None)
          if isinstance(explicit, AppointmentType):
              return explicit
          return AppointmentTypeFactory.create(name="Foo", custom_id="123456")

      class Meta:
          model = Appointment


  class AppointmentAttendanceFactory(DjangoModelFactory):
      appointment = factory.SubFactory(AppointmentFactory)
      member = factory.SubFactory(PersonFactory)

      class Meta:
          model = AppointmentAttendance


  class BetterAppointmentAttendanceFactory(DjangoModelFactory):
      appointment = CustomSubFactory(AppointmentFactory)
      member = factory.SubFactory(PersonFactory)

      class Meta:
          model = AppointmentAttendance


  # -------------------------------------------------------------------------------------


  class TestA(TestCase):
      def test_this_will_work_but_must_come_first(self):
          type_ = AppointmentTypeFactory.create(name="Bar")
          # This usage is setting the original params in the AppointmentFactory class.
          # Those values get stuck there for the second use of this class.
          appointment = AppointmentFactory.create(appointment_type=type_)
          assert appointment.appointment_type.name == "Bar"


  class TestB(TestCase):
      @skip("This will fail because it's not using the CustomSubFactory")
      def test_this_will_fail_if_second(self):
          attendance = AppointmentAttendanceFactory.create()
          # This errors with an InegrityError because the AppointmentFactory is trying to
          # use an AppointmentType that was crafted in the previous test. (note that we
          # can't assert the error because it occurs during the tear down.)
          assert isinstance(attendance.appointment.appointment_type.name, str)

      def test_this_will_work_if_second(self):
          # The CustomSubFactory will reset the _original_params attribute when using the
          # SubFactory. This doens't happen with the default SubFactory.
          attendance = BetterAppointmentAttendanceFactory.create()
          assert isinstance(attendance.appointment.appointment_type.name, str)

Notes

To fix this, we created the following class:

    import factory


    class CustomSubFactory(factory.SubFactory):
        def evaluate(self, *args, **kwargs):
            subfactory = self.get_factory()
            subfactory._original_params = None
            return super().evaluate(*args, **kwargs)

This resets the _original_params when the factory is called through as a SubFactory
Maybe another way to see it is as a DjangoModelSubFactory.

[sum-rock]

@mrwbarg and I have been working this issue together. Our subclass solution was merged yesterday and working fine in our code base with nearly 5K unittests and several dozen factories. We don't "need" a resolution for this but would be happy to make a PR here with our fix at the SubFactory.evaluate definition level. It might save another team 2 days of bug hunting.

Rather than just making a PR, we thought it was better to start with an issue. I know we are on the fringe here by extending the _create method and using _original_params in a way that might deviate from what was initially intended. However, by reading other issues here, it appears that we're not the only ones who have found an "off label" use for the _original_params classattr. I think it's reasonable to want a way to access the kwargs that were passed to the create or build function.

For that matter, things seem to have worked just fine until we ran into this asymmetric implementation of _create and create with regard to the SubFactory versus the DjangoModelFactory. Having the SubFactory class build using the _create function directly is a reasonable architectural choice (there are discussions in other posts on that choice) and it isn't the true surface area for this issue. In our opinion, the missing state reset on the _original_params attribute when _create is called directly by the SubFactory make this a bug rather than an architectural choice.