fix: allow extension to register multiple publish targets (#4424)

* build extension bundle when building client * enable re-use of plugin host * require name and description for publish plugins * update publish plugins * add multiple publish plugins in sample-ui-plugin * update azure publish description * wrap file path in quote for windows compat * fix tests
microsoft · Oct 19, 2020 · 20efe39 · 20efe39
1 parent ed02171
commit 20efe39
Show file tree

Hide file tree

Showing 38 changed files with 582 additions and 239 deletions.
diff --git a/Composer/packages/client/__tests__/pages/publish/Publish.test.tsx b/Composer/packages/client/__tests__/pages/publish/Publish.test.tsx
@@ -35,6 +35,7 @@ const state = {
       name: 'azurePublish',
       description: 'azure publish',
       instructions: 'plugin instruction',
+      extensionId: 'azurePublish',
       schema: {
         default: {
           test: 'test',

diff --git a/Composer/packages/client/package.json b/Composer/packages/client/package.json
@@ -9,7 +9,7 @@
   },
   "scripts": {
     "start": "yarn build:extension-bundles && node scripts/start.js",
-    "build": "node --max_old_space_size=4096 scripts/build.js",
+    "build": "yarn build:extension-bundles && node --max_old_space_size=4096 scripts/build.js",
     "build:extension-bundles": "webpack --config ./config/extensions.config.js --env production",
     "clean": "rimraf build",
     "test": "jest",

diff --git a/Composer/packages/client/src/components/PluginHost/PluginHost.tsx b/Composer/packages/client/src/components/PluginHost/PluginHost.tsx
@@ -2,14 +2,18 @@
 // Licensed under the MIT License.
 
 /** @jsx jsx */
-import { jsx, SerializedStyles } from '@emotion/core';
-import React, { useState, useEffect, useRef } from 'react';
+import { jsx, css, SerializedStyles } from '@emotion/core';
+import React, { useEffect, useRef } from 'react';
 import { Shell } from '@botframework-composer/types';
 import { PluginType } from '@bfc/extension-client';
 
 import { PluginAPI } from '../../plugins/api';
 
-import { iframeStyle } from './styles';
+export const iframeStyle = css`
+  height: 100%;
+  width: 100%;
+  border: 0;
+`;
 
 interface PluginHostProps {
   extraIframeStyles?: SerializedStyles[];
@@ -19,6 +23,11 @@ interface PluginHostProps {
   shell?: Shell;
 }
 
+function resetIframe(iframeDoc: Document) {
+  iframeDoc.head.innerHTML = '';
+  iframeDoc.body.innerHTML = '';
+}
+
 /** Binds closures around Composer client code to plugin iframe's window object */
 function attachPluginAPI(win: Window, type: PluginType, shell?: object) {
   const api = { ...PluginAPI[type], ...PluginAPI.auth };
@@ -45,20 +54,35 @@ function injectScript(doc: Document, id: string, src: string, async: boolean, on
  */
 export const PluginHost: React.FC<PluginHostProps> = (props) => {
   const targetRef = useRef<HTMLIFrameElement>(null);
-  const [isLoaded, setIsLoaded] = useState(false);
   const { extraIframeStyles = [], pluginType, pluginName, bundleId, shell } = props;
 
+  const loadBundle = (name: string, bundle: string, type: PluginType) => {
+    const iframeWindow = targetRef.current?.contentWindow as Window;
+    const iframeDocument = targetRef.current?.contentDocument as Document;
+
+    attachPluginAPI(iframeWindow, type, shell);
+
+    //load the bundle for the specified plugin
+    const pluginScriptId = `plugin-${type}-${name}`;
+    const bundleUri = `/api/extensions/${name}/${bundle}`;
+    // If plugin bundles end up being too large and block the client thread due to the load, enable the async flag on this call
+    injectScript(iframeDocument, pluginScriptId, bundleUri, false);
+  };
+
   useEffect(() => {
     // renders the plugin's UI inside of the iframe
-    if (pluginName && pluginType) {
-      const iframeDocument = targetRef.current?.contentDocument as Document;
+    if (pluginName && pluginType && targetRef.current) {
+      const iframeDocument = targetRef.current.contentDocument as Document;
+
+      // cleanup
+      resetIframe(iframeDocument);
 
-      // // load the preload script to setup the plugin API
+      // load the preload script to setup the plugin API
       injectScript(iframeDocument, 'preload-bundle', '/plugin-host-preload.js', false);
 
       const onPreloaded = (ev) => {
         if (ev.data === 'host-preload-complete') {
-          setIsLoaded(true);
+          loadBundle(pluginName, bundleId, pluginType);
         }
       };
 
@@ -68,29 +92,15 @@ export const PluginHost: React.FC<PluginHostProps> = (props) => {
         window.removeEventListener('message', onPreloaded);
       };
     }
-  }, [pluginName, pluginType, bundleId, targetRef]);
-
-  useEffect(() => {
-    if (isLoaded && pluginType && pluginName && bundleId) {
-      const iframeWindow = targetRef.current?.contentWindow as Window;
-      const iframeDocument = targetRef.current?.contentDocument as Document;
-
-      attachPluginAPI(iframeWindow, pluginType, shell);
-
-      //load the bundle for the specified plugin
-      const pluginScriptId = `plugin-${pluginType}-${pluginName}`;
-      const bundleUri = `/api/extensions/${pluginName}/${bundleId}`;
-      // If plugin bundles end up being too large and block the client thread due to the load, enable the async flag on this call
-      injectScript(iframeDocument, pluginScriptId, bundleUri, false);
-    }
-  }, [isLoaded]);
+  }, [pluginName, pluginType, bundleId]);
 
   // sync the shell to the iframe store when shell changes
   useEffect(() => {
-    if (isLoaded && targetRef.current) {
-      targetRef.current.contentWindow?.Composer.sync(shell);
+    const frameApi = targetRef.current?.contentWindow?.Composer;
+    if (frameApi && typeof frameApi.sync === 'function') {
+      frameApi.sync(shell);
     }
-  }, [isLoaded, shell]);
+  }, [shell]);
 
-  return <iframe ref={targetRef} css={[iframeStyle, ...extraIframeStyles]} title={`${pluginName} host`}></iframe>;
+  return <iframe ref={targetRef} css={[iframeStyle, ...extraIframeStyles]} title={`${pluginName} host`} />;
 };
diff --git a/Composer/packages/client/src/components/PluginHost/styles.ts b/Composer/packages/client/src/components/PluginHost/styles.ts
diff --git a/Composer/packages/client/src/pages/publish/createPublishTarget.tsx b/Composer/packages/client/src/pages/publish/createPublishTarget.tsx
@@ -65,12 +65,8 @@ const CreatePublishTarget: React.FC<CreatePublishTargetProps> = (props) => {
     }
   };
 
-  const instructions: string | undefined = useMemo((): string | undefined => {
-    return targetType ? props.types.find((t) => t.name === targetType)?.instructions : '';
-  }, [props.targets, targetType]);
-
-  const schema = useMemo(() => {
-    return targetType ? props.types.find((t) => t.name === targetType)?.schema : undefined;
+  const selectedTarget = useMemo(() => {
+    return props.types.find((t) => t.name === targetType);
   }, [props.targets, targetType]);
 
   const targetBundleId = useMemo(() => {
@@ -107,34 +103,34 @@ const CreatePublishTarget: React.FC<CreatePublishTargetProps> = (props) => {
   };
 
   const publishTargetContent = useMemo(() => {
-    if (targetBundleId && targetType) {
+    if (selectedTarget?.bundleId) {
       // render custom plugin view
       return (
         <PluginHost
-          bundleId={targetBundleId}
+          bundleId={selectedTarget.bundleId}
           extraIframeStyles={[customPublishUISurface]}
-          pluginName={targetType}
+          pluginName={selectedTarget.extensionId}
           pluginType="publish"
-        ></PluginHost>
+        />
       );
     }
     // render default instruction / schema editor view
     return (
       <Fragment>
-        {instructions && <p>{instructions}</p>}
+        {selectedTarget?.instructions && <p>{selectedTarget?.instructions}</p>}
         <div css={label}>{formatMessage('Publish Configuration')}</div>
         <JsonEditor
           key={targetType}
           editorSettings={userSettings.codeEditor}
           height={200}
-          schema={schema}
+          schema={selectedTarget?.schema}
           value={config}
           onChange={updateConfig}
         />
         <button hidden disabled={saveDisabled} type="submit" />
       </Fragment>
     );
-  }, [targetType, instructions, schema, targetBundleId, saveDisabled]);
+  }, [selectedTarget, targetType, saveDisabled]);
 
   return (
     <Fragment>

diff --git a/Composer/packages/client/src/recoilModel/types.ts b/Composer/packages/client/src/recoilModel/types.ts
@@ -30,6 +30,7 @@ export interface StorageFolder extends File {
 export interface PublishType {
   name: string;
   description: string;
+  extensionId: string;
   bundleId?: string;
   instructions?: string;
   schema?: JSONSchema7;

diff --git a/Composer/packages/extension/package.json b/Composer/packages/extension/package.json
@@ -16,7 +16,6 @@
     "@botframework-composer/test-utils": "*",
     "@types/express": "^4.17.6",
     "@types/fs-extra": "^9.0.1",
-    "@types/passport": "^1.0.3",
     "@types/path-to-regexp": "^1.7.0",
     "@types/tar": "^4.0.3",
     "json-schema": "^0.2.5",
@@ -25,6 +24,7 @@
   },
   "dependencies": {
     "@botframework-composer/types": "*",
+    "@types/passport": "^1.0.3",
     "debug": "^4.1.1",
     "fs-extra": "^9.0.1",
     "globby": "^11.0.0",

diff --git a/Composer/packages/extension/src/extensionContext.ts b/Composer/packages/extension/src/extensionContext.ts
@@ -70,18 +70,25 @@ class ExtensionContext {
 
   public async loadPlugin(name: string, description: string, thisPlugin: any) {
     log('Loading extension: %s', name);
-    const pluginRegistration = new ExtensionRegistration(this, name, description);
-    if (typeof thisPlugin.default === 'function') {
-      // the module exported just an init function
-      thisPlugin.default.call(null, pluginRegistration);
-    } else if (thisPlugin.default && thisPlugin.default.initialize) {
-      // the module exported an object with an initialize method
-      thisPlugin.default.initialize.call(null, pluginRegistration);
-    } else if (thisPlugin.initialize && typeof thisPlugin.initialize === 'function') {
-      // the module exported an object with an initialize method
-      thisPlugin.initialize.call(null, pluginRegistration);
-    } else {
-      throw new Error(formatMessage('Could not init plugin'));
+
+    try {
+      const pluginRegistration = new ExtensionRegistration(this, name, description);
+      if (typeof thisPlugin.default === 'function') {
+        // the module exported just an init function
+        await thisPlugin.default.call(null, pluginRegistration);
+      } else if (thisPlugin.default && thisPlugin.default.initialize) {
+        // the module exported an object with an initialize method
+        await thisPlugin.default.initialize.call(null, pluginRegistration);
+      } else if (thisPlugin.initialize && typeof thisPlugin.initialize === 'function') {
+        // the module exported an object with an initialize method
+        await thisPlugin.initialize.call(null, pluginRegistration);
+      } else {
+        throw new Error(formatMessage('Could not init plugin'));
+      }
+    } catch (err) {
+      log('Error loading extension: %s', name);
+      // eslint-disable-next-line no-console
+      console.error(err);
     }
   }
 

diff --git a/Composer/packages/extension/src/extensionRegistration.ts b/Composer/packages/extension/src/extensionRegistration.ts
@@ -58,12 +58,17 @@ export class ExtensionRegistration {
    * Publish related features
    *************************************************************************************/
   public async addPublishMethod(plugin: PublishPlugin) {
-    log('registering publish method', this.name);
-    this.context.extensions.publish[plugin.customName || this.name] = {
+    if (this.context.extensions.publish[plugin.name]) {
+      throw new Error(`Duplicate publish method. Cannot register publish method with name ${plugin.name}.`);
+    }
+
+    log('registering publish method', plugin.name);
+    this.context.extensions.publish[plugin.name] = {
       plugin: {
-        name: plugin.customName || this.name,
-        description: plugin.customDescription || this.description,
+        name: plugin.name,
+        description: plugin.description || this.description,
         instructions: plugin.instructions,
+        extensionId: this.name,
         bundleId: plugin.bundleId,
         schema: plugin.schema,
       },

diff --git a/Composer/packages/server/src/controllers/extensions.ts b/Composer/packages/server/src/controllers/extensions.ts
@@ -124,14 +124,21 @@ export async function getBundleForView(req: ExtensionViewBundleRequest, res: Res
   const extension = ExtensionManager.find(id);
 
   if (extension) {
-    const bundle = ExtensionManager.getBundle(id, bundleId);
-    if (bundle) {
-      res.sendFile(bundle);
-      return;
+    try {
+      const bundle = ExtensionManager.getBundle(id, bundleId);
+      if (bundle) {
+        res.sendFile(bundle);
+        return;
+      }
+    } catch (err) {
+      if (err.message && err.message.includes('not found')) {
+        res.status(404).json({ error: 'bundle not found' });
+        return;
+      }
     }
   }
 
-  res.status(404).json({ error: 'extension or bundle not found' });
+  res.status(404).json({ error: 'extension not found' });
 }
 
 export async function performExtensionFetch(req: ExtensionFetchRequest, res: Response) {

diff --git a/Composer/packages/server/src/controllers/publisher.ts b/Composer/packages/server/src/controllers/publisher.ts
@@ -20,6 +20,7 @@ export const PublishController = {
             description: plugin.description,
             instructions: plugin.instructions,
             schema: plugin.schema,
+            extensionId: plugin.extensionId,
             bundleId: plugin.bundleId,
             features: {
               history: typeof methods.history === 'function',

diff --git a/Composer/packages/types/package.json b/Composer/packages/types/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@botframework-composer/types",
-  "version": "0.0.1",
+  "version": "0.0.2",
   "description": "Shared types for Botframework Composer..",
   "main": "lib/index.js",
   "files": [
@@ -15,10 +15,10 @@
   "author": "andy.brown@microsoft.com",
   "license": "MIT",
   "dependencies": {
+    "@types/express": "^4.16.1",
     "json-schema": "^0.2.5"
   },
   "devDependencies": {
-    "@types/express": "^4.16.1",
     "eslint": "7.0.0",
     "rimraf": "^3.0.2",
     "typescript": "^3.8.3"

diff --git a/Composer/packages/types/src/extension.ts b/Composer/packages/types/src/extension.ts
@@ -87,6 +87,7 @@ export type ExtensionCollection = {
       plugin: {
         name: string;
         description: string;
+        extensionId: string;
         /** (Optional instructions displayed in the UI) */
         instructions?: string;
         /** (Optional) Schema for publishing configuration. */

diff --git a/Composer/packages/types/src/publish.ts b/Composer/packages/types/src/publish.ts
@@ -24,6 +24,9 @@ export type PublishResponse = {
 
 // TODO: Add types for project, metadata
 export type PublishPlugin<Config = any> = {
+  name: string;
+  description: string;
+
   // methods plugins should support
   publish: (config: Config, project: IBotProject, metadata: any, user?: UserIdentity) => Promise<PublishResponse>;
   getStatus?: (config: Config, project: IBotProject, user?: UserIdentity) => Promise<PublishResponse>;
@@ -38,8 +41,6 @@ export type PublishPlugin<Config = any> = {
   // other properties
   schema?: JSONSchema7;
   instructions?: string;
-  customName?: string;
-  customDescription?: string;
   bundleId?: string;
   [key: string]: any;
 };

diff --git a/Composer/scripts/compileExtensions.js b/Composer/scripts/compileExtensions.js
@@ -47,7 +47,7 @@ const getLastModified = (files = []) => {
 
   for (const f of files) {
     // returns last modified date of file in ISO 8601 format
-    const gitTimestamp = execSync(`git log -1 --pretty="%cI" ${f}`).toString().trim();
+    const gitTimestamp = execSync(`git log -1 --pretty="%cI" "${f}"`).toString().trim();
     const timestamp = new Date(gitTimestamp);
     if (timestamp > last) {
       last = timestamp;

diff --git a/extensions/.prettierrc b/extensions/.prettierrc
@@ -0,0 +1,7 @@
+{
+  "printWidth": 120,
+  "parser": "typescript",
+  "singleQuote": true,
+  "tabWidth": 2,
+  "endOfLine": "auto"
+}
diff --git a/extensions/authTest/index.js b/extensions/authTest/index.js
@@ -4,11 +4,11 @@
 const LocalStrategy = require('passport-local').Strategy;
 
 module.exports = {
-  initialize: composer => {
+  initialize: (composer) => {
     console.log('Register auth plugin');
 
     composer.usePassportStrategy(
-      new LocalStrategy(function(username, password, done) {
+      new LocalStrategy(function (username, password, done) {
         if (username === 'admin' && password === 'secret') {
           done(null, {
             name: 'admin',