Merge pull request #177 from netzkolchose/revamp_m2m_handling

through model expansion on m2m fields

Author: jerch

README.md

@@ -7,7 +7,7 @@
 django-computedfields provides autoupdated database fields
 for model methods.
 
-Tested with Django 3.2 and 4.2 (Python 3.8 to 3.11).
+Tested with Django 4.2 and 5.2 (Python 3.8 to 3.13).
 
 
 #### Example

computedfields/graph.py

@@ -60,6 +60,12 @@ class ILocalMroData(TypedDict):
     fields: Dict[str, int]
 
 
+class IM2mData(TypedDict):
+    left: str
+    right: str
+IM2mMap = Dict[Type[Model], IM2mData]
+
+
 # global deps: {cfModel: {cfname: {srcModel: {'path': lookup_path, 'depends': src_fieldname}}}}
 IGlobalDeps = Dict[Type[Model], Dict[str, Dict[Type[Model], List[IDependsData]]]]
 # local deps: {Model: {'cfname': {'depends', 'on', 'these', 'local', 'fieldnames'}}}
@@ -454,6 +460,7 @@ def __init__(self, computed_models: Dict[Type[Model], Dict[str, IComputedField]]
         ``computed_models`` is ``Resolver.computed_models``.
         """
         super(ComputedModelsGraph, self).__init__()
+        self._m2m: IM2mMap = {}
         self._computed_models: Dict[Type[Model], Dict[str, IComputedField]] = computed_models
         self.models: Dict[str, Type[Model]] = {}
         self.resolved: IResolvedDeps = self.resolve_dependencies(computed_models)
@@ -473,6 +480,40 @@ def _right_constrain(self, model: Type[Model], fieldname: str) -> None:
         f = model._meta.get_field(fieldname)
         if not f.concrete or f.many_to_many:
             raise ComputedFieldsException(f"{model} has no concrete field named '{fieldname}'")
+    
+    def _expand_m2m(self, model: Type[Model], path: str) -> str:
+        """
+        Expand M2M dependencies into through model.
+        """
+        cls: Type[Model] = model
+        symbols: list[str] = []
+        for symbol in path.split('.'):
+            try:
+                rel: Any = cls._meta.get_field(symbol)
+            except FieldDoesNotExist:
+                descriptor = getattr(cls, symbol)
+                rel = getattr(descriptor, 'rel', None) or getattr(descriptor, 'related')
+            if rel.many_to_many:
+                if hasattr(rel, 'through'):
+                    through = rel.through
+                    m2m_field_name = rel.remote_field.m2m_field_name()
+                    m2m_reverse_field_name = rel.remote_field.m2m_reverse_field_name()
+                    symbols.append(through._meta.get_field(m2m_reverse_field_name).related_query_name())
+                    symbols.append(m2m_field_name)
+                else:
+                    through = rel.remote_field.through
+                    m2m_field_name = rel.m2m_field_name()
+                    m2m_reverse_field_name = rel.m2m_reverse_field_name()
+                    symbols.append(through._meta.get_field(m2m_field_name).related_query_name())
+                    symbols.append(m2m_reverse_field_name)
+                self._m2m[through] = {
+                    'left': m2m_field_name,
+                    'right': m2m_reverse_field_name
+                }
+            else:
+                symbols.append(symbol)
+            cls = rel.related_model
+        return '.'.join(symbols)
 
     def resolve_dependencies(
         self,
@@ -489,8 +530,7 @@ def resolve_dependencies(
 
         - fk relations are added on the model holding the fk field
         - reverse fk relations are added on related model holding the fk field
-        - m2m fields and backrelations are added on the model directly, but
-          only used for inter-model resolving, never for field lookups during ``save``
+        - m2m fields are expanded via their through model
         """
         global_deps: IGlobalDeps = OrderedDict()
         local_deps: ILocalDeps = {}
@@ -543,17 +583,15 @@ def resolve_dependencies(
                             self._right_constrain(model, fieldname)
                         local_deps.setdefault(model, {}).setdefault(field, set()).update(fieldnames)
                         continue
+
+                    # expand m2m into through model
+                    path = self._expand_m2m(model, path)
+
                     path_segments: List[str] = []
                     cls: Type[Model] = model
                     for symbol in path.split('.'):
                         try:
                             rel: Any = cls._meta.get_field(symbol)
-                            if rel.many_to_many:
-                                # add dependency to m2m relation fields
-                                path_segments.append(symbol)
-                                fieldentry.setdefault(rel.related_model, []).append(
-                                    {'path': '__'.join(path_segments), 'depends': rel.remote_field.name})
-                                path_segments.pop()
                         except FieldDoesNotExist:
                             # handle reverse relation (not a concrete field)
                             descriptor = getattr(cls, symbol)

computedfields/handlers.py

@@ -15,7 +15,7 @@
 from .settings import settings
 
 # typing imports
-from typing import Any, Dict, Iterable, List, Set, Type, cast
+from typing import Iterable, Type, cast
 from django.db.models import Model
 
 
@@ -105,116 +105,58 @@ def postdelete_handler(sender: Type[Model], instance: Model, **kwargs) -> None:
                     querysize=settings.COMPUTEDFIELDS_QUERYSIZE
                 )
 
-
-def merge_pk_maps(
-    obj1: Dict[Type[Model], List[Any]],
-    obj2: Dict[Type[Model], List[Any]]
-) -> Dict[Type[Model], List[Any]]:
-    """
-    Merge pk map in `obj2` on `obj1`.
-    Updates obj1 inplace and also returns it.
-    """
-    for model, data in obj2.items():
-        m2_pks, m2_fields = data
-        m1_pks, m1_fields = obj1.setdefault(model, [set(), set()])
-        m1_pks.update(m2_pks)
-        m1_fields.update(m2_fields)
-    return obj1
-
-
-def merge_qs_maps(
-    obj1: Dict[Type[Model], List[Any]],
-    obj2: Dict[Type[Model], List[Any]]
-) -> Dict[Type[Model], List[Any]]:
-    """
-    Merge queryset map in `obj2` on `obj1`.
-    Updates obj1 inplace and also returns it.
-    """
-    for model, [qs2, fields2] in obj2.items():
-        query_field = obj1.setdefault(model, [model._base_manager.none(), set()])
-        query_field[0] = query_field[0].union(qs2)  # or'ed querysets
-        query_field[1].update(fields2)              # add fields
-    return obj1
-
 # M2M tests: test_full.tests.test05_m2m test_full.tests.test06_m2mback test_full.tests.test_43.TestBetterM2M test_full.tests.test_m2m_advanced test_full.tests.test_norelated.TestNoReverse test_full.tests.test_proxymodels.TestProxyModelsM2M
 def m2m_handler(sender: Type[Model], instance: Model, **kwargs) -> None:
     """
     ``m2m_change`` handler.
 
     Works like the other handlers but on the corresponding
     m2m actions.
-
-    .. NOTE::
-        The handler triggers updates for both ends of the m2m relation,
-        which might lead to massive database interaction.
     """
     fields = active_resolver._m2m.get(sender)
     # exit early if we have no update rule on the through model
     if not fields:
         return
 
-    # since the graph does not handle the m2m through model
-    # we have to trigger updates for both ends (left and right side)
     reverse = kwargs['reverse']
     left = fields['right'] if reverse else fields['left']   # fieldname on instance
     right = fields['left'] if reverse else fields['right']  # fieldname on model
     action = kwargs.get('action')
-    model = kwargs['model']
 
     if action == 'post_add':
-        pks_add: Set[Any] = kwargs['pk_set']
-        data_add: Dict[Type[Model], List[Any]] = active_resolver._querysets_for_update(
-            type(instance), instance, update_fields={left})
-        other_add: Dict[Type[Model], List[Any]] = active_resolver._querysets_for_update(
-            model, model._base_manager.filter(pk__in=pks_add), update_fields={right}, m2m=instance)
-        if other_add:
-            merge_qs_maps(data_add, other_add)
-        if data_add:
-            with transaction.atomic():
-                for queryset, update_fields in data_add.values():
-                    active_resolver.bulk_updater(
-                        queryset,
-                        update_fields,
-                        querysize=settings.COMPUTEDFIELDS_QUERYSIZE
-                    )
+        active_resolver.update_dependent(
+            sender.objects.filter(**{left: instance.pk, right+'__in': kwargs['pk_set']}),
+            sender,
+            update_local=False,
+            querysize=settings.COMPUTEDFIELDS_QUERYSIZE
+        )
 
     elif action == 'pre_remove':
-        pks_remove: Set[Any] = kwargs['pk_set']
-        data_remove: Dict[Type[Model], List[Any]] = active_resolver._querysets_for_update(
-            type(instance), instance, update_fields={left}, pk_list=True)
-        other_remove: Dict[Type[Model], List[Any]] = active_resolver._querysets_for_update(
-            model, model._base_manager.filter(pk__in=pks_remove), update_fields={right}, pk_list=True, m2m=instance)
-        if other_remove:
-            merge_pk_maps(data_remove, other_remove)
-        if data_remove:
-            get_M2M_REMOVE()[instance] = data_remove
+        get_M2M_REMOVE()[instance] = active_resolver.preupdate_dependent(
+            sender.objects.filter(**{left: instance.pk, right+'__in': kwargs['pk_set']})
+        )
 
     elif action == 'post_remove':
-        updates_remove: Dict[Type[Model], List[Any]] = get_M2M_REMOVE().pop(instance, None)
-        if updates_remove:
+        old = get_M2M_REMOVE().pop(instance, None)
+        if old:
             with transaction.atomic():
-                for _model, [pks, update_fields] in updates_remove.items():
+                for _model, [pks, update_fields] in old.items():
                     active_resolver.bulk_updater(
                         _model._base_manager.filter(pk__in=pks),
                         update_fields,
                         querysize=settings.COMPUTEDFIELDS_QUERYSIZE
                     )
 
     elif action == 'pre_clear':
-        data: Dict[Type[Model], List[Any]] = active_resolver._querysets_for_update(
-            type(instance), instance, update_fields={left}, pk_list=True)
-        other: Dict[Type[Model], List[Any]] = active_resolver._querysets_for_update(
-            model, getattr(instance, left).all(), update_fields={right}, pk_list=True, m2m=instance)
-        if other:
-            merge_pk_maps(data, other)
-        if data:
-            get_M2M_CLEAR()[instance] = data
+        get_M2M_CLEAR()[instance] = active_resolver.preupdate_dependent(
+            sender.objects.filter(**{left: instance.pk})
+        )
 
     elif action == 'post_clear':
-        updates_clear: Dict[Type[Model], List[Any]] = get_M2M_CLEAR().pop(instance, None)
-        if updates_clear:
+        old = get_M2M_CLEAR().pop(instance, None)
+        if old:
             with transaction.atomic():
-                for _model, [pks, update_fields] in updates_clear.items():
+                for _model, [pks, update_fields] in old.items():
                     active_resolver.bulk_updater(
                         _model._base_manager.filter(pk__in=pks),
                         update_fields,

computedfields/helpers.py

@@ -85,5 +85,6 @@ def proxy_to_base_model(proxymodel: Type[Model]) -> Union[Type[Model], None]:
             return m
     return None
 
+
 def are_same(*args) -> bool:
     return len(set(args)) == 1

computedfields/management/commands/_helpers.py

@@ -44,7 +44,7 @@ def retrieve_computed_models(app_labels):
 
 def retrieve_models(app_labels):
     if not app_labels:
-        return apps.get_models()
+        return set(apps.get_models())
     considered = set()
     for label in app_labels:
         app_model = label.split('.')

computedfields/management/commands/showdependencies.py

@@ -23,11 +23,17 @@ def add_arguments(self, parser):
 
     def handle(self, *app_labels, **options):
         models = retrieve_models(app_labels)
+        auto_through = set()
+        for through in active_resolver._m2m.keys():
+            if not through in models:
+                models.add(through)
+                auto_through.add(through)
         for model in models:
             if not model in active_resolver._map:
                 print(f'- {modelname(model)}: None')
                 continue
-            print(f'- {self.style.MIGRATE_LABEL(modelname(model))}:')
+            through_msg = ' (auto-generated through)' if model in auto_through else ''
+            print(f'- {self.style.MIGRATE_LABEL(modelname(model))}{through_msg}:')
             for source_field, targets in active_resolver._map[model].items():
                 real_source_field = model._meta.get_field(source_field)
                 if real_source_field.is_relation and not real_source_field.concrete: