citusdata / django-multitenant

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

Cannot delete instance of model referenced by ManyToManyField #90

Closed pickyuptruck closed 4 years ago

pickyuptruck commented 4 years ago

I have the following models:

models.py

class School(TenantModel):
    id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
    name = models.CharField(max_length=100)
    tenant_id = 'id'

class User(AbstractBaseUser, PermissionsMixin):
    email = models.EmailField(max_length=255, unique=True)
    objects = UserManager()

    USERNAME_FIELD = 'email'

class UserProfile(TenantModel):
    user = models.ForeignKey(User, on_delete=models.CASCADE, related_name='userprofiles')
    school = models.ForeignKey(School, on_delete=models.CASCADE, related_name='members')
    departments = models.ManyToManyField('Department', blank=True, related_name="members")
    is_admin = models.BooleanField(default=False)

    tenant_id = 'school_id'

class Department(TenantModel):
    name = models.CharField(max_length=500, unique=True)
    school = models.ForeignKey(School, on_delete=models.CASCADE)

    tenant_id = 'school_id'

Attempting to delete a Department instance throws the error "TypeError: related_objects() takes 3 positional arguments but 4 were given". This happens even if a tenanted through model is used.

Traceback


  File "/Users/alex/Library/Mobile Documents/com~apple~CloudDocs/Documents/bunsen/backend/schools/tests/test_schools_api.py", line 171, in test_delete_department_successful
    response = self.client.delete(
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/test.py", line 317, in delete
    response = super().delete(
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/test.py", line 219, in delete
    return self.generic('DELETE', path, data, content_type, **extra)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/test.py", line 231, in generic
    return super().generic(
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/django/test/client.py", line 470, in generic
    return self.request(**r)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/test.py", line 283, in request
    return super().request(**kwargs)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/test.py", line 235, in request
    request = super().request(**kwargs)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/django/test/client.py", line 709, in request
    self.check_exception(response)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/django/test/client.py", line 571, in check_exception
    raise exc_value
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/django/core/handlers/base.py", line 179, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/viewsets.py", line 114, in view
    return self.dispatch(request, *args, **kwargs)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/views.py", line 505, in dispatch
    response = self.handle_exception(exc)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/views.py", line 465, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/views.py", line 476, in raise_uncaught_exception
    raise exc
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/views.py", line 502, in dispatch
    response = handler(request, *args, **kwargs)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/mixins.py", line 91, in destroy
    self.perform_destroy(instance)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/rest_framework/mixins.py", line 95, in perform_destroy
    instance.delete()
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/django/db/models/base.py", line 943, in delete
    collector.collect([self], keep_parents=keep_parents)
  File "/Users/alex/.local/share/virtualenvs/backend-xQfT061F/lib/python3.8/site-packages/django/db/models/deletion.py", line 313, in collect
    sub_objs = self.related_objects(related_model, related_fields, batch)
TypeError: related_objects() takes 3 positional arguments but 4 were given```
louiseGrandjonc commented 4 years ago

Hi,

to use ManyToMany with django-multitenant, you need to add a through model. In the citus documentationn, we put an example: https://docs.citusdata.com/en/v9.4/develop/migration_mt_django.html?#introducing-the-tenant-column-to-models-belonging-to-an-account

It's needed to shard the query to retrieve/delete the rows in the table.

What version of django are you using?

louiseGrandjonc commented 4 years ago

Seems to be related to Django 3.X. I'm fixing it and will notify you once it's released

louiseGrandjonc commented 4 years ago

Hi @pickyuptruck, it should be fixed in version 2.3.0

pickyuptruck commented 4 years ago

Works a treat! Thanks very much, and thank you for this brilliant package.

pickyuptruck commented 4 years ago

one final question: are through models essential? Things seem to be working fine without them

louiseGrandjonc commented 4 years ago

The through model is not essential. It's required for distributed databases though. So if you decided to shard your database on the column school_id and wanted to distribute the members relation. If you're not using a distributed database, then it works perfectly fine :)