--- a/gpp/forums/models.py	Sat Jul 02 03:52:43 2011 +0000
+++ b/gpp/forums/models.py	Sat Jul 02 23:35:45 2011 +0000
@@ -30,24 +30,6 @@
     def __unicode__(self):
         return self.name
 
-    def can_access(self, user):
-        """
-        Checks to see if the given user has permission to access
-        this category.
-        If this category has no groups assigned to it, return true.
-        Else, return true if the user belongs to a group that has been
-        assigned to this category, and false otherwise.
-        """
-        if self.groups.count() == 0:
-            return True
-        if user.is_authenticated():
-            return self.groups.filter(user__pk=user.id).count() > 0
-        return False
-
-    def is_public(self):
-        """Returns true if this is a public category, viewable by all."""
-        return self.groups.count() == 0
-
 
 class ForumManager(models.Manager):
     """

--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/gpp/forums/permissions.py	Sat Jul 02 23:35:45 2011 +0000
@@ -0,0 +1,114 @@
+"""
+This module does permissions checking for the forums application.
+
+"""
+from django.core.cache import cache
+
+# How long (in secs) to cache group information for various entities:
+CATEGORY_TIMEOUT = 4 * 60 * 60
+FORUM_TIMEOUT = 4 * 60 * 60
+USER_TIMEOUT = 15 * 60
+
+
+def can_access(category, user):
+    """
+    This function returns True if the given user can access the forum category
+    and False otherwise.
+
+    """
+    if user.is_superuser:
+        return True
+
+    # If this category has no groups assigned to it, return True. Else, return
+    # True if the user belongs to a group that has been assigned to this
+    # category, and False otherwise.
+
+    # Get the groups assigned to this category.
+    cat_groups = get_category_groups(category)
+
+    if len(cat_groups) == 0:
+        return True         # No groups => public category
+
+    user_groups = get_user_groups(user)
+    return bool(user_groups & cat_groups)
+
+
+def can_moderate(forum, user):
+    """
+    Returns True if the user can moderate the forum.
+
+    """
+    # Get the simple cases out of the way first:
+    if not user.is_authenticated():
+        return False
+    elif user.is_superuser:
+        return True
+
+    # If we get here, we have to see if there is an intersection between the
+    # user's groups and the forum's moderator groups.
+
+    forum_groups = get_forum_groups(forum)
+    user_groups = get_user_groups(user)
+
+    return bool(user_groups & forum_groups)
+
+
+def can_post(topic, user):
+    """
+    Returns True if the user can post in the topic and False otherwise.
+
+    """
+    if not user.is_authenticated():
+        return False
+    if user.is_superuser or can_moderate(topic.forum, user):
+        return True
+
+    return not topic.locked and can_access(topic.forum.category, user)
+
+
+def get_user_groups(user):
+    """
+    Returns a set of group ID's that the user belongs to.
+
+    """
+    user_groups_key = '%s_groups' % user.username
+    return _get_groups(user_groups_key, user.groups.all(), USER_TIMEOUT)
+
+
+def get_forum_groups(forum):
+    """
+    Returns a set of group ID's of the forum's moderator groups.
+
+    """
+    forum_groups_key = 'forum_%d_mods' % forum.id
+    return _get_groups(forum_groups_key, forum.moderators.all(), FORUM_TIMEOUT)
+
+
+def get_category_groups(category):
+    """
+    Returns a set of group ID's of the groups that can access this forum
+    category.
+
+    """
+    cat_groups_key = 'cat_%d_groups' % category.id
+    return _get_groups(cat_groups_key, category.groups.all(), CATEGORY_TIMEOUT)
+
+
+def _get_groups(key, qs, timeout):
+    """
+    This internal function contains the code common to the get_xxx_groups()
+    functions. Returns a set of group ID's from the cache. If the set is not
+    found in the cache, the set is generated from the queryset qs and cached
+    with the given timeout.
+
+    key - the cache key for the set of group ID's
+    qs - the query set of groups to query if the set is not in the cache
+    timeout - the cache timeout to use
+
+    """
+    groups = cache.get(key)
+    if groups is None:
+        groups = set([g.id for g in qs])
+        cache.set(key, groups, timeout)
+
+    return groups

--- a/gpp/forums/views/favorites.py	Sat Jul 02 03:52:43 2011 +0000
+++ b/gpp/forums/views/favorites.py	Sat Jul 02 23:35:45 2011 +0000
@@ -13,6 +13,7 @@
 
 from core.paginator import DiggPaginator
 from forums.models import Topic
+import forums.permissions as perms
 
 
 @login_required
@@ -23,11 +24,11 @@
     user.
     """
     topic = get_object_or_404(Topic.objects.select_related(), id=topic_id)
