Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ETag header support to /state #17974

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/17974.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add ETag header to the /rooms/$room_id/state endpoint, and return no content on cache hit.
17 changes: 15 additions & 2 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
# [This file includes modifications made by New Vector Limited]
#
#
import hashlib
import logging
import random
from http import HTTPStatus
Expand Down Expand Up @@ -173,7 +174,8 @@ async def get_state_events(
room_id: str,
state_filter: Optional[StateFilter] = None,
at_token: Optional[StreamToken] = None,
) -> List[dict]:
last_hash: Optional[str] = None,
) -> Tuple[List[dict], Optional[str]]:
"""Retrieve all state events for a given room. If the user is
joined to the room then return the current state. If the user has
left the room return the state events from when they left. If an explicit
Expand All @@ -190,6 +192,8 @@ async def get_state_events(
state based on the current_state_events table.
Returns:
A list of dicts representing state events. [{}, {}, {}]
A hash of the state IDs representing the state events. This is only calculated if
no at_token is given and the user is joined to the room.
Raises:
NotFoundError (404) if the at token does not yield an event

Expand All @@ -200,6 +204,7 @@ async def get_state_events(
state_filter = StateFilter.all()

user_id = requester.user.to_string()
hash = None

if at_token:
last_event_id = (
Expand Down Expand Up @@ -239,6 +244,14 @@ async def get_state_events(
state_ids = await self._state_storage_controller.get_current_state_ids(
room_id, state_filter=state_filter
)

hash = hashlib.sha1(
",".join(state_ids.values()).encode("utf-8")
).hexdigest()
# If the requester's hash matches ours, their cache is up to date and we can skip
# fetching events.
if last_hash == hash:
return [], hash
room_state = await self.store.get_events(state_ids.values())
elif membership == Membership.LEAVE:
# If the membership is not JOIN, then the event ID should exist.
Expand All @@ -257,7 +270,7 @@ async def get_state_events(
self.clock.time_msec(),
config=SerializeEventConfig(requester=requester),
)
return events
return events, hash

async def _user_can_see_state_at_event(
self, user_id: str, room_id: str, event_id: str
Expand Down
15 changes: 12 additions & 3 deletions synapse/rest/client/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ async def on_GET(
membership = parse_string(request, "membership")
not_membership = parse_string(request, "not_membership")

events = await handler.get_state_events(
events, _ = await handler.get_state_events(
room_id=room_id,
requester=requester,
at_token=at_token,
Expand Down Expand Up @@ -835,13 +835,22 @@ def __init__(self, hs: "HomeServer"):
@cancellable
async def on_GET(
self, request: SynapseRequest, room_id: str
) -> Tuple[int, List[JsonDict]]:
) -> Tuple[int, Optional[List[JsonDict]]]:
requester = await self.auth.get_user_by_req(request, allow_guest=True)
existing_hash = request.getHeader(b"If-None-Match")

# Get all the current state for this room
events = await self.message_handler.get_state_events(
events, hash = await self.message_handler.get_state_events(
room_id=room_id,
requester=requester,
# Trim quotes from hash.
last_hash=existing_hash.decode("ascii")[1:-1] if existing_hash else None,
)
request.setHeader(b"ETag", f'"{hash}"'.encode("ascii"))

if len(events) == 0:
return 304, None

return 200, events


Expand Down
58 changes: 58 additions & 0 deletions tests/rest/client/test_rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,64 @@ def test_get_state_event_cancellation(self) -> None:
self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.result["body"])
self.assertEqual(channel.json_body, {"membership": "join"})

def test_get_state_etag_match(self) -> None:
"""Test that `/rooms/$room_id/state` returns a NOT_MODIFIED response when provided with the correct ETag."""
room_id = self.helper.create_room_as(self.user_id)
channel = self.make_request(
"GET",
"/rooms/%s/state" % room_id,
)

etagheader = channel.headers.getRawHeaders(b"ETag")
self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.result["body"])
assert etagheader
channel2 = self.make_request(
"GET",
"/rooms/%s/state" % room_id,
custom_headers=[
(
b"If-None-Match",
etagheader[0],
),
],
)
self.assertEqual(
HTTPStatus.NOT_MODIFIED,
channel2.code,
"Responds with not modified when provided with the correct ETag",
)
self.assertEqual(
etagheader,
channel2.headers.getRawHeaders(b"ETag"),
"returns the same etag",
)

def test_get_state_etag_nonmatch(self) -> None:
"""Test that `/rooms/$room_id/state` returns a normal response to an unrecognised ETag."""
room_id = self.helper.create_room_as(self.user_id)
channel = self.make_request(
"GET",
"/rooms/%s/state" % room_id,
custom_headers=(
(
b"If-None-Match",
'"notavalidetag"',
),
),
)

self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.result["body"])
self.assertCountEqual(
[state_event["type"] for state_event in channel.json_list],
{
"m.room.create",
"m.room.power_levels",
"m.room.join_rules",
"m.room.member",
"m.room.history_visibility",
},
)


class RoomsMemberListTestCase(RoomBase):
"""Tests /rooms/$room_id/members/list REST events."""
Expand Down
Loading