Support stable identifiers for MSC2285: private read receipts. (#13273)

This adds support for the stable identifiers of MSC2285 while
continuing to support the unstable identifiers behind the configuration
flag. These will be removed in a future version.

3 files changed, 119 insertions, 52 deletions

diff --git a/tests/handlers/test_receipts.py b/tests/handlers/test_receipts.py
index a95868b5c0..5f70a2db79 100644
--- a/tests/handlers/test_receipts.py
+++ b/tests/handlers/test_receipts.py
@@ -15,6 +15,8 @@
 from copy import deepcopy
 from typing import List
 
+from parameterized import parameterized
+
 from synapse.api.constants import EduTypes, ReceiptTypes
 from synapse.types import JsonDict
 
@@ -25,13 +27,16 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
     def prepare(self, reactor, clock, hs):
         self.event_source = hs.get_event_sources().sources.receipt
 
-    def test_filters_out_private_receipt(self):
+    @parameterized.expand(
+        [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": {
-                            ReceiptTypes.READ_PRIVATE: {
+                            receipt_type: {
                                 "@rikj:jki.re": {
                                     "ts": 1436451550453,
                                 }
@@ -45,13 +50,18 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             [],
         )
 
-    def test_filters_out_private_receipt_and_ignores_rest(self):
+    @parameterized.expand(
+        [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": {
-                            ReceiptTypes.READ_PRIVATE: {
+                            receipt_type: {
                                 "@rikj:jki.re": {
                                     "ts": 1436451550453,
                                 },
@@ -84,13 +94,18 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-    def test_filters_out_event_with_only_private_receipts_and_ignores_the_rest(self):
+    @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
+    ) -> None:
         self._test_filters_private(
             [
                 {
                     "content": {
                         "$14356419edgd14394fHBLK:matrix.org": {
-                            ReceiptTypes.READ_PRIVATE: {
+                            receipt_type: {
                                 "@rikj:jki.re": {
                                     "ts": 1436451550453,
                                 },
@@ -125,7 +140,7 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-    def test_handles_empty_event(self):
+    def test_handles_empty_event(self) -> None:
         self._test_filters_private(
             [
                 {
@@ -160,13 +175,18 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-    def test_filters_out_receipt_event_with_only_private_receipt_and_ignores_rest(self):
+    @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
+    ) -> None:
         self._test_filters_private(
             [
                 {
                     "content": {
                         "$14356419edgd14394fHBLK:matrix.org": {
-                            ReceiptTypes.READ_PRIVATE: {
+                            receipt_type: {
                                 "@rikj:jki.re": {
                                     "ts": 1436451550453,
                                 },
@@ -207,7 +227,7 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-    def test_handles_string_data(self):
+    def test_handles_string_data(self) -> None:
         """
         Tests that an invalid shape for read-receipts is handled.
         Context: https://github.com/matrix-org/synapse/issues/10603
@@ -242,13 +262,16 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-    def test_leaves_our_private_and_their_public(self):
+    @parameterized.expand(
+        [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": {
-                            ReceiptTypes.READ_PRIVATE: {
+                            receipt_type: {
                                 "@me:server.org": {
                                     "ts": 1436451550453,
                                 },
@@ -273,7 +296,7 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
                 {
                     "content": {
                         "$1dgdgrd5641916114394fHBLK:matrix.org": {
-                            ReceiptTypes.READ_PRIVATE: {
+                            receipt_type: {
                                 "@me:server.org": {
                                     "ts": 1436451550453,
                                 },
@@ -296,13 +319,16 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
             ],
         )
 
-    def test_we_do_not_mutate(self):
+    @parameterized.expand(
+        [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": {
-                        ReceiptTypes.READ_PRIVATE: {
+                        receipt_type: {
                             "@rikj:jki.re": {
                                 "ts": 1436451550453,
                             }
@@ -320,7 +346,7 @@ class ReceiptsTestCase(unittest.HomeserverTestCase):
 
     def _test_filters_private(
         self, events: List[JsonDict], expected_output: List[JsonDict]
-    ):
+    ) -> None:
         """Tests that the _filter_out_private returns the expected output"""
         filtered_events = self.event_source.filter_out_private_receipts(
             events, "@me:server.org"
diff --git a/tests/rest/client/test_sync.py b/tests/rest/client/test_sync.py
index ae16184828..de0dec8539 100644
--- a/tests/rest/client/test_sync.py
+++ b/tests/rest/client/test_sync.py
@@ -38,7 +38,6 @@ from tests.federation.transport.test_knocking import (
     KnockingStrippedStateEventHelperMixin,
 )
 from tests.server import TimedOutException
-from tests.unittest import override_config
 
 
 class FilterTestCase(unittest.HomeserverTestCase):
@@ -390,6 +389,12 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
         sync.register_servlets,
     ]
 
+    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)
+
     def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         self.url = "/sync?since=%s"
         self.next_batch = "s0"
@@ -408,15 +413,17 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
         # Join the second user
         self.helper.join(room=self.room_id, user=self.user2, tok=self.tok2)
 
-    @override_config({"experimental_features": {"msc2285_enabled": True}})
-    def test_private_read_receipts(self) -> None:
+    @parameterized.expand(
+        [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/org.matrix.msc2285.read.private/{res['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res['event_id']}",
             {},
             access_token=self.tok2,
         )
@@ -425,8 +432,10 @@ 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())
 
-    @override_config({"experimental_features": {"msc2285_enabled": True}})
-    def test_public_receipt_can_override_private(self) -> None:
+    @parameterized.expand(
+        [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.
@@ -437,7 +446,7 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
         # Send a private read receipt
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ_PRIVATE}/{res['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res['event_id']}",
             {},
             access_token=self.tok2,
         )
@@ -456,8 +465,10 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
         # Test that we did override the private read receipt
         self.assertNotEqual(self._get_read_receipt(), None)
 
-    @override_config({"experimental_features": {"msc2285_enabled": True}})
-    def test_private_receipt_cannot_override_public(self) -> None:
+    @parameterized.expand(
+        [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.
@@ -478,7 +489,7 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
         # Send a private read receipt
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/{ReceiptTypes.READ_PRIVATE}/{res['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res['event_id']}",
             {},
             access_token=self.tok2,
         )
@@ -590,7 +601,10 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
             tok=self.tok,
         )
 
-    def test_unread_counts(self) -> None:
+    @parameterized.expand(
+        [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.
@@ -624,7 +638,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/org.matrix.msc2285.read.private/{res['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res['event_id']}",
             {},
             access_token=self.tok,
         )
@@ -700,7 +714,7 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
         self._check_unread_count(5)
         res2 = self.helper.send(self.room_id, "hello", tok=self.tok2)
 
-        # Make sure both m.read and org.matrix.msc2285.read.private advance
+        # Make sure both m.read and m.read.private advance
         channel = self.make_request(
             "POST",
             f"/rooms/{self.room_id}/receipt/m.read/{res1['event_id']}",
@@ -712,16 +726,22 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
 
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/org.matrix.msc2285.read.private/{res2['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res2['event_id']}",
             {},
             access_token=self.tok,
         )
         self.assertEqual(channel.code, 200, channel.json_body)
         self._check_unread_count(0)
 
-    # We test for both receipt types that influence notification counts
-    @parameterized.expand([ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE])
-    def test_read_receipts_only_go_down(self, receipt_type: ReceiptTypes) -> None:
+    # We test for all three receipt types that influence notification counts
+    @parameterized.expand(
+        [
+            ReceiptTypes.READ,
+            ReceiptTypes.READ_PRIVATE,
+            ReceiptTypes.UNSTABLE_READ_PRIVATE,
+        ]
+    )
+    def test_read_receipts_only_go_down(self, receipt_type: str) -> None:
         # Join the new user
         self.helper.join(room=self.room_id, user=self.user2, tok=self.tok2)
 
@@ -739,11 +759,11 @@ class UnreadMessagesTestCase(unittest.HomeserverTestCase):
         self.assertEqual(channel.code, 200, channel.json_body)
         self._check_unread_count(0)
 
-        # Make sure neither m.read nor org.matrix.msc2285.read.private make the
+        # Make sure neither m.read nor m.read.private make the
         # read receipt go up to an older event
         channel = self.make_request(
             "POST",
-            f"/rooms/{self.room_id}/receipt/org.matrix.msc2285.read.private/{res1['event_id']}",
+            f"/rooms/{self.room_id}/receipt/{receipt_type}/{res1['event_id']}",
             {},
             access_token=self.tok,
         )
diff --git a/tests/storage/test_receipts.py b/tests/storage/test_receipts.py
index b1a8f8bba7..191c957fb5 100644
--- a/tests/storage/test_receipts.py
+++ b/tests/storage/test_receipts.py
@@ -12,6 +12,8 @@
 # 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
 
@@ -23,7 +25,7 @@ OUR_USER_ID = "@our:test"
 
 
 class ReceiptTestCase(HomeserverTestCase):
-    def prepare(self, reactor, clock, homeserver):
+    def prepare(self, reactor, clock, homeserver) -> None:
         super().prepare(reactor, clock, homeserver)
 
         self.store = homeserver.get_datastores().main
@@ -83,10 +85,15 @@ class ReceiptTestCase(HomeserverTestCase):
             )
         )
 
-    def test_return_empty_with_no_data(self):
+    def test_return_empty_with_no_data(self) -> None:
         res = self.get_success(
             self.store.get_receipts_for_user(
-                OUR_USER_ID, [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE]
+                OUR_USER_ID,
+                [
+                    ReceiptTypes.READ,
+                    ReceiptTypes.READ_PRIVATE,
+                    ReceiptTypes.UNSTABLE_READ_PRIVATE,
+                ],
             )
         )
         self.assertEqual(res, {})
@@ -94,7 +101,11 @@ class ReceiptTestCase(HomeserverTestCase):
         res = self.get_success(
             self.store.get_receipts_for_user_with_orderings(
                 OUR_USER_ID,
-                [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE],
+                [
+                    ReceiptTypes.READ,
+                    ReceiptTypes.READ_PRIVATE,
+                    ReceiptTypes.UNSTABLE_READ_PRIVATE,
+                ],
             )
         )
         self.assertEqual(res, {})
@@ -103,12 +114,19 @@ class ReceiptTestCase(HomeserverTestCase):
             self.store.get_last_receipt_event_id_for_user(
                 OUR_USER_ID,
                 self.room_id1,
-                [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE],
+                [
+                    ReceiptTypes.READ,
+                    ReceiptTypes.READ_PRIVATE,
+                    ReceiptTypes.UNSTABLE_READ_PRIVATE,
+                ],
             )
         )
         self.assertEqual(res, None)
 
-    def test_get_receipts_for_user(self):
+    @parameterized.expand(
+        [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)
@@ -126,14 +144,14 @@ class ReceiptTestCase(HomeserverTestCase):
         # Send private read receipt for the second event
         self.get_success(
             self.store.insert_receipt(
-                self.room_id1, ReceiptTypes.READ_PRIVATE, OUR_USER_ID, [event1_2_id], {}
+                self.room_id1, receipt_type, 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, ReceiptTypes.READ_PRIVATE]
+                OUR_USER_ID, [ReceiptTypes.READ, receipt_type]
             )
         )
         self.assertEqual(res, {self.room_id1: event1_2_id})
@@ -146,7 +164,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, [ReceiptTypes.READ_PRIVATE])
+            self.store.get_receipts_for_user(OUR_USER_ID, [receipt_type])
         )
         self.assertEqual(res, {self.room_id1: event1_2_id})
 
@@ -169,17 +187,20 @@ class ReceiptTestCase(HomeserverTestCase):
         # Test new room is reflected in what the method returns
         self.get_success(
             self.store.insert_receipt(
-                self.room_id2, ReceiptTypes.READ_PRIVATE, OUR_USER_ID, [event2_1_id], {}
+                self.room_id2, receipt_type, OUR_USER_ID, [event2_1_id], {}
             )
         )
         res = self.get_success(
             self.store.get_receipts_for_user(
-                OUR_USER_ID, [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE]
+                OUR_USER_ID, [ReceiptTypes.READ, receipt_type]
             )
         )
         self.assertEqual(res, {self.room_id1: event1_2_id, self.room_id2: event2_1_id})
 
-    def test_get_last_receipt_event_id_for_user(self):
+    @parameterized.expand(
+        [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)
@@ -197,7 +218,7 @@ class ReceiptTestCase(HomeserverTestCase):
         # Send private read receipt for the second event
         self.get_success(
             self.store.insert_receipt(
-                self.room_id1, ReceiptTypes.READ_PRIVATE, OUR_USER_ID, [event1_2_id], {}
+                self.room_id1, receipt_type, OUR_USER_ID, [event1_2_id], {}
             )
         )
 
@@ -206,7 +227,7 @@ class ReceiptTestCase(HomeserverTestCase):
             self.store.get_last_receipt_event_id_for_user(
                 OUR_USER_ID,
                 self.room_id1,
-                [ReceiptTypes.READ, ReceiptTypes.READ_PRIVATE],
+                [ReceiptTypes.READ, receipt_type],
             )
         )
         self.assertEqual(res, event1_2_id)
@@ -222,7 +243,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, [ReceiptTypes.READ_PRIVATE]
+                OUR_USER_ID, self.room_id1, [receipt_type]
             )
         )
         self.assertEqual(res, event1_2_id)
@@ -248,14 +269,14 @@ class ReceiptTestCase(HomeserverTestCase):
         # Test new room is reflected in what the method returns
         self.get_success(
             self.store.insert_receipt(
-                self.room_id2, ReceiptTypes.READ_PRIVATE, OUR_USER_ID, [event2_1_id], {}
+                self.room_id2, receipt_type, 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, ReceiptTypes.READ_PRIVATE],
+                [ReceiptTypes.READ, receipt_type],
             )
         )
         self.assertEqual(res, event2_1_id)