Device manager - remove client information events when disabling setting (#9384)

* remove client information events when disabling setting

* tweak naming

Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>

src/DeviceListener.ts

@@ -42,7 +42,10 @@ import { Action } from "./dispatcher/actions";
 import { isLoggedIn } from "./utils/login";
 import SdkConfig from "./SdkConfig";
 import PlatformPeg from "./PlatformPeg";
-import { recordClientInformation } from "./utils/device/clientInformation";
+import {
+    recordClientInformation,
+    removeClientInformation,
+} from "./utils/device/clientInformation";
 import SettingsStore, { CallbackFn } from "./settings/SettingsStore";
 
 const KEY_BACKUP_POLL_INTERVAL = 5 * 60 * 1000;
@@ -90,7 +93,7 @@ export default class DeviceListener {
         );
         this.dispatcherRef = dis.register(this.onAction);
         this.recheck();
-        this.recordClientInformation();
+        this.updateClientInformation();
     }
 
     public stop() {
@@ -216,7 +219,7 @@ export default class DeviceListener {
     private onAction = ({ action }: ActionPayload) => {
         if (action !== Action.OnLoggedIn) return;
         this.recheck();
-        this.recordClientInformation();
+        this.updateClientInformation();
     };
 
     // The server doesn't tell us when key backup is set up, so we poll
@@ -368,25 +371,26 @@ export default class DeviceListener {
 
         this.shouldRecordClientInformation = !!newValue;
 
-        if (this.shouldRecordClientInformation && !prevValue) {
+        if (this.shouldRecordClientInformation !== prevValue) {
-            this.recordClientInformation();
+            this.updateClientInformation();
         }
     };
 
-    private recordClientInformation = async () => {
+    private updateClientInformation = async () => {
-        if (!this.shouldRecordClientInformation) {
-            return;
-        }
         try {
-            await recordClientInformation(
+            if (this.shouldRecordClientInformation) {
-                MatrixClientPeg.get(),
+                await recordClientInformation(
-                SdkConfig.get(),
+                    MatrixClientPeg.get(),
-                PlatformPeg.get(),
+                    SdkConfig.get(),
-            );
+                    PlatformPeg.get(),
+                );
+            } else {
+                await removeClientInformation(MatrixClientPeg.get());
+            }
         } catch (error) {
             // this is a best effort operation
             // log the error without rethrowing
-            logger.error('Failed to record client information', error);
+            logger.error('Failed to update client information', error);
         }
     };
 }

src/utils/device/clientInformation.ts

@@ -65,6 +65,24 @@ export const recordClientInformation = async (
     });
 };
 
+/**
+ * Remove extra client information
+ * @todo(kerrya) revisit after MSC3391: account data deletion is done
+ * (PSBE-12)
+ */
+export const removeClientInformation = async (
+    matrixClient: MatrixClient,
+): Promise<void> => {
+    const deviceId = matrixClient.getDeviceId();
+    const type = getClientInformationEventType(deviceId);
+    const clientInformation = getDeviceClientInformation(matrixClient, deviceId);
+
+    // if a non-empty client info event exists, overwrite to remove the content
+    if (clientInformation.name || clientInformation.version || clientInformation.url) {
+        await matrixClient.setAccountData(type, {});
+    }
+};
+
 const sanitizeContentString = (value: unknown): string | undefined =>
     value && typeof value === 'string' ? value : undefined;
 

test/DeviceListener-test.ts

@@ -17,7 +17,7 @@ limitations under the License.
 
 import { EventEmitter } from "events";
 import { mocked } from "jest-mock";
-import { Room } from "matrix-js-sdk/src/matrix";
+import { MatrixEvent, Room } from "matrix-js-sdk/src/matrix";
 import { logger } from "matrix-js-sdk/src/logger";
 
 import DeviceListener from "../src/DeviceListener";
@@ -66,6 +66,7 @@ class MockClient extends EventEmitter {
     getClientWellKnown = jest.fn();
     getDeviceId = jest.fn().mockReturnValue(deviceId);
     setAccountData = jest.fn();
+    getAccountData = jest.fn();
 }
 const mockDispatcher = mocked(dis);
 const flushPromises = async () => await new Promise(process.nextTick);
@@ -138,7 +139,7 @@ describe('DeviceListener', () => {
                 await createAndStart();
 
                 expect(errorLogSpy).toHaveBeenCalledWith(
-                    'Failed to record client information',
+                    'Failed to update client information',
                     error,
                 );
             });
@@ -161,19 +162,39 @@ describe('DeviceListener', () => {
         });
 
         describe('when device client information feature is disabled', () => {
+            const clientInfoEvent = new MatrixEvent({ type: `io.element.matrix_client_information.${deviceId}`,
+                content: { name: 'hello' },
+            });
+            const emptyClientInfoEvent = new MatrixEvent({ type: `io.element.matrix_client_information.${deviceId}` });
             beforeEach(() => {
                 jest.spyOn(SettingsStore, 'getValue').mockReturnValue(false);
+
+                mockClient.getAccountData.mockReturnValue(undefined);
             });
 
             it('does not save client information on start', async () => {
                 await createAndStart();
 
-                expect(mockClient.setAccountData).not.toHaveBeenCalledWith(
+                expect(mockClient.setAccountData).not.toHaveBeenCalled();
+            });
+
+            it('removes client information on start if it exists', async () => {
+                mockClient.getAccountData.mockReturnValue(clientInfoEvent);
+                await createAndStart();
+
+                expect(mockClient.setAccountData).toHaveBeenCalledWith(
                     `io.element.matrix_client_information.${deviceId}`,
-                    { name: 'Element', url: 'localhost', version: '1.2.3' },
+                    {},
                 );
             });
 
+            it('does not try to remove client info event that are already empty', async () => {
+                mockClient.getAccountData.mockReturnValue(emptyClientInfoEvent);
+                await createAndStart();
+
+                expect(mockClient.setAccountData).not.toHaveBeenCalled();
+            });
+
             it('does not save client information on logged in action', async () => {
                 const instance = await createAndStart();
 
@@ -182,10 +203,7 @@ describe('DeviceListener', () => {
 
                 await flushPromises();
 
-                expect(mockClient.setAccountData).not.toHaveBeenCalledWith(
+                expect(mockClient.setAccountData).not.toHaveBeenCalled();
-                    `io.element.matrix_client_information.${deviceId}`,
-                    { name: 'Element', url: 'localhost', version: '1.2.3' },
-                );
             });
 
             it('saves client information after setting is enabled', async () => {