diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartBuildQueryRegistrySingleton.js b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartBuildQueryRegistrySingleton.js index eed835cce6bcd..86ed784fee1be 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartBuildQueryRegistrySingleton.js +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartBuildQueryRegistrySingleton.js @@ -2,7 +2,7 @@ import { Registry, makeSingleton } from '@superset-ui/core'; class ChartBuildQueryRegistry extends Registry { constructor() { - super('ChartBuildQuery'); + super({ name: 'ChartBuildQuery' }); } } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartComponentRegistrySingleton.js b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartComponentRegistrySingleton.js index aea7630bf7b64..3ff82e50d7b8b 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartComponentRegistrySingleton.js +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartComponentRegistrySingleton.js @@ -2,7 +2,7 @@ import { Registry, makeSingleton } from '@superset-ui/core'; class ChartComponentRegistry extends Registry { constructor() { - super('ChartComponent'); + super({ name: 'ChartComponent' }); } } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartMetadataRegistrySingleton.js b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartMetadataRegistrySingleton.js index 0f9c7695389c0..02dc35c7ff273 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartMetadataRegistrySingleton.js +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartMetadataRegistrySingleton.js @@ -2,7 +2,7 @@ import { Registry, makeSingleton } from '@superset-ui/core'; class ChartMetadataRegistry extends Registry { constructor() { - super('ChartMetadata'); + super({ name: 'ChartMetadata' }); } } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartTransformPropsRegistrySingleton.js b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartTransformPropsRegistrySingleton.js index df724ef7b5636..915814918a4a5 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartTransformPropsRegistrySingleton.js +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/registries/ChartTransformPropsRegistrySingleton.js @@ -2,7 +2,7 @@ import { Registry, makeSingleton } from '@superset-ui/core'; class ChartTransformPropsRegistry extends Registry { constructor() { - super('ChartTransformProps'); + super({ name: 'ChartTransformProps' }); } } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/package.json b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/package.json index ff3904b7a8e8f..f7b4aee17bcb0 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/package.json +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/package.json @@ -25,6 +25,9 @@ "publishConfig": { "access": "public" }, + "devDependencies": { + "jest-mock-console": "^0.4.0" + }, "dependencies": { "lodash": "^4.17.11" } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Registry.js b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Registry.js index 00c0e7602c91b..e897593d82591 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Registry.js +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Registry.js @@ -1,5 +1,14 @@ +/* eslint no-console: 0 */ + +const OverwritePolicy = { + ALLOW: 'ALLOW', + PROHIBIT: 'PROHIBIT', + WARN: 'WARN', +}; + export default class Registry { - constructor(name = '') { + constructor({ name = '', overwritePolicy = OverwritePolicy.ALLOW } = {}) { + this.overwritePolicy = overwritePolicy; this.name = name; this.items = {}; this.promises = {}; @@ -20,6 +29,13 @@ export default class Registry { registerValue(key, value) { const item = this.items[key]; + if (item && item.value !== value) { + if (this.overwritePolicy === OverwritePolicy.WARN) { + console.warn(`Item with key "${key}" already exists. You are assigning a new value.`); + } else if (this.overwritePolicy === OverwritePolicy.PROHIBIT) { + throw new Error(`Item with key "${key}" already exists. Cannot overwrite.`); + } + } if (!item || item.value !== value) { this.items[key] = { value }; delete this.promises[key]; @@ -30,6 +46,13 @@ export default class Registry { registerLoader(key, loader) { const item = this.items[key]; + if (item && item.loader !== loader) { + if (this.overwritePolicy === OverwritePolicy.WARN) { + console.warn(`Item with key "${key}" already exists. You are assigning a new value.`); + } else if (this.overwritePolicy === OverwritePolicy.PROHIBIT) { + throw new Error(`Item with key "${key}" already exists. Cannot overwrite.`); + } + } if (!item || item.loader !== loader) { this.items[key] = { loader }; delete this.promises[key]; @@ -122,3 +145,5 @@ export default class Registry { return this; } } + +Registry.OverwritePolicy = OverwritePolicy; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/RegistryWithDefaultKey.js b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/RegistryWithDefaultKey.js index a88d76624d80c..50ebdf421fd97 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/RegistryWithDefaultKey.js +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/RegistryWithDefaultKey.js @@ -1,8 +1,8 @@ import Registry from './Registry'; export default class RegistryWithDefaultKey extends Registry { - constructor({ name, initialDefaultKey = undefined, setFirstItemAsDefault = false } = {}) { - super(name); + constructor({ initialDefaultKey = undefined, setFirstItemAsDefault = false, ...rest } = {}) { + super(rest); this.initialDefaultKey = initialDefaultKey; this.defaultKey = initialDefaultKey; this.setFirstItemAsDefault = setFirstItemAsDefault; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Registry.test.js b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Registry.test.js index 83f3e0975809b..c5353d83e0219 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Registry.test.js +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Registry.test.js @@ -1,3 +1,5 @@ +/* eslint no-console: 0 */ +import mockConsole from 'jest-mock-console'; import Registry from '../../src/models/Registry'; describe('Registry', () => { @@ -5,13 +7,13 @@ describe('Registry', () => { expect(Registry !== undefined).toBe(true); }); - describe('new Registry(name)', () => { - it('can create a new registry when name is not given', () => { + describe('new Registry(config)', () => { + it('can create a new registry when config.name is not given', () => { const registry = new Registry(); expect(registry).toBeInstanceOf(Registry); }); - it('can create a new registry when name is given', () => { - const registry = new Registry('abc'); + it('can create a new registry when config.name is given', () => { + const registry = new Registry({ name: 'abc' }); expect(registry).toBeInstanceOf(Registry); expect(registry.name).toBe('abc'); }); @@ -265,4 +267,79 @@ describe('Registry', () => { expect(registry.remove('a')).toBe(registry); }); }); + + describe('config.overwritePolicy', () => { + describe('=ALLOW', () => { + describe('.registerValue(key, value)', () => { + it('registers normally', () => { + const restoreConsole = mockConsole(); + const registry = new Registry(); + registry.registerValue('a', 'testValue'); + expect(() => registry.registerValue('a', 'testValue2')).not.toThrow(); + expect(registry.get('a')).toEqual('testValue2'); + expect(console.warn).not.toHaveBeenCalled(); + restoreConsole(); + }); + }); + describe('.registerLoader(key, loader)', () => { + it('registers normally', () => { + const restoreConsole = mockConsole(); + const registry = new Registry(); + registry.registerLoader('a', () => 'testValue'); + expect(() => registry.registerLoader('a', () => 'testValue2')).not.toThrow(); + expect(registry.get('a')).toEqual('testValue2'); + expect(console.warn).not.toHaveBeenCalled(); + restoreConsole(); + }); + }); + }); + describe('=WARN', () => { + describe('.registerValue(key, value)', () => { + it('warns when overwrite', () => { + const restoreConsole = mockConsole(); + const registry = new Registry({ + overwritePolicy: Registry.OverwritePolicy.WARN, + }); + registry.registerValue('a', 'testValue'); + expect(() => registry.registerValue('a', 'testValue2')).not.toThrow(); + expect(registry.get('a')).toEqual('testValue2'); + expect(console.warn).toHaveBeenCalled(); + restoreConsole(); + }); + }); + describe('.registerLoader(key, loader)', () => { + it('warns when overwrite', () => { + const restoreConsole = mockConsole(); + const registry = new Registry({ + overwritePolicy: Registry.OverwritePolicy.WARN, + }); + registry.registerLoader('a', () => 'testValue'); + expect(() => registry.registerLoader('a', () => 'testValue2')).not.toThrow(); + expect(registry.get('a')).toEqual('testValue2'); + expect(console.warn).toHaveBeenCalled(); + restoreConsole(); + }); + }); + }); + describe('=PROHIBIT', () => { + describe('.registerValue(key, value)', () => { + it('throws error when overwrite', () => { + const registry = new Registry({ + overwritePolicy: Registry.OverwritePolicy.PROHIBIT, + }); + registry.registerValue('a', 'testValue'); + expect(() => registry.registerValue('a', 'testValue2')).toThrow(); + }); + }); + describe('.registerLoader(key, loader)', () => { + it('warns when overwrite', () => { + const registry = new Registry({ + overwritePolicy: Registry.OverwritePolicy.PROHIBIT, + }); + registry.registerLoader('a', () => 'testValue'); + expect(() => registry.registerLoader('a', () => 'testValue2')).toThrow(); + }); + }); + }); + }); }); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/RegistryWithDefaultKey.test.js b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/RegistryWithDefaultKey.test.js index 117d6aebf09b1..c0b53ba45d389 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/RegistryWithDefaultKey.test.js +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/RegistryWithDefaultKey.test.js @@ -12,7 +12,7 @@ describe('RegistryWithDefaultKey', () => { registry = new RegistryWithDefaultKey(); }); - describe('new RegistryWithDefaultKey(options)', () => { + describe('new RegistryWithDefaultKey(config)', () => { it('returns a class that extends from Registry', () => { expect(registry).toBeInstanceOf(Registry); }); @@ -61,7 +61,7 @@ describe('RegistryWithDefaultKey', () => { }); }); - describe('options.defaultKey', () => { + describe('config.defaultKey', () => { describe('when not set', () => { it(`After creation, default key is undefined`, () => { expect(registry.defaultKey).toBeUndefined(); @@ -72,14 +72,14 @@ describe('RegistryWithDefaultKey', () => { expect(registry.getDefaultKey()).toBeUndefined(); }); }); - describe('when options.initialDefaultKey is set', () => { + describe('when config.initialDefaultKey is set', () => { const registry2 = new RegistryWithDefaultKey({ initialDefaultKey: 'def', }); it(`After creation, default key is undefined`, () => { expect(registry2.defaultKey).toEqual('def'); }); - it('.clear() reset defaultKey to this options.defaultKey', () => { + it('.clear() reset defaultKey to this config.defaultKey', () => { registry2.setDefaultKey('abc'); registry2.clear(); expect(registry2.getDefaultKey()).toEqual('def'); @@ -87,7 +87,7 @@ describe('RegistryWithDefaultKey', () => { }); }); - describe('options.setFirstItemAsDefault', () => { + describe('config.setFirstItemAsDefault', () => { describe('when true', () => { const registry2 = new RegistryWithDefaultKey({ setFirstItemAsDefault: true }); beforeEach(() => {