Merge pull request #1088 from digitalfabrik/refactoring/page-performance

Improve performance of page tree

.circleci/config.yml

@@ -134,14 +134,15 @@ jobs:
       - checkout
       - attach_workspace:
           at: .
+      - run:
+          name: Check for missing migrations
+          command: pipenv run integreat-cms-cli makemigrations cms --check --settings=integreat_cms.core.circleci_settings
       - run:
           name: Migrate database
-          command: |
-            pipenv run integreat-cms-cli makemigrations cms --settings=integreat_cms.core.circleci_settings
-            pipenv run integreat-cms-cli migrate --settings=integreat_cms.core.circleci_settings
+          command: pipenv run integreat-cms-cli migrate --settings=integreat_cms.core.circleci_settings
       - run:
           name: Run tests
-          command: pipenv run integreat-cms-cli test integreat_cms --set=COVERAGE --settings=integreat_cms.core.circleci_settings
+          command: pipenv run integreat-cms-cli test tests --set=COVERAGE --settings=integreat_cms.core.circleci_settings
       - store_artifacts:
           path: htmlcov
   build-package:

.gitignore

@@ -29,9 +29,6 @@ htmlcov/
 # Compiled translations
 integreat_cms/locale/de/LC_MESSAGES/django.mo
 
-# Migrations
-integreat_cms/cms/migrations/
-
 # Compressed JS & CSS Files
 integreat_cms/static/dist/
 

.pylintrc

@@ -23,7 +23,7 @@ extension-pkg-whitelist=lxml
 
 # Add files or directories to the blacklist. They should be base names, not
 # paths.
-ignore=.venv,node_modules,migrations,build
+ignore=.venv,node_modules,build
 
 # Add files or directories matching the regex patterns to the blacklist. The
 # regex matches against base names, not paths.

CHANGELOG.md

@@ -9,6 +9,10 @@ UNRELEASED
 * Add APIv3 parents/ancestors endpoint
 * Add API tests
 * Improve performance of feedback list
+* Replace django-mptt by django-treebeard
+* Improve performance of page tree, event and POI lists
+* Improve performance of page, event and POI API endpoints
+* Add database migrations
 
 
 2021.12.0-beta

dev-tools/_duplicate_pages.py

@@ -0,0 +1,90 @@
+"""
+Please run this script via the dev tool duplicate_pages.sh to make sure all environment
+variables are passed correctly and Django can be set up.
+"""
+import copy
+import logging
+import random
+import string
+import sys
+
+from django.utils.text import slugify
+
+logger = logging.getLogger(__name__)
+
+if __name__ != "django.core.management.commands.shell":
+    logger.error("Please run this script via the dev tool duplicate_pages.sh")
+    sys.exit()
+
+# pylint: disable=wrong-import-position
+from integreat_cms.cms.models import Region, Page
+
+
+def duplicate_page(old_page, new_parent=None):
+    """
+    Duplicate a page and insert it as child of the given new parent
+
+    :param old_page: The old page which should be copied
+    :type old_page: ~integreat_cms.cms.models.pages.page.Page
+
+    :param new_parent: The new parent where the duplicate should reside
+    :type new_parent: ~integreat_cms.cms.models.pages.page.Page
+
+    :return: The copied page
+    :rtype: ~integreat_cms.cms.models.pages.page.Page
+    """
+    if new_parent:
+        # Re-query from database to update tree structure
+        new_parent = Page.objects.get(id=new_parent.id)
+    logger.debug("Duplicating page %r with new parent %r", old_page, new_parent)
+    translations = old_page.translations.all()
+    logger.debug("Old translations: %r", translations)
+    new_page = copy.deepcopy(old_page)
+    new_page.id = None
+    # pylint: disable=protected-access
+    new_page._state.adding = True
+    if new_parent:
+        new_page = new_parent.add_child(instance=new_page)
+    else:
+        new_page = Page.add_root(instance=new_page)
+    # Fix parent field
+    new_page = Page.objects.get(id=new_page.id)
+    new_page.save()
+    for translation in translations:
+        rand_str = "".join(random.choices(string.printable, k=5))
+        new_title = translation.title.split(" - ")[0] + f" - {rand_str}"
+        translation.id = None
+        translation.page = new_page
+        translation.title = new_title
+        translation.slug = slugify(new_title)
+        translation.save()
+    logger.debug("New translations: %r", new_page.translations.all())
+    return new_page
+
+
+def duplicate_pages(region, old_parent=None, new_parent=None):
+    """
+    Duplicate pages recursively for the given region
+
+    :param region: The given region
+    :type region: ~integreat_cms.cms.models.regions.region.Region
+
+    :param old_parent: The old parent page which descendants should be copied
+    :type old_parent: ~integreat_cms.cms.models.pages.page.Page
+
+    :param new_parent: The new parent where the duplicates should reside
+    :type new_parent: ~integreat_cms.cms.models.pages.page.Page
+    """
+    logger.info(
+        "Duplicating pages for old parent %r and new parent %r",
+        old_parent,
+        new_parent,
+    )
+    for old_page in region.pages.filter(parent=old_parent):
+        new_page = duplicate_page(old_page, new_parent=new_parent)
+        duplicate_pages(region, old_parent=old_page, new_parent=new_page)
+
+
+selected_region = Region.objects.first()
+logger.info("Duplicating pages for region %r", selected_region)
+duplicate_pages(selected_region)

