From 9382cc5d2fb80b46867400a30d74897c0e2bde0d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 12 Jul 2022 13:02:55 -0400 Subject: [PATCH 1/8] Call the remove push actions method. --- tests/storage/test_event_push_actions.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/storage/test_event_push_actions.py b/tests/storage/test_event_push_actions.py index e8c53f16d9c5..83b9da5f45db 100644 --- a/tests/storage/test_event_push_actions.py +++ b/tests/storage/test_event_push_actions.py @@ -62,7 +62,7 @@ def test_count_aggregation(self) -> None: def _assert_counts(noitf_count: int, highlight_count: int) -> None: counts = self.get_success( self.store.db_pool.runInteraction( - "", + "get-unread-counts", self.store._get_unread_counts_by_pos_txn, room_id, user_id, @@ -167,12 +167,8 @@ def _mark_read(stream: int, depth: int) -> None: _rotate(6) _assert_counts(1, 0) - self.get_success( - self.store.db_pool.simple_delete( - table="event_push_actions", keyvalues={"1": 1}, desc="" - ) - ) - + # Delete old event push actions, this should not affect the (summarised) count. + self.get_success(self.store._remove_old_push_actions_that_have_rotated()) _assert_counts(1, 0) _mark_read(6, 6) From cbb0ed81c7a59ae8499b0ede8f79c85b63091ca9 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 12 Jul 2022 13:03:42 -0400 Subject: [PATCH 2/8] Remove unused parameter. --- tests/storage/test_event_push_actions.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/storage/test_event_push_actions.py b/tests/storage/test_event_push_actions.py index 83b9da5f45db..b53c03fa172c 100644 --- a/tests/storage/test_event_push_actions.py +++ b/tests/storage/test_event_push_actions.py @@ -132,7 +132,7 @@ def _rotate(stream: int) -> None: ) ) - def _mark_read(stream: int, depth: int) -> None: + def _mark_read(stream: int) -> None: last_read_stream_ordering[0] = stream self.get_success( @@ -157,10 +157,10 @@ def _mark_read(stream: int, depth: int) -> None: _assert_counts(2, 0) _inject_actions(5, PlAIN_NOTIF) - _mark_read(3, 3) + _mark_read(3) _assert_counts(1, 0) - _mark_read(5, 5) + _mark_read(5) _assert_counts(0, 0) _inject_actions(6, PlAIN_NOTIF) @@ -171,7 +171,7 @@ def _mark_read(stream: int, depth: int) -> None: self.get_success(self.store._remove_old_push_actions_that_have_rotated()) _assert_counts(1, 0) - _mark_read(6, 6) + _mark_read(6) _assert_counts(0, 0) _inject_actions(8, HIGHLIGHT) @@ -187,14 +187,14 @@ def _mark_read(stream: int, depth: int) -> None: # Check that sending read receipts at different points results in the # right counts. - _mark_read(8, 8) + _mark_read(8) _assert_counts(1, 0) - _mark_read(10, 10) + _mark_read(10) _assert_counts(0, 0) _inject_actions(11, HIGHLIGHT) _assert_counts(1, 1) - _mark_read(11, 11) + _mark_read(11) _assert_counts(0, 0) _rotate(11) _assert_counts(0, 0) From b526e09cd95ce4999cd2c0db9ab14f1ae6c56397 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 12 Jul 2022 13:05:28 -0400 Subject: [PATCH 3/8] Newsfragment --- changelog.d/13260.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13260.misc diff --git a/changelog.d/13260.misc b/changelog.d/13260.misc new file mode 100644 index 000000000000..b55ff32c7624 --- /dev/null +++ b/changelog.d/13260.misc @@ -0,0 +1 @@ +Clean-up tests for notifications. From 4c0d5790142c2c27ab1ea2f78d21dc6477fd87dc Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 13 Jul 2022 13:03:10 -0400 Subject: [PATCH 4/8] Create a real room with real events. --- tests/storage/test_event_push_actions.py | 176 ++++++++++------------- 1 file changed, 76 insertions(+), 100 deletions(-) diff --git a/tests/storage/test_event_push_actions.py b/tests/storage/test_event_push_actions.py index b53c03fa172c..d603d54db4d3 100644 --- a/tests/storage/test_event_push_actions.py +++ b/tests/storage/test_event_push_actions.py @@ -16,6 +16,8 @@ from twisted.test.proto_helpers import MemoryReactor +from synapse.rest import admin +from synapse.rest.client import login, room from synapse.server import HomeServer from synapse.storage.databases.main.event_push_actions import NotifCounts from synapse.util import Clock @@ -33,6 +35,12 @@ class EventPushActionsStoreTestCase(HomeserverTestCase): + servlets = [ + admin.register_servlets, + room.register_servlets, + login.register_servlets, + ] + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.store = hs.get_datastores().main persist_events_store = hs.get_datastores().persist_events @@ -54,150 +62,118 @@ def test_get_unread_push_actions_for_user_in_range_for_email(self) -> None: ) def test_count_aggregation(self) -> None: - room_id = "!foo:example.com" - user_id = "@user1235:test" + # Create a user to receive notifications and send receipts. + user_id = self.register_user("user1235", "pass") + token = self.login("user1235", "pass") + + # And another users to send events. + other_id = self.register_user("other", "pass") + other_token = self.login("other", "pass") - last_read_stream_ordering = [0] + # Create a room and put both users in it. + room_id = self.helper.create_room_as(user_id, tok=token) + self.helper.join(room_id, other_id, tok=other_token) - def _assert_counts(noitf_count: int, highlight_count: int) -> None: + last_event_id = None + + def _assert_counts( + noitf_count: int, unread_count: int, highlight_count: int + ) -> None: counts = self.get_success( self.store.db_pool.runInteraction( "get-unread-counts", - self.store._get_unread_counts_by_pos_txn, + self.store._get_unread_counts_by_receipt_txn, room_id, user_id, - last_read_stream_ordering[0], ) ) self.assertEqual( counts, NotifCounts( notify_count=noitf_count, - unread_count=0, # Unread counts are tested in the sync tests. + unread_count=unread_count, highlight_count=highlight_count, ), ) - def _inject_actions(stream: int, action: list) -> None: - event = Mock() - event.room_id = room_id - event.event_id = f"$test{stream}:example.com" - event.internal_metadata.stream_ordering = stream - event.internal_metadata.is_outlier.return_value = False - event.depth = stream - - self.store._events_stream_cache.entity_has_changed(room_id, stream) - - self.get_success( - self.store.db_pool.simple_insert( - table="events", - values={ - "stream_ordering": stream, - "topological_ordering": stream, - "type": "m.room.message", - "room_id": room_id, - "processed": True, - "outlier": False, - "event_id": event.event_id, - }, - ) - ) - - self.get_success( - self.store.add_push_actions_to_staging( - event.event_id, - {user_id: action}, - False, - ) - ) - self.get_success( - self.store.db_pool.runInteraction( - "", - self.persist_events_store._set_push_actions_for_event_and_users_txn, - [(event, None)], - [(event, None)], - ) - ) - - def _rotate(stream: int) -> None: - self.get_success( - self.store.db_pool.runInteraction( - "rotate-receipts", self.store._handle_new_receipts_for_notifs_txn - ) - ) - - self.get_success( - self.store.db_pool.runInteraction( - "rotate-notifs", self.store._rotate_notifs_before_txn, stream - ) + def _inject_actions(highlight: bool = False) -> None: + result = self.helper.send_event( + room_id, + type="m.room.message", + content={"msgtype": "m.text", "body": user_id if highlight else ""}, + tok=other_token, ) + nonlocal last_event_id + last_event_id = result["event_id"] + return last_event_id - def _mark_read(stream: int) -> None: - last_read_stream_ordering[0] = stream + def _rotate() -> None: + self.get_success(self.store._rotate_notifs()) + def _mark_read(event_id) -> None: self.get_success( self.store.insert_receipt( room_id, "m.read", user_id=user_id, - event_ids=[f"$test{stream}:example.com"], + event_ids=[event_id], data={}, ) ) - _assert_counts(0, 0) - _inject_actions(1, PlAIN_NOTIF) - _assert_counts(1, 0) - _rotate(1) - _assert_counts(1, 0) + _assert_counts(0, 0, 0) + _inject_actions() + _assert_counts(1, 0, 0) + _rotate() + _assert_counts(1, 0, 0) - _inject_actions(3, PlAIN_NOTIF) - _assert_counts(2, 0) - _rotate(3) - _assert_counts(2, 0) + event_id = _inject_actions() + _assert_counts(2, 0, 0) + _rotate() + _assert_counts(2, 0, 0) - _inject_actions(5, PlAIN_NOTIF) - _mark_read(3) - _assert_counts(1, 0) + _inject_actions() + _mark_read(event_id) + _assert_counts(1, 0, 0) - _mark_read(5) - _assert_counts(0, 0) + _mark_read(last_event_id) + _assert_counts(0, 0, 0) - _inject_actions(6, PlAIN_NOTIF) - _rotate(6) - _assert_counts(1, 0) + _inject_actions() + _rotate() + _assert_counts(1, 0, 0) # Delete old event push actions, this should not affect the (summarised) count. self.get_success(self.store._remove_old_push_actions_that_have_rotated()) - _assert_counts(1, 0) + _assert_counts(1, 0, 0) - _mark_read(6) - _assert_counts(0, 0) + _mark_read(last_event_id) + _assert_counts(0, 0, 0) - _inject_actions(8, HIGHLIGHT) - _assert_counts(1, 1) - _rotate(8) - _assert_counts(1, 1) + event_id = _inject_actions(True) + _assert_counts(1, 1, 1) + _rotate() + _assert_counts(1, 1, 1) # Check that adding another notification and rotating after highlight # works. - _inject_actions(10, PlAIN_NOTIF) - _rotate(10) - _assert_counts(2, 1) + _inject_actions() + _rotate() + _assert_counts(2, 0, 1) # Check that sending read receipts at different points results in the # right counts. - _mark_read(8) - _assert_counts(1, 0) - _mark_read(10) - _assert_counts(0, 0) - - _inject_actions(11, HIGHLIGHT) - _assert_counts(1, 1) - _mark_read(11) - _assert_counts(0, 0) - _rotate(11) - _assert_counts(0, 0) + _mark_read(event_id) + _assert_counts(1, 0, 0) + _mark_read(last_event_id) + _assert_counts(0, 0, 0) + + _inject_actions(True) + _assert_counts(1, 1, 1) + _mark_read(last_event_id) + _assert_counts(0, 0, 0) + _rotate() + _assert_counts(0, 0, 0) def test_find_first_stream_ordering_after_ts(self) -> None: def add_event(so: int, ts: int) -> None: From 1f7c2c90b33dca2ff161822822fa73906df08e1c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 13 Jul 2022 13:05:53 -0400 Subject: [PATCH 5/8] Lint --- tests/storage/test_event_push_actions.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/storage/test_event_push_actions.py b/tests/storage/test_event_push_actions.py index d603d54db4d3..ee6fe9c4a1cf 100644 --- a/tests/storage/test_event_push_actions.py +++ b/tests/storage/test_event_push_actions.py @@ -12,8 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from unittest.mock import Mock - from twisted.test.proto_helpers import MemoryReactor from synapse.rest import admin @@ -74,7 +72,7 @@ def test_count_aggregation(self) -> None: room_id = self.helper.create_room_as(user_id, tok=token) self.helper.join(room_id, other_id, tok=other_token) - last_event_id = None + last_event_id: str def _assert_counts( noitf_count: int, unread_count: int, highlight_count: int @@ -96,7 +94,7 @@ def _assert_counts( ), ) - def _inject_actions(highlight: bool = False) -> None: + def _inject_actions(highlight: bool = False) -> str: result = self.helper.send_event( room_id, type="m.room.message", @@ -110,7 +108,7 @@ def _inject_actions(highlight: bool = False) -> None: def _rotate() -> None: self.get_success(self.store._rotate_notifs()) - def _mark_read(event_id) -> None: + def _mark_read(event_id: str) -> None: self.get_success( self.store.insert_receipt( room_id, From fbba68ae3d3c5da9bcc1b5f0a60d2c0c8da7981b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 13 Jul 2022 13:30:41 -0400 Subject: [PATCH 6/8] Remove unused constants. --- tests/storage/test_event_push_actions.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/storage/test_event_push_actions.py b/tests/storage/test_event_push_actions.py index ee6fe9c4a1cf..9226c3fed056 100644 --- a/tests/storage/test_event_push_actions.py +++ b/tests/storage/test_event_push_actions.py @@ -24,13 +24,6 @@ USER_ID = "@user:example.com" -PlAIN_NOTIF = ["notify", {"set_tweak": "highlight", "value": False}] -HIGHLIGHT = [ - "notify", - {"set_tweak": "sound", "value": "default"}, - {"set_tweak": "highlight"}, -] - class EventPushActionsStoreTestCase(HomeserverTestCase): servlets = [ From 45284807d392b62d8a6e6d7dcfbed5e481de107b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 13 Jul 2022 15:00:16 -0400 Subject: [PATCH 7/8] Rename a function. --- tests/storage/test_event_push_actions.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/storage/test_event_push_actions.py b/tests/storage/test_event_push_actions.py index 9226c3fed056..b8edd06368f9 100644 --- a/tests/storage/test_event_push_actions.py +++ b/tests/storage/test_event_push_actions.py @@ -87,7 +87,7 @@ def _assert_counts( ), ) - def _inject_actions(highlight: bool = False) -> str: + def _create_event(highlight: bool = False) -> str: result = self.helper.send_event( room_id, type="m.room.message", @@ -113,24 +113,24 @@ def _mark_read(event_id: str) -> None: ) _assert_counts(0, 0, 0) - _inject_actions() + _create_event() _assert_counts(1, 0, 0) _rotate() _assert_counts(1, 0, 0) - event_id = _inject_actions() + event_id = _create_event() _assert_counts(2, 0, 0) _rotate() _assert_counts(2, 0, 0) - _inject_actions() + _create_event() _mark_read(event_id) _assert_counts(1, 0, 0) _mark_read(last_event_id) _assert_counts(0, 0, 0) - _inject_actions() + _create_event() _rotate() _assert_counts(1, 0, 0) @@ -141,14 +141,14 @@ def _mark_read(event_id: str) -> None: _mark_read(last_event_id) _assert_counts(0, 0, 0) - event_id = _inject_actions(True) + event_id = _create_event(True) _assert_counts(1, 1, 1) _rotate() _assert_counts(1, 1, 1) # Check that adding another notification and rotating after highlight # works. - _inject_actions() + _create_event() _rotate() _assert_counts(2, 0, 1) @@ -159,7 +159,7 @@ def _mark_read(event_id: str) -> None: _mark_read(last_event_id) _assert_counts(0, 0, 0) - _inject_actions(True) + _create_event(True) _assert_counts(1, 1, 1) _mark_read(last_event_id) _assert_counts(0, 0, 0) From ceecc10443d20690c46abe4335c5f1b792ec57f8 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 15 Jul 2022 07:43:59 -0400 Subject: [PATCH 8/8] Fix unread count in tests. --- tests/storage/test_event_push_actions.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/storage/test_event_push_actions.py b/tests/storage/test_event_push_actions.py index b8edd06368f9..ba40124c8a00 100644 --- a/tests/storage/test_event_push_actions.py +++ b/tests/storage/test_event_push_actions.py @@ -91,7 +91,7 @@ def _create_event(highlight: bool = False) -> str: result = self.helper.send_event( room_id, type="m.room.message", - content={"msgtype": "m.text", "body": user_id if highlight else ""}, + content={"msgtype": "m.text", "body": user_id if highlight else "msg"}, tok=other_token, ) nonlocal last_event_id @@ -114,29 +114,29 @@ def _mark_read(event_id: str) -> None: _assert_counts(0, 0, 0) _create_event() - _assert_counts(1, 0, 0) + _assert_counts(1, 1, 0) _rotate() - _assert_counts(1, 0, 0) + _assert_counts(1, 1, 0) event_id = _create_event() - _assert_counts(2, 0, 0) + _assert_counts(2, 2, 0) _rotate() - _assert_counts(2, 0, 0) + _assert_counts(2, 2, 0) _create_event() _mark_read(event_id) - _assert_counts(1, 0, 0) + _assert_counts(1, 1, 0) _mark_read(last_event_id) _assert_counts(0, 0, 0) _create_event() _rotate() - _assert_counts(1, 0, 0) + _assert_counts(1, 1, 0) # Delete old event push actions, this should not affect the (summarised) count. self.get_success(self.store._remove_old_push_actions_that_have_rotated()) - _assert_counts(1, 0, 0) + _assert_counts(1, 1, 0) _mark_read(last_event_id) _assert_counts(0, 0, 0) @@ -150,12 +150,12 @@ def _mark_read(event_id: str) -> None: # works. _create_event() _rotate() - _assert_counts(2, 0, 1) + _assert_counts(2, 2, 1) # Check that sending read receipts at different points results in the # right counts. _mark_read(event_id) - _assert_counts(1, 0, 0) + _assert_counts(1, 1, 0) _mark_read(last_event_id) _assert_counts(0, 0, 0)