Stop sending events when creating or deleting aliases (#6904)
Stop sending events when creating or deleting associations (room aliases). Send an updated canonical alias event if one of the alt_aliases is deleted.pull/6945/head
parent
3be2abd0a9
commit
fe3941f6e3
|
@ -0,0 +1 @@
|
||||||
|
Stop sending alias events during adding / removing aliases. Check alt_aliases in the latest canonical aliases event when deleting an alias.
|
|
@ -81,13 +81,7 @@ class DirectoryHandler(BaseHandler):
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def create_association(
|
def create_association(
|
||||||
self,
|
self, requester, room_alias, room_id, servers=None, check_membership=True,
|
||||||
requester,
|
|
||||||
room_alias,
|
|
||||||
room_id,
|
|
||||||
servers=None,
|
|
||||||
send_event=True,
|
|
||||||
check_membership=True,
|
|
||||||
):
|
):
|
||||||
"""Attempt to create a new alias
|
"""Attempt to create a new alias
|
||||||
|
|
||||||
|
@ -97,7 +91,6 @@ class DirectoryHandler(BaseHandler):
|
||||||
room_id (str)
|
room_id (str)
|
||||||
servers (list[str]|None): List of servers that others servers
|
servers (list[str]|None): List of servers that others servers
|
||||||
should try and join via
|
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
|
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).
|
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)
|
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
|
@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
|
"""Remove an alias from the directory
|
||||||
|
|
||||||
(this is only meant for human users; AS users should call
|
(this is only meant for human users; AS users should call
|
||||||
|
@ -168,9 +154,6 @@ class DirectoryHandler(BaseHandler):
|
||||||
Args:
|
Args:
|
||||||
requester (Requester):
|
requester (Requester):
|
||||||
room_alias (RoomAlias):
|
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:
|
Returns:
|
||||||
Deferred[unicode]: room id that the alias used to point to
|
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)
|
room_id = yield self._delete_association(room_alias)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
if send_event:
|
|
||||||
yield self.send_room_alias_update_event(requester, room_id)
|
|
||||||
|
|
||||||
yield self._update_canonical_alias(
|
yield self._update_canonical_alias(
|
||||||
requester, requester.user.to_string(), room_id, room_alias
|
requester, requester.user.to_string(), room_id, room_alias
|
||||||
)
|
)
|
||||||
|
@ -319,25 +299,50 @@ class DirectoryHandler(BaseHandler):
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def _update_canonical_alias(self, requester, user_id, room_id, room_alias):
|
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(
|
alias_event = yield self.state.get_current_state(
|
||||||
room_id, EventTypes.CanonicalAlias, ""
|
room_id, EventTypes.CanonicalAlias, ""
|
||||||
)
|
)
|
||||||
|
|
||||||
alias_str = room_alias.to_string()
|
# There is no canonical alias, nothing to do.
|
||||||
if not alias_event or alias_event.content.get("alias", "") != alias_str:
|
if not alias_event:
|
||||||
return
|
return
|
||||||
|
|
||||||
yield self.event_creation_handler.create_and_send_nonmember_event(
|
# Obtain a mutable version of the event content.
|
||||||
requester,
|
content = dict(alias_event.content)
|
||||||
{
|
send_update = False
|
||||||
"type": EventTypes.CanonicalAlias,
|
|
||||||
"state_key": "",
|
# Remove the alias property if it matches the removed alias.
|
||||||
"room_id": room_id,
|
alias_str = room_alias.to_string()
|
||||||
"sender": user_id,
|
if alias_event.content.get("alias", "") == alias_str:
|
||||||
"content": {},
|
send_update = True
|
||||||
},
|
content.pop("alias", "")
|
||||||
ratelimit=False,
|
|
||||||
)
|
# 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
|
@defer.inlineCallbacks
|
||||||
def get_association_from_room_alias(self, room_alias):
|
def get_association_from_room_alias(self, room_alias):
|
||||||
|
|
|
@ -478,9 +478,7 @@ class RoomCreationHandler(BaseHandler):
|
||||||
for alias_str in aliases:
|
for alias_str in aliases:
|
||||||
alias = RoomAlias.from_string(alias_str)
|
alias = RoomAlias.from_string(alias_str)
|
||||||
try:
|
try:
|
||||||
yield directory_handler.delete_association(
|
yield directory_handler.delete_association(requester, alias)
|
||||||
requester, alias, send_event=False
|
|
||||||
)
|
|
||||||
removed_aliases.append(alias_str)
|
removed_aliases.append(alias_str)
|
||||||
except SynapseError as e:
|
except SynapseError as e:
|
||||||
logger.warning("Unable to remove alias %s from old room: %s", alias, 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),
|
RoomAlias.from_string(alias),
|
||||||
new_room_id,
|
new_room_id,
|
||||||
servers=(self.hs.hostname,),
|
servers=(self.hs.hostname,),
|
||||||
send_event=False,
|
|
||||||
check_membership=False,
|
check_membership=False,
|
||||||
)
|
)
|
||||||
logger.info("Moved alias %s to new room", alias)
|
logger.info("Moved alias %s to new room", alias)
|
||||||
|
@ -664,7 +661,6 @@ class RoomCreationHandler(BaseHandler):
|
||||||
room_id=room_id,
|
room_id=room_id,
|
||||||
room_alias=room_alias,
|
room_alias=room_alias,
|
||||||
servers=[self.hs.hostname],
|
servers=[self.hs.hostname],
|
||||||
send_event=False,
|
|
||||||
check_membership=False,
|
check_membership=False,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
|
@ -18,9 +18,11 @@ from mock import Mock
|
||||||
|
|
||||||
from twisted.internet import defer
|
from twisted.internet import defer
|
||||||
|
|
||||||
|
import synapse.api.errors
|
||||||
|
from synapse.api.constants import EventTypes
|
||||||
from synapse.config.room_directory import RoomDirectoryConfig
|
from synapse.config.room_directory import RoomDirectoryConfig
|
||||||
from synapse.rest.client.v1 import directory, room
|
from synapse.rest.client.v1 import directory, login, room
|
||||||
from synapse.types import RoomAlias
|
from synapse.types import RoomAlias, create_requester
|
||||||
|
|
||||||
from tests import unittest
|
from tests import unittest
|
||||||
|
|
||||||
|
@ -85,6 +87,38 @@ class DirectoryTestCase(unittest.HomeserverTestCase):
|
||||||
ignore_backoff=True,
|
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):
|
def test_incoming_fed_query(self):
|
||||||
self.get_success(
|
self.get_success(
|
||||||
self.store.create_room_alias_association(
|
self.store.create_room_alias_association(
|
||||||
|
@ -99,6 +133,122 @@ class DirectoryTestCase(unittest.HomeserverTestCase):
|
||||||
self.assertEquals({"room_id": "!8765asdf:test", "servers": ["test"]}, response)
|
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):
|
class TestCreateAliasACL(unittest.HomeserverTestCase):
|
||||||
user_id = "@test:test"
|
user_id = "@test:test"
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue