citusdata / django-multitenant

Python/Django support for distributed multi-tenant databases like Postgres+Citus
MIT License
712 stars 117 forks source link

Through model saves are not supported #73

Open rob101 opened 4 years ago

rob101 commented 4 years ago

When using a ManyToManyFieldwith a through model, the tenant_id is not applied to the through model when the parent instance is saved.

Consider the following:

  class Store(TenantModel):
    tenant_id = 'id'
    name =  models.CharField(max_length=50)
    address = models.CharField(max_length=255)
    email = models.CharField(max_length=50)

  class Product(TenantModel):
    store = models.ForeignKey(Store)
    tenant_id='store_id'
    name = models.CharField(max_length=255)
    description = models.TextField()
    class Meta(object):
      unique_together = ["id", "store"]

  class Purchase(TenantModel):
    store = models.ForeignKey(Store)
    tenant_id='store_id'
    product_purchased = models.ManyToManyField(Product, through=Transaction)

 class Transaction(TenantModel):
    store = models.ForeignKey(Store)
    tenant_id='store_id'
    purchase = TenantForeignKey(Purchase)
    product = TenantForeignKey(Product)
    date = models.DateField()

When a Purchase instance is created and/or the product_purchased field is updated, the tenant_id is not set for the Transaction.

This appears to be because the tenant_value is set in the model save function:

    def save(self, *args, **kwargs):
        tenant_value = get_current_tenant_value()
        if not self.pk and tenant_value and not isinstance(tenant_value, list):
            setattr(self, self.tenant_field, tenant_value)

        return super(TenantModelMixin, self).save(*args, **kwargs)

This means it is not called.

louiseGrandjonc commented 4 years ago

Hi @rob101,

could you give an example around this? I would assume that you create a Transaction objects and then do purchase. product_purchased.add(transaction). In which case the save would be called.

danlls commented 3 years ago

Hi @louiseGrandjonc, I ran into this issue myself today. So, I did a little digging in Django 3.0 source code

I would assume that you create a Transaction objects and then do purchase. product_purchased.add(transaction). In which case the save would be called.

seems to be not true. Instead, django calls the manager queryset's bulk_create method.

add(...) calls _add_items() https://github.com/django/django/blob/3.0.10/django/db/models/fields/related_descriptors.py#L944

_add_items() calls bulk_create(..) https://github.com/django/django/blob/3.0.10/django/db/models/fields/related_descriptors.py#L1144

I noticed TenantManager does overwrite bulk_create to set the tenant attribute. However, Django is using _default_manager.using(..).bulk_create(..) instead of _default_manager.bulk_create(..) so it wasn't applied, below is my finding using pdb.

(Pdb) self.through._default_manager.using(db).bulk_create
<bound method QuerySet.bulk_create of <QuerySet []>>
(Pdb) self.through._default_manager.bulk_create
<bound method TenantManagerMixin.bulk_create of <django_multitenant.models.TenantManager object at 0x7fc149b57ee0>>

My current workaround is to set the through_defaults (using @rob101 's example): purchase.product_purchased.add(transaction, through_defaults={transaction.tenant_field: get_current_tenant_value()}