From 6c8b9cf34b879dfd1973e27dac73d17d59eb6a2f Mon Sep 17 00:00:00 2001
From: liweitian <weitli@microsoft.com>
Date: Wed, 27 Nov 2019 03:22:29 +0800
Subject: [PATCH] feat: support default path environment variable (#1652)

* save tmp code

* save tmp code

* save current path in data.json

* initialize default path according to OS

* use default path if current path does not work

* fix bug about updating last accessed path

* create default folder

* handle comments

* handle comments

* fix lint error

* fix test case

* fix test case

* merge data.json and data.template.json and support defautlpath environment variable

* resolve default path

* use warped path utility

* remove console.log

* fix bug

* fix bug when user set default path as D: rather than D:/

* add debug logger

* add COMPOSER_BOTS_FOLDER to env settings

* resolve default path in settings

* log all settings in debug mode

* use default path from settings

* move setting default bots folder to settings

* run migrations on start up

* use TestBots directory for e2e tests

* create COMPOSER_BOTS_FOLDER if it doesn't exist

* use debug log when creating new bots

* fix bugs

* fix issue accessing defaultFolder from development settings

add interface for settings to catch this at compile time
---
 Composer/.gitignore                           |  2 +
 Composer/package.json                         |  2 +-
 .../LocationBrowser/LocationSelectContent.tsx | 10 ++--
 .../client/src/store/action/storage.ts        |  4 ++
 .../server/__tests__/services/project.test.ts |  1 +
 .../server/__tests__/services/storage.test.ts |  1 +
 .../server/src/controllers/project.ts         |  8 +--
 .../server/src/controllers/storage.ts         |  6 +++
 Composer/packages/server/src/logger.ts        |  6 +++
 Composer/packages/server/src/router/api.ts    |  1 +
 .../packages/server/src/services/storage.ts   | 30 ++++++++++-
 Composer/packages/server/src/settings/env.ts  |  7 +++
 .../packages/server/src/settings/index.ts     | 19 +++++--
 .../server/src/settings/settings.json         | 12 -----
 .../packages/server/src/settings/settings.ts  | 21 ++++++++
 .../server/src/store/data.template.ts         |  5 +-
 .../packages/server/src/store/migrations.ts   | 54 +++++++++++++++++++
 Composer/packages/server/src/store/store.ts   | 27 ++++++----
 .../packages/server/src/utility/storage.ts    |  6 +--
 azure-pipelines.yml                           |  1 +
 20 files changed, 177 insertions(+), 46 deletions(-)
 create mode 100644 Composer/packages/server/src/logger.ts
 delete mode 100644 Composer/packages/server/src/settings/settings.json
 create mode 100644 Composer/packages/server/src/settings/settings.ts
 create mode 100644 Composer/packages/server/src/store/migrations.ts

diff --git a/Composer/.gitignore b/Composer/.gitignore
index 51892b75d0..1603d74bba 100644
--- a/Composer/.gitignore
+++ b/Composer/.gitignore
@@ -4,3 +4,5 @@ junit.xml
 cypress/screenshots
 cypress/results
 cypress/videos
+
+TestBots/
diff --git a/Composer/package.json b/Composer/package.json
index d269fc9fbe..ac6795cb0c 100644
--- a/Composer/package.json
+++ b/Composer/package.json
@@ -31,7 +31,7 @@
     "test:coverage": "yarn test --coverage --no-cache --reporters=default",
     "test:integration": "cypress run --browser chrome",
     "test:integration:open": "cypress open",
-    "test:integration:clean": "rimraf ../MyBots/__Test* packages/server/data.json",
+    "test:integration:clean": "rimraf TestBots/*",
     "lint": "wsrun --exclude-missing --collect-logs --report lint",
     "lint:fix": "wsrun --exclude-missing --collect-logs --report lint:fix",
     "typecheck": "concurrently --kill-others-on-fail \"npm:typecheck:*\"",
diff --git a/Composer/packages/client/src/CreationFlow/LocationBrowser/LocationSelectContent.tsx b/Composer/packages/client/src/CreationFlow/LocationBrowser/LocationSelectContent.tsx
index a172cc6a5f..b8dad48093 100644
--- a/Composer/packages/client/src/CreationFlow/LocationBrowser/LocationSelectContent.tsx
+++ b/Composer/packages/client/src/CreationFlow/LocationBrowser/LocationSelectContent.tsx
@@ -7,14 +7,10 @@ import path from 'path';
 import { jsx } from '@emotion/core';
 import { Fragment, useEffect, useState, useContext, useRef } from 'react';
 
-import storage from '../../utils/storage';
-
 import { FileSelector } from './FileSelector';
 import { StoreContext } from './../../store';
 import { FileTypes } from './../../constants';
 
-const NEW_BOT_LOCATION_KEY = 'newBotLocation';
-
 export function LocationSelectContent(props) {
   const { state, actions } = useContext(StoreContext);
   const { storages, focusedStorageFolder, storageFileLoadingStatus } = state;
@@ -22,7 +18,7 @@ export function LocationSelectContent(props) {
 
   const { fetchFolderItemsByPath } = actions;
   const currentStorageIndex = useRef(0);
-  const [currentPath, setCurrentPath] = useState(storage.get(NEW_BOT_LOCATION_KEY, ''));
+  const [currentPath, setCurrentPath] = useState('');
   const currentStorageId = storages[currentStorageIndex.current] ? storages[currentStorageIndex.current].id : 'default';
 
   useEffect(() => {
@@ -39,6 +35,7 @@ export function LocationSelectContent(props) {
       // const formatedPath = path.normalize(newPath.replace(/\\/g, '/'));
       const formatedPath = path.normalize(newPath);
       await fetchFolderItemsByPath(storageId, formatedPath);
+      await actions.updateCurrentPath(formatedPath);
       setCurrentPath(formatedPath);
     }
   };
@@ -48,7 +45,7 @@ export function LocationSelectContent(props) {
     let path = currentPath;
     let id = '';
     if (storages[index]) {
-      path = path || storages[index].path;
+      path = storages[index].path;
       id = storages[index].id;
     }
     updateCurrentPath(path, id);
@@ -58,7 +55,6 @@ export function LocationSelectContent(props) {
     if (onChange) {
       onChange(currentPath);
     }
-    storage.set(NEW_BOT_LOCATION_KEY, currentPath);
   }, [currentPath]);
 
   const onSelectionChanged = item => {
diff --git a/Composer/packages/client/src/store/action/storage.ts b/Composer/packages/client/src/store/action/storage.ts
index 8cbb1f0cbc..13ef23ca4c 100644
--- a/Composer/packages/client/src/store/action/storage.ts
+++ b/Composer/packages/client/src/store/action/storage.ts
@@ -93,3 +93,7 @@ export const fetchFolderItemsByPath: ActionCreator = async ({ dispatch }, id, pa
     });
   }
 };
+
+export const updateCurrentPath: ActionCreator = async ({ dispatch }, path) => {
+  await httpClient.put(`/storages/currentPath`, { path: path });
+};
diff --git a/Composer/packages/server/__tests__/services/project.test.ts b/Composer/packages/server/__tests__/services/project.test.ts
index cbca239796..1e89ca3385 100644
--- a/Composer/packages/server/__tests__/services/project.test.ts
+++ b/Composer/packages/server/__tests__/services/project.test.ts
@@ -16,6 +16,7 @@ jest.mock('../../src/store/store', () => {
         name: 'This PC',
         type: 'LocalDisk',
         path: '.',
+        defaultPath: '.',
       },
     ],
     recentBotProjects: [] as any[],
diff --git a/Composer/packages/server/__tests__/services/storage.test.ts b/Composer/packages/server/__tests__/services/storage.test.ts
index 1e507c1157..b93b445482 100644
--- a/Composer/packages/server/__tests__/services/storage.test.ts
+++ b/Composer/packages/server/__tests__/services/storage.test.ts
@@ -24,6 +24,7 @@ jest.mock('../../src/store/store', () => {
       name: 'This PC',
       type: 'LocalDisk',
       path: '.',
+      defaultPath: '.',
     },
   ];
   return {
diff --git a/Composer/packages/server/src/controllers/project.ts b/Composer/packages/server/src/controllers/project.ts
index 4cc95230ca..7753fbdc84 100644
--- a/Composer/packages/server/src/controllers/project.ts
+++ b/Composer/packages/server/src/controllers/project.ts
@@ -9,7 +9,7 @@ import { BotProjectService } from '../services/project';
 import AssectService from '../services/asset';
 import { LocationRef } from '../models/bot/interface';
 import StorageService from '../services/storage';
-import settings from '../settings/settings.json';
+import settings from '../settings';
 
 import { Path } from './../utility/path';
 
@@ -21,7 +21,7 @@ async function createProject(req: Request, res: Response) {
   }
 
   // default the path to the default folder.
-  let path = settings.development.defaultFolder;
+  let path = settings.botsFolder;
   // however, if path is specified as part of post body, use that one.
   // this allows developer to specify a custom home for their bot.
   if (location) {
@@ -116,7 +116,7 @@ async function saveProjectAs(req: Request, res: Response) {
 
   const locationRef: LocationRef = {
     storageId,
-    path: Path.resolve(settings.development.defaultFolder, name),
+    path: Path.resolve(settings.botsFolder, name),
   };
 
   try {
@@ -352,7 +352,7 @@ async function publishLuis(req: Request, res: Response) {
 
 async function getAllProjects(req: Request, res: Response) {
   const storageId = 'default';
-  const folderPath = Path.resolve(settings.development.defaultFolder);
+  const folderPath = Path.resolve(settings.botsFolder);
   try {
     res.status(200).json(await StorageService.getBlob(storageId, folderPath));
   } catch (e) {
diff --git a/Composer/packages/server/src/controllers/storage.ts b/Composer/packages/server/src/controllers/storage.ts
index 33f1ee4515..a5249252ed 100644
--- a/Composer/packages/server/src/controllers/storage.ts
+++ b/Composer/packages/server/src/controllers/storage.ts
@@ -15,6 +15,11 @@ function createStorageConnection(req: Request, res: Response) {
   res.status(200).json(StorageService.getStorageConnections());
 }
 
+function updateCurrentPath(req: Request, res: Response) {
+  StorageService.updateCurrentPath(req.body.path);
+  res.status(200).json('success');
+}
+
 async function getBlob(req: Request, res: Response) {
   const storageId = req.params.storageId;
   const reqpath = decodeURI(req.params.path);
@@ -34,4 +39,5 @@ export const StorageController = {
   getStorageConnections,
   createStorageConnection,
   getBlob,
+  updateCurrentPath,
 };
diff --git a/Composer/packages/server/src/logger.ts b/Composer/packages/server/src/logger.ts
new file mode 100644
index 0000000000..124db0d707
--- /dev/null
+++ b/Composer/packages/server/src/logger.ts
@@ -0,0 +1,6 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+import debug from 'debug';
+
+export default debug('composer');
diff --git a/Composer/packages/server/src/router/api.ts b/Composer/packages/server/src/router/api.ts
index ac359258c3..4a829acc1a 100644
--- a/Composer/packages/server/src/router/api.ts
+++ b/Composer/packages/server/src/router/api.ts
@@ -32,6 +32,7 @@ router.post('/projects/opened/project/saveAs', ProjectController.saveProjectAs);
 router.get('/projects/recent', ProjectController.getRecentProjects);
 
 // storages
+router.put('/storages/currentPath', StorageController.updateCurrentPath);
 router.get('/storages', StorageController.getStorageConnections);
 router.post('/storages', StorageController.createStorageConnection);
 router.get('/storages/:storageId/blobs/:path(*)', StorageController.getBlob);
diff --git a/Composer/packages/server/src/services/storage.ts b/Composer/packages/server/src/services/storage.ts
index 29c84cf788..d38ee66fab 100644
--- a/Composer/packages/server/src/services/storage.ts
+++ b/Composer/packages/server/src/services/storage.ts
@@ -18,6 +18,7 @@ class StorageService {
 
   constructor() {
     this.storageConnections = Store.get(this.STORE_KEY);
+    this.ensureDefaultBotFoldersExist();
   }
 
   public getStorageClient = (storageId: string): IFileStorage => {
@@ -39,11 +40,18 @@ class StorageService {
   };
 
   public getStorageConnections = (): StorageConnection[] => {
-    return this.storageConnections.map(s => {
+    const connections = this.storageConnections.map(s => {
       const temp = Object.assign({}, s);
-      temp.path = Path.resolve(s.path); // resolve path if path is relative, and change it to unix pattern
+      // if the last accessed path exist
+      if (fs.existsSync(s.path)) {
+        temp.path = Path.resolve(s.path); // resolve path if path is relative, and change it to unix pattern
+      } else {
+        temp.path = Path.resolve(s.defaultPath);
+      }
       return temp;
     });
+    this.ensureDefaultBotFoldersExist();
+    return connections;
   };
 
   public checkBlob = async (storageId: string, filePath: string): Promise<boolean> => {
@@ -85,6 +93,17 @@ class StorageService {
     }
   };
 
+  public updateCurrentPath = (path: string) => {
+    this.storageConnections[0].path = path;
+    Store.set(this.STORE_KEY, this.storageConnections);
+  };
+
+  private ensureDefaultBotFoldersExist = () => {
+    this.storageConnections.forEach(s => {
+      this.createFolderRecurively(s.defaultPath);
+    });
+  };
+
   private isBotFolder = (path: string) => {
     // locate Main.dialog
     const mainPath = Path.join(path, 'ComposerDialogs/Main', 'Main.dialog');
@@ -119,6 +138,13 @@ class StorageService {
     const result = await Promise.all(children);
     return result.filter(item => !!item);
   };
+
+  private createFolderRecurively = (path: string) => {
+    if (!fs.existsSync(path)) {
+      this.createFolderRecurively(Path.dirname(path));
+      fs.mkdirSync(path);
+    }
+  };
 }
 
 const service = new StorageService();
diff --git a/Composer/packages/server/src/settings/env.ts b/Composer/packages/server/src/settings/env.ts
index 0fe13fe621..fb7b623929 100644
--- a/Composer/packages/server/src/settings/env.ts
+++ b/Composer/packages/server/src/settings/env.ts
@@ -5,3 +5,10 @@ export const absHosted = process.env.COMPOSER_AUTH_PROVIDER === 'abs-h';
 export const absHostRoot = process.env.WEBSITE_HOSTNAME
   ? `https://${process.env.WEBSITE_HOSTNAME}`
   : 'http://localhost:3978';
+
+let folder = process.env.COMPOSER_BOTS_FOLDER;
+if (folder && folder.endsWith(':')) {
+  folder = folder + '/';
+}
+
+export const botsFolder = folder;
diff --git a/Composer/packages/server/src/settings/index.ts b/Composer/packages/server/src/settings/index.ts
index 9d6f9151b4..2b5db176c4 100644
--- a/Composer/packages/server/src/settings/index.ts
+++ b/Composer/packages/server/src/settings/index.ts
@@ -3,17 +3,28 @@
 
 import merge from 'lodash/merge';
 
-import settings from './settings.json';
+import log from '../logger';
+
+import settings from './settings';
 
 // overall the guidance in settings.json is to list every item in "development"
 // section with a default value, and override the value for different environment
 // in later sections
 
+interface Settings {
+  botAdminEndpoint: string;
+  botEndpoint: string;
+  assetsLibray: string;
+  runtimeFolder: string;
+  botsFolder: string;
+}
+
 const defaultSettings = settings.development;
 const environment = process.env.NODE_ENV || 'development';
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-const environmentSettings = (settings as any)[environment];
+const environmentSettings = settings[environment];
+
+const finalSettings = merge<Settings, Settings>(defaultSettings, environmentSettings);
 
-const finalSettings = merge(defaultSettings, environmentSettings);
+log('App Settings: %O', finalSettings);
 
 export default finalSettings;
diff --git a/Composer/packages/server/src/settings/settings.json b/Composer/packages/server/src/settings/settings.json
deleted file mode 100644
index 3c40395d5e..0000000000
--- a/Composer/packages/server/src/settings/settings.json
+++ /dev/null
@@ -1,12 +0,0 @@
-{
-  "development": {
-    "botAdminEndpoint": "http://localhost:3979",
-    "botEndpoint": "http://localhost:3979",
-    "assetsLibray": "./assets",
-    "runtimeFolder": "../../../BotProject/Templates",
-    "defaultFolder": "../../../MyBots"
-  },
-  "container": {
-    "botAdminEndpoint": "http://botruntime:80"
-  }
-}
\ No newline at end of file
diff --git a/Composer/packages/server/src/settings/settings.ts b/Composer/packages/server/src/settings/settings.ts
new file mode 100644
index 0000000000..618f8b110b
--- /dev/null
+++ b/Composer/packages/server/src/settings/settings.ts
@@ -0,0 +1,21 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+import os from 'os';
+
+import { Path } from '../utility/path';
+
+import { botsFolder } from './env';
+
+export default {
+  development: {
+    botAdminEndpoint: 'http://localhost:3979',
+    botEndpoint: 'http://localhost:3979',
+    assetsLibray: Path.resolve('./assets'),
+    runtimeFolder: Path.resolve('../../../BotProject/Templates'),
+    botsFolder: botsFolder || Path.join(os.homedir(), 'Documents', 'Composer'),
+  },
+  container: {
+    botAdminEndpoint: 'http://botruntime:80',
+  },
+};
diff --git a/Composer/packages/server/src/store/data.template.ts b/Composer/packages/server/src/store/data.template.ts
index 9bd5830cdc..4b5a090d31 100644
--- a/Composer/packages/server/src/store/data.template.ts
+++ b/Composer/packages/server/src/store/data.template.ts
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import path from 'path';
+import settings from '../settings';
 
 export default {
   storageConnections: [
@@ -9,7 +9,8 @@ export default {
       id: 'default',
       name: 'This PC',
       type: 'LocalDisk',
-      path: path.resolve(__dirname, '../../../../../MyBots'),
+      path: '', // this is used as last accessed path, if it is invalid, use defaultPath
+      defaultPath: settings.botsFolder,
     },
   ],
   recentBotProjects: [],
diff --git a/Composer/packages/server/src/store/migrations.ts b/Composer/packages/server/src/store/migrations.ts
new file mode 100644
index 0000000000..286148eeb6
--- /dev/null
+++ b/Composer/packages/server/src/store/migrations.ts
@@ -0,0 +1,54 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+/* eslint-disable @typescript-eslint/no-explicit-any */
+
+import get from 'lodash/get';
+import set from 'lodash/set';
+
+import log from '../logger';
+import settings from '../settings';
+
+interface Migration {
+  /**
+   * Migration label. Will be printed to the console in debug.
+   * @example 'Update Designer to Composer'
+   */
+  name: string;
+  /**
+   * Use to check if a condition exists that requires migration.
+   * @example data => data.name === 'Designer';
+   */
+  condition: (data: any) => boolean;
+  /**
+   * Data transform to run if condition is met.
+   * @example data => ({ ...data, name: 'Composer' });
+   */
+  run: (data: any) => any;
+}
+
+const migrations: Migration[] = [
+  {
+    name: 'Add defaultPath',
+    condition: data => get(data, 'storageConnections.0.defaultPath') !== settings.botsFolder,
+    run: data => set(data, 'storageConnections[0].defaultPath', settings.botsFolder),
+  },
+];
+
+export function runMigrations(initialData: any): any {
+  const migrationsToRun: Migration[] = migrations.filter(m => m.condition(initialData));
+  if (migrationsToRun.length > 0) {
+    log('migration: running migrations...');
+
+    const data = migrationsToRun.reduce((data, m, i) => {
+      log('migration: %s (%d / %d)', m.name, i + 1, migrationsToRun.length);
+      return m.run(data);
+    }, initialData);
+
+    log('migration: done!');
+
+    return data;
+  }
+
+  return initialData;
+}
diff --git a/Composer/packages/server/src/store/store.ts b/Composer/packages/server/src/store/store.ts
index d1eb6c55eb..b8af7d62ec 100644
--- a/Composer/packages/server/src/store/store.ts
+++ b/Composer/packages/server/src/store/store.ts
@@ -4,25 +4,32 @@
 import fs from 'fs';
 import path from 'path';
 
+import log from '../logger';
+
 import localInitData from './data.template';
 import abhInitData from './abh-template.json';
-
+import { runMigrations } from './migrations';
 const isHostedInAzure = !!process.env.WEBSITE_NODE_DEFAULT_VERSION;
 const dataStorePath =
   isHostedInAzure && process.env.HOME
     ? path.resolve(process.env.HOME, './site/data.json')
     : path.resolve(__dirname, '../../data.json');
 
-const initData = isHostedInAzure ? abhInitData : localInitData;
+let initData = isHostedInAzure ? abhInitData : localInitData;
 
-// create data.json if not exits
-if (!fs.existsSync(dataStorePath)) {
-  fs.writeFileSync(dataStorePath, JSON.stringify(initData, null, 2) + '\n');
-  if (isHostedInAzure) {
-    // for some very odd reason on Azure webapp, fs.readFileSync after writeFileSync doesn't
-    // always find the file, add one more io to kick the  virtual file system
-    fs.appendFileSync(dataStorePath, ' ');
-  }
+if (fs.existsSync(dataStorePath)) {
+  const userData = JSON.parse(fs.readFileSync(dataStorePath, 'utf-8'));
+  initData = runMigrations(userData);
+} else {
+  log('Database not found. Seeding data.json with: %O', initData);
+}
+
+fs.writeFileSync(dataStorePath, JSON.stringify(initData, null, 2) + '\n');
+
+if (isHostedInAzure) {
+  // for some very odd reason on Azure webapp, fs.readFileSync after writeFileSync doesn't
+  // always find the file, add one more io to kick the  virtual file system
+  fs.appendFileSync(dataStorePath, ' ');
 }
 
 interface KVStore {
diff --git a/Composer/packages/server/src/utility/storage.ts b/Composer/packages/server/src/utility/storage.ts
index 55eac7a864..853a6882cc 100644
--- a/Composer/packages/server/src/utility/storage.ts
+++ b/Composer/packages/server/src/utility/storage.ts
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+import log from '../logger';
 import { IFileStorage } from '../models/storage/interface';
 
 /**
@@ -24,10 +25,7 @@ export async function copyDir(srcDir: string, srcStorage: IFileStorage, dstDir:
     const srcPath = `${srcDir}/${path}`;
     const dstPath = `${dstDir}/${path}`;
 
-    if (process.env.NODE_ENV !== 'test') {
-      // eslint-disable-next-line no-console
-      console.log(`copying ${srcPath} to ${dstPath}`);
-    }
+    log('copying %s to %s', srcPath, dstPath);
 
     if ((await srcStorage.stat(srcPath)).isFile) {
       // copy files
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index 5cbf7b7cae..0d9e256363 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -53,6 +53,7 @@ jobs:
           CYPRESS_SCREENSHOTS_FOLDER: $(Build.ArtifactStagingDirectory)/cypress/screenshots
           CYPRESS_VIDEOS_FOLDER: $(Build.ArtifactStagingDirectory)/cypress/videos
           TERM: xterm
+          COMPOSER_BOTS_FOLDER: $(Pipeline.Workspace)/Composer/TestBots
       - task: PublishPipelineArtifact@1
         condition: in(variables['Agent.JobStatus'], 'SucceededWithIssues', 'Failed')
         continueOnError: true