QueryManager constraints don't apply to RelatedManagers

[jonashaag]

What?

Let's say you have a model like this:

class Article(models.Model):
    author = models.ForeignKey('Author')
    published = models.BooleanField()

    published_only = QueryManager(published=True)

Note that published_only is the default manager here.

Now if you do

an_author.article_set...

the published=True constraint is NOT added to the query.

Why?

When you access a related model that way, Django internally subclasses the model's default manager (db/models/fields/related.py, ForeignRelatedObjectsDescriptor:related_manager_cls) and uses that manager for the query. The issue here is that by the current implementation, django-model-utils' QueryManager keeps the constraints as instance state and not as class state. Which obviously means that the constraints are lost in the Django-generated related manager.

Solution 1

Fix this in Django: The Django-generated manager class should not only extend the default manager's class but the concrete instance. By not using inheritance for example.

Solution 2

Make QueryManager(...) call actually generate a subclass that incorporates the constraints (arguments). Probably easier but doesn't feel "right".

[carljm]

Yes, this was also a problem for PassThroughManager and we fixed it with Solution 2, which as you say is ugly but works (requires some care to avoid breaking pickling). That's certainly possible here as well and I'd accept a pull request for it.

Solution 1 is an interesting proposal for Django; it would certainly make it easier to implement managers that are customized at the instance level and allow them to work for related managers as well. Speaking as a Django core dev, I'd welcome it if you filed a ticket for that at code.djangoproject.com.

[jonashaag]

https://code.djangoproject.com/ticket/20057

I won't code up any of this though.

[rouge8]

This is also still a problem for PassThroughManager unless you use it with the PassThroughManager.for_queryset_class()() syntax. However, if you inherit from PassThroughManager and override get_query_set somewhere in your manager's inheritance tree, you can't use the for_queryset_class style.

I have the general structure of my models in a gist. MyModel.objects returns a queryset with only the latest versions and with the passthrough methods, as it should. However, RelatedManagers don't keep the passthrough methods, as expected. If I use the for_queryset_class style, the get_query_set method of my LatestVersionManager is never called.

Is there a reasonable way to approach this problem?

[carljm]

It might be possible to fix for_queryset_class() so it doesn't ignore the superclass version of get_query_set. Specifically I'm thinking that Queryset._clone() accepts a klass argument to supply a new class for the cloned queryset. So instead of just instantiating and returning a new queryset of class queryset_cls, The get_query_set() method of the dynamically created manager class could instead call super, get its queryset, and then clone it; something like::

def get_query_set(self):
    qs = super(_PassThroughManager, self).get_query_set()
    return qs._clone(klass=queryset_cls)

Not entirely positive this would work, but it might be worth a try.

Really this is all quite ugly and hacky, and the best solution would be for someone to take a serious run at https://code.djangoproject.com/ticket/20057 and fix the problem at the source. That'll be more work.

[rouge8]

Yeah, that works and doesn't break any of the existing tests. Should I make a pull request with that change and a test case?

[carljm]

Looks good to me! A pull request would be great; just add a test, add a note in CHANGES.rst, and add yourself to AUTHORS. I don't think this requires any updates to the documentation in README.rst; it already recommends for_queryset_class().

[gregsadetsky]

Hey, is it possible that the fix was only applied for PassThroughManager and was not applied to QueryManager?

I added a custom manager on Article (to follow the example given here):

class Article(models.Model):
    author = models.ForeignKey('Author')
    published = models.BooleanField()

    objects = models.Manager()
    published_objects = QueryManager(published=True)

When calling author.article_set(manager='published_objects') (following the example from the Django docs), the published=True filtering is not applied.

Thank you!