Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): Finalize app.teardown() functionality #2584

Merged
merged 1 commit into from
Mar 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/adapter-commons/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"scripts": {
"prepublish": "npm run compile",
"compile": "shx rm -rf lib/ && tsc",
"test": "mocha --config ../../.mocharc.json --recursive test/**.test.ts test/**/*.test.ts"
"mocha": "mocha --config ../../.mocharc.json --recursive test/**.test.ts test/**/*.test.ts",
"test": "npm run compile && npm run mocha"
},
"directories": {
"lib": "lib"
Expand Down
2 changes: 1 addition & 1 deletion packages/express/src/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export interface ExpressOverrides<Services> {
listen(port: number, hostname: string, callback?: () => void): Promise<http.Server>;
listen(port: number|string|any, callback?: () => void): Promise<http.Server>;
listen(callback?: () => void): Promise<http.Server>;
close (): Promise<void>;
use: ExpressUseHandler<this, Services>;
server: http.Server;
}

export type Application<Services = any, Settings = any> =
Expand Down
22 changes: 7 additions & 15 deletions packages/express/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import express, { Express } from 'express';
import { Application as FeathersApplication, defaultServiceMethods } from '@feathersjs/feathers';
import { routing } from '@feathersjs/transport-commons';
import { createDebug } from '@feathersjs/commons';
import http from 'http';

import { Application } from './declarations';

Expand All @@ -26,8 +25,7 @@ export default function feathersExpress<S = any, C = any> (feathersApp?: Feather

const app = expressApp as any as Application<S, C>;
const { use: expressUse, listen: expressListen } = expressApp as any;
const feathersUse = feathersApp.use;
let server:http.Server | undefined;
const { use: feathersUse, teardown: feathersTeardown } = feathersApp;

Object.assign(app, {
use (location: string & keyof S, ...rest: any[]) {
Expand Down Expand Up @@ -71,25 +69,19 @@ export default function feathersExpress<S = any, C = any> (feathersApp?: Feather
},

async listen (...args: any[]) {
server = expressListen.call(this, ...args);
const server = expressListen.call(this, ...args);

this.server = server;
await this.setup(server);
debug('Feathers application listening');

return server;
},

async close () {
if ( server ) {
server.close();

await new Promise((resolve) => {
server.on('close', () => { resolve(true) });
})
}

debug('Feathers application closing');
await this.teardown();
async teardown (server?: any) {
return feathersTeardown.call(this, server).then(() =>
new Promise((resolve, reject) => this.server.close(e => e ? reject(e) : resolve(this)))
);
}
} as Application<S, C>);

Expand Down
8 changes: 4 additions & 4 deletions packages/express/test/authentication.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ describe('@feathersjs/express/authentication', () => {
const password = 'superexpress';

let app: express.Application;
let server: any;
let user: any;
let authResult: AuthenticationResult;

Expand All @@ -26,15 +25,16 @@ describe('@feathersjs/express/authentication', () => {
.configure(express.rest());

app = createApplication(expressApp as any) as unknown as express.Application;
server = await app.listen(9876);

await app.listen(9876);

app.use('/dummy', {
get (id, params) {
return Promise.resolve({ id, params });
}
});

//@ts-ignore
// @ts-ignore
app.use('/protected', express.authenticate('jwt'), (req, res) => {
res.json(req.feathers.user);
});
Expand All @@ -60,7 +60,7 @@ describe('@feathersjs/express/authentication', () => {
authResult = res.data;
});

after(done => server.close(done));
after(() => app.teardown());

describe('service authentication', () => {
it('successful local authentication', () => {
Expand Down
27 changes: 2 additions & 25 deletions packages/express/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,30 +174,7 @@ describe('@feathersjs/express', () => {
await new Promise(resolve => server.close(() => resolve(server)));
});

it('.close calls .teardown', async () => {
const app = feathersExpress(feathers());
let called = false;

app.use('/myservice', {
async get (id: Id) {
return { id };
},

async teardown (appParam, path) {
assert.strictEqual(appParam, app);
assert.strictEqual(path, 'myservice');
called = true;
}

});

await app.listen(8787);
await app.close();

assert.ok(called);
});

it('.close closes http server', async () => {
it('.teardown closes http server', async () => {
const app = feathersExpress(feathers());
let called = false;

Expand All @@ -206,7 +183,7 @@ describe('@feathersjs/express', () => {
called = true;
})

await app.close();
await app.teardown();
assert.ok(called);
});

Expand Down
7 changes: 3 additions & 4 deletions packages/express/test/rest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ describe('@feathersjs/express/rest provider', () => {
});

describe('CRUD', () => {
let server: Server;
let app: express.Application;

before(async () => {
Expand All @@ -97,10 +96,10 @@ describe('@feathersjs/express/rest provider', () => {
.use('/', new Service())
.use('todo', new Service());

server = await app.listen(4777, () => app.use('tasks', new Service()));
await app.listen(4777, () => app.use('tasks', new Service()));
});

after(done => server.close(done));
after(() => app.teardown());

restTests('Services', 'todo', 4777);
restTests('Root Service', '/', 4777);
Expand Down Expand Up @@ -197,7 +196,7 @@ describe('@feathersjs/express/rest provider', () => {

app.service('hook-status').hooks({
after (hook: HookContext) {
hook.http!.statusCode = 206;
hook.http.statusCode = 206;
}
});

Expand Down
36 changes: 13 additions & 23 deletions packages/feathers/src/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,46 +140,36 @@ export class Feathers<Services, Settings> extends EventEmitter implements Feathe
}

setup () {
let promise = Promise.resolve();

// Setup each service (pass the app so that they can look up other services etc.)
for (const path of Object.keys(this.services)) {
promise = promise.then(() => {
return Object.keys(this.services).reduce((current, path) => current
.then(() => {
const service: any = this.service(path as any);

if (typeof service.setup === 'function') {
debug(`Setting up service for \`${path}\``);

return service.setup(this, path);
}
}), Promise.resolve())
.then(() => {
this._isSetup = true;
return this;
});
}

return promise.then(() => {
this._isSetup = true;
return this;
});
}

teardown () {
let promise = Promise.resolve();

// Teardown each service (pass the app so that they can look up other services etc.)
for (const path of Object.keys(this.services)) {
promise = promise.then(() => {
return Object.keys(this.services).reduce((current, path) => current
.then(() => {
const service: any = this.service(path as any);

if (typeof service.teardown === 'function') {
debug(`Teardown service for \`${path}\``);
debug(`Tearing down service for \`${path}\``);

return service.teardown(this, path);
}
}), Promise.resolve())
.then(() => {
this._isSetup = false;
return this;
});
}

return promise.then(() => {
this._isSetup = false;
return this;
});
}
}
19 changes: 17 additions & 2 deletions packages/feathers/src/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,16 +211,26 @@ export interface FeathersApplication<Services = any, Settings = any> {
path: L
): FeathersService<this, keyof any extends keyof Services ? Service : Services[L]>;

/**
* Set up the application and call all services `.setup` method if available.
*
* @param server A server instance (optional)
*/
setup (server?: any): Promise<this>;

/**
* Tear down the application and call all services `.teardown` method if available.
*
* @param server A server instance (optional)
*/
teardown (server?: any): Promise<this>;

/**
* Register application level hooks.
*
* @param map The application hook settings.
*/
hooks (map: HookOptions<this, any>): this;

teardown (cb?: () => Promise<void>): Promise<this>;
}

// This needs to be an interface instead of a type
Expand Down Expand Up @@ -365,3 +375,8 @@ export type HookMap<A, S> = {

export type HookOptions<A, S> =
HookMap<A, S> | HookFunction<A, S>[] | RegularHookMap<A, S>;

export type AppHookOptions<A> = HookOptions<A, any> & {
setup: any[],
teardown: any[]
}
22 changes: 20 additions & 2 deletions packages/feathers/test/application.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,16 +293,23 @@ describe('Feathers application', () => {
});
});

describe('.setup', () => {
it('app.setup calls .setup on all services', async () => {
describe('.setup and .teardown', () => {
it('app.setup and app.teardown calls .setup and .teardown on all services', async () => {
const app = feathers();
let setupCount = 0;
let teardownCount = 0;

app.use('/dummy', {
async setup (appRef: any, path: any) {
setupCount++;
assert.strictEqual(appRef, app);
assert.strictEqual(path, 'dummy');
},

async teardown (appRef: any, path: any) {
teardownCount++;
assert.strictEqual(appRef, app);
assert.strictEqual(path, 'dummy');
}
});

Expand All @@ -317,13 +324,24 @@ describe('Feathers application', () => {
setupCount++;
assert.strictEqual(appRef, app);
assert.strictEqual(path, 'dummy2');
},

async teardown (appRef: any, path: any) {
teardownCount++;
assert.strictEqual(appRef, app);
assert.strictEqual(path, 'dummy2');
}
});

await app.setup();

assert.ok((app as any)._isSetup);
assert.strictEqual(setupCount, 2);

await app.teardown();

assert.ok(!(app as any)._isSetup);
assert.strictEqual(teardownCount, 2);
});

it('registering a service after app.setup will be set up', done => {
Expand Down
2 changes: 1 addition & 1 deletion packages/koa/src/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import { Application as FeathersApplication, HookContext, Params, RouteLookup }
import '@feathersjs/authentication';

export type ApplicationAddons = {
server: Server;
listen (port?: number, ...args: any[]): Promise<Server>;
close (): Promise<void>;
}

export type Application<T = any, C = any> =
Expand Down
25 changes: 8 additions & 17 deletions packages/koa/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import koaQs from 'koa-qs';
import { Application as FeathersApplication } from '@feathersjs/feathers';
import { routing } from '@feathersjs/transport-commons';
import { createDebug } from '@feathersjs/commons';
import http from 'http';

import { Application } from './declarations';

Expand All @@ -28,39 +27,31 @@ export function koa<S = any, C = any> (feathersApp?: FeathersApplication<S, C>,

const app = feathersApp as any as Application<S, C>;
const { listen: koaListen, use: koaUse } = koaApp;
const feathersUse = feathersApp.use as any;
let server:http.Server | undefined;
const { use: feathersUse, teardown: feathersTeardown } = feathersApp;

Object.assign(app, {
use (location: string|Koa.Middleware, ...args: any[]) {
if (typeof location === 'string') {
return feathersUse.call(this, location, ...args);
return (feathersUse as any).call(this, location, ...args);
}

return koaUse.call(this, location);
},

async listen (port?: number, ...args: any[]) {
server = koaListen.call(this, port, ...args);
const server = koaListen.call(this, port, ...args);

this.server = server;
await this.setup(server);
debug('Feathers application listening');

return server;
},

async close () {
if ( server ) {
server.close();

await new Promise((resolve) => {
server.on('close', () => { resolve(true) });
})
}

debug('Feathers server closed');

await this.teardown();
async teardown (server?: any) {
return feathersTeardown.call(this, server).then(() =>
new Promise((resolve, reject) => this.server.close(e => e ? reject(e) : resolve(this)))
);
}
} as Application);

Expand Down
Loading