Merge pull request #478 from winged/fully_self_implemented_hierarchical_queries

refactor: drop django-tree-queries & implement array-based tree functions

Author: open-dynaMIX

emeis/core/migrations/0010_scope_full_name.py

@@ -10,7 +10,7 @@ def set_full_name(apps, schema_editor):
     scope_model = apps.get_model("emeis_core.scope")
     for scope in scope_model.objects.all().iterator():
         # explicitly trigger the set_full_name signal handler
-        models.set_full_name(instance=scope, sender=set_full_name)
+        models.set_full_name_and_parents(instance=scope, sender=set_full_name)
         scope.save()
 
 

emeis/core/migrations/0012_parents_as_arrayfield.py

@@ -0,0 +1,54 @@
+# Generated by Django 3.2.25 on 2024-06-13 07:05
+
+import django.contrib.postgres.fields
+import django.contrib.postgres.indexes
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+def set_all_parents(apps, schema_editor):
+    from emeis.core.models import set_full_name_and_parents
+
+    scope_model = apps.get_model("emeis_core.scope")
+    for scope in scope_model.objects.all().iterator():
+        # explicitly trigger the set_full_name signal handler
+        set_full_name_and_parents(instance=scope, sender=set_all_parents)
+        scope.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("emeis_core", "0011_mptt_to_tree_queries"),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name="scope",
+            options={"ordering": ["name"]},
+        ),
+        migrations.AddField(
+            model_name="scope",
+            name="all_parents",
+            field=django.contrib.postgres.fields.ArrayField(
+                base_field=models.UUIDField(), default=list, size=None
+            ),
+        ),
+        migrations.AlterField(
+            model_name="scope",
+            name="parent",
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name="children",
+                to="emeis_core.scope",
+            ),
+        ),
+        migrations.AddIndex(
+            model_name="scope",
+            index=django.contrib.postgres.indexes.GinIndex(
+                fields=["all_parents"], name="emeis_core__all_par_f8231c_gin"
+            ),
+        ),
+        migrations.RunPython(set_all_parents, migrations.RunPython.noop),
+    ]

emeis/core/models.py

@@ -1,18 +1,20 @@
-import operator
 import unicodedata
 import uuid
-from functools import reduce
 
 from django.conf import settings
 from django.contrib.auth.models import AbstractBaseUser, UserManager
 from django.contrib.auth.validators import UnicodeUsernameValidator
+from django.contrib.postgres.aggregates import ArrayAgg
+from django.contrib.postgres.fields import ArrayField
+from django.contrib.postgres.indexes import GinIndex
+from django.core.exceptions import ValidationError
 from django.db import models
+from django.db.models import Exists, OuterRef, Q, Subquery
 from django.db.models.signals import pre_save
 from django.dispatch import receiver
 from django.utils import timezone, translation
 from django.utils.translation import gettext_lazy as _
 from localized_fields.fields import LocalizedCharField, LocalizedTextField
-from tree_queries.models import TreeNode, TreeQuerySet
 
 
 def make_uuid():
@@ -160,59 +162,34 @@ def is_authenticated(self):
         return True
 
 
-class ScopeQuerySet(TreeQuerySet):
-    # django-tree-queries sadly does not (yet?) support ancestors query
-    # for QS - only for single nodes. So we're providing all_descendants()
-    # and all_ancestors() queryset methods.
-
+class ScopeQuerySet(models.QuerySet):
     def all_descendants(self, include_self=False):
-        """Return a QS that contains all descendants of the given QS.
+        """Return a QS that contains all descendants."""
+        expr = Q(all_parents__overlap=self.aggregate(all_pks=ArrayAgg("pk"))["all_pks"])
 
-        This is a workaround for django-tree-queries, which currently does
-        not support this query (it can only do it on single nodes).
+        if include_self:
+            expr = expr | Q(pk__in=self)
 
-        This is in contrast to .descendants(), which can only give the descendants
-        of one model instance.
-        """
-        descendants_q = reduce(
-            operator.or_,
-            [
-                models.Q(
-                    pk__in=entry.descendants(include_self=include_self).values("pk")
-                )
-                for entry in self
-            ],
-            models.Q(),
-        )
-        return self.model.objects.filter(descendants_q)
+        return Scope.objects.filter(expr)
 
     def all_ancestors(self, include_self=False):
-        """Return a QS that contains all ancestors of the given QS.
+        """Return a QS that contains all ancestors."""
 
-        This is a workaround for django-tree-queries, which currently does
-        not support this query (it can only do it on single nodes).
+        filter_qs = self.filter(all_parents__contains=[OuterRef("pk")])
 
-        This is in contrast to .ancestors(), which can only give the ancestors
-        of one model instance.
-        """
+        new_qs = Scope.objects.all().annotate(_is_ancestor=Exists(Subquery(filter_qs)))
+        expr = Q(_is_ancestor=True)
 
-        descendants_q = reduce(
-            operator.or_,
-            [
-                models.Q(pk__in=entry.ancestors(include_self=include_self).values("pk"))
-                for entry in self
-            ],
-            models.Q(),
-        )
-        return self.model.objects.filter(descendants_q)
+        if include_self:
+            expr = expr | Q(pk__in=self)
+
+        return new_qs.filter(expr)
 
     def all_roots(self):
