From f121e40848a18c67c38985d1f803d53e9ccd2c80 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Thu, 18 Mar 2021 14:33:52 -0700 Subject: [PATCH] message: Record whether unread_msgs data is truncated. This is preparatory work for investigating reports of missing unread messages. It's a little surprising that not test failed after adding the code without API documentation. Co-Author-By: Tushar Upadhyay (tushar912). --- templates/zerver/api/changelog.md | 7 +++++++ version.py | 2 +- zerver/lib/message.py | 11 +++++++++++ zerver/openapi/zulip.yaml | 12 ++++++++++++ zerver/tests/test_message_flags.py | 6 ++++++ 5 files changed, 37 insertions(+), 1 deletion(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 6bffb6a552..600ddd2032 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -10,6 +10,13 @@ below features are supported. ## Changes in Zulip 4.0 +**Feature level 44** + +* [`POST /register`](/api/register-queue): extended the `unread_msgs` + object to include `old_unreads_missing`, which indicates whether the + server truncated the `unread_msgs` due to excessive total unread + messages. + **Feature level 43** * [`GET /users/{user_id_or_email}/presence`]: Added support for diff --git a/version.py b/version.py index 350e00d99f..50beb87e0f 100644 --- a/version.py +++ b/version.py @@ -30,7 +30,7 @@ DESKTOP_WARNING_VERSION = "5.2.0" # # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md. -API_FEATURE_LEVEL = 43 +API_FEATURE_LEVEL = 44 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/lib/message.py b/zerver/lib/message.py index d7497a8e33..e062642436 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -73,6 +73,7 @@ class RawUnreadMessagesResult(TypedDict): mentions: Set[int] muted_stream_ids: List[int] unmuted_stream_msgs: Set[int] + old_unreads_missing: bool class UnreadMessagesResult(TypedDict): @@ -81,6 +82,7 @@ class UnreadMessagesResult(TypedDict): huddles: List[Dict[str, Any]] mentions: List[int] count: int + old_unreads_missing: bool @dataclass @@ -981,6 +983,7 @@ def extract_unread_data_from_um_rows( unmuted_stream_msgs: Set[int] = set() huddle_dict: Dict[int, Any] = {} mentions: Set[int] = set() + total_unreads = 0 raw_unread_messages: RawUnreadMessagesResult = dict( pm_dict=pm_dict, @@ -989,6 +992,7 @@ def extract_unread_data_from_um_rows( unmuted_stream_msgs=unmuted_stream_msgs, huddle_dict=huddle_dict, mentions=mentions, + old_unreads_missing=False, ) if user_profile is None: @@ -1019,6 +1023,7 @@ def extract_unread_data_from_um_rows( return user_ids_string for row in rows: + total_unreads += 1 message_id = row["message_id"] msg_type = row["message__recipient__type"] recipient_id = row["message__recipient_id"] @@ -1071,6 +1076,11 @@ def extract_unread_data_from_um_rows( else: # nocoverage # TODO: Test wildcard mentions in PMs. mentions.add(message_id) + # Record whether the user had more than MAX_UNREAD_MESSAGES total + # unreads -- that's a state where Zulip's behavior will start to + # be erroneous, and clients should display a warning. + raw_unread_messages["old_unreads_missing"] = total_unreads == MAX_UNREAD_MESSAGES + return raw_unread_messages @@ -1115,6 +1125,7 @@ def aggregate_unread_data(raw_data: RawUnreadMessagesResult) -> UnreadMessagesRe huddles=huddle_objects, mentions=mentions, count=count, + old_unreads_missing=raw_data["old_unreads_missing"], ) return result diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index a571a62d0d..ab4188ab02 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -6438,6 +6438,18 @@ paths: For muted streams, wildcard mentions will not be considered for this array. items: type: integer + old_unreads_missing: + type: boolean + description: | + True if this data set was truncated because the user has too many + unread messages. When truncation occurs, only the most recent + `MAX_UNREAD_MESSAGES` (currently 50000) messages will be considered + when forming this response. When true, we recommend that clients + display a warning, as they are likely to produce erroneous results + until reloaded with the user having fewer than `MAX_UNREAD_MESSAGES` + unread messages. + + **Changes**: New in Zulip 4.0 (feature level 44). starred_messages: type: array items: diff --git a/zerver/tests/test_message_flags.py b/zerver/tests/test_message_flags.py index 284f020e87..b4376615bd 100644 --- a/zerver/tests/test_message_flags.py +++ b/zerver/tests/test_message_flags.py @@ -854,12 +854,18 @@ class GetUnreadMsgsTest(ZulipTestCase): aggregated_data = aggregate_unread_data(raw_unread_data) return aggregated_data + with mock.patch("zerver.lib.message.MAX_UNREAD_MESSAGES", 4): + result = get_unread_data() + self.assertEqual(result["count"], 2) + self.assertTrue(result["old_unreads_missing"]) + result = get_unread_data() # The count here reflects the count of unread messages that we will # report to users in the bankruptcy dialog, and for now it excludes unread messages # from muted treams, but it doesn't exclude unread messages from muted topics yet. self.assertEqual(result["count"], 4) + self.assertFalse(result["old_unreads_missing"]) unread_pm = result["pms"][0] self.assertEqual(unread_pm["sender_id"], sender_id)