diff --git a/changelog.d/6904.removal b/changelog.d/6904.removal
new file mode 100644
index 0000000000..a5cc0c3605
--- /dev/null
+++ b/changelog.d/6904.removal
@@ -0,0 +1 @@
+Stop sending alias events during adding / removing aliases. Check alt_aliases in the latest canonical aliases event when deleting an alias.
diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py
index 8c5980cb0c..f718388884 100644
--- a/synapse/handlers/directory.py
+++ b/synapse/handlers/directory.py
@@ -81,13 +81,7 @@ class DirectoryHandler(BaseHandler):
@defer.inlineCallbacks
def create_association(
- self,
- requester,
- room_alias,
- room_id,
- servers=None,
- send_event=True,
- check_membership=True,
+ self, requester, room_alias, room_id, servers=None, check_membership=True,
):
"""Attempt to create a new alias
@@ -97,7 +91,6 @@ class DirectoryHandler(BaseHandler):
room_id (str)
servers (list[str]|None): List of servers that others servers
should try and join via
- send_event (bool): Whether to send an updated m.room.aliases event
check_membership (bool): Whether to check if the user is in the room
before the alias can be set (if the server's config requires it).
@@ -150,16 +143,9 @@ class DirectoryHandler(BaseHandler):
)
yield self._create_association(room_alias, room_id, servers, creator=user_id)
- if send_event:
- try:
- yield self.send_room_alias_update_event(requester, room_id)
- except AuthError as e:
- # sending the aliases event may fail due to the user not having
- # permission in the room; this is permitted.
- logger.info("Skipping updating aliases event due to auth error %s", e)
@defer.inlineCallbacks
- def delete_association(self, requester, room_alias, send_event=True):
+ def delete_association(self, requester, room_alias):
"""Remove an alias from the directory
(this is only meant for human users; AS users should call
@@ -168,9 +154,6 @@ class DirectoryHandler(BaseHandler):
Args:
requester (Requester):
room_alias (RoomAlias):
- send_event (bool): Whether to send an updated m.room.aliases event.
- Note that, if we delete the canonical alias, we will always attempt
- to send an m.room.canonical_alias event
Returns:
Deferred[unicode]: room id that the alias used to point to
@@ -206,9 +189,6 @@ class DirectoryHandler(BaseHandler):
room_id = yield self._delete_association(room_alias)
try:
- if send_event:
- yield self.send_room_alias_update_event(requester, room_id)
-
yield self._update_canonical_alias(
requester, requester.user.to_string(), room_id, room_alias
)
@@ -319,25 +299,50 @@ class DirectoryHandler(BaseHandler):
@defer.inlineCallbacks
def _update_canonical_alias(self, requester, user_id, room_id, room_alias):
+ """
+ Send an updated canonical alias event if the removed alias was set as
+ the canonical alias or listed in the alt_aliases field.
+ """
alias_event = yield self.state.get_current_state(
room_id, EventTypes.CanonicalAlias, ""
)
- alias_str = room_alias.to_string()
- if not alias_event or alias_event.content.get("alias", "") != alias_str:
+ # There is no canonical alias, nothing to do.
+ if not alias_event:
return
- yield self.event_creation_handler.create_and_send_nonmember_event(
- requester,
- {
- "type": EventTypes.CanonicalAlias,
- "state_key": "",
- "room_id": room_id,
- "sender": user_id,
- "content": {},
- },
- ratelimit=False,
- )
+ # Obtain a mutable version of the event content.
+ content = dict(alias_event.content)
+ send_update = False
+
+ # Remove the alias property if it matches the removed alias.
+ alias_str = room_alias.to_string()
+ if alias_event.content.get("alias", "") == alias_str:
+ send_update = True
+ content.pop("alias", "")
+
+ # Filter alt_aliases for the removed alias.
+ alt_aliases = content.pop("alt_aliases", None)
+ # If the aliases are not a list (or not found) do not attempt to modify
+ # the list.
+ if isinstance(alt_aliases, list):
+ send_update = True
+ alt_aliases = [alias for alias in alt_aliases if alias != alias_str]
+ if alt_aliases:
+ content["alt_aliases"] = alt_aliases
+
+ if send_update:
+ yield self.event_creation_handler.create_and_send_nonmember_event(
+ requester,
+ {
+ "type": EventTypes.CanonicalAlias,
+ "state_key": "",
+ "room_id": room_id,
+ "sender": user_id,
+ "content": content,
+ },
+ ratelimit=False,
+ )
@defer.inlineCallbacks
def get_association_from_room_alias(self, room_alias):
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 033083acac..49ec2f48bc 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -478,9 +478,7 @@ class RoomCreationHandler(BaseHandler):
for alias_str in aliases:
alias = RoomAlias.from_string(alias_str)
try:
- yield directory_handler.delete_association(
- requester, alias, send_event=False
- )
+ yield directory_handler.delete_association(requester, alias)
removed_aliases.append(alias_str)
except SynapseError as e:
logger.warning("Unable to remove alias %s from old room: %s", alias, e)
@@ -511,7 +509,6 @@ class RoomCreationHandler(BaseHandler):
RoomAlias.from_string(alias),
new_room_id,
servers=(self.hs.hostname,),
- send_event=False,
check_membership=False,
)
logger.info("Moved alias %s to new room", alias)
@@ -664,7 +661,6 @@ class RoomCreationHandler(BaseHandler):
room_id=room_id,
room_alias=room_alias,
servers=[self.hs.hostname],
- send_event=False,
check_membership=False,
)
diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py
index ee88cf5a4b..27b916aed4 100644
--- a/tests/handlers/test_directory.py
+++ b/tests/handlers/test_directory.py
@@ -18,9 +18,11 @@ from mock import Mock
from twisted.internet import defer
+import synapse.api.errors
+from synapse.api.constants import EventTypes
from synapse.config.room_directory import RoomDirectoryConfig
-from synapse.rest.client.v1 import directory, room
-from synapse.types import RoomAlias
+from synapse.rest.client.v1 import directory, login, room
+from synapse.types import RoomAlias, create_requester
from tests import unittest
@@ -85,6 +87,38 @@ class DirectoryTestCase(unittest.HomeserverTestCase):
ignore_backoff=True,
)
+ def test_delete_alias_not_allowed(self):
+ room_id = "!8765qwer:test"
+ self.get_success(
+ self.store.create_room_alias_association(self.my_room, room_id, ["test"])
+ )
+
+ self.get_failure(
+ self.handler.delete_association(
+ create_requester("@user:test"), self.my_room
+ ),
+ synapse.api.errors.AuthError,
+ )
+
+ def test_delete_alias(self):
+ room_id = "!8765qwer:test"
+ user_id = "@user:test"
+ self.get_success(
+ self.store.create_room_alias_association(
+ self.my_room, room_id, ["test"], user_id
+ )
+ )
+
+ result = self.get_success(
+ self.handler.delete_association(create_requester(user_id), self.my_room)
+ )
+ self.assertEquals(room_id, result)
+
+ # The alias should not be found.
+ self.get_failure(
+ self.handler.get_association(self.my_room), synapse.api.errors.SynapseError
+ )
+
def test_incoming_fed_query(self):
self.get_success(
self.store.create_room_alias_association(
@@ -99,6 +133,122 @@ class DirectoryTestCase(unittest.HomeserverTestCase):
self.assertEquals({"room_id": "!8765asdf:test", "servers": ["test"]}, response)
+class CanonicalAliasTestCase(unittest.HomeserverTestCase):
+ """Test modifications of the canonical alias when delete aliases.
+ """
+
+ servlets = [
+ synapse.rest.admin.register_servlets,
+ login.register_servlets,
+ room.register_servlets,
+ directory.register_servlets,
+ ]
+
+ def prepare(self, reactor, clock, hs):
+ self.store = hs.get_datastore()
+ self.handler = hs.get_handlers().directory_handler
+ self.state_handler = hs.get_state_handler()
+
+ # Create user
+ self.admin_user = self.register_user("admin", "pass", admin=True)
+ self.admin_user_tok = self.login("admin", "pass")
+
+ # Create a test room
+ self.room_id = self.helper.create_room_as(
+ self.admin_user, tok=self.admin_user_tok
+ )
+
+ self.test_alias = "#test:test"
+ self.room_alias = RoomAlias.from_string(self.test_alias)
+
+ # Create a new alias to this room.
+ self.get_success(
+ self.store.create_room_alias_association(
+ self.room_alias, self.room_id, ["test"], self.admin_user
+ )
+ )
+
+ def test_remove_alias(self):
+ """Removing an alias that is the canonical alias should remove it there too."""
+ # Set this new alias as the canonical alias for this room
+ self.helper.send_state(
+ self.room_id,
+ "m.room.canonical_alias",
+ {"alias": self.test_alias, "alt_aliases": [self.test_alias]},
+ tok=self.admin_user_tok,
+ )
+
+ data = self.get_success(
+ self.state_handler.get_current_state(
+ self.room_id, EventTypes.CanonicalAlias, ""
+ )
+ )
+ self.assertEqual(data["content"]["alias"], self.test_alias)
+ self.assertEqual(data["content"]["alt_aliases"], [self.test_alias])
+
+ # Finally, delete the alias.
+ self.get_success(
+ self.handler.delete_association(
+ create_requester(self.admin_user), self.room_alias
+ )
+ )
+
+ data = self.get_success(
+ self.state_handler.get_current_state(
+ self.room_id, EventTypes.CanonicalAlias, ""
+ )
+ )
+ self.assertNotIn("alias", data["content"])
+ self.assertNotIn("alt_aliases", data["content"])
+
+ def test_remove_other_alias(self):
+ """Removing an alias listed as in alt_aliases should remove it there too."""
+ # Create a second alias.
+ other_test_alias = "#test2:test"
+ other_room_alias = RoomAlias.from_string(other_test_alias)
+ self.get_success(
+ self.store.create_room_alias_association(
+ other_room_alias, self.room_id, ["test"], self.admin_user
+ )
+ )
+
+ # Set the alias as the canonical alias for this room.
+ self.helper.send_state(
+ self.room_id,
+ "m.room.canonical_alias",
+ {
+ "alias": self.test_alias,
+ "alt_aliases": [self.test_alias, other_test_alias],
+ },
+ tok=self.admin_user_tok,
+ )
+
+ data = self.get_success(
+ self.state_handler.get_current_state(
+ self.room_id, EventTypes.CanonicalAlias, ""
+ )
+ )
+ self.assertEqual(data["content"]["alias"], self.test_alias)
+ self.assertEqual(
+ data["content"]["alt_aliases"], [self.test_alias, other_test_alias]
+ )
+
+ # Delete the second alias.
+ self.get_success(
+ self.handler.delete_association(
+ create_requester(self.admin_user), other_room_alias
+ )
+ )
+
+ data = self.get_success(
+ self.state_handler.get_current_state(
+ self.room_id, EventTypes.CanonicalAlias, ""
+ )
+ )
+ self.assertEqual(data["content"]["alias"], self.test_alias)
+ self.assertEqual(data["content"]["alt_aliases"], [self.test_alias])
+
+
class TestCreateAliasACL(unittest.HomeserverTestCase):
user_id = "@test:test"
|
