Django Models, Custom Model Managers and Foreign Key -- don't play well together

Question

Using Django 3.2 -- I will simplify the problem as much as I can.

I have three model classes:

# abstract base class
MyAbstractModel(models.Model)

# derived model classes
Person(MyAbstractModel)
LogoImage(MyAbstractModel)

Each Person has:

image = ForeignKey(LogoImage, db_index=True, related_name="person", null=True, 
                         on_delete=models.PROTECT)

The MyAbstractModel defines a few model managers:

  objects = CustomModelManager()
  objects_all_states = models.Manager()

as well as a state field, that can be either active or inactive

CustomModelManager is defined as something that'll bring only records that have state == 'active':

class CustomModelManager(models.Manager):
    def get_queryset(self):
        return super().get_query().filter(self.model, using=self._db).filter(state='active')

In my database I have two objects in two tables:

Person ID 1 state = 'active'
Image ID 1 state = 'inactive'

Person ID 1 has a foreign key connection to Image ID 1 via the Person.image field.

------ NOW for the issue ----------------

# CORRECT: gives me the person object
person = Person.objects.get(id=1)
# INCORRECT: I get the image, but it should not work... 
image = person.image

Why is that incorrect? because I queried for the person object using the objects model manager which is supposed to bring only those items with active status. It brought the Person which is fine, because Person (ID=1) is state==active -- but the object under person.image is state==inactive. Why am I getting it?

WORKAROND ATTEMPT:

added base_manager_name = "objects" to the MyAbstractModel class Meta: section

ATTEMPTING AGAIN:

# CORRECT: gives me the person object
person = Person.objects.get(id=1)
# CORRECT: gives me a "Does not Exist" exception.  
image = person.image

However..... Now I try this:

# CORRECT: getting the person
person.objects_all_states.get(id=1)
# INCORRECT: throws a DoesNotExist, as it's trying to use the `objects` model manager I hard coded in the `MyAbstractModel` class meta. 
image = person.image

Since I got the Person under the objects_all_states which does not care about state==active -- I expect I would also get the person.image in a similar way. But that doesn't work as expected.

THE ROOT ISSUE

How do I force the same model manager used to fetch the parent object (Person) -- in the fetching of every single ForeignKey object a Person has? I can't find the answer. I've been going in circles for days. There is simply no clear answer anywhere. Either I am missing something very fundamental, or Django has a design flaw (which of course I don't really believe) -- so, what am I missing here?

user_8601760 · Answer

Why they don't play well together

Foreign key classes use separate instances of managers, so there's no shared state.
There's no information about the manager used on the parent instance either.
As per django.db.models.Model._base_manager, Django simply uses _base_manager:
```
return self.field.remote_field.model._base_manager.db_manager(hints=hints).all()
```
...where hints would be {'instance': }.

Since we have a reference to the parent, in some scenarios, we could support this inference.

Fair warning

Django specifically mentions not to do this.

From django.db.models.Model._base_manager:

Don’t filter away any results in this type of manager subclass

This manager is used to access objects that are related to from some other model. In those situations, Django has to be able to see all the objects for the model it is fetching, so that anything which is referred to can be retrieved.

Therefore, you should not override get_queryset() to filter out any rows. If you do so, Django will return incomplete results.

1. How you could implement this inference

You could:

override get() to actively store some information on the instance (that will be passed as hint) about whether an instance of CustomModelManager was used to get it, and then
in get_queryset, check that and try to fallback on objects_all_states.

class CustomModelManager(models.Manager):

    def get(self, *args, **kwargs):
        instance = super().get(*args, **kwargs)
        instance.hint_manager = self
        return instance

    def get_queryset(self):
        hint = self._hints.get('instance')
        if hint and isinstance(hint.__class__.objects, self.__class__):
            hint_manager = getattr(hint, 'hint_manager', None)
            if not hint_manager or not isinstance(hint_manager, self.__class__):
                manager = getattr(self.model, 'objects_all_states', None)
                if manager:
                    return manager.db_manager(hints=self._hints).get_queryset()
        return super().get_queryset().filter(state='active')

Limitations

One of possibly many edge cases where this wouldn't work is if you queried person via Person.objects.filter(id=1).first().

2. Using explicit instance context

Usage:

person = Person.objects_all_states.get(id=1)
# image = person.image
with CustomModelManager.disable_for_instance(person):
    image = person.image

Implementation:

class CustomModelManager(models.Manager):
    _disabled_for_instances = set()

    @classmethod
    @contextmanager
    def disable_for_instance(cls, instance):
        is_already_in = instance in cls._disabled_for_instances
        if not is_already_in:
            cls._disabled_for_instances.add(instance)
        yield
        if not is_already_in:
            cls._disabled_for_instances.remove(instance)

    def get_queryset(self):
        if self._hints.get('instance') in self._disabled_for_instances:
            return super().get_queryset()
        return super().get_queryset().filter(state='active')

3. Using explicit thread-local context

Usage:

# person = Person.objects_all_states.get(id=1)
# image = person.image
with CustomModelManager.disable():
    person = Person.objects.get(id=1)
    image = person.image

Implementation:

import threading
from contextlib import contextmanager

from django.db import models
from django.utils.functional import classproperty


class CustomModelManager(models.Manager):
    _data = threading.local()

    @classmethod
    @contextmanager
    def disable(cls):
        is_disabled = cls._is_disabled
        cls._data.is_disabled = True
        yield
        cls._data.is_disabled = is_disabled

    @classproperty
    def _is_disabled(cls):
        return getattr(cls._data, 'is_disabled', None)

    def get_queryset(self):
        if self._is_disabled:
            return super().get_queryset()
        return super().get_queryset().filter(state='active')

user_7851254 · Answer

Well, i must point out a few design flaws in your approach. First - you should not override get_queryset method for manager. Instead, make a separate method to filter specific cases. Even better if you make a custom QuerySet class with those methods, since then you will able to chain them

class ActiveQuerySet(QuerySet):
    def active(self):
        return self.filter(state="active")

# in your model
objects = ActiveQueryset.as_manager()

Also, you should not place field state in every model and expect, that Django will handle this for you. It will be much easier to handle for you if you decide from domain perspective, which model is your root model and have state there. For example, if Person can be inactive, then probably all of his images are also inactive, so you may safely assume, that Persons status is shared by all related models.

I would specifically look for a way to avoid such issue from design perspective, instead of trying to brutforce Django to process such filtration cases