Skip to content

Commit 81826d7

Browse files
joker23joker23
committed
chore: redid storage to 2 layer arch
1 parent bbfd875 commit 81826d7

4 files changed

Lines changed: 135 additions & 154 deletions

File tree

‎packages/sdk/electron/__tests__/platform/ElectronPlatform.test.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import type { LDLogger } from '@launchdarkly/js-client-sdk-common';
2+
3+
import ElectronPlatform from '../../src/platform/ElectronPlatform';
4+
5+
const failingStorage = {
6+
get: jest.fn().mockRejectedValue(new Error('disk read failed')),
7+
set: jest.fn().mockRejectedValue(new Error('disk write failed')),
8+
clear: jest.fn().mockRejectedValue(new Error('disk clear failed')),
9+
};
10+
11+
jest.mock('../../src/platform/ElectronStorage', () => ({
12+
getElectronStorage: () => failingStorage,
13+
}));
14+
15+
const logger: LDLogger = {
16+
debug: jest.fn(),
17+
info: jest.fn(),
18+
warn: jest.fn(),
19+
error: jest.fn(),
20+
};
21+
22+
let platform: ElectronPlatform;
23+
24+
beforeEach(() => {
25+
jest.clearAllMocks();
26+
platform = new ElectronPlatform(logger, {});
27+
});
28+
29+
it('logs error and returns null when storage get fails', async () => {
30+
const result = await platform.storage!.get('some-key');
31+
32+
expect(result).toBeNull();
33+
expect(logger.error).toHaveBeenCalledWith(
34+
expect.stringContaining('Error getting key from storage: some-key'),
35+
);
36+
});
37+
38+
it('logs error and swallows when storage set fails', async () => {
39+
await platform.storage!.set('some-key', 'some-value');
40+
41+
expect(logger.error).toHaveBeenCalledWith(
42+
expect.stringContaining('Error setting key in storage: some-key'),
43+
);
44+
});
45+
46+
it('logs error and swallows when storage clear fails', async () => {
47+
await platform.storage!.clear('some-key');
48+
49+
expect(logger.error).toHaveBeenCalledWith(
50+
expect.stringContaining('Error clearing key from storage: some-key'),
51+
);
52+
});

‎packages/sdk/electron/__tests__/platform/ElectronStorage.test.ts‎

Lines changed: 30 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import * as fs from 'fs/promises';
22

