From 29df573a94122197c6082eb7b8df4dc0b643709e Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> Date: Fri, 25 Jun 2021 02:17:19 -0700 Subject: [PATCH] feat(core): Registry listeners (#1191) * feat(core): registry listeners + hook * remove hook * tests * equivalent function * name changes * test broken listeners --- .../superset-ui-core/src/models/Registry.ts | 60 ++++++++++++-- .../test/models/Registry.test.ts | 79 +++++++++++++++++++ 2 files changed, 133 insertions(+), 6 deletions(-) diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Registry.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Registry.ts index 714458382490..90fc5875555a 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Registry.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Registry.ts @@ -30,6 +30,24 @@ interface ItemWithLoader { loader: () => T; } +/** + * Type of value returned from loader function when using registerLoader() + */ +type InclusiveLoaderResult = V | Promise; + +export type RegistryValue> = V | W | undefined; + +export type RegistryEntry> = { + key: string; + value: RegistryValue; +}; + +/** + * A listener is called whenever a registry's entries change. + * Keys indicates which entries been affected. + */ +export type Listener = (keys: string[]) => void; + export interface RegistryConfig { name?: string; overwritePolicy?: OverwritePolicy; @@ -41,12 +59,11 @@ export interface RegistryConfig { * Can use generic to specify type of item in the registry * @type V Type of value * @type W Type of value returned from loader function when using registerLoader(). - * W can be either V, Promise or V | Promise * Set W=V when does not support asynchronous loader. * By default W is set to V | Promise to support * both synchronous and asynchronous loaders. */ -export default class Registry = V | Promise> { +export default class Registry = InclusiveLoaderResult> { name: string; overwritePolicy: OverwritePolicy; @@ -59,17 +76,23 @@ export default class Registry = V | Promise> { [key: string]: Promise; }; + listeners: Set; + constructor(config: RegistryConfig = {}) { const { name = '', overwritePolicy = OverwritePolicy.ALLOW } = config; this.name = name; this.overwritePolicy = overwritePolicy; this.items = {}; this.promises = {}; + this.listeners = new Set(); } clear() { + const keys = this.keys(); + this.items = {}; this.promises = {}; + this.notifyListeners(keys); return this; } @@ -95,6 +118,7 @@ export default class Registry = V | Promise> { if (!item || willOverwrite) { this.items[key] = { value }; delete this.promises[key]; + this.notifyListeners([key]); } return this; @@ -115,6 +139,7 @@ export default class Registry = V | Promise> { if (!item || willOverwrite) { this.items[key] = { loader }; delete this.promises[key]; + this.notifyListeners([key]); } return this; @@ -152,7 +177,7 @@ export default class Registry = V | Promise> { getMap() { return this.keys().reduce<{ - [key: string]: V | W | undefined; + [key: string]: RegistryValue; }>((prev, key) => { const map = prev; map[key] = this.get(key); @@ -180,7 +205,7 @@ export default class Registry = V | Promise> { return Object.keys(this.items); } - values(): (V | W | undefined)[] { + values(): RegistryValue[] { return this.keys().map(key => this.get(key)); } @@ -188,7 +213,7 @@ export default class Registry = V | Promise> { return Promise.all(this.keys().map(key => this.getAsPromise(key))); } - entries(): { key: string; value: V | W | undefined }[] { + entries(): RegistryEntry[] { return this.keys().map(key => ({ key, value: this.get(key), @@ -198,7 +223,7 @@ export default class Registry = V | Promise> { entriesAsPromise(): Promise<{ key: string; value: V }[]> { const keys = this.keys(); - return Promise.all(keys.map(key => this.getAsPromise(key))).then(values => + return this.valuesAsPromise().then(values => values.map((value, i) => ({ key: keys[i], value, @@ -207,9 +232,32 @@ export default class Registry = V | Promise> { } remove(key: string) { + const isChange = this.has(key); delete this.items[key]; delete this.promises[key]; + if (isChange) { + this.notifyListeners([key]); + } return this; } + + addListener(listener: Listener) { + this.listeners.add(listener); + } + + removeListener(listener: Listener) { + this.listeners.delete(listener); + } + + private notifyListeners(keys: string[]) { + this.listeners.forEach(listener => { + try { + listener(keys); + } catch (e) { + // eslint-disable-next-line no-console + console.error('Exception thrown from a registry listener:', e); + } + }); + } } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Registry.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Registry.test.ts index a0c08ec10b5a..a7d0a0aa0977 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Registry.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Registry.test.ts @@ -353,4 +353,83 @@ describe('Registry', () => { }); }); }); + + describe('listeners', () => { + let registry = new Registry(); + let listener = jest.fn(); + beforeEach(() => { + registry = new Registry(); + listener = jest.fn(); + registry.addListener(listener); + }); + + it('calls the listener when a value is registered', () => { + registry.registerValue('foo', 'bar'); + expect(listener).toBeCalledWith(['foo']); + }); + + it('calls the listener when a loader is registered', () => { + registry.registerLoader('foo', () => 'bar'); + expect(listener).toBeCalledWith(['foo']); + }); + + it('calls the listener when a value is overriden', () => { + registry.registerValue('foo', 'bar'); + listener.mockClear(); + registry.registerValue('foo', 'baz'); + expect(listener).toBeCalledWith(['foo']); + }); + + it('calls the listener when a value is removed', () => { + registry.registerValue('foo', 'bar'); + listener.mockClear(); + registry.remove('foo'); + expect(listener).toBeCalledWith(['foo']); + }); + + it('does not call the listener when a value is not actually removed', () => { + registry.remove('foo'); + expect(listener).not.toBeCalled(); + }); + + it('calls the listener when registry is cleared', () => { + registry.registerValue('foo', 'bar'); + registry.registerLoader('fluz', () => 'baz'); + listener.mockClear(); + registry.clear(); + expect(listener).toBeCalledWith(['foo', 'fluz']); + }); + + it('removes listeners correctly', () => { + registry.removeListener(listener); + registry.registerValue('foo', 'bar'); + expect(listener).not.toBeCalled(); + }); + + describe('with a broken listener', () => { + let restoreConsole: any; + beforeEach(() => { + restoreConsole = mockConsole(); + }); + afterEach(() => { + restoreConsole(); + }); + + it('keeps working', () => { + const errorListener = jest.fn().mockImplementation(() => { + throw new Error('test error'); + }); + const lastListener = jest.fn(); + + registry.addListener(errorListener); + registry.addListener(lastListener); + registry.registerValue('foo', 'bar'); + + expect(listener).toBeCalledWith(['foo']); + expect(errorListener).toBeCalledWith(['foo']); + expect(lastListener).toBeCalledWith(['foo']); + expect(console.error).toBeCalled(); + }); + }); + }); });