Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Fix /initialSync error due to unhashable RoomStreamToken (#10827)
Browse files Browse the repository at this point in the history
The deprecated /initialSync endpoint maintains a cache of responses,
using parameter values as part of the cache key. When a `from` or `to`
parameter is specified, it gets converted into a `StreamToken`, which
contains a `RoomStreamToken` and forms part of the cache key.
`RoomStreamToken`s need to be made hashable for this to work.
  • Loading branch information
squahtx authored Sep 22, 2021
1 parent 52913d5 commit 9391de3
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 6 deletions.
1 change: 1 addition & 0 deletions changelog.d/10827.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix error in deprecated `/initialSync` endpoint when using the undocumented `from` and `to` parameters.
4 changes: 3 additions & 1 deletion synapse/storage/databases/main/stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
from collections import namedtuple
from typing import TYPE_CHECKING, Collection, Dict, List, Optional, Set, Tuple

from frozendict import frozendict

from twisted.internet import defer

from synapse.api.filtering import Filter
Expand Down Expand Up @@ -379,7 +381,7 @@ def get_room_max_token(self) -> RoomStreamToken:
if p > min_pos
}

return RoomStreamToken(None, min_pos, positions)
return RoomStreamToken(None, min_pos, frozendict(positions))

async def get_room_events_stream_for_rooms(
self,
Expand Down
20 changes: 15 additions & 5 deletions synapse/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
)

import attr
from frozendict import frozendict
from signedjson.key import decode_verify_key_bytes
from unpaddedbase64 import decode_base64
from zope.interface import Interface
Expand Down Expand Up @@ -457,6 +458,9 @@ class RoomStreamToken:
Note: The `RoomStreamToken` cannot have both a topological part and an
instance map.
For caching purposes, `RoomStreamToken`s and by extension, all their
attributes, must be hashable.
"""

topological = attr.ib(
Expand All @@ -466,12 +470,12 @@ class RoomStreamToken:
stream = attr.ib(type=int, validator=attr.validators.instance_of(int))

instance_map = attr.ib(
type=Dict[str, int],
factory=dict,
type="frozendict[str, int]",
factory=frozendict,
validator=attr.validators.deep_mapping(
key_validator=attr.validators.instance_of(str),
value_validator=attr.validators.instance_of(int),
mapping_validator=attr.validators.instance_of(dict),
mapping_validator=attr.validators.instance_of(frozendict),
),
)

Expand Down Expand Up @@ -507,7 +511,7 @@ async def parse(cls, store: "DataStore", string: str) -> "RoomStreamToken":
return cls(
topological=None,
stream=stream,
instance_map=instance_map,
instance_map=frozendict(instance_map),
)
except Exception:
pass
Expand Down Expand Up @@ -540,7 +544,7 @@ def copy_and_advance(self, other: "RoomStreamToken") -> "RoomStreamToken":
for instance in set(self.instance_map).union(other.instance_map)
}

return RoomStreamToken(None, max_stream, instance_map)
return RoomStreamToken(None, max_stream, frozendict(instance_map))

def as_historical_tuple(self) -> Tuple[int, int]:
"""Returns a tuple of `(topological, stream)` for historical tokens.
Expand Down Expand Up @@ -593,6 +597,12 @@ async def to_string(self, store: "DataStore") -> str:

@attr.s(slots=True, frozen=True)
class StreamToken:
"""A collection of positions within multiple streams.
For caching purposes, `StreamToken`s and by extension, all their attributes,
must be hashable.
"""

room_key = attr.ib(
type=RoomStreamToken, validator=attr.validators.instance_of(RoomStreamToken)
)
Expand Down

0 comments on commit 9391de3

Please sign in to comment.