3-
import type { LDLogger } from '@launchdarkly/js-client-sdk-common';
4-
53
import ElectronStorage, {
64
getElectronStorage,
75
resetElectronStorage,
@@ -17,40 +15,25 @@ jest.mock('electron', () => ({
1715
const storageFile = '/user/data/ldcache';
1816
const tempFile = `${storageFile}.tmp`;
1917

20-
const logger: LDLogger = {
21-
debug: jest.fn(),
22-
info: jest.fn(),
23-
warn: jest.fn(),
24-
error: jest.fn(),
25-
};
26-
2718
beforeEach(() => {
2819
jest.clearAllMocks();
2920
resetElectronStorage();
3021
});
3122

32-
it('handles failed initialization when clearing values', async () => {
23+
it('throws on clear when initialization failed', async () => {
3324
(fs.readFile as jest.Mock).mockRejectedValueOnce(new Error('file not found'));
3425
(fs.writeFile as jest.Mock).mockRejectedValueOnce(new Error('write failed'));
3526

36-
const storage = new ElectronStorage(logger);
37-
await storage.clear('key1');
38-
39-
expect(logger.debug).not.toHaveBeenCalled();
40-
expect(logger.info).not.toHaveBeenCalled();
41-
expect(logger.warn).not.toHaveBeenCalled();
42-
expect(logger.error).toHaveBeenCalledWith('Error initializing storage: write failed');
43-
expect(logger.error).toHaveBeenCalledWith(
44-
'Error clearing key from storage: key1, reason: Storage is not initialized',
45-
);
27+
const storage = new ElectronStorage();
28+
await expect(storage.clear('key1')).rejects.toThrow('Storage is not initialized: write failed');
4629
});
4730

4831
it('can clear values', async () => {
4932
(fs.readFile as jest.Mock).mockReturnValueOnce(
5033
JSON.stringify({ key1: 'value1', key2: 'value2' }),
5134
);
5235

53-
const storage = new ElectronStorage(logger);
36+
const storage = new ElectronStorage();
5437
await storage.clear('key1');
5538

5639
expect(await storage.get('key1')).toBeNull();
@@ -61,112 +44,72 @@ it('can clear values', async () => {
6144
expect.anything(),
6245
);
6346
expect(fs.rename).toHaveBeenCalledWith(tempFile, storageFile);
64-
65-
expect(logger.debug).not.toHaveBeenCalled();
66-
expect(logger.info).not.toHaveBeenCalled();
67-
expect(logger.warn).not.toHaveBeenCalled();
68-
expect(logger.error).not.toHaveBeenCalled();
6947
});
7048

7149
it('does nothing when clearing a non-existent key', async () => {
7250
(fs.readFile as jest.Mock).mockReturnValueOnce(
7351
JSON.stringify({ key1: 'value1', key2: 'value2' }),
7452
);
7553

76-
const storage = new ElectronStorage(logger);
54+
const storage = new ElectronStorage();
7755
await storage.clear('key3');
7856

7957
expect(fs.writeFile).not.toHaveBeenCalled();
8058
expect(fs.rename).not.toHaveBeenCalled();
81-
82-
expect(logger.debug).not.toHaveBeenCalled();
83-
expect(logger.info).not.toHaveBeenCalled();
84-
expect(logger.warn).not.toHaveBeenCalled();
85-
expect(logger.error).not.toHaveBeenCalled();
8659
});
8760

88-
it('handles error when clearing values', async () => {
61+
it('updates cache even when disk flush fails on clear', async () => {
8962
(fs.readFile as jest.Mock).mockReturnValueOnce(
9063
JSON.stringify({ key1: 'value1', key2: 'value2' }),
9164
);
9265
(fs.writeFile as jest.Mock).mockRejectedValueOnce(new Error('write failed'));
9366

94-
const storage = new ElectronStorage(logger);
95-
await storage.clear('key1');
67+
const storage = new ElectronStorage();
68+
// The flush error propagates so the per-client wrapper can log it
69+
await expect(storage.clear('key1')).rejects.toThrow('write failed');
9670

9771
// Cache is updated synchronously — the in-memory state reflects the clear
98-
// even if the disk write failed. The data will be flushed on the next write.
72+
// even if the disk write failed.
9973
expect(await storage.get('key1')).toBeNull();
100-
101-
expect(logger.debug).not.toHaveBeenCalled();
102-
expect(logger.info).not.toHaveBeenCalled();
103-
expect(logger.warn).not.toHaveBeenCalled();
104-
expect(logger.error).toHaveBeenCalledWith('Error flushing storage to disk: write failed');
10574
});
10675

107-
it('handles failed initialization when getting values', async () => {
76+
it('throws on get when initialization failed', async () => {
10877
(fs.readFile as jest.Mock).mockRejectedValueOnce(new Error('file not found'));
10978
(fs.writeFile as jest.Mock).mockRejectedValueOnce(new Error('write failed'));
11079

111-
const storage = new ElectronStorage(logger);
112-
const value = await storage.get('key1');
113-
114-
expect(value).toBeNull();
115-
116-
expect(logger.debug).not.toHaveBeenCalled();
117-
expect(logger.info).not.toHaveBeenCalled();
118-
expect(logger.warn).not.toHaveBeenCalled();
119-
expect(logger.error).toHaveBeenCalledWith('Error initializing storage: write failed');
120-
expect(logger.error).toHaveBeenCalledWith(
121-
'Error getting key from storage: key1, reason: Storage is not initialized',
122-
);
80+
const storage = new ElectronStorage();
81+
await expect(storage.get('key1')).rejects.toThrow('Storage is not initialized: write failed');
12382
});
12483

12584
it('can get values', async () => {
12685
(fs.readFile as jest.Mock).mockReturnValueOnce(
12786
JSON.stringify({ key1: 'value1', key2: 'value2' }),
12887
);
12988

130-
const storage = new ElectronStorage(logger);
89+
const storage = new ElectronStorage();
13190
const value = await storage.get('key1');
13291

13392
expect(value).toEqual('value1');
134-
135-
expect(logger.debug).not.toHaveBeenCalled();
136-
expect(logger.info).not.toHaveBeenCalled();
137-
expect(logger.warn).not.toHaveBeenCalled();
138-
expect(logger.error).not.toHaveBeenCalled();
13993
});
14094

14195
it('returns null when getting a non-existent key', async () => {
14296
(fs.readFile as jest.Mock).mockReturnValueOnce(
14397
JSON.stringify({ key1: 'value1', key2: 'value2' }),
14498
);
14599

146-
const storage = new ElectronStorage(logger);
100+
const storage = new ElectronStorage();
147101
const value = await storage.get('key3');
148102

149103
expect(value).toBeNull();
150-
151-
expect(logger.debug).not.toHaveBeenCalled();
152-
expect(logger.info).not.toHaveBeenCalled();
153-
expect(logger.warn).not.toHaveBeenCalled();
154-
expect(logger.error).not.toHaveBeenCalled();
155104
});
156105

157-
it('handles failed initialization when setting values', async () => {
106+
it('throws on set when initialization failed', async () => {
158107
(fs.readFile as jest.Mock).mockRejectedValueOnce(new Error('file not found'));
159108
(fs.writeFile as jest.Mock).mockRejectedValueOnce(new Error('write failed'));
160109

161-
const storage = new ElectronStorage(logger);
162-
await storage.set('key3', 'value3');
163-
164-
expect(logger.debug).not.toHaveBeenCalled();
165-
expect(logger.info).not.toHaveBeenCalled();
166-
expect(logger.warn).not.toHaveBeenCalled();
167-
expect(logger.error).toHaveBeenCalledWith('Error initializing storage: write failed');
168-
expect(logger.error).toHaveBeenCalledWith(
169-
'Error setting key in storage: key3, reason: Storage is not initialized',
110+
const storage = new ElectronStorage();
111+
await expect(storage.set('key3', 'value3')).rejects.toThrow(
112+
'Storage is not initialized: write failed',
170113
);
171114
});
172115

@@ -175,7 +118,7 @@ it('can set values', async () => {
175118
JSON.stringify({ key1: 'value1', key2: 'value2' }),
176119
);
177120

178-
const storage = new ElectronStorage(logger);
121+
const storage = new ElectronStorage();
179122
await storage.set('key3', 'value3');
180123

181124
expect(await storage.get('key3')).toEqual('value3');
@@ -186,19 +129,14 @@ it('can set values', async () => {
186129
expect.anything(),
187130
);
188131
expect(fs.rename).toHaveBeenCalledWith(tempFile, storageFile);
189-
190-
expect(logger.debug).not.toHaveBeenCalled();
191-
expect(logger.info).not.toHaveBeenCalled();
192-
expect(logger.warn).not.toHaveBeenCalled();
193-
expect(logger.error).not.toHaveBeenCalled();
194132
});
195133

196134
it('can set values with existing keys', async () => {
197135
(fs.readFile as jest.Mock).mockReturnValueOnce(
198136
JSON.stringify({ key1: 'value1', key2: 'value2' }),
199137
);
200138

201-
const storage = new ElectronStorage(logger);
139+
const storage = new ElectronStorage();
202140
await storage.set('key1', 'new-value1');
203141

204142
expect(await storage.get('key1')).toEqual('new-value1');
@@ -209,41 +147,32 @@ it('can set values with existing keys', async () => {
209147
expect.anything(),
210148
);
211149
expect(fs.rename).toHaveBeenCalledWith(tempFile, storageFile);
212-
213-
expect(logger.debug).not.toHaveBeenCalled();
214-
expect(logger.info).not.toHaveBeenCalled();
215-
expect(logger.warn).not.toHaveBeenCalled();
216-
expect(logger.error).not.toHaveBeenCalled();
217150
});
218151

219-
it('handles error when setting values', async () => {
152+
it('updates cache even when disk flush fails on set', async () => {
220153
(fs.readFile as jest.Mock).mockReturnValueOnce(
221154
JSON.stringify({ key1: 'value1', key2: 'value2' }),
222155
);
223156
(fs.writeFile as jest.Mock).mockRejectedValueOnce(new Error('write failed'));
224157

225-
const storage = new ElectronStorage(logger);
226-
await storage.set('key3', 'value3');
158+
const storage = new ElectronStorage();
159+
// The flush error propagates so the per-client wrapper can log it
160+
await expect(storage.set('key3', 'value3')).rejects.toThrow('write failed');
227161

228162
// Cache is updated synchronously — the in-memory state reflects the set
229-
// even if the disk write failed. The data will be flushed on the next write.
163+
// even if the disk write failed.
230164
expect(await storage.get('key3')).toEqual('value3');
231-
232-
expect(logger.debug).not.toHaveBeenCalled();
233-
expect(logger.info).not.toHaveBeenCalled();
234-
expect(logger.warn).not.toHaveBeenCalled();
235-
expect(logger.error).toHaveBeenCalledWith('Error flushing storage to disk: write failed');
236165
});
237166

238167
it('getElectronStorage returns the same instance on subsequent calls', () => {
239-
const a = getElectronStorage(logger);
240-
const b = getElectronStorage(logger);
168+
const a = getElectronStorage();
169+
const b = getElectronStorage();
241170
expect(a).toBe(b);
242171
});
243172

244173
it('resetElectronStorage causes a fresh instance on next call', () => {
245-
const a = getElectronStorage(logger);
174+
const a = getElectronStorage();
246175
resetElectronStorage();
247-
const b = getElectronStorage(logger);
176+
const b = getElectronStorage();
248177
expect(a).not.toBe(b);
249178
});

‎packages/sdk/electron/src/platform/ElectronPlatform.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,31 @@ export default class ElectronPlatform implements platform.Platform {
2222
requests: platform.Requests;
2323

2424
constructor(logger: LDLogger, options: ElectronOptions) {
25-
this.storage = getElectronStorage(logger);
25+
const globalStorage = getElectronStorage();
26+
this.storage = {
27+
async get(key: string): Promise<string | null> {
28+
try {
29+
return await globalStorage.get(key);
30+
} catch (error) {
31+
logger.error(`Error getting key from storage: ${key}, reason: ${error}`);
32+
return null;
33+
}
34+
},
35+
async set(key: string, value: string): Promise<void> {
36+
try {
37+
await globalStorage.set(key, value);
38+
} catch (error) {
39+
logger.error(`Error setting key in storage: ${key}, reason: ${error}`);
40+
}
41+
},
42+
async clear(key: string): Promise<void> {
43+
try {
44+
await globalStorage.clear(key);
45+
} catch (error) {
46+
logger.error(`Error clearing key from storage: ${key}, reason: ${error}`);
47+
}
48+
},
49+
};
2650
this.requests = new ElectronRequests(
2751
options.tlsParams,
2852
options.proxyOptions,

0 commit comments

Comments
 (0)