citusdata/django-multitenant

__set_attr__ of TenantModelMixin breaks migrations

s4ke opened this issue · 7 comments

s4ke commented

Hey,

With the current version of django-multitenant, we are having problems with using classes with the mixin in migrations. Basically, the issue is that set_attr does not play well with objects that are resolved like this:

    Model = apps.get_model('model', 'Model')
    for _model in Model.all_objects.all():
        ...

The reason for this is that Django resolves the classes one by one and when resolving the TenantModelMixin it calls set_attr differently than in normal mode. Then self.tenant_field somehow triggers a Django reload from database which that then makes everything crash:

File "/<snip>/venv/lib/python3.8/site-packages/django/db/models/query.py", line 287, in __iter__
    self._fetch_all()
  File "/<snip>/venv/lib/python3.8/site-packages/django/db/models/query.py", line 1308, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/<snip>/venv/lib/python3.8/site-packages/django/db/models/query.py", line 71, in __iter__
    obj = model_cls.from_db(db, init_list, row[model_fields_start:model_fields_end])
  File "/<snip>/venv/lib/python3.8/site-packages/django/db/models/base.py", line 513, in from_db
    new = cls(*values)
  File "/<snip>/venv/lib/python3.8/site-packages/django_multitenant/mixins.py", line 58, in __init__
    super(TenantModelMixin, self).__init__(*args, **kwargs)
  File "/<snip>/venv/lib/python3.8/site-packages/django/db/models/base.py", line 416, in __init__
    self._state = ModelState()
  File "/<snip>/venv/lib/python3.8/site-packages/django_multitenant/mixins.py", line 62, in __setattr__
    if (attrname in (self.tenant_field, get_tenant_field(self).name)
  File "/<snip>/venv/lib/python3.8/site-packages/django_multitenant/mixins.py", line 115, in tenant_field
    return self.tenant_id
  File "/<snip>/venv/lib/python3.8/site-packages/django/db/models/query_utils.py", line 149, in __get__
    instance.refresh_from_db(fields=[field_name])
  File "/<snip>/venv/lib/python3.8/site-packages/django/db/models/base.py", line 623, in refresh_from_db
    db_instance_qs = self.__class__._base_manager.db_manager(using, hints=hints).filter(pk=self.pk)
  File "/<snip>/venv/lib/python3.8/site-packages/django/db/models/base.py", line 573, in _get_pk_val
    return getattr(self, meta.pk.attname)
  File "/<snip>/venv/lib/python3.8/site-packages/django/db/models/query_utils.py", line 147, in __get__
    val = self._check_parent_chain(instance)
  File "/<snip>/venv/lib/python3.8/site-packages/django/db/models/query_utils.py", line 163, in _check_parent_chain
    return getattr(instance, link_field.attname)
AttributeError: 'NoneType' object has no attribute 'attname'

To reproduce, you can simply create a migration and try to use the model either via all_objects or via any constructing method (e.g. Model.objects.create).

My workaround for now is to run this code at the top of my urls.py:

def monkey_patch_multitenant() -> None:

    def __setattr__(self: Any, attrname: Any, val: Any) -> Any:
        if (
            attrname in ('tenant_id', 'tenant')
            and not self._state.adding
            and val
            and self.tenant_value
            and val != self.tenant_value
            and val != self.tenant_object
        ):
            self._try_update_tenant = True

        return super(TenantModelMixin, self).__setattr__(attrname, val)

    TenantModelMixin.__setattr__ = __setattr__

    @property
    def tenant_field(self) -> Any:
        return 'tenant_id'

    TenantModelMixin.tenant_field = tenant_field

    @property
    def tenant_value(self) -> Any:
        return self.tenant_id

    TenantModelMixin.tenant_value = tenant_value

    @property
    def tenant_object(self) -> Any:
        return self.tenant

    TenantModelMixin.tenant_object = tenant_object

This is obviously not the fix for everyone, but i think illustrates the issue rather nicely.

s4ke commented

This concerns Django 3.1 compatibility.

Running into the same issue when trying to add django_multitenant to project.

Only spent a few minutes with @s4ke's workaround, haven't gotten to work yet.

For clarification, @s4ke, run your workaround in your urls.py (root level? app level? haven't had luck with either location)

s4ke commented

@quinceleaf I added it in urls.py

s4ke commented

But in the end it varies on your import order. You have to run this as soon as possible so that models are not initialized yet.

@quinceleaf Did u manage to solve this issue? Tried the solution by s4ke but doesnt seem to work for me either.

@s4ke @Keekteng @quinceleaf started working on the issue. However, as a workaraound, you can check this issue
#138

I added a test which shows this issue does not exist any more.
PR that tests this case
#152