subscriptions: Avoid sending unneeded subscriber information.

The `users/me/subscriptions` endpoint accidentally started returning
subscriber information for each stream.  This is convenient, but
unnecessarily costly for those clients which either don't need it
(most API apps) or already acquire this information via /register
(including Zulip's apps).

This change removes that data set from the default response.  Clients
which had come to rely on it, or would like to rely on it in future,
may still access it via an additional documented API parameter.

Fixes #12917.
This commit is contained in:
meiomorphism
2019-08-08 12:58:38 -07:00
committed by Tim Abbott
parent 671844d667
commit 1af72a2745
5 changed files with 61 additions and 25 deletions

View File

@@ -37,11 +37,18 @@ curl -X GET {{ api_url }}/v1/users/me/subscriptions \
-u BOT_EMAIL_ADDRESS:BOT_API_KEY -u BOT_EMAIL_ADDRESS:BOT_API_KEY
``` ```
You may pass the `include_subscribers` query parameter as follows:
``` curl
curl -X GET {{ api_url }}/v1/users/me/subscriptions?include_subscribers=true \
-u BOT_EMAIL_ADDRESS:BOT_API_KEY
```
{end_tabs} {end_tabs}
## Arguments ## Arguments
This request takes no arguments. {generate_api_arguments_table|zulip.yaml|/users/me/subscriptions:get}
## Response ## Response
@@ -55,7 +62,7 @@ This request takes no arguments.
* `invite-only`: Specifies whether a stream is private or not. * `invite-only`: Specifies whether a stream is private or not.
Only people who have been invited can access a private stream. Only people who have been invited can access a private stream.
* `subscribers`: A list of email addresses of users who are also subscribed * `subscribers`: A list of email addresses of users who are also subscribed
to a given stream. to a given stream. Included only if `include_subscribers` is `true`.
* `desktop_notifications`: A boolean specifiying whether desktop notifications * `desktop_notifications`: A boolean specifiying whether desktop notifications
are enabled for the given stream. are enabled for the given stream.
* `push_notifications`: A boolean specifiying whether push notifications * `push_notifications`: A boolean specifiying whether push notifications

View File

@@ -4736,20 +4736,28 @@ def gather_subscriptions_helper(user_profile: UserProfile,
sorted(unsubscribed, key=lambda x: x['name']), sorted(unsubscribed, key=lambda x: x['name']),
sorted(never_subscribed, key=lambda x: x['name'])) sorted(never_subscribed, key=lambda x: x['name']))
def gather_subscriptions(user_profile: UserProfile) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]]]: def gather_subscriptions(
subscribed, unsubscribed, never_subscribed = gather_subscriptions_helper(user_profile) user_profile: UserProfile,
user_ids = set() include_subscribers: bool=False,
for subs in [subscribed, unsubscribed, never_subscribed]: ) -> Tuple[List[Dict[str, Any]], List[Dict[str, Any]]]:
for sub in subs: subscribed, unsubscribed, _ = gather_subscriptions_helper(
if 'subscribers' in sub: user_profile, include_subscribers=include_subscribers)
for subscriber in sub['subscribers']:
user_ids.add(subscriber)
email_dict = get_emails_from_user_ids(list(user_ids))
for subs in [subscribed, unsubscribed]: if include_subscribers:
for sub in subs: user_ids = set()
if 'subscribers' in sub: for subs in [subscribed, unsubscribed]:
sub['subscribers'] = sorted([email_dict[user_id] for user_id in sub['subscribers']]) for sub in subs:
if 'subscribers' in sub:
for subscriber in sub['subscribers']:
user_ids.add(subscriber)
email_dict = get_emails_from_user_ids(list(user_ids))
for subs in [subscribed, unsubscribed]:
for sub in subs:
if 'subscribers' in sub:
sub['subscribers'] = sorted([
email_dict[user_id] for user_id in sub['subscribers']
])
return (subscribed, unsubscribed) return (subscribed, unsubscribed)

View File

