From 00de6da968cae8b9aa49188223e8b75d271a8402 Mon Sep 17 00:00:00 2001 From: Mo Balaa Date: Wed, 20 Dec 2023 16:53:32 -0600 Subject: [PATCH 1/8] Handle wildcard type filters properly --- synapse/storage/databases/main/stream.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/synapse/storage/databases/main/stream.py b/synapse/storage/databases/main/stream.py index 1b2a65bed2..3928771350 100644 --- a/synapse/storage/databases/main/stream.py +++ b/synapse/storage/databases/main/stream.py @@ -398,14 +398,25 @@ def filter_to_clause(event_filter: Optional[Filter]) -> Tuple[str, List[str]]: clauses = [] args = [] + # Handle event types with potential wildcard characters if event_filter.types: - clauses.append( - "(%s)" % " OR ".join("event.type = ?" for _ in event_filter.types) - ) - args.extend(event_filter.types) + type_clauses = [] + for typ in event_filter.types: + if '*' in typ: + type_clauses.append("event.type LIKE ?") + typ = typ.replace('*', '%') # Replace * with % for SQL LIKE pattern + else: + type_clauses.append("event.type = ?") + args.append(typ) + clauses.append("(%s)" % " OR ".join(type_clauses)) + # Handle event types to exclude with potential wildcard characters for typ in event_filter.not_types: - clauses.append("event.type != ?") + if '*' in typ: + clauses.append("event.type NOT LIKE ?") + typ = typ.replace('*', '%') + else: + clauses.append("event.type != ?") args.append(typ) if event_filter.senders: From fbab81e7a4aefe3255f3844ecf9e78e23e099ff0 Mon Sep 17 00:00:00 2001 From: Mo Balaa Date: Thu, 4 Jan 2024 10:55:37 -0600 Subject: [PATCH 2/8] add regression test for wildcard filters --- tests/rest/client/test_rooms.py | 26 ++++++++++++++++++++++++++ tests/rest/client/utils.py | 3 ++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 0e71cdcd88..bc4f1e7823 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2154,6 +2154,32 @@ def test_room_messages_purge(self) -> None: chunk = channel.json_body["chunk"] self.assertEqual(len(chunk), 0, [event["content"] for event in chunk]) + def test_room_message_filter_wildcard(self) -> None: + store = self.hs.get_datastores().main + # Send a first message in the room, which will be removed by the purge. + self.helper.send(self.room_id, "message 1", type="f.message.1") + self.helper.send(self.room_id, "message 1", type="f.message.2") + + first_event_id = self.helper.send(self.room_id, "message 1")["event_id"] + first_token = self.get_success( + store.get_topological_token_for_event(first_event_id) + ) + first_token_str = self.get_success(first_token.to_string(store)) + + channel = self.make_request( + "GET", + "/rooms/%s/messages?access_token=x&from=%s&dir=b&filter=%s" + % ( + self.room_id, + first_token_str, + json.dumps({"types": ['f.message.*']}), + ), + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) + + chunk = channel.json_body["chunk"] + self.assertEqual(len(chunk), 2, [event["content"] for event in chunk]) + class RoomSearchTestCase(unittest.HomeserverTestCase): servlets = [ diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py index 7bcbed246b..3bf3663e3a 100644 --- a/tests/rest/client/utils.py +++ b/tests/rest/client/utils.py @@ -364,6 +364,7 @@ def send( tok: Optional[str] = None, expect_code: int = HTTPStatus.OK, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, + type: str = "m.room.message", ) -> JsonDict: if body is None: body = "body_text_here" @@ -372,7 +373,7 @@ def send( return self.send_event( room_id, - "m.room.message", + type, content, txn_id, tok, From 13c2348c2b73114b6e8c9552deaf65418f0a0e48 Mon Sep 17 00:00:00 2001 From: Mo Balaa Date: Thu, 4 Jan 2024 11:23:16 -0600 Subject: [PATCH 3/8] Run linters --- synapse/storage/databases/main/stream.py | 8 ++++---- tests/rest/client/test_rooms.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/storage/databases/main/stream.py b/synapse/storage/databases/main/stream.py index 3928771350..7be4e98fb4 100644 --- a/synapse/storage/databases/main/stream.py +++ b/synapse/storage/databases/main/stream.py @@ -402,9 +402,9 @@ def filter_to_clause(event_filter: Optional[Filter]) -> Tuple[str, List[str]]: if event_filter.types: type_clauses = [] for typ in event_filter.types: - if '*' in typ: + if "*" in typ: type_clauses.append("event.type LIKE ?") - typ = typ.replace('*', '%') # Replace * with % for SQL LIKE pattern + typ = typ.replace("*", "%") # Replace * with % for SQL LIKE pattern else: type_clauses.append("event.type = ?") args.append(typ) @@ -412,9 +412,9 @@ def filter_to_clause(event_filter: Optional[Filter]) -> Tuple[str, List[str]]: # Handle event types to exclude with potential wildcard characters for typ in event_filter.not_types: - if '*' in typ: + if "*" in typ: clauses.append("event.type NOT LIKE ?") - typ = typ.replace('*', '%') + typ = typ.replace("*", "%") else: clauses.append("event.type != ?") args.append(typ) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index bc4f1e7823..a24edd4177 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2172,7 +2172,7 @@ def test_room_message_filter_wildcard(self) -> None: % ( self.room_id, first_token_str, - json.dumps({"types": ['f.message.*']}), + json.dumps({"types": ["f.message.*"]}), ), ) self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body) From 87951ebfad792819d479e6a60ad0888a0e2857b0 Mon Sep 17 00:00:00 2001 From: Mo Balaa Date: Thu, 4 Jan 2024 11:27:00 -0600 Subject: [PATCH 4/8] Add changelog --- changelog.d/14984.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/14984.bugfix diff --git a/changelog.d/14984.bugfix b/changelog.d/14984.bugfix new file mode 100644 index 0000000000..b694f6d167 --- /dev/null +++ b/changelog.d/14984.bugfix @@ -0,0 +1 @@ +Handle wildcard type filters properly for room messages endpoint. Contributed by Mo Balaa. From f4a377247184e60be8c7b49bd50e0bbecfc0f478 Mon Sep 17 00:00:00 2001 From: Mo Balaa Date: Thu, 4 Jan 2024 11:37:38 -0600 Subject: [PATCH 5/8] cleanup test --- tests/rest/client/test_rooms.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index a24edd4177..8116402324 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2159,19 +2159,12 @@ def test_room_message_filter_wildcard(self) -> None: # Send a first message in the room, which will be removed by the purge. self.helper.send(self.room_id, "message 1", type="f.message.1") self.helper.send(self.room_id, "message 1", type="f.message.2") - - first_event_id = self.helper.send(self.room_id, "message 1")["event_id"] - first_token = self.get_success( - store.get_topological_token_for_event(first_event_id) - ) - first_token_str = self.get_success(first_token.to_string(store)) - + self.helper.send(self.room_id, "not returned in filter") channel = self.make_request( "GET", - "/rooms/%s/messages?access_token=x&from=%s&dir=b&filter=%s" + "/rooms/%s/messages?access_token=x&dir=b&filter=%s" % ( self.room_id, - first_token_str, json.dumps({"types": ["f.message.*"]}), ), ) From 6df14af4e8f1cd7e13192b5d4593096da1c77dad Mon Sep 17 00:00:00 2001 From: Mo Balaa Date: Tue, 16 Jan 2024 11:36:37 -0600 Subject: [PATCH 6/8] Update test_rooms.py --- tests/rest/client/test_rooms.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 8116402324..32efa2fce2 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2155,7 +2155,6 @@ def test_room_messages_purge(self) -> None: self.assertEqual(len(chunk), 0, [event["content"] for event in chunk]) def test_room_message_filter_wildcard(self) -> None: - store = self.hs.get_datastores().main # Send a first message in the room, which will be removed by the purge. self.helper.send(self.room_id, "message 1", type="f.message.1") self.helper.send(self.room_id, "message 1", type="f.message.2") From 2ae62c0088909560557ea6f1a9cb5c2442e64ad3 Mon Sep 17 00:00:00 2001 From: Mo Balaa Date: Tue, 16 Jan 2024 13:53:23 -0600 Subject: [PATCH 7/8] Update test_rooms.py --- tests/rest/client/test_rooms.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 32efa2fce2..55826c08ed 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2154,6 +2154,15 @@ def test_room_messages_purge(self) -> None: chunk = channel.json_body["chunk"] self.assertEqual(len(chunk), 0, [event["content"] for event in chunk]) + +class RoomMessageFilterTestCase(RoomBase): + """Tests /rooms/$room_id/messages REST events.""" + + user_id = "@sid1:red" + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.room_id = self.helper.create_room_as(self.user_id) + def test_room_message_filter_wildcard(self) -> None: # Send a first message in the room, which will be removed by the purge. self.helper.send(self.room_id, "message 1", type="f.message.1") From a444ddb02f04fdaaf0871f59461a1e7d0ed1493c Mon Sep 17 00:00:00 2001 From: Mo Balaa Date: Tue, 16 Jan 2024 13:58:29 -0600 Subject: [PATCH 8/8] Update test_rooms.py --- tests/rest/client/test_rooms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 55826c08ed..89b161dd0a 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2162,7 +2162,7 @@ class RoomMessageFilterTestCase(RoomBase): def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.room_id = self.helper.create_room_as(self.user_id) - + def test_room_message_filter_wildcard(self) -> None: # Send a first message in the room, which will be removed by the purge. self.helper.send(self.room_id, "message 1", type="f.message.1")