validation: Use JsonableError for extractors.

The distinction between ValueError and TypeError
is not useful in these functions:

    - extract_stream_indicator
    - extract_private_recipients (or its callees)

These are always invoked in views to validate
user input.

When we use REQ to wrap the validators, any
Exception gets turned into a JsonableError, so
the distinction is not important.

And if we don't use REQ to wrap the validators,
the errors aren't caught.

Now we just let these functions directly produce
the desired end result for both codepaths.

Also, we now flag the error strings for translation.
This commit is contained in:
Steve Howell 2020-04-01 21:10:57 +00:00 committed by Tim Abbott
parent 5a5fc6caa1
commit f6503a4061
2 changed files with 14 additions and 14 deletions

View File

@ -2044,7 +2044,7 @@ def extract_stream_indicator(s: str) -> Union[str, int]:
# once we improve our documentation.
if isinstance(data, list):
if len(data) != 1: # nocoverage
raise ValueError("Expected exactly one stream")
raise JsonableError(_("Expected exactly one stream"))
data = data[0]
if isinstance(data, str):
@ -2055,7 +2055,7 @@ def extract_stream_indicator(s: str) -> Union[str, int]:
# We had a stream id.
return data
raise ValueError("Invalid data type for stream")
raise JsonableError(_("Invalid data type for stream"))
def extract_private_recipients(s: str) -> Union[List[str], List[int]]:
# We try to accept multiple incoming formats for recipients.
@ -2070,7 +2070,7 @@ def extract_private_recipients(s: str) -> Union[List[str], List[int]]:
data = data.split(',')
if not isinstance(data, list):
raise ValueError("Invalid data type for recipients")
raise JsonableError(_("Invalid data type for recipients"))
if not data:
# We don't complain about empty message recipients here
@ -2080,21 +2080,21 @@ def extract_private_recipients(s: str) -> Union[List[str], List[int]]:
return get_validated_emails(data)
if not isinstance(data[0], int):
raise ValueError("Invalid data type for recipients")
raise JsonableError(_("Invalid data type for recipients"))
return get_validated_user_ids(data)
def get_validated_user_ids(user_ids: Iterable[int]) -> List[int]:
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.")
raise JsonableError(_("Recipient lists may contain emails or user IDs, but not both."))
return list(set(user_ids))
def get_validated_emails(emails: Iterable[str]) -> List[str]:
for email in emails:
if not isinstance(email, str):
raise TypeError("Recipient lists may contain emails or user IDs, but not both.")
raise JsonableError(_("Recipient lists may contain emails or user IDs, but not both."))
return list(filter(bool, {email.strip() for email in emails}))

View File

@ -669,13 +669,13 @@ class ExtractTest(TestCase):
123,
)
with self.assertRaisesRegex(ValueError, 'Invalid data type for stream'):
with self.assertRaisesRegex(JsonableError, 'Invalid data type for stream'):
extract_stream_indicator('{}')
with self.assertRaisesRegex(ValueError, 'Invalid data type for stream'):
with self.assertRaisesRegex(JsonableError, 'Invalid data type for stream'):
extract_stream_indicator('[{}]')
with self.assertRaisesRegex(ValueError, 'Expected exactly one stream'):
with self.assertRaisesRegex(JsonableError, 'Expected exactly one stream'):
extract_stream_indicator('[1,2,"general"]')
def test_extract_private_recipients_emails(self) -> None:
@ -707,11 +707,11 @@ class ExtractTest(TestCase):
# Invalid data
s = ujson.dumps(dict(color='red'))
with self.assertRaisesRegex(ValueError, 'Invalid data type for recipients'):
with self.assertRaisesRegex(JsonableError, 'Invalid data type for recipients'):
extract_private_recipients(s)
s = ujson.dumps([{}])
with self.assertRaisesRegex(ValueError, 'Invalid data type for recipients'):
with self.assertRaisesRegex(JsonableError, 'Invalid data type for recipients'):
extract_private_recipients(s)
# Empty list
@ -719,7 +719,7 @@ class ExtractTest(TestCase):
# 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.'):
with self.assertRaisesRegex(JsonableError, 'Recipient lists may contain emails or user IDs, but not both.'):
extract_private_recipients(mixed)
def test_extract_recipient_ids(self) -> None:
@ -730,12 +730,12 @@ class ExtractTest(TestCase):
# Invalid data
ids = ujson.dumps(dict(recipient=12))
with self.assertRaisesRegex(ValueError, 'Invalid data type for recipients'):
with self.assertRaisesRegex(JsonableError, 'Invalid data type for recipients'):
extract_private_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.'):
with self.assertRaisesRegex(JsonableError, 'Recipient lists may contain emails or user IDs, but not both.'):
extract_private_recipients(mixed)
class PersonalMessagesTest(ZulipTestCase):