Support client_gravatar field for event registration.

This commit allows clients to register client_gravatar=True, and
then we recognize that flag for message events.  If the flag is
True, we will not calculate gravatar URLs and let the clients do
it themselves.  (Clients can calculate gravatar URLs based on
emails with just a little bit of code.)
This commit is contained in:
Steve Howell 2017-10-31 10:36:18 -07:00 committed by Tim Abbott
parent f105c480b3
commit 2bbfda041a
8 changed files with 176 additions and 62 deletions

View File

@ -520,10 +520,10 @@ def apply_event(state, event, user_profile, include_subscribers):
else:
raise AssertionError("Unexpected event type %s" % (event['type'],))
def do_events_register(user_profile, user_client, apply_markdown=True,
def do_events_register(user_profile, user_client, apply_markdown=True, client_gravatar=False,
event_types=None, queue_lifespan_secs=0, all_public_streams=False,
include_subscribers=True, narrow=[], fetch_event_types=None):
# type: (UserProfile, Client, bool, Optional[Iterable[str]], int, bool, bool, Iterable[Sequence[Text]], Optional[Iterable[str]]) -> Dict[str, Any]
# type: (UserProfile, Client, bool, bool, Optional[Iterable[str]], int, bool, bool, Iterable[Sequence[Text]], Optional[Iterable[str]]) -> Dict[str, Any]
# Technically we don't need to check this here because
# build_narrow_filter will check it, but it's nicer from an error
@ -532,7 +532,7 @@ def do_events_register(user_profile, user_client, apply_markdown=True,
# Note that we pass event_types, not fetch_event_types here, since
# that's what controls which future events are sent.
queue_id = request_event_queue(user_profile, user_client, apply_markdown,
queue_id = request_event_queue(user_profile, user_client, apply_markdown, client_gravatar,
queue_lifespan_secs, event_types, all_public_streams,
narrow=narrow)

View File

@ -129,6 +129,7 @@ class MissedMessageNotificationsTest(ZulipTestCase):
result = self.tornado_call(get_events_backend, user_profile,
{"apply_markdown": ujson.dumps(True),
"client_gravatar": ujson.dumps(True),
"event_types": ujson.dumps(["message"]),
"user_client": "website",
"dont_block": ujson.dumps(True),

View File

@ -260,6 +260,7 @@ class GetEventsTest(ZulipTestCase):
result = self.tornado_call(get_events_backend, user_profile,
{"apply_markdown": ujson.dumps(True),
"client_gravatar": ujson.dumps(True),
"event_types": ujson.dumps(["message"]),
"user_client": "website",
"dont_block": ujson.dumps(True),
@ -269,6 +270,7 @@ class GetEventsTest(ZulipTestCase):
recipient_result = self.tornado_call(get_events_backend, recipient_user_profile,
{"apply_markdown": ujson.dumps(True),
"client_gravatar": ujson.dumps(True),
"event_types": ujson.dumps(["message"]),
"user_client": "website",
"dont_block": ujson.dumps(True),
@ -364,13 +366,14 @@ class GetEventsTest(ZulipTestCase):
email = user_profile.email
self.login(email)
def get_message(apply_markdown):
# type: (bool) -> Dict[str, Any]
def get_message(apply_markdown, client_gravatar):
# type: (bool, bool) -> Dict[str, Any]
result = self.tornado_call(
get_events_backend,
user_profile,
dict(
apply_markdown=ujson.dumps(apply_markdown),
client_gravatar=ujson.dumps(client_gravatar),
event_types=ujson.dumps(["message"]),
narrow=ujson.dumps([["stream", "denmark"]]),
user_client="website",
@ -406,13 +409,25 @@ class GetEventsTest(ZulipTestCase):
self.assertEqual(events[0]["type"], "message")
return events[0]['message']
message = get_message(apply_markdown=False)
message = get_message(apply_markdown=False, client_gravatar=False)
self.assertEqual(message["display_recipient"], "Denmark")
self.assertEqual(message["content"], "**hello**")
self.assertIn('gravatar.com', message["avatar_url"])
message = get_message(apply_markdown=True)
message = get_message(apply_markdown=True, client_gravatar=False)
self.assertEqual(message["display_recipient"], "Denmark")
self.assertEqual(message["content"], "<p><strong>hello</strong></p>")
self.assertIn('gravatar.com', message["avatar_url"])
message = get_message(apply_markdown=False, client_gravatar=True)
self.assertEqual(message["display_recipient"], "Denmark")
self.assertEqual(message["content"], "**hello**")
self.assertEqual(message["avatar_url"], None)
message = get_message(apply_markdown=True, client_gravatar=True)
self.assertEqual(message["display_recipient"], "Denmark")
self.assertEqual(message["content"], "<p><strong>hello</strong></p>")
self.assertEqual(message["avatar_url"], None)
class EventsRegisterTest(ZulipTestCase):
@ -439,9 +454,10 @@ class EventsRegisterTest(ZulipTestCase):
])),
])
def do_test(self, action, event_types=None, include_subscribers=True, state_change_expected=True,
num_events=1):
# type: (Callable[[], Any], Optional[List[str]], bool, bool, int) -> List[Dict[str, Any]]
def do_test(self, action, event_types=None,
include_subscribers=True, state_change_expected=True,
client_gravatar=False, num_events=1):
# type: (Callable[[], Any], Optional[List[str]], bool, bool, bool, int) -> List[Dict[str, Any]]
'''
Make sure we have a clean slate of client descriptors for these tests.
@ -457,6 +473,7 @@ class EventsRegisterTest(ZulipTestCase):
event_types = event_types,
client_type_name = "website",
apply_markdown = True,
client_gravatar = client_gravatar,
all_public_streams = False,
queue_timeout = 600,
last_connection_time = time.time(),
@ -586,34 +603,52 @@ class EventsRegisterTest(ZulipTestCase):
def test_stream_send_message_events(self):
# type: () -> None
schema_checker = self.check_events_dict([
('type', equals('message')),
('flags', check_list(None)),
('message', self.check_events_dict([
('avatar_url', check_string),
('client', check_string),
('content', check_string),
('content_type', equals('text/html')),
('display_recipient', check_string),
('is_me_message', check_bool),
('reactions', check_list(None)),
('recipient_id', check_int),
('sender_realm_str', check_string),
('sender_email', check_string),
('sender_full_name', check_string),
('sender_id', check_int),
('sender_short_name', check_string),
('stream_id', check_int),
('subject', check_string),
('subject_links', check_list(None)),
('timestamp', check_int),
('type', check_string),
])),
])
def check_none(var_name, val):
# type: (str, Any) -> Optional[str]
assert(val is None)
return None
def get_checker(check_gravatar):
# type: (Validator) -> Validator
schema_checker = self.check_events_dict([
('type', equals('message')),
('flags', check_list(None)),
('message', self.check_events_dict([
('avatar_url', check_gravatar),
('client', check_string),
('content', check_string),
('content_type', equals('text/html')),
('display_recipient', check_string),
('is_me_message', check_bool),
('reactions', check_list(None)),
('recipient_id', check_int),
('sender_realm_str', check_string),
('sender_email', check_string),
('sender_full_name', check_string),
('sender_id', check_int),
('sender_short_name', check_string),
('stream_id', check_int),
('subject', check_string),
('subject_links', check_list(None)),
('timestamp', check_int),
('type', check_string),
])),
])
return schema_checker
events = self.do_test(
lambda: self.send_stream_message(self.example_email("hamlet"), "Verona", "hello"),
client_gravatar=False,
)
schema_checker = get_checker(check_gravatar=check_string)
error = schema_checker('events[0]', events[0])
self.assert_on_error(error)
events = self.do_test(
lambda: self.send_stream_message(self.example_email("hamlet"), "Verona", "hello"),
client_gravatar=True,
)
schema_checker = get_checker(check_gravatar=check_none)
error = schema_checker('events[0]', events[0])
self.assert_on_error(error)
@ -2011,6 +2046,7 @@ class ClientDescriptorsTest(ZulipTestCase):
queue_data = dict(
all_public_streams=True,
apply_markdown=True,
client_gravatar=True,
client_type_name='website',
event_types=['message'],
last_connection_time=time.time(),
@ -2035,6 +2071,7 @@ class ClientDescriptorsTest(ZulipTestCase):
dct = client_info[client.event_queue.id]
self.assertEqual(dct['client'].apply_markdown, True)
self.assertEqual(dct['client'].client_gravatar, True)
self.assertEqual(dct['client'].user_profile_id, hamlet.id)
self.assertEqual(dct['flags'], None)
self.assertEqual(dct['is_sender'], False)
@ -2058,13 +2095,14 @@ class ClientDescriptorsTest(ZulipTestCase):
cordelia = self.example_user('cordelia')
realm = hamlet.realm
def test_get_info(apply_markdown):
# type: (bool) -> None
def test_get_info(apply_markdown, client_gravatar):
# type: (bool, bool) -> None
clear_client_event_queues_for_testing()
queue_data = dict(
all_public_streams=False,
apply_markdown=apply_markdown,
client_gravatar=client_gravatar,
client_type_name='website',
event_types=['message'],
last_connection_time=time.time(),
@ -2074,7 +2112,6 @@ class ClientDescriptorsTest(ZulipTestCase):
)
client = allocate_client_descriptor(queue_data)
message_event = dict(
realm_id=realm.id,
stream_name='whatever',
@ -2100,22 +2137,27 @@ class ClientDescriptorsTest(ZulipTestCase):
dct = client_info[client.event_queue.id]
self.assertEqual(dct['client'].apply_markdown, apply_markdown)
self.assertEqual(dct['client'].client_gravatar, client_gravatar)
self.assertEqual(dct['client'].user_profile_id, hamlet.id)
self.assertEqual(dct['flags'], ['mentioned'])
self.assertEqual(dct['is_sender'], False)
test_get_info(apply_markdown=False)
test_get_info(apply_markdown=True)
test_get_info(apply_markdown=False, client_gravatar=False)
test_get_info(apply_markdown=True, client_gravatar=False)
test_get_info(apply_markdown=False, client_gravatar=True)
test_get_info(apply_markdown=True, client_gravatar=True)
def test_process_message_event_with_mocked_client_info(self):
# type: () -> None
hamlet = self.example_user("hamlet")
class MockClient:
def __init__(self, user_profile_id, apply_markdown):
# type: (int, bool) -> None
def __init__(self, user_profile_id, apply_markdown, client_gravatar):
# type: (int, bool, bool) -> None
self.user_profile_id = user_profile_id
self.apply_markdown = apply_markdown
self.client_gravatar = client_gravatar
self.client_type_name = 'whatever'
self.events = [] # type: List[Dict[str, Any]]
@ -2135,11 +2177,25 @@ class ClientDescriptorsTest(ZulipTestCase):
client1 = MockClient(
user_profile_id=hamlet.id,
apply_markdown=True,
client_gravatar=False,
)
client2 = MockClient(
user_profile_id=hamlet.id,
apply_markdown=False,
client_gravatar=False,
)
client3 = MockClient(
user_profile_id=hamlet.id,
apply_markdown=True,
client_gravatar=True,
)
client4 = MockClient(
user_profile_id=hamlet.id,
apply_markdown=False,
client_gravatar=True,
)
client_info = {
@ -2151,6 +2207,14 @@ class ClientDescriptorsTest(ZulipTestCase):
client=client2,
flags=['has_alert_word'],
),
'client:3': dict(
client=client3,
flags=[],
),
'client:4': dict(
client=client4,
flags=[],
),
}
sender = hamlet
@ -2169,8 +2233,8 @@ class ClientDescriptorsTest(ZulipTestCase):
# that they can compute their own gravatar URLs.
sender_email=sender.email,
sender_realm_id=sender.realm_id,
sender_avatar_source=None,
sender_avatar_version=sender.avatar_version,
sender_avatar_source=UserProfile.AVATAR_FROM_GRAVATAR,
sender_avatar_version=1,
sender_is_mirror_dummy=None,
raw_display_recipient=None,
recipient_type=None,
@ -2227,6 +2291,40 @@ class ClientDescriptorsTest(ZulipTestCase):
),
])
self.assertEqual(client3.events, [
dict(
type='message',
message=dict(
type='stream',
sender_id=sender.id,
sender_email=sender.email,
avatar_url=None,
id=999,
content='<b>hello</b>',
content_type='text/html',
client='website',
),
flags=[],
),
])
self.assertEqual(client4.events, [
dict(
type='message',
message=dict(
type='stream',
sender_id=sender.id,
sender_email=sender.email,
avatar_url=None,
id=999,
content='**hello**',
content_type='text/x-markdown',
client='website',
),
flags=[],
),
])
class FetchQueriesTest(ZulipTestCase):
def test_queries(self):
# type: () -> None

View File

@ -238,6 +238,7 @@ class TornadoTestCase(WebSocketBaseTestCase):
'client_type_name': 'website',
'new_queue_data': {
'apply_markdown': True,
'client_gravatar': False,
'narrow': [],
'user_profile_email': user_profile.email,
'all_public_streams': False,

View File

@ -63,9 +63,9 @@ HEARTBEAT_MIN_FREQ_SECS = 45
class ClientDescriptor:
def __init__(self, user_profile_id, user_profile_email, realm_id, event_queue,
event_types, client_type_name, apply_markdown=True,
event_types, client_type_name, apply_markdown=True, client_gravatar=True,
all_public_streams=False, lifespan_secs=0, narrow=[]):
# type: (int, Text, int, EventQueue, Optional[Sequence[str]], Text, bool, bool, int, Iterable[Sequence[Text]]) -> None
# type: (int, Text, int, EventQueue, Optional[Sequence[str]], Text, bool, bool, bool, int, Iterable[Sequence[Text]]) -> None
# These objects are serialized on shutdown and restored on restart.
# If fields are added or semantics are changed, temporary code must be
# added to load_event_queues() to update the restored objects.
@ -80,6 +80,7 @@ class ClientDescriptor:
self.event_types = event_types
self.last_connection_time = time.time()
self.apply_markdown = apply_markdown
self.client_gravatar = client_gravatar
self.all_public_streams = all_public_streams
self.client_type_name = client_type_name
self._timeout_handle = None # type: Any # TODO: should be return type of ioloop.add_timeout
@ -102,6 +103,7 @@ class ClientDescriptor:
event_types=self.event_types,
last_connection_time=self.last_connection_time,
apply_markdown=self.apply_markdown,
client_gravatar=self.client_gravatar,
all_public_streams=self.all_public_streams,
narrow=self.narrow,
client_type_name=self.client_type_name)
@ -120,10 +122,23 @@ class ClientDescriptor:
if 'client_type' in d:
# Temporary migration for the rename of client_type to client_type_name
d['client_type_name'] = d['client_type']
ret = cls(d['user_profile_id'], d['user_profile_email'], d['realm_id'],
EventQueue.from_dict(d['event_queue']), d['event_types'],
d['client_type_name'], d['apply_markdown'], d['all_public_streams'],
d['queue_timeout'], d.get('narrow', []))
if 'client_gravatar' not in d:
# Temporary migration for the addition of the client_gravatar field
d['client_gravatar'] = False
ret = cls(
d['user_profile_id'],
d['user_profile_email'],
d['realm_id'],
EventQueue.from_dict(d['event_queue']),
d['event_types'],
d['client_type_name'],
d['apply_markdown'],
d['client_gravatar'],
d['all_public_streams'],
d['queue_timeout'],
d.get('narrow', [])
)
ret.last_connection_time = d['last_connection_time']
return ret
@ -568,13 +583,14 @@ def extract_json_response(resp):
else:
return resp.json # type: ignore # mypy trusts the stub, not the runtime type checking of this fn
def request_event_queue(user_profile, user_client, apply_markdown,
def request_event_queue(user_profile, user_client, apply_markdown, client_gravatar,
queue_lifespan_secs, event_types=None, all_public_streams=False,
narrow=[]):
# type: (UserProfile, Client, bool, int, Optional[Iterable[str]], bool, Iterable[Sequence[Text]]) -> Optional[str]
# type: (UserProfile, Client, bool, bool, int, Optional[Iterable[str]], bool, Iterable[Sequence[Text]]) -> Optional[str]
if settings.TORNADO_SERVER:
req = {'dont_block': 'true',
'apply_markdown': ujson.dumps(apply_markdown),
'client_gravatar': ujson.dumps(client_gravatar),
'all_public_streams': ujson.dumps(all_public_streams),
'client': 'internal',
'user_client': user_client.name,
@ -835,13 +851,7 @@ def process_message_event(event_template, users):
# message data unnecessarily
continue
'''
In this codepath we do net yet optimize for clients
that can compute their own gravatar URLs.
'''
client_gravatar = False
message_dict = get_client_payload(client.apply_markdown, client_gravatar)
message_dict = get_client_payload(client.apply_markdown, client.client_gravatar)
# Make sure Zephyr mirroring bots know whether stream is invite-only
if "mirror" in client.client_type_name and event_template.get("invite_only"):

View File

@ -43,12 +43,13 @@ def get_events_backend(request, user_profile, handler,
last_event_id = REQ(converter=int, default=None),
queue_id = REQ(default=None),
apply_markdown = REQ(default=False, validator=check_bool),
client_gravatar = REQ(default=False, validator=check_bool),
all_public_streams = REQ(default=False, validator=check_bool),
event_types = REQ(default=None, validator=check_list(check_string)),
dont_block = REQ(default=False, validator=check_bool),
narrow = REQ(default=[], validator=check_list(None)),
lifespan_secs = REQ(default=0, converter=int)):
# type: (HttpRequest, UserProfile, BaseHandler, Optional[Client], Optional[int], Optional[List[Text]], bool, bool, Optional[Text], bool, Iterable[Sequence[Text]], int) -> Union[HttpResponse, _RespondAsynchronously]
# type: (HttpRequest, UserProfile, BaseHandler, Optional[Client], Optional[int], Optional[List[Text]], bool, bool, bool, Optional[Text], bool, Iterable[Sequence[Text]], int) -> Union[HttpResponse, _RespondAsynchronously]
if user_client is None:
user_client = request.client
@ -73,6 +74,7 @@ def get_events_backend(request, user_profile, handler,
event_types = event_types,
client_type_name = user_client.name,
apply_markdown = apply_markdown,
client_gravatar = client_gravatar,
all_public_streams = all_public_streams,
queue_timeout = lifespan_secs,
last_connection_time = time.time(),

View File

@ -25,17 +25,18 @@ def _default_narrow(user_profile: UserProfile,
@has_request_variables
def events_register_backend(request, user_profile,
apply_markdown=REQ(default=False, validator=check_bool),
client_gravatar=REQ(default=False, validator=check_bool),
all_public_streams=REQ(default=None, validator=check_bool),
include_subscribers=REQ(default=False, validator=check_bool),
event_types=REQ(validator=check_list(check_string), default=None),
fetch_event_types=REQ(validator=check_list(check_string), default=None),
narrow=REQ(validator=check_list(check_list(check_string, length=2)), default=[]),
queue_lifespan_secs=REQ(converter=int, default=0)):
# type: (HttpRequest, UserProfile, bool, Optional[bool], bool, Optional[Iterable[str]], Optional[Iterable[str]], Iterable[Sequence[Text]], int) -> HttpResponse
# type: (HttpRequest, UserProfile, bool, bool, Optional[bool], bool, Optional[Iterable[str]], Optional[Iterable[str]], Iterable[Sequence[Text]], int) -> HttpResponse
all_public_streams = _default_all_public_streams(user_profile, all_public_streams)
narrow = _default_narrow(user_profile, narrow)
ret = do_events_register(user_profile, request.client, apply_markdown,
ret = do_events_register(user_profile, request.client, apply_markdown, client_gravatar,
event_types, queue_lifespan_secs, all_public_streams,
narrow=narrow, include_subscribers=include_subscribers,
fetch_event_types=fetch_event_types)

View File

@ -114,7 +114,8 @@ def home_real(request):
narrow.append(["topic", narrow_topic])
register_ret = do_events_register(user_profile, request.client,
apply_markdown=True, narrow=narrow)
apply_markdown=True, client_gravatar=False,
narrow=narrow)
user_has_messages = (register_ret['max_message_id'] != -1)
# Reset our don't-spam-users-with-email counter since the