Skip to content

Commit

Permalink
v2.1.1 fix: onCreate & onDestroy hook does not work.
Browse files Browse the repository at this point in the history
change the signature of _invokeHookFn, and add tests.
  • Loading branch information
Starrah committed Mar 26, 2022
1 parent 0d4c37b commit 568f10e
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 13 deletions.
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "express-router-dynamic",
"version": "2.1.0",
"version": "2.1.1",
"description": "Express Router which loads js codes dynamically as well as serves static files, and routes based on directory structure.",
"main": "bld/index.js",
"type": "commonjs",
Expand Down
18 changes: 11 additions & 7 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ export interface DynamicRouter extends Function, RequestHandler, Prefix$IsAny {

}

type HandlerObj = { handler: RequestHandler } & Hooks

export class DynamicRouter {
config: Config
dirConfigs: Dict<DirectoryConfig> = {}
handlers: Dict<{ handler: RequestHandler } & Hooks> = {}
handlers: Dict<HandlerObj> = {}
nofile_cache: Set<string> = new Set()
watcher: FSWatcher
extra_watchers: FSWatcher[] = []
Expand Down Expand Up @@ -329,8 +331,9 @@ export class DynamicRouter {
const config = this._getDirConfig(filename)
const relaPath = path.relative(this.config.webroot, filename)
if (this.handlers[filename]) {
const handler = this.handlers[filename]
delete this.handlers[filename]
await this._invokeHookFn(filename, "onDestroy", this) // onDestroy调用不管其中是否抛出异常,都要往下执行,因此无视返回值
await this._invokeHookFn(handler, filename, "onDestroy", this) // onDestroy调用不管其中是否抛出异常,都要往下执行,因此无视返回值
verb = "Removed"
}
if (!config.load_on_demand && shouldReload) {
Expand Down Expand Up @@ -421,24 +424,25 @@ export class DynamicRouter {
if (hook) existedHooks[hookName] = hook
}

const e = await this._invokeHookFn(filename, "onCreate", this)
const handlerObj = {handler, ...existedHooks}
const e = await this._invokeHookFn(handlerObj, filename, "onCreate", this)
if (e) {
this.logger.error(`Failed to load ${relaPath}: hook onCreate failed:`, e)
throw e
}

this.handlers[filename] = {handler, ...existedHooks}
this.handlers[filename] = handlerObj
const hooks = _.keys(existedHooks)
if (verbose) {
this.logger.info(`Loaded handler ${relaPath}${hooks.length ? `, loaded hooks: ${hooks.join(",")}` : ""}`)
}
return {hooks}
}

private async _invokeHookFn(filename: string, hookName: string, ...args): Promise<Error> {
if (typeof this.handlers[filename]?.[hookName] === "function") {
private async _invokeHookFn(handler: HandlerObj, filename: string, hookName: string, ...args): Promise<Error> {
if (typeof handler?.[hookName] === "function") {
try {
await this.handlers[filename][hookName].apply(this.handlers[filename].handler, args)
await handler[hookName].apply(handler.handler, args)
this.logger.info(`${hookName} Hook invoked for ${filename}`)
} catch (e) {
this.logger.warn(`Error encountered while invoking ${hookName} Hook for ${filename}:`, e)
Expand Down
45 changes: 42 additions & 3 deletions test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,46 @@ describe('File Watching', function () {
expect(data).property("test2").equal("/esm")
});
})
})
});

describe('Hook', function () {
this.timeout(5000)
let app, server, axios, tempDir, port, router

before(async () => {
({app, server, axios, tempDir, port} = await createApp())
await fscp("./test/testRoute/core", path.join(tempDir, "route"), {recursive: true})
router = new DynamicRouter({webroot: path.join(tempDir, "route")})
app.use(router)
})

after(async () => {
await router.onDestroy()
await destroyApp(server, tempDir)
})

it('should call onDestroy', async function () {
// first load the handler
const {data} = await axios.get("/hook-test")
expect(data).property("test").equal("/hook-test")

delete global["onDestroy_hook_called"]
expect(global["onDestroy_hook_called"]).be.undefined
// write the file to destroy the handler
await fsp.appendFile(path.join(tempDir, "route", "hook-test.route.js"), `
// something of no use`)
await delay(2000) // wait until onDestroy hook called
expect(global["onDestroy_hook_called"]).equal("/hook-test")
});

it('should call onCreate', async function () {
delete global["onCreate_hook_called"]
expect(global["onCreate_hook_called"]).be.undefined
const {data} = await axios.get("/hook-test")
expect(data).property("test").equal("/hook-test")
expect(global["onCreate_hook_called"]).equal("/hook-test")
});
});

describe('Security', function () {
let app, server, axios, tempDir, port, router
Expand Down Expand Up @@ -546,7 +585,7 @@ describe('Security', function () {
({status} = await axios.get("../dynamicB", {validateStatus: () => true}));
expect(status).equal(404)
});
})
});

describe('ESModule Support', function () {
this.timeout(5000)
Expand Down Expand Up @@ -614,4 +653,4 @@ describe('ESModule Support', function () {
expect(data).property("test").equal("/async-esm")
});
})
})
});
14 changes: 14 additions & 0 deletions test/testRoute/core/hook-test.route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
exports.default = function (req, res, next) {
res.json({
test: "/hook-test",
url: req.url
})
}

exports.onCreate = function () {
global["onCreate_hook_called"] = "/hook-test"
}

exports.onDestroy = function () {
global["onDestroy_hook_called"] = "/hook-test"
}

0 comments on commit 568f10e

Please sign in to comment.