factory_boy icon indicating copy to clipboard operation
factory_boy copied to clipboard

FactoryBoy creates a new object from SubFactory despite FACTORY_DJANGO_GET_OR_CREATE

Open RacingTadpole opened this issue 11 years ago • 11 comments

I am using the setting FACTORY_DJANGO_GET_OR_CREATE. But in some cases, when I ask for an existing object with an existing SubFactory object, it creates an unused object despite this setting.

For example, in a brand new project, I typed:

# models.py
from django.db import models

class A(models.Model):
    name = models.CharField(max_length=10)

class B(models.Model):
    name = models.CharField(max_length=10)
    a = models.ForeignKey(A)

And

# factories.py
import factory

from . import models

class AFactory(factory.DjangoModelFactory):
    FACTORY_FOR = models.A
    FACTORY_DJANGO_GET_OR_CREATE = ('name',)

    name = factory.Sequence(lambda n: 'A-{0}'.format(n))

class BFactory(factory.DjangoModelFactory):
    FACTORY_FOR = models.B
    FACTORY_DJANGO_GET_OR_CREATE = ('name',)

    name = factory.Sequence(lambda n: 'B-{0}'.format(n))
    a = factory.SubFactory(AFactory)

Then:

from factories import *

a = AFactory(name="Apple")
models.A.objects.all()
# one object
b = BFactory(a__name="Apple", name="Beetle")
models.B.objects.all()
models.A.objects.all()
# one A object, one B object
b = BFactory(name="Beetle")
models.B.objects.all()
models.A.objects.all()
# still one B object, but now a new, unused A object too

The final call to BFactory has brought into being a new object of class A, even though the B object with name Beetle already exists (and is not re-created). Why, and how do I stop this new A object being created?

(I know I can get around this by calling instead:

b = BFactory(name="Beetle", a__name="Apple")

but in my actual use case, I have several dependencies and levels of hierarchy, and it's messy to supply extra redundant parameters this way - and I can't seem to get the right combination of parameters.)

Thanks!

(I also asked this on StackOverflow here too.)

RacingTadpole avatar Jul 05 '13 21:07 RacingTadpole

Hi,

Thanks for the report!

You've just found a nasty bug in factory_boy, due to its core design :/

The internal algorithm used in factory_boy when calling MyFactory(**kwargs) is:

  1. Retrieve all fields from class declaration + passed-in kwargs
  2. Put them in a dict
  3. "Compute" them all (resolve all lazy values, including SubFactory)
  4. Call cls._build or cls._create (this is where get_or_create is triggered).
  5. Return the generated object

Here, the problem is that, when the factory has a DJANGO_FACTORY_GET_OR_CREATE param, we'd expect the "does it exist?" part to run before trying to resolve all fields.

I've added a test exhibiting the failure, now I need to find a way to alter the behavior…

rbarrois avatar Jul 10 '13 23:07 rbarrois

Hi,

Thanks for getting back to me and looking into it! Factory_boy has saved me quite a bit of effort - I really appreciate your work on it.

cheers Arthur

RacingTadpole avatar Jul 12 '13 01:07 RacingTadpole

Hi There,

Is there any progress on this - I am also seeing this behavior.

rh0dium avatar Nov 20 '13 17:11 rh0dium

+1

flavianmissi avatar Jan 30 '15 10:01 flavianmissi

Has anyone developed any work arounds for this?

saulshanabrook avatar Jul 25 '17 17:07 saulshanabrook

I implemented a simple work around that created a factory instance once and only once, by just saving it as a variable:

class BlogIndexPageFactory(WagtailPageFactory):
    intro = "Index"
    title = "Index"
    parent = factory.SubFactory(WagtailRootPageFactory)

    class Meta:
        model = blog_models.BlogIndexPage
        django_get_or_create = ('intro',)


# we need these helper functions to generate at most one root page
# and index page because of this bug
# https://github.com/FactoryBoy/factory_boy/issues/69
_blog_index_page = None


def _get_blog_index_page():
    global _blog_index_page
    if _blog_index_page is None:
        _blog_index_page = BlogIndexPageFactory()
    return _blog_index_page


@register
class WagtailSiteFactory(wagtail_factories.SiteFactory):
    is_default_site = True
    site_name = None
    # root_page = factory.SubFactory(BlogIndexPageFactory)
    root_page = factory.LazyFunction(_get_blog_index_page)
    class Meta:
        django_get_or_create = ('site_name',)

saulshanabrook avatar Jul 25 '17 19:07 saulshanabrook

This also affects the post-generation methods. I didn’t expect them to be called if the object already existed with get-or-create.

QasimK avatar Sep 29 '17 12:09 QasimK

5 years... Is there any hope to fix this yet?

gabrielbiasi avatar Nov 30 '18 10:11 gabrielbiasi

@gabrielbiasi Contributions are welcome :)

If you're interested, I'd suggest starting with a few other, simpler, changes to get a better understanding of the internals of factory_boy.

This unexpected behaviour is directly linked to the internal logic of factory_boy, as stated in my initial comment; a few refactors have brought us closer to finding a solution to this, but it's still a very complex and risky change.

rbarrois avatar Nov 30 '18 10:11 rbarrois

Hi @rbarrois! I was working and suddenly I run into this issue. I'm glad that you already know whats going on. I leave here for you 2 questions.

  1. Why this error seems to be not idempotent? Debugging the issue I found out that I call the Factory with the same args and some tomes returns the one already created, other times creates a new one and other times it returns an Exception
  2. Is there any way to solve this temporally until is fixed?

Thanks in advanced Regards

nicolasroma avatar Apr 06 '21 17:04 nicolasroma

Hello everyone,

I’ve just looked at how subfactories are called, and it appeared that any kwargs in the SubFactory call are passed to the factory (the first argument in that call)

So I tried with the following and it works just fine

class BatchFileFactory(factory.django.DjangoModelFactory):
    class Meta:
        model = BatchFile
        django_get_or_create = ('id',)


class RefundFactory(factory.django.DjangoModelFactory):
    class Meta:
        model = Refund

    batch_file = factory.SubFactory(BatchFileFactory, id=122)
    amount = 50

And then several calls to RefundFactory all use the same BatchFile with id 122.

Seems like factory-boy also accepts factory.SelfAttribute calls inside those kwargs, like so,

class BatchFileFactory(factory.django.DjangoModelFactory):
    class Meta:
        model = BatchFile
        django_get_or_create = ('id',)


class RefundFactory(factory.django.DjangoModelFactory):
    class Meta:
        model = Refund

    batch_file = factory.SubFactory(BatchFileFactory, id=factory.SelfAttribute("..amount"))
    amount = 50

But then "..amount" must be a field on the upper object’s class, here Refund.

Hope this can help

daillouf avatar Oct 18 '22 12:10 daillouf