_bulk_create flag is now creating related objects as well

[berinhard]

Fixes #202

@amureki and @anapaulagomes please take a read before on Djangos bulk_create docs and you'll understand more the context behind this PR:

https://docs.djangoproject.com/en/3.0/ref/models/querysets/#bulk-create

Short summary: Django does not update the created objects via bulk and, because of that, there no way to avoid subsequent save calls on related objects. I've implemented with recursion because the same is true for the related of the related objects =}

[anapaulagomes]

Really good, @berinhard! 🎉 (please note that the pipeline is broken for py38 mypy)

[berinhard]

Thanks @anapaulagomes! I noticed the mypy issue and I'll fix it as soon as possible! Thanks for the review =)

[berinhard]

Also, @timjklein36 can review this one too 👍

[timjklein36]

I just had one question and a suggestion about replacing QueryCount with Django's assertNumQueries.

Also, any chance we could use bulk_create for all the related models as well? From the depth of the nested related objects up the tree?

[timjklein36]

What if the fk_objects do not match up 1-to-1 with objects? (One way the code looks like this could happen is if the condition on line 638 is not True and a given object's related model for this iteration's fk is not appended to the list. Could that ever happen? If not, then is the condition on line 638 necessary?

[berinhard]

I added the condition on 638 just as an explicit safeguard. For example, that condition enables people to write tests such as:

user = baker.make(User)
logs = baker.make(LogEntries, user=user, _quantity=10, _bulk_create=True)

Every log entry will already have a associated user with and id because it has already been previously created. If we remove 638 condition, baker can perform unexpected triggers on the user object. For example, what if's there a post_save method on the User model.

I mean, I just think it was safer to add this check to not perform extra and unnecessary save operations.

[timjklein36]

I think I originally introduced this class when I was unaware of a built-in TransactionTestCase (Django's base TestCase class) way of doing it: https://docs.djangoproject.com/en/3.2/topics/testing/tools/#django.test.TransactionTestCase.assertNumQueries.

We could replace our use of this class like so:

Old Style

queries = QueryCount()

with queries.start_count():
    baker.make(models.LonelyPerson, _quantity=5, _bulk_create=True)
    assert queries.count == 6

With built-in assertNumQueries

with self.assertNumQueries(6):
    baker.make(models.LonelyPerson, _quantity=5, _bulk_create=True)

[berinhard]

Yes, we can refactor this to use django's assertNumQueries. Can you open an issue documenting it and label it as a good first issue?

[berinhard]

@timjklein36 unfortunatelly we can't use bulk_create for the FKs because the method doesn't return the created objects nor update the existing one with their IDs. That's the reason why I had to call save manually: to be able to get the referenced IDs from the related objects without having to query for them after creating them using django's bulk_create.

Thanks for you review =)