@@ -1210,6 +1210,16 @@ paths:
/users/me/subscriptions: /users/me/subscriptions:
get: get:
description: Get information on every stream the user is subscribed to. description: Get information on every stream the user is subscribed to.
parameters:
- name: include_subscribers
in: query
description: Set to `true` if you would like each stream's info to include a
list of current subscribers to that stream. (This may be significantly slower
in organizations with thousands of users.)
schema:
type: boolean
default: false
example: true
security: security:
- basicAuth: [] - basicAuth: []
responses: responses:

View File

@@ -3515,9 +3515,10 @@ class GetSubscribersTest(ZulipTestCase):
self.assert_user_got_subscription_notification(msg) self.assert_user_got_subscription_notification(msg)
with queries_captured() as queries: with queries_captured() as queries:
subscriptions = gather_subscriptions(self.user_profile) subscribed_streams, _ = gather_subscriptions(
self.assertTrue(len(subscriptions[0]) >= 11) self.user_profile, include_subscribers=True)
for sub in subscriptions[0]: self.assertTrue(len(subscribed_streams) >= 11)
for sub in subscribed_streams:
if not sub["name"].startswith("stream_"): if not sub["name"].startswith("stream_"):
continue continue
self.assertTrue(len(sub["subscribers"]) == len(users_to_subscribe)) self.assertTrue(len(sub["subscribers"]) == len(users_to_subscribe))
@@ -3695,10 +3696,11 @@ class GetSubscribersTest(ZulipTestCase):
self.assert_json_success(ret) self.assert_json_success(ret)
with queries_captured() as queries: with queries_captured() as queries:
subscriptions = gather_subscriptions(mit_user_profile) subscribed_streams, _ = gather_subscriptions(
mit_user_profile, include_subscribers=True)
self.assertTrue(len(subscriptions[0]) >= 2) self.assertTrue(len(subscribed_streams) >= 2)
for sub in subscriptions[0]: for sub in subscribed_streams:
if not sub["name"].startswith("mit_"): if not sub["name"].startswith("mit_"):
raise AssertionError("Unexpected stream!") raise AssertionError("Unexpected stream!")
if sub["name"] == "mit_invite_only": if sub["name"] == "mit_invite_only":
@@ -3752,11 +3754,12 @@ class GetSubscribersTest(ZulipTestCase):
def test_json_get_subscribers(self) -> None: def test_json_get_subscribers(self) -> None:
""" """
json_get_subscribers in zerver/views/streams.py json_get_subscribers in zerver/views/streams.py
also returns the list of subscribers for a stream. also returns the list of subscribers for a stream, when requested.
""" """
stream_name = gather_subscriptions(self.user_profile)[0][0]['name'] stream_name = gather_subscriptions(self.user_profile)[0][0]['name']
stream_id = get_stream(stream_name, self.user_profile.realm).id stream_id = get_stream(stream_name, self.user_profile.realm).id
expected_subscribers = gather_subscriptions(self.user_profile)[0][0]['subscribers'] expected_subscribers = gather_subscriptions(
self.user_profile, include_subscribers=True)[0][0]['subscribers']
result = self.client_get("/json/streams/%d/members" % (stream_id,)) result = self.client_get("/json/streams/%d/members" % (stream_id,))
self.assert_json_success(result) self.assert_json_success(result)
result_dict = result.json() result_dict = result.json()

View File

@@ -180,8 +180,16 @@ def update_stream_backend(
do_change_stream_invite_only(stream, is_private, history_public_to_subscribers) do_change_stream_invite_only(stream, is_private, history_public_to_subscribers)
return json_success() return json_success()
def list_subscriptions_backend(request: HttpRequest, user_profile: UserProfile) -> HttpResponse: @has_request_variables
return json_success({"subscriptions": gather_subscriptions(user_profile)[0]}) def list_subscriptions_backend(
request: HttpRequest,
user_profile: UserProfile,
include_subscribers: bool=REQ(validator=check_bool, default=False),
) -> HttpResponse:
subscribed, _ = gather_subscriptions(
user_profile, include_subscribers=include_subscribers
)
return json_success({"subscriptions": subscribed})
FuncKwargPair = Tuple[Callable[..., HttpResponse], Dict[str, Union[int, Iterable[Any]]]] FuncKwargPair = Tuple[Callable[..., HttpResponse], Dict[str, Union[int, Iterable[Any]]]]