Add data structures for chart plugin system (#6028)

* add unit tests

* add test structure

* add unit tests for Registry

* add LoaderRegistry unit test

* add unit test for makeSingleton

* add type check

* add plugin data structures

* simplify API

* add preset tests

* update test message

* fix lint

* update makeSingleton

* update Plugin, Preset and unit test

* revise Registry code

* update unit test, add remove function

* update test

* update unit test

* update plugin unit test

* add .keys(), .entries() and .entriesAsPromise()

* update test description

superset/assets/spec/javascripts/modules/Registry_spec.js

@@ -0,0 +1,175 @@
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+import Registry from '../../../src/modules/Registry';
+
+describe('Registry', () => {
+  it('exists', () => {
+    expect(Registry !== undefined).to.equal(true);
+  });
+
+  describe('new Registry(name)', () => {
+    it('can create a new registry when name is not given', () => {
+      const registry = new Registry();
+      expect(registry).to.be.instanceOf(Registry);
+    });
+    it('can create a new registry when name is given', () => {
+      const registry = new Registry('abc');
+      expect(registry).to.be.instanceOf(Registry);
+      expect(registry.name).to.equal('abc');
+    });
+  });
+
+  describe('.has(key)', () => {
+    it('returns true if an item with the given key exists', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'testValue');
+      expect(registry.has('a')).to.equal(true);
+      registry.registerLoader('b', () => 'testValue2');
+      expect(registry.has('b')).to.equal(true);
+    });
+    it('returns false if an item with the given key does not exist', () => {
+      const registry = new Registry();
+      expect(registry.has('a')).to.equal(false);
+    });
+  });
+
+  describe('.registerValue(key, value)', () => {
+    it('registers the given value with the given key', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'testValue');
+      expect(registry.has('a')).to.equal(true);
+      expect(registry.get('a')).to.equal('testValue');
+    });
+    it('returns the registry itself', () => {
+      const registry = new Registry();
+      expect(registry.registerValue('a', 'testValue')).to.equal(registry);
+    });
+  });
+
+  describe('.registerLoader(key, loader)', () => {
+    it('registers the given loader with the given key', () => {
+      const registry = new Registry();
+      registry.registerLoader('a', () => 'testValue');
+      expect(registry.has('a')).to.equal(true);
+      expect(registry.get('a')).to.equal('testValue');
+    });
+    it('returns the registry itself', () => {
+      const registry = new Registry();
+      expect(registry.registerLoader('a', () => 'testValue')).to.equal(registry);
+    });
+  });
+
+  describe('.get(key)', () => {
+    it('given the key, returns the value if the item is a value', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'testValue');
+      expect(registry.get('a')).to.equal('testValue');
+    });
+    it('given the key, returns the result of the loader function if the item is a loader', () => {
+      const registry = new Registry();
+      registry.registerLoader('b', () => 'testValue2');
+      expect(registry.get('b')).to.equal('testValue2');
+    });
+    it('returns null if the item with specified key does not exist', () => {
+      const registry = new Registry();
+      expect(registry.get('a')).to.equal(null);
+    });
+    it('If the key was registered multiple times, returns the most recent item.', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'testValue');
+      expect(registry.get('a')).to.equal('testValue');
+      registry.registerLoader('a', () => 'newValue');
+      expect(registry.get('a')).to.equal('newValue');
+    });
+  });
+
+  describe('.getAsPromise(key)', () => {
+    it('given the key, returns a promise of item value if the item is a value', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'testValue');
+      return registry.getAsPromise('a').then((value) => {
+        expect(value).to.equal('testValue');
+      });
+    });
+    it('given the key, returns a promise of result of the loader function if the item is a loader ', () => {
+      const registry = new Registry();
+      registry.registerLoader('a', () => 'testValue');
+      return registry.getAsPromise('a').then((value) => {
+        expect(value).to.equal('testValue');
+      });
+    });
+    it('returns a rejected promise if the item with specified key does not exist', () => {
+      const registry = new Registry();
+      return registry.getAsPromise('a').then(null, (err) => {
+        expect(err).to.equal('Item with key "a" is not registered.');
+      });
+    });
+    it('If the key was registered multiple times, returns a promise of the most recent item.', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'testValue');
+      const promise1 = registry.getAsPromise('a').then((value) => {
+        expect(value).to.equal('testValue');
+      });
+      registry.registerLoader('a', () => 'newValue');
+      const promise2 = registry.getAsPromise('a').then((value) => {
+        expect(value).to.equal('newValue');
+      });
+      return Promise.all([promise1, promise2]);
+    });
+  });
+
+  describe('.keys()', () => {
+    it('returns an array of keys', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'testValue');
+      registry.registerLoader('b', () => 'test2');
+      expect(registry.keys()).to.deep.equal(['a', 'b']);
+    });
+  });
+
+  describe('.entries()', () => {
+    it('returns an array of { key, value }', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'test1');
+      registry.registerLoader('b', () => 'test2');
+      expect(registry.entries()).to.deep.equal([
+        { key: 'a', value: 'test1' },
+        { key: 'b', value: 'test2' },
+      ]);
+    });
+  });
+
+  describe('.entriesAsPromise()', () => {
+    it('returns a Promise of an array { key, value }', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'test1');
+      registry.registerLoader('b', () => 'test2');
+      registry.registerLoader('c', () => Promise.resolve('test3'));
+      return registry.entriesAsPromise().then((entries) => {
+        expect(entries).to.deep.equal([
+          { key: 'a', value: 'test1' },
+          { key: 'b', value: 'test2' },
+          { key: 'c', value: 'test3' },
+        ]);
+      });
+    });
+  });
+
+  describe('.remove(key)', () => {
+    it('removes the item with given key', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'testValue');
+      registry.remove('a');
+      expect(registry.get('a')).to.equal(null);
+    });
+    it('does not throw error if the key does not exist', () => {
+      const registry = new Registry();
+      expect(() => registry.remove('a')).to.not.throw();
+    });
+    it('returns itself', () => {
+      const registry = new Registry();
+      registry.registerValue('a', 'testValue');
+      expect(registry.remove('a')).to.equal(registry);
+    });
+  });
+});

