Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance of page tree #1088

Merged
merged 11 commits into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 0 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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/

Expand Down
2 changes: 1 addition & 1 deletion .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
229 changes: 116 additions & 113 deletions Pipfile.lock

Large diffs are not rendered by default.

90 changes: 90 additions & 0 deletions dev-tools/_duplicate_pages.py
Original file line number Diff line number Diff line change
@@ -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)
2 changes: 0 additions & 2 deletions dev-tools/_functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions dev-tools/duplicate_pages.sh
Original file line number Diff line number Diff line change
@@ -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
4 changes: 0 additions & 4 deletions dev-tools/prune_database.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion dev-tools/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion dev-tools/test_cov.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions integreat_cms/api/middleware/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from .json_debug_toolbar_middleware import JsonDebugToolbarMiddleware
42 changes: 42 additions & 0 deletions integreat_cms/api/middleware/json_debug_toolbar_middleware.py
Original file line number Diff line number Diff line change
@@ -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

This file was deleted.

5 changes: 2 additions & 3 deletions integreat_cms/api/v3/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions integreat_cms/api/v3/imprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from django.conf import settings
from django.http import JsonResponse

from ...cms.models import Region
from ..decorators import json_response


Expand Down Expand Up @@ -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:
Expand Down
25 changes: 11 additions & 14 deletions integreat_cms/api/v3/languages.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"""
from django.http import JsonResponse

from ...cms.models import Region
from ..decorators import json_response


Expand All @@ -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
5 changes: 2 additions & 3 deletions integreat_cms/api/v3/locations.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from django.conf import settings
from django.http import JsonResponse

from ...cms.models import Region
from ..decorators import json_response


Expand Down Expand Up @@ -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))
Expand Down
3 changes: 1 addition & 2 deletions integreat_cms/api/v3/offers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand Down