Mercurial > moin > 1.9
changeset 6022:fa5f81a0dfa9
getSubscribers: cleanups, optimizations, locking fix, .valid fix
removed superfluous/wrong cache.lock('r') call, the cache file is already
closed and unlocked. And as it is not used again in this method, there is
no need to lock it again. This is the same state as for the other code path,
if the cache existed.
fix: check the user.valid state, so behaviour of cached and uncached function
is the same.
docstrings/comments updated/fixed.
typos, formatting, style fixed.
use same name for the timer as for the function
break out of search loop as soon as we know that user IS subscribed
author | Thomas Waldmann <tw AT waldmann-edv DOT de> |
---|---|
date | Tue, 11 Feb 2014 15:55:54 +0100 |
parents | 54dc774ff49b |
children | 7e3e406f21d9 |
files | MoinMoin/Page.py MoinMoin/user.py |
diffstat | 2 files changed, 20 insertions(+), 19 deletions(-) [+] |
line wrap: on
line diff
--- a/MoinMoin/Page.py Tue Feb 11 14:08:06 2014 +0100 +++ b/MoinMoin/Page.py Tue Feb 11 15:55:54 2014 +0100 @@ -811,8 +811,9 @@ return link - def getSubscribersUncached(self, request, **kw): # original function renamed + def getSubscribersUncached(self, request, **kw): """ Get all subscribers of this page. + (original method, slow for many users, needs to open all user profiles) @param request: the request object @keyword include_self: if 1, include current user (default: 0) @@ -878,9 +879,9 @@ return subscriber_list - # cached version of getSubscribers def getSubscribers(self, request, **kw): """ Get all subscribers of this page. + (new method, uses caching) @param request: the request object @keyword include_self: if 1, include current user (default: 0) @@ -891,7 +892,7 @@ include_self = kw.get('include_self', self.include_self) return_users = kw.get('return_users', 0) - request.clock.start('getSubscribersCached') + request.clock.start('getSubscribers') # extract categories of this page pageList = self.getCategories(request) @@ -902,16 +903,16 @@ key = 'page_sub' # get or create cache file - page_sub = {} cache = caching.CacheEntry(request, arena, key, scope='wiki', use_pickle=True) if cache.exists(): page_sub = cache.content() else: - #build a cache if it doesn't exist + # build a cache if it doesn't exist cache = caching.CacheEntry(request, arena, key, scope='wiki', use_pickle=True, do_locking=False) # lock to stop anybody else interfering with the data while we're working cache.lock('w') userlist = user.getUserList(request) + page_sub = {} for uid in userlist: subscriber = user.User(request, uid) # we don't care about storing entries for users without any page subscriptions @@ -919,18 +920,17 @@ page_sub[subscriber.id] = { 'name': subscriber.name, 'email': subscriber.email, - 'subscribed_pages': subscriber.subscribed_pages + 'subscribed_pages': subscriber.subscribed_pages, } cache.update(page_sub) cache.unlock() - cache.lock('r') # to go back to the same mode as if it had existed all along if self.cfg.SecurityPolicy: UserPerms = self.cfg.SecurityPolicy else: from MoinMoin.security import Default as UserPerms - # get email addresses of the all wiki user which have a profile stored; + # get email addresses of all wiki users which have a profile stored; # add the address only if the user has subscribed to the page and # the user is not the current editor userlist = page_sub @@ -942,33 +942,37 @@ # Create text for regular expression search text = '\n'.join(pages) - for uid in userlist.keys(): + for uid in userlist: if uid == request.user.id and not include_self: continue # no self notification - isSubscribed = False - # 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. if not userlist[uid]['email']: continue # skip empty email addresses - # now check patters for actual match + # now check patterns for actual match + isSubscribed = False for pattern in userlist[uid]['subscribed_pages']: if pattern in pages: isSubscribed = True + break try: pattern = re.compile(r'^%s$' % pattern, re.M) except re.error: continue if pattern.search(text): isSubscribed = True + break - # only if subscribed, then read user info from file + # only if subscribed, create a User object from the profile if isSubscribed: subscriber = user.User(request, uid) + if not subscriber.valid: + continue + if not UserPerms(subscriber).read(self.page_name): continue @@ -980,7 +984,7 @@ else: subscriber_list[lang].append(subscriber.email) - request.clock.stop('getSubscribersCached') + request.clock.stop('getSubscribers') return subscriber_list def parse_processing_instructions(self):
--- a/MoinMoin/user.py Tue Feb 11 14:08:06 2014 +0100 +++ b/MoinMoin/user.py Tue Feb 11 15:55:54 2014 +0100 @@ -807,9 +807,6 @@ The subscription list may contain page names or interwiki page names. e.g 'Page Name' or 'WikiName:Page_Name' - TODO: check if it's fast enough when getting called for many - users from page.getSubscribersList() - @param pagelist: list of pages to check for subscription @rtype: bool @return: if user is subscribed any page in pagelist @@ -910,7 +907,7 @@ page_sub = {} cache = caching.CacheEntry(self._request, arena, key, scope='wiki', use_pickle=True, do_locking=False) if not cache.exists(): - return # if no cache file, just don't do anything + return # if no cache file exists, just don't do anything cache.lock('w') page_sub = cache.content() @@ -920,7 +917,7 @@ page_sub[self.id] = { 'name': self.name, 'email': self.email, - 'subscribed_pages': self.subscribed_pages + 'subscribed_pages': self.subscribed_pages, } elif page_sub.get(self.id): del page_sub[self.id]