superset/assets/spec/javascripts/utils/isRequired_spec.js

@@ -0,0 +1,9 @@
+import { it, describe } from 'mocha';
+import { expect } from 'chai';
+import isRequired from '../../../src/utils/isRequired';
+
+describe('isRequired(field)', () => {
+  it('should throw error with the given field in the message', () => {
+    expect(() => isRequired('myField')).to.throw(Error, 'myField is required.');
+  });
+});

superset/assets/spec/javascripts/utils/makeSingleton_spec.js

@@ -0,0 +1,38 @@
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+import makeSingleton from '../../../src/utils/makeSingleton';
+
+describe('makeSingleton()', () => {
+  class Dog {
+    constructor(name) {
+      this.name = name;
+    }
+    sit() {
+      this.isSitting = true;
+    }
+  }
+  describe('makeSingleton(BaseClass)', () => {
+    const getInstance = makeSingleton(Dog);
+
+    it('returns a function for getting singleton instance of a given base class', () => {
+      expect(getInstance).to.be.a('Function');
+      expect(getInstance()).to.be.instanceOf(Dog);
+    });
+    it('returned function returns same instance across all calls', () => {
+      expect(getInstance()).to.equal(getInstance());
+    });
+  });
+  describe('makeSingleton(BaseClass, ...args)', () => {
+    const getInstance = makeSingleton(Dog, 'Doug');
+
+    it('returns a function for getting singleton instance of a given base class constructed with the given arguments', () => {
+      expect(getInstance).to.be.a('Function');
+      expect(getInstance()).to.be.instanceOf(Dog);
+      expect(getInstance().name).to.equal('Doug');
+    });
+    it('returned function returns same instance across all calls', () => {
+      expect(getInstance()).to.equal(getInstance());
+    });
+  });
+
+});

