Remove support for unstable private read receipts (#13653)

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

changelog.d/13653.removal

@@ -0,0 +1 @@
+Remove support for unstable [private read receipts](https://github.com/matrix-org/matrix-spec-proposals/pull/2285).

synapse/api/constants.py

@@ -258,7 +258,6 @@ class GuestAccess:
 class ReceiptTypes:
     READ: Final = "m.read"
     READ_PRIVATE: Final = "m.read.private"
-    UNSTABLE_READ_PRIVATE: Final = "org.matrix.msc2285.read.private"
     FULLY_READ: Final = "m.fully_read"
 
 

synapse/config/experimental.py

@@ -32,9 +32,6 @@ class ExperimentalConfig(Config):
         # MSC2716 (importing historical messages)
         self.msc2716_enabled: bool = experimental.get("msc2716_enabled", False)
 
-        # MSC2285 (unstable private read receipts)
-        self.msc2285_enabled: bool = experimental.get("msc2285_enabled", False)
-
         # MSC3244 (room version capabilities)
         self.msc3244_enabled: bool = experimental.get("msc3244_enabled", True)
 

synapse/handlers/receipts.py

@@ -163,10 +163,7 @@ class ReceiptsHandler:
         if not is_new:
             return
 
-        if self.federation_sender and receipt_type not in (
+        if self.federation_sender and receipt_type != ReceiptTypes.READ_PRIVATE:
-            ReceiptTypes.READ_PRIVATE,
-            ReceiptTypes.UNSTABLE_READ_PRIVATE,
-        ):
             await self.federation_sender.send_read_receipt(receipt)
 
 
@@ -206,38 +203,24 @@ class ReceiptEventSource(EventSource[int, JsonDict]):
             for event_id, orig_event_content in room.get("content", {}).items():
                 event_content = orig_event_content
                 # If there are private read receipts, additional logic is necessary.
-                if (
+                if ReceiptTypes.READ_PRIVATE in event_content:
-                    ReceiptTypes.READ_PRIVATE in event_content
-                    or ReceiptTypes.UNSTABLE_READ_PRIVATE in event_content
-                ):
                     # Make a copy without private read receipts to avoid leaking
                     # other user's private read receipts..
                     event_content = {
                         receipt_type: receipt_value
                         for receipt_type, receipt_value in event_content.items()
-                        if receipt_type
+                        if receipt_type != ReceiptTypes.READ_PRIVATE
-                        not in (
-                            ReceiptTypes.READ_PRIVATE,
-                            ReceiptTypes.UNSTABLE_READ_PRIVATE,
-                        )
                     }
 
                     # Copy the current user's private read receipt from the
                     # original content, if it exists.
-                    user_private_read_receipt = orig_event_content.get(
+                    user_private_read_receipt = orig_event_content[
-                        ReceiptTypes.READ_PRIVATE, {}
+                        ReceiptTypes.READ_PRIVATE
-                    ).get(user_id, None)
+                    ].get(user_id, None)
                     if user_private_read_receipt:
                         event_content[ReceiptTypes.READ_PRIVATE] = {
                             user_id: user_private_read_receipt
                         }
-                    user_unstable_private_read_receipt = orig_event_content.get(
-                        ReceiptTypes.UNSTABLE_READ_PRIVATE, {}
-                    ).get(user_id, None)
-                    if user_unstable_private_read_receipt:
-                        event_content[ReceiptTypes.UNSTABLE_READ_PRIVATE] = {
-                            user_id: user_unstable_private_read_receipt
-                        }
 
                 # Include the event if there is at least one non-private read
                 # receipt or the current user has a private read receipt.

synapse/replication/tcp/client.py

@@ -416,10 +416,7 @@ class FederationSenderHandler:
             if not self._is_mine_id(receipt.user_id):
                 continue
             # Private read receipts never get sent over federation.
-            if receipt.receipt_type in (
+            if receipt.receipt_type == ReceiptTypes.READ_PRIVATE:
-                ReceiptTypes.READ_PRIVATE,
-                ReceiptTypes.UNSTABLE_READ_PRIVATE,
-            ):
                 continue
             receipt_info = ReadReceipt(
                 receipt.room_id,

synapse/rest/client/notifications.py

@@ -62,7 +62,6 @@ class NotificationsServlet(RestServlet):
             [
                 ReceiptTypes.READ,
                 ReceiptTypes.READ_PRIVATE,
-                ReceiptTypes.UNSTABLE_READ_PRIVATE,
             ],
         )
 

synapse/rest/client/read_marker.py

@@ -45,8 +45,6 @@ class ReadMarkerRestServlet(RestServlet):
             ReceiptTypes.FULLY_READ,
             ReceiptTypes.READ_PRIVATE,
         }
-        if hs.config.experimental.msc2285_enabled:
-            self._known_receipt_types.add(ReceiptTypes.UNSTABLE_READ_PRIVATE)
 
     async def on_POST(
         self, request: SynapseRequest, room_id: str

synapse/rest/client/receipts.py

@@ -49,8 +49,6 @@ class ReceiptRestServlet(RestServlet):
             ReceiptTypes.READ_PRIVATE,
             ReceiptTypes.FULLY_READ,
         }
-        if hs.config.experimental.msc2285_enabled:
-            self._known_receipt_types.add(ReceiptTypes.UNSTABLE_READ_PRIVATE)
 
     async def on_POST(
         self, request: SynapseRequest, room_id: str, receipt_type: str, event_id: str

synapse/rest/client/versions.py

@@ -95,7 +95,6 @@ class VersionsRestServlet(RestServlet):
                     "org.matrix.msc3026.busy_presence": self.config.experimental.msc3026_enabled,
                     # Supports receiving private read receipts as per MSC2285
                     "org.matrix.msc2285.stable": True,  # TODO: Remove when MSC2285 becomes a part of the spec
-                    "org.matrix.msc2285": self.config.experimental.msc2285_enabled,
                     # Supports filtering of /publicRooms by room type as per MSC3827
                     "org.matrix.msc3827.stable": True,
                     # Adds support for importing historical messages as per MSC2716

synapse/storage/databases/main/event_push_actions.py

@@ -274,7 +274,6 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas
             receipt_types=(
                 ReceiptTypes.READ,
                 ReceiptTypes.READ_PRIVATE,
-                ReceiptTypes.UNSTABLE_READ_PRIVATE,
             ),
         )
 
@@ -468,7 +467,6 @@ class EventPushActionsWorkerStore(ReceiptsWorkerStore, StreamWorkerStore, SQLBas
             (
                 ReceiptTypes.READ,
                 ReceiptTypes.READ_PRIVATE,
-                ReceiptTypes.UNSTABLE_READ_PRIVATE,
             ),
         )
 

tests/handlers/test_receipts.py

@@ -15,8 +15,6 @@
 from copy import deepcopy
 from typing import List
 
-from parameterized import parameterized
-
 from synapse.api.constants import EduTypes, ReceiptTypes
 from synapse.types import JsonDict
 
@@ -27,16 +25,13 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
     def prepare(self, reactor, clock, hs):
         self.event_source = hs.get_event_sources().sources.receipt
 
-    @parameterized.expand(
+    def test_filters_out_private_receipt(self) -> None:
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
-    def test_filters_out_private_receipt(self, receipt_type: str) -> None:
         self._test_filters_private(
             [
                 {
                     "content": {
                         "$1435641916114394fHBLK:matrix.org": {
-                            receipt_type: {
+                            ReceiptTypes.READ_PRIVATE: {
                                 "@rikj:jki.re": {
                                     "ts": 1436451550453,
                                 }
@@ -50,18 +45,13 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             [],
         )
 
-    @parameterized.expand(
+    def test_filters_out_private_receipt_and_ignores_rest(self) -> None:
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
-    def test_filters_out_private_receipt_and_ignores_rest(
-        self, receipt_type: str
-    ) -> None:
         self._test_filters_private(
             [
                 {
                     "content": {
                         "$1dgdgrd5641916114394fHBLK:matrix.org": {
-                            receipt_type: {
+                            ReceiptTypes.READ_PRIVATE: {
                                 "@rikj:jki.re": {
                                     "ts": 1436451550453,
                                 },
@@ -94,18 +84,15 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-    @parameterized.expand(
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
     def test_filters_out_event_with_only_private_receipts_and_ignores_the_rest(
-        self, receipt_type: str
+        self,
     ) -> None:
         self._test_filters_private(
             [
                 {
                     "content": {
                         "$14356419edgd14394fHBLK:matrix.org": {
-                            receipt_type: {
+                            ReceiptTypes.READ_PRIVATE: {
                                 "@rikj:jki.re": {
                                     "ts": 1436451550453,
                                 },
@@ -175,18 +162,15 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-    @parameterized.expand(
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
     def test_filters_out_receipt_event_with_only_private_receipt_and_ignores_rest(
-        self, receipt_type: str
+        self,
     ) -> None:
         self._test_filters_private(
             [
                 {
                     "content": {
                         "$14356419edgd14394fHBLK:matrix.org": {
-                            receipt_type: {
+                            ReceiptTypes.READ_PRIVATE: {
                                 "@rikj:jki.re": {
                                     "ts": 1436451550453,
                                 },
@@ -262,16 +246,13 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-    @parameterized.expand(
+    def test_leaves_our_private_and_their_public(self) -> None:
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
-    def test_leaves_our_private_and_their_public(self, receipt_type: str) -> None:
         self._test_filters_private(
             [
                 {
                     "content": {
                         "$1dgdgrd5641916114394fHBLK:matrix.org": {
-                            receipt_type: {
+                            ReceiptTypes.READ_PRIVATE: {
                                 "@me:server.org": {
                                     "ts": 1436451550453,
                                 },
@@ -296,7 +277,7 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
                 {
                     "content": {
                         "$1dgdgrd5641916114394fHBLK:matrix.org": {
-                            receipt_type: {
+                            ReceiptTypes.READ_PRIVATE: {
                                 "@me:server.org": {
                                     "ts": 1436451550453,
                                 },
@@ -319,16 +300,13 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-    @parameterized.expand(
+    def test_we_do_not_mutate(self) -> None:
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
-    def test_we_do_not_mutate(self, receipt_type: str) -> None:
         """Ensure the input values are not modified."""
         events = [
             {
                 "content": {
                     "$1435641916114394fHBLK:matrix.org": {
-                        receipt_type: {
+                        ReceiptTypes.READ_PRIVATE: {
                             "@rikj:jki.re": {
                                 "ts": 1436451550453,
                             }

tests/rest/client/test_sync.py

@@ -391,7 +391,6 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
 
     def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
         config = self.default_config()
-        config["experimental_features"] = {"msc2285_enabled": True}
 
         return self.setup_test_homeserver(config=config)
 
@@ -413,17 +412,14 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
         # Join the second user
         self.helper.join(room=self.room_id, user=self.user2, tok=self.tok2)
 
-    @parameterized.expand(
+    def test_private_read_receipts(self) -> None:
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
-    def test_private_read_receipts(self, receipt_type: str) -> None:
         # Send a message as the first user
         res = self.helper.send(self.room_id, body="hello", tok=self.tok)
 
         # Send a private read receipt to tell the server the first user's message was read
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ_PRIVATE}/{res['event_id']}",
             {},
             access_token=self.tok2,
         )
@@ -432,10 +428,7 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
         # Test that the first user can't see the other user's private read receipt
         self.assertIsNone(self._get_read_receipt())
 
-    @parameterized.expand(
+    def test_public_receipt_can_override_private(self) -> None:
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
-    def test_public_receipt_can_override_private(self, receipt_type: str) -> None:
         """
         Sending a public read receipt to the same event which has a private read
         receipt should cause that receipt to become public.
@@ -446,7 +439,7 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
         # Send a private read receipt
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ_PRIVATE}/{res['event_id']}",
             {},
             access_token=self.tok2,
         )
@@ -465,10 +458,7 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
         # Test that we did override the private read receipt
         self.assertNotEqual(self._get_read_receipt(), None)
 
-    @parameterized.expand(
+    def test_private_receipt_cannot_override_public(self) -> None:
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
-    def test_private_receipt_cannot_override_public(self, receipt_type: str) -> None:
         """
         Sending a private read receipt to the same event which has a public read
         receipt should cause no change.
@@ -489,7 +479,7 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
         # Send a private read receipt
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ_PRIVATE}/{res['event_id']}",
             {},
             access_token=self.tok2,
         )
@@ -554,7 +544,6 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
         config = super().default_config()
         config["experimental_features"] = {
             "msc2654_enabled": True,
-            "msc2285_enabled": True,
         }
         return config
 
@@ -601,10 +590,7 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
             tok=self.tok,
         )
 
-    @parameterized.expand(
+    def test_unread_counts(self) -> None:
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
-    def test_unread_counts(self, receipt_type: str) -> None:
         """Tests that /sync returns the right value for the unread count (MSC2654)."""
 
         # Check that our own messages don't increase the unread count.
@@ -638,7 +624,7 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
         # Send a read receipt to tell the server we've read the latest event.
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ_PRIVATE}/{res['event_id']}",
             {},
             access_token=self.tok,
         )
@@ -726,7 +712,7 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
 
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res2['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ_PRIVATE}/{res2['event_id']}",
             {},
             access_token=self.tok,
         )
@@ -738,7 +724,6 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
         [
             ReceiptTypes.READ,
             ReceiptTypes.READ_PRIVATE,
-            ReceiptTypes.UNSTABLE_READ_PRIVATE,
         ]
     )
     def test_read_receipts_only_go_down(self, receipt_type: str) -> None:
@@ -752,7 +737,7 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
         # Read last event
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res2['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ_PRIVATE}/{res2['event_id']}",
             {},
             access_token=self.tok,
         )
@@ -763,7 +748,7 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
         # read receipt go up to an older event
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res1['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ_PRIVATE}/{res1['event_id']}",
             {},
             access_token=self.tok,
         )

tests/storage/test_receipts.py

@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from parameterized import parameterized
 
 from synapse.api.constants import ReceiptTypes
 from synapse.types import UserID, create_requester
@@ -92,7 +91,6 @@ class ReceiptTestCase(HomeserverTestCase):
                 [
                     ReceiptTypes.READ,
                     ReceiptTypes.READ_PRIVATE,
-                    ReceiptTypes.UNSTABLE_READ_PRIVATE,
                 ],
             )
         )
@@ -104,7 +102,6 @@ class ReceiptTestCase(HomeserverTestCase):
                 [
                     ReceiptTypes.READ,
                     ReceiptTypes.READ_PRIVATE,
-                    ReceiptTypes.UNSTABLE_READ_PRIVATE,
                 ],
             )
         )
@@ -117,16 +114,12 @@ class ReceiptTestCase(HomeserverTestCase):
                 [
                     ReceiptTypes.READ,
                     ReceiptTypes.READ_PRIVATE,
-                    ReceiptTypes.UNSTABLE_READ_PRIVATE,
                 ],
             )
         )
         self.assertEqual(res, None)
 
-    @parameterized.expand(
+    def test_get_receipts_for_user(self) -> None:
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
-    def test_get_receipts_for_user(self, receipt_type: str) -> None:
         # Send some events into the first room
         event1_1_id = self.create_and_send_event(
             self.room_id1, UserID.from_string(OTHER_USER_ID)
@@ -144,14 +137,14 @@ class ReceiptTestCase(HomeserverTestCase):
         # Send private read receipt for the second event
         self.get_success(
             self.store.insert_receipt(
-                self.room_id1, receipt_type, OUR_USER_ID, [event1_2_id], {}
+                self.room_id1, ReceiptTypes.READ_PRIVATE, OUR_USER_ID, [event1_2_id], {}
             )
         )
 
         # Test we get the latest event when we want both private and public receipts
         res = self.get_success(
             self.store.get_receipts_for_user(
-                OUR_USER_ID, [ReceiptTypes.READ, receipt_type]
+                OUR_USER_ID, [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE]
             )
         )
         self.assertEqual(res, {self.room_id1: event1_2_id})
@@ -164,7 +157,7 @@ class ReceiptTestCase(HomeserverTestCase):
 
         # Test we get the latest event when we want only the public receipt
         res = self.get_success(
-            self.store.get_receipts_for_user(OUR_USER_ID, [receipt_type])
+            self.store.get_receipts_for_user(OUR_USER_ID, [ReceiptTypes.READ_PRIVATE])
         )
         self.assertEqual(res, {self.room_id1: event1_2_id})
 
@@ -187,20 +180,17 @@ class ReceiptTestCase(HomeserverTestCase):
         # Test new room is reflected in what the method returns
         self.get_success(
             self.store.insert_receipt(
-                self.room_id2, receipt_type, OUR_USER_ID, [event2_1_id], {}
+                self.room_id2, ReceiptTypes.READ_PRIVATE, OUR_USER_ID, [event2_1_id], {}
             )
         )
         res = self.get_success(
             self.store.get_receipts_for_user(
-                OUR_USER_ID, [ReceiptTypes.READ, receipt_type]
+                OUR_USER_ID, [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE]
             )
         )
         self.assertEqual(res, {self.room_id1: event1_2_id, self.room_id2: event2_1_id})
 
-    @parameterized.expand(
+    def test_get_last_receipt_event_id_for_user(self) -> None:
-        [ReceiptTypes.READ_PRIVATE, ReceiptTypes.UNSTABLE_READ_PRIVATE]
-    )
-    def test_get_last_receipt_event_id_for_user(self, receipt_type: str) -> None:
         # Send some events into the first room
         event1_1_id = self.create_and_send_event(
             self.room_id1, UserID.from_string(OTHER_USER_ID)
@@ -218,7 +208,7 @@ class ReceiptTestCase(HomeserverTestCase):
         # Send private read receipt for the second event
         self.get_success(
             self.store.insert_receipt(
-                self.room_id1, receipt_type, OUR_USER_ID, [event1_2_id], {}
+                self.room_id1, ReceiptTypes.READ_PRIVATE, OUR_USER_ID, [event1_2_id], {}
             )
         )
 
@@ -227,7 +217,7 @@ class ReceiptTestCase(HomeserverTestCase):
             self.store.get_last_receipt_event_id_for_user(
                 OUR_USER_ID,
                 self.room_id1,
-                [ReceiptTypes.READ, receipt_type],
+                [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE],
             )
         )
         self.assertEqual(res, event1_2_id)
@@ -243,7 +233,7 @@ class ReceiptTestCase(HomeserverTestCase):
         # Test we get the latest event when we want only the private receipt
         res = self.get_success(
             self.store.get_last_receipt_event_id_for_user(
-                OUR_USER_ID, self.room_id1, [receipt_type]
+                OUR_USER_ID, self.room_id1, [ReceiptTypes.READ_PRIVATE]
             )
         )
         self.assertEqual(res, event1_2_id)
@@ -269,14 +259,14 @@ class ReceiptTestCase(HomeserverTestCase):
         # Test new room is reflected in what the method returns
         self.get_success(
             self.store.insert_receipt(
-                self.room_id2, receipt_type, OUR_USER_ID, [event2_1_id], {}
+                self.room_id2, ReceiptTypes.READ_PRIVATE, OUR_USER_ID, [event2_1_id], {}
             )
         )
         res = self.get_success(
             self.store.get_last_receipt_event_id_for_user(
                 OUR_USER_ID,
                 self.room_id2,
-                [ReceiptTypes.READ, receipt_type],
+                [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE],
             )
         )
         self.assertEqual(res, event2_1_id)