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

Commit

Permalink
Fix a bug in the send_local_online_presence_to module API (#14880)
Browse files Browse the repository at this point in the history
Destination was being used incorrectly (a single destination instead
of a list of destinations was being passed).

This also updates some of the types in the area to not use Collection[str],
which is a footgun.
  • Loading branch information
clokep authored Jan 25, 2023
1 parent 3c3ba31 commit 7e8d455
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 11 deletions.
1 change: 1 addition & 0 deletions changelog.d/14880.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug when using the `send_local_online_presence_to` module API.
18 changes: 12 additions & 6 deletions synapse/handlers/presence.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,13 @@
from synapse.replication.tcp.streams import PresenceFederationStream, PresenceStream
from synapse.storage.databases.main import DataStore
from synapse.streams import EventSource
from synapse.types import JsonDict, StreamKeyType, UserID, get_domain_from_id
from synapse.types import (
JsonDict,
StrCollection,
StreamKeyType,
UserID,
get_domain_from_id,
)
from synapse.util.async_helpers import Linearizer
from synapse.util.metrics import Measure
from synapse.util.wheel_timer import WheelTimer
Expand Down Expand Up @@ -320,7 +326,7 @@ async def maybe_send_presence_to_interested_destinations(
for destination, host_states in hosts_to_states.items():
self._federation.send_presence_to_destinations(host_states, [destination])

async def send_full_presence_to_users(self, user_ids: Collection[str]) -> None:
async def send_full_presence_to_users(self, user_ids: StrCollection) -> None:
"""
Adds to the list of users who should receive a full snapshot of presence
upon their next sync. Note that this only works for local users.
Expand Down Expand Up @@ -1601,7 +1607,7 @@ async def get_new_events(
# Having a default limit doesn't match the EventSource API, but some
# callers do not provide it. It is unused in this class.
limit: int = 0,
room_ids: Optional[Collection[str]] = None,
room_ids: Optional[StrCollection] = None,
is_guest: bool = False,
explicit_room_id: Optional[str] = None,
include_offline: bool = True,
Expand Down Expand Up @@ -1688,7 +1694,7 @@ async def get_new_events(

# The set of users that we're interested in and that have had a presence update.
# We'll actually pull the presence updates for these users at the end.
interested_and_updated_users: Collection[str]
interested_and_updated_users: StrCollection

if from_key is not None:
# First get all users that have had a presence update
Expand Down Expand Up @@ -2120,7 +2126,7 @@ def __init__(self, hs: "HomeServer", presence_handler: BasePresenceHandler):
# stream_id, destinations, user_ids)`. We don't store the full states
# for efficiency, and remote workers will already have the full states
# cached.
self._queue: List[Tuple[int, int, Collection[str], Set[str]]] = []
self._queue: List[Tuple[int, int, StrCollection, Set[str]]] = []

self._next_id = 1

Expand All @@ -2142,7 +2148,7 @@ def _clear_queue(self) -> None:
self._queue = self._queue[index:]

def send_presence_to_destinations(
self, states: Collection[UserPresenceState], destinations: Collection[str]
self, states: Collection[UserPresenceState], destinations: StrCollection
) -> None:
"""Send the presence states to the given destinations.
Expand Down
2 changes: 1 addition & 1 deletion synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1158,7 +1158,7 @@ async def send_local_online_presence_to(self, users: Iterable[str]) -> None:
# Send to remote destinations.
destination = UserID.from_string(user).domain
presence_handler.get_federation_queue().send_presence_to_destinations(
presence_events, destination
presence_events, [destination]
)

def looping_background_call(
Expand Down
3 changes: 2 additions & 1 deletion synapse/notifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
JsonDict,
PersistedEventPosition,
RoomStreamToken,
StrCollection,
StreamKeyType,
StreamToken,
UserID,
Expand Down Expand Up @@ -716,7 +717,7 @@ async def check_for_updates(

async def _get_room_ids(
self, user: UserID, explicit_room_id: Optional[str]
) -> Tuple[Collection[str], bool]:
) -> Tuple[StrCollection, bool]:
joined_room_ids = await self.store.get_rooms_for_user(user.to_string())
if explicit_room_id:
if explicit_room_id in joined_room_ids:
Expand Down
6 changes: 3 additions & 3 deletions synapse/streams/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Collection, Generic, List, Optional, Tuple, TypeVar
from typing import Generic, List, Optional, Tuple, TypeVar

from synapse.types import UserID
from synapse.types import StrCollection, UserID

# The key, this is either a stream token or int.
K = TypeVar("K")
Expand All @@ -28,7 +28,7 @@ async def get_new_events(
user: UserID,
from_key: K,
limit: int,
room_ids: Collection[str],
room_ids: StrCollection,
is_guest: bool,
explicit_room_id: Optional[str] = None,
) -> Tuple[List[R], K]:
Expand Down

0 comments on commit 7e8d455

Please sign in to comment.