superset/assets/spec/javascripts/visualizations/models/ChartPlugin_spec.js

@@ -0,0 +1,42 @@
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+import ChartPlugin from '../../../../src/visualizations/core/models/ChartPlugin';
+import ChartMetadata from '../../../../src/visualizations/core/models/ChartMetadata';
+
+describe('ChartPlugin', () => {
+  const metadata = new ChartMetadata({});
+
+  it('exists', () => {
+    expect(ChartPlugin).to.not.equal(undefined);
+  });
+
+  describe('new ChartPlugin()', () => {
+    it('creates a new plugin', () => {
+      const plugin = new ChartPlugin({
+        metadata,
+        Chart() {},
+      });
+      expect(plugin).to.be.instanceof(ChartPlugin);
+    });
+    it('throws an error if metadata is not specified', () => {
+      expect(() => new ChartPlugin()).to.throw(Error);
+    });
+    it('throws an error if none of Chart or loadChart is specified', () => {
+      expect(() => new ChartPlugin({ metadata })).to.throw(Error);
+    });
+  });
+
+  describe('.register(key)', () => {
+    const plugin = new ChartPlugin({
+      metadata,
+      Chart() {},
+    });
+    it('throws an error if key is not provided', () => {
+      expect(() => plugin.register()).to.throw(Error);
+      expect(() => plugin.configure({ key: 'abc' }).register()).to.not.throw(Error);
+    });
+    it('returns itself', () => {
+      expect(plugin.configure({ key: 'abc' }).register()).to.equal(plugin);
+    });
+  });
+});

superset/assets/spec/javascripts/visualizations/models/Plugin_spec.js

@@ -0,0 +1,48 @@
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+import Plugin from '../../../../src/visualizations/core/models/Plugin';
+
+describe('Plugin', () => {
+  it('exists', () => {
+    expect(Plugin).to.not.equal(undefined);
+  });
+
+  describe('new Plugin()', () => {
+    it('creates a new plugin', () => {
+      const plugin = new Plugin();
+      expect(plugin).to.be.instanceof(Plugin);
+    });
+  });
+
+  describe('.configure(config, replace)', () => {
+    it('extends the default config with given config when replace is not set or false', () => {
+      const plugin = new Plugin();
+      plugin.configure({ key: 'abc', foo: 'bar' });
+      plugin.configure({ key: 'def' });
+      expect(plugin.config).to.deep.equal({ key: 'def', foo: 'bar' });
+    });
+    it('replaces the default config with given config when replace is true', () => {
+      const plugin = new Plugin();
+      plugin.configure({ key: 'abc', foo: 'bar' });
+      plugin.configure({ key: 'def' }, true);
+      expect(plugin.config).to.deep.equal({ key: 'def' });
+    });
+    it('returns the plugin itself', () => {
+      const plugin = new Plugin();
+      expect(plugin.configure({ key: 'abc' })).to.equal(plugin);
+    });
+  });
+
+  describe('.resetConfig()', () => {
+    it('resets config back to default', () => {
+      const plugin = new Plugin();
+      plugin.configure({ key: 'abc', foo: 'bar' });
+      plugin.resetConfig();
+      expect(plugin.config).to.deep.equal({});
+    });
+    it('returns the plugin itself', () => {
+      const plugin = new Plugin();
+      expect(plugin.resetConfig()).to.equal(plugin);
+    });
+  });
+});

superset/assets/spec/javascripts/visualizations/models/Preset_spec.js