-    if topic.forum.category.can_access(request.user):
+    if perms.can_access(topic.forum.category, request.user):
         topic.bookmarkers.add(request.user)
         return HttpResponseRedirect(
             reverse("forums-favorites_status", args=[topic.id]))
-    raise Http404   # TODO return HttpResponseForbidden instead
+    return HttpResponseForbidden()
 
 
 @login_required

--- a/gpp/forums/views/main.py	Sat Jul 02 03:52:43 2011 +0000
+++ b/gpp/forums/views/main.py	Sat Jul 02 23:35:45 2011 +0000
@@ -21,19 +21,21 @@
 from django.utils.text import wrap
 from django.db.models import F
 
+import antispam
+import antispam.utils
+from bio.models import UserProfile, BadgeOwnership
 from core.paginator import DiggPaginator
 from core.functions import email_admins
-from forums.models import Forum, Topic, Post, FlaggedPost, TopicLastVisit, \
-        ForumLastVisit, Attachment
-from forums.forms import NewTopicForm, NewPostForm, PostForm, MoveTopicForm, \
-        SplitTopicForm
-from forums.unread import get_forum_unread_status, get_topic_unread_status, \
-        get_post_unread_status, get_unread_topics
 
-from bio.models import UserProfile, BadgeOwnership
-import antispam
-import antispam.utils
+from forums.models import (Forum, Topic, Post, FlaggedPost, TopicLastVisit,
+        ForumLastVisit, Attachment)
+from forums.forms import (NewTopicForm, NewPostForm, PostForm, MoveTopicForm,
+        SplitTopicForm)
+from forums.unread import (get_forum_unread_status, get_topic_unread_status,
+        get_post_unread_status, get_unread_topics)
+
 from forums.attachments import AttachmentProcessor
+import forums.permissions as perms
 
 #######################################################################
 
@@ -117,7 +119,7 @@
     """
     forum = get_object_or_404(Forum.objects.select_related(), slug=slug)
 
-    if not forum.category.can_access(request.user):
+    if not perms.can_access(forum.category, request.user):
         return HttpResponseForbidden()
 
     feed = None
@@ -141,7 +143,7 @@
     # we do this for the template since it is rendered twice
     page_nav = render_to_string('forums/pagination.html', {'page': page})
 
-    can_moderate = _can_moderate(forum, request.user)
+    can_moderate = perms.can_moderate(forum, request.user)
 
     return render_to_response('forums/forum_index.html', {
         'forum': forum,
@@ -160,7 +162,7 @@
     topic = get_object_or_404(Topic.objects.select_related(
         'forum', 'forum__category', 'last_post'), pk=id)
 
-    if not topic.forum.category.can_access(request.user):
+    if not perms.can_access(topic.forum.category, request.user):
         return HttpResponseForbidden()
 
     topic.view_count = F('view_count') + 1
@@ -222,7 +224,7 @@
     # we do this for the template since it is rendered twice
     page_nav = render_to_string('forums/pagination.html', {'page': page})
 
-    can_moderate = _can_moderate(topic.forum, request.user)
+    can_moderate = perms.can_moderate(topic.forum, request.user)
 
     can_reply = request.user.is_authenticated() and (
         not topic.locked or can_moderate)
@@ -289,7 +291,7 @@
     """
     forum = get_object_or_404(Forum.objects.select_related(), slug=slug)
 
-    if not forum.category.can_access(request.user):
+    if not perms.can_access(forum.category, request.user):
         return HttpResponseForbidden()
 
     if request.method == 'POST':
@@ -338,7 +340,7 @@
 
     form = NewPostForm(request.POST)
     if form.is_valid():
-        if not _can_post_in_topic(form.topic, request.user):
+        if not perms.can_post(form.topic, request.user):
             return HttpResponseForbidden("You don't have permission to post in this topic.")
         if antispam.utils.spam_check(request, form.cleaned_data['body']):
             return HttpResponseForbidden(antispam.BUSTED_MESSAGE)
@@ -352,7 +354,7 @@
 
         return render_to_response('forums/display_post.html', {
             'post': post,
-            'can_moderate': _can_moderate(form.topic.forum, request.user),
+            'can_moderate': perms.can_moderate(form.topic.forum, request.user),
             'can_reply': True,
             },
             context_instance=RequestContext(request))
@@ -419,7 +421,7 @@
     """
     post = get_object_or_404(Post.objects.select_related(), pk=id)
 
-    can_moderate = _can_moderate(post.topic.forum, request.user)
+    can_moderate = perms.can_moderate(post.topic.forum, request.user)
     can_edit = can_moderate or request.user == post.user
 
     if not can_edit:
@@ -578,7 +580,7 @@
     to a topic.
     """
     topic = get_object_or_404(Topic.objects.select_related(), pk=topic_id)
