From a928bb96b03ea677a9c6389b33df643386bcde02 Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 31 Jul 2024 15:41:22 +0100 Subject: [PATCH 1/5] Clean up editor drafts for unknown rooms and add tests. --- src/DraftCleaner.ts | 77 +++++++++++++++++++ src/components/structures/MatrixChat.tsx | 3 + .../views/rooms/SendMessageComposer.tsx | 5 +- .../components/structures/MatrixChat-test.tsx | 32 ++++++++ 4 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 src/DraftCleaner.ts diff --git a/src/DraftCleaner.ts b/src/DraftCleaner.ts new file mode 100644 index 00000000000..f31163974c7 --- /dev/null +++ b/src/DraftCleaner.ts @@ -0,0 +1,77 @@ +/* +Copyright 2024 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { logger } from "matrix-js-sdk/src/logger"; +import { MatrixClientPeg } from "./MatrixClientPeg"; +import { EDITOR_STATE_STORAGE_PREFIX } from "./components/views/rooms/SendMessageComposer"; + +// The key used to persist the the timestamp we last cleaned up drafts +export const DRAFT_LAST_CLEANUP_KEY = "mx_draft_cleanup"; +// The period of time we wait between cleaning drafts +export const DRAFT_CLEANUP_PERIOD = 1000 * 60 * 60 * 24 * 30; + +/** + * Checks if `DRAFT_CLEANUP_PERIOD` has expired, if so, deletes any stord editor drafts that exist for rooms that are not in the known list. + */ +export function cleanUpDraftsIfRequired() { + if (!shouldCleanupDrafts()) { + return; + } + logger.debug(`Cleaning up editor drafts...`); + cleaupDrafts(); + try { + localStorage.setItem(DRAFT_LAST_CLEANUP_KEY, String(Date.now())); + } catch (error) { + logger.error("Failed to persist draft cleanup key", error); + } +} + +/** + * + * @returns {bool} True if the timestamp has not been persisted or the `DRAFT_CLEANUP_PERIOD` has expired. + */ +function shouldCleanupDrafts(): boolean { + try { + const lastCleanupTimestamp = localStorage.getItem(DRAFT_LAST_CLEANUP_KEY); + if (!lastCleanupTimestamp) { + return true; + } + const parsedTimestamp = Number.parseInt(lastCleanupTimestamp || "", 10); + if (!Number.isInteger(parsedTimestamp)) { + return true; + } + return Date.now() > parsedTimestamp + DRAFT_CLEANUP_PERIOD; + } catch (error) { + return true; + } +} + +/** + * Clear all drafts for the CIDER editor if the room does not exist in the known rooms. + */ +function cleaupDrafts() { + for (let i = 0; i < localStorage.length; i++) { + const keyName = localStorage.key(i); + if (!keyName?.startsWith(EDITOR_STATE_STORAGE_PREFIX)) continue; + // Remove the prefix and the optional event id suffix to leave the room id + const roomId = keyName.slice(EDITOR_STATE_STORAGE_PREFIX.length).split("_$")[0]; + const room = MatrixClientPeg.safeGet().getRoom(roomId); + if (!room) { + logger.debug(`Removing draft for unknown room with key ${keyName}`); + localStorage.removeItem(keyName); + } + } +} diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index f9d6db3f535..029bdb5aedd 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -143,6 +143,7 @@ import { checkSessionLockFree, getSessionLock } from "../../utils/SessionLock"; import { SessionLockStolenView } from "./auth/SessionLockStolenView"; import { ConfirmSessionLockTheftView } from "./auth/ConfirmSessionLockTheftView"; import { LoginSplashView } from "./auth/LoginSplashView"; +import { cleanUpDraftsIfRequired } from "../../DraftCleaner"; // legacy export export { default as Views } from "../../Views"; @@ -1542,6 +1543,8 @@ export default class MatrixChat extends React.PureComponent { showNotificationsToast(false); } + cleanUpDraftsIfRequired(); + dis.fire(Action.FocusSendMessageComposer); this.setState({ ready: true, diff --git a/src/components/views/rooms/SendMessageComposer.tsx b/src/components/views/rooms/SendMessageComposer.tsx index c5972ee86ac..49f122217b0 100644 --- a/src/components/views/rooms/SendMessageComposer.tsx +++ b/src/components/views/rooms/SendMessageComposer.tsx @@ -71,6 +71,9 @@ import { IDiff } from "../../../editor/diff"; import { getBlobSafeMimeType } from "../../../utils/blobs"; import { EMOJI_REGEX } from "../../../HtmlUtils"; +// The prefix used when persisting editor drafts to localstorage. +export const EDITOR_STATE_STORAGE_PREFIX = "mx_cider_state_"; + /** * Build the mentions information based on the editor model (and any related events): * @@ -605,7 +608,7 @@ export class SendMessageComposer extends React.Component ({ completeAuthorizationCodeGrant: jest.fn(), @@ -598,6 +599,37 @@ describe("", () => { expect(screen.getByText(`Welcome ${userId}`)).toBeInTheDocument(); }); + describe("clean up drafts", () => { + const roomId = "!room:server.org"; + const unknownRoomId = "!room2:server.org"; + const room = new Room(roomId, mockClient, userId); + const timestamp = 2345678901234; + beforeEach(() => { + localStorage.setItem(`mx_cider_state_${unknownRoomId}`, "fake_content"); + localStorage.setItem(`mx_cider_state_${roomId}`, "fake_content"); + mockClient.getRoom.mockImplementation((id) => [room].find((room) => room.roomId === id) || null); + }); + afterEach(() => { + jest.restoreAllMocks(); + }); + it("should clean up drafts", async () => { + Date.now = jest.fn(() => timestamp); + localStorage.setItem(`mx_cider_state_${roomId}`, "fake_content"); + localStorage.setItem(`mx_cider_state_${unknownRoomId}`, "fake_content"); + await getComponentAndWaitForReady(); + expect(localStorage.getItem(`mx_cider_state_${roomId}`)).not.toBeNull(); + expect(localStorage.getItem(`mx_cider_state_${unknownRoomId}`)).toBeNull(); + }); + + it("should not clean up drafts before expiry", async () => { + // Set the last cleanup to the recent past + localStorage.setItem(`mx_cider_state_${unknownRoomId}`, "fake_content"); + localStorage.setItem(DRAFT_LAST_CLEANUP_KEY, String(timestamp - 100)); + await getComponentAndWaitForReady(); + expect(localStorage.getItem(`mx_cider_state_${unknownRoomId}`)).not.toBeNull(); + }); + }); + describe("onAction()", () => { beforeEach(() => { jest.spyOn(defaultDispatcher, "dispatch").mockClear(); From e12f5d0378bf9d7762d00d7dfa3724e264c060ff Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 31 Jul 2024 15:55:13 +0100 Subject: [PATCH 2/5] lint --- src/DraftCleaner.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/DraftCleaner.ts b/src/DraftCleaner.ts index f31163974c7..5e6c1cbae7f 100644 --- a/src/DraftCleaner.ts +++ b/src/DraftCleaner.ts @@ -15,6 +15,7 @@ limitations under the License. */ import { logger } from "matrix-js-sdk/src/logger"; + import { MatrixClientPeg } from "./MatrixClientPeg"; import { EDITOR_STATE_STORAGE_PREFIX } from "./components/views/rooms/SendMessageComposer"; @@ -26,7 +27,7 @@ export const DRAFT_CLEANUP_PERIOD = 1000 * 60 * 60 * 24 * 30; /** * Checks if `DRAFT_CLEANUP_PERIOD` has expired, if so, deletes any stord editor drafts that exist for rooms that are not in the known list. */ -export function cleanUpDraftsIfRequired() { +export function cleanUpDraftsIfRequired(): void { if (!shouldCleanupDrafts()) { return; } @@ -62,7 +63,7 @@ function shouldCleanupDrafts(): boolean { /** * Clear all drafts for the CIDER editor if the room does not exist in the known rooms. */ -function cleaupDrafts() { +function cleaupDrafts(): void { for (let i = 0; i < localStorage.length; i++) { const keyName = localStorage.key(i); if (!keyName?.startsWith(EDITOR_STATE_STORAGE_PREFIX)) continue; From eb605f020d5e385a713fc1b6a42e37b9fbc40893 Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 1 Aug 2024 15:47:01 +0100 Subject: [PATCH 3/5] Call cleanUpDraftsIfRequired when we know a live update has completed. --- src/components/structures/MatrixChat.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 85cc92ba79d..53514ab817b 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -1529,6 +1529,9 @@ export default class MatrixChat extends React.PureComponent { } if (state === SyncState.Syncing && prevState === SyncState.Syncing) { + // We know we have performabed a live update and known rooms should be in a good state. + // Now is a good time to clean up drafts. + cleanUpDraftsIfRequired(); return; } logger.debug(`MatrixClient sync state => ${state}`); @@ -1543,8 +1546,6 @@ export default class MatrixChat extends React.PureComponent { showNotificationsToast(false); } - cleanUpDraftsIfRequired(); - dis.fire(Action.FocusSendMessageComposer); this.setState({ ready: true, From 71d8517e80a2fe9a12e28da503f943f478b2b43e Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 1 Aug 2024 17:01:29 +0100 Subject: [PATCH 4/5] Fix test for new call site of draft cleaning --- test/components/structures/MatrixChat-test.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index ca19de32e2d..c8ef51510c6 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -617,6 +617,9 @@ describe("", () => { localStorage.setItem(`mx_cider_state_${roomId}`, "fake_content"); localStorage.setItem(`mx_cider_state_${unknownRoomId}`, "fake_content"); await getComponentAndWaitForReady(); + mockClient.emit(ClientEvent.Sync, SyncState.Syncing, SyncState.Syncing); + // let things settle + await flushPromises(); expect(localStorage.getItem(`mx_cider_state_${roomId}`)).not.toBeNull(); expect(localStorage.getItem(`mx_cider_state_${unknownRoomId}`)).toBeNull(); }); From 549c67fde65d5f45635e5f8a8a039c1b8f38adce Mon Sep 17 00:00:00 2001 From: David Langley Date: Tue, 6 Aug 2024 17:02:05 +0100 Subject: [PATCH 5/5] fix test --- test/components/structures/MatrixChat-test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index c8ef51510c6..f4c0fb7ffa2 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -629,6 +629,7 @@ describe("", () => { localStorage.setItem(`mx_cider_state_${unknownRoomId}`, "fake_content"); localStorage.setItem(DRAFT_LAST_CLEANUP_KEY, String(timestamp - 100)); await getComponentAndWaitForReady(); + mockClient.emit(ClientEvent.Sync, SyncState.Syncing, SyncState.Syncing); expect(localStorage.getItem(`mx_cider_state_${unknownRoomId}`)).not.toBeNull(); }); });