message_fetch: Add include_anchor parameter.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit is contained in:
Anders Kaseorg 2022-11-10 18:32:09 -08:00 committed by Tim Abbott
parent dae4633745
commit ee2cb855f0
8 changed files with 68 additions and 9 deletions

View File

@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 6.0 ## 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** **Feature level 154**
* [`POST /streams/{stream_id}/delete_topic`](/api/delete-topic): * [`POST /streams/{stream_id}/delete_topic`](/api/delete-topic):

View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
# Changes should be accompanied by documentation explaining what the # Changes should be accompanied by documentation explaining what the
# new level means in templates/zerver/api/changelog.md, as well as # new level means in templates/zerver/api/changelog.md, as well as
# "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. # "**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 # 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 # only when going from an old version of the code to a newer version. Bump

View File

@ -994,6 +994,7 @@ def limit_query_to_range(
num_before: int, num_before: int,
num_after: int, num_after: int,
anchor: int, anchor: int,
include_anchor: bool,
anchored_to_left: bool, anchored_to_left: bool,
anchored_to_right: bool, anchored_to_right: bool,
id_col: ColumnElement[Integer], id_col: ColumnElement[Integer],
@ -1010,10 +1011,10 @@ def limit_query_to_range(
# The semantics of our flags are as follows: # 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 # 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 # 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 # we do things like `after_limit = num_after + 1` to grab the
# anchor row in the "after" query. # anchor row in the "after" query.
@ -1026,13 +1027,13 @@ def limit_query_to_range(
before_limit = num_before before_limit = num_before
after_limit = num_after + 1 after_limit = num_after + 1
elif need_before_query: elif need_before_query:
before_anchor = anchor before_anchor = anchor - (not include_anchor)
before_limit = num_before before_limit = num_before
if not anchored_to_right: if not anchored_to_right:
before_limit += 1 before_limit += include_anchor
elif need_after_query: elif need_after_query:
after_anchor = max(anchor, first_visible_message_id) after_anchor = max(anchor + (not include_anchor), first_visible_message_id)
after_limit = num_after + 1 after_limit = num_after + include_anchor
if need_before_query: if need_before_query:
before_query = query before_query = query
@ -1163,6 +1164,7 @@ def fetch_messages(
realm: Realm, realm: Realm,
is_web_public_query: bool, is_web_public_query: bool,
anchor: Optional[int], anchor: Optional[int],
include_anchor: bool,
num_before: int, num_before: int,
num_after: int, num_after: int,
) -> FetchedMessages: ) -> FetchedMessages:
@ -1228,6 +1230,7 @@ def fetch_messages(
num_before=num_before, num_before=num_before,
num_after=num_after, num_after=num_after,
anchor=anchor, anchor=anchor,
include_anchor=include_anchor,
anchored_to_left=anchored_to_left, anchored_to_left=anchored_to_left,
anchored_to_right=anchored_to_right, anchored_to_right=anchored_to_right,
id_col=inner_msg_id_col, id_col=inner_msg_id_col,

View File

@ -1044,12 +1044,14 @@ Output:
num_before: int = 100, num_before: int = 100,
num_after: int = 100, num_after: int = 100,
use_first_unread_anchor: bool = False, use_first_unread_anchor: bool = False,
include_anchor: bool = True,
) -> Dict[str, List[Dict[str, Any]]]: ) -> Dict[str, List[Dict[str, Any]]]:
post_params = { post_params = {
"anchor": anchor, "anchor": anchor,
"num_before": num_before, "num_before": num_before,
"num_after": num_after, "num_after": num_after,
"use_first_unread_anchor": orjson.dumps(use_first_unread_anchor).decode(), "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)) result = self.client_get("/json/messages", dict(post_params))
data = result.json() data = result.json()

View File

@ -5057,6 +5057,7 @@ paths:
- client_gravatar - client_gravatar
- apply_markdown - apply_markdown
- use_first_unread_anchor - use_first_unread_anchor
- include_anchor
parameters: parameters:
- name: anchor - name: anchor
in: query in: query
@ -5086,6 +5087,17 @@ paths:
- type: string - type: string
- type: integer - type: integer
example: 43 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 - name: num_before
in: query in: query
description: | description: |
@ -5220,8 +5232,9 @@ paths:
description: | description: |
Whether the anchor message is included in the Whether the anchor message is included in the
response. If the message with the ID specified response. If the message with the ID specified
in the request does not exist or did not match in the request does not exist, did not match
the narrow, this will be false. the narrow, or was excluded via
`include_anchor=false`, this will be false.
history_limited: history_limited:
type: boolean type: boolean
description: | description: |

View File

@ -2864,6 +2864,28 @@ class GetOldMessagesTest(ZulipTestCase):
self.assertEqual(data["found_newest"], True) self.assertEqual(data["found_newest"], True)
self.assertEqual(data["history_limited"], False) 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: def test_missing_params(self) -> None:
""" """
anchor, num_before, and num_after are all required anchor, num_before, and num_after are all required
@ -2914,6 +2936,13 @@ class GetOldMessagesTest(ZulipTestCase):
result = self.client_get("/json/messages", post_params) result = self.client_get("/json/messages", post_params)
self.assert_json_error(result, f"Bad value for '{param}': {type}") 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: def test_bad_narrow_type(self) -> None:
""" """
narrow must be a list of string pairs. narrow must be a list of string pairs.

View File

@ -819,6 +819,7 @@ class TestCurlExampleGeneration(ZulipTestCase):
"curl -sSX GET -G http://localhost:9991/api/v1/messages \\", "curl -sSX GET -G http://localhost:9991/api/v1/messages \\",
" -u BOT_EMAIL_ADDRESS:BOT_API_KEY \\", " -u BOT_EMAIL_ADDRESS:BOT_API_KEY \\",
" --data-urlencode anchor=43 \\", " --data-urlencode anchor=43 \\",
" --data-urlencode include_anchor=false \\",
" --data-urlencode num_before=4 \\", " --data-urlencode num_before=4 \\",
" --data-urlencode num_after=8 \\", " --data-urlencode num_after=8 \\",
' --data-urlencode \'narrow=[{"operand": "Denmark", "operator": "stream"}]\' \\', ' --data-urlencode \'narrow=[{"operand": "Denmark", "operator": "stream"}]\' \\',
@ -893,6 +894,7 @@ class TestCurlExampleGeneration(ZulipTestCase):
"curl -sSX GET -G http://localhost:9991/api/v1/messages \\", "curl -sSX GET -G http://localhost:9991/api/v1/messages \\",
" -u BOT_EMAIL_ADDRESS:BOT_API_KEY \\", " -u BOT_EMAIL_ADDRESS:BOT_API_KEY \\",
" --data-urlencode anchor=43 \\", " --data-urlencode anchor=43 \\",
" --data-urlencode include_anchor=false \\",
" --data-urlencode num_before=4 \\", " --data-urlencode num_before=4 \\",
" --data-urlencode num_after=8 \\", " --data-urlencode num_after=8 \\",
' --data-urlencode \'narrow=[{"operand": "Denmark", "operator": "stream"}]\' \\', ' --data-urlencode \'narrow=[{"operand": "Denmark", "operator": "stream"}]\' \\',

View File

@ -84,6 +84,7 @@ def get_messages_backend(
request: HttpRequest, request: HttpRequest,
maybe_user_profile: Union[UserProfile, AnonymousUser], maybe_user_profile: Union[UserProfile, AnonymousUser],
anchor_val: Optional[str] = REQ("anchor", default=None), 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_before: int = REQ(converter=to_non_negative_int),
num_after: 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), narrow: OptionalNarrowListT = REQ("narrow", converter=narrow_parameter, default=None),
@ -100,6 +101,8 @@ def get_messages_backend(
MAX_MESSAGES_PER_FETCH, 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) realm = get_valid_realm_from_request(request)
if not maybe_user_profile.is_authenticated: if not maybe_user_profile.is_authenticated:
@ -160,6 +163,7 @@ def get_messages_backend(
realm=realm, realm=realm,
is_web_public_query=is_web_public_query, is_web_public_query=is_web_public_query,
anchor=anchor, anchor=anchor,
include_anchor=include_anchor,
num_before=num_before, num_before=num_before,
num_after=num_after, num_after=num_after,
) )