From 801b0fa9b3644654d1bd19322b57dda2ab3bae56 Mon Sep 17 00:00:00 2001 From: Alexander Schepanovski Date: Fri, 19 May 2023 22:41:45 +0700 Subject: [PATCH] Better handle model families with abstracts in them 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 #456. --- cacheops/conf.py | 2 +- cacheops/utils.py | 25 +++++++++++++------------ tests/models.py | 32 ++++++++++++++++++++++++++++++++ tests/settings.py | 1 + tests/test_extras.py | 23 +++++++++++++++++++---- 5 files changed, 66 insertions(+), 17 deletions(-) diff --git a/cacheops/conf.py b/cacheops/conf.py index 8940998e..32e5b6fa 100644 --- a/cacheops/conf.py +++ b/cacheops/conf.py @@ -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 diff --git a/cacheops/utils.py b/cacheops/utils.py index c7d85143..cd71c6ea 100644 --- a/cacheops/utils.py +++ b/cacheops/utils.py @@ -1,7 +1,7 @@ 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 @@ -9,10 +9,6 @@ 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. @@ -20,14 +16,19 @@ def model_family(model): 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): diff --git a/tests/models.py b/tests/models.py index fb6fa8ff..c5b9c352 100644 --- a/tests/models.py +++ b/tests/models.py @@ -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 diff --git a/tests/settings.py b/tests/settings.py index f480272c..3ec6945a 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -111,6 +111,7 @@ 'tests.*': {}, 'tests.noncachedvideoproxy': None, 'tests.noncachedmedia': None, + 'tests.noprofile': None, 'auth.*': {}, } diff --git a/tests/test_extras.py b/tests/test_extras.py index 959eefdf..04afb9fa 100644 --- a/tests/test_extras.py +++ b/tests/test_extras.py @@ -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): @@ -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}