break user_subreddits and default_subreddits apart and remove limit =…

… None query in organic.py

r2/r2/controllers/listingcontroller.py

@@ -220,8 +220,7 @@ def query(self):
             self.fix_listing = False
 
         if c.site == Default:
-            user = c.user if c.user_is_loggedin else None
-            sr_ids = Subreddit.user_subreddits(user,
+            sr_ids = Subreddit.user_subreddits(c.user,
                                                limit = g.num_default_reddits)
             return normalized_hot(sr_ids)
         #if not using the query_cache we still want cached front pages

r2/r2/lib/organic.py

@@ -96,7 +96,7 @@ def my_keepfn(l):
 @memoize('cached_organic_links', time = organic_lifetime)
 def cached_organic_links(user_id, langs):
     if user_id is None:
-        sr_ids = Subreddit.user_subreddits(None)
+        sr_ids = Subreddit.default_subreddits()
     else:
         user = Account._byID(user_id, data=True)
         sr_ids = Subreddit.user_subreddits(user)
@@ -137,7 +137,7 @@ def cached_organic_links(user_id, langs):
 def organic_links(user):
     from r2.controllers.reddit_base import organic_pos
 
-    sr_ids = Subreddit.user_subreddits(user, limit = None)
+    sr_ids = Subreddit.user_subreddits(user)
     # make sure that these are sorted so the cache keys are constant
     sr_ids.sort()
 

r2/r2/lib/pages/pages.py

@@ -726,12 +726,8 @@ class SubredditTopBar(Wrapped):
     def __init__(self):
         Wrapped.__init__(self)
 
-        my_reddits = []
-        sr_ids = Subreddit.user_subreddits(c.user if c.user_is_loggedin else None)
-        if sr_ids:
-            my_reddits = Subreddit._byID(sr_ids, True,
-                                         return_dict = False)
-            my_reddits.sort(key = lambda sr: sr.name.lower())
+        my_reddits = Subreddit.user_subreddits(c.user, ids = False)
+        my_reddits.sort(key = lambda sr: sr.name.lower())
 
         drop_down_buttons = []    
         for sr in my_reddits:
@@ -748,7 +744,8 @@ def __init__(self):
                                          title = _('my reddits'),
                                          type = 'srdrop')
 
-        pop_reddits = Subreddit.user_subreddits(None, ids = False)
+        pop_reddits = Subreddit.default_subreddits(ids = False,
+                                                   limit = Subreddit.sr_limit)
         buttons = [SubredditButton(sr) for sr in c.recent_reddits]
         for sr in pop_reddits:
             if sr not in c.recent_reddits:
@@ -761,14 +758,7 @@ class SubscriptionBox(Wrapped):
     """The list of reddits a user is currently subscribed to to go in
     the right pane."""
     def __init__(self):
-        user = c.user if c.user_is_loggedin else None
-        # user_subreddits does know to limit to just
-        # g.num_default_reddits if the user is not subscribed.
-        if not user or not user.has_subscribed:
-            limit = g.num_default_reddits
-        else:
-            limit = Subreddit.sr_limit
-        srs = Subreddit.user_subreddits(user, ids = False, limit = limit)
+        srs = Subreddit.user_subreddits(c.user, ids = False)
         srs.sort(key = lambda sr: sr.name.lower())
         b = IDBuilder([sr._fullname for sr in srs])
         self.reddits = LinkListing(b).listing().things

r2/r2/models/subreddit.py

@@ -243,8 +243,7 @@ def get_links(self, sort, time):
     def add_props(cls, user, wrapped):
         names = ('subscriber', 'moderator', 'contributor')
         rels = (SRMember._fast_query(wrapped, [user], names) if user else {})
-        defaults = Subreddit.user_subreddits(None,
-                                             limit = g.num_default_reddits)
+        defaults = Subreddit.default_subreddits()
         for item in wrapped:
             if not user or not user.has_subscribed:
                 item.subscriber = item._id in defaults