-        return Scope.objects.all().filter(
-            pk__in=[scope.ancestors(include_self=True).first().pk for scope in self]
-        )
+        return self.all_ancestors(include_self=True).filter(parent__isnull=True)
 
 
-class Scope(TreeNode, UUIDModel):
+class Scope(UUIDModel):
     name = LocalizedCharField(_("scope name"), blank=False, null=False, required=False)
 
     full_name = LocalizedCharField(
@@ -224,26 +201,67 @@ class Scope(TreeNode, UUIDModel):
     )
     is_active = models.BooleanField(default=True)
 
-    objects = ScopeQuerySet.as_manager(with_tree_fields=True)
+    objects = ScopeQuerySet.as_manager()
+
+    parent = models.ForeignKey(
+        "Scope",
+        null=True,
+        blank=True,
+        on_delete=models.CASCADE,
+        related_name="children",
+    )
+
+    all_parents = ArrayField(models.UUIDField(null=False), default=list)
+
+    def ancestors(self, include_self=False):
+        expr = Q(pk__in=self.all_parents)
+        if include_self:
+            expr = expr | Q(pk=self.pk)
+        return Scope.objects.all().filter(expr)
+
+    def descendants(self, include_self=False):
+        expr = Q(all_parents__contains=[self.pk])
+
+        if include_self:
+            expr = expr | Q(pk=self.pk)
+
+        return Scope.objects.all().filter(expr)
 
     def get_root(self):
-        return self.ancestors(include_self=True).first()
+        if self.parent_id:
+            return Scope.objects.get(pk=self.all_parents[0])
+        else:
+            return self
 
     def save(self, *args, **kwargs):
-        # django-tree-queries does validation in TreeNode.clean(), which is not
-        # called by DRF (only by django forms), so we have to do this here
-        self.clean()
+        self._ensure_no_loop()
         return super().save(*args, **kwargs)
 
+    def _ensure_no_loop(self):
+        parent = self.parent
+        while parent:
+            if parent == self:
+                raise ValidationError(
+                    "A node cannot be made a descendant or parent of itself"
+                )
+            parent = parent.parent
+
     def __str__(self):
         return f"{type(self).__name__} ({self.full_name}, pk={self.pk})"
 
     class Meta:
         ordering = ["name"]
+        indexes = [GinIndex(fields=["all_parents"])]
 
 
 @receiver(pre_save, sender=Scope)
-def set_full_name(instance, sender, **kwargs):
+def set_full_name_and_parents(instance, sender, **kwargs):
+    """Update the `full_name` and `all_parents` properties of the Scope.
+
+    The full name depends on the complete list of parents of the Scope.
+    And to ensure correct behaviour in the queries, the `all_parents`
+    attribute needs to be updated as well
+    """
     if kwargs.get("raw"):  # pragma: no cover
         # Raw is set while loading fixtures. In those
         # cases we don't want to mess with data that
@@ -255,6 +273,9 @@ def set_full_name(instance, sender, **kwargs):
 
     forced_lang = settings.EMEIS_FORCE_MODEL_LOCALE.get("scope", None)
 
+    old_all_parents = [*instance.all_parents]
+    old_full_name = {**instance.full_name}
+
     if forced_lang:
         # If scope is forced monolingual, do not fill non-forced language fields
         languages = [forced_lang]
@@ -263,25 +284,34 @@ def set_full_name(instance, sender, **kwargs):
         with translation.override(lang):
             instance.full_name[lang] = str(instance.name)
 
+    parent_ids = []
     parent = instance.parent
     while parent:
+        parent_ids.append(parent.pk)
         for lang in languages:
             with translation.override(lang):
                 new_fullname = f"{parent.name} {sep} {instance.full_name[lang]}"
                 instance.full_name[lang] = new_fullname
         parent = parent.parent
 
+    # make it root-first
+    parent_ids.reverse()
+    instance.all_parents = parent_ids
+
     if forced_lang:
         # Ensure only the "forced" language full_name is set, and nothing else
         full_name = instance.full_name[forced_lang]
         instance.full_name.clear()
         instance.full_name[forced_lang] = full_name
 
-    # Force update of all children (recursively)
-    for child in instance.children.all():
-        # save() triggers the set_full_name signal handler, which will
-        # recurse all the way down, updating the full_name
-        child.save()
+    if old_all_parents != instance.all_parents or old_full_name != dict(
+        instance.full_name
+    ):
+        # Something changed - force update all children (recursively)
+        for child in instance.children.all():
+            # save() triggers the signal handler, which will
+            # recurse all the way down, updating the full_name
+            child.save()
 
 
 class Role(SlugModel):

emeis/core/serializers.py

@@ -80,24 +80,12 @@ class ScopeSerializer(BaseSerializer):
     level = serializers.SerializerMethodField()
 
     def get_level(self, obj):
-        depth = getattr(obj, "tree_depth", None)
-        if depth is not None:
-            return depth
-
-        # Note: This should only happen on CREATE, never in GET (Either list,
-        # detail, or include!) In CREATE, it's a new object that doesn't come
-        # from a QS
-
-        # Sometimes, the model object may come out of a non-django-tree-queries
-        # QS, and thus would not have the `tree_*` attributes amended. Then we
-        # need to go the "slow path"
         if not obj.pk and obj.parent_id:
-            # unsaved object, sometimes used in unit tests etc
+            # unsaved object, sometimes used in unit tests etc. Can't rely on
+            # `all_parents` being set just yet
             return self.get_level(obj.parent) + 1
 
-        if obj.parent_id:
-            return obj.ancestors().count()
-        return 0
+        return len(obj.all_parents)
 
     class Meta:
         model = Scope