Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Convert the typing handler to async/await. #7679

Merged
merged 3 commits into from
Jun 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7679.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Convert typing handler to async/await.
29 changes: 11 additions & 18 deletions synapse/handlers/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
from collections import namedtuple
from typing import List

from twisted.internet import defer

from synapse.api.errors import AuthError, SynapseError
from synapse.logging.context import run_in_background
from synapse.types import UserID, get_domain_from_id
Expand Down Expand Up @@ -115,8 +113,7 @@ def _handle_timeouts(self):
def is_typing(self, member):
return member.user_id in self._room_typing.get(member.room_id, [])

@defer.inlineCallbacks
def started_typing(self, target_user, auth_user, room_id, timeout):
async def started_typing(self, target_user, auth_user, room_id, timeout):
target_user_id = target_user.to_string()
auth_user_id = auth_user.to_string()

Expand All @@ -126,7 +123,7 @@ def started_typing(self, target_user, auth_user, room_id, timeout):
if target_user_id != auth_user_id:
raise AuthError(400, "Cannot set another user's typing state")

yield self.auth.check_user_in_room(room_id, target_user_id)
await self.auth.check_user_in_room(room_id, target_user_id)

logger.debug("%s has started typing in %s", target_user_id, room_id)

Expand All @@ -145,8 +142,7 @@ def started_typing(self, target_user, auth_user, room_id, timeout):

self._push_update(member=member, typing=True)

@defer.inlineCallbacks
def stopped_typing(self, target_user, auth_user, room_id):
async def stopped_typing(self, target_user, auth_user, room_id):
target_user_id = target_user.to_string()
auth_user_id = auth_user.to_string()

Expand All @@ -156,20 +152,19 @@ def stopped_typing(self, target_user, auth_user, room_id):
if target_user_id != auth_user_id:
raise AuthError(400, "Cannot set another user's typing state")

yield self.auth.check_user_in_room(room_id, target_user_id)
await self.auth.check_user_in_room(room_id, target_user_id)

logger.debug("%s has stopped typing in %s", target_user_id, room_id)

member = RoomMember(room_id=room_id, user_id=target_user_id)

self._stopped_typing(member)

@defer.inlineCallbacks
def user_left_room(self, user, room_id):
user_id = user.to_string()
if self.is_mine_id(user_id):
member = RoomMember(room_id=room_id, user_id=user_id)
yield self._stopped_typing(member)
self._stopped_typing(member)

def _stopped_typing(self, member):
if member.user_id not in self._room_typing.get(member.room_id, set()):
Expand All @@ -188,10 +183,9 @@ def _push_update(self, member, typing):

self._push_update_local(member=member, typing=typing)

@defer.inlineCallbacks
def _push_remote(self, member, typing):
async def _push_remote(self, member, typing):
try:
users = yield self.state.get_current_users_in_room(member.room_id)
users = await self.state.get_current_users_in_room(member.room_id)
self._member_last_federation_poke[member] = self.clock.time_msec()

now = self.clock.time_msec()
Expand All @@ -215,8 +209,7 @@ def _push_remote(self, member, typing):
except Exception:
logger.exception("Error pushing typing notif to remotes")

@defer.inlineCallbacks
def _recv_edu(self, origin, content):
async def _recv_edu(self, origin, content):
room_id = content["room_id"]
user_id = content["user_id"]

Expand All @@ -231,7 +224,7 @@ def _recv_edu(self, origin, content):
)
return

users = yield self.state.get_current_users_in_room(room_id)
users = await self.state.get_current_users_in_room(room_id)
domains = {get_domain_from_id(u) for u in users}

if self.server_name in domains:
Expand Down Expand Up @@ -306,7 +299,7 @@ def _make_event_for(self, room_id):
"content": {"user_ids": list(typing)},
}

def get_new_events(self, from_key, room_ids, **kwargs):
async def get_new_events(self, from_key, room_ids, **kwargs):
with Measure(self.clock, "typing.get_new_events"):
from_key = int(from_key)
handler = self.get_typing_handler()
Expand All @@ -320,7 +313,7 @@ def get_new_events(self, from_key, room_ids, **kwargs):

events.append(self._make_event_for(room_id))

return defer.succeed((events, handler._latest_room_serial))
return (events, handler._latest_room_serial)

def get_current_key(self):
return self.get_typing_handler()._latest_room_serial
13 changes: 7 additions & 6 deletions tests/handlers/test_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ def get_received_txn_response(*args):
def check_user_in_room(room_id, user_id):
if user_id not in [u.to_string() for u in self.room_members]:
raise AuthError(401, "User is not in the room")
return defer.succeed(None)

hs.get_auth().check_user_in_room = check_user_in_room

Expand All @@ -138,7 +139,7 @@ def get_joined_hosts_for_room(room_id):
self.datastore.get_joined_hosts_for_room = get_joined_hosts_for_room

def get_current_users_in_room(room_id):
return {str(u) for u in self.room_members}
return defer.succeed({str(u) for u in self.room_members})

hs.get_state_handler().get_current_users_in_room = get_current_users_in_room

Expand All @@ -163,7 +164,7 @@ def test_started_typing_local(self):

self.assertEquals(self.event_source.get_current_key(), 0)

self.successResultOf(
self.get_success(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these aren't quite the same thing. successResultOf asserts that the thing has already completed; get_success does not do that. I think tests.test_utils.get_awaitable_result should do what you want here.

That said, it probably doesn't matter here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_success seems to just handle if it is an awaitable, wrap it in a ensureDeferred and then call successResultOf:

synapse/tests/unittest.py

Lines 443 to 449 in 0361932

def get_success(self, d, by=0.0):
if inspect.isawaitable(d):
d = ensureDeferred(d)
if not isinstance(d, Deferred):
return d
self.pump(by=by)
return self.successResultOf(d)

So I think it does the same thing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is the self.pump.

self.handler.started_typing(
target_user=U_APPLE, auth_user=U_APPLE, room_id=ROOM_ID, timeout=20000
)
Expand All @@ -190,7 +191,7 @@ def test_started_typing_local(self):
def test_started_typing_remote_send(self):
self.room_members = [U_APPLE, U_ONION]

self.successResultOf(
self.get_success(
self.handler.started_typing(
target_user=U_APPLE, auth_user=U_APPLE, room_id=ROOM_ID, timeout=20000
)
Expand Down Expand Up @@ -265,7 +266,7 @@ def test_stopped_typing(self):

self.assertEquals(self.event_source.get_current_key(), 0)

self.successResultOf(
self.get_success(
self.handler.stopped_typing(
target_user=U_APPLE, auth_user=U_APPLE, room_id=ROOM_ID
)
Expand Down Expand Up @@ -305,7 +306,7 @@ def test_typing_timeout(self):

self.assertEquals(self.event_source.get_current_key(), 0)

self.successResultOf(
self.get_success(
self.handler.started_typing(
target_user=U_APPLE, auth_user=U_APPLE, room_id=ROOM_ID, timeout=10000
)
Expand Down Expand Up @@ -344,7 +345,7 @@ def test_typing_timeout(self):

# SYN-230 - see if we can still set after timeout

self.successResultOf(
self.get_success(
self.handler.started_typing(
target_user=U_APPLE, auth_user=U_APPLE, room_id=ROOM_ID, timeout=10000
)
Expand Down