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

Add type hints to handlers.message and events.builder #8067

Merged
merged 4 commits into from
Aug 12, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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/8067.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add type hints to `synapse.handlers.message` and `synapse.events.builder`.
3 changes: 3 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,6 @@ ignore_missing_imports = True

[mypy-rust_python_jaeger_reporter.*]
ignore_missing_imports = True

[mypy-nacl.*]
ignore_missing_imports = True
34 changes: 19 additions & 15 deletions synapse/events/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import attr
from nacl.signing import SigningKey

from synapse.api.auth import Auth
from synapse.api.constants import MAX_DEPTH
from synapse.api.errors import UnsupportedRoomVersionError
from synapse.api.room_versions import (
Expand All @@ -27,6 +28,8 @@
)
from synapse.crypto.event_signing import add_hashes_and_signatures
from synapse.events import EventBase, _EventInternalMetadata, make_event_from_dict
from synapse.state import StateHandler
from synapse.storage.databases.main import DataStore
from synapse.types import EventID, JsonDict
from synapse.util import Clock
from synapse.util.stringutils import random_string
Expand Down Expand Up @@ -57,30 +60,31 @@ class EventBuilder(object):
_signing_key: The signing key to use to sign the event as the server
"""

_state = attr.ib()
_auth = attr.ib()
_store = attr.ib()
_clock = attr.ib()
_hostname = attr.ib()
_signing_key = attr.ib()
_state = attr.ib(type=StateHandler)
clokep marked this conversation as resolved.
Show resolved Hide resolved
_auth = attr.ib(type=Auth)
_store = attr.ib(type=DataStore)
_clock = attr.ib(type=Clock)
_hostname = attr.ib(type=str)
_signing_key = attr.ib(type=SigningKey)

room_version = attr.ib(type=RoomVersion)

room_id = attr.ib()
type = attr.ib()
sender = attr.ib()
room_id = attr.ib(type=str)
type = attr.ib(type=str)
sender = attr.ib(type=str)

content = attr.ib(default=attr.Factory(dict))
unsigned = attr.ib(default=attr.Factory(dict))
content = attr.ib(default=attr.Factory(dict), type=JsonDict)
unsigned = attr.ib(default=attr.Factory(dict), type=JsonDict)

# These only exist on a subset of events, so they raise AttributeError if
# someone tries to get them when they don't exist.
_state_key = attr.ib(default=None)
_redacts = attr.ib(default=None)
_origin_server_ts = attr.ib(default=None)
_state_key = attr.ib(default=None, type=Optional[str])
_redacts = attr.ib(default=None, type=Optional[str])
_origin_server_ts = attr.ib(default=None, type=Optional[int])

internal_metadata = attr.ib(
default=attr.Factory(lambda: _EventInternalMetadata({}))
default=attr.Factory(lambda: _EventInternalMetadata({})),
type=_EventInternalMetadata,
)

@property
Expand Down
22 changes: 13 additions & 9 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from typing import TYPE_CHECKING, List, Optional, Tuple
from typing import TYPE_CHECKING, Dict, List, Optional, Tuple

from canonicaljson import encode_canonical_json, json

Expand Down Expand Up @@ -93,11 +93,11 @@ def __init__(self, hs):

async def get_room_data(
self,
user_id: str = None,
room_id: str = None,
event_type: Optional[str] = None,
state_key: str = "",
is_guest: bool = False,
user_id: str,
room_id: str,
event_type: str,
state_key: str,
is_guest: bool,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is called from one place, and it uses everything. The event_type isn't actually optional was what caused me to change these.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK FINE A COUPLE OF TESTS WERE USING IT BUT SHHHHHHH

) -> dict:
""" Get data from a room.

Expand Down Expand Up @@ -407,7 +407,7 @@ def __init__(self, hs: "HomeServer"):
#
# map from room id to time-of-last-attempt.
#
self._rooms_to_exclude_from_dummy_event_insertion = {} # type: dict[str, int]
self._rooms_to_exclude_from_dummy_event_insertion = {} # type: Dict[str, int]

