mypy: Use tuples for muted_topics.

We now have our muted topics use tuples internally,
which allows us to tighten up the annotation
for get_topic_mutes, as well as our schema
checking.

We want to deprecate sub_validator=None
for check_list, so we also introduce
check_tuple here.  Now we also want to deprecate
check_tuple, but it's at least isolated now.

We will use this for data structures that are tuples,
but which are sent as lists over the wire.  Fortunately,
we don't have too many of those.

The plan is to convert tuples to dictionaries,
but backward compatibility may be tricky in some
places.
This commit is contained in:
Steve Howell
2020-06-24 14:38:35 +00:00
committed by Tim Abbott
parent 8e8228535e
commit c7b82d3ece
5 changed files with 46 additions and 7 deletions

View File

@@ -1,5 +1,5 @@
import datetime
from typing import Any, Callable, Dict, List, Optional, Union
from typing import Any, Callable, Dict, List, Optional, Tuple
from django.utils.timezone import now as timezone_now
from sqlalchemy.sql import Selectable, and_, column, not_, or_
@@ -9,7 +9,7 @@ from zerver.lib.topic import topic_match_sa
from zerver.models import MutedTopic, UserProfile, get_stream
def get_topic_mutes(user_profile: UserProfile) -> List[List[Union[str, float]]]:
def get_topic_mutes(user_profile: UserProfile) -> List[Tuple[str, str, float]]:
rows = MutedTopic.objects.filter(
user_profile=user_profile,
).values(
@@ -18,7 +18,7 @@ def get_topic_mutes(user_profile: UserProfile) -> List[List[Union[str, float]]]:
'date_muted',
)
return [
[row['stream__name'], row['topic_name'], datetime_to_timestamp(row['date_muted'])]
(row['stream__name'], row['topic_name'], datetime_to_timestamp(row['date_muted']))
for row in rows
]

View File

@@ -163,6 +163,24 @@ def check_list(sub_validator: Optional[Validator[ResultT]]=None, length: Optiona
return cast(List[ResultT], val)
return f
def check_tuple(sub_validators: List[Validator[ResultT]]) -> Validator[Tuple[Any, ...]]:
def f(var_name: str, val: object) -> Tuple[Any, ...]:
if not isinstance(val, tuple):
raise ValidationError(_('{var_name} is not a tuple').format(var_name=var_name))
desired_len = len(sub_validators)
if desired_len != len(val):
raise ValidationError(_('{var_name} should have exactly {desired_len} items').format(
var_name=var_name, desired_len=desired_len,
))
for i, sub_validator in enumerate(sub_validators):
vname = f'{var_name}[{i}]'
sub_validator(vname, val[i])
return val
return f
# https://zulip.readthedocs.io/en/latest/testing/mypy.html#using-overload-to-accurately-describe-variations
@overload
def check_dict(required_keys: Iterable[Tuple[str, Validator[object]]]=[],

View File

@@ -68,6 +68,7 @@ from zerver.lib.validator import (
check_string_in,
check_string_or_int,
check_string_or_int_list,
check_tuple,
check_union,
check_url,
equals,
@@ -848,6 +849,21 @@ class ValidatorTestCase(TestCase):
with self.assertRaisesRegex(ValidationError, r'color is not a string'):
check_color('color', z)
def test_check_tuple(self) -> None:
x: Any = 999
with self.assertRaisesRegex(ValidationError, r'x is not a tuple'):
check_tuple([check_string])('x', x)
x = (5, 2)
with self.assertRaisesRegex(ValidationError, r'x\[0\] is not a string'):
check_tuple([check_string, check_string])('x', x)
x = (1, 2, 3)
with self.assertRaisesRegex(ValidationError, r'x should have exactly 2 items'):
check_tuple([check_int, check_int])('x', x)
check_tuple([check_string, check_int])('x', ('string', 42))
def test_check_list(self) -> None:
x: Any = 999
with self.assertRaisesRegex(ValidationError, r'x is not a list'):

View File

@@ -139,6 +139,7 @@ from zerver.lib.validator import (
check_list,
check_none_or,
check_string,
check_tuple,
check_url,
equals,
)
@@ -1493,7 +1494,11 @@ class EventsRegisterTest(ZulipTestCase):
def test_muted_topics_events(self) -> None:
muted_topics_checker = self.check_events_dict([
('type', equals('muted_topics')),
('muted_topics', check_list(check_list(sub_validator=None, length=3))),
('muted_topics', check_list(check_tuple([
check_string, # stream name
check_string, # topic name
check_int, # timestamp
]))),
])
stream = get_stream('Denmark', self.user_profile.realm)
recipient = stream.recipient

View File

@@ -73,7 +73,7 @@ class MutedTopicsTests(ZulipTestCase):
result = self.api_patch(user, url, data)
self.assert_json_success(result)
self.assertIn([stream.name, 'Verona3', mock_date_muted], get_topic_mutes(user))
self.assertIn((stream.name, 'Verona3', mock_date_muted), get_topic_mutes(user))
self.assertTrue(topic_is_muted(user, stream.id, 'Verona3'))
self.assertTrue(topic_is_muted(user, stream.id, 'verona3'))
@@ -106,12 +106,12 @@ class MutedTopicsTests(ZulipTestCase):
topic_name='Verona3',
date_muted=datetime(2020, 1, 1, tzinfo=timezone.utc),
)
self.assertIn([stream.name, 'Verona3', mock_date_muted], get_topic_mutes(user))
self.assertIn((stream.name, 'Verona3', mock_date_muted), get_topic_mutes(user))
result = self.api_patch(user, url, data)
self.assert_json_success(result)
self.assertNotIn([stream.name, 'Verona3', mock_date_muted], get_topic_mutes(user))
self.assertNotIn((stream.name, 'Verona3', mock_date_muted), get_topic_mutes(user))
self.assertFalse(topic_is_muted(user, stream.id, 'verona3'))
def test_muted_topic_add_invalid(self) -> None: