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

packages/adapter-commons/package.json

@@ -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"

packages/express/src/declarations.ts

@@ -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> =

packages/express/src/index.ts

@@ -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';
 
@@ -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[]) {
@@ -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>);
 

packages/express/test/authentication.test.ts

@@ -16,7 +16,6 @@ describe('@feathersjs/express/authentication', () => {
   const password = 'superexpress';
 
   let app: express.Application;
-  let server: any;
   let user: any;
   let authResult: AuthenticationResult;
 
@@ -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);
     });
@@ -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', () => {

packages/express/test/index.test.ts

@@ -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;
 
@@ -206,7 +183,7 @@ describe('@feathersjs/express', () => {
       called = true;
     })
 
-    await app.close();
+    await app.teardown();
     assert.ok(called);
   });
 

packages/express/test/rest.test.ts

@@ -78,7 +78,6 @@ describe('@feathersjs/express/rest provider', () => {
   });
 
   describe('CRUD', () => {
-    let server: Server;
     let app: express.Application;
 
     before(async () => {
@@ -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);
@@ -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;
           }
         });
 

packages/feathers/src/application.ts

@@ -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;
-    });
   }
 }

packages/feathers/src/declarations.ts

@@ -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
@@ -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[]
+}

packages/feathers/test/application.test.ts

@@ -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');
         }
       });
 
@@ -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 => {

packages/koa/src/declarations.ts

@@ -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> =

packages/koa/src/index.ts

@@ -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';
 
@@ -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);