Skip to content

Commit

Permalink
Merge pull request #2565 from botpress/ya-require-cache
Browse files Browse the repository at this point in the history
fix(core): caching required files in actions and hooks
  • Loading branch information
allardy committed Nov 7, 2019
2 parents c602ad5 + 59496a5 commit 069d013
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 12 deletions.
21 changes: 17 additions & 4 deletions src/bp/core/modules/require.ts
Expand Up @@ -2,6 +2,9 @@ import fs from 'fs'
import _ from 'lodash'
import path from 'path'

let requireCache = {}
const getRequireCacheKey = (scriptPath, module) => `req-${scriptPath}_${module}`

export const explodePath = (location: string): string[] => {
const parts: string[] = location.split(path.sep)
const paths: string[] = []
Expand All @@ -23,7 +26,13 @@ export const explodePath = (location: string): string[] => {
return paths.reverse()
}

export const requireAtPaths = (module: string, locations: string[]) => {
export const requireAtPaths = (module: string, locations: string[], scriptPath?: string) => {
const requireKey = getRequireCacheKey(scriptPath, module)

if (requireCache[requireKey] && scriptPath) {
return requireCache[requireKey]
}

const folders = _.flatten(locations.map(explodePath))
const lookups = _.flatten(
folders.map(folder => {
Expand All @@ -45,7 +54,7 @@ export const requireAtPaths = (module: string, locations: string[]) => {
if (!fs.existsSync(loc)) {
continue
}
return require(loc)
return (requireCache[requireKey] = require(loc))
} else {
// package.json
const pkgPath = path.join(loc, 'package.json')
Expand All @@ -57,14 +66,18 @@ export const requireAtPaths = (module: string, locations: string[]) => {
continue
}
const pkgEntry = path.join(loc, pkg.main)
return require(pkgEntry)
return (requireCache[requireKey] = require(pkgEntry))
}
} catch (err) {}
}

try {
return require(module)
return (requireCache[requireKey] = require(module))
} catch (err) {
throw new Error(`Module "${module}" not found. Tried these locations: "${locations.join(', ')}"`)
}
}

export const clearRequireCache = () => {
requireCache = {}
}
12 changes: 8 additions & 4 deletions src/bp/core/services/action/action-service.ts
Expand Up @@ -10,7 +10,7 @@ import { NodeVM } from 'vm2'

import { GhostService } from '..'
import { createForAction } from '../../api'
import { requireAtPaths } from '../../modules/require'
import { clearRequireCache, requireAtPaths } from '../../modules/require'
import { TYPES } from '../../types'
import { ActionExecutionError, BPError } from '../dialog/errors'

Expand Down Expand Up @@ -49,6 +49,8 @@ export default class ActionService {
Object.keys(require.cache)
.filter(r => r.match(/(\\|\/)actions(\\|\/)/g))
.map(file => delete require.cache[file])

clearRequireCache()
}

forBot(botId: string): ScopedActionService {
Expand Down Expand Up @@ -150,7 +152,9 @@ export class ScopedActionService {
return !!actions.find(x => x.name === actionName)
}

private _prepareRequire(actionLocation: string) {
private _prepareRequire(fullPath: string) {
const actionLocation = path.dirname(fullPath)

let parts = path.relative(process.PROJECT_LOCATION, actionLocation).split(path.sep)
parts = parts.slice(parts.indexOf('actions') + 1) // We only keep the parts after /actions/...

Expand All @@ -161,7 +165,7 @@ export class ScopedActionService {
lookups.unshift(process.LOADED_MODULES[parts[0]])
}

return module => requireAtPaths(module, lookups)
return module => requireAtPaths(module, lookups, fullPath)
}

async runAction(actionName: string, incomingEvent: any, actionArgs: any): Promise<any> {
Expand All @@ -176,7 +180,7 @@ export class ScopedActionService {
const botFolder = action.location === 'global' ? 'global' : 'bots/' + this.botId
const dirPath = path.resolve(path.join(process.PROJECT_LOCATION, `/data/${botFolder}/actions/${actionName}.js`))

const _require = this._prepareRequire(path.dirname(dirPath))
const _require = this._prepareRequire(dirPath)

const modRequire = new Proxy(
{},
Expand Down
12 changes: 8 additions & 4 deletions src/bp/core/services/hook/hook-service.ts
Expand Up @@ -11,7 +11,7 @@ import path from 'path'
import { NodeVM } from 'vm2'

import { GhostService } from '..'
import { requireAtPaths } from '../../modules/require'
import { clearRequireCache, requireAtPaths } from '../../modules/require'
import { TYPES } from '../../types'
import { VmRunner } from '../action/vm'
import { Incident } from '../alerting-service'
Expand Down Expand Up @@ -163,6 +163,8 @@ export class HookService {
Object.keys(require.cache)
.filter(r => r.match(/(\\|\/)hooks(\\|\/)/g))
.map(file => delete require.cache[file])

clearRequireCache()
}

async executeHook(hook: Hooks.BaseHook): Promise<void> {
Expand Down Expand Up @@ -215,7 +217,9 @@ export class HookService {
}
}

private _prepareRequire(hookLocation: string, hookType: string) {
private _prepareRequire(fullPath: string, hookType: string) {
const hookLocation = path.dirname(fullPath)

let parts = path.relative(process.PROJECT_LOCATION, hookLocation).split(path.sep)
parts = parts.slice(parts.indexOf(hookType) + 1) // We only keep the parts after /hooks/{type}/...

Expand All @@ -226,14 +230,14 @@ export class HookService {
lookups.unshift(process.LOADED_MODULES[parts[0]])
}

return module => requireAtPaths(module, lookups)
return module => requireAtPaths(module, lookups, fullPath)
}

private async runScript(hookScript: HookScript, hook: Hooks.BaseHook) {
const hookPath = `/data/global/hooks/${hook.folder}/${hookScript.path}.js`
const dirPath = path.resolve(path.join(process.PROJECT_LOCATION, hookPath))

const _require = this._prepareRequire(path.dirname(dirPath), hook.folder)
const _require = this._prepareRequire(dirPath, hook.folder)

const modRequire = new Proxy(
{},
Expand Down

0 comments on commit 069d013

Please sign in to comment.