refactor(nucleus): consolidate get/create (#236)

qlik-oss · Dec 11, 2019 · 0359ab6 · 0359ab6
1 parent efefa0d
commit 0359ab6
Show file tree

Hide file tree

Showing 20 changed files with 268 additions and 103 deletions.
diff --git a/apis/nucleus/src/components/Cell.jsx b/apis/nucleus/src/components/Cell.jsx
@@ -16,13 +16,13 @@ import useLayout from '../hooks/useLayout';
 import LocaleContext from '../contexts/LocaleContext';
 import { createObjectSelectionAPI } from '../selections';
 
-const initialState = {
+const initialState = err => ({
   loading: false,
   loaded: false,
   longRunningQuery: false,
-  error: null,
+  error: err ? { title: err.message } : null,
   sn: null,
-};
+});
 
 const contentReducer = (state, action) => {
   // console.log('content reducer', action.type);
@@ -157,22 +157,25 @@ const loadType = async ({ dispatch, types, name, version, layout, model, app })
   return undefined;
 };
 
-const Cell = forwardRef(({ nebulaContext, model, initialSnContext, initialSnOptions, onMount }, ref) => {
+const Cell = forwardRef(({ nebulaContext, model, initialSnContext, initialSnOptions, initialError, onMount }, ref) => {
   const {
     app,
     nebbie: { types },
   } = nebulaContext;
 
   const translator = useContext(LocaleContext);
   const theme = useTheme();
-  const [state, dispatch] = useReducer(contentReducer, initialState);
+  const [state, dispatch] = useReducer(contentReducer, initialState(initialError));
   const [layout, validating, cancel, retry] = useLayout({ app, model });
   const [contentRef, contentRect, , contentNode] = useRect();
   const [snContext, setSnContext] = useState(initialSnContext);
   const [snOptions, setSnOptions] = useState(initialSnOptions);
   const cellRef = useRef();
 
   useEffect(() => {
+    if (initialError) {
+      return undefined;
+    }
     const validate = sn => {
       const [showError, error] = validateTargets(translator, layout, sn.generator.qae.data);
       if (showError) {

diff --git a/apis/nucleus/src/components/glue.jsx b/apis/nucleus/src/components/glue.jsx
@@ -2,7 +2,15 @@ import React from 'react';
 import ReactDOM from 'react-dom';
 import Cell from './Cell';
 
-export default function glue({ nebulaContext, element, model, initialSnContext, initialSnOptions, onMount }) {
+export default function glue({
+  nebulaContext,
+  element,
+  model,
+  initialSnContext,
+  initialSnOptions,
+  onMount,
+  initialError,
+}) {
   const { root } = nebulaContext;
   const cellRef = React.createRef();
   const portal = ReactDOM.createPortal(
@@ -12,6 +20,7 @@ export default function glue({ nebulaContext, element, model, initialSnContext,
       model={model}
       initialSnContext={initialSnContext}
       initialSnOptions={initialSnOptions}
+      initialError={initialError}
       onMount={onMount}
     />,
     element,

diff --git a/apis/nucleus/src/index.js b/apis/nucleus/src/index.js
@@ -105,14 +105,17 @@ function nuked(configuration = {}, prev = {}) {
 
   const config = {
     env: {
+      // env is provided as is to the supernova method
+      // consider it part of the public API
       Promise,
       translator: locale.translator,
       nucleus, // eslint-disable-line no-use-before-define
     },
     load: configuration.load,
+    logger,
   };
 
-  const types = typesFn({ logger, config, parent: prev.types });
+  const types = typesFn({ config, parent: prev.types });
 
   configuration.types.forEach(t =>
     types.register(

diff --git a/apis/nucleus/src/object/__tests__/create-object.spec.js b/apis/nucleus/src/object/__tests__/create-object.spec.js
@@ -6,26 +6,37 @@ describe('create-object', () => {
   let create;
   let populator;
   let sandbox;
+  let init;
+  let objectModel;
   before(() => {
     sandbox = sinon.createSandbox();
     populator = sandbox.stub();
-    [{ default: create }] = aw.mock([['**/populator.js', () => populator]], ['../create-object.js']);
+    init = sandbox.stub();
+    [{ default: create }] = aw.mock(
+      [
+        ['**/populator.js', () => populator],
+        ['**/initiate.js', () => init],
+      ],
+      ['../create-object.js']
+    );
   });
 
   beforeEach(() => {
+    objectModel = { id: 'id' };
     types = {
       get: sandbox.stub(),
     };
     context = {
       app: {
-        createSessionObject: sandbox.stub().returns(Promise.resolve({ id: 'id' })),
+        createSessionObject: sandbox.stub().returns(Promise.resolve(objectModel)),
       },
       nebbie: {
-        get: sandbox.stub().returns('got it'),
         types,
       },
     };
 
+    init.returns('api');
+
     sn = { qae: { properties: { onChange: sandbox.stub() } } };
     merged = { m: 'true' };
     const t = {
@@ -72,17 +83,28 @@ describe('create-object', () => {
     expect(context.app.createSessionObject).to.have.been.calledWithExactly(merged);
   });
 
-  it('should call nebbie get', async () => {
-    const ret = await create({ type: 't', version: 'v', fields: 'f' }, { properties: 'props', x: 'a' }, context);
-    expect(ret).to.equal('got it');
-    expect(context.nebbie.get).to.have.been.calledWithExactly(
-      {
-        id: 'id',
-      },
-      {
-        x: 'a',
-        properties: {},
-      }
-    );
+  it('should create a dummy session object when error is thrown', async () => {
+    types.get.throws('oops');
+    await create({ type: 't', version: 'v', fields: 'f' }, { properties: 'props' }, context);
+    expect(context.app.createSessionObject).to.have.been.calledWithExactly({
+      qInfo: { qType: 't' },
+      visualization: 't',
+    });
+  });
+
+  it('should call init', async () => {
+    const optional = { properties: 'props', x: 'a' };
+    const ret = await create({ type: 't', version: 'v', fields: 'f' }, optional, context);
+    expect(ret).to.equal('api');
+    expect(init).to.have.been.calledWithExactly(objectModel, optional, context, undefined);
+  });
+
+  it('should catch and pass error', async () => {
+    const err = new Error('oops');
+    types.get.throws(err);
+    const optional = { properties: 'props' };
+    const ret = await create({ type: 't' }, optional, context);
+    expect(ret).to.equal('api');
+    expect(init).to.have.been.calledWithExactly(objectModel, optional, context, err);
   });
 });
diff --git a/apis/nucleus/src/object/__tests__/get-object.spec.js b/apis/nucleus/src/object/__tests__/get-object.spec.js
@@ -0,0 +1,42 @@
+describe('get-object', () => {
+  const optional = 'optional';
+  let context = {};
+  let create;
+  let sandbox;
+  let init;
+  let objectModel;
+  before(() => {
+    sandbox = sinon.createSandbox();
+    init = sandbox.stub();
+    [{ default: create }] = aw.mock([['**/initiate.js', () => init]], ['../get-object.js']);
+  });
+
+  beforeEach(() => {
+    objectModel = sandbox.stub();
+    init.returns('api');
+    context = { app: { id: 'appid', getObject: id => Promise.resolve(objectModel(id)) } };
+  });
+
+  afterEach(() => {
+    sandbox.reset();
+  });
+
+  it('should get object from app only once', async () => {
+    const model = 'model-x';
+    objectModel.withArgs('x').returns(model);
+    const spy = sandbox.spy(context.app, 'getObject');
+    await create({ id: 'x' }, optional, context);
+    await create({ id: 'x' }, optional, context);
+    await create({ id: 'x' }, optional, context);
+    expect(spy.callCount).to.equal(1);
+    expect(spy).to.have.been.calledWithExactly('x');
+  });
+
+  it('should call init', async () => {
+    const model = 'model-x';
+    objectModel.withArgs('x').returns(model);
+    const ret = await create({ id: 'x' }, optional, context);
+    expect(ret).to.equal('api');
+    expect(init).to.have.been.calledWithExactly(model, optional, context);
+  });
+});
diff --git a/apis/nucleus/src/object/__tests__/initiate.spec.js b/apis/nucleus/src/object/__tests__/initiate.spec.js
@@ -0,0 +1,49 @@
+describe('initiate api', () => {
+  const optional = 'optional';
+  const context = 'context';
+  const model = 'model';
+  let create;
+  let sandbox;
+  let viz;
+  let api;
+  before(() => {
+    sandbox = sinon.createSandbox();
+    viz = sandbox.stub();
+    [{ default: create }] = aw.mock([['**/viz.js', () => viz]], ['../initiate.js']);
+  });
+
+  beforeEach(() => {
+    api = {
+      mount: sandbox.stub(),
+      options: sandbox.stub(),
+      context: sandbox.stub(),
+    };
+    viz.returns(api);
+  });
+
+  afterEach(() => {
+    sandbox.reset();
+  });
+
+  it('should call viz api', async () => {
+    const initialError = 'err';
+    const ret = await create(model, optional, context, initialError);
+    expect(viz).to.have.been.calledWithExactly({ model, context, initialError });
+    expect(ret).to.equal(api);
+  });
+
+  it('should call mount when element is provided ', async () => {
+    await create(model, { element: 'el' }, context);
+    expect(api.mount).to.have.been.calledWithExactly('el');
+  });
+
+  it('should call options when provided ', async () => {
+    await create(model, { options: 'opts' }, context);
+    expect(api.options).to.have.been.calledWithExactly('opts');
+  });
+
+  it('should call context when provided ', async () => {
+    await create(model, { context: 'ctx' }, context);
+    expect(api.context).to.have.been.calledWithExactly('ctx');
+  });
+});
diff --git a/apis/nucleus/src/object/create-object.js b/apis/nucleus/src/object/create-object.js
@@ -1,4 +1,5 @@
 import populateData from './populator';
+import init from './initiate';
 
 /**
  * @typedef {object} CreateObjectConfig
@@ -8,28 +9,39 @@ import populateData from './populator';
  */
 
 export default async function create({ type, version, fields }, optional, context) {
-  const t = context.nebbie.types.get({ name: type, version });
-  const mergedProps = await t.initialProperties(optional.properties);
-  const sn = await t.supernova();
-  if (fields) {
-    populateData(
-      {
-        sn,
-        properties: mergedProps,
-        fields,
+  let mergedProps = {};
+  let error;
+  try {
+    const t = context.nebbie.types.get({ name: type, version });
+    mergedProps = await t.initialProperties(optional.properties);
+    const sn = await t.supernova();
+    if (fields) {
+      populateData(
+        {
+          sn,
+          properties: mergedProps,
+          fields,
+        },
+        context
+      );
+    }
+    if (optional.properties && sn && sn.qae.properties.onChange) {
+      sn.qae.properties.onChange.call({}, mergedProps);
+    }
+  } catch (e) {
+    error = e;
+    // minimal dummy object properties to allow it to be created
+    // and rendered with the error
+    mergedProps = {
+      qInfo: {
+        qType: type,
       },
-      context
-    );
-  }
-  if (optional.properties && sn && sn.qae.properties.onChange) {
-    sn.qae.properties.onChange.call({}, mergedProps);
+      visualization: type,
+    };
+    // console.error(e); // eslint-disable-line
   }
 
   const model = await context.app.createSessionObject(mergedProps);
-  return context.nebbie.get(
-    {
-      id: model.id,
-    },
-    { ...optional, properties: {} }
-  );
+
+  return init(model, optional, context, error);
 }
diff --git a/apis/nucleus/src/object/get-object.js b/apis/nucleus/src/object/get-object.js
@@ -1,4 +1,4 @@
-import vizualizationAPI from '../viz';
+import init from './initiate';
 
 const cache = {};
 /**
@@ -17,19 +17,6 @@ const cache = {};
 export default async function initiate({ id }, optional, context) {
   const cacheKey = `${context.app.id}/${id}`;
   const model = cache[cacheKey] || (await context.app.getObject(id));
-  const api = vizualizationAPI({
-    model,
-    context,
-  });
-  if (optional.element) {
-    await api.mount(optional.element);
-  }
-  if (optional.options) {
-    api.options(optional.options);
-  }
-  if (optional.context) {
-    api.context(optional.context);
-  }
   cache[cacheKey] = model;
-  return api;
+  return init(model, optional, context);
 }
diff --git a/apis/nucleus/src/object/initiate.js b/apis/nucleus/src/object/initiate.js
@@ -0,0 +1,20 @@
+import vizualizationAPI from '../viz';
+
+export default async function(model, optional, context, initialError) {
+  const api = vizualizationAPI({
+    model,
+    context,
+    initialError,
+  });
+  if (optional.element) {
+    await api.mount(optional.element);
+  }
+  if (optional.options) {
+    api.options(optional.options);
+  }
+  if (optional.context) {
+    api.context(optional.context);
+  }
+
+  return api;
+}
diff --git a/apis/nucleus/src/sn/load.js b/apis/nucleus/src/sn/load.js
@@ -12,7 +12,7 @@ const LOADED = {};
 export async function load(name, version, config, loader) {
   const key = `${name}__${version}`;
   if (!LOADED[key]) {
-    const sKey = `${name}${version && ` v${version}`}`;
+    const sKey = `${name}${(version && ` v${version}`) || ''}`;
     const p = (loader || config.load)(
       {
         name,