Skip to content

Commit

Permalink
Better handle model families with abstracts in them
Browse files Browse the repository at this point in the history
Should not try to call `family_profile()` for an abstract models
anymore. Also better works for multiple inheritance multi-table models,
which are not supported anyway though.

Closes Suor#456.
  • Loading branch information
Suor committed May 19, 2023
1 parent 762eb04 commit 801b0fa
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 17 deletions.
2 changes: 1 addition & 1 deletion cacheops/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def model_profile(model):
"""
Returns cacheops profile for a model
"""
assert not model._meta.abstract, "This should be handled by caller"
assert not model._meta.abstract, "Can't get profile for %s" % model
# Django migrations create lots of fake models, just skip them
if model.__module__ == '__fake__':
return None
Expand Down
25 changes: 13 additions & 12 deletions cacheops/utils.py
Original file line number Diff line number Diff line change
@@ -1,33 +1,34 @@
import re
import json
import inspect
from funcy import memoize, compose, wraps, any, any_fn, select_values, lmapcat
from funcy import memoize, compose, wraps, any, any_fn, select_values, mapcat

from django.db import models
from django.http import HttpRequest

from .conf import model_profile


def get_table_model(model):
return next((b for b in model.__mro__ if issubclass(b, models.Model) and b is not models.Model
and not b._meta.proxy and not b._meta.abstract), None)

def model_family(model):
"""
The family is models sharing a database table, events on one should affect each other.
We simply collect a list of all proxy models, including subclasess, superclasses and siblings.
Two descendants of an abstract model are not family - they cannot affect each other.
"""
def class_tree(cls):
return [cls] + lmapcat(class_tree, cls.__subclasses__())

# NOTE: we also list multitable submodels here, we just don't care.
# Cacheops doesn't support them anyway.
table_model = get_table_model(model)
return class_tree(table_model) if table_model else []
if model._meta.abstract: # No table - no family
return set()

@memoize
def class_tree(cls):
# NOTE: we also list multitable submodels here, we just don't care.
# Cacheops doesn't support them anyway.
return {cls} | set(mapcat(class_tree, cls.__subclasses__()))

table_bases = {b for b in model.__mro__ if issubclass(b, models.Model) and b is not models.Model
and not b._meta.proxy and not b._meta.abstract}
family = set(mapcat(class_tree, table_bases))
return {cls for cls in family if not cls._meta.abstract}

@memoize
def family_has_profile(cls):
Expand Down
32 changes: 32 additions & 0 deletions tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,38 @@ def _curried(*moreargs, **morekwargs):
name = models.CharField(max_length=255)


# Abstract models
class Abs(models.Model):
class Meta:
abstract = True

class Concrete1(Abs):
pass

class AbsChild(Abs):
class Meta:
abstract = True

class Concrete2(AbsChild):
pass

class NoProfile(models.Model):
title = models.CharField(max_length=128)

class NoProfileProxy(NoProfile):
class Meta:
proxy = True

class AbsNoProfile(NoProfile):
class Meta:
abstract = True

class NoProfileChild(AbsNoProfile):
pass


class Mess(Post, Category):
pass

class MessChild(Mess):
pass
1 change: 1 addition & 0 deletions tests/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
'tests.*': {},
'tests.noncachedvideoproxy': None,
'tests.noncachedmedia': None,
'tests.noprofile': None,
'auth.*': {},
}

Expand Down
23 changes: 19 additions & 4 deletions tests/test_extras.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from cacheops.signals import cache_read, cache_invalidated

from .utils import BaseTestCase, make_inc
from .models import Post, Category, Local, DbAgnostic, DbBinded, Abs
from .models import Post, Category, Local, DbAgnostic, DbBinded


class SettingsTests(TestCase):
Expand Down Expand Up @@ -185,7 +185,22 @@ def test_db_agnostic_disabled(self):
list(DbBinded.objects.cache().using('slave'))


def test_abstract_family():
def test_model_family():
from cacheops.utils import model_family

assert model_family(Abs) == []
from .models import Abs, Concrete1, AbsChild, Concrete2
from .models import NoProfile, NoProfileProxy, AbsNoProfile, NoProfileChild, Mess, MessChild

# Abstract models do not have family, children of an abstract model are not a family
assert model_family(Abs) == set()
assert model_family(Concrete1) == {Concrete1}
assert model_family(AbsChild) == set()
assert model_family(Concrete2) == {Concrete2}

# Everything in but an abstract model
assert model_family(NoProfile) == {NoProfile, NoProfileProxy, NoProfileChild}
assert model_family(NoProfileProxy) == {NoProfile, NoProfileProxy, NoProfileChild}
assert model_family(AbsNoProfile) == set()
assert model_family(NoProfileChild) == {NoProfile, NoProfileProxy, NoProfileChild}

# The worst of multiple inheritance
assert model_family(Mess) == {Mess, MessChild, Post, Category}

0 comments on commit 801b0fa

Please sign in to comment.