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

Add support for stable prefixes for MSC2285: private read receipts #13273

Merged
merged 39 commits into from
Aug 5, 2022
Merged

Add support for stable prefixes for MSC2285: private read receipts #13273

Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
83b7af1
Switch to stable prefixes for MSC2285
SimonBrandner Jul 14, 2022
01fbb6d
Add changelog
SimonBrandner Jul 14, 2022
cafe4be
Avoid leaking unstable private read receipts
SimonBrandner Jul 14, 2022
967f8ed
Delint
SimonBrandner Jul 14, 2022
a9f2329
Fix code
SimonBrandner Jul 14, 2022
08d874d
Merge remote-tracking branch 'upstream/develop' into SimonBrandner/fe…
SimonBrandner Jul 23, 2022
ba20652
Use a const
SimonBrandner Jul 23, 2022
d18a485
Convert list to tuple.
clokep Jul 25, 2022
8323358
Merge remote-tracking branch 'upstream/develop' into SimonBrandner/fe…
SimonBrandner Jul 28, 2022
404459f
Add background update to remove unstable private read receipts
SimonBrandner Jul 29, 2022
57251c8
Remove handling for unstable private read receipts
SimonBrandner Jul 29, 2022
e12706c
Don't bump `SCHEMA_VERSION`
SimonBrandner Jul 29, 2022
c8cf10e
Remove `READ_PRIVATE_UNSTABLE` from `ReceiptTypes`
SimonBrandner Jul 29, 2022
84299f6
Assume return value
SimonBrandner Jul 29, 2022
208e97a
Remove non-relevant test
SimonBrandner Jul 29, 2022
a7c3a1c
Don't bother with a migration script
SimonBrandner Jul 29, 2022
252146d
Remove non-schema thing
SimonBrandner Jul 29, 2022
3cbd80b
Only use single `'`
SimonBrandner Jul 29, 2022
8854e35
Use `!=`
SimonBrandner Jul 29, 2022
baa9e84
Merge pull request #1 from SimonBrandner/SimonBrandner/feat/db-rr
SimonBrandner Jul 29, 2022
4455232
Changelog
SimonBrandner Aug 4, 2022
de9005f
Merge remote-tracking branch 'upstream/develop' into SimonBrandner/fe…
SimonBrandner Aug 4, 2022
c0b3c90
Support both stable and unstable private RRs
SimonBrandner Aug 4, 2022
3612ceb
Test both stable and unstable private RRs
SimonBrandner Aug 4, 2022
a37a17d
Advertise unstable prefix
SimonBrandner Aug 4, 2022
14b36a3
Check for all receipt types
SimonBrandner Aug 4, 2022
746351c
Delint
SimonBrandner Aug 4, 2022
50bffe4
Further dlint
SimonBrandner Aug 4, 2022
1b66814
Remove `(`
SimonBrandner Aug 4, 2022
bcdbb5d
This won't work...
SimonBrandner Aug 4, 2022
0f498e0
Reformat...
SimonBrandner Aug 4, 2022
660c776
Get tests passing
SimonBrandner Aug 4, 2022
b1ddf1a
Merge remote-tracking branch 'origin/develop' into SimonBrandner/feat…
clokep Aug 4, 2022
3094b39
Fix type hints.
clokep Aug 4, 2022
011d6d2
Put experimental flag back in place
SimonBrandner Aug 5, 2022
ee6512f
Only advertise `org.matrix.msc2285` if enabled
SimonBrandner Aug 5, 2022
2cd29bd
Fix tests
SimonBrandner Aug 5, 2022
89d5bd4
Merge remote-tracking branch 'origin/SimonBrandner/feat/disable-rr' i…
SimonBrandner Aug 5, 2022
f0c531d
Fix tests
SimonBrandner Aug 5, 2022
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
Prev Previous commit
Next Next commit
Merge remote-tracking branch 'origin/develop' into SimonBrandner/feat… 
…/disable-rr
  • Loading branch information
@clokep
clokep committed Aug 4, 2022
commit b1ddf1aae361bc0becb1c6e8c10923a7405c8d0d
14 changes: 7 additions & 7 deletions synapse/storage/databases/main/event_push_actions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,9 +478,9 @@ def get_after_receipt(
SELECT room_id,
MAX(stream_ordering) as stream_ordering
FROM events
INNER JOIN receipts_linearized USING (room_id, event_id)
WHERE {receipt_types_clause} AND user_id = ?
GROUP BY room_id
INNER JOIN receipts_linearized USING (room_id, event_id)
WHERE {receipt_types_clause} AND user_id = ?
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Member

Choose a reason for hiding this comment

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

Was my thinking incorrect?

That's my understanding as well. I'm trying to figure out the user-impact of fixing this bug.

Copy link
Member

Choose a reason for hiding this comment

The 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:

  1. There are some new events in a room.
  2. The user reads them and sends a private read receipt.
  3. They have an email or HTTP pusher configured.
  4. The private read receipt would not be considered read when checking for events to push.
  5. The new events would be pushed even though they're before the private read receipt.

Good that we found it, but doesn't seem to be the end of the world!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good that we found it

Thank you very much for doing that! I've totally missed it

GROUP BY room_id
) AS rl,
event_push_actions AS ep
WHERE
Expand Down Expand Up @@ -526,8 +526,8 @@ def get_no_receipt(
WHERE
ep.room_id NOT IN (
SELECT room_id FROM receipts_linearized
WHERE {receipt_types_clause} AND user_id = ?
GROUP BY room_id
WHERE {receipt_types_clause} AND user_id = ?
GROUP BY room_id
)
AND ep.user_id = ?
AND ep.stream_ordering > ?
Expand Down Expand Up @@ -659,8 +659,8 @@ def get_no_receipt(
WHERE
ep.room_id NOT IN (
SELECT room_id FROM receipts_linearized
WHERE {receipt_types_clause} AND user_id = ?
GROUP BY room_id
WHERE {receipt_types_clause} AND user_id = ?
GROUP BY room_id
)
AND ep.user_id = ?
AND ep.stream_ordering > ?
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.