Stop migrating to MSC4278 if the config exists. (#29924)

* Stop migrationg to MSC4278 if the config exists.

* Run migration after we have synced the client.

* Setup the SettingsController with a client.

* Add tests to check migration behaviour.

* update copyright

* Wait for sync properly

* Catch failure

* Docs

* licence

* Inline async code

* Fix migrateURLPreviewsE2EE too

* drop an import

* go away

src/settings/SettingsStore.ts

@@ -1,5 +1,5 @@
 /*
-Copyright 2024 New Vector Ltd.
+Copyright 2024, 2025 New Vector Ltd.
 Copyright 2019, 2020 The Matrix.org Foundation C.I.C.
 Copyright 2017 Travis Ralston
 
@@ -9,7 +9,7 @@ Please see LICENSE files in the repository root for full details.
 
 import { logger } from "matrix-js-sdk/src/logger";
 import { type ReactNode } from "react";
-import { ClientEvent, SyncState } from "matrix-js-sdk/src/matrix";
+import { ClientEvent } from "matrix-js-sdk/src/matrix";
 
 import DeviceSettingsHandler from "./handlers/DeviceSettingsHandler";
 import RoomDeviceSettingsHandler from "./handlers/RoomDeviceSettingsHandler";
@@ -667,35 +667,25 @@ export default class SettingsStore {
 
         const client = MatrixClientPeg.safeGet();
 
-        const doMigration = async (): Promise<void> => {
+        while (!client.isInitialSyncComplete()) {
-            logger.info("Performing one-time settings migration of URL previews in E2EE rooms");
+            await new Promise((r) => client.once(ClientEvent.Sync, r));
+        }
 
-            const roomAccounthandler = LEVEL_HANDLERS[SettingLevel.ROOM_ACCOUNT];
+        logger.info("Performing one-time settings migration of URL previews in E2EE rooms");
 
-            for (const room of client.getRooms()) {
+        const roomAccounthandler = LEVEL_HANDLERS[SettingLevel.ROOM_ACCOUNT];
-                // We need to use the handler directly because this setting is no longer supported
-                // at this level at all
-                const val = roomAccounthandler.getValue("urlPreviewsEnabled_e2ee", room.roomId);
 
-                if (val !== undefined) {
+        for (const room of client.getRooms()) {
-                    await SettingsStore.setValue("urlPreviewsEnabled_e2ee", room.roomId, SettingLevel.ROOM_DEVICE, val);
+            // We need to use the handler directly because this setting is no longer supported
-                }
+            // at this level at all
+            const val = roomAccounthandler.getValue("urlPreviewsEnabled_e2ee", room.roomId);
+
+            if (val !== undefined) {
+                await SettingsStore.setValue("urlPreviewsEnabled_e2ee", room.roomId, SettingLevel.ROOM_DEVICE, val);
             }
+        }
 
-            localStorage.setItem(MIGRATION_DONE_FLAG, "true");
+        localStorage.setItem(MIGRATION_DONE_FLAG, "true");
-        };
-
-        const onSync = (state: SyncState): void => {
-            if (state === SyncState.Prepared) {
-                client.removeListener(ClientEvent.Sync, onSync);
-
-                doMigration().catch((e) => {
-                    logger.error("Failed to migrate URL previews in E2EE rooms:", e);
-                });
-            }
-        };
-
-        client.on(ClientEvent.Sync, onSync);
     }
 
     /**
@@ -718,25 +708,31 @@ export default class SettingsStore {
 
     /**
      * Migrate the setting for visible images to a setting.
+     *
+     * @param isFreshLogin True if the user has just logged in, false if a previous session is being restored.
      */
-    private static migrateMediaControlsToSetting(): void {
+    private static async migrateMediaControlsToSetting(isFreshLogin: boolean): Promise<void> {
-        const MIGRATION_DONE_FLAG = "mx_migrate_media_controls";
+        if (isFreshLogin) return;
-        if (localStorage.getItem(MIGRATION_DONE_FLAG)) return;
+        const client = MatrixClientPeg.safeGet();
 
+        while (!client.isInitialSyncComplete()) {
+            await new Promise((r) => client.once(ClientEvent.Sync, r));
+        }
+        // Never migrate if the config already exists.
+        if (client.getAccountData("io.element.msc4278.media_preview_config")) {
+            return;
+        }
         logger.info("Performing one-time settings migration of show images and invite avatars to account data");
         const handler = LEVEL_HANDLERS[SettingLevel.ACCOUNT];
         const showImages = handler.getValue("showImages", null);
         const showAvatarsOnInvites = handler.getValue("showAvatarsOnInvites", null);
 
-        const AccountHandler = LEVEL_HANDLERS[SettingLevel.ACCOUNT];
+        if (typeof showImages === "boolean" || typeof showAvatarsOnInvites === "boolean") {
-        if (showImages !== null || showAvatarsOnInvites !== null) {
+            this.setValue("mediaPreviewConfig", null, SettingLevel.ACCOUNT, {
-            AccountHandler.setValue("mediaPreviewConfig", null, {
                 invite_avatars: showAvatarsOnInvites === false ? MediaPreviewValue.Off : MediaPreviewValue.On,
                 media_previews: showImages === false ? MediaPreviewValue.Off : MediaPreviewValue.On,
             });
         } // else, we don't set anything and use the server value
-
-        localStorage.setItem(MIGRATION_DONE_FLAG, "true");
     }
 
     /**
@@ -748,7 +744,9 @@ export default class SettingsStore {
         // (so around October 2024).
         // The consequences of missing the migration are only that URL previews will
         // be disabled in E2EE rooms.
-        SettingsStore.migrateURLPreviewsE2EE(isFreshLogin);
+        SettingsStore.migrateURLPreviewsE2EE(isFreshLogin).catch((e) => {
+            logger.error("Failed to migrate URL previews in E2EE rooms:", e);
+        });
 
         // This can be removed once enough users have run a version of Element with
         // this migration.
@@ -760,8 +758,9 @@ export default class SettingsStore {
         // this migration.
         // The consequences of missing the migration are that the previously set
         // media controls for this user will be missing
-        SettingsStore.migrateMediaControlsToSetting();
+        SettingsStore.migrateMediaControlsToSetting(isFreshLogin).catch((e) => {
-
+            logger.error("Failed to migrate media config settings", e);
+        });
         // Dev notes: to add your migration, just add a new `migrateMyFeature` function, call it, and
         // add a comment to note when it can be removed.
         return;

test/test-utils/test-utils.ts

@@ -164,6 +164,7 @@ export function createTestClient(): MatrixClient {
         getVisibleRooms: jest.fn().mockReturnValue([]),
         loginFlows: jest.fn(),
         on: eventEmitter.on.bind(eventEmitter),
+        once: eventEmitter.once.bind(eventEmitter),
         off: eventEmitter.off.bind(eventEmitter),
         removeListener: eventEmitter.removeListener.bind(eventEmitter),
         emit: eventEmitter.emit.bind(eventEmitter),

test/unit-tests/settings/SettingsStore-test.ts

@@ -1,5 +1,5 @@
 /*
-Copyright 2024 New Vector Ltd.
+Copyright 2024, 2025 New Vector Ltd.
 Copyright 2022 The Matrix.org Foundation C.I.C.
 
 SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
@@ -7,6 +7,7 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import { ClientEvent, type MatrixClient, type Room, SyncState } from "matrix-js-sdk/src/matrix";
+import { waitFor } from "jest-matrix-react";
 
 import type BasePlatform from "../../../src/BasePlatform";
 import SdkConfig from "../../../src/SdkConfig";
@@ -14,6 +15,7 @@ import { SettingLevel } from "../../../src/settings/SettingLevel";
 import SettingsStore from "../../../src/settings/SettingsStore";
 import { mkStubRoom, mockPlatformPeg, stubClient } from "../../test-utils";
 import { type SettingKey } from "../../../src/settings/Settings.tsx";
+import MatrixClientBackedController from "../../../src/settings/controllers/MatrixClientBackedController.ts";
 
 const TEST_DATA = [
     {
@@ -139,5 +141,61 @@ describe("SettingsStore", () => {
 
             expect(room.getAccountData).not.toHaveBeenCalled();
         });
+
+        describe("Migrate media preview configuration", () => {
+            beforeEach(() => {
+                MatrixClientBackedController.matrixClient = client;
+                client.getAccountData = jest.fn().mockImplementation((type) => {
+                    if (type === "im.vector.web.settings") {
+                        return {
+                            getContent: jest.fn().mockReturnValue({
+                                showImages: false,
+                                showAvatarsOnInvites: false,
+                            }),
+                        };
+                    } else {
+                        return undefined;
+                    }
+                });
+            });
+
+            it("migrates media preview configuration immediately", async () => {
+                client.setAccountData = jest.fn();
+                SettingsStore.runMigrations(false);
+                expect(client.setAccountData).toHaveBeenCalledWith("io.element.msc4278.media_preview_config", {
+                    invite_avatars: "off",
+                    media_previews: "off",
+                });
+            });
+            it("migrates media preview configuration once client is ready", async () => {
+                client.setAccountData = jest.fn();
+                const mockInitialSync = (client.isInitialSyncComplete = jest.fn().mockReturnValue(false));
+                SettingsStore.runMigrations(false);
+                mockInitialSync.mockReturnValue(true);
+                client.emit(ClientEvent.Sync, SyncState.Prepared, null);
+                // Update is asynchronous
+                waitFor(() => {
+                    expect(client.setAccountData).toHaveBeenCalledWith("io.element.msc4278.media_preview_config", {
+                        invite_avatars: "off",
+                        media_previews: "off",
+                    });
+                });
+            });
+
+            it("does not migrate media preview configuration if the session is fresh", async () => {
+                client.setAccountData = jest.fn();
+                SettingsStore.runMigrations(true);
+                client.emit(ClientEvent.Sync, SyncState.Prepared, null);
+                expect(client.setAccountData).not.toHaveBeenCalled();
+            });
+
+            it("does not migrate media preview configuration if the account data is already set", async () => {
+                client.setAccountData = jest.fn();
+                client.getAccountData = jest.fn().mockReturnValue({});
+                SettingsStore.runMigrations(false);
+                client.emit(ClientEvent.Sync, SyncState.Prepared, null);
+                expect(client.setAccountData).not.toHaveBeenCalled();
+            });
+        });
     });
 });