Add zerver/lib/stream_subscription.py.

The first method we extract to this library is
get_active_subscriptions_for_stream_id().

We also move num_subscribers_for_stream_id() to here, which
is slightly annoying (having the method on Stream was nice)
but avoids some circular dependency issues.

zerver/lib/actions.py

@@ -34,6 +34,10 @@ from zerver.lib.message import (
 from zerver.lib.realm_icon import realm_icon_url
 from zerver.lib.retention import move_message_to_archive
 from zerver.lib.send_email import send_email, FromAddress
+from zerver.lib.stream_subscription import (
+    get_active_subscriptions_for_stream_id,
+    num_subscribers_for_stream_id,
+)
 from zerver.lib.stream_topic import StreamTopicTarget
 from zerver.lib.topic_mutes import (
     get_topic_mutes,
@@ -159,11 +163,7 @@ def private_stream_user_ids(stream_id):
     # type: (int) -> Set[int]
 
     # TODO: Find similar queries elsewhere and de-duplicate this code.
-    subscriptions = Subscription.objects.filter(
-        recipient__type=Recipient.STREAM,
-        recipient__type_id=stream_id,
-        active=True)
-
+    subscriptions = get_active_subscriptions_for_stream_id(stream_id)
     return {sub['user_profile_id'] for sub in subscriptions.values('user_profile_id')}
 
 def bot_owner_user_ids(user_profile):
@@ -632,10 +632,7 @@ def do_deactivate_stream(stream, log=True):
     # Get the affected user ids *before* we deactivate everybody.
     affected_user_ids = can_access_stream_user_ids(stream)
 
-    Subscription.objects.select_related('user_profile').filter(
-        recipient__type=Recipient.STREAM,
-        recipient__type_id=stream.id,
-        active=True).update(active=False)
+    get_active_subscriptions_for_stream_id(stream.id).update(active=False)
 
     was_invite_only = stream.invite_only
     stream.deactivated = True
@@ -1648,7 +1645,7 @@ def send_pm_if_empty_stream(sender, stream, stream_name, realm):
         return
 
     if stream is not None:
-        num_subscribers = stream.num_subscribers()
+        num_subscribers = num_subscribers_for_stream_id(stream.id)
         if num_subscribers > 0:
             return
 
@@ -2012,10 +2009,9 @@ def get_subscribers_query(stream, requesting_user):
     # Note that non-active users may still have "active" subscriptions, because we
     # want to be able to easily reactivate them with their old subscriptions.  This
     # is why the query here has to look at the UserProfile.is_active flag.
-    subscriptions = Subscription.objects.filter(recipient__type=Recipient.STREAM,
-                                                recipient__type_id=stream.id,
-                                                user_profile__is_active=True,
-                                                active=True)
+    subscriptions = get_active_subscriptions_for_stream_id(stream.id).filter(
+        user_profile__is_active=True
+    )
     return subscriptions
 
 def get_subscribers(stream, requesting_user=None):

zerver/lib/stream_subscription.py

@@ -0,0 +1,19 @@
+from django.db.models.query import QuerySet
+from zerver.models import (
+    Recipient,
+    Subscription,
+)
+
+def get_active_subscriptions_for_stream_id(stream_id):
+    # type: (int) -> QuerySet
+    return Subscription.objects.filter(
+        recipient__type=Recipient.STREAM,
+        recipient__type_id=stream_id,
+        active=True,
+    )
+
+def num_subscribers_for_stream_id(stream_id):
+    # type: (int) -> int
+    return get_active_subscriptions_for_stream_id(stream_id).filter(
+        user_profile__is_active=True,
+    ).count()

zerver/models.py

@@ -888,20 +888,6 @@ class Stream(ModelReprMixin, models.Model):
     class Meta(object):
         unique_together = ("name", "realm")
 
-    @staticmethod
-    def num_subscribers_for_stream_id(stream_id):
-        # type: (int) -> int
-        return Subscription.objects.filter(
-            recipient__type=Recipient.STREAM,
-            recipient__type_id=stream_id,
-            user_profile__is_active=True,
-            active=True
-        ).count()
-
-    def num_subscribers(self):
-        # type: () -> int
-        return Stream.num_subscribers_for_stream_id(self.id)
-
     # This is stream information that is sent to clients
     def to_dict(self):
         # type: () -> Dict[str, Any]

zerver/tests/test_subs.py

@@ -29,6 +29,11 @@ from zerver.lib.streams import (
     access_stream_by_id, access_stream_by_name
 )
 
+from zerver.lib.stream_subscription import (
+    get_active_subscriptions_for_stream_id,
+    num_subscribers_for_stream_id,
+)
+
 from zerver.lib.test_runner import (
     slow
 )
@@ -201,11 +206,8 @@ class StreamAdminTest(ZulipTestCase):
 
         result = self.client_delete('/json/streams/%d' % (stream.id,))
         self.assert_json_success(result)
-        subscription_exists = Subscription.objects.filter(
+        subscription_exists = get_active_subscriptions_for_stream_id(stream.id).filter(
             user_profile=user_profile,
-            recipient__type_id=stream.id,
-            recipient__type=Recipient.STREAM,
-            active=True,
         ).exists()
         self.assertFalse(subscription_exists)
 
@@ -1646,7 +1648,7 @@ class SubscriptionAPITest(ZulipTestCase):
                 self.assertEqual(ev['event']['op'], 'peer_add')
 
         stream = get_stream('multi_user_stream', realm)
-        self.assertEqual(stream.num_subscribers(), 2)
+        self.assertEqual(num_subscribers_for_stream_id(stream.id), 2)
 
         # Now add ourselves
         events = []
@@ -1675,7 +1677,7 @@ class SubscriptionAPITest(ZulipTestCase):
         self.assertEqual(add_peer_event['event']['user_id'], self.user_profile.id)
 
         stream = get_stream('multi_user_stream', realm)
-        self.assertEqual(stream.num_subscribers(), 3)
+        self.assertEqual(num_subscribers_for_stream_id(stream.id), 3)
 
         # Finally, add othello.
         events = []
@@ -2104,9 +2106,7 @@ class SubscriptionAPITest(ZulipTestCase):
         # We can't see invite-only streams here
         self.assert_json_error(result, "Invalid stream name 'Saxony'", status_code=404)
         # Importantly, we are not now subscribed
-        self.assertEqual(Subscription.objects.filter(
-            recipient__type=Recipient.STREAM,
-            recipient__type_id=stream.id).count(), 1)
+        self.assertEqual(num_subscribers_for_stream_id(stream.id), 1)
 
         # A user who is subscribed still sees the stream exists
         self.login(self.example_email("cordelia"))