Drastically improved coverage of cms.menu.get_visible_pages and fixed…

… some bugs in the process

cms/menu.py

@@ -1,7 +1,5 @@
 # -*- coding: utf-8 -*-
 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
@@ -18,7 +16,7 @@
 from menus.menu_pool import menu_pool
 
 
-def get_visible_pages(request, pages):
+def get_visible_pages(request, pages, site=None):
     # 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'):
@@ -37,64 +35,38 @@ def get_visible_pages(request, pages):
     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 site is None:
+        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])
+        ) & Q(can_view=True) & Q(Q(sites__in=[site.pk]) | Q(sites__isnull=True))
         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')
+    def has_global_perm():
+        if has_global_perm.cache < 0:
+            has_global_perm.cache = 1 if request.user.has_perm('cms.view_page') else 0
+        return bool(has_global_perm.cache)
+    has_global_perm.cache = -1
+
 
     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'):
+            elif 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
+            elif has_global_perm():
                 page_ids.append(page.pk)
-                continue
-        if has_global_perm:
-            page_ids.append(page.pk)
-            continue
-
-        if page.pk in generic_permissions:
+        elif not is_restricted and settings.CMS_PUBLIC_FOR == 'all':
+            # anonymous user, no restriction saved in database
             page_ids.append(page.pk)
-            continue
     return page_ids
 
 def page_to_node(page, home, cut):
@@ -187,7 +159,7 @@ def get_nodes(self, request):
         actual_pages = []
 
         # cache view perms
-        visible_pages = get_visible_pages(request, pages)
+        visible_pages = get_visible_pages(request, pages, site)
 
         for page in pages:
             # Pages are ordered by tree_id, therefore the first page is the root

cms/tests/menu.py

@@ -1,13 +1,18 @@
 # -*- coding: utf-8 -*-
 from __future__ import with_statement
 from cms.api import create_page
-from cms.menu import CMSMenu
+from cms.menu import CMSMenu, get_visible_pages
 from cms.models import Page
+from cms.models.permissionmodels import GlobalPagePermission, PagePermission
 from cms.test_utils.fixtures.menus import (MenusFixture, SubMenusFixture, 
     SoftrootFixture)
 from cms.test_utils.testcases import SettingsOverrideTestCase
+from cms.test_utils.util.context_managers import SettingsOverride
 from cms.test_utils.util.mock import AttributeObject
 from django.conf import settings
+from django.contrib.auth.models import AnonymousUser, User, Permission
+from django.contrib.contenttypes.models import ContentType
+from django.contrib.sites.models import Site
 from django.template import Template
 from menus.base import NavigationNode
 from menus.menu_pool import menu_pool, _build_nodes_inner_for_one_menu
@@ -17,7 +22,6 @@
 class BaseMenuTest(SettingsOverrideTestCase):
     settings_overrides = {
         'CMS_MODERATOR': False,
-        'DEBUG': True
     }
 
     def _get_nodes(self, path='/'):
