From 8b12cc606a6e6480c4bc0e1c3ceb4ca47af390d9 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Sat, 7 Oct 2023 13:52:37 +0530 Subject: [PATCH] request: Extract out methods from 'scheduled_messages' to reuse. This is a prep commit that extracts the following two methods from '/actions/scheduled_messages' to reuse in the next commit. * extract_stream_id * extract_direct_message_recipient_ids The 'to' parameter for 'POST /typing' will follow the same pattern in the next commit as we currently have for the 'to' parameter in 'POST /scheduled_messages', so we can reuse these functions. --- zerver/actions/scheduled_messages.py | 30 ++-------------- zerver/lib/recipient_parsing.py | 31 +++++++++++++++++ zerver/tests/test_recipient_parsing.py | 46 +++++++++++++++++++++++++ zerver/tests/test_scheduled_messages.py | 42 ---------------------- zerver/views/scheduled_messages.py | 6 ++-- 5 files changed, 83 insertions(+), 72 deletions(-) create mode 100644 zerver/lib/recipient_parsing.py create mode 100644 zerver/tests/test_recipient_parsing.py diff --git a/zerver/actions/scheduled_messages.py b/zerver/actions/scheduled_messages.py index 8a6c073b4d..18945a0571 100644 --- a/zerver/actions/scheduled_messages.py +++ b/zerver/actions/scheduled_messages.py @@ -2,7 +2,6 @@ import datetime import logging from typing import List, Optional, Sequence, Tuple -import orjson from django.conf import settings from django.db import transaction from django.utils.timezone import now as timezone_now @@ -18,6 +17,7 @@ from zerver.actions.uploads import check_attachment_reference_change, do_claim_a from zerver.lib.addressee import Addressee from zerver.lib.exceptions import JsonableError, RealmDeactivatedError, UserDeactivatedError from zerver.lib.message import SendMessageRequest, render_markdown, truncate_topic +from zerver.lib.recipient_parsing import extract_direct_message_recipient_ids, extract_stream_id from zerver.lib.scheduled_messages import access_scheduled_message from zerver.lib.string_validation import check_stream_topic from zerver.models import ( @@ -34,31 +34,6 @@ from zerver.tornado.django_api import send_event SCHEDULED_MESSAGE_LATE_CUTOFF_MINUTES = 10 -def extract_stream_id(req_to: str) -> List[int]: - # Recipient should only be a single stream ID. - try: - stream_id = int(req_to) - except ValueError: - raise JsonableError(_("Invalid data type for stream ID")) - return [stream_id] - - -def extract_direct_message_recipient_ids(req_to: str) -> List[int]: - try: - user_ids = orjson.loads(req_to) - except orjson.JSONDecodeError: - user_ids = req_to - - if not isinstance(user_ids, list): - raise JsonableError(_("Invalid data type for recipients")) - - for user_id in user_ids: - if not isinstance(user_id, int): - raise JsonableError(_("Recipient list may only contain user IDs")) - - return list(set(user_ids)) - - def check_schedule_message( sender: UserProfile, client: Client, @@ -182,7 +157,8 @@ def edit_scheduled_message( # Update message recipient if changed. if message_to is not None: if updated_recipient_type_name == "stream": - updated_recipient = extract_stream_id(message_to) + stream_id = extract_stream_id(message_to) + updated_recipient = [stream_id] else: updated_recipient = extract_direct_message_recipient_ids(message_to) else: diff --git a/zerver/lib/recipient_parsing.py b/zerver/lib/recipient_parsing.py new file mode 100644 index 0000000000..78fd6b17f4 --- /dev/null +++ b/zerver/lib/recipient_parsing.py @@ -0,0 +1,31 @@ +from typing import List + +import orjson +from django.utils.translation import gettext as _ + +from zerver.lib.exceptions import JsonableError + + +def extract_stream_id(req_to: str) -> int: + # Recipient should only be a single stream ID. + try: + stream_id = int(req_to) + except ValueError: + raise JsonableError(_("Invalid data type for stream ID")) + return stream_id + + +def extract_direct_message_recipient_ids(req_to: str) -> List[int]: + try: + user_ids = orjson.loads(req_to) + except orjson.JSONDecodeError: + user_ids = req_to + + if not isinstance(user_ids, list): + raise JsonableError(_("Invalid data type for recipients")) + + for user_id in user_ids: + if not isinstance(user_id, int): + raise JsonableError(_("Recipient list may only contain user IDs")) + + return list(set(user_ids)) diff --git a/zerver/tests/test_recipient_parsing.py b/zerver/tests/test_recipient_parsing.py new file mode 100644 index 0000000000..a485e334bf --- /dev/null +++ b/zerver/tests/test_recipient_parsing.py @@ -0,0 +1,46 @@ +import orjson + +from zerver.lib.exceptions import JsonableError +from zerver.lib.recipient_parsing import extract_direct_message_recipient_ids, extract_stream_id +from zerver.lib.test_classes import ZulipTestCase + + +class TestRecipientParsing(ZulipTestCase): + def test_extract_stream_id(self) -> None: + # stream message recipient = single stream ID. + stream_id = extract_stream_id("1") + self.assertEqual(stream_id, 1) + + with self.assertRaisesRegex(JsonableError, "Invalid data type for stream ID"): + extract_stream_id("1,2") + + with self.assertRaisesRegex(JsonableError, "Invalid data type for stream ID"): + extract_stream_id("[1]") + + with self.assertRaisesRegex(JsonableError, "Invalid data type for stream ID"): + extract_stream_id("general") + + def test_extract_recipient_ids(self) -> None: + # direct message recipients = user IDs. + user_ids = "[3,2,1]" + result = sorted(extract_direct_message_recipient_ids(user_ids)) + self.assertEqual(result, [1, 2, 3]) + + # JSON list w/duplicates + user_ids = orjson.dumps([3, 3, 12]).decode() + result = sorted(extract_direct_message_recipient_ids(user_ids)) + self.assertEqual(result, [3, 12]) + + # Invalid data + user_ids = "1, 12" + with self.assertRaisesRegex(JsonableError, "Invalid data type for recipients"): + extract_direct_message_recipient_ids(user_ids) + + user_ids = orjson.dumps(dict(recipient=12)).decode() + with self.assertRaisesRegex(JsonableError, "Invalid data type for recipients"): + extract_direct_message_recipient_ids(user_ids) + + # Heterogeneous lists are not supported + user_ids = orjson.dumps([3, 4, "eeshan@example.com"]).decode() + with self.assertRaisesRegex(JsonableError, "Recipient list may only contain user IDs"): + extract_direct_message_recipient_ids(user_ids) diff --git a/zerver/tests/test_scheduled_messages.py b/zerver/tests/test_scheduled_messages.py index 3dbd2f4c4b..a8cb6008b8 100644 --- a/zerver/tests/test_scheduled_messages.py +++ b/zerver/tests/test_scheduled_messages.py @@ -11,12 +11,9 @@ from django.utils.timezone import now as timezone_now from zerver.actions.scheduled_messages import ( SCHEDULED_MESSAGE_LATE_CUTOFF_MINUTES, - extract_direct_message_recipient_ids, - extract_stream_id, try_deliver_one_scheduled_message, ) from zerver.actions.users import change_user_is_active -from zerver.lib.exceptions import JsonableError from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import most_recent_message from zerver.lib.timestamp import timestamp_to_datetime @@ -717,42 +714,3 @@ class ScheduledMessageTest(ZulipTestCase): [scheduled_message.id], ) self.assertEqual(scheduled_message.has_attachment, True) - - def test_extract_stream_id(self) -> None: - # Scheduled stream message recipient = single stream ID. - stream_id = extract_stream_id("1") - self.assertEqual(stream_id, [1]) - - with self.assertRaisesRegex(JsonableError, "Invalid data type for stream ID"): - extract_stream_id("1,2") - - with self.assertRaisesRegex(JsonableError, "Invalid data type for stream ID"): - extract_stream_id("[1]") - - with self.assertRaisesRegex(JsonableError, "Invalid data type for stream ID"): - extract_stream_id("general") - - def test_extract_recipient_ids(self) -> None: - # Scheduled direct message recipients = user IDs. - user_ids = "[3,2,1]" - result = sorted(extract_direct_message_recipient_ids(user_ids)) - self.assertEqual(result, [1, 2, 3]) - - # JSON list w/duplicates - user_ids = orjson.dumps([3, 3, 12]).decode() - result = sorted(extract_direct_message_recipient_ids(user_ids)) - self.assertEqual(result, [3, 12]) - - # Invalid data - user_ids = "1, 12" - with self.assertRaisesRegex(JsonableError, "Invalid data type for recipients"): - extract_direct_message_recipient_ids(user_ids) - - user_ids = orjson.dumps(dict(recipient=12)).decode() - with self.assertRaisesRegex(JsonableError, "Invalid data type for recipients"): - extract_direct_message_recipient_ids(user_ids) - - # Heterogeneous lists are not supported - user_ids = orjson.dumps([3, 4, "eeshan@example.com"]).decode() - with self.assertRaisesRegex(JsonableError, "Recipient list may only contain user IDs"): - extract_direct_message_recipient_ids(user_ids) diff --git a/zerver/views/scheduled_messages.py b/zerver/views/scheduled_messages.py index cc37ed2219..b192799baa 100644 --- a/zerver/views/scheduled_messages.py +++ b/zerver/views/scheduled_messages.py @@ -8,10 +8,9 @@ from zerver.actions.scheduled_messages import ( check_schedule_message, delete_scheduled_message, edit_scheduled_message, - extract_direct_message_recipient_ids, - extract_stream_id, ) from zerver.lib.exceptions import JsonableError +from zerver.lib.recipient_parsing import extract_direct_message_recipient_ids, extract_stream_id from zerver.lib.request import REQ, RequestNotes, has_request_variables from zerver.lib.response import json_success from zerver.lib.scheduled_messages import get_undelivered_scheduled_messages @@ -133,7 +132,8 @@ def create_scheduled_message_backend( assert client is not None if recipient_type_name == "stream": - message_to = extract_stream_id(req_to) + stream_id = extract_stream_id(req_to) + message_to = [stream_id] else: message_to = extract_direct_message_recipient_ids(req_to)