Skip to content

Commit 501b000

Browse files
stefcameronstefcameron
authored
ExtensionStore is no longer a Singleton in Lens since some 6.4.x release (#1359)
- https://github.com/lensapp/lens/blob/master/packages/core/src/extensions/extension-store.ts - lensapp/lens#6690 ("Make base store non Singleton") - lensapp/lens#4573 ("Elimination of global shared state") The `createInstance()` and `getInstance()` methods had been deprecated in the Lens Extensions API, so they have been eliminated from our code base also. But we're still using "global shared state" Cloud and Sync store instances. It's just that Lens isn't the one creating and holding the instance anymore, we are.
1 parent 3de24d9 commit 501b000

17 files changed

+156
-178
lines changed

Diff for: ‎__mocks__/@k8slens/extensions.js

+6-38
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ class Singleton {
117117
// set in the prototype chain directly off the Singleton constructor function (when this
118118
// code is distilled down to ES5) and then called as `ExtendingSingletonClass.createInstance()`,
119119
// it will be a reference to the constructor function of the class extending from
120-
// Singleton (e.g. the `ExtendingSingleClass` function in this case, which we can "new"
120+
// Singleton (e.g. the `ExtendingSingletonClass` function in this case, which we can "new"
121121
// because it's a function).
122122
// NOTE: The prototype chain will be like this, `ExtendingSingletonClass.__proto__ -> Singleton`
123123
// (the Singleton function itself, setup as the "prototype of" the ExtendingSingletonClass
@@ -413,51 +413,19 @@ MenuActions.proptypes = {
413413

414414
// NOTE: in reality, ExtensionStore extends BaseStore, but we don't need to access BaseStore
415415
// separately anywhere in our code, so the mock just fakes everything in ExtensionStore
416-
class ExtensionStore extends Singleton {
417-
/**
418-
* __MOCK ONLY__
419-
* @type {{ [index: string]: { created: boolean, json: Object } }} map of store name to
420-
* object with `created` true if store has been constructed, and `json` being the current
421-
* state of the store from "disk"
422-
*/
423-
static stores = {};
424-
425-
/**
426-
* __MOCK ONLY__
427-
*
428-
* Initializes a store before it gets created.
429-
* @param {string} name Store name.
430-
* @param {Object} json Store state as it would be read from disk by Lens in reality.
431-
*/
432-
static initStore(name, json) {
433-
ExtensionStore.stores[name] = { created: false, json };
434-
}
435-
416+
class ExtensionStore {
436417
constructor({ configName, defaults }) {
437-
super();
438-
439418
this.configName = configName;
440419
this.defaults = defaults;
441420
}
442421

443422
loadExtension(extension) {
444-
if (!ExtensionStore.stores[this.configName]?.created) {
445-
if (!ExtensionStore.stores[this.configName]) {
446-
// create store state
447-
ExtensionStore.stores[this.configName] = {
448-
created: false,
449-
json: this.defaults,
450-
};
451-
}
452-
}
453-
454-
const state = ExtensionStore.stores[this.configName];
455-
state.created = true;
456-
457-
// real impl doesn't appear to have any async behavior in it: calls fromStore() immediately
458-
this.fromStore(state.json);
423+
this.fromStore(this.defaults);
459424
}
460425

426+
// NOTE: in unit tests, call this method directly on the instance (it won't be abstract)
427+
// in order to load the store with some seed data if you're doing on an instance that
428+
// already exists (like the `globalCloudStore` or `globalSyncStore`)
461429
fromStore(store) {
462430
throw new Error('abstract');
463431
}

Diff for: ‎src/main/SyncManager.js

+15-13
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import YAML from 'yaml';
66
import { Common } from '@k8slens/extensions';
77
import { DATA_CLOUD_EVENTS, DataCloud } from '../common/DataCloud';
88
import { CONNECTION_STATUSES } from '../common/Cloud';
9-
import { cloudStore } from '../store/CloudStore';
10-
import { syncStore } from '../store/SyncStore';
9+
import { globalCloudStore } from '../store/CloudStore';
10+
import { globalSyncStore } from '../store/SyncStore';
1111
import { apiCredentialKinds, apiKinds } from '../api/apiConstants';
1212
import { logValue } from '../util/logger';
1313
import * as consts from '../constants';
@@ -180,7 +180,7 @@ export class SyncManager extends Singleton {
180180
// @see https://mobx.js.org/reactions.html#reaction
181181
reaction(
182182
// react to changes in `clouds` array
183-
() => cloudStore.clouds,
183+
() => globalCloudStore.clouds,
184184
this.handleCloudStoreChange,
185185
{
186186
// fire right away because it's unclear if the SyncStore will be loaded by now
@@ -198,11 +198,11 @@ export class SyncManager extends Singleton {
198198
reaction(
199199
// react to changes in SyncStore lists
200200
() => ({
201-
credentials: syncStore.credentials,
202-
sshKeys: syncStore.sshKeys,
203-
clusters: syncStore.clusters,
204-
licenses: syncStore.licenses,
205-
proxies: syncStore.proxies,
201+
credentials: globalSyncStore.credentials,
202+
sshKeys: globalSyncStore.sshKeys,
203+
clusters: globalSyncStore.clusters,
204+
licenses: globalSyncStore.licenses,
205+
proxies: globalSyncStore.proxies,
206206
}),
207207
this.handleSyncStoreChange,
208208
{
@@ -256,7 +256,7 @@ export class SyncManager extends Singleton {
256256
? [this.catalogSource]
257257
: // NOTE: we can't just flatten all the SyncStore lists into one because it'll lose its observability
258258
// and we won't get actual models, we'll get "Mobx binding items" of some kind (not what you think!)
259-
syncStore.getListNames().map((name) => syncStore[name]);
259+
globalSyncStore.getListNames().map((name) => globalSyncStore[name]);
260260

261261
lists.forEach((list) => {
262262
let idx;
@@ -1030,7 +1030,7 @@ export class SyncManager extends Singleton {
10301030
const oldSyncedDCs = { ...this.dataClouds };
10311031

10321032
// add new DataClouds for new Clouds added to CloudStore
1033-
Object.entries(cloudStore.clouds).forEach(([cloudUrl, cloud]) => {
1033+
Object.entries(globalCloudStore.clouds).forEach(([cloudUrl, cloud]) => {
10341034
if (!this.dataClouds[cloudUrl]) {
10351035
// NOTE: SyncManager only runs on the MAIN thread and always needs full data
10361036
// so we create full instances, not preview ones
@@ -1057,7 +1057,7 @@ export class SyncManager extends Singleton {
10571057
// which will soon dispatch DataCloud.EVENTS.DATA_UPDATED events, calling the
10581058
// `SyncManager.handleDataCloudUpdated()` handler
10591059
Object.keys(oldSyncedDCs).forEach((cloudUrl) => {
1060-
if (cloudStore.clouds[cloudUrl]) {
1060+
if (globalCloudStore.clouds[cloudUrl]) {
10611061
// still exists so remove from the old list so we don't destroy it
10621062
// NOTE: the CloudStore is careful to update existing Cloud instances when
10631063
// they change in the cloud-store.json file, and those updates will cause
@@ -1317,7 +1317,7 @@ export class SyncManager extends Singleton {
13171317
// avoid them taking place while we're still churning through Cloud updates
13181318
// NOTE: we can update the `catalogSource` directly because we don't watch it;
13191319
// it's a one-way change here and reflected in Lens
1320-
const jsonStore = syncStore.toPureJSON();
1320+
const jsonStore = globalSyncStore.toPureJSON();
13211321

13221322
const types = [
13231323
'clusters',
@@ -1343,7 +1343,9 @@ export class SyncManager extends Singleton {
13431343
await this.updateCatalogEntities(dataCloud, resources, jsonStore);
13441344

13451345
// save the models in the store
1346-
Object.keys(jsonStore).forEach((key) => (syncStore[key] = jsonStore[key]));
1346+
Object.keys(jsonStore).forEach(
1347+
(key) => (globalSyncStore[key] = jsonStore[key])
1348+
);
13471349

13481350
this.ipcMain.capture(
13491351
'info',

Diff for: ‎src/main/main.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Main } from '@k8slens/extensions';
22
import { observable } from 'mobx';
3-
import { cloudStore } from '../store/CloudStore';
4-
import { syncStore } from '../store/SyncStore';
3+
import { globalCloudStore } from '../store/CloudStore';
4+
import { globalSyncStore } from '../store/SyncStore';
55
import { IpcMain } from './IpcMain';
66
import { logger as loggerUtil } from '../util/logger';
77
import { SyncManager } from './SyncManager';
@@ -46,8 +46,8 @@ export default class ExtensionMain extends Main.LensExtension {
4646

4747
const ipcMain = IpcMain.createInstance(this);
4848

49-
cloudStore.loadExtension(this, { ipcMain });
50-
syncStore.loadExtension(this, { ipcMain });
49+
globalCloudStore.loadExtension(this, { ipcMain });
50+
globalSyncStore.loadExtension(this, { ipcMain });
5151

5252
// AFTER load stores
5353
SyncManager.createInstance({

Diff for: ‎src/renderer/components/GlobalSyncPage/EnhancedTable/__tests__/EnhancedTable.spec.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { EnhancedTable } from '../EnhancedTable';
55
import { Cloud } from '../../../../../common/Cloud'; // MOCKED
66
import { CloudProvider } from '../../../../store/CloudProvider';
77
import { IpcRenderer } from '../../../../IpcRenderer';
8-
import { CloudStore } from '../../../../../store/CloudStore';
8+
import { globalCloudStore } from '../../../../../store/CloudStore';
99

1010
jest.mock('../../../../../common/Cloud');
1111

@@ -14,6 +14,7 @@ describe('/renderer/components/GlobalSyncPage/EnhancedTable/EnhancedTable', () =
1414
let fakeCloudFoo;
1515
let fakeCloudBar;
1616
let user;
17+
let ipcRenderer;
1718

1819
beforeEach(() => {
1920
user = userEvent.setup();
@@ -55,8 +56,8 @@ describe('/renderer/components/GlobalSyncPage/EnhancedTable/EnhancedTable', () =
5556
],
5657
});
5758

58-
const ipcRenderer = IpcRenderer.createInstance(extension);
59-
CloudStore.createInstance().loadExtension(extension, { ipcRenderer });
59+
ipcRenderer = IpcRenderer.createInstance(extension);
60+
globalCloudStore.loadExtension(extension, { ipcRenderer });
6061
});
6162

6263
[true, false].forEach((isSelectiveSyncView) => {

Diff for: ‎src/renderer/components/GlobalSyncPage/EnhancedTable/__tests__/EnhancedTableRow.spec.js

+3-5
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,15 @@ import { EnhancedTableRow } from '../EnhancedTableRow';
55
import { Cloud, CONNECTION_STATUSES } from '../../../../../common/Cloud'; // MOCKED
66
import { CloudProvider } from '../../../../store/CloudProvider';
77
import { IpcRenderer } from '../../../../IpcRenderer';
8-
import { CloudStore } from '../../../../../store/CloudStore';
8+
import { globalCloudStore } from '../../../../../store/CloudStore';
99
import * as strings from '../../../../../strings';
1010

1111
jest.mock('../../../../../common/Cloud');
1212

1313
describe('/renderer/components/GlobalSyncPage/EnhancedTable/EnhancedTable', () => {
1414
const extension = {};
1515
let user;
16+
let ipcRenderer;
1617

1718
const colorGreen = {
1819
color: 'var(--colorSuccess)',
@@ -22,13 +23,12 @@ describe('/renderer/components/GlobalSyncPage/EnhancedTable/EnhancedTable', () =
2223
color: 'var(--halfGray)',
2324
};
2425

25-
let ipcRenderer;
26-
2726
beforeEach(() => {
2827
user = userEvent.setup();
2928
mockConsole(['log', 'info', 'warn']); // automatically restored after each test
3029

3130
ipcRenderer = IpcRenderer.createInstance(extension);
31+
globalCloudStore.loadExtension(extension, { ipcRenderer });
3232
});
3333

3434
describe('render', () => {
@@ -60,8 +60,6 @@ describe('/renderer/components/GlobalSyncPage/EnhancedTable/EnhancedTable', () =
6060
name: 'bar',
6161
cloudUrl: 'http://bar.com',
6262
});
63-
64-
CloudStore.createInstance().loadExtension(extension, { ipcRenderer });
6563
});
6664

6765
[true, false].forEach((isSyncStarted) => {

Diff for: ‎src/renderer/components/GlobalSyncPage/EnhancedTable/__tests__/TableRowListenerWrapper.spec.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,22 @@ import {
99
} from '../../../../../common/Cloud'; // MOCKED
1010
import { CloudProvider } from '../../../../store/CloudProvider';
1111
import { IpcRenderer } from '../../../../IpcRenderer';
12-
import { CloudStore } from '../../../../../store/CloudStore';
12+
import { globalCloudStore } from '../../../../../store/CloudStore';
1313
import * as strings from '../../../../../strings';
1414

1515
jest.mock('../../../../../common/Cloud');
1616

1717
describe('/renderer/components/GlobalSyncPage/EnhancedTable/TableRowListenerWrapper', () => {
1818
const extension = {};
1919
let user;
20+
let ipcRenderer;
2021

2122
beforeEach(() => {
2223
user = userEvent.setup();
2324
mockConsole(['log', 'info', 'warn']); // automatically restored after each test
2425

25-
const ipcRenderer = IpcRenderer.createInstance(extension);
26-
CloudStore.createInstance().loadExtension(extension, { ipcRenderer });
26+
ipcRenderer = IpcRenderer.createInstance(extension);
27+
globalCloudStore.loadExtension(extension, { ipcRenderer });
2728
});
2829

2930
(function () {

Diff for: ‎src/renderer/components/GlobalSyncPage/__tests__/AddCloudInstance.spec.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { AddCloudInstance } from '../AddCloudInstance';
55
import { MOCK_CONNECT_FAILURE_CLOUD_NAME } from '../../../../common/Cloud'; // MOCKED
66
import { CloudProvider } from '../../../store/CloudProvider';
77
import { IpcRenderer } from '../../../IpcRenderer';
8-
import { CloudStore } from '../../../../store/CloudStore';
8+
import { globalCloudStore } from '../../../../store/CloudStore';
99
import * as strings from '../../../../strings';
1010

1111
jest.mock('../../../../common/Cloud');
@@ -76,7 +76,7 @@ describe('/renderer/components/GlobalSyncPage/AddCloudInstance', () => {
7676
mockConsole(['log', 'info', 'warn']); // automatically restored after each test
7777

7878
const ipcRenderer = IpcRenderer.createInstance(extension);
79-
CloudStore.createInstance().loadExtension(extension, { ipcRenderer });
79+
globalCloudStore.loadExtension(extension, { ipcRenderer });
8080
});
8181

8282
describe('render', () => {

Diff for: ‎src/renderer/components/GlobalSyncPage/__tests__/ConnectionBlock.spec.js

+6-39
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { ConnectionBlock } from '../ConnectionBlock';
55
import { mkCloudJson, CONNECTION_STATUSES } from '../../../../common/Cloud'; // MOCKED
66
import { CloudProvider } from '../../../store/CloudProvider';
77
import { IpcRenderer } from '../../../IpcRenderer';
8-
import { CloudStore } from '../../../../store/CloudStore';
8+
import { globalCloudStore } from '../../../../store/CloudStore';
99
import * as strings from '../../../../strings';
1010

1111
jest.mock('../../../../common/Cloud');
@@ -29,13 +29,10 @@ describe('/renderer/components/GlobalSyncPage/ConnectionBlock', () => {
2929
mockConsole(); // automatically restored after each test
3030

3131
ipcRenderer = IpcRenderer.createInstance(extension);
32+
globalCloudStore.loadExtension(extension, { ipcRenderer });
3233
});
3334

3435
describe('renders', () => {
35-
beforeEach(() => {
36-
CloudStore.createInstance().loadExtension(extension, { ipcRenderer });
37-
});
38-
3936
it('renders connection block', () => {
4037
render(<TestConnectionBlockComponent loading={false} />);
4138

@@ -128,10 +125,6 @@ describe('/renderer/components/GlobalSyncPage/ConnectionBlock', () => {
128125
});
129126

130127
describe('triggers', () => {
131-
beforeEach(() => {
132-
CloudStore.createInstance().loadExtension(extension, { ipcRenderer });
133-
});
134-
135128
it('triggers setUrl() handler by changing cluster url input', async () => {
136129
render(<TestConnectionBlockComponent loading={false} />);
137130

@@ -188,7 +181,6 @@ describe('/renderer/components/GlobalSyncPage/ConnectionBlock', () => {
188181

189182
describe('validators', () => {
190183
let fakeFooBarCloudJson;
191-
let fakeBarFooCloudJson;
192184

193185
beforeEach(() => {
194186
fakeFooBarCloudJson = mkCloudJson({
@@ -209,30 +201,9 @@ describe('/renderer/components/GlobalSyncPage/ConnectionBlock', () => {
209201
},
210202
],
211203
});
212-
213-
fakeBarFooCloudJson = mkCloudJson({
214-
__mockStatus: CONNECTION_STATUSES.CONNECTED,
215-
name: 'barfoo',
216-
cloudUrl: 'http://barfoo.com',
217-
namespaces: [
218-
{
219-
cloudUrl: 'https://barfoo.com',
220-
clusterCount: 4,
221-
credentialCount: 4,
222-
licenseCount: 1,
223-
machineCount: 12,
224-
name: 'barfoo namespace',
225-
proxyCount: 0,
226-
sshKeyCount: 2,
227-
synced: true,
228-
},
229-
],
230-
});
231204
});
232205

233206
it('shows error message if name is invalid', async () => {
234-
CloudStore.createInstance().loadExtension(extension, { ipcRenderer });
235-
236207
render(<TestConnectionBlockComponent loading={false} />);
237208

238209
const testInvalidName = '...';
@@ -261,14 +232,12 @@ describe('/renderer/components/GlobalSyncPage/ConnectionBlock', () => {
261232
});
262233

263234
it('shows error message if cloud with incoming name already synced', async () => {
264-
CloudStore.initStore('cloud-store', {
235+
globalCloudStore.fromStore({
265236
clouds: {
266237
'http://foobar.com': fakeFooBarCloudJson,
267238
},
268239
});
269240

270-
CloudStore.createInstance().loadExtension(extension, { ipcRenderer });
271-
272241
render(<TestConnectionBlockComponent loading={false} />);
273242

274243
const testAlreadySyncedName = 'foobar';
@@ -294,17 +263,15 @@ describe('/renderer/components/GlobalSyncPage/ConnectionBlock', () => {
294263
});
295264

296265
it('shows error message if cloud with incoming url already synced', async () => {
297-
CloudStore.initStore('cloud-store', {
266+
globalCloudStore.fromStore({
298267
clouds: {
299-
'http://barfoo.com': fakeBarFooCloudJson,
268+
'http://foobar.com': fakeFooBarCloudJson,
300269
},
301270
});
302271

303-
CloudStore.createInstance().loadExtension(extension, { ipcRenderer });
304-
305272
render(<TestConnectionBlockComponent loading={false} />);
306273

307-
const testAlreadySyncedUrl = 'http://barfoo.com';
274+
const testAlreadySyncedUrl = 'http://foobar.com';
308275

309276
const inputUrlEl = document.getElementById('cclex-cluster-url');
310277

0 commit comments

Comments
 (0)