FactoryBoy / factory_boy

A test fixtures replacement for Python
https://factoryboy.readthedocs.io/
MIT License
3.48k stars 392 forks source link

FactoryBoy creates a new object from SubFactory despite FACTORY_DJANGO_GET_OR_CREATE #69

Open RacingTadpole opened 11 years ago

RacingTadpole commented 11 years ago

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.)

rbarrois commented 11 years ago

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…

RacingTadpole commented 11 years ago

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

rh0dium commented 10 years ago

Hi There,

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

flavianmissi commented 9 years ago

+1

saulshanabrook commented 7 years ago

Has anyone developed any work arounds for this?

saulshanabrook commented 7 years ago

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',)
QasimK commented 6 years ago

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

gabrielbiasi commented 5 years ago

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

rbarrois commented 5 years ago

@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.

nicolasroma commented 3 years ago

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

daillouf commented 1 year ago

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