extract_recipients: Support user IDs.

This is a part of our efforts surrounding #9474.

zerver/lib/actions.py

@@ -1876,12 +1876,14 @@ def already_sent_mirrored_message_id(message: Message) -> Optional[int]:
         return messages[0].id
     return None
 
-def extract_recipients(s: Union[str, Iterable[str]]) -> List[str]:
+def extract_recipients(
+        s: Union[str, Iterable[str], Iterable[int]]
+) -> Union[List[str], List[int]]:
     # We try to accept multiple incoming formats for recipients.
     # See test_extract_recipients() for examples of what we allow.
     try:
         data = ujson.loads(s)  # type: ignore # This function has a super weird union argument.
-    except ValueError:
+    except (ValueError, TypeError):
         data = s
 
     if isinstance(data, str):
@@ -1890,12 +1892,40 @@ def extract_recipients(s: Union[str, Iterable[str]]) -> List[str]:
     if not isinstance(data, list):
         raise ValueError("Invalid data type for recipients")
 
-    recipients = data
+    if not data:
+        # We don't complain about empty message recipients here
+        return data
 
-    # Strip recipients, and then remove any duplicates and any that
+    if isinstance(data[0], str):
-    # are the empty string after being stripped.
+        recipients = extract_emails(data)  # type: Union[List[str], List[int]]
-    recipients = [recipient.strip() for recipient in recipients]
+
-    return list(set(recipient for recipient in recipients if recipient))
+    if isinstance(data[0], int):
+        recipients = extract_user_ids(data)
+
+    # Remove any duplicates.
+    return list(set(recipients))  # type: ignore # mypy gets confused about what's passed to set()
+
+def extract_user_ids(user_ids: Iterable[int]) -> List[int]:
+    recipients = []
+    for user_id in user_ids:
+        if not isinstance(user_id, int):
+            raise TypeError("Recipient lists may contain emails or user IDs, but not both.")
+
+        recipients.append(user_id)
+
+    return recipients
+
+def extract_emails(emails: Iterable[str]) -> List[str]:
+    recipients = []
+    for email in emails:
+        if not isinstance(email, str):
+            raise TypeError("Recipient lists may contain emails or user IDs, but not both.")
+
+        email = email.strip()
+        if email:
+            recipients.append(email)
+
+    return recipients
 
 def check_send_stream_message(sender: UserProfile, client: Client, stream_name: str,
                               topic: str, body: str) -> int:

zerver/lib/request.pyi

@@ -7,8 +7,7 @@
 # types.
 
 from typing import Any, List, Callable, TypeVar, Optional, Union, Type
-from zerver.lib.types import ViewFuncT, Validator
+from zerver.lib.types import ViewFuncT, Validator, ExtractRecipients
-
 from zerver.lib.exceptions import JsonableError as JsonableError
 
 ResultT = TypeVar('ResultT')
@@ -23,7 +22,7 @@ NotSpecified = _NotSpecified()
 def REQ(whence: Optional[str] = None,
         *,
         type: Type[ResultT] = Type[None],
-        converter: Optional[Callable[[str], ResultT]] = None,
+        converter: Union[Optional[Callable[[str], ResultT]], ExtractRecipients] = None,
         default: Union[_NotSpecified, ResultT, None] = Optional[NotSpecified],
         validator: Optional[Validator] = None,
         str_validator: Optional[Validator] = None,

zerver/lib/types.py

@@ -1,4 +1,4 @@
-from typing import TypeVar, Callable, Optional, List, Dict, Union, Tuple, Any
+from typing import TypeVar, Callable, Optional, List, Dict, Union, Tuple, Any, Iterable
 from django.http import HttpResponse
 
 ViewFuncT = TypeVar('ViewFuncT', bound=Callable[..., HttpResponse])
@@ -6,6 +6,12 @@ ViewFuncT = TypeVar('ViewFuncT', bound=Callable[..., HttpResponse])
 # See zerver/lib/validator.py for more details of Validators,
 # including many examples
 Validator = Callable[[str, object], Optional[str]]
+
+# This type is specific for zerver.lib.actions.extract_recipients. After
+# deciding to support IDs for some of our API, extract_recipients'
+# implementation diverged from other converter functions in many ways.
+# See zerver/lib/request.pyi to see how this is used.
+ExtractRecipients = Callable[[Union[str, Iterable[str], Iterable[int]]], Union[List[str], List[int]]]
 ExtendedValidator = Callable[[str, str, object], Optional[str]]
 RealmUserValidator = Callable[[int, List[int], bool], Optional[str]]
 

zerver/tests/test_messages.py

@@ -528,11 +528,14 @@ class InternalPrepTest(ZulipTestCase):
         Stream.objects.get(name=stream_name, realm_id=realm.id)
 
 class ExtractedRecipientsTest(TestCase):
-    def test_extract_recipients(self) -> None:
+    def test_extract_recipients_emails(self) -> None:
 
         # JSON list w/dups, empties, and trailing whitespace
         s = ujson.dumps([' alice@zulip.com ', ' bob@zulip.com ', '   ', 'bob@zulip.com'])
-        self.assertEqual(sorted(extract_recipients(s)), ['alice@zulip.com', 'bob@zulip.com'])
+        # sorted() gets confused by extract_recipients' return type
+        # For testing, ignorance here is better than manual casting
+        result = sorted(extract_recipients(s))  # type: ignore
+        self.assertEqual(result, ['alice@zulip.com', 'bob@zulip.com'])
 
         # simple string with one name
         s = 'alice@zulip.com    '
@@ -544,17 +547,43 @@ class ExtractedRecipientsTest(TestCase):
 
         # bare comma-delimited string
         s = 'bob@zulip.com, alice@zulip.com'
-        self.assertEqual(sorted(extract_recipients(s)), ['alice@zulip.com', 'bob@zulip.com'])
+        result = sorted(extract_recipients(s))  # type: ignore
+        self.assertEqual(result, ['alice@zulip.com', 'bob@zulip.com'])
 
         # JSON-encoded, comma-delimited string
         s = '"bob@zulip.com,alice@zulip.com"'
-        self.assertEqual(sorted(extract_recipients(s)), ['alice@zulip.com', 'bob@zulip.com'])
+        result = sorted(extract_recipients(s))  # type: ignore
+        self.assertEqual(result, ['alice@zulip.com', 'bob@zulip.com'])
 
         # Invalid data
         s = ujson.dumps(dict(color='red'))
         with self.assertRaisesRegex(ValueError, 'Invalid data type for recipients'):
             extract_recipients(s)
 
+        # Empty list
+        self.assertEqual(extract_recipients([]), [])
+
+        # Heterogeneous lists are not supported
+        mixed = ujson.dumps(['eeshan@example.com', 3, 4])
+        with self.assertRaisesRegex(TypeError, 'Recipient lists may contain emails or user IDs, but not both.'):
+            extract_recipients(mixed)
+
+    def test_extract_recipient_ids(self) -> None:
+        # JSON list w/dups
+        s = ujson.dumps([3, 3, 12])
+        result = sorted(extract_recipients(s))  # type: ignore
+        self.assertEqual(result, [3, 12])
+
+        # Invalid data
+        ids = ujson.dumps(dict(recipient=12))
+        with self.assertRaisesRegex(ValueError, 'Invalid data type for recipients'):
+            extract_recipients(ids)
+
+        # Heterogeneous lists are not supported
+        mixed = ujson.dumps([3, 4, 'eeshan@example.com'])
+        with self.assertRaisesRegex(TypeError, 'Recipient lists may contain emails or user IDs, but not both.'):
+            extract_recipients(mixed)
+
 class PersonalMessagesTest(ZulipTestCase):
 
     def test_near_pm_message_url(self) -> None: