-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Format presence events on the edges instead of reformatting them multiple times #2013
Changes from 1 commit
6c82de5
a297155
e892457
f83ac78
a8f96c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ | |
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
from synapse.api.errors import SynapseError | ||
from synapse.storage.presence import UserPresenceState | ||
from synapse.types import UserID, RoomID | ||
|
||
from twisted.internet import defer | ||
|
@@ -253,19 +254,30 @@ def check(self, event): | |
Returns: | ||
bool: True if the event matches | ||
""" | ||
sender = event.get("sender", None) | ||
if not sender: | ||
# Presence events have their 'sender' in content.user_id | ||
content = event.get("content") | ||
# account_data has been allowed to have non-dict content, so check type first | ||
if isinstance(content, dict): | ||
sender = content.get("user_id") | ||
if isinstance(event, UserPresenceState): | ||
sender = event.user_id | ||
room_id = None | ||
ev_type = "m.presence" | ||
is_url = False | ||
else: | ||
sender = event.get("sender", None) | ||
if not sender: | ||
# Presence events have their 'sender' in content.user_id | ||
content = event.get("content") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this still needed? It looks like it is presence specific and presence should be handled as UserPresenceState tuples/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does account data have a sender? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't remember tbh. I think only presence has a |
||
# account_data has been allowed to have non-dict content, so | ||
# check type first | ||
if isinstance(content, dict): | ||
sender = content.get("user_id") | ||
|
||
room_id = event.get("room_id", None) | ||
ev_type = event.get("type", None) | ||
is_url = "url" in event.get("content", {}) | ||
|
||
return self.check_fields( | ||
event.get("room_id", None), | ||
room_id, | ||
sender, | ||
event.get("type", None), | ||
"url" in event.get("content", {}) | ||
ev_type, | ||
is_url, | ||
) | ||
|
||
def check_fields(self, room_id, sender, event_type, contains_url): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -719,9 +719,7 @@ def get_states(self, target_user_ids, as_event=False): | |
for state in updates | ||
]) | ||
else: | ||
defer.returnValue([ | ||
format_user_presence_state(state, now) for state in updates | ||
]) | ||
defer.returnValue(updates) | ||
|
||
@defer.inlineCallbacks | ||
def set_state(self, target_user, state, ignore_status_msg=False): | ||
|
@@ -795,6 +793,9 @@ def get_presence_list(self, observer_user, accepted=None): | |
as_event=False, | ||
) | ||
|
||
now = self.clock.time_msec() | ||
results[:] = [format_user_presence_state(r, now) for r in results] | ||
|
||
is_accepted = { | ||
row["observed_user_id"]: row["accepted"] for row in presence_list | ||
} | ||
|
@@ -847,6 +848,7 @@ def invite_presence(self, observed_user, observer_user): | |
) | ||
|
||
state_dict = yield self.get_state(observed_user, as_event=False) | ||
state_dict = format_user_presence_state(state_dict, self.clock.time_msec()) | ||
|
||
self.federation.send_edu( | ||
destination=observer_user.domain, | ||
|
@@ -979,14 +981,15 @@ def should_notify(old_state, new_state): | |
return False | ||
|
||
|
||
def format_user_presence_state(state, now): | ||
def format_user_presence_state(state, now, include_user_id=True): | ||
"""Convert UserPresenceState to a format that can be sent down to clients | ||
and to other servers. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add a comment to explain why the "user_id" is optional? something like
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (done) |
||
""" | ||
content = { | ||
"presence": state.state, | ||
"user_id": state.user_id, | ||
} | ||
if include_user_id: | ||
content["user_id"] = state.user_id | ||
if state.last_active_ts: | ||
content["last_active_ago"] = now - state.last_active_ts | ||
if state.status_msg and state.state != PresenceState.OFFLINE: | ||
|
@@ -1073,16 +1076,13 @@ def get_new_events(self, user, from_key, room_ids=None, include_offline=True, | |
|
||
updates = yield presence.current_state_for_users(user_ids_changed) | ||
|
||
now = self.clock.time_msec() | ||
|
||
defer.returnValue(([ | ||
{ | ||
"type": "m.presence", | ||
"content": format_user_presence_state(s, now), | ||
} | ||
for s in updates.values() | ||
if include_offline or s.state != PresenceState.OFFLINE | ||
], max_token)) | ||
if include_offline: | ||
defer.returnValue((updates.values(), max_token)) | ||
else: | ||
defer.returnValue(([ | ||
s for s in updates.itervalues() | ||
if s.state != PresenceState.OFFLINE | ||
], max_token)) | ||
|
||
def get_current_key(self): | ||
return self.store.get_current_presence_token() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you comment what's going on here? Just a quick comment to explain why both branches are necessary would be enough.