From 3102b5f1ad12fde3f14805521aa8796b6b151b60 Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Wed, 9 Mar 2022 13:40:51 +0000 Subject: [PATCH 1/8] Add combined HTTP pusher and push rule test --- tests/push/test_push_rules_http.py | 116 +++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 tests/push/test_push_rules_http.py diff --git a/tests/push/test_push_rules_http.py b/tests/push/test_push_rules_http.py new file mode 100644 index 000000000000..23313b707770 --- /dev/null +++ b/tests/push/test_push_rules_http.py @@ -0,0 +1,116 @@ +# Copyright 2018 New Vector +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from unittest.mock import Mock + +from twisted.internet.defer import Deferred + +import synapse.rest.admin +from synapse.logging.context import make_deferred_yieldable +from synapse.rest.client import login, push_rule, receipts, room + +from tests.unittest import HomeserverTestCase + + +class HTTPPusherTests(HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets_for_client_rest_resource, + room.register_servlets, + login.register_servlets, + receipts.register_servlets, + push_rule.register_servlets, + ] + user_id = True + hijack_auth = False + + def default_config(self): + config = super().default_config() + config["start_pushers"] = True + return config + + def make_homeserver(self, reactor, clock): + self.push_attempts = [] + + m = Mock() + + def post_json_get_json(url, body): + d = Deferred() + self.push_attempts.append((d, url, body)) + return make_deferred_yieldable(d) + + m.post_json_get_json = post_json_get_json + + hs = self.setup_test_homeserver(proxied_blacklisted_http_client=m) + + return hs + + def _make_user_with_pusher(self, username): + user_id = self.register_user(username, "pass") + access_token = self.login(username, "pass") + + # Register the pusher + user_tuple = self.get_success( + self.hs.get_datastores().main.get_user_by_access_token(access_token) + ) + token_id = user_tuple.token_id + + self.get_success( + self.hs.get_pusherpool().add_pusher( + user_id=user_id, + access_token=token_id, + kind="http", + app_id="m.http", + app_display_name="HTTP Push Notifications", + device_display_name="pushy push", + pushkey="a@example.com", + lang=None, + data={"url": "http://example.com/_matrix/push/v1/notify"}, + ) + ) + + return user_id, access_token + + def test_dont_notify_rule_overrides_message(self): + """ + The override push rule will suppress notification + """ + + user_id, access_token = self._make_user_with_pusher("user") + other_user_id, other_access_token = self._make_user_with_pusher("otheruser") + + # Create a room + room = self.helper.create_room_as(user_id, tok=access_token) + + # Disable user notifications for this room -> user + body = { + "conditions": [{"kind": "event_match", "key": "room_id", "pattern": room}], + "actions": ["dont_notify"], + } + channel = self.make_request( + "PUT", + "/pushrules/global/override/best.friend", + body, + access_token=access_token, + ) + self.assertEqual(channel.code, 200) + + # The other user joins + self.helper.join(room=room, user=other_user_id, tok=other_access_token) + + # The other user sends a message (ignored by dont_notify push rule set above) + self.helper.send(room, body="Hi!", tok=other_access_token) + + # The user sends a message back (sends a notification) + self.helper.send(room, body="Hello", tok=access_token) + + self.assertEqual(len(self.push_attempts), 1) From b0e4bc758ee1b99f189ecaed1230b016192ba09b Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Wed, 9 Mar 2022 13:43:08 +0000 Subject: [PATCH 2/8] Rename `_id` -> `_cache_key` in base push rules ID is ambiguous and thus confusing, this change is a much clearer indicator for what the field does. --- synapse/push/baserules.py | 38 ++++++++++++------------ synapse/push/bulk_push_rule_evaluator.py | 2 +- synapse/push/clientformat.py | 2 +- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 832eaa34e9ef..f42f605f2383 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -169,7 +169,7 @@ def make_base_prepend_rules( "kind": "event_match", "key": "content.msgtype", "pattern": "m.notice", - "_id": "_suppress_notices", + "_cache_key": "_suppress_notices", } ], "actions": ["dont_notify"], @@ -183,13 +183,13 @@ def make_base_prepend_rules( "kind": "event_match", "key": "type", "pattern": "m.room.member", - "_id": "_member", + "_cache_key": "_member", }, { "kind": "event_match", "key": "content.membership", "pattern": "invite", - "_id": "_invite_member", + "_cache_key": "_invite_member", }, {"kind": "event_match", "key": "state_key", "pattern_type": "user_id"}, ], @@ -212,7 +212,7 @@ def make_base_prepend_rules( "kind": "event_match", "key": "type", "pattern": "m.room.member", - "_id": "_member", + "_cache_key": "_member", } ], "actions": ["dont_notify"], @@ -237,12 +237,12 @@ def make_base_prepend_rules( "kind": "event_match", "key": "content.body", "pattern": "@room", - "_id": "_roomnotif_content", + "_cache_key": "_roomnotif_content", }, { "kind": "sender_notification_permission", "key": "room", - "_id": "_roomnotif_pl", + "_cache_key": "_roomnotif_pl", }, ], "actions": ["notify", {"set_tweak": "highlight", "value": True}], @@ -254,13 +254,13 @@ def make_base_prepend_rules( "kind": "event_match", "key": "type", "pattern": "m.room.tombstone", - "_id": "_tombstone", + "_cache_key": "_tombstone", }, { "kind": "event_match", "key": "state_key", "pattern": "", - "_id": "_tombstone_statekey", + "_cache_key": "_tombstone_statekey", }, ], "actions": ["notify", {"set_tweak": "highlight", "value": True}], @@ -272,7 +272,7 @@ def make_base_prepend_rules( "kind": "event_match", "key": "type", "pattern": "m.reaction", - "_id": "_reaction", + "_cache_key": "_reaction", } ], "actions": ["dont_notify"], @@ -288,7 +288,7 @@ def make_base_prepend_rules( "kind": "event_match", "key": "type", "pattern": "m.call.invite", - "_id": "_call", + "_cache_key": "_call", } ], "actions": [ @@ -302,12 +302,12 @@ def make_base_prepend_rules( { "rule_id": "global/underride/.m.rule.room_one_to_one", "conditions": [ - {"kind": "room_member_count", "is": "2", "_id": "member_count"}, + {"kind": "room_member_count", "is": "2", "_cache_key": "member_count"}, { "kind": "event_match", "key": "type", "pattern": "m.room.message", - "_id": "_message", + "_cache_key": "_message", }, ], "actions": [ @@ -321,12 +321,12 @@ def make_base_prepend_rules( { "rule_id": "global/underride/.m.rule.encrypted_room_one_to_one", "conditions": [ - {"kind": "room_member_count", "is": "2", "_id": "member_count"}, + {"kind": "room_member_count", "is": "2", "_cache_key": "member_count"}, { "kind": "event_match", "key": "type", "pattern": "m.room.encrypted", - "_id": "_encrypted", + "_cache_key": "_encrypted", }, ], "actions": [ @@ -342,7 +342,7 @@ def make_base_prepend_rules( "kind": "event_match", "key": "type", "pattern": "m.room.message", - "_id": "_message", + "_cache_key": "_message", } ], "actions": ["notify", {"set_tweak": "highlight", "value": False}], @@ -356,7 +356,7 @@ def make_base_prepend_rules( "kind": "event_match", "key": "type", "pattern": "m.room.encrypted", - "_id": "_encrypted", + "_cache_key": "_encrypted", } ], "actions": ["notify", {"set_tweak": "highlight", "value": False}], @@ -368,19 +368,19 @@ def make_base_prepend_rules( "kind": "event_match", "key": "type", "pattern": "im.vector.modular.widgets", - "_id": "_type_modular_widgets", + "_cache_key": "_type_modular_widgets", }, { "kind": "event_match", "key": "content.type", "pattern": "jitsi", - "_id": "_content_type_jitsi", + "_cache_key": "_content_type_jitsi", }, { "kind": "event_match", "key": "state_key", "pattern": "*", - "_id": "_is_state_event", + "_cache_key": "_is_state_event", }, ], "actions": ["notify", {"set_tweak": "highlight", "value": False}], diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index fecf86034eb9..9fc989c6d086 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -274,7 +274,7 @@ def _condition_checker( cache: Dict[str, bool], ) -> bool: for cond in conditions: - _id = cond.get("_id", None) + _id = cond.get("_cache_key", None) if _id: res = cache.get(_id, None) if res is False: diff --git a/synapse/push/clientformat.py b/synapse/push/clientformat.py index c5708cd8885b..63b22d50aea9 100644 --- a/synapse/push/clientformat.py +++ b/synapse/push/clientformat.py @@ -40,7 +40,7 @@ def format_push_rules_for_user( # Remove internal stuff. for c in r["conditions"]: - c.pop("_id", None) + c.pop("_cache_key", None) pattern_type = c.pop("pattern_type", None) if pattern_type == "user_id": From a4dcf105042d1e0678e96117d65031329b67c1cf Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Wed, 9 Mar 2022 14:38:15 +0000 Subject: [PATCH 3/8] Fix typing on push test modules --- tests/push/test_http.py | 5 +++-- tests/push/test_push_rules_http.py | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/push/test_http.py b/tests/push/test_http.py index c284beb37ce0..9523c7718d79 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from typing import List from unittest.mock import Mock from twisted.internet.defer import Deferred @@ -39,12 +40,12 @@ def default_config(self): return config def make_homeserver(self, reactor, clock): - self.push_attempts = [] + self.push_attempts: List[tuple[Deferred, str, dict]] = [] m = Mock() def post_json_get_json(url, body): - d = Deferred() + d: Deferred = Deferred() self.push_attempts.append((d, url, body)) return make_deferred_yieldable(d) diff --git a/tests/push/test_push_rules_http.py b/tests/push/test_push_rules_http.py index 23313b707770..4314bddcbd53 100644 --- a/tests/push/test_push_rules_http.py +++ b/tests/push/test_push_rules_http.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from typing import List from unittest.mock import Mock from twisted.internet.defer import Deferred @@ -39,12 +40,12 @@ def default_config(self): return config def make_homeserver(self, reactor, clock): - self.push_attempts = [] + self.push_attempts: List[tuple[Deferred, str, dict]] = [] m = Mock() def post_json_get_json(url, body): - d = Deferred() + d: Deferred = Deferred() self.push_attempts.append((d, url, body)) return make_deferred_yieldable(d) From 504a8ad2b167fa3a001ae4f8cb3b08976d99b350 Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Wed, 9 Mar 2022 14:51:51 +0000 Subject: [PATCH 4/8] Add changelog file --- changelog.d/12188.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12188.misc diff --git a/changelog.d/12188.misc b/changelog.d/12188.misc new file mode 100644 index 000000000000..403158481cee --- /dev/null +++ b/changelog.d/12188.misc @@ -0,0 +1 @@ +Add combined test for HTTP pusher and push rule. Contributed by Nick @ Beeper. From 26121c719c873ff74e912c69ac8edf59380a1e5b Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Fri, 11 Mar 2022 09:02:22 +0100 Subject: [PATCH 5/8] Merge new combined push rule test into existing HTTP tests --- tests/push/test_http.py | 64 +++++++++++++++- tests/push/test_push_rules_http.py | 117 ----------------------------- 2 files changed, 63 insertions(+), 118 deletions(-) delete mode 100644 tests/push/test_push_rules_http.py diff --git a/tests/push/test_http.py b/tests/push/test_http.py index 9523c7718d79..edd5cee6a9c1 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -19,7 +19,7 @@ import synapse.rest.admin from synapse.logging.context import make_deferred_yieldable from synapse.push import PusherConfigException -from synapse.rest.client import login, receipts, room +from synapse.rest.client import login, push_rule, receipts, room from tests.unittest import HomeserverTestCase, override_config @@ -30,6 +30,7 @@ class HTTPPusherTests(HomeserverTestCase): room.register_servlets, login.register_servlets, receipts.register_servlets, + push_rule.register_servlets, ] user_id = True hijack_auth = False @@ -720,3 +721,64 @@ def _send_read_request(self, access_token, message_event_id, room_id): access_token=access_token, ) self.assertEqual(channel.code, 200, channel.json_body) + + def _make_user_with_pusher(self, username): + user_id = self.register_user(username, "pass") + access_token = self.login(username, "pass") + + # Register the pusher + user_tuple = self.get_success( + self.hs.get_datastores().main.get_user_by_access_token(access_token) + ) + token_id = user_tuple.token_id + + self.get_success( + self.hs.get_pusherpool().add_pusher( + user_id=user_id, + access_token=token_id, + kind="http", + app_id="m.http", + app_display_name="HTTP Push Notifications", + device_display_name="pushy push", + pushkey="a@example.com", + lang=None, + data={"url": "http://example.com/_matrix/push/v1/notify"}, + ) + ) + + return user_id, access_token + + def test_dont_notify_rule_overrides_message(self): + """ + The override push rule will suppress notification + """ + + user_id, access_token = self._make_user_with_pusher("user") + other_user_id, other_access_token = self._make_user_with_pusher("otheruser") + + # Create a room + room = self.helper.create_room_as(user_id, tok=access_token) + + # Disable user notifications for this room -> user + body = { + "conditions": [{"kind": "event_match", "key": "room_id", "pattern": room}], + "actions": ["dont_notify"], + } + channel = self.make_request( + "PUT", + "/pushrules/global/override/best.friend", + body, + access_token=access_token, + ) + self.assertEqual(channel.code, 200) + + # The other user joins + self.helper.join(room=room, user=other_user_id, tok=other_access_token) + + # The other user sends a message (ignored by dont_notify push rule set above) + self.helper.send(room, body="Hi!", tok=other_access_token) + + # The user sends a message back (sends a notification) + self.helper.send(room, body="Hello", tok=access_token) + + self.assertEqual(len(self.push_attempts), 1) diff --git a/tests/push/test_push_rules_http.py b/tests/push/test_push_rules_http.py deleted file mode 100644 index 4314bddcbd53..000000000000 --- a/tests/push/test_push_rules_http.py +++ /dev/null @@ -1,117 +0,0 @@ -# Copyright 2018 New Vector -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -from typing import List -from unittest.mock import Mock - -from twisted.internet.defer import Deferred - -import synapse.rest.admin -from synapse.logging.context import make_deferred_yieldable -from synapse.rest.client import login, push_rule, receipts, room - -from tests.unittest import HomeserverTestCase - - -class HTTPPusherTests(HomeserverTestCase): - servlets = [ - synapse.rest.admin.register_servlets_for_client_rest_resource, - room.register_servlets, - login.register_servlets, - receipts.register_servlets, - push_rule.register_servlets, - ] - user_id = True - hijack_auth = False - - def default_config(self): - config = super().default_config() - config["start_pushers"] = True - return config - - def make_homeserver(self, reactor, clock): - self.push_attempts: List[tuple[Deferred, str, dict]] = [] - - m = Mock() - - def post_json_get_json(url, body): - d: Deferred = Deferred() - self.push_attempts.append((d, url, body)) - return make_deferred_yieldable(d) - - m.post_json_get_json = post_json_get_json - - hs = self.setup_test_homeserver(proxied_blacklisted_http_client=m) - - return hs - - def _make_user_with_pusher(self, username): - user_id = self.register_user(username, "pass") - access_token = self.login(username, "pass") - - # Register the pusher - user_tuple = self.get_success( - self.hs.get_datastores().main.get_user_by_access_token(access_token) - ) - token_id = user_tuple.token_id - - self.get_success( - self.hs.get_pusherpool().add_pusher( - user_id=user_id, - access_token=token_id, - kind="http", - app_id="m.http", - app_display_name="HTTP Push Notifications", - device_display_name="pushy push", - pushkey="a@example.com", - lang=None, - data={"url": "http://example.com/_matrix/push/v1/notify"}, - ) - ) - - return user_id, access_token - - def test_dont_notify_rule_overrides_message(self): - """ - The override push rule will suppress notification - """ - - user_id, access_token = self._make_user_with_pusher("user") - other_user_id, other_access_token = self._make_user_with_pusher("otheruser") - - # Create a room - room = self.helper.create_room_as(user_id, tok=access_token) - - # Disable user notifications for this room -> user - body = { - "conditions": [{"kind": "event_match", "key": "room_id", "pattern": room}], - "actions": ["dont_notify"], - } - channel = self.make_request( - "PUT", - "/pushrules/global/override/best.friend", - body, - access_token=access_token, - ) - self.assertEqual(channel.code, 200) - - # The other user joins - self.helper.join(room=room, user=other_user_id, tok=other_access_token) - - # The other user sends a message (ignored by dont_notify push rule set above) - self.helper.send(room, body="Hi!", tok=other_access_token) - - # The user sends a message back (sends a notification) - self.helper.send(room, body="Hello", tok=access_token) - - self.assertEqual(len(self.push_attempts), 1) From 39b8758777a6cde5d34b74072a36a7deaa59ea73 Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Fri, 11 Mar 2022 09:03:01 +0100 Subject: [PATCH 6/8] Check for no push attempts after no/initial message sends --- tests/push/test_http.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/push/test_http.py b/tests/push/test_http.py index edd5cee6a9c1..9cd037d0caa0 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -772,13 +772,16 @@ def test_dont_notify_rule_overrides_message(self): ) self.assertEqual(channel.code, 200) + # Check we start with no pushes + self.assertEqual(len(self.push_attempts), 0) + # The other user joins self.helper.join(room=room, user=other_user_id, tok=other_access_token) # The other user sends a message (ignored by dont_notify push rule set above) self.helper.send(room, body="Hi!", tok=other_access_token) + self.assertEqual(len(self.push_attempts), 0) # The user sends a message back (sends a notification) self.helper.send(room, body="Hello", tok=access_token) - self.assertEqual(len(self.push_attempts), 1) From 3e8dac0dcc25ddedcfff7c6dcdb457599d038f57 Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Fri, 11 Mar 2022 09:03:12 +0100 Subject: [PATCH 7/8] Rename variable `_id` -> `_cache_key` --- synapse/push/bulk_push_rule_evaluator.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 9fc989c6d086..8140afcb6b37 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -274,17 +274,17 @@ def _condition_checker( cache: Dict[str, bool], ) -> bool: for cond in conditions: - _id = cond.get("_cache_key", None) - if _id: - res = cache.get(_id, None) + _cache_key = cond.get("_cache_key", None) + if _cache_key: + res = cache.get(_cache_key, None) if res is False: return False elif res is True: continue res = evaluator.matches(cond, uid, display_name) - if _id: - cache[_id] = bool(res) + if _cache_key: + cache[_cache_key] = bool(res) if not res: return False From e10edf91bfc1da7b29d6cdc90d874d8fb24ab96d Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Fri, 11 Mar 2022 09:07:52 +0100 Subject: [PATCH 8/8] Add type hints to new function --- tests/push/test_http.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/push/test_http.py b/tests/push/test_http.py index 9cd037d0caa0..6691e0712896 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -from typing import List +from typing import List, Tuple from unittest.mock import Mock from twisted.internet.defer import Deferred @@ -722,7 +722,7 @@ def _send_read_request(self, access_token, message_event_id, room_id): ) self.assertEqual(channel.code, 200, channel.json_body) - def _make_user_with_pusher(self, username): + def _make_user_with_pusher(self, username: str) -> Tuple[str, str]: user_id = self.register_user(username, "pass") access_token = self.login(username, "pass")