From 8ea0c5bbad824f7f320a20195c161dd1fdbbb975 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Wed, 28 Jun 2023 16:19:45 +0000 Subject: [PATCH] narrow_filter: Pass message/flags to narrow_filter. We no longer pass in a big opaque event to narrow_filter (which is inside build_narrow_filter). We instead explicitly pass in message and flags. This leads to a bit more type safety, and it's also more flexible. There's no reason to build an entire event just to see if a message belongs to a narrow. The changes to the test work around the fact that the fixtures are sloppy with types. I plan a subsequent commit to clean up those tests significantly. --- zerver/lib/narrow.py | 10 +++++----- zerver/tests/test_message_fetch.py | 16 ++++++++++++++-- zerver/tornado/event_queue.py | 2 +- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/zerver/lib/narrow.py b/zerver/lib/narrow.py index 10e58d68c4..a5962aca0b 100644 --- a/zerver/lib/narrow.py +++ b/zerver/lib/narrow.py @@ -9,7 +9,6 @@ from typing import ( Generic, Iterable, List, - Mapping, Optional, Sequence, Set, @@ -23,6 +22,7 @@ from django.conf import settings from django.core.exceptions import ValidationError from django.db import connection from django.utils.translation import gettext as _ +from mypy_extensions import NamedArg from sqlalchemy.dialects import postgresql from sqlalchemy.engine import Connection, Row from sqlalchemy.sql import ( @@ -132,14 +132,14 @@ def is_web_public_narrow(narrow: Optional[Iterable[Dict[str, Any]]]) -> bool: ) -def build_narrow_filter(narrow: Collection[Sequence[str]]) -> Callable[[Mapping[str, Any]], bool]: +def build_narrow_filter( + narrow: Collection[Sequence[str]], +) -> Callable[[NamedArg(Dict[str, Any], "message"), NamedArg(List[str], "flags")], bool]: """Changes to this function should come with corresponding changes to NarrowLibraryTest.""" check_supported_events_narrow_filter(narrow) - def narrow_filter(event: Mapping[str, Any]) -> bool: - message = event["message"] - flags = event["flags"] + def narrow_filter(*, message: Dict[str, Any], flags: List[str]) -> bool: for element in narrow: operator = element[0] operand = element[1] diff --git a/zerver/tests/test_message_fetch.py b/zerver/tests/test_message_fetch.py index 9135264ddc..b1a381bba2 100644 --- a/zerver/tests/test_message_fetch.py +++ b/zerver/tests/test_message_fetch.py @@ -603,9 +603,21 @@ class NarrowLibraryTest(ZulipTestCase): reject_events = scenario["reject_events"] narrow_filter = build_narrow_filter(narrow) for e in accept_events: - self.assertTrue(narrow_filter(e)) + message = e["message"] + flags = e["flags"] + if message is None: + message = {} + if flags is None: + flags = [] + self.assertTrue(narrow_filter(message=message, flags=flags)) for e in reject_events: - self.assertFalse(narrow_filter(e)) + message = e["message"] + flags = e["flags"] + if message is None: + message = {} + if flags is None: + flags = [] + self.assertFalse(narrow_filter(message=message, flags=flags)) def test_build_narrow_filter_invalid(self) -> None: with self.assertRaises(JsonableError): diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py index 6823844dd2..3fdb90ad5f 100644 --- a/zerver/tornado/event_queue.py +++ b/zerver/tornado/event_queue.py @@ -226,7 +226,7 @@ class ClientDescriptor: # older servers that don't support user_topic. return False if event["type"] == "message": - return self.narrow_filter(event) + return self.narrow_filter(message=event["message"], flags=event["flags"]) if event["type"] == "typing" and "stream_id" in event: # Typing notifications for stream messages are only # delivered if the stream_typing_notifications