Merge pull request #2134 from botpress/ya-bot-migrations

fix(bots): added migration for bot imports

modules/nlu/src/migrations/v12_0_1-1562193005-migrate_numeral_entities_to_number.ts

@@ -3,12 +3,11 @@ import * as sdk from 'botpress/sdk'
 const migration: sdk.ModuleMigration = {
   info: {
     description: 'Migrates slots from type numeral to number',
+    target: 'bot',
     type: 'config'
   },
-  up: async (bp: typeof sdk): Promise<sdk.MigrationResult> => {
-    const bots = await bp.bots.getAllBots()
-
-    for (const botId of Array.from(bots.keys())) {
+  up: async ({ bp, metadata }: sdk.ModuleMigrationOpts): Promise<sdk.MigrationResult> => {
+    const updateBot = async botId => {
       const bpfs = bp.ghost.forBot(botId)
       const intents = await bpfs.directoryListing('./intents', '*.json')
       for (const file of intents) {
@@ -23,6 +22,15 @@ const migration: sdk.ModuleMigration = {
       }
     }
 
+    if (metadata.botId) {
+      await updateBot(metadata.botId)
+    } else {
+      const bots = await bp.bots.getAllBots()
+      for (const botId of Array.from(bots.keys())) {
+        await updateBot(botId)
+      }
+    }
+
     return { success: true, message: 'Slots migrated successfully' }
   }
 }

src/bp/core/services/bot-service.ts

@@ -27,6 +27,7 @@ import { CMSService } from './cms'
 import { FileContent, GhostService } from './ghost/service'
 import { Hooks, HookService } from './hook/hook-service'
 import { JobService } from './job-service'
+import { MigrationService } from './migration'
 import { ModuleResourceLoader } from './module/resources-loader'
 import RealtimeService from './realtime'
 import { WorkspaceService } from './workspace-service'
@@ -64,7 +65,8 @@ export class BotService {
     @inject(TYPES.JobService) private jobService: JobService,
     @inject(TYPES.Statistics) private stats: Statistics,
     @inject(TYPES.WorkspaceService) private workspaceService: WorkspaceService,
-    @inject(TYPES.RealtimeService) private realtimeService: RealtimeService
+    @inject(TYPES.RealtimeService) private realtimeService: RealtimeService,
+    @inject(TYPES.MigrationService) private migrationService: MigrationService
   ) {
     this._botIds = undefined
   }
@@ -244,7 +246,10 @@ export class BotService {
         }
         await this.configProvider.mergeBotConfig(botId, newConfigs)
         await this.workspaceService.addBotRef(botId, workspaceId)
+
+        await this._migrateBotContent(botId)
         await this.mountBot(botId)
+
         this.logger.forBot(botId).info(`Import of bot ${botId} successful`)
       } else {
         this.logger.forBot(botId).info(`Import of bot ${botId} was denied by hook validation`)
@@ -254,6 +259,11 @@ export class BotService {
     }
   }
 
+  private async _migrateBotContent(botId: string): Promise<void> {
+    const config = await this.configProvider.getBotConfig(botId)
+    return this.migrationService.executeMissingBotMigrations(botId, config.version)
+  }
+
   async requestStageChange(botId: string, requested_by: string) {
     const botConfig = (await this.findBotById(botId)) as BotConfig
     if (!botConfig) {
@@ -420,7 +430,8 @@ export class BotService {
         const mergedConfigs = {
           ...DEFAULT_BOT_CONFIGS,
           ...templateConfig,
-          ...botConfig
+          ...botConfig,
+          version: process.BOTPRESS_VERSION
         }
 
         if (!mergedConfigs.imports.contentTypes) {

src/bp/core/services/migration/index.ts

@@ -14,6 +14,7 @@ import path from 'path'
 import semver from 'semver'
 
 import { container } from '../../app.inversify'
+import { GhostService } from '../ghost/service'
 
 const debug = DEBUG('migration')
 
@@ -25,8 +26,6 @@ const types = {
 
 @injectable()
 export class MigrationService {
-  /** This is the declared version in the configuration file */
-  private configVersion!: string
   /** This is the actual running version (package.json) */
   private currentVersion: string
   private loadedMigrations: { [filename: string]: Migration | sdk.ModuleMigration } = {}
@@ -37,29 +36,30 @@ export class MigrationService {
     @inject(TYPES.Logger)
     private logger: sdk.Logger,
     @inject(TYPES.Database) private database: Database,
-    @inject(TYPES.ConfigProvider) private configProvider: ConfigProvider
+    @inject(TYPES.ConfigProvider) private configProvider: ConfigProvider,
+    @inject(TYPES.GhostService) private ghostService: GhostService
   ) {
     this.currentVersion = process.env.MIGRATION_TEST_VERSION || process.BOTPRESS_VERSION
     this.completedMigrationsDir = path.resolve(process.PROJECT_LOCATION, `data/migrations`)
     mkdirp.sync(this.completedMigrationsDir)
   }
 
   async initialize() {
-    this.configVersion = (await this.configProvider.getBotpressConfig()).version
-    debug(`Migration Check: %o`, { configVersion: this.configVersion, currentVersion: this.currentVersion })
+    const configVersion = (await this.configProvider.getBotpressConfig()).version
+    debug(`Migration Check: %o`, { configVersion, currentVersion: this.currentVersion })
 
     if (process.env.SKIP_MIGRATIONS) {
       debug(`Skipping Migrations`)
       return
     }
 
-    const missingMigrations = [...this._getMissingCoreMigrations(), ...this._getMissingModuleMigrations()]
+    const missingMigrations = this.filterMissing(this.getAllMigrations(), configVersion)
     if (!missingMigrations.length) {
       return
     }
 
     this._loadMigrations(missingMigrations)
-    this._displayStatus()
+    this.displayMigrationStatus(configVersion, missingMigrations, this.logger)
 
     if (!process.AUTO_MIGRATE) {
       await this.logger.error(
@@ -68,51 +68,59 @@ export class MigrationService {
       process.exit(1)
     }
 
-    await this.execute()
+    await this.executeMigrations(missingMigrations)
   }
 
-  private _displayStatus() {
-    const migrations = Object.keys(this.loadedMigrations).map(filename => this.loadedMigrations[filename].info)
+  async executeMissingBotMigrations(botId: string, botVersion: string) {
+    debug.forBot(botId, `Checking missing migrations for bot `, { botId, botVersion })
 
-    this.logger.warn(chalk`========================================
-{bold ${center(`Migration Required`, 40)}}
-{dim ${center(`Version ${this.configVersion} => ${this.currentVersion} `, 40)}}
-{dim ${center(`${migrations.length} changes`, 40)}}
-========================================`)
+    const missingMigrations = this.filterBotTarget(this.filterMissing(this.getAllMigrations(), botVersion))
+    if (!missingMigrations.length) {
+      return
+    }
 
-    Object.keys(types).map(type => {
-      this.logger.warn(chalk`{bold ${types[type]}}`)
-      const filtered = migrations.filter(x => x.type === type)
+    this.displayMigrationStatus(botVersion, missingMigrations, this.logger.forBot(botId))
+    const opts = await this.getMigrationOpts({ botId })
+    let hasFailures
 
-      if (filtered.length) {
-        filtered.map(x => this.logger.warn(`- ${x.description}`))
+    await Promise.mapSeries(missingMigrations, async ({ filename }) => {
+      const result = await this.loadedMigrations[filename].up(opts)
+      debug.forBot(botId, `Migration step finished`, { filename, result })
+      if (result.success) {
+        await this.logger.info(`- ${result.message || 'Success'}`)
       } else {
-        this.logger.warn(`- None`)
+        hasFailures = true
+        await this.logger.error(`- ${result.message || 'Failure'}`)
       }
     })
+
+    if (hasFailures) {
+      return this.logger.error(`Could not complete bot migration. It may behave unexpectedly.`)
+    }
+
+    await this.configProvider.mergeBotConfig(botId, { version: this.currentVersion })
   }
 
-  async execute() {
-    const migrationCount = Object.keys(this.loadedMigrations).length
-    const api = await container.get<BotpressAPIProvider>(TYPES.BotpressAPIProvider).create('Migration', 'core')
+  private async executeMigrations(missingMigrations: MigrationFile[]) {
+    const opts = await this.getMigrationOpts()
 
     this.logger.info(chalk`========================================
-{bold ${center(`Executing ${migrationCount.toString()} migrations`, 40)}}
+{bold ${center(`Executing ${missingMigrations.length.toString()} migrations`, 40)}}
 ========================================`)
 
     const completed = this._getCompletedMigrations()
     let hasFailures = false
 
-    await Promise.mapSeries(Object.keys(this.loadedMigrations), async file => {
-      if (completed.includes(file)) {
-        return this.logger.info(`Skipping already migrated file "${file}"`)
+    await Promise.mapSeries(missingMigrations, async ({ filename }) => {
+      if (completed.includes(filename)) {
+        return this.logger.info(`Skipping already migrated file "${filename}"`)
       }
 
-      this.logger.info(`Running ${file}`)
+      this.logger.info(`Running ${filename}`)
 
-      const result = await this.loadedMigrations[file].up(api, this.configProvider, this.database, container)
+      const result = await this.loadedMigrations[filename].up(opts)
       if (result.success) {
-        this._saveCompletedMigration(file, result)
+        this._saveCompletedMigration(filename, result)
         await this.logger.info(`- ${result.message || 'Success'}`)
       } else {
         hasFailures = true
@@ -127,32 +135,78 @@ export class MigrationService {
       process.exit(1)
     }
 
-    await this.configProvider.mergeBotpressConfig({ version: this.currentVersion })
+    await this.updateAllVersions()
     this.logger.info(`Migrations completed successfully! `)
   }
 
-  private _getCompletedMigrations(): string[] {
-    return fs.readdirSync(this.completedMigrationsDir)
+  private async updateAllVersions() {
+    await this.configProvider.mergeBotpressConfig({ version: this.currentVersion })
+
+    const botIds = (await this.ghostService.bots().directoryListing('/', 'bot.config.json')).map(path.dirname)
+    for (const botId of botIds) {
+      await this.configProvider.mergeBotConfig(botId, { version: this.currentVersion })
+    }
   }
 
-  private _saveCompletedMigration(filename: string, result: sdk.MigrationResult) {
-    fs.writeFileSync(path.resolve(`${this.completedMigrationsDir}/${filename}`), JSON.stringify(result, undefined, 2))
+  private displayMigrationStatus(configVersion: string, missingMigrations: MigrationFile[], logger: sdk.Logger) {
+    const migrations = missingMigrations.map(x => this.loadedMigrations[x.filename].info)
+
+    logger.warn(chalk`========================================
+{bold ${center(`Migration Required`, 40)}}
+{dim ${center(`Version ${configVersion} => ${this.currentVersion} `, 40)}}
+{dim ${center(`${migrations.length} changes`, 40)}}
+========================================`)
+
+    Object.keys(types).map(type => {
+      logger.warn(chalk`{bold ${types[type]}}`)
+      const filtered = migrations.filter(x => x.type === type)
+
+      if (filtered.length) {
+        filtered.map(x => logger.warn(`- ${x.description}`))
+      } else {
+        logger.warn(`- None`)
+      }
+    })
   }
 
-  private _loadMigrations = (fileList: MigrationFile[]) =>
-    fileList.map(file => (this.loadedMigrations[file.filename] = require(file.location).default))
+  private async getMigrationOpts(metadata?: sdk.MigrationMetadata) {
+    return {
+      bp: await container.get<BotpressAPIProvider>(TYPES.BotpressAPIProvider).create('Migration', 'core'),
+      configProvider: this.configProvider,
+      database: this.database,
+      inversify: container,
+      metadata: metadata || {}
+    }
+  }
 
-  private _getMissingCoreMigrations() {
-    return this._getFilteredMigrations(path.join(__dirname, '../../../migrations'))
+  private filterBotTarget(migrationFiles: MigrationFile[]): MigrationFile[] {
+    this._loadMigrations(migrationFiles)
+    return migrationFiles.filter(x => this.loadedMigrations[x.filename].info.target === 'bot')
   }
 
-  private _getMissingModuleMigrations() {
-    return _.flatMap(Object.keys(process.LOADED_MODULES), module =>
-      this._getFilteredMigrations(path.join(process.LOADED_MODULES[module], 'dist/migrations'))
+  private getAllMigrations(): MigrationFile[] {
+    const coreMigrations = this._getMigrations(path.join(__dirname, '../../../migrations'))
+    const moduleMigrations = _.flatMap(Object.keys(process.LOADED_MODULES), module =>
+      this._getMigrations(path.join(process.LOADED_MODULES[module], 'dist/migrations'))
     )
+
+    return [...coreMigrations, ...moduleMigrations]
   }
 
-  private _getFilteredMigrations = (rootPath: string) => this._filterMissing(this._getMigrations(rootPath))
+  private _getCompletedMigrations(): string[] {
+    return fs.readdirSync(this.completedMigrationsDir)
+  }
+
+  private _saveCompletedMigration(filename: string, result: sdk.MigrationResult) {
+    fs.writeFileSync(path.resolve(`${this.completedMigrationsDir}/${filename}`), JSON.stringify(result, undefined, 2))
+  }
+
+  private _loadMigrations = (fileList: MigrationFile[]) =>
+    fileList.map(file => {
+      if (!this.loadedMigrations[file.filename]) {
+        this.loadedMigrations[file.filename] = require(file.location).default
+      }
+    })
 
   private _getMigrations(rootPath: string): MigrationFile[] {
     return _.orderBy(
@@ -170,28 +224,32 @@ export class MigrationService {
     )
   }
 
-  private _filterMissing = (files: MigrationFile[]) =>
-    files.filter(file => semver.satisfies(file.version, `>${this.configVersion} <= ${this.currentVersion}`))
+  private filterMissing = (files: MigrationFile[], version) =>
+    files.filter(file => semver.satisfies(file.version, `>${version} <= ${this.currentVersion}`))
 }
 
-interface MigrationFile {
+export interface MigrationFile {
   date: number
   version: string
   location: string
   filename: string
   title: string
 }
 
+export interface MigrationOpts {
+  bp: typeof sdk
+  configProvider: ConfigProvider
+  database: Database
+  inversify: Container
+  metadata: sdk.MigrationMetadata
+}
+
 export interface Migration {
   info: {
     description: string
+    target?: 'core' | 'bot'
     type: 'database' | 'config' | 'content'
   }
-  up: (bp: typeof sdk, config: ConfigProvider, database: Database, inversify: Container) => Promise<sdk.MigrationResult>
-  down?: (
-    bp: typeof sdk,
-    config: ConfigProvider,
-    database: Database,
-    inversify: Container
-  ) => Promise<sdk.MigrationResult>
+  up: (opts: MigrationOpts | sdk.ModuleMigrationOpts) => Promise<sdk.MigrationResult>
+  down?: (opts: MigrationOpts | sdk.ModuleMigrationOpts) => Promise<sdk.MigrationResult>
 }

src/bp/core/services/migration/template_core.ts

@@ -1,20 +1,19 @@
 import * as sdk from 'botpress/sdk'
-import { ConfigProvider } from 'core/config/config-loader'
-import Database from 'core/database'
 import { Migration } from 'core/services/migration'
-import { Container } from 'inversify'
 
 const migration: Migration = {
   info: {
     description: '',
+    target: 'core',
     type: 'config'
   },
-  up: async (
-    bp: typeof sdk,
-    configProvider: ConfigProvider,
-    database: Database,
-    inversify: Container
-  ): Promise<sdk.MigrationResult> => {
+  up: async ({
+    bp,
+    configProvider,
+    database,
+    inversify,
+    metadata
+  }: sdk.ModuleMigrationOpts): Promise<sdk.MigrationResult> => {
     return { success: true, message: 'Configuration updated successfully' }
   }
 }

src/bp/core/services/migration/template_module.ts

@@ -3,9 +3,10 @@ import * as sdk from 'botpress/sdk'
 const migration: sdk.ModuleMigration = {
   info: {
     description: '',
+    target: 'core',
     type: 'config'
   },
-  up: async (bp: typeof sdk): Promise<sdk.MigrationResult> => {
+  up: async ({ bp, metadata }: sdk.ModuleMigrationOpts): Promise<sdk.MigrationResult> => {
     return { success: true, message: 'Configuration updated successfully' }
   }
 }