From 44ef464e22ff7d214e4cdb4c309932ea46df2fa4 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Fri, 21 Feb 2025 15:58:48 +0100 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(back)=20migrate=20from=20dja?= =?UTF-8?q?ngo=5Ftreebeard=20to=20native=20postgres=20ltree?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Django has a native extension able to manage records in a hierchical way combined with an index allowing to have high performance when querying the database. The django_ltree lib is used, it adds some helper but is really lightweight --- Dockerfile | 3 +- src/backend/.pylintrc | 2 +- src/backend/core/admin.py | 6 +- src/backend/core/api/serializers.py | 15 +- src/backend/core/api/utils.py | 4 +- src/backend/core/api/viewsets.py | 99 ++++----- src/backend/core/factories.py | 36 +-- src/backend/core/migrations/0001_initial.py | 1 + ...item_options_remove_item_depth_and_more.py | 36 +++ src/backend/core/models.py | 137 +++++++----- .../items/test_api_items_children_list.py | 28 +-- .../core/tests/items/test_api_items_list.py | 45 ++-- .../core/tests/items/test_api_items_move.py | 100 ++------- .../tests/items/test_api_items_retrieve.py | 28 ++- .../tests/items/test_api_items_trashbin.py | 14 +- .../tests/test_api_utils_filter_root_paths.py | 104 ++++----- .../core/tests/test_models_invitations.py | 2 +- src/backend/core/tests/test_models_items.py | 209 ++++++++++++++++-- src/backend/drive/settings.py | 2 +- src/backend/pyproject.toml | 10 +- 20 files changed, 524 insertions(+), 357 deletions(-) create mode 100644 src/backend/core/migrations/0002_alter_item_options_remove_item_depth_and_more.py diff --git a/Dockerfile b/Dockerfile index 85afa1ea..4aa256ec 100644 --- a/Dockerfile +++ b/Dockerfile @@ -8,7 +8,8 @@ RUN python -m pip install --upgrade pip setuptools # Upgrade system packages to install security updates RUN apk update && \ - apk upgrade + apk upgrade && \ + apk add git # ---- Back-end builder image ---- FROM base AS back-builder diff --git a/src/backend/.pylintrc b/src/backend/.pylintrc index 9807de7d..1aeec307 100644 --- a/src/backend/.pylintrc +++ b/src/backend/.pylintrc @@ -229,7 +229,7 @@ redefining-builtins-modules=six.moves,past.builtins,future.builtins expected-line-ending-format= # Regexp for a line that is allowed to be longer than the limit. -ignore-long-lines=^\s*(# )??$ +ignore-long-lines=^\s*(#\s*)??$ # Number of spaces of indent required inside a hanging or continued line. indent-after-paren=4 diff --git a/src/backend/core/admin.py b/src/backend/core/admin.py index ecf9e0e7..25064b00 100644 --- a/src/backend/core/admin.py +++ b/src/backend/core/admin.py @@ -4,9 +4,6 @@ from django.contrib import admin from django.contrib.auth import admin as auth_admin from django.utils.translation import gettext_lazy as _ -from treebeard.admin import TreeAdmin -from treebeard.forms import movenodeform_factory - from . import models @@ -105,7 +102,7 @@ class ItemAccessInline(admin.TabularInline): @admin.register(models.Item) -class ItemAdmin(TreeAdmin): +class ItemAdmin(admin.ModelAdmin): """item admin interface declaration.""" fieldsets = ( @@ -139,7 +136,6 @@ class ItemAdmin(TreeAdmin): }, ), ) - form = movenodeform_factory(models.Item) inlines = (ItemAccessInline,) list_display = ( "id", diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 9d1e894a..0ec93912 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -6,7 +6,7 @@ from django.utils.translation import gettext_lazy as _ from rest_framework import exceptions, serializers -from core import enums, models +from core import models from core.api import utils @@ -429,28 +429,15 @@ class MoveItemSerializer(serializers.Serializer): Fields: - target_item_id (UUIDField): The ID of the target parent item where the item should be moved. This field is required and must be a valid UUID. - - position (ChoiceField): Specifies the position of the item in relation to - the target parent's children. - Choices: - - "first-child": Place the item as the first child of the target parent. - - "last-child": Place the item as the last child of the target parent (default). - - "left": Place the item as the left sibling of the target parent. - - "right": Place the item as the right sibling of the target parent. Example: Input payload for moving a item: { "target_item_id": "123e4567-e89b-12d3-a456-426614174000", - "position": "first-child" } Notes: - The `target_item_id` is mandatory. - - The `position` defaults to "last-child" if not provided. """ target_item_id = serializers.UUIDField(required=True) - position = serializers.ChoiceField( - choices=enums.MoveNodePositionChoices.choices, - default=enums.MoveNodePositionChoices.LAST_CHILD, - ) diff --git a/src/backend/core/api/utils.py b/src/backend/core/api/utils.py index 5d42b163..9f3167f3 100644 --- a/src/backend/core/api/utils.py +++ b/src/backend/core/api/utils.py @@ -12,7 +12,7 @@ def filter_root_paths(paths, skip_sorting=False): A root path is defined as a path that is not a prefix of any other path. Args: - paths (list of str): The list of paths. + paths (list of PathValue): The list of paths. Returns: list of str: The filtered list of root paths. @@ -23,7 +23,7 @@ def filter_root_paths(paths, skip_sorting=False): root_paths = [] for path in paths: # If the current path is not a prefix of the last added root path, add it - if not root_paths or not path.startswith(root_paths[-1]): + if not root_paths or not str(path).startswith(str(root_paths[-1])): root_paths.append(path) return root_paths diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 2e5d0847..63fab9f9 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -13,7 +13,6 @@ from django.core.exceptions import ValidationError from django.db import models as db from django.db import transaction from django.db.models.expressions import RawSQL -from django.db.models.functions import Left, Length import rest_framework as drf from django_filters import rest_framework as drf_filters @@ -415,7 +414,7 @@ class ItemViewSet( if user.is_authenticated: user_roles_subquery = models.ItemAccess.objects.filter( db.Q(user=user) | db.Q(team__in=user.teams), - item__path=Left(db.OuterRef("path"), Length("item__path")), + item__path__ancestors=db.OuterRef("path"), ).values_list("role", flat=True) return queryset.annotate( @@ -535,7 +534,7 @@ class ItemViewSet( @transaction.atomic def perform_create(self, serializer): """Set the current user as creator and owner of the newly created object.""" - obj = models.Item.add_root( + obj = models.Item.objects.create_child( creator=self.request.user, **serializer.validated_data, ) @@ -610,64 +609,53 @@ class ItemViewSet( return self.get_response_for_queryset(queryset) - @drf.decorators.action(detail=True, methods=["post"]) - @transaction.atomic - def move(self, request, *args, **kwargs): - """ - Move an item to another location within the item tree. + # @drf.decorators.action(detail=True, methods=["post"]) + # @transaction.atomic + # def move(self, request, *args, **kwargs): + # """ + # Move an item to another location within the item tree. - The user must be an administrator or owner of both the item being moved - and the target parent item. - """ - user = request.user - item = self.get_object() # including permission checks + # The user must be an administrator or owner of both the item being moved + # and the target parent item. + # """ + # user = request.user + # item = self.get_object() # including permission checks - # Validate the input payload - serializer = serializers.MoveItemSerializer(data=request.data) - serializer.is_valid(raise_exception=True) - validated_data = serializer.validated_data + # # Validate the input payload + # serializer = serializers.MoveItemSerializer(data=request.data) + # serializer.is_valid(raise_exception=True) + # validated_data = serializer.validated_data - target_item_id = validated_data["target_item_id"] - try: - target_item = models.Item.objects.get( - id=target_item_id, ancestors_deleted_at__isnull=True - ) - except models.Item.DoesNotExist: - return drf.response.Response( - {"target_item_id": "Target parent item does not exist."}, - status=status.HTTP_400_BAD_REQUEST, - ) + # target_item_id = validated_data["target_item_id"] + # try: + # target_item = models.Item.objects.get( + # id=target_item_id, ancestors_deleted_at__isnull=True + # ) + # except models.Item.DoesNotExist: + # return drf.response.Response( + # {"target_item_id": "Target parent item does not exist."}, + # status=status.HTTP_400_BAD_REQUEST, + # ) - position = validated_data["position"] - message = None + # message = None - if position in [ - enums.MoveNodePositionChoices.FIRST_CHILD, - enums.MoveNodePositionChoices.LAST_CHILD, - ]: - if not target_item.get_abilities(user).get("move"): - message = ( - "You do not have permission to move items " - "as a child to this target item." - ) - elif not target_item.is_root(): - if not target_item.get_parent().get_abilities(user).get("move"): - message = ( - "You do not have permission to move items " - "as a sibling of this target item." - ) + # if not target_item.get_abilities(user).get("move"): + # message = ( + # "You do not have permission to move items " + # "as a child to this target item." + # ) - if message: - return drf.response.Response( - {"target_item_id": message}, - status=status.HTTP_400_BAD_REQUEST, - ) + # if message: + # return drf.response.Response( + # {"target_item_id": message}, + # status=status.HTTP_400_BAD_REQUEST, + # ) - item.move(target_item, pos=position) + # item.move(target_item) - return drf.response.Response( - {"message": "item moved successfully."}, status=status.HTTP_200_OK - ) + # return drf.response.Response( + # {"message": "item moved successfully."}, status=status.HTTP_200_OK + # ) @drf.decorators.action( detail=True, @@ -703,8 +691,9 @@ class ItemViewSet( serializer.is_valid(raise_exception=True) with transaction.atomic(): - child_item = item.add_child( + child_item = models.Item.objects.create_child( creator=request.user, + parent=item, **serializer.validated_data, ) models.ItemAccess.objects.create( @@ -721,7 +710,7 @@ class ItemViewSet( ) # GET: List children - queryset = item.get_children().filter(deleted_at__isnull=True) + queryset = item.children().filter(deleted_at__isnull=True) queryset = self.filter_queryset(queryset) queryset = self.annotate_is_favorite(queryset) queryset = self.annotate_user_roles(queryset) diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index 85bff90c..7ca58dfe 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -63,7 +63,7 @@ class ItemFactory(factory.django.DjangoModelFactory): django_get_or_create = ("title",) skip_postgeneration_save = True - parent = ParentNodeFactory() + # parent = ParentNodeFactory() title = factory.Sequence(lambda n: f"item{n}") creator = factory.SubFactory(UserFactory) @@ -81,23 +81,27 @@ class ItemFactory(factory.django.DjangoModelFactory): else None ) + # @classmethod + # def _create(cls, model_class, *args, **kwargs): + # """ + # Custom creation logic for the factory: creates an item as a child node if + # a parent is provided; otherwise, creates it as a root node. + # """ + # parent = kwargs.pop("parent", None) + + # if parent: + # # Add as a child node + # kwargs["ancestors_deleted_at"] = ( + # kwargs.get("ancestors_deleted_at") or parent.ancestors_deleted_at + # ) + # return parent.add_child(**kwargs) + + # # Add as a root node + # return model_class.add_root(**kwargs) + @classmethod def _create(cls, model_class, *args, **kwargs): - """ - Custom creation logic for the factory: creates an item as a child node if - a parent is provided; otherwise, creates it as a root node. - """ - parent = kwargs.pop("parent", None) - - if parent: - # Add as a child node - kwargs["ancestors_deleted_at"] = ( - kwargs.get("ancestors_deleted_at") or parent.ancestors_deleted_at - ) - return parent.add_child(**kwargs) - - # Add as a root node - return model_class.add_root(**kwargs) + return model_class.objects.create_child(**kwargs) @factory.lazy_attribute def ancestors_deleted_at(self): diff --git a/src/backend/core/migrations/0001_initial.py b/src/backend/core/migrations/0001_initial.py index e87024da..f3d27ce5 100644 --- a/src/backend/core/migrations/0001_initial.py +++ b/src/backend/core/migrations/0001_initial.py @@ -15,6 +15,7 @@ class Migration(migrations.Migration): dependencies = [ ('auth', '0012_alter_user_first_name_max_length'), + ('django_ltree', '0001_create_extension'), ] operations = [ diff --git a/src/backend/core/migrations/0002_alter_item_options_remove_item_depth_and_more.py b/src/backend/core/migrations/0002_alter_item_options_remove_item_depth_and_more.py new file mode 100644 index 00000000..d99cc47d --- /dev/null +++ b/src/backend/core/migrations/0002_alter_item_options_remove_item_depth_and_more.py @@ -0,0 +1,36 @@ +# Generated by Django 5.1.5 on 2025-02-18 08:34 + +import django_ltree.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0001_initial'), + ] + + operations = [ + migrations.AlterModelOptions( + name='item', + options={'verbose_name': 'Item', 'verbose_name_plural': 'Items'}, + ), + migrations.RemoveField( + model_name='item', + name='depth', + ), + migrations.RemoveField( + model_name='item', + name='numchild', + ), + migrations.AlterField( + model_name='item', + name='path', + field=django_ltree.fields.PathField(unique=True), + ), + migrations.AlterField( + model_name='user', + name='language', + field=models.CharField(choices="(('en-us', 'English'), ('fr-fr', 'French'), ('de-de', 'German'))", default='en-us', help_text='The language in which the user wants to see the interface.', max_length=10, verbose_name='language'), + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index d5b199b8..a30adf1a 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -11,21 +11,23 @@ from logging import getLogger from django.conf import settings from django.contrib.auth import models as auth_models from django.contrib.auth.base_user import AbstractBaseUser +from django.contrib.postgres.indexes import GistIndex from django.contrib.sites.models import Site from django.core import mail, validators from django.core.cache import cache from django.core.exceptions import ValidationError from django.core.mail import send_mail from django.db import models, transaction -from django.db.models.functions import Left, Length +from django.db.models.expressions import RawSQL from django.template.loader import render_to_string from django.utils import timezone from django.utils.functional import cached_property, lazy from django.utils.translation import get_language, override from django.utils.translation import gettext_lazy as _ +from django_ltree.managers import TreeManager +from django_ltree.models import TreeModel from timezone_field import TimeZoneField -from treebeard.mp_tree import MP_Node logger = getLogger(__name__) @@ -374,7 +376,27 @@ class BaseAccess(BaseModel): } -class Item(MP_Node, BaseModel): +class ItemManager(TreeManager): + """Custom manager for Item model overriding create_child method.""" + + def create_child(self, parent=None, **kwargs): + """ + Check if the item can have children before adding one and if the title is + unique in the same path. + """ + if parent: + if parent.type != ItemTypeChoices.FOLDER: + raise ValidationError({"type": _("Only folders can have children.")}) + + if self.children(parent.path).filter(title=kwargs.get("title")).exists(): + raise ValidationError( + {"title": _("title already exists in this folder.")} + ) + + return super().create_child(parent=parent, **kwargs) + + +class Item(TreeModel, BaseModel): """Item in the tree.""" title = models.CharField(_("title"), max_length=255) @@ -396,13 +418,6 @@ class Item(MP_Node, BaseModel): deleted_at = models.DateTimeField(null=True, blank=True) ancestors_deleted_at = models.DateTimeField(null=True, blank=True) - # Tree structure - alphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" - steplen = 7 # nb siblings max: 3,521,614,606,208 - node_order_by = [] # Manual ordering - - path = models.CharField(max_length=7 * 36, unique=True, db_collation="C") - filename = models.CharField(max_length=255, null=True, blank=True) type = models.CharField( max_length=30, @@ -416,9 +431,12 @@ class Item(MP_Node, BaseModel): blank=True, ) + label_size = 7 + + objects = ItemManager() + class Meta: db_table = "drive_item" - ordering = ("path",) verbose_name = _("Item") verbose_name_plural = _("Items") constraints = [ @@ -439,9 +457,12 @@ class Item(MP_Node, BaseModel): name="check_filename_set_for_files", ), ] + indexes = [ + GistIndex(fields=["path"]), + ] def __str__(self): - return str(self.title) if self.title else str(_("Untitled Item")) + return str(self.title) def save(self, *args, **kwargs): """Set the upload state to pending if it's the first save and it's a file""" @@ -450,6 +471,14 @@ class Item(MP_Node, BaseModel): return super().save(*args, **kwargs) + def ancestors(self): + """Return the ancestors of the item excluding the item itself.""" + return super().ancestors().exclude(id=self.id) + + def descendants(self): + """Return the descendants of the item excluding the item itself.""" + return super().descendants().exclude(id=self.id) + @property def key_base(self): """Key base of the location where the item is stored in object storage.""" @@ -471,6 +500,25 @@ class Item(MP_Node, BaseModel): return f"{self.key_base}/{self.filename}" + @property + def depth(self): + """Return the depth of the item in the tree.""" + return len(self.path) + + @property + def numchild(self): + """Return the number of children of the item.""" + if self.type != ItemTypeChoices.FOLDER: + return 0 + return self.children().count() + + @property + def numfolder_children(self): + """Return the number of folder children of the item.""" + if self.type != ItemTypeChoices.FOLDER: + return 0 + return self.children().filter(type=ItemTypeChoices.FOLDER).count() + def get_nb_accesses_cache_key(self): """Generate a unique cache key for each item.""" return f"item_{self.id!s}_nb_accesses" @@ -483,7 +531,7 @@ class Item(MP_Node, BaseModel): if nb_accesses is None: nb_accesses = ItemAccess.objects.filter( - item__path=Left(models.Value(self.path), Length("item__path")), + item__path__ancestors=self.path, ).count() cache.set(cache_key, nb_accesses) @@ -493,7 +541,7 @@ class Item(MP_Node, BaseModel): """ Invalidate the cache for number of accesses, including on affected descendants. """ - for item in Item.objects.filter(path__startswith=self.path).only("id"): + for item in Item.objects.filter(path__descendants=self.path).only("id"): cache_key = item.get_nb_accesses_cache_key() cache.delete(cache_key) @@ -508,7 +556,7 @@ class Item(MP_Node, BaseModel): try: roles = ItemAccess.objects.filter( models.Q(user=user) | models.Q(team__in=user.teams), - item__path=Left(models.Value(self.path), Length("item__path")), + item__path__ancestors=self.path, ).values_list("role", flat=True) except (models.ObjectDoesNotExist, IndexError): roles = [] @@ -521,8 +569,10 @@ class Item(MP_Node, BaseModel): # Ancestors links definitions are only interesting if the item is not the highest # ancestor to which the current user has access. Look for the annotation: - if self.depth > 1 and not getattr(self, "is_highest_ancestor_for_user", False): - for ancestor in self.get_ancestors().values("link_reach", "link_role"): + if len(self.path) > 1 and not getattr( + self, "is_highest_ancestor_for_user", False + ): + for ancestor in self.ancestors().values("link_reach", "link_role"): links_definitions.setdefault(ancestor["link_reach"], set()).add( ancestor["link_role"] ) @@ -650,7 +700,7 @@ class Item(MP_Node, BaseModel): raise RuntimeError("This item is already deleted or has deleted ancestors.") # Check if any ancestors are deleted - if self.get_ancestors().filter(deleted_at__isnull=False).exists(): + if self.ancestors().filter(deleted_at__isnull=False).exists(): raise RuntimeError( "Cannot delete this item because one or more ancestors are already deleted." ) @@ -659,7 +709,7 @@ class Item(MP_Node, BaseModel): self.save() # Mark all descendants as soft deleted - self.get_descendants().filter(ancestors_deleted_at__isnull=True).update( + self.descendants().filter(ancestors_deleted_at__isnull=True).update( ancestors_deleted_at=self.ancestors_deleted_at ) @@ -679,54 +729,41 @@ class Item(MP_Node, BaseModel): } ) + # save the current deleted_at value to exclude it from the descendants update + current_deleted_at = self.deleted_at + # Restore the current item self.deleted_at = None # Calculate the minimum `deleted_at` among all ancestors ancestors_deleted_at = ( - self.get_ancestors() + self.ancestors() .filter(deleted_at__isnull=False) .values_list("deleted_at", flat=True) ) self.ancestors_deleted_at = min(ancestors_deleted_at, default=None) self.save() - # Update descendants excluding those who were deleted prior to the deletion of the - # current item (the ancestor_deleted_at date for those should already by good) - # The number of deleted descendants should not be too big so we can handcraft a union - # clause for them: - deleted_descendants_paths = ( - self.get_descendants() - .filter(deleted_at__isnull=False) - .values_list("path", flat=True) - ) - exclude_condition = models.Q( - *(models.Q(path__startswith=path) for path in deleted_descendants_paths) - ) - self.get_descendants().exclude(exclude_condition).update( - ancestors_deleted_at=self.ancestors_deleted_at - ) + self.descendants().exclude( + models.Q(deleted_at__isnull=False) + | models.Q(ancestors_deleted_at__lt=current_deleted_at) + ).update(ancestors_deleted_at=self.ancestors_deleted_at) - def add_child(self, **kwargs): - """Check if the item can have children before adding one.""" - if self.type != ItemTypeChoices.FOLDER: - raise ValidationError({"type": _("Only folders can have children.")}) - - if (title := kwargs.get("title")) and Item.objects.filter( - models.Q(path__startswith=self.path, title=title), - ~models.Q(id=self.id), - ).exists(): - raise ValidationError({"title": _("title already exists in this folder.")}) - - return super().add_child(**kwargs) - - def move(self, target, pos=None): + def move(self, target): + """ + Move an item to a new position in the tree. Inspired by + https://patshaughnessy.net/2017/12/14/manipulating-trees-using-sql-and-the-postgres-ltree-extension + """ if target.type != ItemTypeChoices.FOLDER: raise ValidationError( {"target": _("Only folders can be targeted when moving an item")} ) - return super().move(target, pos) + Item.objects.filter(path__descendants=self.path).update( + path=RawSQL( + "%s || subpath(path, nlevel(%s)-1)", (str(target.path), str(self.path)) + ) + ) class LinkTrace(BaseModel): diff --git a/src/backend/core/tests/items/test_api_items_children_list.py b/src/backend/core/tests/items/test_api_items_children_list.py index 2a7c0933..546a567b 100644 --- a/src/backend/core/tests/items/test_api_items_children_list.py +++ b/src/backend/core/tests/items/test_api_items_children_list.py @@ -41,7 +41,7 @@ def test_api_items_children_list_anonymous_public_standalone(): "link_role": child1.link_role, "numchild": 0, "nb_accesses": 1, - "path": child1.path, + "path": str(child1.path), "title": child1.title, "updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -62,7 +62,7 @@ def test_api_items_children_list_anonymous_public_standalone(): "link_role": child2.link_role, "numchild": 0, "nb_accesses": 0, - "path": child2.path, + "path": str(child2.path), "title": child2.title, "updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -122,7 +122,7 @@ def test_api_items_children_list_anonymous_public_parent(): "link_role": child1.link_role, "numchild": 0, "nb_accesses": 1, - "path": child1.path, + "path": str(child1.path), "title": child1.title, "updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -141,7 +141,7 @@ def test_api_items_children_list_anonymous_public_parent(): "link_role": child2.link_role, "numchild": 0, "nb_accesses": 0, - "path": child2.path, + "path": str(child2.path), "title": child2.title, "updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -205,7 +205,7 @@ def test_api_items_children_list_authenticated_unrelated_public_or_authenticated "link_role": child1.link_role, "numchild": 0, "nb_accesses": 1, - "path": child1.path, + "path": str(child1.path), "title": child1.title, "updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -226,7 +226,7 @@ def test_api_items_children_list_authenticated_unrelated_public_or_authenticated "link_role": child2.link_role, "numchild": 0, "nb_accesses": 0, - "path": child2.path, + "path": str(child2.path), "title": child2.title, "updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -284,7 +284,7 @@ def test_api_items_children_list_authenticated_public_or_authenticated_parent( "link_role": child1.link_role, "numchild": 0, "nb_accesses": 1, - "path": child1.path, + "path": str(child1.path), "title": child1.title, "updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -305,7 +305,7 @@ def test_api_items_children_list_authenticated_public_or_authenticated_parent( "link_role": child2.link_role, "numchild": 0, "nb_accesses": 0, - "path": child2.path, + "path": str(child2.path), "title": child2.title, "updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -381,7 +381,7 @@ def test_api_items_children_list_authenticated_related_direct(): "link_role": child1.link_role, "numchild": 0, "nb_accesses": 3, - "path": child1.path, + "path": str(child1.path), "title": child1.title, "updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [access.role], @@ -402,7 +402,7 @@ def test_api_items_children_list_authenticated_related_direct(): "link_role": child2.link_role, "numchild": 0, "nb_accesses": 2, - "path": child2.path, + "path": str(child2.path), "title": child2.title, "updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [access.role], @@ -461,7 +461,7 @@ def test_api_items_children_list_authenticated_related_parent(): "link_role": child1.link_role, "numchild": 0, "nb_accesses": 2, - "path": child1.path, + "path": str(child1.path), "title": child1.title, "updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [grand_parent_access.role], @@ -482,7 +482,7 @@ def test_api_items_children_list_authenticated_related_parent(): "link_role": child2.link_role, "numchild": 0, "nb_accesses": 1, - "path": child2.path, + "path": str(child2.path), "title": child2.title, "updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [grand_parent_access.role], @@ -590,7 +590,7 @@ def test_api_items_children_list_authenticated_related_team_members( "link_role": child1.link_role, "numchild": 0, "nb_accesses": 1, - "path": child1.path, + "path": str(child1.path), "title": child1.title, "updated_at": child1.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [access.role], @@ -611,7 +611,7 @@ def test_api_items_children_list_authenticated_related_team_members( "link_role": child2.link_role, "numchild": 0, "nb_accesses": 1, - "path": child2.path, + "path": str(child2.path), "title": child2.title, "updated_at": child2.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [access.role], diff --git a/src/backend/core/tests/items/test_api_items_list.py b/src/backend/core/tests/items/test_api_items_list.py index 8b9eb413..ba4c3959 100644 --- a/src/backend/core/tests/items/test_api_items_list.py +++ b/src/backend/core/tests/items/test_api_items_list.py @@ -84,7 +84,7 @@ def test_api_items_list_format(): "link_role": item2.link_role, "nb_accesses": 3, "numchild": 0, - "path": item2.path, + "path": str(item2.path), "title": item2.title, "updated_at": item2.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [access2.role], @@ -103,7 +103,7 @@ def test_api_items_list_format(): "link_role": item.link_role, "nb_accesses": 3, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [access.role], @@ -151,9 +151,13 @@ def test_api_items_list_authenticated_direct(django_assert_num_queries): # Children of hidden items should get listed when visible by the logged-in user hidden_root = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) - child3_with_access = factories.ItemFactory(parent=hidden_root) + child3_with_access = factories.ItemFactory( + parent=hidden_root, type=models.ItemTypeChoices.FILE + ) factories.UserItemAccessFactory(user=user, item=child3_with_access) - child4_with_access = factories.ItemFactory(parent=hidden_root) + child4_with_access = factories.ItemFactory( + parent=hidden_root, type=models.ItemTypeChoices.FILE + ) factories.UserItemAccessFactory(user=user, item=child4_with_access) # items that are soft deleted and children of a soft deleted item should not be listed @@ -191,11 +195,11 @@ def test_api_items_list_authenticated_direct(django_assert_num_queries): str(child4_with_access.id), } - with django_assert_num_queries(8): + with django_assert_num_queries(10): response = client.get("/api/v1.0/items/") # nb_accesses should now be cached - with django_assert_num_queries(4): + with django_assert_num_queries(6): response = client.get("/api/v1.0/items/") assert response.status_code == 200 @@ -220,11 +224,15 @@ def test_api_items_list_authenticated_via_team( items_team1 = [ access.item - for access in factories.TeamItemAccessFactory.create_batch(2, team="team1") + for access in factories.TeamItemAccessFactory.create_batch( + 2, team="team1", item__type=models.ItemTypeChoices.FILE + ) ] items_team2 = [ access.item - for access in factories.TeamItemAccessFactory.create_batch(3, team="team2") + for access in factories.TeamItemAccessFactory.create_batch( + 3, team="team2", item__type=models.ItemTypeChoices.FILE + ) ] expected_ids = {str(item.id) for item in items_team1 + items_team2} @@ -255,11 +263,15 @@ def test_api_items_list_authenticated_link_reach_restricted( client = APIClient() client.force_login(user) - item = factories.ItemFactory(link_traces=[user], link_reach="restricted") + item = factories.ItemFactory( + link_traces=[user], link_reach="restricted", type=models.ItemTypeChoices.FILE + ) # Link traces for other items or other users should not interfere models.LinkTrace.objects.create(item=item, user=factories.UserFactory()) - other_item = factories.ItemFactory(link_reach="public") + other_item = factories.ItemFactory( + link_reach="public", type=models.ItemTypeChoices.FILE + ) models.LinkTrace.objects.create(item=other_item, user=user) with django_assert_num_queries(5): @@ -310,15 +322,16 @@ def test_api_items_list_authenticated_link_reach_public_or_authenticated( link_traces=[user], link_reach=random.choice(["public", "authenticated"]), parent=hidden_item, + type=models.ItemTypeChoices.FILE, ) expected_ids = {str(item1.id), str(item2.id), str(visible_child.id)} - with django_assert_num_queries(7): + with django_assert_num_queries(9): response = client.get("/api/v1.0/items/") # nb_accesses should now be cached - with django_assert_num_queries(4): + with django_assert_num_queries(6): response = client.get("/api/v1.0/items/") assert response.status_code == 200 @@ -405,8 +418,12 @@ def test_api_items_list_favorites_no_extra_queries(django_assert_num_queries): client = APIClient() client.force_login(user) - special_items = factories.ItemFactory.create_batch(3, users=[user]) - factories.ItemFactory.create_batch(2, users=[user]) + special_items = factories.ItemFactory.create_batch( + 3, users=[user], type=models.ItemTypeChoices.FILE + ) + factories.ItemFactory.create_batch( + 2, users=[user], type=models.ItemTypeChoices.FILE + ) url = "/api/v1.0/items/" with django_assert_num_queries(9): diff --git a/src/backend/core/tests/items/test_api_items_move.py b/src/backend/core/tests/items/test_api_items_move.py index fd9957a7..93a60d6a 100644 --- a/src/backend/core/tests/items/test_api_items_move.py +++ b/src/backend/core/tests/items/test_api_items_move.py @@ -14,6 +14,8 @@ from core import enums, factories, models pytestmark = pytest.mark.django_db +pytest.skip("move API is not re implemented using ltree yet", allow_module_level=True) + def test_api_items_move_anonymous_user(): """Anonymous users should not be able to move items.""" @@ -59,7 +61,7 @@ def test_api_items_move_authenticated_item_no_permission(role): def test_api_items_move_invalid_target_string(): - """Test for moving a item to an invalid target as a random string.""" + """Test for moving an item to an invalid target as a random string.""" user = factories.UserFactory() client = APIClient() client.force_login(user) @@ -76,7 +78,7 @@ def test_api_items_move_invalid_target_string(): def test_api_items_move_invalid_target_uuid(): - """Test for moving a item to an invalid target that looks like a UUID.""" + """Test for moving an item to an invalid target that looks like a UUID.""" user = factories.UserFactory() client = APIClient() client.force_login(user) @@ -92,34 +94,10 @@ def test_api_items_move_invalid_target_uuid(): assert response.json() == {"target_item_id": "Target parent item does not exist."} -def test_api_items_move_invalid_position(): - """Test moving a item to an invalid position.""" - user = factories.UserFactory() - client = APIClient() - client.force_login(user) - - item = factories.UserItemAccessFactory(user=user, role="owner").item - target = factories.UserItemAccessFactory(user=user, role="owner").item - - response = client.post( - f"/api/v1.0/items/{item.id!s}/move/", - data={ - "target_item_id": str(target.id), - "position": "invalid-position", - }, - ) - - assert response.status_code == 400 - assert response.json() == { - "position": ['"invalid-position" is not a valid choice.'] - } - - -@pytest.mark.parametrize("position", enums.MoveNodePositionChoices.values) @pytest.mark.parametrize("target_parent_role", models.RoleChoices.values) @pytest.mark.parametrize("target_role", models.RoleChoices.values) def test_api_items_move_authenticated_target_roles_mocked( - target_role, target_parent_role, position + target_role, target_parent_role ): """ Authenticated users with insufficient permissions on the target item (or its @@ -135,14 +113,15 @@ def test_api_items_move_authenticated_target_roles_mocked( item = factories.ItemFactory( users=[(user, random.choice(power_roles))], type=models.ItemTypeChoices.FOLDER ) - children = factories.ItemFactory.create_batch( + # children + factories.ItemFactory.create_batch( 3, parent=item, type=models.ItemTypeChoices.FOLDER ) target_parent = factories.ItemFactory( users=[(user, target_parent_role)], type=models.ItemTypeChoices.FOLDER ) - sibling1, target, sibling2 = factories.ItemFactory.create_batch( + _sibling1, target, _sibling2 = factories.ItemFactory.create_batch( 3, parent=target_parent, type=models.ItemTypeChoices.FOLDER ) models.ItemAccess.objects.create(item=target, user=user, role=target_role) @@ -150,66 +129,15 @@ def test_api_items_move_authenticated_target_roles_mocked( response = client.post( f"/api/v1.0/items/{item.id!s}/move/", - data={"target_item_id": str(target.id), "position": position}, + data={"target_item_id": str(target.id)}, ) item.refresh_from_db() - if ( - position in ["first-child", "last-child"] - and (target_role in power_roles or target_parent_role in power_roles) - ) or ( - position in ["first-sibling", "last-sibling", "left", "right"] - and target_parent_role in power_roles - ): - assert response.status_code == 200 - assert response.json() == {"message": "item moved successfully."} + assert response.status_code == 200 + assert response.json() == {"message": "item moved successfully."} - match position: - case "first-child": - assert list(target.get_children()) == [item, *target_children] - case "last-child": - assert list(target.get_children()) == [*target_children, item] - case "first-sibling": - assert list(target.get_siblings()) == [ - item, - sibling1, - target, - sibling2, - ] - case "last-sibling": - assert list(target.get_siblings()) == [ - sibling1, - target, - sibling2, - item, - ] - case "left": - assert list(target.get_siblings()) == [ - sibling1, - item, - target, - sibling2, - ] - case "right": - assert list(target.get_siblings()) == [ - sibling1, - target, - item, - sibling2, - ] - case _: - raise ValueError(f"Invalid position: {position}") - - # Verify that the item's children have also been moved - assert list(item.get_children()) == children - else: - assert response.status_code == 400 - assert ( - "You do not have permission to move items" - in response.json()["target_item_id"] - ) - assert item.is_root() is True + assert list(target.children()) == [item, *target_children] def test_api_items_move_authenticated_deleted_item(): @@ -290,7 +218,7 @@ def test_api_items_move_authenticated_target_not_folder_should_fail(): ) def test_api_items_move_authenticated_deleted_target_as_child(position): """ - It should not be possible to move a item as a child of a deleted target + It should not be possible to move an item as a child of a deleted target even for a owner. """ user = factories.UserFactory() @@ -338,7 +266,7 @@ def test_api_items_move_authenticated_deleted_target_as_child(position): ) def test_api_items_move_authenticated_deleted_target_as_sibling(position): """ - It should not be possible to move a item as a sibling of a deleted target item + It should not be possible to move an item as a sibling of a deleted target item if the user has no rigths on its parent. """ user = factories.UserFactory() diff --git a/src/backend/core/tests/items/test_api_items_retrieve.py b/src/backend/core/tests/items/test_api_items_retrieve.py index d1468881..c4c19b84 100644 --- a/src/backend/core/tests/items/test_api_items_retrieve.py +++ b/src/backend/core/tests/items/test_api_items_retrieve.py @@ -51,7 +51,7 @@ def test_api_items_retrieve_anonymous_public_standalone(): "link_role": item.link_role, "nb_accesses": 0, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -108,7 +108,7 @@ def test_api_items_retrieve_anonymous_public_parent(): "link_role": item.link_role, "nb_accesses": 0, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -195,7 +195,7 @@ def test_api_items_retrieve_authenticated_unrelated_public_or_authenticated(reac "link_role": item.link_role, "nb_accesses": 0, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -257,7 +257,7 @@ def test_api_items_retrieve_authenticated_public_or_authenticated_parent(reach): "link_role": item.link_role, "nb_accesses": 0, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], @@ -367,7 +367,7 @@ def test_api_items_retrieve_authenticated_related_direct(): "link_role": item.link_role, "nb_accesses": 2, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [access.role], @@ -431,7 +431,7 @@ def test_api_items_retrieve_authenticated_related_parent(): "link_role": item.link_role, "nb_accesses": 2, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [access.role], @@ -583,7 +583,7 @@ def test_api_items_retrieve_authenticated_related_team_members( "link_role": item.link_role, "nb_accesses": 5, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": roles, @@ -643,7 +643,7 @@ def test_api_items_retrieve_authenticated_related_team_administrators( "link_role": item.link_role, "nb_accesses": 5, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": roles, @@ -703,7 +703,7 @@ def test_api_items_retrieve_authenticated_related_team_owners( "link_role": item.link_role, "nb_accesses": 5, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": roles, @@ -732,7 +732,9 @@ def test_api_items_retrieve_user_roles(django_assert_num_queries): type=models.ItemTypeChoices.FOLDER, ) item = factories.ItemFactory( - parent=parent, users=factories.UserFactory.create_batch(2) + parent=parent, + type=models.ItemTypeChoices.FILE, + users=factories.UserFactory.create_batch(2), ) accesses = ( @@ -757,7 +759,9 @@ def test_api_items_retrieve_numqueries_with_link_trace(django_assert_num_queries client = APIClient() client.force_login(user) - item = factories.ItemFactory(users=[user], link_traces=[user]) + item = factories.ItemFactory( + users=[user], link_traces=[user], type=models.ItemTypeChoices.FILE + ) with django_assert_num_queries(4): response = client.get(f"/api/v1.0/items/{item.id!s}/") @@ -973,7 +977,7 @@ def test_api_items_retrieve_file_uploaded(): "link_role": item.link_role, "nb_accesses": 0, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": [], diff --git a/src/backend/core/tests/items/test_api_items_trashbin.py b/src/backend/core/tests/items/test_api_items_trashbin.py index b1ef5fbe..888acacf 100644 --- a/src/backend/core/tests/items/test_api_items_trashbin.py +++ b/src/backend/core/tests/items/test_api_items_trashbin.py @@ -91,7 +91,7 @@ def test_api_items_trashbin_format(): "link_role": item.link_role, "nb_accesses": 3, "numchild": 0, - "path": item.path, + "path": str(item.path), "title": item.title, "updated_at": item.updated_at.isoformat().replace("+00:00", "Z"), "user_roles": ["owner"], @@ -132,7 +132,9 @@ def test_api_items_trashbin_authenticated_direct(django_assert_num_queries): # Nested items should also get listed parent = factories.ItemFactory(parent=item1, type=models.ItemTypeChoices.FOLDER) - item3 = factories.ItemFactory(parent=parent, deleted_at=now) + item3 = factories.ItemFactory( + parent=parent, deleted_at=now, type=models.ItemTypeChoices.FILE + ) models.ItemAccess.objects.create(item=parent, user=user, role="owner") # Permanently deleted items should not be listed @@ -143,10 +145,10 @@ def test_api_items_trashbin_authenticated_direct(django_assert_num_queries): expected_ids = {str(item1.id), str(item2.id), str(item3.id)} - with django_assert_num_queries(7): + with django_assert_num_queries(9): response = client.get("/api/v1.0/items/trashbin/") - with django_assert_num_queries(4): + with django_assert_num_queries(6): response = client.get("/api/v1.0/items/trashbin/") assert response.status_code == 200 @@ -171,13 +173,13 @@ def test_api_items_trashbin_authenticated_via_team( mock_user_teams.return_value = ["team1", "team2", "unknown"] deleted_item_team1 = factories.ItemFactory( - teams=[("team1", "owner")], deleted_at=now + teams=[("team1", "owner")], deleted_at=now, type=models.ItemTypeChoices.FILE ) factories.ItemFactory(teams=[("team1", "owner")]) factories.ItemFactory(teams=[("team1", "administrator")], deleted_at=now) factories.ItemFactory(teams=[("team1", "administrator")]) deleted_item_team2 = factories.ItemFactory( - teams=[("team2", "owner")], deleted_at=now + teams=[("team2", "owner")], deleted_at=now, type=models.ItemTypeChoices.FILE ) factories.ItemFactory(teams=[("team2", "owner")]) factories.ItemFactory(teams=[("team2", "administrator")], deleted_at=now) diff --git a/src/backend/core/tests/test_api_utils_filter_root_paths.py b/src/backend/core/tests/test_api_utils_filter_root_paths.py index 1375d223..b5aec908 100644 --- a/src/backend/core/tests/test_api_utils_filter_root_paths.py +++ b/src/backend/core/tests/test_api_utils_filter_root_paths.py @@ -2,6 +2,8 @@ Unit tests for the filter_root_paths utility function. """ +from django_ltree.fields import PathValue + from core.api.utils import filter_root_paths @@ -14,36 +16,36 @@ def test_api_utils_filter_root_paths_success(): only the minimal set of root paths is returned. """ paths = [ - "0001", - "00010001", - "000100010001", - "000100010002", + PathValue("0001"), + PathValue("0001.0001"), + PathValue("0001.0001.0001"), + PathValue("0001.0001.0002"), # missing 00010002 - "000100020001", - "000100020002", - "0002", - "00020001", - "00020002", + PathValue("0001.0002.0001"), + PathValue("0001.0002.0002"), + PathValue("0002"), + PathValue("0002.0001"), + PathValue("0002.0002"), # missing 0003 - "00030001", - "000300010001", - "00030002", + PathValue("0003.0001"), + PathValue("0003.0001.0001"), + PathValue("0003.0002"), # missing 0004 # missing 00040001 # missing 000400010001 # missing 000400010002 - "000400010003", - "0004000100030001", - "000400010004", + PathValue("0004.0001.0003"), + PathValue("0004.0001.0003.0001"), + PathValue("0004.0001.0004"), ] filtered_paths = filter_root_paths(paths, skip_sorting=True) assert filtered_paths == [ - "0001", - "0002", - "00030001", - "00030002", - "000400010003", - "000400010004", + PathValue("0001"), + PathValue("0002"), + PathValue("0003.0001"), + PathValue("0003.0002"), + PathValue("0004.0001.0003"), + PathValue("0004.0001.0004"), ] @@ -55,40 +57,40 @@ def test_api_utils_filter_root_paths_sorting(): when sorting is enabled, the result is correctly ordered and minimal. """ paths = [ - "0001", - "00010001", - "000100010001", - "000100020002", - "000100010002", - "000100020001", - "00020001", - "0002", - "00020002", - "000300010001", - "00030001", - "00030002", - "0004000100030001", - "000400010003", - "000400010004", + PathValue("0001"), + PathValue("0001.0001"), + PathValue("0001.0001.0001"), + PathValue("0001.0002.0002"), + PathValue("0001.0001.0002"), + PathValue("0001.0002.0001"), + PathValue("0002.0001"), + PathValue("0002"), + PathValue("0002.0002"), + PathValue("0003.0001.0001"), + PathValue("0003.0001"), + PathValue("0003.0002"), + PathValue("0004.0001.0003.0001"), + PathValue("0004.0001.0003"), + PathValue("0004.0001.0004"), ] filtered_paths = filter_root_paths(paths, skip_sorting=True) assert filtered_paths == [ - "0001", - "00020001", - "0002", - "000300010001", - "00030001", - "00030002", - "0004000100030001", - "000400010003", - "000400010004", + PathValue("0001"), + PathValue("0002.0001"), + PathValue("0002"), + PathValue("0003.0001.0001"), + PathValue("0003.0001"), + PathValue("0003.0002"), + PathValue("0004.0001.0003.0001"), + PathValue("0004.0001.0003"), + PathValue("0004.0001.0004"), ] filtered_paths = filter_root_paths(paths) assert filtered_paths == [ - "0001", - "0002", - "00030001", - "00030002", - "000400010003", - "000400010004", + PathValue("0001"), + PathValue("0002"), + PathValue("0003.0001"), + PathValue("0003.0002"), + PathValue("0004.0001.0003"), + PathValue("0004.0001.0004"), ] diff --git a/src/backend/core/tests/test_models_invitations.py b/src/backend/core/tests/test_models_invitations.py index f7bc1442..5dd15dce 100644 --- a/src/backend/core/tests/test_models_invitations.py +++ b/src/backend/core/tests/test_models_invitations.py @@ -144,7 +144,7 @@ def test_models_invitationd_new_user_filter_expired_invitations(): @pytest.mark.parametrize("num_invitations, num_queries", [(0, 3), (1, 7), (20, 7)]) -def test_models_invitationd_new_userd_user_creation_constant_num_queries( +def test_models_invitations_new_userd_user_creation_constant_num_queries( django_assert_num_queries, num_invitations, num_queries ): """ diff --git a/src/backend/core/tests/test_models_items.py b/src/backend/core/tests/test_models_items.py index a321e08a..af5cf823 100644 --- a/src/backend/core/tests/test_models_items.py +++ b/src/backend/core/tests/test_models_items.py @@ -54,25 +54,9 @@ def test_models_items_file_key(): assert item.file_key == "item/9531a5f1-42b1-496c-b3f4-1c09ed139b3c/logo.png" -def test_models_items_tree_alphabet(): - """Test the creation of items with treebeard methods.""" - models.Item.load_bulk( - [ - { - "data": { - "title": f"item-{i}", - } - } - for i in range(len(models.Item.alphabet) * 2) - ] - ) - - assert models.Item.objects.count() == 124 - - @pytest.mark.parametrize("depth", range(5)) def test_models_items_soft_delete(depth): - """Trying to delete a item that is already deleted or is a descendant of + """Trying to delete an item that is already deleted or is a descendant of a deleted item should raise an error. """ items = [] @@ -90,20 +74,23 @@ def test_models_items_soft_delete(depth): deleted_item = random.choice(items) deleted_item.soft_delete() - with pytest.raises(RuntimeError): + with pytest.raises(RuntimeError) as exc_info: items[-1].soft_delete() + assert str(exc_info) == "This item is already deleted or has deleted ancestors." assert deleted_item.deleted_at is not None assert deleted_item.ancestors_deleted_at == deleted_item.deleted_at - descendants = deleted_item.get_descendants() + descendants = deleted_item.descendants() for child in descendants: + assert child.id != deleted_item.id assert child.deleted_at is None assert child.ancestors_deleted_at is not None assert child.ancestors_deleted_at == deleted_item.deleted_at - ancestors = deleted_item.get_ancestors() + ancestors = deleted_item.ancestors() for parent in ancestors: + assert parent.id != deleted_item.id assert parent.deleted_at is None assert parent.ancestors_deleted_at is None @@ -128,7 +115,7 @@ def test_models_items_get_abilities_forbidden( is_authenticated, reach, role, django_assert_num_queries ): """ - Check abilities returned for a item giving insufficient roles to link holders + Check abilities returned for an item giving insufficient roles to link holders i.e anonymous users or authenticated users who have no specific role on the item. """ item = factories.ItemFactory(link_reach=reach, link_role=role) @@ -625,15 +612,191 @@ def test_models_items_unique_title_in_current_path(): filename="file.txt", ) - with pytest.raises(ValidationError): + with pytest.raises(ValidationError) as exc_info: factories.ItemFactory( parent=parent, title="folder", type=models.ItemTypeChoices.FOLDER ) + assert exc_info.value.message_dict == { + "title": "title already exists in this folder." + } - with pytest.raises(ValidationError): + with pytest.raises(ValidationError) as exc_info: factories.ItemFactory( parent=parent, title="file.txt", type=models.ItemTypeChoices.FILE, filename="file.txt", ) + assert exc_info.value.message_dict == { + "title": "title already exists in this folder." + } + + +def test_models_items_numchild(): + """The numchild property should return the number of children.""" + parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + assert parent.numchild == 0 + + factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + parent.refresh_from_db() + assert parent.numchild == 1 + + factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FILE) + parent.refresh_from_db() + assert parent.numchild == 2 + + to_delete = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + parent.refresh_from_db() + assert parent.numchild == 3 + + to_delete.soft_delete() + parent.refresh_from_db() + assert parent.numchild == 2 + + to_delete.restore() + parent.refresh_from_db() + assert parent.numchild == 3 + + +def test_models_items_numchild_folder(): + """The numchild_folder property should return the number of folder children.""" + parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + assert parent.numchild_folder == 0 + + factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + parent.refresh_from_db() + assert parent.numchild_folder == 1 + + factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FILE) + parent.refresh_from_db() + assert parent.numchild_folder == 1 + + to_delete = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + parent.refresh_from_db() + assert parent.numchild_folder == 2 + + to_delete.soft_delete() + parent.refresh_from_db() + assert parent.numchild_folder == 1 + + to_delete.restore() + parent.refresh_from_db() + assert parent.numchild_folder == 2 + + +def test_models_items_restore(): + """The restore method should restore a soft-deleted item.""" + item = factories.ItemFactory() + item.soft_delete() + item.refresh_from_db() + assert item.deleted_at is not None + assert item.ancestors_deleted_at == item.deleted_at + + item.restore() + item.refresh_from_db() + assert item.deleted_at is None + assert item.ancestors_deleted_at == item.deleted_at + + +def test_models_items_restore_complex(): + """The restore method should restore a soft-deleted item and its ancestors.""" + grand_parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + parent = factories.ItemFactory( + parent=grand_parent, type=models.ItemTypeChoices.FOLDER + ) + item = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + + child1 = factories.ItemFactory(parent=item) + child2 = factories.ItemFactory(parent=item) + + # Soft delete first the item + item.soft_delete() + item.refresh_from_db() + child1.refresh_from_db() + child2.refresh_from_db() + assert item.deleted_at is not None + assert item.ancestors_deleted_at == item.deleted_at + assert child1.ancestors_deleted_at == item.deleted_at + assert child2.ancestors_deleted_at == item.deleted_at + + # Soft delete the grand parent + grand_parent.soft_delete() + grand_parent.refresh_from_db() + parent.refresh_from_db() + assert grand_parent.deleted_at is not None + assert grand_parent.ancestors_deleted_at == grand_parent.deleted_at + assert parent.ancestors_deleted_at == grand_parent.deleted_at + # item, child1 and child2 should not be affected + item.refresh_from_db() + child1.refresh_from_db() + child2.refresh_from_db() + assert item.deleted_at is not None + assert item.ancestors_deleted_at == item.deleted_at + assert child1.ancestors_deleted_at == item.deleted_at + assert child2.ancestors_deleted_at == item.deleted_at + + # Restore the item + item.restore() + item.refresh_from_db() + child1.refresh_from_db() + child2.refresh_from_db() + grand_parent.refresh_from_db() + assert item.deleted_at is None + assert item.ancestors_deleted_at == grand_parent.deleted_at + # child 1 and child 2 should now have the same ancestors_deleted_at as the grand parent + assert child1.ancestors_deleted_at == grand_parent.deleted_at + assert child2.ancestors_deleted_at == grand_parent.deleted_at + + +def test_models_items_restore_complex_bis(): + """The restore method should restore a soft-deleted item and its ancestors.""" + grand_parent = factories.ItemFactory(type=models.ItemTypeChoices.FOLDER) + parent = factories.ItemFactory( + parent=grand_parent, type=models.ItemTypeChoices.FOLDER + ) + item = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER) + + child1 = factories.ItemFactory(parent=item) + child2 = factories.ItemFactory(parent=item) + + # Soft delete first the item + item.soft_delete() + item.refresh_from_db() + child1.refresh_from_db() + child2.refresh_from_db() + assert item.deleted_at is not None + assert item.ancestors_deleted_at == item.deleted_at + assert child1.ancestors_deleted_at == item.deleted_at + assert child2.ancestors_deleted_at == item.deleted_at + + # Soft delete the grand parent + grand_parent.soft_delete() + grand_parent.refresh_from_db() + parent.refresh_from_db() + assert grand_parent.deleted_at is not None + assert grand_parent.ancestors_deleted_at == grand_parent.deleted_at + assert parent.ancestors_deleted_at == grand_parent.deleted_at + # item, child1 and child2 should not be affected + item.refresh_from_db() + child1.refresh_from_db() + child2.refresh_from_db() + assert item.deleted_at is not None + assert item.ancestors_deleted_at == item.deleted_at + assert child1.ancestors_deleted_at == item.deleted_at + assert child2.ancestors_deleted_at == item.deleted_at + + # Restoring the grand parent should all the tree + grand_parent.restore() + grand_parent.refresh_from_db() + parent.refresh_from_db() + item.refresh_from_db() + child1.refresh_from_db() + child2.refresh_from_db() + assert grand_parent.deleted_at is None + assert grand_parent.ancestors_deleted_at is None + assert parent.deleted_at is None + assert parent.ancestors_deleted_at is None + assert item.deleted_at is not None + assert item.ancestors_deleted_at == item.deleted_at + assert child1.ancestors_deleted_at == item.deleted_at + assert child2.ancestors_deleted_at == item.deleted_at diff --git a/src/backend/drive/settings.py b/src/backend/drive/settings.py index 6efc9432..47b240f7 100755 --- a/src/backend/drive/settings.py +++ b/src/backend/drive/settings.py @@ -301,7 +301,7 @@ class Base(Configuration): "dockerflow.django", "rest_framework", "parler", - "treebeard", + "django_ltree", "easy_thumbnails", # Django "django.contrib.admin", diff --git a/src/backend/pyproject.toml b/src/backend/pyproject.toml index f6ee9f97..7696840b 100644 --- a/src/backend/pyproject.toml +++ b/src/backend/pyproject.toml @@ -27,34 +27,34 @@ requires-python = ">=3.12" dependencies = [ "boto3==1.36.7", "Brotli==1.1.0", + "django==5.1.5", "django-configurations==2.5.1", "django-cors-headers==4.6.0", "django-countries==7.6.1", "django-filter==24.3", + "django-ltree@git+https://github.com/lunika/django-ltree@e434ec4723750be54dc014b582a65dc99ae53eae", "django-parler==2.3", - "redis==5.2.1", "django-redis==5.4.0", "django-storages[s3]==1.14.4", "django-timezone-field>=5.1", - "django==5.1.5", - "django-treebeard==4.7.1", "djangorestframework==3.15.2", - "drf_spectacular==0.28.0", "dockerflow==2024.4.2", + "drf_spectacular==0.28.0", "easy_thumbnails==2.10", "factory_boy==3.3.1", "gunicorn==23.0.0", "jsonschema==4.23.0", "markdown==3.7", + "mozilla-django-oidc==4.0.1", "nested-multipart-parser==1.5.0", "psycopg[binary]==3.2.4", "PyJWT==2.10.1", "python-magic==0.4.27", + "redis==5.2.1", "requests==2.32.3", "sentry-sdk==2.20.0", "url-normalize==1.4.3", "whitenoise==6.8.2", - "mozilla-django-oidc==4.0.1", ] [project.urls]