From ee2cb855f0f61462aa46b8ff21411a4bedc9b553 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Thu, 10 Nov 2022 18:32:09 -0800 Subject: [PATCH] message_fetch: Add include_anchor parameter. Signed-off-by: Anders Kaseorg --- templates/zerver/api/changelog.md | 6 ++++++ version.py | 2 +- zerver/lib/narrow.py | 15 +++++++++------ zerver/lib/test_classes.py | 2 ++ zerver/openapi/zulip.yaml | 17 +++++++++++++++-- zerver/tests/test_message_fetch.py | 29 +++++++++++++++++++++++++++++ zerver/tests/test_openapi.py | 2 ++ zerver/views/message_fetch.py | 4 ++++ 8 files changed, 68 insertions(+), 9 deletions(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index b2b87afc58..e439877981 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 6.0 +**Feature level 155** + +* [`GET /messages`](/api/get-messages): The new `include_anchor` + parameter controls whether a message with ID matching the specified + `anchor` should be included. + **Feature level 154** * [`POST /streams/{stream_id}/delete_topic`](/api/delete-topic): diff --git a/version.py b/version.py index 96ebfd0fbe..1120da258f 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3" # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md, as well as # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 154 +API_FEATURE_LEVEL = 155 # 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/narrow.py b/zerver/lib/narrow.py index 6fd3c64a09..697de10d9b 100644 --- a/zerver/lib/narrow.py +++ b/zerver/lib/narrow.py @@ -994,6 +994,7 @@ def limit_query_to_range( num_before: int, num_after: int, anchor: int, + include_anchor: bool, anchored_to_left: bool, anchored_to_right: bool, id_col: ColumnElement[Integer], @@ -1010,10 +1011,10 @@ def limit_query_to_range( # The semantics of our flags are as follows: # - # num_after = number of rows < anchor + # num_before = number of rows < anchor # num_after = number of rows > anchor # - # But we also want the row where id == anchor (if it exists), + # But we may also want the row where id == anchor (if it exists), # and we don't want to union up to 3 queries. So in some cases # we do things like `after_limit = num_after + 1` to grab the # anchor row in the "after" query. @@ -1026,13 +1027,13 @@ def limit_query_to_range( before_limit = num_before after_limit = num_after + 1 elif need_before_query: - before_anchor = anchor + before_anchor = anchor - (not include_anchor) before_limit = num_before if not anchored_to_right: - before_limit += 1 + before_limit += include_anchor elif need_after_query: - after_anchor = max(anchor, first_visible_message_id) - after_limit = num_after + 1 + after_anchor = max(anchor + (not include_anchor), first_visible_message_id) + after_limit = num_after + include_anchor if need_before_query: before_query = query @@ -1163,6 +1164,7 @@ def fetch_messages( realm: Realm, is_web_public_query: bool, anchor: Optional[int], + include_anchor: bool, num_before: int, num_after: int, ) -> FetchedMessages: @@ -1228,6 +1230,7 @@ def fetch_messages( num_before=num_before, num_after=num_after, anchor=anchor, + include_anchor=include_anchor, anchored_to_left=anchored_to_left, anchored_to_right=anchored_to_right, id_col=inner_msg_id_col, diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 35da3d69f2..54434473ce 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1044,12 +1044,14 @@ Output: num_before: int = 100, num_after: int = 100, use_first_unread_anchor: bool = False, + include_anchor: bool = True, ) -> Dict[str, List[Dict[str, Any]]]: post_params = { "anchor": anchor, "num_before": num_before, "num_after": num_after, "use_first_unread_anchor": orjson.dumps(use_first_unread_anchor).decode(), + "include_anchor": orjson.dumps(include_anchor).decode(), } result = self.client_get("/json/messages", dict(post_params)) data = result.json() diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index dde8faa973..9abd3e472c 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -5057,6 +5057,7 @@ paths: - client_gravatar - apply_markdown - use_first_unread_anchor + - include_anchor parameters: - name: anchor in: query @@ -5086,6 +5087,17 @@ paths: - type: string - type: integer example: 43 + - name: include_anchor + in: query + description: | + Whether a message with the specified ID matching the narrow + should be included. + + **Changes**: New in Zulip 6.0 (feature level 155). + schema: + type: boolean + default: true + example: false - name: num_before in: query description: | @@ -5220,8 +5232,9 @@ paths: description: | Whether the anchor message is included in the response. If the message with the ID specified - in the request does not exist or did not match - the narrow, this will be false. + in the request does not exist, did not match + the narrow, or was excluded via + `include_anchor=false`, this will be false. history_limited: type: boolean description: | diff --git a/zerver/tests/test_message_fetch.py b/zerver/tests/test_message_fetch.py index 96209979c5..904aa152ce 100644 --- a/zerver/tests/test_message_fetch.py +++ b/zerver/tests/test_message_fetch.py @@ -2864,6 +2864,28 @@ class GetOldMessagesTest(ZulipTestCase): self.assertEqual(data["found_newest"], True) self.assertEqual(data["history_limited"], False) + data = self.get_messages_response( + anchor=message_ids[5], num_before=3, num_after=0, include_anchor=False + ) + + messages = data["messages"] + self.assertEqual(data["found_anchor"], False) + self.assertEqual(data["found_oldest"], False) + self.assertEqual(data["found_newest"], False) + self.assertEqual(data["history_limited"], False) + messages_matches_ids(messages, message_ids[2:5]) + + data = self.get_messages_response( + anchor=message_ids[5], num_before=0, num_after=3, include_anchor=False + ) + + messages = data["messages"] + self.assertEqual(data["found_anchor"], False) + self.assertEqual(data["found_oldest"], False) + self.assertEqual(data["found_newest"], False) + self.assertEqual(data["history_limited"], False) + messages_matches_ids(messages, message_ids[6:9]) + def test_missing_params(self) -> None: """ anchor, num_before, and num_after are all required @@ -2914,6 +2936,13 @@ class GetOldMessagesTest(ZulipTestCase): result = self.client_get("/json/messages", post_params) self.assert_json_error(result, f"Bad value for '{param}': {type}") + def test_bad_include_anchor(self) -> None: + self.login("hamlet") + result = self.client_get( + "/json/messages", dict(anchor=1, num_before=1, num_after=1, include_anchor="false") + ) + self.assert_json_error(result, "The anchor can only be excluded at an end of the range") + def test_bad_narrow_type(self) -> None: """ narrow must be a list of string pairs. diff --git a/zerver/tests/test_openapi.py b/zerver/tests/test_openapi.py index 4b64c8105c..4fe3b98692 100644 --- a/zerver/tests/test_openapi.py +++ b/zerver/tests/test_openapi.py @@ -819,6 +819,7 @@ class TestCurlExampleGeneration(ZulipTestCase): "curl -sSX GET -G http://localhost:9991/api/v1/messages \\", " -u BOT_EMAIL_ADDRESS:BOT_API_KEY \\", " --data-urlencode anchor=43 \\", + " --data-urlencode include_anchor=false \\", " --data-urlencode num_before=4 \\", " --data-urlencode num_after=8 \\", ' --data-urlencode \'narrow=[{"operand": "Denmark", "operator": "stream"}]\' \\', @@ -893,6 +894,7 @@ class TestCurlExampleGeneration(ZulipTestCase): "curl -sSX GET -G http://localhost:9991/api/v1/messages \\", " -u BOT_EMAIL_ADDRESS:BOT_API_KEY \\", " --data-urlencode anchor=43 \\", + " --data-urlencode include_anchor=false \\", " --data-urlencode num_before=4 \\", " --data-urlencode num_after=8 \\", ' --data-urlencode \'narrow=[{"operand": "Denmark", "operator": "stream"}]\' \\', diff --git a/zerver/views/message_fetch.py b/zerver/views/message_fetch.py index 3826361048..b7271e03a3 100644 --- a/zerver/views/message_fetch.py +++ b/zerver/views/message_fetch.py @@ -84,6 +84,7 @@ def get_messages_backend( request: HttpRequest, maybe_user_profile: Union[UserProfile, AnonymousUser], anchor_val: Optional[str] = REQ("anchor", default=None), + include_anchor: bool = REQ(json_validator=check_bool, default=True), num_before: int = REQ(converter=to_non_negative_int), num_after: int = REQ(converter=to_non_negative_int), narrow: OptionalNarrowListT = REQ("narrow", converter=narrow_parameter, default=None), @@ -100,6 +101,8 @@ def get_messages_backend( MAX_MESSAGES_PER_FETCH, ) ) + if num_before > 0 and num_after > 0 and not include_anchor: + raise JsonableError(_("The anchor can only be excluded at an end of the range")) realm = get_valid_realm_from_request(request) if not maybe_user_profile.is_authenticated: @@ -160,6 +163,7 @@ def get_messages_backend( realm=realm, is_web_public_query=is_web_public_query, anchor=anchor, + include_anchor=include_anchor, num_before=num_before, num_after=num_after, )