Skip to content

Commit

Permalink
feat(scopes): denormalize full_name and use it for sorting
Browse files Browse the repository at this point in the history
The full_name attribute on the Scopes was dynamically generated on access.
This is rather inefficient, as it may cause a bunch of DB hits, especially
in deep hierarchies.

Additionally, most of the time we want scopes to be sorted in a
meaningful manner (namely, by hierarchical name). MPTTModel supports
this, but as the ordering may differ between languages, we cannot use
that functionality. This is a secondary use of the denormalized full_name.

This also required a fix in the case insensitive ordering code, as
localized fields were not treated correctly yet (casting directly to
text instead of extracting the correct language and using that to sort)
  • Loading branch information
winged committed Sep 23, 2022
1 parent d919402 commit 4921028
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 48 deletions.
16 changes: 15 additions & 1 deletion emeis/core/filters.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
from django.conf import settings
from django.contrib.postgres.fields.hstore import KeyTransform
from django.core.exceptions import FieldDoesNotExist
from django.db.models import TextField
from django.db.models.functions import Cast, Lower
from django.utils import translation
from django_filters import FilterSet
from django_filters.filters import CharFilter
from localized_fields.fields import LocalizedField
from rest_framework import filters

from emeis.core.models import ACL, Scope, User
Expand Down Expand Up @@ -94,8 +98,18 @@ def _make_ordering_field(self, field, view):
desc = field.startswith("-")
field_name = field[1:] if desc else field

field_col = field_name
try:
model_field = view.queryset.model._meta.get_field(field_name)
if isinstance(model_field, LocalizedField):
field_col = KeyTransform(translation.get_language(), field_name)
except FieldDoesNotExist:
# This happens with metainfo__foobar style lookups,
# which we don't need to worry about here
pass

if field_name in self._get_case_insensitive_fields(view):
field_expr = Lower(Cast(field_name, output_field=TextField()))
field_expr = Lower(Cast(field_col, output_field=TextField()))
return field_expr.desc() if desc else field_expr
return field

Expand Down
32 changes: 32 additions & 0 deletions emeis/core/migrations/0010_scope_full_name.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Generated by Django 3.2.15 on 2022-09-23 09:13

from django.db import migrations
import localized_fields.fields.char_field

from emeis.core import models


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)
scope.save()


class Migration(migrations.Migration):

dependencies = [
("emeis_core", "0009_alter_scope_parent"),
]

operations = [
migrations.AddField(
model_name="scope",
name="full_name",
field=localized_fields.fields.char_field.LocalizedCharField(
blank=True, null=True, required=[], verbose_name="scope name"
),
),
migrations.RunPython(set_full_name, migrations.RunPython.noop),
]
33 changes: 23 additions & 10 deletions emeis/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
from django.contrib.auth.models import AbstractBaseUser, UserManager
from django.contrib.auth.validators import UnicodeUsernameValidator
from django.db import models
from django.db.models.signals import pre_save
from django.dispatch import receiver
from django.utils import timezone
from django.utils.translation import gettext_lazy as _
from localized_fields.fields import LocalizedCharField, LocalizedTextField
Expand Down Expand Up @@ -158,6 +160,11 @@ def is_authenticated(self):

class Scope(MPTTModel, UUIDModel):
name = LocalizedCharField(_("scope name"), blank=False, null=False, required=False)

full_name = LocalizedCharField(
_("scope name"), blank=True, null=True, required=False
)

description = LocalizedTextField(
_("scope description"), null=True, blank=True, required=False
)
Expand All @@ -170,19 +177,25 @@ class Scope(MPTTModel, UUIDModel):
)
is_active = models.BooleanField(default=True)

def full_name(self, sep="\u00bb", language=None):
"""Return full name of the scope, including parent scopes."""
own_name = str(self.name) if language is None else self.name[language]
def __str__(self):
return f"{type(self).__name__} ({self.full_name}, pk={self.pk})"

if self.parent:
parent_name = self.parent.full_name(sep, language)
return f"{parent_name} {sep} {own_name}"

return own_name
@receiver(pre_save, sender=Scope)
def set_full_name(instance, sender, **kwargs):
sep = "\u00bb"

def __str__(self):
name = self.full_name()
return f"{type(self).__name__} ({name}, pk={self.pk})"
languages = [lang for lang, _ in settings.LANGUAGES]

instance.full_name = {lang: instance.name[lang] for lang in languages}

parent = instance.parent
while parent:
instance.full_name = {
lang: f"{parent.name[lang]} {sep} {instance.full_name[lang]}"
for lang in languages
}
parent = parent.parent


class Role(SlugModel):
Expand Down
10 changes: 1 addition & 9 deletions emeis/core/serializers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from django.conf import settings
from django.contrib.auth.models import AnonymousUser
from generic_permissions.validation import ValidatorMixin
from rest_framework_json_api import serializers
Expand Down Expand Up @@ -74,14 +73,6 @@ class Meta:


class ScopeSerializer(BaseSerializer):
full_name = serializers.SerializerMethodField()

def get_full_name(self, instance):
return {
language: instance.full_name(language=language)
for language, _ in settings.LANGUAGES
}

class Meta:
model = Scope
fields = BaseSerializer.Meta.fields + (
Expand All @@ -92,6 +83,7 @@ class Meta:
"full_name",
"is_active",
)
read_only_fields = ["full_name"]


class PermissionSerializer(BaseSerializer):
Expand Down
Loading

0 comments on commit 4921028

Please sign in to comment.