@@ -0,0 +1,65 @@
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+import Preset from '../../../../src/visualizations/core/models/Preset';
+import Plugin from '../../../../src/visualizations/core/models/Plugin';
+
+describe('Preset', () => {
+  it('exists', () => {
+    expect(Preset).to.not.equal(undefined);
+  });
+
+  describe('new Preset()', () => {
+    it('creates new preset', () => {
+      const preset = new Preset();
+      expect(preset).to.be.instanceOf(Preset);
+    });
+  });
+
+  describe('.register()', () => {
+    it('register all listed presets then plugins', () => {
+      const values = [];
+      class Plugin1 extends Plugin {
+        register() {
+          values.push(1);
+        }
+      }
+      class Plugin2 extends Plugin {
+        register() {
+          values.push(2);
+        }
+      }
+      class Plugin3 extends Plugin {
+        register() {
+          values.push(3);
+        }
+      }
+      class Plugin4 extends Plugin {
+        register() {
+          const { key } = this.config;
+          values.push(key);
+        }
+      }
+
+      const preset1 = new Preset({
+        plugins: [new Plugin1()],
+      });
+      const preset2 = new Preset({
+        plugins: [new Plugin2()],
+      });
+      const preset3 = new Preset({
+        presets: [preset1, preset2],
+        plugins: [
+          new Plugin3(),
+          new Plugin4().configure({ key: 'abc' }),
+        ],
+      });
+      preset3.register();
+      expect(values).to.deep.equal([1, 2, 3, 'abc']);
+    });
+
+    it('returns itself', () => {
+      const preset = new Preset();
+      expect(preset.register()).to.equal(preset);
+    });
+  });
+});

superset/assets/src/modules/Registry.js

@@ -0,0 +1,72 @@
+export default class Registry {
+  constructor(name = '') {
+    this.name = name;
+    this.items = {};
+    this.promises = {};
+  }
+
+  has(key) {
+    const item = this.items[key];
+    return item !== null && item !== undefined;
+  }
+
+  registerValue(key, value) {
+    this.items[key] = { value };
+    delete this.promises[key];
+    return this;
+  }
+
+  registerLoader(key, loader) {
+    this.items[key] = { loader };
+    delete this.promises[key];
+    return this;
+  }
+
+  get(key) {
+    const item = this.items[key];
+    if (item) {
+      return item.loader ? item.loader() : item.value;
+    }
+    return null;
+  }
+
+  getAsPromise(key) {
+    const promise = this.promises[key];
+    if (promise) {
+      return promise;
+    }
+    const item = this.get(key);
+    if (item) {
+      const newPromise = Promise.resolve(item);
+      this.promises[key] = newPromise;
+      return newPromise;
+    }
+    return Promise.reject(`Item with key "${key}" is not registered.`);
+  }
+
+  keys() {
+    return Object.keys(this.items);
+  }
+
+  entries() {
+    return this.keys().map(key => ({
+      key,
+      value: this.get(key),
+    }));
+  }
+
+  entriesAsPromise() {
+    const keys = this.keys();
+    return Promise.all(keys.map(key => this.getAsPromise(key)))
+      .then(values => values.map((value, i) => ({
+        key: keys[i],
+        value,
+      })));
+  }
+
+  remove(key) {
+    delete this.items[key];
+    delete this.promises[key];
+    return this;
+  }
+}

superset/assets/src/utils/isRequired.js

@@ -0,0 +1,3 @@
+export default function isRequired(field) {
+  throw new Error(`${field} is required.`);
+}

superset/assets/src/utils/makeSingleton.js

@@ -0,0 +1,10 @@
+export default function makeSingleton(BaseClass, ...args) {
+  let singleton;
+
+  return function getInstance() {
+    if (!singleton) {
+      singleton = new BaseClass(...args);
+    }
+    return singleton;
+  };
+}

superset/assets/src/visualizations/core/models/ChartMetadata.js

@@ -0,0 +1,13 @@
+export default class ChartMetadata {
+  constructor({
+    name,
+    description,
+    thumbnail,
+    show = true,
+  }) {
+    this.name = name;
+    this.description = description;
+    this.thumbnail = thumbnail;
+    this.show = show;
+  }
+}

superset/assets/src/visualizations/core/models/ChartPlugin.js

