Skip to content
1 change: 1 addition & 0 deletions changelog.d/19897.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Change the [MSC3814](https://github.com/matrix-org/matrix-spec-proposals/pull/3814) dehydrated device `/events` endpoint paging to match spec conventions.
13 changes: 9 additions & 4 deletions synapse/handlers/devicemessage.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,9 @@ async def get_events_for_dehydrated_device(
since_token: stream id to start from when fetching messages
limit: the number of messages to fetch
Returns:
A dict containing the to-device messages, as well as a token that the client
can provide in the next call to fetch the next batch of messages
A dict containing the to-device `messages`, as well as a `next_batch` token that the
client can provide in the next call to fetch the next batch of messages. If `next_batch`
is missing, there are no more messages to fetch.
"""

user_id = requester.user.to_string()
Expand Down Expand Up @@ -426,11 +427,15 @@ async def get_events_for_dehydrated_device(
user_id,
)

return {
ret: JsonDict = {
"events": messages,
"next_batch": f"d{stream_id}",
}

if len(messages) > 0:
ret["next_batch"] = f"d{stream_id}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that the client will have to make an extra round-trip when it gets to the end of the pending messages, just to double-check that there are no more messages. Could we make get_messages_for_device return an additional boolean return value, indicating whether or not the result was "limited"?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about c701266 ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's valid in the type system!

Seems reasonable in principle though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch, thank you. I've pushed fixes for this and a couple of other problems.


return ret


def split_device_messages_into_edus(
sender_user_id: str,
Expand Down
4 changes: 2 additions & 2 deletions synapse/rest/client/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,13 +255,13 @@ async def on_GET(
) -> tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request)

next_batch = parse_string(request, "next_batch")
since_token = parse_string(request, "from")
limit = parse_integer(request, "limit", 100)

msgs = await self.message_handler.get_events_for_dehydrated_device(
requester=requester,
device_id=device_id,
since_token=next_batch,
since_token=since_token,
limit=limit,
)

Expand Down
3 changes: 2 additions & 1 deletion tests/rest/client/test_devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,13 @@ def test_dehydrate_msc3814(self) -> None:
# messages so we should receive an empty array
Comment thread
andybalaam marked this conversation as resolved.
Outdated
channel = self.make_request(
"GET",
f"_matrix/client/unstable/org.matrix.msc3814.v1/dehydrated_device/{device_id}/events?next_batch={next_batch_token}",
f"_matrix/client/unstable/org.matrix.msc3814.v1/dehydrated_device/{device_id}/events?from={next_batch_token}",
access_token=token,
shorthand=False,
)
self.assertEqual(channel.code, 200)
self.assertEqual(channel.json_body["events"], [])
self.assertNotIn("next_batch", channel.json_body)

# make sure we can delete the dehydrated device
channel = self.make_request(
Expand Down
Loading