Permalink
Browse files

Improved menu building by getting all view permissions at once for al…

…l pages in the menu
  • Loading branch information...
Jonas Obrist
Jonas Obrist committed Aug 5, 2011
1 parent fe9f1df commit 6be78423c36aa75e378561e7e02848d6315c867e
Showing with 163 additions and 34 deletions.
  1. +97 −25 cms/menu.py
  2. +66 −9 cms/tests/menu.py
View
@@ -1,32 +1,101 @@
# -*- coding: utf-8 -*-
-from menus.menu_pool import menu_pool
-from menus.base import Menu, NavigationNode, Modifier
+from cms.apphook_pool import apphook_pool
+from cms.cache.permissions import get_permission_cache, set_permission_cache
+from cms.models.managers import PagePermissionsPermissionManager
+from cms.models.moderatormodels import (ACCESS_DESCENDANTS,
+ ACCESS_PAGE_AND_DESCENDANTS, ACCESS_CHILDREN, ACCESS_PAGE_AND_CHILDREN)
+from cms.models.permissionmodels import PagePermission, GlobalPagePermission
+from cms.models.titlemodels import Title
from cms.utils import get_language_from_request
+from cms.utils.i18n import get_fallback_languages
from cms.utils.moderator import get_page_queryset, get_title_queryset
+from cms.utils.plugins import current_site
from django.conf import settings
from django.contrib.sites.models import Site
-from cms.utils.i18n import get_fallback_languages
-from cms.apphook_pool import apphook_pool
-from cms.models.titlemodels import Title
+from django.db.models.query_utils import Q
+from django.utils.functional import lazy
+from menus.base import Menu, NavigationNode, Modifier
+from menus.menu_pool import menu_pool
+
-def populate_ancestors(page_list):
- ancestors = []
- populated_pages = []
- for idx, page in enumerate(page_list):
- if idx == 0:
- ancestors = page.get_cached_ancestors(ascending=True)
- try:
- next_page = page_list[idx+1]
- except IndexError:
- next_page = None
- page.ancestors_ascending = ancestors
- if next_page:
- if next_page.level > page.level:
- ancestors.append(page)
- if next_page.level < page.level:
- ancestors.pop()
- populated_pages.append(page)
- return populated_pages
+def get_visible_pages(request, pages):
+ # This code is basically a many-pages-at-once version of
+ # Page.has_view_permission, check there to see wtf is going on here.
+ if request.user.is_staff and settings.CMS_PUBLIC_FOR in ('staff', 'all'):
+ return [page.pk for page in pages]
+ page_ids = []
+
+ pages_perms_q = Q()
+ for page in pages:
+ page_q = Q(page__tree_id=page.tree_id) & (
+ Q(page=page)
+ | (Q(page__level__lt=page.level) & (Q(grant_on=ACCESS_DESCENDANTS) | Q(grant_on=ACCESS_PAGE_AND_DESCENDANTS)))
+ | (Q(page__level=page.level - 1) & (Q(grant_on=ACCESS_CHILDREN) | Q(grant_on=ACCESS_PAGE_AND_CHILDREN)))
+ )
+ pages_perms_q |= page_q
+ pages_perms_q &= Q(can_view=True)
+ page_permissions = PagePermission.objects.filter(pages_perms_q).select_related('page')
+ restriced_pages = [page_permission.page.pk for page_permission in page_permissions]
+
+ site = current_site(request)
+
+ if request.user.is_authenticated():
+ #return self.filter(Q(user=user) | Q(group__user=user))
+ global_page_perm_q = Q(
+ Q(user=request.user) | Q(group__user=request.user)
+ ) & Q(can_view=True, sites__in=[site.pk])
+ global_view_perms = GlobalPagePermission.objects.filter(global_page_perm_q).exists()
+
+ def get_generic_permissions():
+ if request.user.is_superuser or not settings.CMS_PERMISSION:
+ # got superuser, or permissions aren't enabled? just return grant
+ # all mark
+ return [page.pk for page in pages]
+ # read from cache if posssible
+ cached = get_permission_cache(request.user, 'can_view')
+ if cached is not None:
+ return cached
+ return []
+
+ generic_permissions = lazy(get_generic_permissions, list)()
+
+ has_global_perm = lazy(request.user.has_perm, bool)('cms.view_page')
+
+ for page in pages:
+ # TODO: needs to be refactored to not contain continue statements
+ is_restricted = page.pk in restriced_pages
+
+ if request.user.is_authenticated():
+ # a global permission was given to the request's user
+ if global_view_perms:
+ page_ids.append(page.pk)
+ continue
+ # authenticated user, no restriction and public for all fallback
+ if (not is_restricted and not global_view_perms and
+ not settings.CMS_PUBLIC_FOR == 'all'):
+ continue
+ # authenticated user, no restriction and public for all
+ if (not is_restricted and not global_view_perms and
+ settings.CMS_PUBLIC_FOR == 'all'):
+ page_ids.append(page.pk)
+ continue
+ else:
+ #anonymous user
+ if is_restricted or not settings.CMS_PUBLIC_FOR == 'all':
+ # anyonymous user, page has restriction and global access is permitted
+ continue
+ else:
+ # anonymous user, no restriction saved in database
+ page_ids.append(page.pk)
+ continue
+ if has_global_perm:
+ page_ids.append(page.pk)
+ continue
+
+ if page.pk in generic_permissions:
+ page_ids.append(page.pk)
+ continue
+ return page_ids
def page_to_node(page, home, cut):
'''
@@ -116,11 +185,14 @@ def get_nodes(self, request):
home_children = []
home = None
actual_pages = []
+
+ # cache view perms
+ visible_pages = get_visible_pages(request, pages)
- for page in populate_ancestors(pages):
+ for page in pages:
# Pages are ordered by tree_id, therefore the first page is the root
# of the page tree (a.k.a "home")
- if not page.has_view_permission(request):
+ if page.pk not in visible_pages:
# Don't include pages the user doesn't have access to
continue
if not home:
View
@@ -81,9 +81,8 @@ def test_basic_cms_menu(self):
def test_show_menu(self):
context = self.get_context()
# test standard show_menu
- with self.assertNumQueries(1):
- tpl = Template("{% load menu_tags %}{% show_menu %}")
- tpl.render(context)
+ tpl = Template("{% load menu_tags %}{% show_menu %}")
+ tpl.render(context)
nodes = context['children']
self.assertEqual(len(nodes), 2)
self.assertEqual(nodes[0].selected, True)
@@ -96,6 +95,20 @@ def test_show_menu(self):
self.assertEqual(nodes[1].sibling, True)
self.assertEqual(nodes[1].selected, False)
+ def test_show_menu_num_queries(self):
+ context = self.get_context()
+ # test standard show_menu
+ with self.assertNumQueries(4):
+ """
+ The 4 queries should be:
+ get all pages
+ get all page permissions
+ get all titles
+ set the menu cache key
+ """
+ tpl = Template("{% load menu_tags %}{% show_menu %}")
+ tpl.render(context)
+
def test_only_active_tree(self):
context = self.get_context()
# test standard show_menu
@@ -716,12 +729,26 @@ def test_show_submenu(self):
page = Page.objects.get(title_set__title='P6')
context = self.get_context(page.get_absolute_url())
# test standard show_menu
- with self.assertNumQueries(19):
- tpl = Template("{% load menu_tags %}{% show_sub_menu %}")
- tpl.render(context)
+ tpl = Template("{% load menu_tags %}{% show_sub_menu %}")
+ tpl.render(context)
nodes = context['children']
self.assertEqual(len(nodes), 1)
self.assertEqual(nodes[0].id, 8)
+
+ def test_show_submenu_num_queries(self):
+ page = Page.objects.get(title_set__title='P6')
+ context = self.get_context(page.get_absolute_url())
+ # test standard show_menu
+ with self.assertNumQueries(4):
+ """
+ The 4 queries should be:
+ get all pages
+ get all page permissions
+ get all titles
+ set the menu cache key
+ """
+ tpl = Template("{% load menu_tags %}{% show_sub_menu %}")
+ tpl.render(context)
class ShowMenuBelowIdTests(BaseMenuTest):
def test_not_in_navigation(self):
@@ -744,9 +771,8 @@ def test_not_in_navigation(self):
create_page('D', 'nav_playground.html', 'en', parent=self.reload(b),
published=True, in_navigation=False)
context = self.get_context(a.get_absolute_url())
- with self.assertNumQueries(11):
- tpl = Template("{% load menu_tags %}{% show_menu_below_id 'a' 0 100 100 100 %}")
- tpl.render(context)
+ tpl = Template("{% load menu_tags %}{% show_menu_below_id 'a' 0 100 100 100 %}")
+ tpl.render(context)
nodes = context['children']
self.assertEqual(len(nodes), 1, nodes)
node = nodes[0]
@@ -755,3 +781,34 @@ def test_not_in_navigation(self):
self.assertEqual(len(children), 1, repr(children))
child = children[0]
self.assertEqual(child.id, c.id)
+
+ def test_not_in_navigation_num_queries(self):
+ """
+ Test for issue 521
+
+ Build the following tree:
+
+ A
+ |-B
+ |-C
+ \-D (not in nav)
+ """
+ a = create_page('A', 'nav_playground.html', 'en', published=True,
+ in_navigation=True, reverse_id='a')
+ b =create_page('B', 'nav_playground.html', 'en', parent=a,
+ published=True, in_navigation=True)
+ c = create_page('C', 'nav_playground.html', 'en', parent=b,
+ published=True, in_navigation=True)
+ create_page('D', 'nav_playground.html', 'en', parent=self.reload(b),
+ published=True, in_navigation=False)
+ context = self.get_context(a.get_absolute_url())
+ with self.assertNumQueries(4):
+ """
+ The 4 queries should be:
+ get all pages
+ get all page permissions
+ get all titles
+ set the menu cache key
+ """
+ tpl = Template("{% load menu_tags %}{% show_menu_below_id 'a' 0 100 100 100 %}")
+ tpl.render(context)

0 comments on commit 6be7842

Please sign in to comment.