@@ -812,3 +816,219 @@ def test_not_in_navigation_num_queries(self):
             """
             tpl = Template("{% load menu_tags %}{% show_menu_below_id 'a' 0 100 100 100 %}")
             tpl.render(context)
+
+
+class ViewPermissionMenuTests(SettingsOverrideTestCase):
+    settings_overrides = {
+        'CMS_MODERATOR': False,
+        'CMS_PERMISSION': True,
+        'CMS_PUBLIC_FOR': 'all',
+    }
+
+    def get_request(self, user=None):
+        attrs = {
+            'user': user or AnonymousUser(),
+            'REQUEST': {},
+            'session': {},
+        }
+        return type('Request', (object,), attrs)
+
+    def test_public_for_all_staff(self):
+        request = self.get_request()
+        request.user.is_staff = True
+        page = Page()
+        page.pk = 1
+        pages = [page]
+        result = get_visible_pages(request, pages)
+        self.assertEqual(result, [1])
+
+    def test_public_for_all_staff_assert_num_queries(self):
+        request = self.get_request()
+        request.user.is_staff = True
+        page = Page()
+        page.pk = 1
+        pages = [page]
+        with self.assertNumQueries(0):
+            get_visible_pages(request, pages)
+
+    def test_public_for_all(self):
+        user = User.objects.create_user('user', 'user@domain.com', 'user')
+        request = self.get_request(user)
+        page = Page()
+        page.pk = 1
+        page.level = 0
+        page.tree_id = 1
+        pages = [page]
+        result = get_visible_pages(request, pages)
+        self.assertEqual(result, [1])
+
+    def test_public_for_all_num_queries(self):
+        user = User.objects.create_user('user', 'user@domain.com', 'user')
+        request = self.get_request(user)
+        site = Site()
+        site.pk = 1
+        page = Page()
+        page.pk = 1
+        page.level = 0
+        page.tree_id = 1
+        pages = [page]
+        with self.assertNumQueries(2):
+            """
+            The two queries are:
+            PagePermission query for affected pages
+            GlobalpagePermission query for user
+            """
+            get_visible_pages(request, pages, site)
+
+    def test_unauthed(self):
+        request = self.get_request()
+        page = Page()
+        page.pk = 1
+        page.level = 0
+        page.tree_id = 1
+        pages = [page]
+        result = get_visible_pages(request, pages)
+        self.assertEqual(result, [1])
+
+    def test_unauthed_num_queries(self):
+        request = self.get_request()
+        site = Site()
+        site.pk = 1
+        page = Page()
+        page.pk = 1
+        page.level = 0
+        page.tree_id = 1
+        pages = [page]
+        with self.assertNumQueries(1):
+            """
+            The query is:
+            PagePermission query for affected pages
+            
+            global is not executed because it's lazy
+            """
+            get_visible_pages(request, pages, site)
+
+    def test_authed_basic_perm(self):
+        with SettingsOverride(CMS_PUBLIC_FOR='staff'):
+            user = User.objects.create_user('user', 'user@domain.com', 'user')
+            user.user_permissions.add(Permission.objects.get(codename='view_page'))
+            request = self.get_request(user)
+            page = Page()
+            page.pk = 1
+            page.level = 0
+            page.tree_id = 1
+            pages = [page]
+            result = get_visible_pages(request, pages)
+            self.assertEqual(result, [1])
+
+    def test_authed_basic_perm_num_queries(self):
+        site = Site()
+        site.pk = 1
+        with SettingsOverride(CMS_PUBLIC_FOR='staff'):
+            user = User.objects.create_user('user', 'user@domain.com', 'user')
+            user.user_permissions.add(Permission.objects.get(codename='view_page'))
+            request = self.get_request(user)
+            page = Page()
+            page.pk = 1
+            page.level = 0
+            page.tree_id = 1
+            pages = [page]
+            with self.assertNumQueries(4):
+                """
+                The two queries are:
+                PagePermission query for affected pages
+                GlobalpagePermission query for user
+                Generic django permission lookup
+                content type lookup by permission lookup
+                """
+                get_visible_pages(request, pages, site)
+
+    def test_authed_no_access(self):
+        with SettingsOverride(CMS_PUBLIC_FOR='staff'):
+            user = User.objects.create_user('user', 'user@domain.com', 'user')
+            request = self.get_request(user)
+            page = Page()
+            page.pk = 1
+            page.level = 0
+            page.tree_id = 1
+            pages = [page]
+            result = get_visible_pages(request, pages)
+            self.assertEqual(result, [])
+
+    def test_authed_no_access_num_queries(self):
+        site = Site()
+        site.pk = 1
+        with SettingsOverride(CMS_PUBLIC_FOR='staff'):
+            user = User.objects.create_user('user', 'user@domain.com', 'user')
+            request = self.get_request(user)
+            page = Page()
+            page.pk = 1
+            page.level = 0
+            page.tree_id = 1
+            pages = [page]
+            with self.assertNumQueries(4):
+                """
+                The two queries are:
+                PagePermission query for affected pages
+                GlobalpagePermission query for user
+                Generic django permission lookup
+                content type lookup by permission lookup
+                """
+                get_visible_pages(request, pages, site)
+
+    def test_unauthed_no_access(self):
+        with SettingsOverride(CMS_PUBLIC_FOR='staff'):
+            request = self.get_request()
+            page = Page()
+            page.pk = 1
+            page.level = 0
+            page.tree_id = 1
+            pages = [page]
+            result = get_visible_pages(request, pages)
+            self.assertEqual(result, [])
+
+    def test_unauthed_no_access_num_queries(self):
+        site = Site()
+        site.pk = 1
+        with SettingsOverride(CMS_PUBLIC_FOR='staff'):
+            request = self.get_request()
+            page = Page()
+            page.pk = 1
+            page.level = 0
+            page.tree_id = 1
+            pages = [page]
+            with self.assertNumQueries(1):
+                get_visible_pages(request, pages, site)
+
+    def test_global_permission(self):
+        user = User.objects.create_user('user', 'user@domain.com', 'user')
+        GlobalPagePermission.objects.create(can_view=True, user=user)
+        request = self.get_request(user)
+        page = Page()
+        page.pk = 1
+        page.level = 0
+        page.tree_id = 1
+        pages = [page]
+        result = get_visible_pages(request, pages)
+        self.assertEqual(result, [1])
+
+    def test_global_permission_num_queries(self):
+        site = Site()
+        site.pk = 1
+        user = User.objects.create_user('user', 'user@domain.com', 'user')
+        GlobalPagePermission.objects.create(can_view=True, user=user)
+        request = self.get_request(user)
+        site = Site()
+        site.pk = 1
+        page = Page()
+        page.pk = 1
+        page.level = 0
+        page.tree_id = 1
+        pages = [page]
+        with self.assertNumQueries(2):
+            """
+            The two queries are:
+            PagePermission query for affected pages
+            GlobalpagePermission query for user
+            """
+            get_visible_pages(request, pages, site)

cms/utils/plugins.py

@@ -131,4 +131,4 @@ def current_site(request):
             except Site.DoesNotExist:
                 return None
         else:
-            return Site.objects.get_current()
+            return Site.objects.get_current()