dev-tools/_functions.sh

@@ -219,8 +219,6 @@ function migrate_database {
         deescalate_privileges pipenv run integreat-cms-cli makemigrations --verbosity "${SCRIPT_VERBOSITY}"
         # Execute migrations
         deescalate_privileges pipenv run integreat-cms-cli migrate --verbosity "${SCRIPT_VERBOSITY}"
-        # Load the role fixtures
-        deescalate_privileges pipenv run integreat-cms-cli loaddata "${PACKAGE_DIR}/cms/fixtures/roles.json" --verbosity "${SCRIPT_VERBOSITY}"
         echo "✔ Finished database migrations" | print_success
         DATABASE_MIGRATED=1
     fi

dev-tools/duplicate_pages.sh

@@ -0,0 +1,16 @@
+#!/bin/bash
+
+# This script duplicates all pages of the first region in the database
+
+# Import utility functions
+# shellcheck source=./dev-tools/_functions.sh
+source "$(dirname "${BASH_SOURCE[0]}")/_functions.sh"
+
+require_installed
+require_database
+
+# Initialize the Redis cache to make sure the cache is invalidated
+configure_redis_cache
+
+deescalate_privileges pipenv run integreat-cms-cli shell --verbosity "${SCRIPT_VERBOSITY}" < "${DEV_TOOL_DIR}/_duplicate_pages.py"
+echo "✔ Duplicated pages" | print_success

dev-tools/prune_database.sh

@@ -50,10 +50,6 @@ else
 
 fi
 
-# Remove migrations
-rm -rfv "${PACKAGE_DIR:?}/cms/migrations"
-echo "Removed database migrations" | print_info
-
 # Remove media files (because they are no longer usable without the corresponding database entries)
 rm -rfv "${PACKAGE_DIR:?}/media"
 echo "Removed media files" | print_info

dev-tools/test.sh

@@ -9,5 +9,5 @@ source "$(dirname "${BASH_SOURCE[0]}")/_functions.sh"
 require_installed
 require_database
 
-deescalate_privileges pipenv run integreat-cms-cli test integreat_cms --verbosity "${SCRIPT_VERBOSITY}"
+deescalate_privileges pipenv run integreat-cms-cli test tests --verbosity "${SCRIPT_VERBOSITY}"
 echo "✔ Tests successfully completed " | print_success

dev-tools/test_cov.sh

@@ -12,7 +12,7 @@ rm -rf "${BASE_DIR:?}/htmlcov/"
 require_installed
 require_database
 
-deescalate_privileges pipenv run integreat-cms-cli test integreat_cms --set=COVERAGE --verbosity "${SCRIPT_VERBOSITY}"
+deescalate_privileges pipenv run integreat-cms-cli test tests --set=COVERAGE --verbosity "${SCRIPT_VERBOSITY}"
 echo "✔ Tests successfully completed " | print_success
 echo -e "Open the following file in your browser to view the test coverage:\n" | print_info
 echo -e "\tfile://${BASE_DIR}/htmlcov/index.html\n" | print_bold

integreat_cms/api/middleware/__init__.py

@@ -0,0 +1 @@
+from .json_debug_toolbar_middleware import JsonDebugToolbarMiddleware

integreat_cms/api/middleware/json_debug_toolbar_middleware.py

@@ -0,0 +1,42 @@
+import json
+from django.http import HttpResponse
+
+
+# pylint: disable=too-few-public-methods
+class JsonDebugToolbarMiddleware:
+    """
+    The Django Debug Toolbar usually only works for views that return HTML.
+    This middleware wraps any JSON response in HTML if the request
+    has a 'debug' query parameter (e.g. http://localhost:8000/api/augsburg/de/pages?debug)
+    """
+
+    def __init__(self, get_response):
+        """
+        Initialize the middleware for the current view
+
+        :param get_response: A callable to get the response for the current request
+        :type get_response: ~collections.abc.Callable
+        """
+        self.get_response = get_response
+
+    def __call__(self, request):
+        """
+        Call the middleware for the current request
+
+        :param request: Django request
+        :type request: ~django.http.HttpRequest
+
+        :return: The modified response
+        :rtype: ~django.http.HttpResponse
+        """
+        response = self.get_response(request)
+        if "debug" in request.GET:
+            if response["Content-Type"] == "application/json":
+                content = json.dumps(
+                    json.loads(response.content), sort_keys=True, indent=2
+                )
+                response = HttpResponse(
+                    f"<!DOCTYPE html><html><body><pre>{content}</pre></body></html>"
+                )
+
+        return response

integreat_cms/api/v3/events.py

@@ -3,7 +3,6 @@
 from django.http import JsonResponse
 from django.utils import timezone
 
-from ...cms.models import Region
 from ...cms.models.events.event_translation import EventTranslation
 from ...cms.utils.slug_utils import generate_unique_slug
 from ..decorators import json_response
@@ -149,10 +148,10 @@ def events(request, region_slug, language_slug):
     :return: JSON object according to APIv3 events endpoint definition
     :rtype: ~django.http.JsonResponse
     """
-    region = Region.get_current_region(request)
+    region = request.region
     result = []
     now = timezone.now().date()
-    for event in region.events.filter(archived=False):
+    for event in region.events.prefetch_public_translations().filter(archived=False):
         event_translation = event.get_public_translation(language_slug)
         if event_translation:
             if event.end_date >= now:

integreat_cms/api/v3/imprint.py

@@ -4,7 +4,6 @@
 from django.conf import settings
 from django.http import JsonResponse
 
-from ...cms.models import Region
 from ..decorators import json_response
 
 
@@ -54,7 +53,7 @@ def imprint(request, region_slug, language_slug):
     :return: JSON object according to APIv3 imprint endpoint definition
     :rtype: ~django.http.JsonResponse
     """
-    region = Region.get_current_region(request)
+    region = request.region
     if hasattr(region, "imprint"):
         imprint_translation = region.imprint.get_public_translation(language_slug)
         if imprint_translation:

integreat_cms/api/v3/languages.py

@@ -3,7 +3,6 @@
 """
 from django.http import JsonResponse
 
-from ...cms.models import Region
 from ..decorators import json_response
 
 
@@ -21,20 +20,18 @@ def languages(request, region_slug):
     :return: JSON object according to APIv3 languages endpoint definition
     :rtype: ~django.http.JsonResponse
     """
-    region = Region.get_current_region(request)
+    region = request.region
 
-    result = list(
-        map(
-            lambda l: {
-                "id": l.language.id,
-                "code": l.language.slug,
-                "bcp47_tag": l.language.bcp47_tag,
-                "native_name": l.language.native_name,
-                "dir": l.language.text_direction,
-            },
-            region.language_tree_nodes.filter(active=True),
-        )
-    )
+    result = [
+        {
+            "id": language.id,
+            "code": language.slug,
+            "bcp47_tag": language.bcp47_tag,
+            "native_name": language.native_name,
+            "dir": language.text_direction,
+        }
+        for language in region.visible_languages
+    ]
     return JsonResponse(
         result, safe=False
     )  # Turn off Safe-Mode to allow serializing arrays

integreat_cms/api/v3/locations.py

@@ -1,7 +1,6 @@
 from django.conf import settings
 from django.http import JsonResponse
 
-from ...cms.models import Region
 from ..decorators import json_response
 
 
@@ -79,9 +78,9 @@ def locations(request, region_slug, language_slug):
     :return: JSON object according to APIv3 locations endpoint definition
     :rtype: ~django.http.JsonResponse
     """
-    region = Region.get_current_region(request)
+    region = request.region
     result = []
-    for poi in region.pois.filter(archived=False):
+    for poi in region.pois.prefetch_public_translations().filter(archived=False):
         translation = poi.get_public_translation(language_slug)
         if translation:
             result.append(transform_poi_translation(translation))

integreat_cms/api/v3/offers.py

@@ -4,7 +4,6 @@
 from django.http import JsonResponse
 
 from ...cms.constants import postal_code
-from ...cms.models import Region
 from ..decorators import json_response
 
 
@@ -92,7 +91,7 @@ def offers(request, region_slug, language_slug=None):
     :return: JSON object according to APIv3 offers endpoint definition
     :rtype: ~django.http.JsonResponse
     """
-    region = Region.get_current_region(request)
+    region = request.region
     result = [transform_offer(offer, region) for offer in region.offers.all()]
     return JsonResponse(
         result, safe=False