diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 36ace7c6134f..b4534ddd1ca9 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -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" diff --git a/synapse/handlers/receipts.py b/synapse/handlers/receipts.py index 6250bb3bdf2b..b5971bdbfc1b 100644 --- a/synapse/handlers/receipts.py +++ b/synapse/handlers/receipts.py @@ -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 @@ -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. @@ -148,7 +148,7 @@ 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]) @@ -156,7 +156,8 @@ async def received_client_receipt( return if self.federation_sender and not ( - self.hs.config.experimental.msc2285_enabled and hidden + self.hs.config.experimental.msc2285_enabled + and receipt_type == ReceiptTypes.READ_PRIVATE ): await self.federation_sender.send_read_receipt(receipt) @@ -178,35 +179,27 @@ def filter_out_hidden(events: List[JsonDict], user_id: str) -> List[JsonDict]: 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() + m_read = event_content.get(ReceiptTypes.READ, None) + if m_read: + new_event["content"][event_id] = {ReceiptTypes.READ: m_read} 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 + m_read_private = event_content.get(ReceiptTypes.READ_PRIVATE, None) + if m_read_private: + new_users = {} + for rr_user_id, user_rr in m_read_private.items(): + if rr_user_id == user_id: + new_users[rr_user_id] = user_rr.copy() + + # Set new users unless empty + if len(new_users.keys()) > 0: + new_event["content"][event_id] = { + ReceiptTypes.READ_PRIVATE: new_users + } + 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} + new_event["content"][event_id] = event_content # Append new_event to visible_events unless empty if len(new_event["content"].keys()) > 0: diff --git a/synapse/rest/client/read_marker.py b/synapse/rest/client/read_marker.py index f51be511d1f4..51e09dc1d87e 100644 --- a/synapse/rest/client/read_marker.py +++ b/synapse/rest/client/read_marker.py @@ -15,7 +15,7 @@ import logging from typing import TYPE_CHECKING, Tuple -from synapse.api.constants import ReadReceiptEventFields, ReceiptTypes +from synapse.api.constants import ReceiptTypes from synapse.api.errors import Codes, SynapseError from synapse.http.server import HttpServer from synapse.http.servlet import RestServlet, parse_json_object_from_request @@ -48,27 +48,26 @@ 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): - raise SynapseError( - 400, - "Param %s must be a boolean, if given" - % ReadReceiptEventFields.MSC2285_HIDDEN, - Codes.BAD_JSON, - ) + 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: + 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, diff --git a/synapse/rest/client/receipts.py b/synapse/rest/client/receipts.py index b24ad2d1be13..d209a8b9953f 100644 --- a/synapse/rest/client/receipts.py +++ b/synapse/rest/client/receipts.py @@ -16,7 +16,7 @@ import re from typing import TYPE_CHECKING, Tuple -from synapse.api.constants import ReadReceiptEventFields, ReceiptTypes +from synapse.api.constants import ReceiptTypes from synapse.api.errors import Codes, SynapseError from synapse.http import get_request_user_agent from synapse.http.server import HttpServer @@ -46,6 +46,7 @@ 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( @@ -53,8 +54,14 @@ async def on_POST( ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) - if receipt_type != ReceiptTypes.READ: - raise SynapseError(400, "Receipt type must be 'm.read'") + if receipt_type in [ + ReceiptTypes.READ, + ReceiptTypes.READ_PRIVATE, + ReceiptTypes.FULLY_READ, + ]: + raise SynapseError( + 400, "Receipt type must be 'm.read', 'm.read.private' or 'm.fully_read'" + ) # Do not allow older SchildiChat and Element Android clients (prior to Element/1.[012].x) to send an empty body. user_agent = get_request_user_agent(request) @@ -63,25 +70,22 @@ async def on_POST( 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, - ) 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=read_marker_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, {} diff --git a/tests/handlers/test_receipts.py b/tests/handlers/test_receipts.py index 5081b97573a0..362eac8f8add 100644 --- a/tests/handlers/test_receipts.py +++ b/tests/handlers/test_receipts.py @@ -15,7 +15,7 @@ from typing import List -from synapse.api.constants import ReadReceiptEventFields +from synapse.api.constants import ReceiptTypes from synapse.types import JsonDict from tests import unittest @@ -25,20 +25,15 @@ class ReceiptsTestCase(unittest.HomeserverTestCase): def prepare(self, reactor, clock, hs): self.event_source = hs.get_event_sources().sources.receipt - # In the first param of _test_filters_hidden we use "hidden" instead of - # ReadReceiptEventFields.MSC2285_HIDDEN. We do this because we're mocking - # the data from the database which doesn't use the prefix - def test_filters_out_hidden_receipt(self): self._test_filters_hidden( [ { "content": { "$1435641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ_PRIVATE: { "@rikj:jki.re": { "ts": 1436451550453, - "hidden": True, } } } @@ -56,10 +51,9 @@ def test_does_not_filter_out_our_hidden_receipt(self): { "content": { "$1435641916hfgh4394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ_PRIVATE: { "@me:server.org": { "ts": 1436451550453, - "hidden": True, }, } } @@ -72,10 +66,9 @@ def test_does_not_filter_out_our_hidden_receipt(self): { "content": { "$1435641916hfgh4394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ_PRIVATE: { "@me:server.org": { "ts": 1436451550453, - ReadReceiptEventFields.MSC2285_HIDDEN: True, }, } } @@ -92,16 +85,19 @@ def test_filters_out_hidden_receipt_and_ignores_rest(self): { "content": { "$1dgdgrd5641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ_PRIVATE: { "@rikj:jki.re": { "ts": 1436451550453, - "hidden": True, }, + } + }, + "$1dgdgrd5641916114394fHBLK:matrix.org": { + ReceiptTypes.READ: { "@user:jki.re": { "ts": 1436451550453, }, } - } + }, }, "room_id": "!jEsUZKDJdhlrceRyVU:example.org", "type": "m.receipt", @@ -111,7 +107,7 @@ def test_filters_out_hidden_receipt_and_ignores_rest(self): { "content": { "$1dgdgrd5641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ: { "@user:jki.re": { "ts": 1436451550453, } @@ -130,15 +126,14 @@ def test_filters_out_event_with_only_hidden_receipts_and_ignores_the_rest(self): { "content": { "$14356419edgd14394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ_PRIVATE: { "@rikj:jki.re": { "ts": 1436451550453, - "hidden": True, }, } }, "$1435641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ: { "@user:jki.re": { "ts": 1436451550453, } @@ -153,7 +148,7 @@ def test_filters_out_event_with_only_hidden_receipts_and_ignores_the_rest(self): { "content": { "$1435641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ: { "@user:jki.re": { "ts": 1436451550453, } @@ -171,9 +166,9 @@ def test_handles_missing_content_of_m_read(self): [ { "content": { - "$14356419ggffg114394fHBLK:matrix.org": {"m.read": {}}, + "$14356419ggffg114394fHBLK:matrix.org": {ReceiptTypes.READ: {}}, "$1435641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ: { "@user:jki.re": { "ts": 1436451550453, } @@ -187,9 +182,9 @@ def test_handles_missing_content_of_m_read(self): [ { "content": { - "$14356419ggffg114394fHBLK:matrix.org": {"m.read": {}}, + "$14356419ggffg114394fHBLK:matrix.org": {ReceiptTypes.READ: {}}, "$1435641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ: { "@user:jki.re": { "ts": 1436451550453, } @@ -209,7 +204,7 @@ def test_handles_empty_event(self): "content": { "$143564gdfg6114394fHBLK:matrix.org": {}, "$1435641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ: { "@user:jki.re": { "ts": 1436451550453, } @@ -225,7 +220,7 @@ def test_handles_empty_event(self): "content": { "$143564gdfg6114394fHBLK:matrix.org": {}, "$1435641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ: { "@user:jki.re": { "ts": 1436451550453, } @@ -244,10 +239,9 @@ def test_filters_out_receipt_event_with_only_hidden_receipt_and_ignores_rest(sel { "content": { "$14356419edgd14394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ_PRIVATE: { "@rikj:jki.re": { "ts": 1436451550453, - "hidden": True, }, } }, @@ -258,7 +252,7 @@ def test_filters_out_receipt_event_with_only_hidden_receipt_and_ignores_rest(sel { "content": { "$1435641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ: { "@user:jki.re": { "ts": 1436451550453, } @@ -273,7 +267,7 @@ def test_filters_out_receipt_event_with_only_hidden_receipt_and_ignores_rest(sel { "content": { "$1435641916114394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ: { "@user:jki.re": { "ts": 1436451550453, } @@ -297,7 +291,20 @@ def test_handles_string_data(self): { "content": { "$14356419edgd14394fHBLK:matrix.org": { - "m.read": { + ReceiptTypes.READ: { + "@rikj:jki.re": "string", + } + }, + }, + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "type": "m.receipt", + }, + ], + [ + { + "content": { + "$14356419edgd14394fHBLK:matrix.org": { + ReceiptTypes.READ: { "@rikj:jki.re": "string", } }, @@ -306,7 +313,6 @@ def test_handles_string_data(self): "type": "m.receipt", }, ], - [], ) def _test_filters_hidden( diff --git a/tests/rest/client/test_sync.py b/tests/rest/client/test_sync.py index 435101395204..3d828f173048 100644 --- a/tests/rest/client/test_sync.py +++ b/tests/rest/client/test_sync.py @@ -413,10 +413,9 @@ def test_hidden_read_receipts(self) -> None: res = self.helper.send(self.room_id, body="hello", tok=self.tok) # Send a read receipt to tell the server the first user's message was read - body = json.dumps({ReadReceiptEventFields.MSC2285_HIDDEN: True}).encode("utf8") channel = self.make_request( "POST", - "/rooms/%s/receipt/m.read/%s" % (self.room_id, res["event_id"]), + "/rooms/%s/receipt/m.read.private/%s" % (self.room_id, res["event_id"]), body, access_token=self.tok2, ) @@ -573,10 +572,9 @@ def test_unread_counts(self) -> None: self._check_unread_count(1) # Send a read receipt to tell the server we've read the latest event. - body = json.dumps({ReadReceiptEventFields.MSC2285_HIDDEN: True}).encode("utf8") channel = self.make_request( "POST", - "/rooms/%s/receipt/m.read/%s" % (self.room_id, res["event_id"]), + "/rooms/%s/receipt/m.read.private/%s" % (self.room_id, res["event_id"]), body, access_token=self.tok, )