-    can_post = _can_post_in_topic(topic, request.user)
+    can_post = perms.can_post(topic, request.user)
 
     if can_post:
         if request.method == 'POST':
@@ -625,7 +627,7 @@
     This view function is for moderators to toggle the sticky status of a topic.
     """
     topic = get_object_or_404(Topic.objects.select_related(), pk=id)
-    if _can_moderate(topic.forum, request.user):
+    if perms.can_moderate(topic.forum, request.user):
         topic.sticky = not topic.sticky
         topic.save()
         return HttpResponseRedirect(topic.get_absolute_url())
@@ -639,7 +641,7 @@
     This view function is for moderators to toggle the locked status of a topic.
     """
     topic = get_object_or_404(Topic.objects.select_related(), pk=id)
-    if _can_moderate(topic.forum, request.user):
+    if perms.can_moderate(topic.forum, request.user):
         topic.locked = not topic.locked
         topic.save()
         return HttpResponseRedirect(topic.get_absolute_url())
@@ -653,7 +655,7 @@
     This view function is for moderators to delete an entire topic.
     """
     topic = get_object_or_404(Topic.objects.select_related(), pk=id)
-    if _can_moderate(topic.forum, request.user):
+    if perms.can_moderate(topic.forum, request.user):
         forum_url = topic.forum.get_absolute_url()
         _delete_topic(topic)
         return HttpResponseRedirect(forum_url)
@@ -667,7 +669,7 @@
     This view function is for moderators to move a topic to a different forum.
     """
     topic = get_object_or_404(Topic.objects.select_related(), pk=id)
-    if not _can_moderate(topic.forum, request.user):
+    if not perms.can_moderate(topic.forum, request.user):
         return HttpResponseForbidden()
 
     if request.method == 'POST':
@@ -696,7 +698,7 @@
     stickying and unstickying, moving, and deleting topics.
     """
     forum = get_object_or_404(Forum.objects.select_related(), slug=slug)
-    if not _can_moderate(forum, request.user):
+    if not perms.can_moderate(forum, request.user):
         return HttpResponseForbidden()
 
     topics = forum.topics.select_related('user', 'last_post', 'last_post__user')
@@ -770,7 +772,7 @@
     """
     forum = get_object_or_404(Forum.objects.select_related(), slug=slug)
 
-    if not forum.category.can_access(request.user):
+    if not perms.can_access(forum.category, request.user):
         return HttpResponseForbidden()
 
     forum.catchup(request.user)
@@ -783,7 +785,7 @@
     This view function allows moderators to split posts off to a new topic.
     """
     topic = get_object_or_404(Topic.objects.select_related(), pk=id)
-    if not _can_moderate(topic.forum, request.user):
+    if not perms.can_moderate(topic.forum, request.user):
         return HttpResponseRedirect(topic.get_absolute_url())
 
     if request.method == "POST":
@@ -930,7 +932,7 @@
     """Displays information about the IP address the post was made from."""
     post = get_object_or_404(Post.objects.select_related(), pk=post_id)
 
-    if not _can_moderate(post.topic.forum, request.user):
+    if not perms.can_moderate(post.topic.forum, request.user):
         return HttpResponseForbidden("You don't have permission for this post.")
 
     ip_users = sorted(set(Post.objects.filter(
@@ -971,23 +973,6 @@
         context_instance=RequestContext(request))
 
 
-def _can_moderate(forum, user):
-    """
-    Determines if a user has permission to moderate a given forum.
-    """
-    return user.is_authenticated() and (
-            user.is_superuser or user in forum.moderators.all())
-
-
-def _can_post_in_topic(topic, user):
-    """
-    This function returns true if the given user can post in the given topic
-    and false otherwise.
-    """
-    return (not topic.locked and topic.forum.category.can_access(user)) or \
-            (user.is_superuser or user in topic.forum.moderators.all())
-
-
 def _bump_post_count(user):
     """
     Increments the forum_post_count for the given user.

--- a/gpp/forums/views/subscriptions.py	Sat Jul 02 03:52:43 2011 +0000
+++ b/gpp/forums/views/subscriptions.py	Sat Jul 02 23:35:45 2011 +0000
@@ -13,6 +13,7 @@
 from django.views.decorators.http import require_POST
 
 from forums.models import Topic
+import forums.permissions as perms
 from core.functions import send_mail
 from core.paginator import DiggPaginator
 
@@ -50,7 +51,7 @@
 def subscribe_topic(request, topic_id):
     """Subscribe the user to the requested topic."""
     topic = get_object_or_404(Topic.objects.select_related(), id=topic_id)
-    if topic.forum.category.can_access(request.user):
+    if perms.can_access(topic.forum.category, request.user):
         topic.subscribers.add(request.user)
         return HttpResponseRedirect(
             reverse("forums-subscription_status", args=[topic.id]))