# we need to construct a ConsentURIBuilder here, as it checks that the necessary
# config options, but *only* if we have a configuration for which we are
Expand Down Expand Up @@ -707,7 +707,7 @@ async def deduplicate_state_event(
async def create_and_send_nonmember_event(
self,
requester: Requester,
event_dict: EventBase,
event_dict: dict,
ratelimit: bool = True,
txn_id: Optional[str] = None,
) -> Tuple[EventBase, int]:
Expand Down Expand Up @@ -971,7 +971,7 @@ async def persist_and_notify_client_event(
# Validate a newly added alias or newly added alt_aliases.

original_alias = None
original_alt_aliases = set()
original_alt_aliases = [] # type: List[str]

original_event_id = event.unsigned.get("replaces_state")
if original_event_id:
Expand Down Expand Up @@ -1019,6 +1019,10 @@ def is_inviter_member_event(e):

current_state_ids = await context.get_current_state_ids()

# We know this event is not an outlier, so this must be
# non-None.
assert current_state_ids is not None

state_to_include_ids = [
e_id
for k, e_id in current_state_ids.items()
Expand Down
12 changes: 8 additions & 4 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import abc
import logging
from http import HTTPStatus
from typing import Dict, Iterable, List, Optional, Tuple, Union
from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, Tuple, Union

from unpaddedbase64 import encode_base64

Expand All @@ -37,6 +37,10 @@

from ._base import BaseHandler

if TYPE_CHECKING:
from synapse.server import HomeServer


logger = logging.getLogger(__name__)


Expand All @@ -48,7 +52,7 @@ class RoomMemberHandler(object):

__metaclass__ = abc.ABCMeta

def __init__(self, hs):
def __init__(self, hs: "HomeServer"):
self.hs = hs
self.store = hs.get_datastore()
self.auth = hs.get_auth()
Expand Down Expand Up @@ -207,7 +211,7 @@ async def _local_membership_update(
return duplicate.event_id, stream_id

stream_id = await self.event_creation_handler.handle_new_client_event(
requester, event, context, extra_users=[target], ratelimit=ratelimit
requester, event, context, extra_users=[target], ratelimit=ratelimit,
)

prev_state_ids = await context.get_prev_state_ids()
Expand Down Expand Up @@ -1000,7 +1004,7 @@ async def _remote_join(

check_complexity = self.hs.config.limit_remote_rooms.enabled
if check_complexity and self.hs.config.limit_remote_rooms.admins_can_join:
check_complexity = not await self.hs.auth.is_server_admin(user)
check_complexity = not await self.hs.get_auth().is_server_admin(user)
Copy link
Member

Choose a reason for hiding this comment

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

I think this can just be self.auth? Not really sure why this change was made though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes. I cheekily add the hs: "HomeServer" and "fixed" this without really thinking.

Copy link
Member

Choose a reason for hiding this comment

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

I guess I'm not sure how this is "fixing" something?

Copy link
Member Author

Choose a reason for hiding this comment

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

mypy doesn't know that hs.auth is a thing, as that gets magically set the first time someone calls hs.get_auth(), so now that it knows that self.hs is a HomeServer object it complains that it doesn't have a auth attribute

Copy link
Member

Choose a reason for hiding this comment

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

Ah! We seem to store this as self.auth in the __init__, should we just use that instead and avoid the property access?

Copy link
Member Author

Choose a reason for hiding this comment

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

Already done 😇

Copy link
Member

Choose a reason for hiding this comment

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

This would explain why this became outdated and I didn't realize it. 🤦


if check_complexity:
# Fetch the room complexity
Expand Down
2 changes: 2 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,15 @@ commands = mypy \
synapse/appservice \
synapse/config \
synapse/event_auth.py \
synapse/events/builder.py \
synapse/events/spamcheck.py \
synapse/federation \
synapse/handlers/auth.py \
synapse/handlers/cas_handler.py \
synapse/handlers/directory.py \
synapse/handlers/federation.py \
synapse/handlers/identity.py \
synapse/handlers/message.py \
synapse/handlers/oidc_handler.py \
synapse/handlers/presence.py \
synapse/handlers/room_member.py \
Expand Down