_bulk_create=True silently and undocumentedly does not create M2M-entries

[richardebeling]

Probably related to #202, as it is the same issue applied to ManyToManyField.

When using baker.make with _bulk_create=True, it will silently ignore arguments that should be used to fill many-to-many fields.

Expected behavior

with

class House(models.Model):
     pass

class HouseDetail(models.Model):
    houses = models.ManyToManyField(House, blank=True)

I'd expect

house = baker.make(House)
details = baker.make(HouseDetail, houses=[house], _quantity=20, _bulk_create=True)
assert details[0].houses.count() == 1

to work.

If it can't, for some technical reason, I'd expect it to be documented that this is not supported, and raise an exception.

Actual behavior

Currently, this code only raises at the assertion, and I couldn't find any remarks in the documentation.

If the _bulk_create=True argument is removed, everything behaves as expected.

Versions

Python: 3.7.5 Django: 3.2 Model Bakery: 1.4.0

[berinhard]

Hi @He3lixxx thanks for opening this issue and you're correct, it has been fixed with the #206 PR that also fixed #202. I'm releasing a new version with the fix.

By the way, here's how I tested it based on your models:

    def test_sample(self):
        with self.assertNumQueries(2):
            house = baker.make(models.House)
            details = baker.make(models.HouseDetail, houses=[house], _quantity=20, _bulk_create=True)
        assert models.House.objects.count() == 1
        assert models.HouseDetail.objects.count() == 20
        d1, d2 = models.HouseDetail.objects.all()[:2]
        assert list(d1.houses.all()) == list(d2.houses.all())

Important: models created with _bulk_create can not have the object's primary key depending on which Django version and database backend you're using. In theory, 3.2 does populate the FK.

[richardebeling]

I'm sorry, but I'm a bit confused here.

here's how I tested it based on your models:

The code snipped you posted asserts that

assert list(d1.houses.all()) == list(d2.houses.all())

which would also be true if .houses was empty on all the HouseDetail instances. For me, with 1.4.0, the assertion from by original post still fails:

assert d1.houses.count() == 1

it has been fixed with the https://github.com/model-bakers/model_bakery/pull/206 PR that also fixed https://github.com/model-bakers/model_bakery/issues/202. I'm releasing a new version with the fix.

#206 was merged in June 2021 and was part of 1.3.3, so if this was fixed in #206, there shouldn't be the need for a new release and I shouldn't be able to make this fail with 1.4.0, right?

[berinhard]

My bad here @He3lixxx. You're correct about the issue, it's still happening. I changed the assert to

        assert list(d1.houses.all()) == list(d2.houses.all()) == [house]

And I got the following traceback:

tests/test_baker.py:1043: in test_sample
    assert list(d1.houses.all()) == list(d2.houses.all()) == [house]
E   AssertionError: assert [] == [<House: House object (1)>]
E     Right contains one more item: <House: House object (1)>
E     Use -v to get the full diff

I'm reopening this issue.

[amureki]

Hey everyone,

I created a fix for this issue here: #354

However, I got hit by something else while running the test on Django 3.2 (Django 4.0 works just fine). Will need a bit more time to understand what is going on (I am suspecting https://docs.djangoproject.com/en/4.1/releases/4.1/#reverse-foreign-key-changes-for-unsaved-model-instances to play certain role here, but don't know for sure).

@He3lixxx if you are still interested in this bug and can test it for your systems, feel free to try out branch version: https://github.com/model-bakers/model_bakery/archive/refs/heads/issues/298/bulk-create-for-m2m.zip

Best, Rust

[amureki]

Ok, found, the difference in Django 3.2 and Django 4.0 that affects SQLite tests: https://docs.djangoproject.com/en/4.1/ref/models/querysets/#django.db.models.query.QuerySet.bulk_create

[amureki]

Hey @He3lixxx !

Sorry, it took us long, but we released the fix in model-bakery 1.9.0.

I'd love to get your feedback on how this works for you 🙏

[richardebeling]

Hey @amureki thanks for putting time into this. The forward-case is indeed solved with 1.9.0. However, when referring to the many2many relationship using its reverse_name, the relationship still is not created. Using the example from the initial post, the first block passes, but the second one fails:

house = baker.make(House)
details = baker.make(HouseDetail, houses=[house], _quantity=20, _bulk_create=True)
assert details[0].houses.count() == 1  # passes since 1.9.0

###

detail = baker.make(HouseDetail)
houses = baker.make(House, housedetail_set=[detail], _quantity=20, _bulk_create=True)
assert houses[0].housedetail_set.count() == 1  # fails

Again, when not using bulk_create, the relationship is created as expected.

Should I open a new issue for this?

[amureki]

@He3lixxx ohh, good point, that one was missed. I'd need to dig into the reverse relations and how to cover them as well. Indeed, a separate issue would be good to have. 🙏 I will find some time soon, but if you are curious and want to dig into it, be my guest. :)