From 33c4fd3efe831925615f0fe09f8714308ed364db Mon Sep 17 00:00:00 2001 From: Kristóf Marussy Date: Mon, 27 Dec 2021 16:47:00 +0100 Subject: refactor: Simplify preload Jest mocking keeps the electron interaction testable --- packages/preload/src/__mocks__/electron.ts | 25 ++++++++ .../src/contextBridge/SophieRendererImpl.ts | 36 +++++------- .../__tests__/SophieRendererImpl.spec.ts | 68 +++++++++++++--------- .../preload/src/services/IpcRendererService.ts | 42 ------------- 4 files changed, 80 insertions(+), 91 deletions(-) create mode 100644 packages/preload/src/__mocks__/electron.ts delete mode 100644 packages/preload/src/services/IpcRendererService.ts (limited to 'packages') diff --git a/packages/preload/src/__mocks__/electron.ts b/packages/preload/src/__mocks__/electron.ts new file mode 100644 index 0000000..52cddf9 --- /dev/null +++ b/packages/preload/src/__mocks__/electron.ts @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2021-2022 Kristóf Marussy + * + * This file is part of Sophie. + * + * Sophie is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +export const ipcRenderer = { + invoke: jest.fn(), + on: jest.fn(), + send: jest.fn(), +}; diff --git a/packages/preload/src/contextBridge/SophieRendererImpl.ts b/packages/preload/src/contextBridge/SophieRendererImpl.ts index bbb4f65..18ab07e 100644 --- a/packages/preload/src/contextBridge/SophieRendererImpl.ts +++ b/packages/preload/src/contextBridge/SophieRendererImpl.ts @@ -18,46 +18,44 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +import { ipcRenderer } from 'electron'; import type { IJsonPatch } from 'mobx-state-tree'; import { Action, action, + MainToRendererIpcMessage, + RendererToMainIpcMessage, sharedStore, SharedStoreListener, SophieRenderer, } from '@sophie/shared'; -import { IpcRendererService } from '../services/IpcRendererService'; - class SophieRendererImpl implements SophieRenderer { - readonly #ipcService; - - #onSharedStoreChangeCalled: boolean = false; + private onSharedStoreChangeCalled: boolean = false; - #listener: SharedStoreListener | null = null; + private listener: SharedStoreListener | null = null; - constructor(ipcService: IpcRendererService) { - this.#ipcService = ipcService; - this.#ipcService.onSharedStorePatch((patch) => { + constructor() { + ipcRenderer.on(MainToRendererIpcMessage.SharedStorePatch, (_event, patch) => { try { // `mobx-state-tree` will validate the patch, so we can safely cast here. - this.#listener?.onPatch(patch as IJsonPatch); + this.listener?.onPatch(patch as IJsonPatch); } catch (err) { console.error('Shared store listener onPatch failed', err); - this.#listener = null; + this.listener = null; } }); } async onSharedStoreChange(listener: SharedStoreListener): Promise { - if (this.#onSharedStoreChangeCalled) { + if (this.onSharedStoreChangeCalled) { throw new Error('Shared store change listener was already set'); } - this.#onSharedStoreChangeCalled = true; + this.onSharedStoreChangeCalled = true; let success = false; let snapshot: unknown | null = null; try { - snapshot = await this.#ipcService.getSharedStoreSnapshot(); + snapshot = await ipcRenderer.invoke(RendererToMainIpcMessage.GetSharedStoreSnapshot); success = true; } catch (err) { console.error('Failed to get initial shared store snapshot', err); @@ -65,7 +63,7 @@ class SophieRendererImpl implements SophieRenderer { if (success) { if (sharedStore.is(snapshot)) { listener.onSnapshot(snapshot); - this.#listener = listener; + this.listener = listener; return; } console.error('Got invalid initial shared store snapshot', snapshot); @@ -78,7 +76,7 @@ class SophieRendererImpl implements SophieRenderer { // since all data it may contain was provided from the main world. const parsedAction = action.parse(actionToDispatch); try { - this.#ipcService.dispatchAction(parsedAction); + ipcRenderer.send(RendererToMainIpcMessage.DispatchAction, parsedAction); } catch (err) { // Do not leak IPC failure details into the main world. const message = 'Failed to dispatch action'; @@ -88,10 +86,8 @@ class SophieRendererImpl implements SophieRenderer { } } -export function createSophieRenderer( - ipcService: IpcRendererService, -): SophieRenderer { - const impl = new SophieRendererImpl(ipcService); +export function createSophieRenderer(): SophieRenderer { + const impl = new SophieRendererImpl(); return { onSharedStoreChange: impl.onSharedStoreChange.bind(impl), dispatchAction: impl.dispatchAction.bind(impl), diff --git a/packages/preload/src/contextBridge/__tests__/SophieRendererImpl.spec.ts b/packages/preload/src/contextBridge/__tests__/SophieRendererImpl.spec.ts index bd2b1d5..2f295e6 100644 --- a/packages/preload/src/contextBridge/__tests__/SophieRendererImpl.spec.ts +++ b/packages/preload/src/contextBridge/__tests__/SophieRendererImpl.spec.ts @@ -19,13 +19,21 @@ */ import { mocked } from 'jest-mock'; +import { ipcRenderer } from 'electron'; import type { IJsonPatch } from 'mobx-state-tree'; -import type { Action, SharedStoreSnapshotIn, SophieRenderer } from '@sophie/shared'; +import { + Action, + MainToRendererIpcMessage, + RendererToMainIpcMessage, + SharedStoreSnapshotIn, + SophieRenderer, +} from '@sophie/shared'; -import { IpcRendererService } from '../../services/IpcRendererService'; import { createSophieRenderer } from '../SophieRendererImpl'; -jest.mock('../../services/IpcRendererService'); +jest.mock('electron'); + +const event: Electron.IpcRendererEvent = null as unknown as Electron.IpcRendererEvent; const snapshot: SharedStoreSnapshotIn = { shouldUseDarkColors: true, @@ -50,44 +58,46 @@ const invalidAction = { action: 'not-a-valid-action', } as unknown as Action; -describe('constructor', () => { +describe('createSophieRenderer', () => { it('registers a shared store patch listener', () => { - const service = new IpcRendererService(); - createSophieRenderer(service); - expect(service.onSharedStorePatch).toHaveBeenCalledTimes(1); + createSophieRenderer(); + expect(ipcRenderer.on).toHaveBeenCalledWith( + MainToRendererIpcMessage.SharedStorePatch, + expect.anything(), + ); }); }); -describe('instance', () => { +describe('SophieRendererImpl', () => { let sut: SophieRenderer; - let service: IpcRendererService; - let onSharedStorePatch: (patch: unknown) => void; + let onSharedStorePatch: (event: Electron.IpcRendererEvent, patch: unknown) => void; let listener = { onSnapshot: jest.fn((_snapshot: SharedStoreSnapshotIn) => {}), onPatch: jest.fn((_patch: IJsonPatch) => {}), }; beforeEach(() => { - service = new IpcRendererService(); - sut = createSophieRenderer(service); - onSharedStorePatch = mocked(service.onSharedStorePatch).mock.calls[0][0]; + sut = createSophieRenderer(); + onSharedStorePatch = mocked(ipcRenderer.on).mock.calls.find(([channel]) => { + return channel === MainToRendererIpcMessage.SharedStorePatch; + })?.[1]!; }); describe('onSharedStoreChange', () => { it('requests a snapshot from the service', async () => { - mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(snapshot); + mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot); await sut.onSharedStoreChange(listener); - expect(service.getSharedStoreSnapshot).toBeCalledTimes(1); + expect(ipcRenderer.invoke).toBeCalledWith(RendererToMainIpcMessage.GetSharedStoreSnapshot); }); it('passes the snapshot to the listener', async () => { - mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(snapshot); + mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot); await sut.onSharedStoreChange(listener); expect(listener.onSnapshot).toBeCalledWith(snapshot); }); it('catches service errors without exposing them', async () => { - mocked(service.getSharedStoreSnapshot).mockRejectedValue(new Error('s3cr3t')); + mocked(ipcRenderer.invoke).mockRejectedValue(new Error('s3cr3t')); await expect(sut.onSharedStoreChange(listener)).rejects.not.toHaveProperty( 'message', expect.stringMatching(/s3cr3t/), @@ -96,7 +106,7 @@ describe('instance', () => { }); it('does not pass on invalid snapshots', async () => { - mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(invalidSnapshot); + mocked(ipcRenderer.invoke).mockResolvedValueOnce(invalidSnapshot); await expect(sut.onSharedStoreChange(listener)).rejects.toBeInstanceOf(Error); expect(listener.onSnapshot).toBeCalledTimes(0); }); @@ -105,18 +115,18 @@ describe('instance', () => { describe('dispatchAction', () => { it('dispatched valid actions', () => { sut.dispatchAction(action); - expect(service.dispatchAction).toBeCalledWith(action); + expect(ipcRenderer.send).toBeCalledWith(RendererToMainIpcMessage.DispatchAction, action); }); it('does not dispatch invalid actions', () => { expect(() => sut.dispatchAction(invalidAction)).toThrowError(); - expect(service.dispatchAction).toBeCalledTimes(0); + expect(ipcRenderer.send).toBeCalledTimes(0); }); }); describe('when no listener is registered', () => { it('discards the received patch', () => { - onSharedStorePatch(patch); + onSharedStorePatch(event, patch); }); }); @@ -128,25 +138,25 @@ describe('instance', () => { function itDoesNotPassPatchesToTheListener() { it('does not pass patches to the listener', () => { - onSharedStorePatch(patch); + onSharedStorePatch(event, patch); expect(listener.onPatch).toBeCalledTimes(0); }); } describe('when a listener registered successfully', () => { beforeEach(async () => { - mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(snapshot); + mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot); await sut.onSharedStoreChange(listener); }); it('passes patches to the listener', () => { - onSharedStorePatch(patch); + onSharedStorePatch(event, patch); expect(listener.onPatch).toBeCalledWith(patch); }); it('catches listener errors', () => { mocked(listener.onPatch).mockImplementation(() => { throw new Error(); }); - onSharedStorePatch(patch); + onSharedStorePatch(event, patch); }); itRefusesToRegisterAnotherListener(); @@ -154,7 +164,7 @@ describe('instance', () => { describe('that later threw an error', () => { beforeEach(() => { mocked(listener.onPatch).mockImplementation(() => { throw new Error(); }); - onSharedStorePatch(patch); + onSharedStorePatch(event, patch); listener.onPatch.mockRestore(); }); @@ -164,7 +174,7 @@ describe('instance', () => { describe('when a listener failed to register due to service error', () => { beforeEach(async () => { - mocked(service.getSharedStoreSnapshot).mockRejectedValue(new Error()); + mocked(ipcRenderer.invoke).mockRejectedValue(new Error()); try { await sut.onSharedStoreChange(listener); } catch { @@ -179,7 +189,7 @@ describe('instance', () => { describe('when a listener failed to register due to an invalid snapshot', () => { beforeEach(async () => { - mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(invalidSnapshot); + mocked(ipcRenderer.invoke).mockResolvedValueOnce(invalidSnapshot); try { await sut.onSharedStoreChange(listener); } catch { @@ -194,7 +204,7 @@ describe('instance', () => { describe('when a listener failed to register due to listener error', () => { beforeEach(async () => { - mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(snapshot); + mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot); mocked(listener.onSnapshot).mockImplementation(() => { throw new Error(); }); try { await sut.onSharedStoreChange(listener); diff --git a/packages/preload/src/services/IpcRendererService.ts b/packages/preload/src/services/IpcRendererService.ts deleted file mode 100644 index 7f51351..0000000 --- a/packages/preload/src/services/IpcRendererService.ts +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright (C) 2021-2022 Kristóf Marussy - * - * This file is part of Sophie. - * - * Sophie is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, version 3. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * - * SPDX-License-Identifier: AGPL-3.0-only - */ - -import { ipcRenderer } from 'electron'; -import { - Action, - MainToRendererIpcMessage, - RendererToMainIpcMessage, -} from '@sophie/shared'; - -export class IpcRendererService { - getSharedStoreSnapshot(): Promise { - return ipcRenderer.invoke(RendererToMainIpcMessage.GetSharedStoreSnapshot); - } - - dispatchAction(actionToDispatch: Action): void { - ipcRenderer.send(RendererToMainIpcMessage.DispatchAction, actionToDispatch); - } - - onSharedStorePatch(callback: (patch: unknown) => void): void { - ipcRenderer.on(MainToRendererIpcMessage.SharedStorePatch, (_event, patch) => { - callback(patch); - }) - } -} -- cgit v1.2.3-70-g09d2