changeset 3300:f048c3edc164

reorder permission checks to last when getting subscribers (cf. MoinMoinBugs/GetSubscribersPerformanceProblem)
author Johannes Berg <johannes AT sipsolutions DOT net>
date Tue, 18 Mar 2008 16:51:57 +0100
parents 34d225168d14
children 885b7ada47b9
files MoinMoin/Page.py
diffstat 1 files changed, 18 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/MoinMoin/Page.py	Tue Mar 18 15:31:17 2008 +0100
+++ b/MoinMoin/Page.py	Tue Mar 18 16:51:57 2008 +0100
@@ -867,6 +867,11 @@
                 continue # no self notification
             subscriber = user.User(request, uid)
 
+            # The following tests should be ordered in order of
+            # decreasing computation complexity, in particular
+            # the permissions check may be expensive; see the bug
+            # MoinMoinBugs/GetSubscribersPerformanceProblem
+
             # This is a bit wrong if return_users=1 (which implies that the caller will process
             # user attributes and may, for example choose to send an SMS)
             # So it _should_ be "not (subscriber.email and return_users)" but that breaks at the moment.
@@ -875,17 +880,22 @@
             if trivial and not subscriber.want_trivial:
                 continue # skip uninterested subscribers
 
+            # skip people not subscribed
+            if not subscriber.isSubscribedTo(pageList):
+                continue
+
+            # skip people who can't read the page
             if not UserPerms(subscriber).read(self.page_name):
                 continue
 
-            if subscriber.isSubscribedTo(pageList):
-                lang = subscriber.language or request.cfg.language_default
-                if not lang in subscriber_list:
-                    subscriber_list[lang] = []
-                if return_users:
-                    subscriber_list[lang].append(subscriber)
-                else:
-                    subscriber_list[lang].append(subscriber.email)
+            # add the user to the list
+            lang = subscriber.language or request.cfg.language_default
+            if not lang in subscriber_list:
+                subscriber_list[lang] = []
+            if return_users:
+                subscriber_list[lang].append(subscriber)
+            else:
+                subscriber_list[lang].append(subscriber.email)
 
         return subscriber_list