From 1ae30bcf1b72abd25ff7fd80a80c9e872f54ce37 Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Thu, 17 Jun 2021 00:42:06 -0400 Subject: [PATCH] Use read receipts to know when to sync media Instead of having to view a LINE chat when a media message is sent, send a placeholder message that gets replaced with the actual media when it's viewed in Matrix. --- matrix_puppeteer_line/db/message.py | 9 +++++---- matrix_puppeteer_line/matrix.py | 10 +++++++--- matrix_puppeteer_line/portal.py | 26 +++++++++++++++++------- matrix_puppeteer_line/rpc/rpc.py | 7 +++++-- matrix_puppeteer_line/user.py | 2 +- puppet/src/client.js | 5 +++-- puppet/src/contentscript.js | 16 +++++++-------- puppet/src/puppet.js | 31 +++++++++++++++++------------ 8 files changed, 65 insertions(+), 41 deletions(-) diff --git a/matrix_puppeteer_line/db/message.py b/matrix_puppeteer_line/db/message.py index cb24a9e..613e467 100644 --- a/matrix_puppeteer_line/db/message.py +++ b/matrix_puppeteer_line/db/message.py @@ -36,10 +36,11 @@ class Message: q = "INSERT INTO message (mxid, mx_room, mid, chat_id) VALUES ($1, $2, $3, $4)" await self.db.execute(q, self.mxid, self.mx_room, self.mid, self.chat_id) - async def update(self) -> None: - q = ("UPDATE message SET mid=$3, chat_id=$4 " - "WHERE mxid=$1 AND mx_room=$2") - await self.db.execute(q, self.mxid, self.mx_room, self.mid, self.chat_id) + async def update_ids(self, new_mxid: EventID, new_mid: int) -> None: + q = ("UPDATE message SET mxid=$1, mid=$2 " + "WHERE mxid=$3 AND mx_room=$4 AND chat_id=$5") + await self.db.execute(q, new_mxid, new_mid, + self.mxid, self.mx_room, self.chat_id) @classmethod async def get_max_mid(cls, room_id: RoomID) -> int: diff --git a/matrix_puppeteer_line/matrix.py b/matrix_puppeteer_line/matrix.py index 8ce2809..cae3463 100644 --- a/matrix_puppeteer_line/matrix.py +++ b/matrix_puppeteer_line/matrix.py @@ -66,8 +66,12 @@ class MatrixHandler(BaseMatrixHandler): async def handle_read_receipt(self, user: 'u.User', portal: 'po.Portal', event_id: EventID, data: SingleReceiptEventContent) -> None: # When reading a bridged message, view its chat in LINE, to make it send a read receipt. + + # TODO Use *null* mids for last messages in a chat!! # Only visit a LINE chat when its LAST bridge message has been read, # because LINE lacks per-message read receipts--it's all or nothing! - if await DBMessage.is_last_by_mxid(event_id, portal.mxid): - # Viewing a chat by updating it whole-hog, lest a ninja arrives - await user.sync_portal(portal) + # TODO Also view if message is non-last but for media, so it can be loaded. + #if await DBMessage.is_last_by_mxid(event_id, portal.mxid): + + # Viewing a chat by updating it whole-hog, lest a ninja arrives + await user.sync_portal(portal) diff --git a/matrix_puppeteer_line/portal.py b/matrix_puppeteer_line/portal.py index 7cce7e7..5f252b7 100644 --- a/matrix_puppeteer_line/portal.py +++ b/matrix_puppeteer_line/portal.py @@ -245,12 +245,17 @@ class Portal(DBPortal, BasePortal): if is_preseen: msg = await DBMessage.get_next_noid_msg(self.mxid) if msg: - self.log.debug(f"Found ID {evt.id} of preseen message in chat {self.mxid} {msg.mxid}") - msg.mid = evt.id - event_id = msg.mxid + self.log.debug(f"Found ID {evt.id} of preseen message in chat {self.mxid}: {msg.mxid}") + prev_event_id = msg.mxid else: self.log.error(f"Could not find an existing event for a message with no ID in chat {self.mxid}") return + else: + prev_event_id = None + + if is_preseen and evt.html: + # No need to update a previewed text message, as their previews are accurate + event_id = prev_event_id elif evt.image and evt.image.url: if not evt.image.is_sticker or self.config["bridge.receive_stickers"]: media_info = await self._handle_remote_media( @@ -268,9 +273,11 @@ class Portal(DBPortal, BasePortal): else: media_info = None send_sticker = self.config["bridge.use_sticker_events"] and evt.image.is_sticker and not self.encrypted and media_info - if send_sticker: - event_id = await intent.send_sticker( - self.mxid, media_info.mxc, image_info, "", timestamp=evt.timestamp) + # TODO Element Web messes up text->sticker edits!! + # File a case on it + if send_sticker and not prev_event_id: + #relates_to = RelatesTo(rel_type=RelationType.REPLACE, event_id=prev_event_id) if prev_event_id else None + event_id = await intent.send_sticker(self.mxid, media_info.mxc, image_info, "", timestamp=evt.timestamp) else: if media_info: content = MediaMessageEventContent( @@ -282,8 +289,11 @@ class Portal(DBPortal, BasePortal): content = TextMessageEventContent( msgtype=MessageType.NOTICE, body=f"<{'sticker' if evt.image.is_sticker else 'image'}>") + if prev_event_id: + content.set_edit(prev_event_id) event_id = await self._send_message(intent, content, timestamp=evt.timestamp) elif evt.html and not evt.html.isspace(): + chunks = [] def handle_data(data): @@ -347,6 +357,8 @@ class Portal(DBPortal, BasePortal): content = TextMessageEventContent( msgtype=MessageType.NOTICE, body="") + if prev_event_id: + content.set_edit(prev_event_id) event_id = await self._send_message(intent, content, timestamp=evt.timestamp) if evt.is_outgoing and evt.receipt_count: @@ -361,7 +373,7 @@ class Portal(DBPortal, BasePortal): except UniqueViolationError as e: self.log.debug(f"Failed to handle remote message {evt.id or 'with no ID'} -> {event_id}: {e}") else: - await msg.update() + await msg.update_ids(new_mxid=event_id, new_mid=evt.id) self.log.debug(f"Handled preseen remote message {evt.id} -> {event_id}") async def handle_remote_receipt(self, receipt: Receipt) -> None: diff --git a/matrix_puppeteer_line/rpc/rpc.py b/matrix_puppeteer_line/rpc/rpc.py index 3400865..cad468b 100644 --- a/matrix_puppeteer_line/rpc/rpc.py +++ b/matrix_puppeteer_line/rpc/rpc.py @@ -33,6 +33,7 @@ class RPCClient: log: TraceLogger = logging.getLogger("mau.rpc") user_id: UserID + ephemeral_events: bool _reader: Optional[asyncio.StreamReader] _writer: Optional[asyncio.StreamWriter] _req_id: int @@ -40,11 +41,12 @@ class RPCClient: _response_waiters: Dict[int, asyncio.Future] _event_handlers: Dict[str, List[EventHandler]] - def __init__(self, user_id: UserID, own_id: str) -> None: + def __init__(self, user_id: UserID, own_id: str, ephemeral_events: bool) -> None: self.log = self.log.getChild(user_id) self.loop = asyncio.get_running_loop() self.user_id = user_id self.own_id = own_id + self.ephemeral_events = ephemeral_events self._req_id = 0 self._min_broadcast_id = 0 self._event_handlers = {} @@ -70,7 +72,8 @@ class RPCClient: self.loop.create_task(self._command_loop()) await self.request("register", user_id=self.user_id, - own_id = self.own_id) + own_id = self.own_id, + ephemeral_events=self.ephemeral_events) async def disconnect(self) -> None: self._writer.write_eof() diff --git a/matrix_puppeteer_line/user.py b/matrix_puppeteer_line/user.py index 662a2f5..b43e83a 100644 --- a/matrix_puppeteer_line/user.py +++ b/matrix_puppeteer_line/user.py @@ -103,7 +103,7 @@ class User(DBUser, BaseUser): async def connect(self) -> None: self.loop.create_task(self.connect_double_puppet()) - self.client = Client(self.mxid, self.own_id) + self.client = Client(self.mxid, self.own_id, self.config["appservice.ephemeral_events"]) self.log.debug("Starting client") await self.send_bridge_notice("Starting up...") state = await self.client.start() diff --git a/puppet/src/client.js b/puppet/src/client.js index 89fa25b..94a74ed 100644 --- a/puppet/src/client.js +++ b/puppet/src/client.js @@ -164,7 +164,7 @@ export default class Client { let started = false if (this.puppet === null) { this.log("Opening new puppeteer for", this.userID) - this.puppet = new MessagesPuppeteer(this.userID, this.ownID, this) + this.puppet = new MessagesPuppeteer(this.userID, this.ownID, this.sendPlaceholders, this) this.manager.puppets.set(this.userID, this.puppet) await this.puppet.start(!!req.debug) started = true @@ -195,7 +195,8 @@ export default class Client { handleRegister = async (req) => { this.userID = req.user_id this.ownID = req.own_id - this.log(`Registered socket ${this.connID} -> ${this.userID}`) + this.sendPlaceholders = req.ephemeral_events + this.log(`Registered socket ${this.connID} -> ${this.userID}${!this.sendPlaceholders ? "" : " (with placeholder message support)"}`) if (this.manager.clients.has(this.userID)) { const oldClient = this.manager.clients.get(this.userID) this.manager.clients.set(this.userID, this) diff --git a/puppet/src/contentscript.js b/puppet/src/contentscript.js index ce36b45..58eb2d7 100644 --- a/puppet/src/contentscript.js +++ b/puppet/src/contentscript.js @@ -740,19 +740,17 @@ class MautrixController { for (const node of change.addedNodes) { } */ - } else if (change.target.tagName == "LI") { + } else if (change.target.tagName == "LI" && change.addedNodes.length == 1) { if (change.target.classList.contains("ExSelected")) { console.debug("Not using chat list mutation response for currently-active chat") continue } - for (const node of change.addedNodes) { - const chat = this.parseChatListItem(node) - if (chat) { - console.log("Added chat list item:", chat) - changedChats.add(chat) - } else { - console.debug("Could not parse added node as a chat list item:", node) - } + const chat = this.parseChatListItem(change.addedNodes[0]) + if (chat) { + console.log("Added chat list item:", chat) + changedChats.add(chat) + } else { + console.debug("Could not parse added node as a chat list item:", node) } } // change.removedNodes tells you which chats that had notifications are now read. diff --git a/puppet/src/puppet.js b/puppet/src/puppet.js index fbb9496..c6f0e40 100644 --- a/puppet/src/puppet.js +++ b/puppet/src/puppet.js @@ -36,13 +36,14 @@ export default class MessagesPuppeteer { * @param {string} id * @param {?Client} [client] */ - constructor(id, ownID, client = null) { + constructor(id, ownID, sendPlaceholders, client = null) { let profilePath = path.join(MessagesPuppeteer.profileDir, id) if (!profilePath.startsWith("/")) { profilePath = path.join(process.cwd(), profilePath) } this.id = id this.ownID = ownID + this.sendPlaceholders = sendPlaceholders this.profilePath = profilePath this.updatedChats = new Set() this.sentMessageIDs = new Set() @@ -764,24 +765,28 @@ export default class MessagesPuppeteer { const diffNumNotifications = chatListInfo.notificationCount - prevNumNotifications if (chatListInfo.notificationCount == 0 && diffNumNotifications < 0) { - // Message was read from another LINE client, so there's no new info to bridge. - // But if the diff == 0, it's an own message sent from LINE, and must bridge it! + this.log("Notifications dropped--must have read messages from another LINE client, skip") this.numChatNotifications.set(chatID, 0) return } const mustSync = // Can only use previews for DMs, because sender can't be found otherwise! + // TODO For non-DMs, send fake messages from bridgebot and delete them. chatListInfo.id.charAt(0) != 'u' - || diffNumNotifications > 1 - // Sync when lastMsg is a canned message for a non-previewable message type. - || chatListInfo.lastMsg.endsWith(" sent a photo.") - || chatListInfo.lastMsg.endsWith(" sent a sticker.") - || chatListInfo.lastMsg.endsWith(" sent a location.") - // TODO More? - // TODO With MSC2409, only sync if >1 new messages arrived, - // or if message is unpreviewable. - // Otherwise, send a dummy notice & sync when its read. + // If >1, a notification was missed. Only way to get them is to view the chat. + // If == 0, might be own message...or just a shuffled chat, or something else. + // To play it safe, just sync them. Should be no harm, as they're viewed already. + || diffNumNotifications != 1 + // Without placeholders, some messages require visiting their chat to be synced. + || !this.sendPlaceholders + && ( + // Sync when lastMsg is a canned message for a non-previewable message type. + chatListInfo.lastMsg.endsWith(" sent a photo.") + || chatListInfo.lastMsg.endsWith(" sent a sticker.") + || chatListInfo.lastMsg.endsWith(" sent a location.") + // TODO More? + ) let messages if (!mustSync) { @@ -789,7 +794,7 @@ export default class MessagesPuppeteer { chat_id: chatListInfo.id, id: null, // because sidebar messages have no ID timestamp: null, // because this message was sent right now - is_outgoing: chatListInfo.notificationCount == 0, + is_outgoing: false, // because there's no reliable way to detect own messages... sender: null, // because only DM messages are handled html: chatListInfo.lastMsg, }]