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

Commit

Permalink
Remove unused room_alias field from /createRoom response (#15093)
Browse files Browse the repository at this point in the history
* Change `create_room` return type

* Don't return room alias from /createRoom

* Update other callsites

* Fix up mypy complaints

It looks like new_room_user_id is None iff new_room_id is None. It's a
shame we haven't expressed this in a way that mypy can understand.

* Changelog
  • Loading branch information
David Robertson authored Feb 22, 2023
1 parent 8219525 commit 647ff3e
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 39 deletions.
1 change: 1 addition & 0 deletions changelog.d/15093.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove the unspecced `room_alias` field from the [`/createRoom`](https://spec.matrix.org/v1.6/client-server-api/#post_matrixclientv3createroom) response.
4 changes: 2 additions & 2 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ async def _create_and_join_rooms(self, user_id: str) -> None:
# create room expects the localpart of the room alias
config["room_alias_name"] = room_alias.localpart

info, _ = await room_creation_handler.create_room(
room_id, _, _ = await room_creation_handler.create_room(
fake_requester,
config=config,
ratelimit=False,
Expand All @@ -490,7 +490,7 @@ async def _create_and_join_rooms(self, user_id: str) -> None:
user_id, authenticated_entity=self._server_name
),
target=UserID.from_string(user_id),
room_id=info["room_id"],
room_id=room_id,
# Since it was just created, there are no remote hosts.
remote_room_hosts=[],
action="join",
Expand Down
38 changes: 19 additions & 19 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -690,13 +690,14 @@ async def create_room(
config: JsonDict,
ratelimit: bool = True,
creator_join_profile: Optional[JsonDict] = None,
) -> Tuple[dict, int]:
) -> Tuple[str, Optional[RoomAlias], int]:
"""Creates a new room.
Args:
requester:
The user who requested the room creation.
config : A dict of configuration options.
requester: The user who requested the room creation.
config: A dict of configuration options. This will be the body of
a /createRoom request; see
https://spec.matrix.org/latest/client-server-api/#post_matrixclientv3createroom
ratelimit: set to False to disable the rate limiter
creator_join_profile:
Expand All @@ -707,14 +708,17 @@ async def create_room(
`avatar_url` and/or `displayname`.
Returns:
First, a dict containing the keys `room_id` and, if an alias
was, requested, `room_alias`. Secondly, the stream_id of the
last persisted event.
A 3-tuple containing:
- the room ID;
- if requested, the room alias, otherwise None; and
- the `stream_id` of the last persisted event.
Raises:
SynapseError if the room ID couldn't be stored, 3pid invitation config
validation failed, or something went horribly wrong.
ResourceLimitError if server is blocked to some resource being
exceeded
SynapseError:
if the room ID couldn't be stored, 3pid invitation config
validation failed, or something went horribly wrong.
ResourceLimitError:
if server is blocked to some resource being
exceeded
"""
user_id = requester.user.to_string()

Expand Down Expand Up @@ -1024,19 +1028,14 @@ async def create_room(
last_sent_event_id = member_event_id
depth += 1

result = {"room_id": room_id}

if room_alias:
result["room_alias"] = room_alias.to_string()

# Always wait for room creation to propagate before returning
await self._replication.wait_for_stream_position(
self.hs.config.worker.events_shard_config.get_instance(room_id),
"events",
last_stream_id,
)

return result, last_stream_id
return room_id, room_alias, last_stream_id

async def _send_events_for_new_room(
self,
Expand Down Expand Up @@ -1825,7 +1824,7 @@ async def shutdown_room(
new_room_user_id, authenticated_entity=requester_user_id
)

info, stream_id = await self._room_creation_handler.create_room(
new_room_id, _, stream_id = await self._room_creation_handler.create_room(
room_creator_requester,
config={
"preset": RoomCreationPreset.PUBLIC_CHAT,
Expand All @@ -1834,7 +1833,6 @@ async def shutdown_room(
},
ratelimit=False,
)
new_room_id = info["room_id"]

logger.info(
"Shutting down room %r, joining to new room: %r", room_id, new_room_id
Expand Down Expand Up @@ -1887,6 +1885,7 @@ async def shutdown_room(

# Join users to new room
if new_room_user_id:
assert new_room_id is not None
await self.room_member_handler.update_membership(
requester=target_requester,
target=target_requester.user,
Expand Down Expand Up @@ -1919,6 +1918,7 @@ async def shutdown_room(

aliases_for_room = await self.store.get_aliases_for_room(room_id)

assert new_room_id is not None
await self.store.update_aliases_for_room(
room_id, new_room_id, requester_user_id
)
Expand Down
6 changes: 3 additions & 3 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1576,14 +1576,14 @@ async def create_room(
)

requester = create_requester(user_id)
room_id_and_alias, _ = await self._hs.get_room_creation_handler().create_room(
room_id, room_alias, _ = await self._hs.get_room_creation_handler().create_room(
requester=requester,
config=config,
ratelimit=ratelimit,
creator_join_profile=creator_join_profile,
)

return room_id_and_alias["room_id"], room_id_and_alias.get("room_alias", None)
room_alias_str = room_alias.to_string() if room_alias else None
return room_id, room_alias_str

async def set_displayname(
self,
Expand Down
4 changes: 2 additions & 2 deletions synapse/rest/client/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,11 @@ def on_PUT(
async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request)

info, _ = await self._room_creation_handler.create_room(
room_id, _, _ = await self._room_creation_handler.create_room(
requester, self.get_room_config(request)
)

return 200, info
return 200, {"room_id": room_id}

def get_room_config(self, request: Request) -> JsonDict:
user_supplied_config = parse_json_object_from_request(request)
Expand Down
3 changes: 1 addition & 2 deletions synapse/server_notices/server_notices_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ async def get_or_create_notice_room_for_user(self, user_id: str) -> str:
"avatar_url": self._config.servernotices.server_notices_mxid_avatar_url,
}

info, _ = await self._room_creation_handler.create_room(
room_id, _, _ = await self._room_creation_handler.create_room(
requester,
config={
"preset": RoomCreationPreset.PRIVATE_CHAT,
Expand All @@ -188,7 +188,6 @@ async def get_or_create_notice_room_for_user(self, user_id: str) -> str:
ratelimit=False,
creator_join_profile=join_profile,
)
room_id = info["room_id"]

self.maybe_get_notice_room_for_user.invalidate((user_id,))

Expand Down
8 changes: 4 additions & 4 deletions tests/storage/test_cleanup_extrems.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ def prepare(
# Create a test user and room
self.user = UserID("alice", "test")
self.requester = create_requester(self.user)
info, _ = self.get_success(self.room_creator.create_room(self.requester, {}))
self.room_id = info["room_id"]
self.room_id, _, _ = self.get_success(
self.room_creator.create_room(self.requester, {})
)

def run_background_update(self) -> None:
"""Re run the background update to clean up the extremities."""
Expand Down Expand Up @@ -275,10 +276,9 @@ def prepare(
self.user = UserID.from_string(self.register_user("user1", "password"))
self.token1 = self.login("user1", "password")
self.requester = create_requester(self.user)
info, _ = self.get_success(
self.room_id, _, _ = self.get_success(
self.room_creator.create_room(self.requester, {"visibility": "public"})
)
self.room_id = info["room_id"]
self.event_creator = homeserver.get_event_creation_handler()
homeserver.config.consent.user_consent_version = self.CONSENT_VERSION

Expand Down
3 changes: 1 addition & 2 deletions tests/storage/test_event_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ def test_exposed_to_prometheus(self) -> None:
events = [(3, 2), (6, 2), (4, 6)]

for event_count, extrems in events:
info, _ = self.get_success(room_creator.create_room(requester, {}))
room_id = info["room_id"]
room_id, _, _ = self.get_success(room_creator.create_room(requester, {}))

last_event = None

Expand Down
10 changes: 6 additions & 4 deletions tests/storage/test_receipts.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,14 @@ def prepare(
self.otherRequester = create_requester(self.otherUser)

# Create a test room
info, _ = self.get_success(self.room_creator.create_room(self.ourRequester, {}))
self.room_id1 = info["room_id"]
self.room_id1, _, _ = self.get_success(
self.room_creator.create_room(self.ourRequester, {})
)

# Create a second test room
info, _ = self.get_success(self.room_creator.create_room(self.ourRequester, {}))
self.room_id2 = info["room_id"]
self.room_id2, _, _ = self.get_success(
self.room_creator.create_room(self.ourRequester, {})
)

# Join the second user to the first room
memberEvent, memberEventContext = self.get_success(
Expand Down
2 changes: 1 addition & 1 deletion tests/test_federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
room_creator.create_room(
our_user, room_creator._presets_dict["public_chat"], ratelimit=False
)
)[0]["room_id"]
)[0]

self.store = self.hs.get_datastores().main

Expand Down

0 comments on commit 647ff3e

Please sign in to comment.