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

Migrate direct message and tag state on room upgrade #4412

Merged
merged 23 commits into from
Jan 28, 2019
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
75942af
Fix typo
anoadragon453 Jan 17, 2019
4ff6d22
Preserve DM status of a room on upgrade
anoadragon453 Jan 17, 2019
1f18c7c
Add changelog
anoadragon453 Jan 17, 2019
887ca93
Prevent crash on user who doesn't have any direct rooms
anoadragon453 Jan 17, 2019
ea8903f
Migrating dm and room tags work for migrator
anoadragon453 Jan 18, 2019
25d64a8
Fix typos
anoadragon453 Jan 18, 2019
8c85f08
tags, m.direct copying over correctly
anoadragon453 Jan 18, 2019
48951f4
Join logic covers both room creator and arbitrary users
anoadragon453 Jan 22, 2019
8086a5c
Fix comments
anoadragon453 Jan 22, 2019
c4875d8
Prevent duplicate room IDs in m.direct
anoadragon453 Jan 22, 2019
117bc94
Merge branch 'develop' of github.com:matrix-org/synapse into anoa/dm_…
anoadragon453 Jan 22, 2019
766a172
lint
anoadragon453 Jan 22, 2019
0862d35
Move tag and direct state copying into separate function
anoadragon453 Jan 25, 2019
821b65a
Merge branch 'develop' of github.com:matrix-org/synapse into anoa/dm_…
anoadragon453 Jan 25, 2019
6f3fda7
Move room_tag declaration to be closer to its use
anoadragon453 Jan 25, 2019
516456b
Remove unnecessary null check
anoadragon453 Jan 25, 2019
c4cdafa
Destructure account data tuple before use
anoadragon453 Jan 25, 2019
8265995
Use python magic
anoadragon453 Jan 25, 2019
da0d221
Clean up direct_rooms access
anoadragon453 Jan 25, 2019
9244a30
Fixes
anoadragon453 Jan 25, 2019
1ce4639
Reuse predecessor method
anoadragon453 Jan 28, 2019
4026d55
Merge branch 'develop' of github.com:matrix-org/synapse into anoa/dm_…
anoadragon453 Jan 28, 2019
f0e96ab
Change return syntax in doc string
anoadragon453 Jan 28, 2019
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/4412.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Copy over whether a room is a direct message and any associated room tags on room upgrade.
5 changes: 3 additions & 2 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def upgrade_room(self, requester, old_room_id, new_version):
)
yield self.auth.check_from_context(tombstone_event, tombstone_context)

