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

Commit 3ce15cc

Browse files
Avoid unnecessary copies when filtering private read receipts. (#12711)
A minor optimization to avoid unnecessary copying/building identical dictionaries when filtering private read receipts. Also clarifies comments and cleans-up some tests.
1 parent b4eb163 commit 3ce15cc

File tree

4 files changed

+92
-73
lines changed

4 files changed

+92
-73
lines changed

changelog.d/12711.misc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Optimize private read receipt filtering.

synapse/handlers/initial_sync.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ async def _snapshot_all_rooms(
143143
to_key=int(now_token.receipt_key),
144144
)
145145
if self.hs.config.experimental.msc2285_enabled:
146-
receipt = ReceiptEventSource.filter_out_private(receipt, user_id)
146+
receipt = ReceiptEventSource.filter_out_private_receipts(receipt, user_id)
147147

148148
tags_by_room = await self.store.get_tags_for_user(user_id)
149149

@@ -449,7 +449,9 @@ async def get_receipts() -> List[JsonDict]:
449449
if not receipts:
450450
return []
451451
if self.hs.config.experimental.msc2285_enabled:
452-
receipts = ReceiptEventSource.filter_out_private(receipts, user_id)
452+
receipts = ReceiptEventSource.filter_out_private_receipts(
453+
receipts, user_id
454+
)
453455
return receipts
454456

455457
presence, receipts, (messages, token) = await make_deferred_yieldable(

synapse/handlers/receipts.py

Lines changed: 61 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -165,43 +165,69 @@ def __init__(self, hs: "HomeServer"):
165165
self.config = hs.config
166166

167167
@staticmethod
168-
def filter_out_private(events: List[JsonDict], user_id: str) -> List[JsonDict]:
168+
def filter_out_private_receipts(
169+
rooms: List[JsonDict], user_id: str
170+
) -> List[JsonDict]:
169171
"""
170-
This method takes in what is returned by
171-
get_linearized_receipts_for_rooms() and goes through read receipts
172-
filtering out m.read.private receipts if they were not sent by the
173-
current user.
174-
"""
175-
176-
visible_events = []
172+
Filters a list of serialized receipts (as returned by /sync and /initialSync)
173+
and removes private read receipts of other users.
177174
178-
# filter out private receipts the user shouldn't see
179-
for event in events:
180-
content = event.get("content", {})
181-
new_event = event.copy()
182-
new_event["content"] = {}
175+
This operates on the return value of get_linearized_receipts_for_rooms(),
176+
which is wrapped in a cache. Care must be taken to ensure that the input
177+
values are not modified.
183178
184-
for event_id, event_content in content.items():
185-
receipt_event = {}
186-
for receipt_type, receipt_content in event_content.items():
187-
if receipt_type == ReceiptTypes.READ_PRIVATE:
188-
user_rr = receipt_content.get(user_id, None)
189-
if user_rr:
190-
receipt_event[ReceiptTypes.READ_PRIVATE] = {
191-
user_id: user_rr.copy()
192-
}
193-
else:
194-
receipt_event[receipt_type] = receipt_content.copy()
195-
196-
# Only include the receipt event if it is non-empty.
197-
if receipt_event:
198-
new_event["content"][event_id] = receipt_event
179+
Args:
180+
rooms: A list of mappings, each mapping has a `content` field, which
181+
is a map of event ID -> receipt type -> user ID -> receipt information.
199182
200-
# Append new_event to visible_events unless empty
201-
if len(new_event["content"].keys()) > 0:
202-
visible_events.append(new_event)
183+
Returns:
184+
The same as rooms, but filtered.
185+
"""
203186

204-
return visible_events
187+
result = []
188+
189+
# Iterate through each room's receipt content.
190+
for room in rooms:
191+
# The receipt content with other user's private read receipts removed.
192+
content = {}
193+
194+
# Iterate over each event ID / receipts for that event.
195+
for event_id, orig_event_content in room.get("content", {}).items():
196+
event_content = orig_event_content
197+
# If there are private read receipts, additional logic is necessary.
198+
if ReceiptTypes.READ_PRIVATE in event_content:
199+
# Make a copy without private read receipts to avoid leaking
200+
# other user's private read receipts..
201+
event_content = {
202+
receipt_type: receipt_value
203+
for receipt_type, receipt_value in event_content.items()
204+
if receipt_type != ReceiptTypes.READ_PRIVATE
205+
}
206+
207+
# Copy the current user's private read receipt from the
208+
# original content, if it exists.
209+
user_private_read_receipt = orig_event_content[
210+
ReceiptTypes.READ_PRIVATE
211+
].get(user_id, None)
212+
if user_private_read_receipt:
213+
event_content[ReceiptTypes.READ_PRIVATE] = {
214+
user_id: user_private_read_receipt
215+
}
216+
217+
# Include the event if there is at least one non-private read
218+
# receipt or the current user has a private read receipt.
219+
if event_content:
220+
content[event_id] = event_content
221+
222+
# Include the event if there is at least one non-private read receipt
223+
# or the current user has a private read receipt.
224+
if content:
225+
# Build a new event to avoid mutating the cache.
226+
new_room = {k: v for k, v in room.items() if k != "content"}
227+
new_room["content"] = content
228+
result.append(new_room)
229+
230+
return result
205231

206232
async def get_new_events(
207233
self,
@@ -223,7 +249,9 @@ async def get_new_events(
223249
)
224250

225251
if self.config.experimental.msc2285_enabled:
226-
events = ReceiptEventSource.filter_out_private(events, user.to_string())
252+
events = ReceiptEventSource.filter_out_private_receipts(
253+
events, user.to_string()
254+
)
227255

228256
return events, to_key
229257

tests/handlers/test_receipts.py

Lines changed: 26 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
15+
from copy import deepcopy
1616
from typing import List
1717

1818
from synapse.api.constants import ReceiptTypes
@@ -125,42 +125,6 @@ def test_filters_out_event_with_only_private_receipts_and_ignores_the_rest(self)
125125
],
126126
)
127127

128-
def test_handles_missing_content_of_m_read(self):
129-
self._test_filters_private(
130-
[
131-
{
132-
"content": {
133-
"$14356419ggffg114394fHBLK:matrix.org": {ReceiptTypes.READ: {}},
134-
"$1435641916114394fHBLK:matrix.org": {
135-
ReceiptTypes.READ: {
136-
"@user:jki.re": {
137-
"ts": 1436451550453,
138-
}
139-
}
140-
},
141-
},
142-
"room_id": "!jEsUZKDJdhlrceRyVU:example.org",
143-
"type": "m.receipt",
144-
}
145-
],
146-
[
147-
{
148-
"content": {
149-
"$14356419ggffg114394fHBLK:matrix.org": {ReceiptTypes.READ: {}},
150-
"$1435641916114394fHBLK:matrix.org": {
151-
ReceiptTypes.READ: {
152-
"@user:jki.re": {
153-
"ts": 1436451550453,
154-
}
155-
}
156-
},
157-
},
158-
"room_id": "!jEsUZKDJdhlrceRyVU:example.org",
159-
"type": "m.receipt",
160-
}
161-
],
162-
)
163-
164128
def test_handles_empty_event(self):
165129
self._test_filters_private(
166130
[
@@ -332,9 +296,33 @@ def test_leaves_our_private_and_their_public(self):
332296
],
333297
)
334298

299+
def test_we_do_not_mutate(self):
300+
"""Ensure the input values are not modified."""
301+
events = [
302+
{
303+
"content": {
304+
"$1435641916114394fHBLK:matrix.org": {
305+
ReceiptTypes.READ_PRIVATE: {
306+
"@rikj:jki.re": {
307+
"ts": 1436451550453,
308+
}
309+
}
310+
}
311+
},
312+
"room_id": "!jEsUZKDJdhlrceRyVU:example.org",
313+
"type": "m.receipt",
314+
}
315+
]
316+
original_events = deepcopy(events)
317+
self._test_filters_private(events, [])
318+
# Since the events are fed in from a cache they should not be modified.
319+
self.assertEqual(events, original_events)
320+
335321
def _test_filters_private(
336322
self, events: List[JsonDict], expected_output: List[JsonDict]
337323
):
338324
"""Tests that the _filter_out_private returns the expected output"""
339-
filtered_events = self.event_source.filter_out_private(events, "@me:server.org")
325+
filtered_events = self.event_source.filter_out_private_receipts(
326+
events, "@me:server.org"
327+
)
340328
self.assertEqual(filtered_events, expected_output)

0 commit comments

Comments
 (0)