diff options
author | Kristóf Marussy <kristof@marussy.com> | 2021-12-27 16:47:00 +0100 |
---|---|---|
committer | Kristóf Marussy <kristof@marussy.com> | 2021-12-27 16:47:00 +0100 |
commit | 33c4fd3efe831925615f0fe09f8714308ed364db (patch) | |
tree | ba579ae1071b272dceb947d51f2d10f08db11468 /packages/preload | |
parent | refactor: Simplify IpcRendererService and its spec (diff) | |
download | sophie-33c4fd3efe831925615f0fe09f8714308ed364db.tar.gz sophie-33c4fd3efe831925615f0fe09f8714308ed364db.tar.zst sophie-33c4fd3efe831925615f0fe09f8714308ed364db.zip |
refactor: Simplify preload
Jest mocking keeps the electron interaction testable
Diffstat (limited to 'packages/preload')
-rw-r--r-- | packages/preload/src/__mocks__/electron.ts (renamed from packages/preload/src/services/IpcRendererService.ts) | 27 | ||||
-rw-r--r-- | packages/preload/src/contextBridge/SophieRendererImpl.ts | 36 | ||||
-rw-r--r-- | packages/preload/src/contextBridge/__tests__/SophieRendererImpl.spec.ts | 68 |
3 files changed, 60 insertions, 71 deletions
diff --git a/packages/preload/src/services/IpcRendererService.ts b/packages/preload/src/__mocks__/electron.ts index 7f51351..52cddf9 100644 --- a/packages/preload/src/services/IpcRendererService.ts +++ b/packages/preload/src/__mocks__/electron.ts | |||
@@ -18,25 +18,8 @@ | |||
18 | * SPDX-License-Identifier: AGPL-3.0-only | 18 | * SPDX-License-Identifier: AGPL-3.0-only |
19 | */ | 19 | */ |
20 | 20 | ||
21 | import { ipcRenderer } from 'electron'; | 21 | export const ipcRenderer = { |
22 | import { | 22 | invoke: jest.fn(), |
23 | Action, | 23 | on: jest.fn(), |
24 | MainToRendererIpcMessage, | 24 | send: jest.fn(), |
25 | RendererToMainIpcMessage, | 25 | }; |
26 | } from '@sophie/shared'; | ||
27 | |||
28 | export class IpcRendererService { | ||
29 | getSharedStoreSnapshot(): Promise<unknown> { | ||
30 | return ipcRenderer.invoke(RendererToMainIpcMessage.GetSharedStoreSnapshot); | ||
31 | } | ||
32 | |||
33 | dispatchAction(actionToDispatch: Action): void { | ||
34 | ipcRenderer.send(RendererToMainIpcMessage.DispatchAction, actionToDispatch); | ||
35 | } | ||
36 | |||
37 | onSharedStorePatch(callback: (patch: unknown) => void): void { | ||
38 | ipcRenderer.on(MainToRendererIpcMessage.SharedStorePatch, (_event, patch) => { | ||
39 | callback(patch); | ||
40 | }) | ||
41 | } | ||
42 | } | ||
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 @@ | |||
18 | * SPDX-License-Identifier: AGPL-3.0-only | 18 | * SPDX-License-Identifier: AGPL-3.0-only |
19 | */ | 19 | */ |
20 | 20 | ||
21 | import { ipcRenderer } from 'electron'; | ||
21 | import type { IJsonPatch } from 'mobx-state-tree'; | 22 | import type { IJsonPatch } from 'mobx-state-tree'; |
22 | import { | 23 | import { |
23 | Action, | 24 | Action, |
24 | action, | 25 | action, |
26 | MainToRendererIpcMessage, | ||
27 | RendererToMainIpcMessage, | ||
25 | sharedStore, | 28 | sharedStore, |
26 | SharedStoreListener, | 29 | SharedStoreListener, |
27 | SophieRenderer, | 30 | SophieRenderer, |
28 | } from '@sophie/shared'; | 31 | } from '@sophie/shared'; |
29 | 32 | ||
30 | import { IpcRendererService } from '../services/IpcRendererService'; | ||
31 | |||
32 | class SophieRendererImpl implements SophieRenderer { | 33 | class SophieRendererImpl implements SophieRenderer { |
33 | readonly #ipcService; | 34 | private onSharedStoreChangeCalled: boolean = false; |
34 | |||
35 | #onSharedStoreChangeCalled: boolean = false; | ||
36 | 35 | ||
37 | #listener: SharedStoreListener | null = null; | 36 | private listener: SharedStoreListener | null = null; |
38 | 37 | ||
39 | constructor(ipcService: IpcRendererService) { | 38 | constructor() { |
40 | this.#ipcService = ipcService; | 39 | ipcRenderer.on(MainToRendererIpcMessage.SharedStorePatch, (_event, patch) => { |
41 | this.#ipcService.onSharedStorePatch((patch) => { | ||
42 | try { | 40 | try { |
43 | // `mobx-state-tree` will validate the patch, so we can safely cast here. | 41 | // `mobx-state-tree` will validate the patch, so we can safely cast here. |
44 | this.#listener?.onPatch(patch as IJsonPatch); | 42 | this.listener?.onPatch(patch as IJsonPatch); |
45 | } catch (err) { | 43 | } catch (err) { |
46 | console.error('Shared store listener onPatch failed', err); | 44 | console.error('Shared store listener onPatch failed', err); |
47 | this.#listener = null; | 45 | this.listener = null; |
48 | } | 46 | } |
49 | }); | 47 | }); |
50 | } | 48 | } |
51 | 49 | ||
52 | async onSharedStoreChange(listener: SharedStoreListener): Promise<void> { | 50 | async onSharedStoreChange(listener: SharedStoreListener): Promise<void> { |
53 | if (this.#onSharedStoreChangeCalled) { | 51 | if (this.onSharedStoreChangeCalled) { |
54 | throw new Error('Shared store change listener was already set'); | 52 | throw new Error('Shared store change listener was already set'); |
55 | } | 53 | } |
56 | this.#onSharedStoreChangeCalled = true; | 54 | this.onSharedStoreChangeCalled = true; |
57 | let success = false; | 55 | let success = false; |
58 | let snapshot: unknown | null = null; | 56 | let snapshot: unknown | null = null; |
59 | try { | 57 | try { |
60 | snapshot = await this.#ipcService.getSharedStoreSnapshot(); | 58 | snapshot = await ipcRenderer.invoke(RendererToMainIpcMessage.GetSharedStoreSnapshot); |
61 | success = true; | 59 | success = true; |
62 | } catch (err) { | 60 | } catch (err) { |
63 | console.error('Failed to get initial shared store snapshot', err); | 61 | console.error('Failed to get initial shared store snapshot', err); |
@@ -65,7 +63,7 @@ class SophieRendererImpl implements SophieRenderer { | |||
65 | if (success) { | 63 | if (success) { |
66 | if (sharedStore.is(snapshot)) { | 64 | if (sharedStore.is(snapshot)) { |
67 | listener.onSnapshot(snapshot); | 65 | listener.onSnapshot(snapshot); |
68 | this.#listener = listener; | 66 | this.listener = listener; |
69 | return; | 67 | return; |
70 | } | 68 | } |
71 | console.error('Got invalid initial shared store snapshot', snapshot); | 69 | console.error('Got invalid initial shared store snapshot', snapshot); |
@@ -78,7 +76,7 @@ class SophieRendererImpl implements SophieRenderer { | |||
78 | // since all data it may contain was provided from the main world. | 76 | // since all data it may contain was provided from the main world. |
79 | const parsedAction = action.parse(actionToDispatch); | 77 | const parsedAction = action.parse(actionToDispatch); |
80 | try { | 78 | try { |
81 | this.#ipcService.dispatchAction(parsedAction); | 79 | ipcRenderer.send(RendererToMainIpcMessage.DispatchAction, parsedAction); |
82 | } catch (err) { | 80 | } catch (err) { |
83 | // Do not leak IPC failure details into the main world. | 81 | // Do not leak IPC failure details into the main world. |
84 | const message = 'Failed to dispatch action'; | 82 | const message = 'Failed to dispatch action'; |
@@ -88,10 +86,8 @@ class SophieRendererImpl implements SophieRenderer { | |||
88 | } | 86 | } |
89 | } | 87 | } |
90 | 88 | ||
91 | export function createSophieRenderer( | 89 | export function createSophieRenderer(): SophieRenderer { |
92 | ipcService: IpcRendererService, | 90 | const impl = new SophieRendererImpl(); |
93 | ): SophieRenderer { | ||
94 | const impl = new SophieRendererImpl(ipcService); | ||
95 | return { | 91 | return { |
96 | onSharedStoreChange: impl.onSharedStoreChange.bind(impl), | 92 | onSharedStoreChange: impl.onSharedStoreChange.bind(impl), |
97 | dispatchAction: impl.dispatchAction.bind(impl), | 93 | 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 @@ | |||
19 | */ | 19 | */ |
20 | 20 | ||
21 | import { mocked } from 'jest-mock'; | 21 | import { mocked } from 'jest-mock'; |
22 | import { ipcRenderer } from 'electron'; | ||
22 | import type { IJsonPatch } from 'mobx-state-tree'; | 23 | import type { IJsonPatch } from 'mobx-state-tree'; |
23 | import type { Action, SharedStoreSnapshotIn, SophieRenderer } from '@sophie/shared'; | 24 | import { |
25 | Action, | ||
26 | MainToRendererIpcMessage, | ||
27 | RendererToMainIpcMessage, | ||
28 | SharedStoreSnapshotIn, | ||
29 | SophieRenderer, | ||
30 | } from '@sophie/shared'; | ||
24 | 31 | ||
25 | import { IpcRendererService } from '../../services/IpcRendererService'; | ||
26 | import { createSophieRenderer } from '../SophieRendererImpl'; | 32 | import { createSophieRenderer } from '../SophieRendererImpl'; |
27 | 33 | ||
28 | jest.mock('../../services/IpcRendererService'); | 34 | jest.mock('electron'); |
35 | |||
36 | const event: Electron.IpcRendererEvent = null as unknown as Electron.IpcRendererEvent; | ||
29 | 37 | ||
30 | const snapshot: SharedStoreSnapshotIn = { | 38 | const snapshot: SharedStoreSnapshotIn = { |
31 | shouldUseDarkColors: true, | 39 | shouldUseDarkColors: true, |
@@ -50,44 +58,46 @@ const invalidAction = { | |||
50 | action: 'not-a-valid-action', | 58 | action: 'not-a-valid-action', |
51 | } as unknown as Action; | 59 | } as unknown as Action; |
52 | 60 | ||
53 | describe('constructor', () => { | 61 | describe('createSophieRenderer', () => { |
54 | it('registers a shared store patch listener', () => { | 62 | it('registers a shared store patch listener', () => { |
55 | const service = new IpcRendererService(); | 63 | createSophieRenderer(); |
56 | createSophieRenderer(service); | 64 | expect(ipcRenderer.on).toHaveBeenCalledWith( |
57 | expect(service.onSharedStorePatch).toHaveBeenCalledTimes(1); | 65 | MainToRendererIpcMessage.SharedStorePatch, |
66 | expect.anything(), | ||
67 | ); | ||
58 | }); | 68 | }); |
59 | }); | 69 | }); |
60 | 70 | ||
61 | describe('instance', () => { | 71 | describe('SophieRendererImpl', () => { |
62 | let sut: SophieRenderer; | 72 | let sut: SophieRenderer; |
63 | let service: IpcRendererService; | 73 | let onSharedStorePatch: (event: Electron.IpcRendererEvent, patch: unknown) => void; |
64 | let onSharedStorePatch: (patch: unknown) => void; | ||
65 | let listener = { | 74 | let listener = { |
66 | onSnapshot: jest.fn((_snapshot: SharedStoreSnapshotIn) => {}), | 75 | onSnapshot: jest.fn((_snapshot: SharedStoreSnapshotIn) => {}), |
67 | onPatch: jest.fn((_patch: IJsonPatch) => {}), | 76 | onPatch: jest.fn((_patch: IJsonPatch) => {}), |
68 | }; | 77 | }; |
69 | 78 | ||
70 | beforeEach(() => { | 79 | beforeEach(() => { |
71 | service = new IpcRendererService(); | 80 | sut = createSophieRenderer(); |
72 | sut = createSophieRenderer(service); | 81 | onSharedStorePatch = mocked(ipcRenderer.on).mock.calls.find(([channel]) => { |
73 | onSharedStorePatch = mocked(service.onSharedStorePatch).mock.calls[0][0]; | 82 | return channel === MainToRendererIpcMessage.SharedStorePatch; |
83 | })?.[1]!; | ||
74 | }); | 84 | }); |
75 | 85 | ||
76 | describe('onSharedStoreChange', () => { | 86 | describe('onSharedStoreChange', () => { |
77 | it('requests a snapshot from the service', async () => { | 87 | it('requests a snapshot from the service', async () => { |
78 | mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(snapshot); | 88 | mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot); |
79 | await sut.onSharedStoreChange(listener); | 89 | await sut.onSharedStoreChange(listener); |
80 | expect(service.getSharedStoreSnapshot).toBeCalledTimes(1); | 90 | expect(ipcRenderer.invoke).toBeCalledWith(RendererToMainIpcMessage.GetSharedStoreSnapshot); |
81 | }); | 91 | }); |
82 | 92 | ||
83 | it('passes the snapshot to the listener', async () => { | 93 | it('passes the snapshot to the listener', async () => { |
84 | mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(snapshot); | 94 | mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot); |
85 | await sut.onSharedStoreChange(listener); | 95 | await sut.onSharedStoreChange(listener); |
86 | expect(listener.onSnapshot).toBeCalledWith(snapshot); | 96 | expect(listener.onSnapshot).toBeCalledWith(snapshot); |
87 | }); | 97 | }); |
88 | 98 | ||
89 | it('catches service errors without exposing them', async () => { | 99 | it('catches service errors without exposing them', async () => { |
90 | mocked(service.getSharedStoreSnapshot).mockRejectedValue(new Error('s3cr3t')); | 100 | mocked(ipcRenderer.invoke).mockRejectedValue(new Error('s3cr3t')); |
91 | await expect(sut.onSharedStoreChange(listener)).rejects.not.toHaveProperty( | 101 | await expect(sut.onSharedStoreChange(listener)).rejects.not.toHaveProperty( |
92 | 'message', | 102 | 'message', |
93 | expect.stringMatching(/s3cr3t/), | 103 | expect.stringMatching(/s3cr3t/), |
@@ -96,7 +106,7 @@ describe('instance', () => { | |||
96 | }); | 106 | }); |
97 | 107 | ||
98 | it('does not pass on invalid snapshots', async () => { | 108 | it('does not pass on invalid snapshots', async () => { |
99 | mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(invalidSnapshot); | 109 | mocked(ipcRenderer.invoke).mockResolvedValueOnce(invalidSnapshot); |
100 | await expect(sut.onSharedStoreChange(listener)).rejects.toBeInstanceOf(Error); | 110 | await expect(sut.onSharedStoreChange(listener)).rejects.toBeInstanceOf(Error); |
101 | expect(listener.onSnapshot).toBeCalledTimes(0); | 111 | expect(listener.onSnapshot).toBeCalledTimes(0); |
102 | }); | 112 | }); |
@@ -105,18 +115,18 @@ describe('instance', () => { | |||
105 | describe('dispatchAction', () => { | 115 | describe('dispatchAction', () => { |
106 | it('dispatched valid actions', () => { | 116 | it('dispatched valid actions', () => { |
107 | sut.dispatchAction(action); | 117 | sut.dispatchAction(action); |
108 | expect(service.dispatchAction).toBeCalledWith(action); | 118 | expect(ipcRenderer.send).toBeCalledWith(RendererToMainIpcMessage.DispatchAction, action); |
109 | }); | 119 | }); |
110 | 120 | ||
111 | it('does not dispatch invalid actions', () => { | 121 | it('does not dispatch invalid actions', () => { |
112 | expect(() => sut.dispatchAction(invalidAction)).toThrowError(); | 122 | expect(() => sut.dispatchAction(invalidAction)).toThrowError(); |
113 | expect(service.dispatchAction).toBeCalledTimes(0); | 123 | expect(ipcRenderer.send).toBeCalledTimes(0); |
114 | }); | 124 | }); |
115 | }); | 125 | }); |
116 | 126 | ||
117 | describe('when no listener is registered', () => { | 127 | describe('when no listener is registered', () => { |
118 | it('discards the received patch', () => { | 128 | it('discards the received patch', () => { |
119 | onSharedStorePatch(patch); | 129 | onSharedStorePatch(event, patch); |
120 | }); | 130 | }); |
121 | }); | 131 | }); |
122 | 132 | ||
@@ -128,25 +138,25 @@ describe('instance', () => { | |||
128 | 138 | ||
129 | function itDoesNotPassPatchesToTheListener() { | 139 | function itDoesNotPassPatchesToTheListener() { |
130 | it('does not pass patches to the listener', () => { | 140 | it('does not pass patches to the listener', () => { |
131 | onSharedStorePatch(patch); | 141 | onSharedStorePatch(event, patch); |
132 | expect(listener.onPatch).toBeCalledTimes(0); | 142 | expect(listener.onPatch).toBeCalledTimes(0); |
133 | }); | 143 | }); |
134 | } | 144 | } |
135 | 145 | ||
136 | describe('when a listener registered successfully', () => { | 146 | describe('when a listener registered successfully', () => { |
137 | beforeEach(async () => { | 147 | beforeEach(async () => { |
138 | mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(snapshot); | 148 | mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot); |
139 | await sut.onSharedStoreChange(listener); | 149 | await sut.onSharedStoreChange(listener); |
140 | }); | 150 | }); |
141 | 151 | ||
142 | it('passes patches to the listener', () => { | 152 | it('passes patches to the listener', () => { |
143 | onSharedStorePatch(patch); | 153 | onSharedStorePatch(event, patch); |
144 | expect(listener.onPatch).toBeCalledWith(patch); | 154 | expect(listener.onPatch).toBeCalledWith(patch); |
145 | }); | 155 | }); |
146 | 156 | ||
147 | it('catches listener errors', () => { | 157 | it('catches listener errors', () => { |
148 | mocked(listener.onPatch).mockImplementation(() => { throw new Error(); }); | 158 | mocked(listener.onPatch).mockImplementation(() => { throw new Error(); }); |
149 | onSharedStorePatch(patch); | 159 | onSharedStorePatch(event, patch); |
150 | }); | 160 | }); |
151 | 161 | ||
152 | itRefusesToRegisterAnotherListener(); | 162 | itRefusesToRegisterAnotherListener(); |
@@ -154,7 +164,7 @@ describe('instance', () => { | |||
154 | describe('that later threw an error', () => { | 164 | describe('that later threw an error', () => { |
155 | beforeEach(() => { | 165 | beforeEach(() => { |
156 | mocked(listener.onPatch).mockImplementation(() => { throw new Error(); }); | 166 | mocked(listener.onPatch).mockImplementation(() => { throw new Error(); }); |
157 | onSharedStorePatch(patch); | 167 | onSharedStorePatch(event, patch); |
158 | listener.onPatch.mockRestore(); | 168 | listener.onPatch.mockRestore(); |
159 | }); | 169 | }); |
160 | 170 | ||
@@ -164,7 +174,7 @@ describe('instance', () => { | |||
164 | 174 | ||
165 | describe('when a listener failed to register due to service error', () => { | 175 | describe('when a listener failed to register due to service error', () => { |
166 | beforeEach(async () => { | 176 | beforeEach(async () => { |
167 | mocked(service.getSharedStoreSnapshot).mockRejectedValue(new Error()); | 177 | mocked(ipcRenderer.invoke).mockRejectedValue(new Error()); |
168 | try { | 178 | try { |
169 | await sut.onSharedStoreChange(listener); | 179 | await sut.onSharedStoreChange(listener); |
170 | } catch { | 180 | } catch { |
@@ -179,7 +189,7 @@ describe('instance', () => { | |||
179 | 189 | ||
180 | describe('when a listener failed to register due to an invalid snapshot', () => { | 190 | describe('when a listener failed to register due to an invalid snapshot', () => { |
181 | beforeEach(async () => { | 191 | beforeEach(async () => { |
182 | mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(invalidSnapshot); | 192 | mocked(ipcRenderer.invoke).mockResolvedValueOnce(invalidSnapshot); |
183 | try { | 193 | try { |
184 | await sut.onSharedStoreChange(listener); | 194 | await sut.onSharedStoreChange(listener); |
185 | } catch { | 195 | } catch { |
@@ -194,7 +204,7 @@ describe('instance', () => { | |||
194 | 204 | ||
195 | describe('when a listener failed to register due to listener error', () => { | 205 | describe('when a listener failed to register due to listener error', () => { |
196 | beforeEach(async () => { | 206 | beforeEach(async () => { |
197 | mocked(service.getSharedStoreSnapshot).mockResolvedValueOnce(snapshot); | 207 | mocked(ipcRenderer.invoke).mockResolvedValueOnce(snapshot); |
198 | mocked(listener.onSnapshot).mockImplementation(() => { throw new Error(); }); | 208 | mocked(listener.onSnapshot).mockImplementation(() => { throw new Error(); }); |
199 | try { | 209 | try { |
200 | await sut.onSharedStoreChange(listener); | 210 | await sut.onSharedStoreChange(listener); |