diff --git a/combo/data/migrations/0035_page_related_cells.py b/combo/data/migrations/0035_page_related_cells.py new file mode 100644 index 00000000..34406ceb --- /dev/null +++ b/combo/data/migrations/0035_page_related_cells.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.12 on 2018-06-02 11:03 +from __future__ import unicode_literals + +from django.db import migrations +import jsonfield.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('data', '0034_redirect'), + ] + + operations = [ + migrations.AddField( + model_name='page', + name='related_cells', + field=jsonfield.fields.JSONField(blank=True, default=dict), + ), + ] diff --git a/combo/data/models.py b/combo/data/models.py index 825ab3bc..f48c2f53 100644 --- a/combo/data/models.py +++ b/combo/data/models.py @@ -34,7 +34,7 @@ from django.core.exceptions import ObjectDoesNotExist, ValidationError, Permissi from django.core import serializers from django.db import models, transaction from django.db.models.base import ModelBase -from django.db.models.signals import pre_save +from django.db.models.signals import pre_save, post_save, post_delete from django.db.models import Max from django.dispatch import receiver from django.forms import models as model_forms @@ -141,6 +141,9 @@ class Page(models.Model): snapshot = models.ForeignKey('PageSnapshot', on_delete=models.CASCADE, null=True, related_name='temporary_page') + # keep a cached list of cell types that are used in the pages. + related_cells = JSONField(blank=True) + _level = None _children = None @@ -154,6 +157,8 @@ class Page(models.Model): return (self.get_online_url().strip('/'), ) def save(self, *args, **kwargs): + if not self.id: + self.related_cells = {'cell_types': []} if not self.order: max_order = Page.objects.all().aggregate(Max('order')).get('order__max') or 0 self.order = max_order + 1 @@ -295,7 +300,16 @@ class Page(models.Model): return element_is_visible(self, user=user) def get_cells(self): - return CellBase.get_cells(page_id=self.id) + return CellBase.get_cells(page=self) + + def build_cell_cache(self): + cells = CellBase.get_cells(page=self, skip_cell_cache=True) + cell_types = set() + for cell in cells: + cell_types.add(cell.get_cell_type_str()) + if cell_types != set(self.related_cells.get('cell_types', [])): + self.related_cells['cell_types'] = list(cell_types) + self.save() def get_serialized_page(self): cells = [x for x in self.get_cells() if x.placeholder and not x.placeholder.startswith('_')] @@ -304,6 +318,8 @@ class Page(models.Model): del serialized_page['model'] if 'snapshot' in serialized_page: del serialized_page['snapshot'] + if 'related_cells' in serialized_page['fields']: + del serialized_page['fields']['related_cells'] serialized_page['cells'] = json.loads(serializers.serialize('json', cells, use_natural_foreign_keys=True, use_natural_primary_keys=True)) serialized_page['fields']['groups'] = [x[0] for x in serialized_page['fields']['groups']] @@ -499,10 +515,27 @@ class CellBase(models.Model): return cell_types @classmethod - def get_cells(cls, cell_filter=None, **kwargs): + def get_cells(cls, cell_filter=None, skip_cell_cache=False, **kwargs): """Returns the list of cells of various classes matching **kwargs""" cells = [] - for klass in get_cell_classes(): + pages = [] + if 'page' in kwargs: + pages = [kwargs['page']] + elif 'page__in' in kwargs: + pages = kwargs['page__in'] + cell_classes = get_cell_classes() + if pages and not skip_cell_cache: + # if there's a request for some specific pages, limit cell types + # to those that are actually in use in those pages. + cell_types = set() + for page in pages: + if page.related_cells and 'cell_types' in page.related_cells: + cell_types |= set(page.related_cells['cell_types']) + else: + break + else: + cell_classes = [get_cell_class(x) for x in cell_types] + for klass in cell_classes: if cell_filter and not cell_filter(klass): continue cells.extend(klass.objects.filter(**kwargs)) @@ -935,26 +968,26 @@ class ParentContentCell(CellBase): def get_parents_cells(self, hierarchy): try: - page_ids = [Page.objects.get(slug='index', parent=None).id] + pages = [Page.objects.get(slug='index', parent=None)] except Page.DoesNotExist: - page_ids = [] - page_ids.extend([x.id for x in hierarchy]) - if not page_ids: + pages = [] + pages.extend(hierarchy) + if not pages: return [] - if len(page_ids) > 1 and page_ids[0] == page_ids[1]: + if len(pages) > 1 and pages[0].id == pages[1].id: # don't duplicate index cells for real children of the index page. - page_ids = page_ids[1:] + pages = pages[1:] cells_by_page = {} - for page_id in page_ids: - cells_by_page[page_id] = [] - for cell in list(CellBase.get_cells(placeholder=self.placeholder, page__in=page_ids)): + for page in pages: + cells_by_page[page.id] = [] + for cell in list(CellBase.get_cells(placeholder=self.placeholder, page__in=pages)): cells_by_page[cell.page_id].append(cell) - cells = cells_by_page[page_ids[-1]] - for page_id in reversed(page_ids[:-1]): + cells = cells_by_page[pages[-1].id] + for page in reversed(pages[:-1]): for i, cell in enumerate(cells): if isinstance(cell, ParentContentCell): - cells[i:i+1] = cells_by_page[page_id] + cells[i:i+1] = cells_by_page[page.id] break else: # no more ParentContentCell, stop folloing the parent page chain @@ -1302,3 +1335,13 @@ def create_redirects(sender, instance, raw, **kwargs): affected_pages.extend(level_pages) for page in affected_pages: Redirect(page=page, old_url=page.get_online_url()).save() + +@receiver(post_save) +@receiver(post_delete) +def cell_maintain_page_cell_cache(sender, instance=None, **kwargs): + if not issubclass(sender, CellBase): + return + if not instance.page_id: + return + page = instance.page + page.build_cell_cache() diff --git a/combo/public/views.py b/combo/public/views.py index b55c035f..d046ecf4 100644 --- a/combo/public/views.py +++ b/combo/public/views.py @@ -239,7 +239,7 @@ def skeleton(request): if placeholder.acquired: cells.append(ParentContentCell(page=selected_page, placeholder=placeholder.key, order=0)) else: - cells = CellBase.get_cells(page_id=selected_page.id) + cells = CellBase.get_cells(page=selected_page) pages = selected_page.get_parents_and_self() combo_template = settings.COMBO_PUBLIC_TEMPLATES[selected_page.template_name] @@ -380,7 +380,7 @@ def publish_page(request, page, status=200, template_name=None): if page.redirect_url: return HttpResponseRedirect(page.get_redirect_url()) - cells = CellBase.get_cells(page_id=page.id) + cells = CellBase.get_cells(page=page) extend_with_parent_cells(cells, hierarchy=pages) cells = [x for x in cells if x.is_visible(user=request.user)] diff --git a/tests/test_cells.py b/tests/test_cells.py index f4b02c9b..4ff246c3 100644 --- a/tests/test_cells.py +++ b/tests/test_cells.py @@ -6,13 +6,16 @@ import requests from combo.data.models import Page, CellBase, TextCell, LinkCell, JsonCellBase, JsonCell, ConfigJsonCell from django.conf import settings +from django.db import connection from django.forms.widgets import Media from django.template import Context from django.test import override_settings from django.test.client import RequestFactory +from django.test.utils import CaptureQueriesContext from django.contrib.auth.models import User from django.core.urlresolvers import reverse +from combo.data.library import get_cell_classes from combo.utils import NothingInCacheException from test_manager import admin_user, login @@ -627,3 +630,44 @@ def test_page_cell_placeholder_restricted_visibility(app, admin_user): assert "