@@ -0,0 +1,43 @@
+import Plugin from './Plugin';
+import isRequired from '../../../utils/isRequired';
+import getChartMetadataRegistry from '../registries/ChartMetadataRegistrySingleton';
+import getChartComponentRegistry from '../registries/ChartComponentRegistrySingleton';
+import getChartTransformPropsRegistry from '../registries/ChartTransformPropsRegistrySingleton';
+
+const IDENTITY = x => x;
+
+export default class ChartPlugin extends Plugin {
+  constructor({
+    metadata = isRequired('metadata'),
+
+    // use transformProps for immediate value
+    transformProps = IDENTITY,
+    // use loadTransformProps for dynamic import (lazy-loading)
+    loadTransformProps,
+
+    // use Chart for immediate value
+    Chart,
+    // use loadChart for dynamic import (lazy-loading)
+    loadChart,
+  } = {}) {
+    super();
+    this.metadata = metadata;
+    this.loadTransformProps = loadTransformProps || (() => transformProps);
+
+    if (loadChart) {
+      this.loadChart = loadChart;
+    } else if (Chart) {
+      this.loadChart = () => Chart;
+    } else {
+      throw new Error('Chart or loadChart is required');
+    }
+  }
+
+  register() {
+    const { key = isRequired('config.key') } = this.config;
+    getChartMetadataRegistry().registerValue(key, this.metadata);
+    getChartComponentRegistry().registerLoader(key, this.loadChart);
+    getChartTransformPropsRegistry().registerLoader(key, this.loadTransformProps);
+    return this;
+  }
+}

superset/assets/src/visualizations/core/models/Plugin.js

@@ -0,0 +1,25 @@
+export default class Plugin {
+  constructor() {
+    this.resetConfig();
+  }
+
+  resetConfig() {
+    // The child class can set default config
+    // by overriding this function.
+    this.config = {};
+    return this;
+  }
+
+  configure(config, replace = false) {
+    if (replace) {
+      this.config = config;
+    } else {
+      this.config = { ...this.config, ...config };
+    }
+    return this;
+  }
+
+  register() {
+    return this;
+  }
+}

superset/assets/src/visualizations/core/models/Preset.js

@@ -0,0 +1,23 @@
+export default class Preset {
+  constructor({
+    name = '',
+    description = '',
+    presets = [],
+    plugins = [],
+  } = {}) {
+    this.name = name;
+    this.description = description;
+    this.presets = presets;
+    this.plugins = plugins;
+  }
+
+  register() {
+    this.presets.forEach((preset) => {
+      preset.register();
+    });
+    this.plugins.forEach((plugin) => {
+      plugin.register();
+    });
+    return this;
+  }
+}

superset/assets/src/visualizations/core/registries/ChartComponentRegistrySingleton.js

@@ -0,0 +1,12 @@
+import Registry from '../../../modules/Registry';
+import makeSingleton from '../../../utils/makeSingleton';
+
+class ChartComponentRegistry extends Registry {
+  constructor() {
+    super('ChartComponent');
+  }
+}
+
+const getInstance = makeSingleton(ChartComponentRegistry);
+
+export default getInstance;

superset/assets/src/visualizations/core/registries/ChartMetadataRegistrySingleton.js

@@ -0,0 +1,12 @@
+import Registry from '../../../modules/Registry';
+import makeSingleton from '../../../utils/makeSingleton';
+
+class ChartMetadataRegistry extends Registry {
+  constructor() {
+    super('ChartMetadata');
+  }
+}
+
+const getInstance = makeSingleton(ChartMetadataRegistry);
+
+export default getInstance;

superset/assets/src/visualizations/core/registries/ChartTransformPropsRegistrySingleton.js

@@ -0,0 +1,12 @@
+import Registry from '../../../modules/Registry';
+import makeSingleton from '../../../utils/makeSingleton';
+
+class ChartTransformPropsRegistry extends Registry {
+  constructor() {
+    super('ChartTransformProps');
+  }
+}
+
+const getInstance = makeSingleton(ChartTransformPropsRegistry);
+
+export default getInstance;