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

Commit

Permalink
Stabilize support for MSC3970.
Browse files Browse the repository at this point in the history
  • Loading branch information
clokep committed May 18, 2023
1 parent 5dc1f25 commit be63aff
Show file tree
Hide file tree
Showing 8 changed files with 8 additions and 36 deletions.
1 change: 1 addition & 0 deletions changelog.d/15629.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Scope transaction IDs to devices (implement [MSC3970](https://github.com/matrix-org/matrix-spec-proposals/pull/3970)).
3 changes: 0 additions & 3 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
"msc3981_recurse_relations", False
)

# MSC3970: Scope transaction IDs to devices
self.msc3970_enabled = experimental.get("msc3970_enabled", False)

# MSC4009: E.164 Matrix IDs
self.msc4009_e164_mxids = experimental.get("msc4009_e164_mxids", False)

Expand Down
13 changes: 2 additions & 11 deletions synapse/events/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,16 +361,13 @@ def serialize_event(
time_now_ms: int,
*,
config: SerializeEventConfig = _DEFAULT_SERIALIZE_EVENT_CONFIG,
msc3970_enabled: bool = False,
) -> JsonDict:
"""Serialize event for clients
Args:
e
time_now_ms
config: Event serialization config
msc3970_enabled: Whether MSC3970 is enabled. It changes whether we should
include the `transaction_id` in the event's `unsigned` section.
Returns:
The serialized event dictionary.
Expand All @@ -396,7 +393,6 @@ def serialize_event(
e.unsigned["redacted_because"],
time_now_ms,
config=config,
msc3970_enabled=msc3970_enabled,
)

# If we have a txn_id saved in the internal_metadata, we should include it in the
Expand All @@ -408,7 +404,7 @@ def serialize_event(
# event internal metadata. Since we were not recording them before, if it hasn't
# been recorded, we fallback to the old behaviour.
event_device_id: Optional[str] = getattr(e.internal_metadata, "device_id", None)
if msc3970_enabled and event_device_id is not None:
if event_device_id is not None:
if event_device_id == config.requester.device_id:
d["unsigned"]["transaction_id"] = txn_id

Expand Down Expand Up @@ -460,9 +456,6 @@ class EventClientSerializer:
clients.
"""

def __init__(self, *, msc3970_enabled: bool = False):
self._msc3970_enabled = msc3970_enabled

def serialize_event(
self,
event: Union[JsonDict, EventBase],
Expand All @@ -487,9 +480,7 @@ def serialize_event(
if not isinstance(event, EventBase):
return event

serialized_event = serialize_event(
event, time_now, config=config, msc3970_enabled=self._msc3970_enabled
)
serialized_event = serialize_event(event, time_now, config=config)

# Check if there are any bundled aggregations to include with the event.
if bundle_aggregations:
Expand Down
4 changes: 1 addition & 3 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -560,8 +560,6 @@ def __init__(self, hs: "HomeServer"):
expiry_ms=30 * 60 * 1000,
)

self._msc3970_enabled = hs.config.experimental.msc3970_enabled

async def create_event(
self,
requester: Requester,
Expand Down Expand Up @@ -906,7 +904,7 @@ async def get_event_from_transaction(
An event if one could be found, None otherwise.
"""

if self._msc3970_enabled and requester.device_id:
if requester.device_id:
# When MSC3970 is enabled, we lookup for events sent by the same device first,
# and fallback to the old behaviour if none were found.
existing_event_id = (
Expand Down
4 changes: 1 addition & 3 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,6 @@ def __init__(self, hs: "HomeServer"):
self.request_ratelimiter = hs.get_request_ratelimiter()
hs.get_notifier().add_new_join_in_room_callback(self._on_user_joined_room)

self._msc3970_enabled = hs.config.experimental.msc3970_enabled

def _on_user_joined_room(self, event_id: str, room_id: str) -> None:
"""Notify the rate limiter that a room join has occurred.
Expand Down Expand Up @@ -424,7 +422,7 @@ async def _local_membership_update(
# do it up front for efficiency.)
if txn_id:
existing_event_id = None
if self._msc3970_enabled and requester.device_id:
if requester.device_id:
# When MSC3970 is enabled, we lookup for events sent by the same device
# first, and fallback to the old behaviour if none were found.
existing_event_id = (
Expand Down
11 changes: 1 addition & 10 deletions synapse/rest/client/transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ def __init__(self, hs: "HomeServer"):
# for at *LEAST* 30 mins, and at *MOST* 60 mins.
self.cleaner = self.clock.looping_call(self._cleanup, CLEANUP_PERIOD_MS)

self._msc3970_enabled = hs.config.experimental.msc3970_enabled

def _get_transaction_key(self, request: IRequest, requester: Requester) -> Hashable:
"""A helper function which returns a transaction key that can be used
with TransactionCache for idempotent requests.
Expand Down Expand Up @@ -79,18 +77,11 @@ def _get_transaction_key(self, request: IRequest, requester: Requester) -> Hasha
return (path, "appservice", requester.app_service.id)

# With MSC3970, we use the user ID and device ID as the transaction key
elif self._msc3970_enabled:
else:
assert requester.user, "Requester must have a user"
assert requester.device_id, "Requester must have a device_id"
return (path, "user", requester.user, requester.device_id)

# Otherwise, the pre-MSC3970 behaviour is to use the access token ID
else:
assert (
requester.access_token_id is not None
), "Requester must have an access_token_id"
return (path, "user", requester.access_token_id)

def fetch_or_execute_request(
self,
request: IRequest,
Expand Down
4 changes: 1 addition & 3 deletions synapse/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -777,9 +777,7 @@ def get_oidc_handler(self) -> "OidcHandler":

@cache_in_self
def get_event_client_serializer(self) -> EventClientSerializer:
return EventClientSerializer(
msc3970_enabled=self.config.experimental.msc3970_enabled
)
return EventClientSerializer()

@cache_in_self
def get_password_policy_handler(self) -> PasswordPolicyHandler:
Expand Down
4 changes: 1 addition & 3 deletions synapse/storage/databases/main/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,6 @@ def __init__(
self._backfill_id_gen: AbstractStreamIdGenerator = self.store._backfill_id_gen
self._stream_id_gen: AbstractStreamIdGenerator = self.store._stream_id_gen

self._msc3970_enabled = hs.config.experimental.msc3970_enabled

@trace
async def _persist_events_and_state_updates(
self,
Expand Down Expand Up @@ -1033,7 +1031,7 @@ def _persist_transaction_ids_txn(
# With MSC3970, we rely on the device_id instead to scope the txn_id for events.
# We're only inserting if MSC3970 is *enabled*, because else the pre-MSC3970
# behaviour would allow for a UNIQUE constraint violation on this table
if to_insert_device_id and self._msc3970_enabled:
if to_insert_device_id:
self.db_pool.simple_insert_many_txn(
txn,
table="event_txn_id_device_id",
Expand Down

0 comments on commit be63aff

Please sign in to comment.