Add RegistryWithDefaultKey class (#27)

* import main lodash instead of lodash/fp

* add factory and unit test

* more unit test to increase branch coverage

* refactor color

* update unit test and references

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-color/src/CategoricalColorNamespace.js

@@ -9,7 +9,7 @@ export default class CategoricalColorNamespace {
   }
 
   getScale(schemeName) {
-    const name = schemeName || getCategoricalSchemeRegistry().getDefaultSchemeName();
+    const name = schemeName || getCategoricalSchemeRegistry().getDefaultKey();
     const scale = this.scales[name];
     if (scale) {
       return scale;

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-color/src/ColorSchemeRegistry.js

@@ -1,46 +1,10 @@
-import { Registry } from '@superset-ui/core';
+import { RegistryWithDefaultKey } from '@superset-ui/core';
 
-class ColorSchemeRegistry extends Registry {
-  clear() {
-    super.clear();
-    this.defaultSchemeName = undefined;
-
-    return this;
-  }
-
-  getDefaultSchemeName() {
-    return this.defaultSchemeName;
-  }
-
-  setDefaultSchemeName(schemeName) {
-    this.defaultSchemeName = schemeName;
-
-    return this;
-  }
-
-  get(schemeName) {
-    return super.get(schemeName || this.defaultSchemeName);
-  }
-
-  registerValue(schemeName, colors) {
-    super.registerValue(schemeName, colors);
-    // If there is no default, set as default
-    if (!this.defaultSchemeName) {
-      this.defaultSchemeName = schemeName;
-    }
-
-    return this;
-  }
-
-  registerLoader(schemeName, loader) {
-    super.registerLoader(schemeName, loader);
-    // If there is no default, set as default
-    if (!this.defaultSchemeName) {
-      this.defaultSchemeName = schemeName;
-    }
-
-    return this;
+export default class ColorSchemeRegistry extends RegistryWithDefaultKey {
+  constructor() {
+    super({
+      name: 'ColorScheme',
+      setFirstItemAsDefault: true,
+    });
   }
 }
-
-export default ColorSchemeRegistry;

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-color/test/ColorSchemeRegistry.test.js

@@ -1,97 +1,8 @@
 import ColorSchemeRegistry from '../src/ColorSchemeRegistry';
-import CategoricalScheme from '../src/CategoricalScheme';
 
 describe('ColorSchemeRegistry', () => {
-  const registry = new ColorSchemeRegistry();
-  const SCHEME1 = new CategoricalScheme({
-    name: 'test',
-    colors: ['red', 'green', 'blue'],
-  });
-  const SCHEME2 = new CategoricalScheme({
-    name: 'test2',
-    colors: ['orange', 'yellow', 'pink'],
-  });
-  const SCHEME3 = new CategoricalScheme({
-    name: 'test3',
-    colors: ['cyan', 'magenta'],
-  });
-
-  beforeEach(() => {
-    registry.clear();
-    registry.registerValue('test', SCHEME1);
-    registry.registerValue('test2', SCHEME2);
-    registry.setDefaultSchemeName('test');
-  });
-  describe('.clear()', () => {
-    it('clears the entries', () => {
-      registry.clear();
-      expect(registry.has('test')).toBeFalsy();
-    });
-    it('clears default scheme name', () => {
-      registry.clear();
-      expect(registry.getDefaultSchemeName()).toBeUndefined();
-    });
-    it('returns the ColorSchemeRegistry instance', () => {
-      const instance = registry.clear();
-      expect(instance).toBe(registry);
-    });
-  });
-  describe('.get()', () => {
-    it('.get() returns default color scheme', () => {
-      const scheme = registry.get();
-      expect(scheme).toEqual(SCHEME1);
-    });
-    it('.get(name) returns color scheme with specified name', () => {
-      const scheme = registry.get('test2');
-      expect(scheme).toEqual(SCHEME2);
-    });
-  });
-  describe('.getDefaultSchemeName()', () => {
-    it('returns default scheme name', () => {
-      const name = registry.getDefaultSchemeName();
-      expect(name).toBe('test');
-    });
-  });
-  describe('.setDefaultSchemeName()', () => {
-    it('set default scheme name', () => {
-      registry.setDefaultSchemeName('test2');
-      const name = registry.getDefaultSchemeName();
-      expect(name).toBe('test2');
-      registry.setDefaultSchemeName('test');
-    });
-    it('returns the ColorSchemeRegistry instance', () => {
-      const instance = registry.setDefaultSchemeName('test');
-      expect(instance).toBe(registry);
-    });
-  });
-  describe('.registerValue(name, colors)', () => {
-    it('sets schemename and color', () => {
-      registry.registerValue('test3', SCHEME3);
-      const scheme = registry.get('test3');
-      expect(scheme).toEqual(SCHEME3);
-    });
-    it('registers this scheme name as default if there is no default scheme', () => {
-      registry.clear().registerValue('test3', SCHEME3);
-      expect(registry.getDefaultSchemeName()).toEqual('test3');
-    });
-    it('returns the ColorSchemeRegistry instance', () => {
-      const instance = registry.registerValue('test3', SCHEME3);
-      expect(instance).toBe(registry);
-    });
-  });
-  describe('.registerLoader(name, colorsLoader)', () => {
-    it('sets schemename and color', () => {
-      registry.registerLoader('test3', () => SCHEME3);
-      const scheme = registry.get('test3');
-      expect(scheme).toEqual(SCHEME3);
-    });
-    it('registers this scheme name as default if there is no default scheme', () => {
-      registry.clear().registerLoader('test3', () => SCHEME3);
-      expect(registry.getDefaultSchemeName()).toEqual('test3');
-    });
-    it('returns the ColorSchemeRegistry instance', () => {
-      const instance = registry.registerLoader('test3', () => SCHEME3);
-      expect(instance).toBe(registry);
-    });
+  it('exists', () => {
+    expect(ColorSchemeRegistry).toBeDefined();
+    expect(ColorSchemeRegistry).toBeInstanceOf(Function);
   });
 });

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/index.js

@@ -1,6 +1,7 @@
 export { default as Plugin } from './models/Plugin';
 export { default as Preset } from './models/Preset';
 export { default as Registry } from './models/Registry';
+export { default as RegistryWithDefaultKey } from './models/RegistryWithDefaultKey';
 
 export { default as convertKeysToCamelCase } from './utils/convertKeysToCamelCase';
 export { default as isDefined } from './utils/isDefined';

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/RegistryWithDefaultKey.js

@@ -0,0 +1,51 @@
+import Registry from './Registry';
+
+export default class RegistryWithDefaultKey extends Registry {
+  constructor({ name, initialDefaultKey = undefined, setFirstItemAsDefault = false } = {}) {
+    super(name);
+    this.initialDefaultKey = initialDefaultKey;
+    this.defaultKey = initialDefaultKey;
+    this.setFirstItemAsDefault = setFirstItemAsDefault;
+  }
+
+  clear() {
+    super.clear();
+    this.defaultKey = this.initialDefaultKey;
+
+    return this;
+  }
+
+  get(key) {
+    return super.get(key || this.defaultKey);
+  }
+
+  registerValue(key, value) {
+    super.registerValue(key, value);
+    // If there is no default, set as default
+    if (this.setFirstItemAsDefault && !this.defaultKey) {
+      this.defaultKey = key;
+    }
+
+    return this;
+  }
+
+  registerLoader(key, loader) {
+    super.registerLoader(key, loader);
+    // If there is no default, set as default
+    if (this.setFirstItemAsDefault && !this.defaultKey) {
+      this.defaultKey = key;
+    }
+
+    return this;
+  }
+
+  getDefaultKey() {
+    return this.defaultKey;
+  }
+
+  setDefaultKey(key) {
+    this.defaultKey = key;
+
+    return this;
+  }
+}

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/utils/convertKeysToCamelCase.js

@@ -1,13 +1,13 @@
-import camelCase from 'lodash/fp/camelCase';
-import isPlainObject from 'lodash/fp/isPlainObject';
-import mapKeys from 'lodash/fp/mapKeys';
+import camelCase from 'lodash/camelCase';
+import isPlainObject from 'lodash/isPlainObject';
+import mapKeys from 'lodash/mapKeys';
 
 export default function convertKeysToCamelCase(object) {
   if (object === null || object === undefined) {
     return object;
   }
   if (isPlainObject(object)) {
-    return mapKeys(k => camelCase(k), object);
+    return mapKeys(object, (_, k) => camelCase(k));
   }
   throw new Error(`Cannot convert input that is not a plain object: ${object}`);
 }

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/index.test.js

@@ -2,6 +2,7 @@ import {
   Plugin,
   Preset,
   Registry,
+  RegistryWithDefaultKey,
   convertKeysToCamelCase,
   isDefined,
   isRequired,
@@ -14,6 +15,7 @@ describe('index', () => {
       Plugin,
       Preset,
       Registry,
+      RegistryWithDefaultKey,
       convertKeysToCamelCase,
       isDefined,
       isRequired,

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Registry.test.js

@@ -52,6 +52,18 @@ describe('Registry', () => {
       expect(registry.has('a')).toBe(true);
       expect(registry.get('a')).toBe('testValue');
     });
+    it('does not overwrite if value is exactly the same', () => {
+      const registry = new Registry();
+      const value = { a: 1 };
+      registry.registerValue('a', value);
+      const promise1 = registry.getAsPromise('a');
+      registry.registerValue('a', value);
+      const promise2 = registry.getAsPromise('a');
+      expect(promise1).toBe(promise2);
+      registry.registerValue('a', { a: 1 });
+      const promise3 = registry.getAsPromise('a');
+      expect(promise1).not.toBe(promise3);
+    });
     it('returns the registry itself', () => {
       const registry = new Registry();
       expect(registry.registerValue('a', 'testValue')).toBe(registry);
@@ -65,6 +77,18 @@ describe('Registry', () => {
       expect(registry.has('a')).toBe(true);
       expect(registry.get('a')).toBe('testValue');
     });
+    it('does not overwrite if loader is exactly the same', () => {
+      const registry = new Registry();
+      const loader = () => 'testValue';
+      registry.registerLoader('a', loader);
+      const promise1 = registry.getAsPromise('a');
+      registry.registerLoader('a', loader);
+      const promise2 = registry.getAsPromise('a');
+      expect(promise1).toBe(promise2);
+      registry.registerLoader('a', () => 'testValue');
+      const promise3 = registry.getAsPromise('a');
+      expect(promise1).not.toBe(promise3);
+    });
     it('returns the registry itself', () => {
       const registry = new Registry();
       expect(registry.registerLoader('a', () => 'testValue')).toBe(registry);