yield self.clone_exiting_room(
yield self.clone_existing_room(
requester,
old_room_id=old_room_id,
new_room_id=new_room_id,
Expand Down Expand Up @@ -230,7 +230,7 @@ def _update_upgraded_room_pls(
)

@defer.inlineCallbacks
def clone_exiting_room(
def clone_existing_room(
self, requester, old_room_id, new_room_id, new_room_version,
tombstone_event_id,
):
Expand Down Expand Up @@ -262,6 +262,7 @@ def clone_exiting_room(

initial_state = dict()

# Replicate relevant room events
types_to_copy = (
(EventTypes.JoinRules, ""),
(EventTypes.Name, ""),
Expand Down
89 changes: 78 additions & 11 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import synapse.types
from synapse.api.constants import EventTypes, Membership
from synapse.api.errors import AuthError, Codes, SynapseError
from synapse.storage.state import StateFilter
from synapse.types import RoomID, UserID
from synapse.util.async_helpers import Linearizer
from synapse.util.distributor import user_joined_room, user_left_room
Expand Down Expand Up @@ -63,7 +64,7 @@ def __init__(self, hs):
self.directory_handler = hs.get_handlers().directory_handler
self.registration_handler = hs.get_handlers().registration_handler
self.profile_handler = hs.get_profile_handler()
self.event_creation_hander = hs.get_event_creation_handler()
self.event_creation_handler = hs.get_event_creation_handler()

self.member_linearizer = Linearizer(name="member")

Expand Down Expand Up @@ -161,21 +162,23 @@ def _local_membership_update(
ratelimit=True,
content=None,
):
user_id = target.to_string()

if content is None:
content = {}

content["membership"] = membership
if requester.is_guest:
content["kind"] = "guest"

event, context = yield self.event_creation_hander.create_event(
event, context = yield self.event_creation_handler.create_event(
requester,
{
"type": EventTypes.Member,
"content": content,
"room_id": room_id,
"sender": requester.user.to_string(),
"state_key": target.to_string(),
"state_key": user_id,

# For backwards compatibility:
"membership": membership,
Expand All @@ -186,14 +189,14 @@ def _local_membership_update(
)

# Check if this event matches the previous membership event for the user.
duplicate = yield self.event_creation_hander.deduplicate_state_event(
duplicate = yield self.event_creation_handler.deduplicate_state_event(
event, context,
)
if duplicate is not None:
# Discard the new event since this membership change is a no-op.
defer.returnValue(duplicate)

yield self.event_creation_hander.handle_new_client_event(
yield self.event_creation_handler.handle_new_client_event(
requester,
event,
context,
Expand All @@ -204,12 +207,12 @@ def _local_membership_update(
prev_state_ids = yield context.get_prev_state_ids(self.store)

prev_member_event_id = prev_state_ids.get(
(EventTypes.Member, target.to_string()),
(EventTypes.Member, user_id),
None
)

if event.membership == Membership.JOIN:
# Only fire user_joined_room if the user has acutally joined the
# Only fire user_joined_room if the user has actually joined the
# room. Don't bother if the user is just changing their profile
# info.
newly_joined = True
Expand All @@ -218,6 +221,21 @@ def _local_membership_update(
newly_joined = prev_member_event.membership != Membership.JOIN
if newly_joined:
yield self._user_joined_room(target, room_id)

# Copy over direct message status and room tags if this is a join
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
# on an upgraded room
# Check if this is an upgraded room
state_ids = yield self.store.get_filtered_current_state_ids(
room_id, StateFilter.from_types([(EventTypes.Create, "")]),
)
create_id = state_ids.get((EventTypes.Create, ""))
if not create_id:
return
create_event = yield self.store.get_event(create_id)

if "predecessor" in create_event["content"]:
Copy link
Member

Choose a reason for hiding this comment

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

presumably we can use the method from the search PR for this

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, we can but it's inside SearchHandler:

def get_old_rooms_from_upgraded_room(self, room_id):
"""Retrieves room IDs of old rooms in the history of an upgraded room.
We do so by checking the m.room.create event of the room for a
`predecessor` key. If it exists, we add the room ID to our return
list and then check that room for a m.room.create event and so on
until we can no longer find any more previous rooms.
The full list of all found rooms in then returned.
Args:
room_id (str): id of the room to search through.
Returns:
Deferred[iterable[unicode]]: predecessor room ids
"""

Shall I move it to the store instead?

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking specifically of get_room_predecessor, which is in the Store I think?

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, so it is. Will add, thanks.

old_room_id = create_event["content"]["predecessor"]["room_id"]
self.copy_room_tags_and_direct_to_room(old_room_id, room_id, user_id)
elif event.membership == Membership.LEAVE:
if prev_member_event_id:
prev_member_event = yield self.store.get_event(prev_member_event_id)
Expand All @@ -226,6 +244,55 @@ def _local_membership_update(

defer.returnValue(event)

@defer.inlineCallbacks
def copy_room_tags_and_direct_to_room(
self,
old_room_id,
new_room_id,
user_id,
):
"""Copies the tags and direct room state from one room to another.

Args:
old_room_id (str)
new_room_id (str)
user_id (str)

Returns:
Deferred|None
Copy link
Member

Choose a reason for hiding this comment

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

ITYM Deferred[None] ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, it was just copied from another method. We seem to have several different ways of writing the same thing spread across different methods?

Copy link
Member

Choose a reason for hiding this comment

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

Deferred|None means: "either a Deferred (of unspecified value type), or None", which is different to what your method does

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh, I see, thank you.

"""
# Retrieve user account data for predecessor room
user_account_data, _ = yield self.store.get_account_data_for_user(
user_id,
)

# Copy direct message state if applicable
direct_rooms = user_account_data.get("m.direct", {})

# Check which key this room is under
if isinstance(direct_rooms, dict):
for key, room_id_list in direct_rooms.items():
if old_room_id in room_id_list and new_room_id not in room_id_list:
# Add new room_id to this key
direct_rooms[key].append(new_room_id)

# Save back to user's m.direct account data
yield self.store.add_account_data_for_user(
user_id, "m.direct", direct_rooms,
)
break

# Copy room tags if applicable
room_tags = yield self.store.get_tags_for_room(
user_id, old_room_id,
)

# Copy each room tag to the new room
for tag, tag_content in room_tags.items():
yield self.store.add_tag_to_room(
user_id, new_room_id, tag, tag_content
)

@defer.inlineCallbacks
def update_membership(
self,
Expand Down Expand Up @@ -493,7 +560,7 @@ def send_membership_event(
else:
requester = synapse.types.create_requester(target_user)

prev_event = yield self.event_creation_hander.deduplicate_state_event(
prev_event = yield self.event_creation_handler.deduplicate_state_event(
event, context,
)
if prev_event is not None:
Expand All @@ -513,7 +580,7 @@ def send_membership_event(
if is_blocked:
raise SynapseError(403, "This room has been blocked on this server")

yield self.event_creation_hander.handle_new_client_event(
yield self.event_creation_handler.handle_new_client_event(
requester,
event,
context,
Expand All @@ -527,7 +594,7 @@ def send_membership_event(
)

if event.membership == Membership.JOIN:
# Only fire user_joined_room if the user has acutally joined the
# Only fire user_joined_room if the user has actually joined the
# room. Don't bother if the user is just changing their profile
# info.
newly_joined = True
Expand Down Expand Up @@ -755,7 +822,7 @@ def _make_and_store_3pid_invite(
)
)

yield self.event_creation_hander.create_and_send_nonmember_event(
yield self.event_creation_handler.create_and_send_nonmember_event(
requester,
{
"type": EventTypes.ThirdPartyInvite,
Expand Down
8 changes: 4 additions & 4 deletions synapse/rest/client/v1/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class RoomStateEventRestServlet(ClientV1RestServlet):
def __init__(self, hs):
super(RoomStateEventRestServlet, self).__init__(hs)
self.handlers = hs.get_handlers()
self.event_creation_hander = hs.get_event_creation_handler()
self.event_creation_handler = hs.get_event_creation_handler()
self.room_member_handler = hs.get_room_member_handler()
self.message_handler = hs.get_message_handler()

Expand Down Expand Up @@ -172,7 +172,7 @@ def on_PUT(self, request, room_id, event_type, state_key, txn_id=None):
content=content,
)
else:
event = yield self.event_creation_hander.create_and_send_nonmember_event(
event = yield self.event_creation_handler.create_and_send_nonmember_event(
requester,
event_dict,
txn_id=txn_id,
Expand All @@ -189,7 +189,7 @@ class RoomSendEventRestServlet(ClientV1RestServlet):

def __init__(self, hs):
super(RoomSendEventRestServlet, self).__init__(hs)
self.event_creation_hander = hs.get_event_creation_handler()
self.event_creation_handler = hs.get_event_creation_handler()

def register(self, http_server):
# /rooms/$roomid/send/$event_type[/$txn_id]
Expand All @@ -211,7 +211,7 @@ def on_POST(self, request, room_id, event_type, txn_id=None):
if b'ts' in request.args and requester.app_service:
event_dict['origin_server_ts'] = parse_integer(request, "ts", 0)

event = yield self.event_creation_hander.create_and_send_nonmember_event(
event = yield self.event_creation_handler.create_and_send_nonmember_event(
requester,
event_dict,
txn_id=txn_id,
Expand Down
2 changes: 1 addition & 1 deletion synapse/storage/background_updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ def register_background_update_handler(self, update_name, update_handler):
* An integer count of the number of items to update in this batch.

The handler should return a deferred integer count of items updated.
The hander is responsible for updating the progress of the update.
The handler is responsible for updating the progress of the update.

Args:
update_name(str): The name of the update that this code handles.
Expand Down