message validation: Clean up extract_private_recipients.

This is mostly refactoring, but we also prevent a new
type of value error (list of non-int-or-string).  The
new test code helps enforce that.

Cleanup includes:

    - Use early-exit for email case.
    - Rename helpers to get_validate_*.
    - Avoid clumsy rebuilding of lists in helpers.
    - Avoid the confusing `recipient` name (which
      can be confused with the model by the same
      name).
    - Just delegate duplicate-id/email-removal to
      the helpers.

The cleaner structure allows us to elminate a couple
mypy workarounds.

zerver/lib/actions.py

@@ -2079,35 +2079,26 @@ def extract_private_recipients(s: str) -> Union[List[str], List[int]]:
         return data
 
     if isinstance(data[0], str):
-        recipients = extract_emails(data)  # type: Union[List[str], List[int]]
+        return get_validated_emails(data)
 
-    if isinstance(data[0], int):
+    if not isinstance(data[0], int):
-        recipients = extract_user_ids(data)
+        raise ValueError("Invalid data type for recipients")
 
-    # Remove any duplicates.
+    return get_validated_user_ids(data)
-    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]:
+def get_validated_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 list(set(user_ids))
 
-    return recipients
+def get_validated_emails(emails: Iterable[str]) -> List[str]:
-
-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()
+    return list(filter(bool, {email.strip() for email in emails}))
-        if email:
-            recipients.append(email)
-
-    return recipients
 
 def check_send_stream_message(sender: UserProfile, client: Client, stream_name: str,
                               topic: str, body: str, realm: Optional[Realm]=None) -> int:

zerver/tests/test_messages.py

@@ -694,6 +694,10 @@ class ExtractTest(TestCase):
         with self.assertRaisesRegex(ValueError, 'Invalid data type for recipients'):
             extract_private_recipients(s)
 
+        s = ujson.dumps([{}])
+        with self.assertRaisesRegex(ValueError, 'Invalid data type for recipients'):
+            extract_private_recipients(s)
+
         # Empty list
         self.assertEqual(extract_private_recipients('[]'), [])