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

Implement changes to MSC2285 (hidden read receipts) #12168

Merged
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
bc2a3db
Changelog
SimonBrandner Mar 5, 2022
926bd84
Implement changes to MSC2285
SimonBrandner Mar 5, 2022
4bf0a36
Improve tests
SimonBrandner Mar 5, 2022
b4e6eea
Add checks for `msc2285_enabled`
SimonBrandner Mar 10, 2022
18a96d2
Simplifie `filter_out_hidden()`
SimonBrandner Mar 10, 2022
6d017ce
Don't recheck `msc2285_enabled`
SimonBrandner Mar 10, 2022
77879c0
Add some comments
SimonBrandner Mar 10, 2022
c574c86
Add a comment regarding `parse_json_object_from_request()`
SimonBrandner Mar 10, 2022
5f1a963
Make handling of multiple receipt types sound
SimonBrandner Mar 15, 2022
a7845c5
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Mar 15, 2022
bce411c
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Mar 16, 2022
533421c
Simplifie code
SimonBrandner Mar 17, 2022
d060e12
Only handle `m.read.private` if MSC enabled
SimonBrandner Mar 17, 2022
c73005f
Use f-strings
SimonBrandner Mar 17, 2022
100120f
Make things more readable - don't use "reverse"
SimonBrandner Mar 17, 2022
3527966
Remove `test_does_not_filter_out_our_hidden_receipt()` as `test_leave…
SimonBrandner Mar 17, 2022
1d7e274
Test that the read receipt doesn't go up for either receipt type
SimonBrandner Mar 17, 2022
26aae3e
Test that we can't override private read receipts
SimonBrandner Mar 17, 2022
245b723
Make sure read receipts don't go up
SimonBrandner Mar 17, 2022
5732c35
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Mar 17, 2022
d4a2d85
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Mar 18, 2022
fd277aa
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Mar 27, 2022
3713dcd
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Mar 29, 2022
38db694
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Mar 30, 2022
678adcd
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Apr 1, 2022
2044845
Fix tests
SimonBrandner Apr 1, 2022
21754ea
Simplify code and improve comment
SimonBrandner Apr 1, 2022
7aca89c
Use `parameterized` instead of a for loop
SimonBrandner Apr 1, 2022
88f47b6
Delint
SimonBrandner Apr 1, 2022
ac3c0ea
Throw when receipt type is not known
SimonBrandner Apr 1, 2022
85aa704
Replace `_check_no_room_changes()`
SimonBrandner Apr 2, 2022
f837981
Test receipt overriding
SimonBrandner Apr 2, 2022
cc3e273
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Apr 5, 2022
21ac95e
Support specifying the receipt types we want
SimonBrandner Apr 8, 2022
373f0b7
Improve `SlavedReceiptTestCase`
SimonBrandner Apr 8, 2022
2c8c550
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Apr 8, 2022
455d500
Use `GROUP BY`
SimonBrandner Apr 8, 2022
90ea618
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Apr 9, 2022
809596a
Use `ORDER BY` and `LIMIT`
SimonBrandner Apr 9, 2022
2e198fe
Fix `GROUP BY` error?
SimonBrandner Apr 9, 2022
958ff56
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Apr 13, 2022
116d1c9
Make `get_receipts_for_user_with_orderings()` handle multiple receipt…
SimonBrandner Apr 14, 2022
7c22108
Be smarter when handling non-allowed keys
SimonBrandner Apr 14, 2022
0f117e4
Fix types
SimonBrandner Apr 14, 2022
084b7ce
Fix types (again)
SimonBrandner Apr 14, 2022
2da9935
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Apr 22, 2022
91a4bf1
Fix typo
SimonBrandner Apr 22, 2022
d48fc99
Merge remote-tracking branch 'origin/develop' into feature/private-re…
clokep Apr 27, 2022
ae0bf97
Add some notes.
clokep Apr 27, 2022
4e255a6
Add caching back to get_last_receipt_event_id_for_user.
clokep Apr 28, 2022
e9055ac
Add caching back to get_latest_receipts_for_user.
clokep Apr 28, 2022
8041f82
Rename get_latest_receipts_for_user back to get_receipts_for_user to …
clokep Apr 28, 2022
25632cf
Merge remote-tracking branch 'origin/develop' into feature/private-re…
clokep Apr 28, 2022
670e3e4
Improve docstrings.
clokep Apr 29, 2022
9e60cdb
Document behavior of ordering of receipt types mattering.
clokep Apr 29, 2022
10822e1
Minor refactoring.
clokep Apr 29, 2022
40782b8
Fix comment
SimonBrandner Apr 30, 2022
1b3987e
Merge remote-tracking branch 'upstream/develop' into feature/private-…
SimonBrandner Apr 30, 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
1 change: 1 addition & 0 deletions changelog.d/12168.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implement [changes](https://github.com/matrix-org/matrix-spec-proposals/pull/2285/commits/4a77139249c2e830aec3c7d6bd5501a514d1cc27) to [MSC2285 (hidden read receipts)](https://github.com/matrix-org/matrix-spec-proposals/pull/2285). Contributed by @SimonBrandner.
6 changes: 2 additions & 4 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,5 @@ class GuestAccess:

class ReceiptTypes:
READ: Final = "m.read"


class ReadReceiptEventFields:
MSC2285_HIDDEN: Final = "org.matrix.msc2285.hidden"
READ_PRIVATE: Final = "org.matrix.msc2285.read.private"
FULLY_READ: Final = "m.fully_read"
63 changes: 26 additions & 37 deletions synapse/handlers/receipts.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import logging
from typing import TYPE_CHECKING, Iterable, List, Optional, Tuple

from synapse.api.constants import ReadReceiptEventFields, ReceiptTypes
from synapse.api.constants import ReceiptTypes
from synapse.appservice import ApplicationService
from synapse.streams import EventSource
from synapse.types import JsonDict, ReadReceipt, UserID, get_domain_from_id
Expand Down Expand Up @@ -138,7 +138,7 @@ async def _handle_new_receipts(self, receipts: List[ReadReceipt]) -> bool:
return True

async def received_client_receipt(
self, room_id: str, receipt_type: str, user_id: str, event_id: str, hidden: bool
self, room_id: str, receipt_type: str, user_id: str, event_id: str
) -> None:
"""Called when a client tells us a local user has read up to the given
event_id in the room.
Expand All @@ -148,16 +148,14 @@ async def received_client_receipt(
receipt_type=receipt_type,
user_id=user_id,
event_ids=[event_id],
data={"ts": int(self.clock.time_msec()), "hidden": hidden},
data={"ts": int(self.clock.time_msec())},
)

is_new = await self._handle_new_receipts([receipt])
if not is_new:
return

if self.federation_sender and not (
self.hs.config.experimental.msc2285_enabled and hidden
):
if self.federation_sender and receipt_type != ReceiptTypes.READ_PRIVATE:
await self.federation_sender.send_read_receipt(receipt)


Expand All @@ -168,6 +166,13 @@ def __init__(self, hs: "HomeServer"):

@staticmethod
def filter_out_hidden(events: List[JsonDict], user_id: str) -> List[JsonDict]:
clokep marked this conversation as resolved.
Show resolved Hide resolved
"""
This method takes in what is returned by
get_linearized_receipts_for_rooms() and goes through read receipts
filtering out m.read.private receipts if they were not sent by the
current user.
"""

visible_events = []

# filter out hidden receipts the user shouldn't see
Expand All @@ -176,37 +181,21 @@ def filter_out_hidden(events: List[JsonDict], user_id: str) -> List[JsonDict]:
new_event = event.copy()
new_event["content"] = {}

for event_id in content.keys():
event_content = content.get(event_id, {})
m_read = event_content.get(ReceiptTypes.READ, {})

# If m_read is missing copy over the original event_content as there is nothing to process here
if not m_read:
new_event["content"][event_id] = event_content.copy()
continue

new_users = {}
for rr_user_id, user_rr in m_read.items():
try:
hidden = user_rr.get("hidden")
except AttributeError:
# Due to https://github.com/matrix-org/synapse/issues/10376
# there are cases where user_rr is a string, in those cases
# we just ignore the read receipt
continue

if hidden is not True or rr_user_id == user_id:
new_users[rr_user_id] = user_rr.copy()
# If hidden has a value replace hidden with the correct prefixed key
if hidden is not None:
new_users[rr_user_id].pop("hidden")
new_users[rr_user_id][
ReadReceiptEventFields.MSC2285_HIDDEN
] = hidden

# Set new users unless empty
if len(new_users.keys()) > 0:
new_event["content"][event_id] = {ReceiptTypes.READ: new_users}
for event_id, event_content in content.items():
receipt_event = {}
for receipt_type, receipt_content in event_content.items():
if receipt_type == ReceiptTypes.READ_PRIVATE:
user_rr = receipt_content.get(user_id, None)
if user_rr:
receipt_event[ReceiptTypes.READ_PRIVATE] = {
user_id: user_rr.copy()
}
else:
receipt_event[receipt_type] = receipt_content.copy()

# Only include the receipt event if it is non-empty.
if receipt_event:
new_event["content"][event_id] = receipt_event

# Append new_event to visible_events unless empty
if len(new_event["content"].keys()) > 0:
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ async def unread_notifs_for_room_id(
last_unread_event_id = await self.store.get_last_receipt_event_id_for_user(
user_id=sync_config.user.to_string(),
room_id=room_id,
receipt_type=ReceiptTypes.READ,
receipt_types=[ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE],
)

return await self.store.get_unread_event_push_actions_by_room_for_user(
Expand Down
4 changes: 3 additions & 1 deletion synapse/push/push_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ async def get_badge_count(store: DataStore, user_id: str, group_by_room: bool) -
invites = await store.get_invited_rooms_for_local_user(user_id)
joins = await store.get_rooms_for_user(user_id)

my_receipts_by_room = await store.get_receipts_for_user(user_id, ReceiptTypes.READ)
my_receipts_by_room = await store.get_latest_receipts_for_user(
user_id, [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE]
)

badge = len(invites)

Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/client/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
)

receipts_by_room = await self.store.get_receipts_for_user_with_orderings(
user_id, ReceiptTypes.READ
user_id, [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE]
)

notif_event_ids = [pa.event_id for pa in push_actions]
Expand Down
32 changes: 22 additions & 10 deletions synapse/rest/client/read_marker.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
import logging
from typing import TYPE_CHECKING, Tuple

from synapse.api.constants import ReadReceiptEventFields, ReceiptTypes
from synapse.api.errors import Codes, SynapseError
from synapse.api.constants import ReceiptTypes
from synapse.api.errors import SynapseError
from synapse.http.server import HttpServer
from synapse.http.servlet import RestServlet, parse_json_object_from_request
from synapse.http.site import SynapseRequest
Expand All @@ -36,6 +36,7 @@ class ReadMarkerRestServlet(RestServlet):
def __init__(self, hs: "HomeServer"):
super().__init__()
self.auth = hs.get_auth()
self.config = hs.config
self.receipts_handler = hs.get_receipts_handler()
self.read_marker_handler = hs.get_read_marker_handler()
self.presence_handler = hs.get_presence_handler()
Expand All @@ -48,27 +49,38 @@ async def on_POST(
await self.presence_handler.bump_presence_active_time(requester.user)

body = parse_json_object_from_request(request)
read_event_id = body.get(ReceiptTypes.READ, None)
hidden = body.get(ReadReceiptEventFields.MSC2285_HIDDEN, False)

if not isinstance(hidden, bool):
valid_receipt_types = {ReceiptTypes.READ, ReceiptTypes.FULLY_READ}
if self.config.experimental.msc2285_enabled:
valid_receipt_types.add(ReceiptTypes.READ_PRIVATE)

if set(body.keys()) > valid_receipt_types:
raise SynapseError(
400,
"Param %s must be a boolean, if given"
% ReadReceiptEventFields.MSC2285_HIDDEN,
Codes.BAD_JSON,
"Receipt type must be 'm.read', 'org.matrix.msc2285.read.private' or 'm.fully_read'"
if self.config.experimental.msc2285_enabled
else "Receipt type must be 'm.read' or 'm.fully_read'",
)

read_event_id = body.get(ReceiptTypes.READ, None)
if read_event_id:
await self.receipts_handler.received_client_receipt(
room_id,
ReceiptTypes.READ,
user_id=requester.user.to_string(),
event_id=read_event_id,
hidden=hidden,
)

read_marker_event_id = body.get("m.fully_read", None)
read_private_event_id = body.get(ReceiptTypes.READ_PRIVATE, None)
if read_private_event_id and self.config.experimental.msc2285_enabled:
SimonBrandner marked this conversation as resolved.
Show resolved Hide resolved
await self.receipts_handler.received_client_receipt(
room_id,
ReceiptTypes.READ_PRIVATE,
user_id=requester.user.to_string(),
event_id=read_private_event_id,
)

read_marker_event_id = body.get(ReceiptTypes.FULLY_READ, None)
if read_marker_event_id:
await self.read_marker_handler.received_client_read_marker(
room_id,
Expand Down
51 changes: 31 additions & 20 deletions synapse/rest/client/receipts.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
import re
from typing import TYPE_CHECKING, Tuple

from synapse.api.constants import ReadReceiptEventFields, ReceiptTypes
from synapse.api.errors import Codes, SynapseError
from synapse.api.constants import ReceiptTypes
from synapse.api.errors import SynapseError
from synapse.http import get_request_user_agent
from synapse.http.server import HttpServer
from synapse.http.servlet import RestServlet, parse_json_object_from_request
Expand Down Expand Up @@ -46,14 +46,27 @@ def __init__(self, hs: "HomeServer"):
self.hs = hs
self.auth = hs.get_auth()
self.receipts_handler = hs.get_receipts_handler()
self.read_marker_handler = hs.get_read_marker_handler()
self.presence_handler = hs.get_presence_handler()

async def on_POST(
SimonBrandner marked this conversation as resolved.
Show resolved Hide resolved
self, request: SynapseRequest, room_id: str, receipt_type: str, event_id: str
) -> Tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request)

if receipt_type != ReceiptTypes.READ:
if self.hs.config.experimental.msc2285_enabled and receipt_type not in [
ReceiptTypes.READ,
ReceiptTypes.READ_PRIVATE,
ReceiptTypes.FULLY_READ,
clokep marked this conversation as resolved.
Show resolved Hide resolved
]:
raise SynapseError(
400,
"Receipt type must be 'm.read', 'org.matrix.msc2285.read.private' or 'm.fully_read'",
)
elif (
not self.hs.config.experimental.msc2285_enabled
and receipt_type != ReceiptTypes.READ
):
raise SynapseError(400, "Receipt type must be 'm.read'")

# Do not allow older SchildiChat and Element Android clients (prior to Element/1.[012].x) to send an empty body.
Expand All @@ -62,26 +75,24 @@ async def on_POST(
if "Android" in user_agent:
if pattern.match(user_agent) or "Riot" in user_agent:
allow_empty_body = True
body = parse_json_object_from_request(request, allow_empty_body)
hidden = body.get(ReadReceiptEventFields.MSC2285_HIDDEN, False)

if not isinstance(hidden, bool):
raise SynapseError(
400,
"Param %s must be a boolean, if given"
% ReadReceiptEventFields.MSC2285_HIDDEN,
Codes.BAD_JSON,
)
# This call makes sure possible empty body is handled correctly
parse_json_object_from_request(request, allow_empty_body)
SimonBrandner marked this conversation as resolved.
Show resolved Hide resolved

await self.presence_handler.bump_presence_active_time(requester.user)

await self.receipts_handler.received_client_receipt(
room_id,
receipt_type,
user_id=requester.user.to_string(),
event_id=event_id,
hidden=hidden,
)
if receipt_type == ReceiptTypes.FULLY_READ:
await self.read_marker_handler.received_client_read_marker(
room_id,
user_id=requester.user.to_string(),
event_id=event_id,
)
else:
await self.receipts_handler.received_client_receipt(
room_id,
receipt_type,
user_id=requester.user.to_string(),
event_id=event_id,
)

return 200, {}

Expand Down
Loading