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

Return read-only collections from @cached methods #13755

Merged
merged 11 commits into from
Feb 10, 2023
1 change: 1 addition & 0 deletions changelog.d/13755.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Re-type hint some collections as read-only.
4 changes: 2 additions & 2 deletions synapse/app/phone_stats_home.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import math
import resource
import sys
from typing import TYPE_CHECKING, List, Sized, Tuple
from typing import TYPE_CHECKING, List, Mapping, Sized, Tuple

from prometheus_client import Gauge

Expand Down Expand Up @@ -194,7 +194,7 @@ def performance_stats_init() -> None:
@wrap_as_background_process("generate_monthly_active_users")
async def generate_monthly_active_users() -> None:
current_mau_count = 0
current_mau_count_by_service = {}
current_mau_count_by_service: Mapping[str, int] = {}
reserved_users: Sized = ()
store = hs.get_datastores().main
if hs.config.server.limit_usage_by_mau or hs.config.server.mau_stats_only:
Expand Down
6 changes: 3 additions & 3 deletions synapse/config/room_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Any, List
from typing import Any, Collection

from matrix_common.regex import glob_to_regex

Expand Down Expand Up @@ -70,7 +70,7 @@ def is_alias_creation_allowed(self, user_id: str, room_id: str, alias: str) -> b
return False

def is_publishing_room_allowed(
self, user_id: str, room_id: str, aliases: List[str]
self, user_id: str, room_id: str, aliases: Collection[str]
) -> bool:
"""Checks if the given user is allowed to publish the room

Expand Down Expand Up @@ -122,7 +122,7 @@ def __init__(self, option_name: str, rule: JsonDict):
except Exception as e:
raise ConfigError("Failed to parse glob into regex") from e

def matches(self, user_id: str, room_id: str, aliases: List[str]) -> bool:
def matches(self, user_id: str, room_id: str, aliases: Collection[str]) -> bool:
"""Tests if this rule matches the given user_id, room_id and aliases.

Args:
Expand Down
6 changes: 3 additions & 3 deletions synapse/events/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union
from typing import TYPE_CHECKING, Any, Collection, Dict, List, Optional, Tuple, Union

import attr
from signedjson.types import SigningKey
Expand Down Expand Up @@ -103,7 +103,7 @@ def is_state(self) -> bool:

