From e4eb0faa2984cf3885e3823f68d37f5fec95661d Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Wed, 15 Sep 2021 14:00:51 +0100 Subject: [PATCH 1/3] Fix /initialSync error due to unhashable `RoomStreamToken` 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. --- changelog.d/10826.bugfix | 1 + synapse/storage/databases/main/stream.py | 4 +++- synapse/types.py | 11 ++++++----- 3 files changed, 10 insertions(+), 6 deletions(-) create mode 100644 changelog.d/10826.bugfix diff --git a/changelog.d/10826.bugfix b/changelog.d/10826.bugfix new file mode 100644 index 000000000000..11a618bf8293 --- /dev/null +++ b/changelog.d/10826.bugfix @@ -0,0 +1 @@ +Fix error in deprecated `/initialSync` endpoint when using the undocumented `from` and `to` parameters. diff --git a/synapse/storage/databases/main/stream.py b/synapse/storage/databases/main/stream.py index 959f13de479f..9a3b6f4acfef 100644 --- a/synapse/storage/databases/main/stream.py +++ b/synapse/storage/databases/main/stream.py @@ -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 @@ -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, diff --git a/synapse/types.py b/synapse/types.py index 90168ce8fab3..74189a346c6b 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -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 @@ -466,12 +467,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=Mapping[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), ), ) @@ -507,7 +508,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 @@ -540,7 +541,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. From 79bf04563f6d4c572e2e4ca910043063bf5279b8 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Wed, 15 Sep 2021 15:04:44 +0100 Subject: [PATCH 2/3] fix newsfile --- changelog.d/{10826.bugfix => 10827.bugfix} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{10826.bugfix => 10827.bugfix} (100%) diff --git a/changelog.d/10826.bugfix b/changelog.d/10827.bugfix similarity index 100% rename from changelog.d/10826.bugfix rename to changelog.d/10827.bugfix From fc7d054c6473e23a53ceac9c21afdd9419252ded Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 17 Sep 2021 10:40:52 +0100 Subject: [PATCH 3/3] add notes to docstrings and fix attribute type --- synapse/types.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/synapse/types.py b/synapse/types.py index 74189a346c6b..ed831a5c1dd8 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -458,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( @@ -467,7 +470,7 @@ class RoomStreamToken: stream = attr.ib(type=int, validator=attr.validators.instance_of(int)) instance_map = attr.ib( - type=Mapping[str, int], + type="frozendict[str, int]", factory=frozendict, validator=attr.validators.deep_mapping( key_validator=attr.validators.instance_of(str), @@ -594,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) )