FactoryBoy / factory_boy

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

Cannot pass id instead of related object to DjangoModelFactory #778

Open arielpontes opened 4 years ago

arielpontes commented 4 years ago

The problem

When I create an instance of a Django model, I can pass a related object directly, or its id.

article = Article.objects.create(author=author)
article = Article.objects.create(author_id=author.pk)

One would expect a Django model factory to work the same way, but it doesn't. Everything breaks and there is no documentation regarding this use case.

Proposed solution

Ideally allow both strategies, but if not at least document this and explain how to do it.

rbarrois commented 4 years ago

Thanks for the suggestion!

However, the behaviour you describe is not a documented Django feature (the only reference to using the _id field occurs in the context of database optimization). The other mention explicitly specifies that this field isn't useful, except when writing custom SQL.

Since it's not a formally supported Django feature, I don't feel factory_boy should go out of its way to support it.

However, it should work in practice:

class ArticleFactory(factory.django.DjangoModelFactory):
    class Meta:
        model = Article
    author_id = factory.fuzzy.FuzzyChoice(Author.objects.values_list('pk', flat=True))

Obviously, if you try to provide both author and author_id, it breaks as it would if calling Article.objects.create(author=author, author_id=some_other_author.pk) — and that's exactly what happens behind the scene.

If you want this behaviour to work in your factories, you should handle that logic in _adjust_kwargs:

class ForeignKeyPKFactory(factory.django.DjangoModelFactory):
    class Meta:
        abstract = True

    @classmethod
    def _adjust_kwargs(cls, **kwargs):
        # Detect fields ending in _id
        # Won't catch a FK if its column_id has been tuned
        potential_fk_ids = [field for field in kwargs if field.endswith('_id')]

        for field in potential_fk_ids:
            # Find `author` from `author_id`, and remove it - precedence to
            # naked ID fields
            if kwargs[field] is not None and field[:-3] in kwargs:
                kwargs.pop(field[:-3])
        return kwargs

I hope this snippet addresses your needs?

arielpontes commented 4 years ago

Well yes, it is indeed for database optimization that I use this feature. When I want to save an object with a foreign key and I have he id of the related object but not the model instance, it makes no sense to fetch it from the database just so I can pass it to the parent. I just pass the id directly. I would expect this to be possible on FactoryBoy as well.

In any case, thanks for the snippet. I wasn't familiar with _adjust_kwargs. Does this allow me to pass both author and author_id (I mean, in different calls of course)?

> author = Author.objects.get(id=1)
> article = ArticleFactory(author_id=1)
> article.author.name
"Ariel Pontes"
> article = ArticleFactory(author=author)
> article.author.name
"Ariel Pontes"

Would the code above work? Should I have both author and author_id setup in the factory?

class ArticleFactory(factory.django.DjangoModelFactory):
    class Meta:
        model = Article
    author = factory.SubFactory(AuthorFactory)
    author_id = factory.fuzzy.FuzzyChoice(Author.objects.values_list('pk', flat=True))

(Sorry, I don't have my environment setup at the moment)

rbarrois commented 4 years ago

Basically, factory_boy doesn't care whether it's working with IDs, instances, or anything else. It takes your declarations, computes the values (following subfactories, etc.), and sends the resulting payload as arguments and keyword arguments to the class you've provided for a model.

If you want a factory that will use a passed-in author_id if provided, but uses a SubFactory to create an Author if needed, you could use the following:

class ArticleFactory(factory.django.DjangoModelFactory):
    class Meta:
        model = Article

    author = factory.Maybe("author_id", no_declaration=factory.SubFactory(AuthorFactory))

This way:

The _adjust_kwargs function is useful in more complex cases, but is only called once the declarations have been resolved; it can't prevent calling an AuthorFactory earlier on, which might be an issue for your code.

MRigal commented 3 years ago

This is awesome documentation @rbarrois !

Maybe to be added to the "Common Recipes" or to a new "Fancy Recipes" section!

rbarrois commented 3 years ago

@MRigal Indeed. Feel free to open a pull request with that prose in there ;)