@@ -277,7 +276,7 @@ def cache_key(wrapped):
         #return 1
 
     @classmethod
-    def default_srs(cls, lang, limit):
+    def top_lang_srs(cls, lang, limit):
         """Returns the default list of subreddits for a given language, sorted
         by popularity"""
         pop_reddits = Subreddit._query(Subreddit.c.type == ('public',
@@ -296,46 +295,63 @@ def default_srs(cls, lang, limit):
 
         return list(pop_reddits)
 
+    @classmethod
+    def default_subreddits(cls, ids = True, limit = g.num_default_reddits):
+        """
+        Generates a list of the subreddits any user with the current
+        set of language preferences and no subscriptions would see.
+
+        An optional kw argument 'limit' is defaulted to g.num_default_reddits
+        """
+        langs = list(c.content_langs)
+        # g.lang will be in the the current set of content langs
+        # unless the user has explicity removed it.  Since most
+        # content is in g.lang, set the subreddit ratio to 50/50.
+        if limit and langs != 'all' and g.lang in langs and len(langs) != 1:
+            # lookup default lang subreddits
+            default_srs = cls.top_lang_srs([g.lang], limit)
+            # remove g.lang from conten_lang list and use it to
+            # grab content_lang subreddits
+            langs.remove(g.lang)
+            lang_srs = cls.top_lang_srs(langs, limit)
+            # interleave the two lists, putting the lang ones first
+            srs = list(interleave_lists(lang_srs, default_srs))
+            if limit:
+                srs = srs[:limit]
+        else:
+            # the user knows better and has set their langs accordingly
+            srs = cls.top_lang_srs(c.content_langs, limit)
+        return [s._id for s in srs] if ids else srs
+
     @classmethod
     def user_subreddits(cls, user, ids = True, limit = sr_limit):
         """
         subreddits that appear in a user's listings. If the user has
         subscribed, returns the stored set of subscriptions.
 
-        Otherwise, returns a set of 'limit' subreddits, making sure to
-        include a fair bit of content from the default language.
+        Otherwise, return the default set.
         """
+        # note: for user not logged in, the fake user account has
+        # has_subscribed == False by default.
         if user and user.has_subscribed:
             sr_ids = Subreddit.reverse_subscriber_ids(user)
             if limit and len(sr_ids) > limit:
                 sr_ids = random.sample(sr_ids, limit)
             return sr_ids if ids else Subreddit._byID(sr_ids, True, False)
         else:
-            langs = list(c.content_langs)
-            # g.lang will be in the the current set of content langs
-            # unless the user has explicity removed it.  Since most
-            # content is in g.lang, set the subreddit ratio to 50/50.
-            if limit and langs != 'all' and g.lang in langs and len(langs) != 1:
-                # lookup default lang subreddits
-                default_srs = cls.default_srs([g.lang], limit)
-                # remove g.lang from conten_lang list and use it to
-                # grab content_lang subreddits
-                langs.remove(g.lang)
-                lang_srs = cls.default_srs(langs, limit)
-                # interleave the two lists, putting the lang ones first
-                srs = list(interleave_lists(lang_srs, default_srs))
-                if limit:
-                    srs = srs[:limit]
-            else:
-                # the user knows better and has set their langs accordingly
-                srs = cls.default_srs(c.content_langs, limit)
-            return [s._id for s in srs] if ids else srs
-
+            # if there is a limit, we want *at most* limit subreddits.
+            # Allow the default_subreddit list to return the number it
+            # would normally and then slice.
+            srs = cls.default_subreddits(ids = ids)
+            if limit:
+                srs = srs[:limit]
+            return srs
+
     def is_subscriber_defaults(self, user):
         if user.has_subscribed:
             return self.is_subscriber(user)
         else:
-            return self in self.user_subreddits(None, ids = False)
+            return self in self.default_subreddits(ids = False)
 
     @classmethod
     def subscribe_defaults(cls, user):