From 52adbb7335c1ec4aa4519f1d21a0d0543f1937a1 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 30 Oct 2020 19:46:02 +0200 Subject: [PATCH] Fix potential bugs in manual bridging --- mautrix_telegram/commands/portal/bridge.py | 24 +++++++++++----- .../commands/portal/create_chat.py | 2 +- mautrix_telegram/portal/base.py | 28 +++++++++++++------ mautrix_telegram/portal/matrix.py | 4 +-- mautrix_telegram/portal/metadata.py | 13 ++++----- mautrix_telegram/web/provisioning/__init__.py | 2 +- 6 files changed, 45 insertions(+), 28 deletions(-) diff --git a/mautrix_telegram/commands/portal/bridge.py b/mautrix_telegram/commands/portal/bridge.py index 84cba621..42ddac1a 100644 --- a/mautrix_telegram/commands/portal/bridge.py +++ b/mautrix_telegram/commands/portal/bridge.py @@ -13,7 +13,7 @@ # # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -from typing import Optional, Tuple, Coroutine +from typing import Optional, Tuple, Coroutine, Dict, Any import asyncio from telethon.tl.types import ChatForbidden, ChannelForbidden @@ -113,10 +113,10 @@ async def cleanup_old_portal_while_bridging(evt: CommandEvent, portal: "po.Porta "Continuing without touching previous Matrix room...") return True, None elif evt.args[0] == "delete-and-continue": - return True, portal.cleanup_portal("Portal deleted (moving to another room)") + return True, portal.cleanup_portal("Portal deleted (moving to another room)", delete=False) elif evt.args[0] == "unbridge-and-continue": return True, portal.cleanup_portal("Room unbridged (portal moving to another room)", - puppets_only=True) + puppets_only=True, delete=False) else: await evt.reply( "The chat you were trying to bridge already has a Matrix portal room.\n\n" @@ -155,6 +155,17 @@ async def confirm_bridge(evt: CommandEvent) -> Optional[EventID]: evt.sender.command_status = None is_logged_in = await evt.sender.is_logged_in() and not status["force_use_bot"] + async with portal._room_create_lock: + if portal.mxid: + return await evt.reply("The portal seems to have created a Matrix room while you were " + "calling this command.\n\n" + "Please start over by calling the bridge command again.") + await _locked_confirm_bridge(evt, portal=portal, room_id=bridge_to_mxid, + is_logged_in=is_logged_in) + + +async def _locked_confirm_bridge(evt: CommandEvent, portal: 'po.Portal', room_id: RoomID, + is_logged_in: bool) -> Optional[EventID]: user = evt.sender if is_logged_in else evt.tgbot try: entity = await user.client.get_entity(portal.peer) @@ -172,14 +183,13 @@ async def confirm_bridge(evt: CommandEvent) -> Optional[EventID]: else: return await evt.reply("The bot doesn't seem to be in that chat.") - direct = False - - portal.mxid = bridge_to_mxid + portal.mxid = room_id + portal.by_mxid[portal.mxid] = portal portal.title, portal.about, levels = await get_initial_state(evt.az.intent, evt.room_id) portal.photo_id = "" await portal.save() - asyncio.ensure_future(portal.update_matrix_room(user, entity, direct, levels=levels), + asyncio.ensure_future(portal.update_matrix_room(user, entity, direct=False, levels=levels), loop=evt.loop) return await evt.reply("Bridging complete. Portal synchronization should begin momentarily.") diff --git a/mautrix_telegram/commands/portal/create_chat.py b/mautrix_telegram/commands/portal/create_chat.py index d1b00b4a..c26c49af 100644 --- a/mautrix_telegram/commands/portal/create_chat.py +++ b/mautrix_telegram/commands/portal/create_chat.py @@ -55,6 +55,6 @@ async def create(evt: CommandEvent) -> EventID: try: await portal.create_telegram_chat(evt.sender, supergroup=supergroup) except ValueError as e: - portal.delete() + await portal.delete() return await evt.reply(e.args[0]) return await evt.reply(f"Telegram chat created. ID: {portal.tgid}") diff --git a/mautrix_telegram/portal/base.py b/mautrix_telegram/portal/base.py index 00c837de..8e4f2367 100644 --- a/mautrix_telegram/portal/base.py +++ b/mautrix_telegram/portal/base.py @@ -292,14 +292,19 @@ class BasePortal(MautrixBasePortal, ABC): authenticated.append(user.mxid) return authenticated - async def cleanup_portal(self, message: str, puppets_only: bool = False) -> None: + async def cleanup_portal(self, message: str, puppets_only: bool = False, delete: bool = True + ) -> None: if self.username: try: await self.main_intent.remove_room_alias(self.alias_localpart) except (MatrixRequestError, IntentError): self.log.warning("Failed to remove alias when cleaning up room", exc_info=True) await self.cleanup_room(self.main_intent, self.mxid, message, puppets_only) - self.delete() + if delete: + await self.delete() + else: + self.delete_matrix() + self.mxid = None # endregion # region Database conversion @@ -323,19 +328,24 @@ class BasePortal(MautrixBasePortal, ABC): config=json.dumps(self.local_config), avatar_url=self.avatar_url, encrypted=self.encrypted) - def delete(self) -> None: - # TODO the superclass delete method is async, this should be too - try: - del self.by_tgid[self.tgid_full] - except KeyError: - pass + async def delete(self) -> None: + self.delete_sync() + + def delete_matrix(self) -> None: try: del self.by_mxid[self.mxid] except KeyError: pass + DBMessage.delete_all(self.mxid) + + def delete_sync(self) -> None: + try: + del self.by_tgid[self.tgid_full] + except KeyError: + pass if self._db_instance: self._db_instance.delete() - DBMessage.delete_all(self.mxid) + self.delete_matrix() self.deleted = True @classmethod diff --git a/mautrix_telegram/portal/matrix.py b/mautrix_telegram/portal/matrix.py index 97c2cb76..1564f69d 100644 --- a/mautrix_telegram/portal/matrix.py +++ b/mautrix_telegram/portal/matrix.py @@ -122,7 +122,7 @@ class PortalMatrix(BasePortal, ABC): if user.tgid == source.tgid: return None if self.peer_type == "user" and user.tgid == self.tgid: - self.delete() + await self.delete() return None if isinstance(user, u.User) and await user.needs_relaybot(self): if not self.bot: @@ -152,7 +152,7 @@ class PortalMatrix(BasePortal, ABC): if self.peer_type == "user": await self.main_intent.leave_room(self.mxid) - self.delete() + await self.delete() try: del self.by_tgid[self.tgid_full] del self.by_mxid[self.mxid] diff --git a/mautrix_telegram/portal/metadata.py b/mautrix_telegram/portal/metadata.py index fa70a767..02a12384 100644 --- a/mautrix_telegram/portal/metadata.py +++ b/mautrix_telegram/portal/metadata.py @@ -97,7 +97,7 @@ class PortalMetadata(BasePortal, ABC): pass try: existing = self.by_tgid[(new_id, new_id)] - existing.delete() + existing.delete_sync() except KeyError: pass self.db_instance.edit(tgid=new_id, tg_receiver=new_id, peer_type=self.peer_type) @@ -295,17 +295,14 @@ class PortalMetadata(BasePortal, ABC): async def _create_matrix_room(self, user: 'AbstractUser', entity: Union[TypeChat, User], invites: InviteList) -> Optional[RoomID]: - direct = self.peer_type == "user" - - if invites is None: - invites = [] - if self.mxid: return self.mxid - - if not self.allow_bridging: + elif not self.allow_bridging: return None + direct = self.peer_type == "user" + invites = invites or [] + if not entity: entity = await self.get_entity(user) self.log.trace("Fetched data: %s", entity) diff --git a/mautrix_telegram/web/provisioning/__init__.py b/mautrix_telegram/web/provisioning/__init__.py index 82a56755..10037927 100644 --- a/mautrix_telegram/web/provisioning/__init__.py +++ b/mautrix_telegram/web/provisioning/__init__.py @@ -244,7 +244,7 @@ class ProvisioningAPI(AuthAPI): try: await portal.create_telegram_chat(user, supergroup=supergroup) except ValueError as e: - portal.delete() + await portal.delete() return self.get_error_response(500, "unknown_error", e.args[0]) return web.json_response({