async def build(
self,
prev_event_ids: List[str],
prev_event_ids: Collection[str],
auth_event_ids: Optional[List[str]],
depth: Optional[int] = None,
) -> EventBase:
Expand Down Expand Up @@ -136,7 +136,7 @@ async def build(

format_version = self.room_version.event_format
# The types of auth/prev events changes between event versions.
prev_events: Union[List[str], List[Tuple[str, Dict[str, str]]]]
prev_events: Union[Collection[str], List[Tuple[str, Dict[str, str]]]]
auth_events: Union[List[str], List[Tuple[str, Dict[str, str]]]]
if format_version == EventFormatVersions.ROOM_V1_V2:
auth_events = await self._store.add_event_hashes(auth_event_ids)
Expand Down
3 changes: 2 additions & 1 deletion synapse/federation/federation_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
Collection,
Dict,
List,
Mapping,
Optional,
Tuple,
Union,
Expand Down Expand Up @@ -1506,7 +1507,7 @@ async def on_query(self, query_type: str, args: dict) -> JsonDict:
def _get_event_ids_for_partial_state_join(
join_event: EventBase,
prev_state_ids: StateMap[str],
summary: Dict[str, MemberSummary],
summary: Mapping[str, MemberSummary],
) -> Collection[str]:
"""Calculate state to be returned in a partial_state send_join

Expand Down
5 changes: 3 additions & 2 deletions synapse/handlers/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import logging
import string
from typing import TYPE_CHECKING, Iterable, List, Optional
from typing import TYPE_CHECKING, Iterable, List, Optional, Sequence

from typing_extensions import Literal

Expand Down Expand Up @@ -485,6 +485,7 @@ async def edit_published_room_list(
)
)
if canonical_alias:
room_aliases = list(room_aliases)
room_aliases.append(canonical_alias)
clokep marked this conversation as resolved.
Show resolved Hide resolved

if not self.config.roomdirectory.is_publishing_room_allowed(
Expand Down Expand Up @@ -528,7 +529,7 @@ async def edit_published_appservice_room_list(

async def get_aliases_for_room(
self, requester: Requester, room_id: str
) -> List[str]:
) -> Sequence[str]:
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
"""
Get a list of the aliases that currently point to this room on this server
"""
Expand Down
4 changes: 2 additions & 2 deletions synapse/handlers/receipts.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from typing import TYPE_CHECKING, Iterable, List, Optional, Tuple
from typing import TYPE_CHECKING, Iterable, List, Optional, Sequence, Tuple

from synapse.api.constants import EduTypes, ReceiptTypes
from synapse.appservice import ApplicationService
Expand Down Expand Up @@ -189,7 +189,7 @@ def __init__(self, hs: "HomeServer"):

@staticmethod
def filter_out_private_receipts(
rooms: List[JsonDict], user_id: str
rooms: Sequence[JsonDict], user_id: str
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
) -> List[JsonDict]:
"""
Filters a list of serialized receipts (as returned by /sync and /initialSync)
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -1927,6 +1927,6 @@ async def shutdown_room(
return {
"kicked_users": kicked_users,
"failed_to_kick_users": failed_to_kick_users,
"local_aliases": aliases_for_room,
"local_aliases": list(aliases_for_room),
"new_room_id": new_room_id,
}
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 7 additions & 5 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -1493,7 +1493,7 @@ async def generate_sync_result(
one_time_keys_count = await self.store.count_e2e_one_time_keys(
user_id, device_id
)
unused_fallback_key_types = (
unused_fallback_key_types = list(
await self.store.get_e2e_unused_fallback_key_types(user_id, device_id)
)
Comment on lines +1522 to 1524
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This goes straight into the SyncResult and is pretty much only going to be JSON-serialized from here on.
Having to take a copy to make mypy happy is unfortunate.

Copy link
Contributor

Choose a reason for hiding this comment

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

At least it's a shallow copy 🤷

Copy link
Member

Choose a reason for hiding this comment

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

I'd be curious of the error here? Is something in our code unhappy or is it a type hint of like json.dump?

I think it'd be OK to add a cast with a comment explaining it is for performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a type hint problem. Only Lists and Tuples are JSON-serializable, except they're mutable types. I would have updated the definition of SyncResult otherwise.


Expand Down Expand Up @@ -1695,7 +1695,7 @@ async def _generate_sync_entry_for_to_device(

async def _generate_sync_entry_for_account_data(
self, sync_result_builder: "SyncResultBuilder"
) -> Dict[str, Dict[str, JsonDict]]:
) -> Mapping[str, Mapping[str, JsonDict]]:
"""Generates the account data portion of the sync response.

Account data (called "Client Config" in the spec) can be set either globally
Expand Down Expand Up @@ -1730,6 +1730,7 @@ async def _generate_sync_entry_for_account_data(
)

if push_rules_changed:
global_account_data = dict(global_account_data)
global_account_data["m.push_rules"] = await self.push_rules_for_user(
sync_config.user
)
Expand All @@ -1739,6 +1740,7 @@ async def _generate_sync_entry_for_account_data(
account_data_by_room,
) = await self.store.get_account_data_for_user(sync_config.user.to_string())

global_account_data = dict(global_account_data)
global_account_data["m.push_rules"] = await self.push_rules_for_user(
sync_config.user
)
clokep marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -1815,7 +1817,7 @@ async def _generate_sync_entry_for_presence(
async def _generate_sync_entry_for_rooms(
self,
sync_result_builder: "SyncResultBuilder",
account_data_by_room: Dict[str, Dict[str, JsonDict]],
account_data_by_room: Mapping[str, Mapping[str, JsonDict]],
) -> Tuple[AbstractSet[str], AbstractSet[str], AbstractSet[str], AbstractSet[str]]:
"""Generates the rooms portion of the sync response. Populates the
`sync_result_builder` with the result.
Expand Down Expand Up @@ -2284,8 +2286,8 @@ async def _generate_room_entry(
sync_result_builder: "SyncResultBuilder",
room_builder: "RoomSyncResultBuilder",
ephemeral: List[JsonDict],
tags: Optional[Dict[str, Dict[str, Any]]],
account_data: Dict[str, JsonDict],
tags: Optional[Mapping[str, Mapping[str, Any]]],
account_data: Mapping[str, JsonDict],
always_include: bool = False,
) -> None:
"""Populates the `joined` and `archived` section of `sync_result_builder`
Expand Down
Loading