-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add support for stable prefixes for MSC2285: private read receipts #13273
Changes from 34 commits
83b7af1
01fbb6d
cafe4be
967f8ed
a9f2329
08d874d
ba20652
d18a485
8323358
404459f
57251c8
e12706c
c8cf10e
84299f6
208e97a
a7c3a1c
252146d
3cbd80b
8854e35
baa9e84
4455232
de9005f
c0b3c90
3612ceb
a37a17d
14b36a3
746351c
50bffe4
1b66814
bcdbb5d
0f498e0
660c776
b1ddf1a
3094b39
011d6d2
ee6512f
2cd29bd
89d5bd4
f0c531d
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 |
---|---|---|
@@ -0,0 +1 @@ | ||
Add support for stable prefixes for [MSC2285 (private read receipts)](https://github.com/matrix-org/matrix-spec-proposals/pull/2285). |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,7 +80,7 @@ | |
|
||
from synapse.api.constants import ReceiptTypes | ||
from synapse.metrics.background_process_metrics import wrap_as_background_process | ||
from synapse.storage._base import SQLBaseStore, db_to_json | ||
from synapse.storage._base import SQLBaseStore, db_to_json, make_in_list_sql_clause | ||
from synapse.storage.database import ( | ||
DatabasePool, | ||
LoggingDatabaseConnection, | ||
|
@@ -259,7 +259,11 @@ def _get_unread_counts_by_receipt_txn( | |
txn, | ||
user_id, | ||
room_id, | ||
receipt_types=(ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE), | ||
receipt_types=( | ||
ReceiptTypes.READ, | ||
ReceiptTypes.READ_PRIVATE, | ||
ReceiptTypes.UNSTABLE_READ_PRIVATE, | ||
), | ||
) | ||
|
||
stream_ordering = None | ||
|
@@ -448,25 +452,37 @@ async def get_unread_push_actions_for_user_in_range_for_http( | |
The list will be ordered by ascending stream_ordering. | ||
The list will have between 0~limit entries. | ||
""" | ||
|
||
# find rooms that have a read receipt in them and return the next | ||
# push actions | ||
def get_after_receipt( | ||
txn: LoggingTransaction, | ||
) -> List[Tuple[str, str, int, str, bool]]: | ||
# find rooms that have a read receipt in them and return the next | ||
# push actions | ||
sql = """ | ||
|
||
receipt_types_clause, args = make_in_list_sql_clause( | ||
self.database_engine, | ||
"receipt_type", | ||
( | ||
ReceiptTypes.READ, | ||
ReceiptTypes.READ_PRIVATE, | ||
ReceiptTypes.UNSTABLE_READ_PRIVATE, | ||
), | ||
) | ||
|
||
sql = f""" | ||
SELECT ep.event_id, ep.room_id, ep.stream_ordering, ep.actions, | ||
ep.highlight | ||
FROM ( | ||
SELECT room_id, | ||
MAX(stream_ordering) as stream_ordering | ||
FROM events | ||
INNER JOIN receipts_linearized USING (room_id, event_id) | ||
WHERE receipt_type = 'm.read' AND user_id = ? | ||
WHERE {receipt_types_clause} AND user_id = ? | ||
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 think the ramifications of this change (and the ones below it) are that we might have HTTP pushed / emailed for something that was actually read using a private read receipt? Does that sound accurate? 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. So I am not sure I understand the code at hand though my thinking was that since both public and private RRs influence notifs in the same way, they should be added here. Was my thinking incorrect? 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.
That's my understanding as well. I'm trying to figure out the user-impact of fixing this bug. 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. OK I double checked this; I think (before fixing this to look at all read receipt types) you could end up in this situation:
Good that we found it, but doesn't seem to be the end of the world! 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.
Thank you very much for doing that! I've totally missed it |
||
GROUP BY room_id | ||
) AS rl, | ||
event_push_actions AS ep | ||
event_push_actions AS ep | ||
WHERE | ||
ep.room_id = rl.room_id | ||
AND ep.stream_ordering > rl.stream_ordering | ||
|
@@ -476,7 +492,9 @@ def get_after_receipt( | |
AND ep.notif = 1 | ||
ORDER BY ep.stream_ordering ASC LIMIT ? | ||
""" | ||
args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit] | ||
args.extend( | ||
(user_id, user_id, min_stream_ordering, max_stream_ordering, limit) | ||
) | ||
txn.execute(sql, args) | ||
return cast(List[Tuple[str, str, int, str, bool]], txn.fetchall()) | ||
|
||
|
@@ -490,15 +508,25 @@ def get_after_receipt( | |
def get_no_receipt( | ||
txn: LoggingTransaction, | ||
) -> List[Tuple[str, str, int, str, bool]]: | ||
sql = """ | ||
receipt_types_clause, args = make_in_list_sql_clause( | ||
self.database_engine, | ||
"receipt_type", | ||
( | ||
ReceiptTypes.READ, | ||
ReceiptTypes.READ_PRIVATE, | ||
ReceiptTypes.UNSTABLE_READ_PRIVATE, | ||
), | ||
) | ||
|
||
sql = f""" | ||
SELECT ep.event_id, ep.room_id, ep.stream_ordering, ep.actions, | ||
ep.highlight | ||
FROM event_push_actions AS ep | ||
INNER JOIN events AS e USING (room_id, event_id) | ||
WHERE | ||
ep.room_id NOT IN ( | ||
SELECT room_id FROM receipts_linearized | ||
WHERE receipt_type = 'm.read' AND user_id = ? | ||
WHERE {receipt_types_clause} AND user_id = ? | ||
GROUP BY room_id | ||
) | ||
AND ep.user_id = ? | ||
|
@@ -507,7 +535,9 @@ def get_no_receipt( | |
AND ep.notif = 1 | ||
ORDER BY ep.stream_ordering ASC LIMIT ? | ||
""" | ||
args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit] | ||
args.extend( | ||
(user_id, user_id, min_stream_ordering, max_stream_ordering, limit) | ||
) | ||
txn.execute(sql, args) | ||
return cast(List[Tuple[str, str, int, str, bool]], txn.fetchall()) | ||
|
||
|
@@ -557,20 +587,31 @@ async def get_unread_push_actions_for_user_in_range_for_email( | |
The list will be ordered by descending received_ts. | ||
The list will have between 0~limit entries. | ||
""" | ||
|
||
# find rooms that have a read receipt in them and return the most recent | ||
# push actions | ||
def get_after_receipt( | ||
txn: LoggingTransaction, | ||
) -> List[Tuple[str, str, int, str, bool, int]]: | ||
sql = """ | ||
receipt_types_clause, args = make_in_list_sql_clause( | ||
self.database_engine, | ||
"receipt_type", | ||
( | ||
ReceiptTypes.READ, | ||
ReceiptTypes.READ_PRIVATE, | ||
ReceiptTypes.UNSTABLE_READ_PRIVATE, | ||
), | ||
) | ||
|
||
sql = f""" | ||
SELECT ep.event_id, ep.room_id, ep.stream_ordering, ep.actions, | ||
ep.highlight, e.received_ts | ||
FROM ( | ||
SELECT room_id, | ||
MAX(stream_ordering) as stream_ordering | ||
FROM events | ||
INNER JOIN receipts_linearized USING (room_id, event_id) | ||
WHERE receipt_type = 'm.read' AND user_id = ? | ||
WHERE {receipt_types_clause} AND user_id = ? | ||
GROUP BY room_id | ||
) AS rl, | ||
event_push_actions AS ep | ||
|
@@ -584,7 +625,9 @@ def get_after_receipt( | |
AND ep.notif = 1 | ||
ORDER BY ep.stream_ordering DESC LIMIT ? | ||
""" | ||
args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit] | ||
args.extend( | ||
(user_id, user_id, min_stream_ordering, max_stream_ordering, limit) | ||
) | ||
txn.execute(sql, args) | ||
return cast(List[Tuple[str, str, int, str, bool, int]], txn.fetchall()) | ||
|
||
|
@@ -598,15 +641,25 @@ def get_after_receipt( | |
def get_no_receipt( | ||
txn: LoggingTransaction, | ||
) -> List[Tuple[str, str, int, str, bool, int]]: | ||
sql = """ | ||
receipt_types_clause, args = make_in_list_sql_clause( | ||
self.database_engine, | ||
"receipt_type", | ||
( | ||
ReceiptTypes.READ, | ||
ReceiptTypes.READ_PRIVATE, | ||
ReceiptTypes.UNSTABLE_READ_PRIVATE, | ||
), | ||
) | ||
|
||
sql = f""" | ||
SELECT ep.event_id, ep.room_id, ep.stream_ordering, ep.actions, | ||
ep.highlight, e.received_ts | ||
FROM event_push_actions AS ep | ||
INNER JOIN events AS e USING (room_id, event_id) | ||
WHERE | ||
ep.room_id NOT IN ( | ||
SELECT room_id FROM receipts_linearized | ||
WHERE receipt_type = 'm.read' AND user_id = ? | ||
WHERE {receipt_types_clause} AND user_id = ? | ||
GROUP BY room_id | ||
) | ||
AND ep.user_id = ? | ||
|
@@ -615,7 +668,9 @@ def get_no_receipt( | |
AND ep.notif = 1 | ||
ORDER BY ep.stream_ordering DESC LIMIT ? | ||
""" | ||
args = [user_id, user_id, min_stream_ordering, max_stream_ordering, limit] | ||
args.extend( | ||
(user_id, user_id, min_stream_ordering, max_stream_ordering, limit) | ||
) | ||
txn.execute(sql, args) | ||
return cast(List[Tuple[str, str, int, str, bool, int]], txn.fetchall()) | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.