Public text

" not in resp.content assert "

Private text

" in resp.content + +def test_related_cell_types_tracking(): + page = Page(title='example page', slug='example-page') + page.save() + assert page.related_cells['cell_types'] == [] + + TextCell(page=page, placeholder='content', order=0, text='hello').save() + assert Page.objects.get(id=page.id).related_cells['cell_types'] == ['data_textcell'] + + TextCell(page=page, placeholder='content', order=1, text='hello').save() + assert Page.objects.get(id=page.id).related_cells['cell_types'] == ['data_textcell'] + + LinkCell(page=page, placeholder='content', order=0, title='Test', url='http://example.net').save() + assert set(Page.objects.get(id=page.id).related_cells['cell_types']) == set(['data_textcell', 'data_linkcell']) + + with CaptureQueriesContext(connection) as ctx: + assert len(CellBase.get_cells(page=Page.objects.get(id=page.id))) == 3 + assert len(ctx.captured_queries) == 1 + 2 + + TextCell.objects.get(order=1).delete() + assert set(Page.objects.get(id=page.id).related_cells['cell_types']) == set(['data_textcell', 'data_linkcell']) + + TextCell.objects.get(order=0).delete() + assert set(Page.objects.get(id=page.id).related_cells['cell_types']) == set(['data_linkcell']) + + with CaptureQueriesContext(connection) as ctx: + assert len(CellBase.get_cells(page=Page.objects.get(id=page.id))) == 1 + assert len(ctx.captured_queries) == 1 + 1 + + # remove tracker, check it is rebuilt correctly + page.related_cells = {} + page.save() + + with CaptureQueriesContext(connection) as ctx: + assert len(CellBase.get_cells(page=Page.objects.get(id=page.id))) == 1 + assert len(ctx.captured_queries) == len(get_cell_classes()) + + Page.objects.get(id=page.id).get_cells() + + TextCell(page=page, placeholder='content', order=0, text='hello').save() + assert set(Page.objects.get(id=page.id).related_cells['cell_types']) == set(['data_textcell', 'data_linkcell']) diff --git a/tests/test_fargo.py b/tests/test_fargo.py index 7c1cea6f..1eeb4d6c 100644 --- a/tests/test_fargo.py +++ b/tests/test_fargo.py @@ -28,7 +28,7 @@ def test_manager_fargo_cell_setup(app, admin_user): resp = app.get(resp.html.find('option', **{'data-add-url': re.compile('recentdoc')})['data-add-url']) - cells = page.get_cells() + cells = Page.objects.get(id=page.id).get_cells() assert len(cells) == 1 assert isinstance(cells[0], RecentDocumentsCell) resp = app.get('/manage/pages/%s/' % page.id) @@ -42,10 +42,10 @@ def test_manager_fargo_cell_setup(app, admin_user): 'backoffice-menu-url': 'http://example.org/manage/',}, }}): resp = app.get('/manage/pages/%s/' % page.id) - cells = page.get_cells() + cells = Page.objects.get(id=page.id).get_cells() resp.forms[0]['c%s-fargo_site' % cells[0].get_reference()].value = 'second' resp = resp.forms[0].submit() - cells = page.get_cells() + cells = Page.objects.get(id=page.id).get_cells() assert cells[0].fargo_site == 'second' diff --git a/tests/test_wcs.py b/tests/test_wcs.py index 129e587e..3bdba821 100644 --- a/tests/test_wcs.py +++ b/tests/test_wcs.py @@ -271,8 +271,9 @@ def test_form_cell_load(): cell.save() site_export = [page.get_serialized_page()] cell.delete() - assert not page.get_cells() + assert not Page.objects.get(id=page.id).get_cells() Page.load_serialized_pages(site_export) + page = Page.objects.get(slug='test_form_cell_save_cache') cells = page.get_cells() assert len(cells) == 1 cell = cells[0] @@ -518,7 +519,7 @@ def test_manager_forms_of_category_cell(app, admin_user): resp = app.get(resp.html.find('option', **{'data-add-url': re.compile('wcsformsofcategorycell')})['data-add-url']) - cells = page.get_cells() + cells = Page.objects.get(id=page.id).get_cells() assert len(cells) == 1 assert isinstance(cells[0], WcsFormsOfCategoryCell) @@ -538,7 +539,7 @@ def test_manager_current_forms(app, admin_user): resp = app.get(resp.html.find('option', **{'data-add-url': re.compile('wcscurrentformscell')})['data-add-url']) - cells = page.get_cells() + cells = Page.objects.get(id=page.id).get_cells() assert len(cells) == 1 assert isinstance(cells[0], WcsCurrentFormsCell)