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

[RFC] Port loopback-phase to TypeScript for LoopBack 4 #1671

Closed
wants to merge 2 commits into from
Closed
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
1 change: 1 addition & 0 deletions greenkeeper.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"packages/openapi-spec-builder/package.json",
"packages/openapi-v3-types/package.json",
"packages/openapi-v3/package.json",
"packages/phase/package.json",
"packages/repository-json-schema/package.json",
"packages/repository/package.json",
"packages/rest-explorer/package.json",
Expand Down
1 change: 1 addition & 0 deletions packages/phase/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package-lock=false
25 changes: 25 additions & 0 deletions packages/phase/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
Copyright (c) IBM Corp. 2018. All Rights Reserved.
Node module: @loopback/phase
This project is licensed under the MIT License, full text below.

--------

MIT license

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
134 changes: 134 additions & 0 deletions packages/phase/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
# @loopback/phase

This module defines `Phase` and `PhaseList` to organize a list of handlers by
groups. It's rewritten in TypeScript based on
https://github.com/strongloop/loopback-phase.

## Installation

```sh
npm install --save @loopback/phase
```

## Basic Use

### Handler

A handler is a function that takes a `Context` object, an optional
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Till I dug into the code, I found the usage of the work Context here confusing as I thought this was referring to @loopback/context.

`HandlerChain` object and returns a `Promise`.

When a handler is invoked as part of a chain, it can control how to proceed as
follows:

- Continue to invoke downstream handlers after it exits

```ts
async ctx => {
console.log(ctx.req.url);
};
```

- Invoke downstream handlers within the method in cascading style

```ts
async (ctx, chain) => {
const start = process.hrtime();
await chain!.next();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please find a better type description for the callback function, one that does not require users to use ! operator. If the framework always provide chain value, then this fact should be encoded in the type signature of the callback.

const duration = process.hrtime(start);
console.log('Duration: ', duration);
};
```

- Terminate the handler chain and return

```ts
async (ctx, chain) => {
if (ctx.req.url === '/status') {
ctx.response = {status: 'ok'};
chain!.return();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find return as ambiguous. Can we use a name that better expresses the intent please? For example: abort, terminate or stop.

}
};
```

- Abort the handler chain by throwing an error

```ts
async (ctx, chain) => {
throw new Error('invalid request');
// or
// chain!.throw(new Error('invalid request'));
};
```

### Phase

A `Phase` is a bucket for organizing handlers. Each phase has an `id` and three
sub-phases:

- before (contains handlers to be invoked before the phase)
- use (contains handlers to be invoked during the phase)
- after (contains handlers to be invoked after the phase)

The handlers within the same subphase will be executed in serial or parallel
depending on the `options.parallel` flag, which is default to `false`.

The three sub-phases within the same phase will be executed in the order of
`before`, `use`, and `after`.

There is also a `failFast` option to control how to handle errors. If `failFast`
is set to `false`, errors will be caught and set on the `ctx.error` without
aborting the handler chain.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this rather dangerous. I expect that most people writing phase-handlers are not going to be aware of the failFast:false mode. As a result, failFast:false mode can execute handlers that are not prepared to handle the situation left after one of the previous handlers failed.

I think a better solution would be to enable failFast:false mode on a per-handler basis - i.e. allow each handler to signal whether it wants to be executed even if there was an error up in the chain.

What is the use case you are envisioning for failFast:false mode?

Can we perhaps leave this feature out of this initial pull request, to get it landed faster?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduce failFast:false mode mostly for error phase so that we can have a list of error handlers to mediate errors.


### PhaseList

A `PhaseList` is an ordered list of phases. Each phase is uniquely identified by
its id within the same `PhaseList`. The `PhaseList` provides methods to manage
phases, such as adding a phase before or after another phase.

In addition to the regular phases, each `PhaseList` has two built-in phases:

- errorPhase (`$error`)
- finalPhase (`$final`)

The PhaseList is a chain of grouped list of handlers. When the handler chain is
invoked with a given context, it passes control to handlers registered for each
regular phase one by one sequentially until a handler changes the process by
cascading, returning, or aborting. The flow is very similar as:

```ts
try {
// await run handlers for phase 1
// await run handlers for phase 2
// ...
// await run handlers for phase N
} catch (e) {
// await run handlers for errorPhase
} finally {
// await run handlers for finalPhase
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we enforcing the order of $error and $final phase? Can users insert additional phases before, in-between or after these two built-in phases?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we enforce them as catch and finally. No additional phases can interfere with them.

}
```

### Pass information across handlers

The `Context` object can be used to pass data across handlers following the
handler chain so that downstream handlers can access such information populated
by upstream handlers. For cascading handlers, it's also possible to receive data
from downstream handlers after calling `await ctx.handlerChain.next()`.

## Contributions

- [Guidelines](https://github.com/strongloop/loopback-next/blob/master/docs/CONTRIBUTING.md)
- [Join the team](https://github.com/strongloop/loopback-next/issues/110)

## Tests

Run `npm test` from the root folder.

## Contributors

See
[all contributors](https://github.com/strongloop/loopback-next/graphs/contributors).

## License

MIT
6 changes: 6 additions & 0 deletions packages/phase/docs.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"content": [
"src/*.ts"
],
"codeSectionDepth": 4
}
6 changes: 6 additions & 0 deletions packages/phase/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright IBM Corp. 2018. All Rights Reserved.
// Node module: @loopback/phase
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

export * from './dist';
6 changes: 6 additions & 0 deletions packages/phase/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright IBM Corp. 2018. All Rights Reserved.
// Node module: @loopback/phase
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

module.exports = require('./dist');
8 changes: 8 additions & 0 deletions packages/phase/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Copyright IBM Corp. 2018. All Rights Reserved.
// Node module: @loopback/phase
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

// DO NOT EDIT THIS FILE
// Add any additional (re)exports to src/index.ts instead.
export * from './src';
50 changes: 50 additions & 0 deletions packages/phase/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
{
"name": "@loopback/phase",
"version": "1.0.0-1",
"description": "Phase based handler chain",
"engines": {
"node": ">=8.9"
},
"scripts": {
"build:apidocs": "lb-apidocs",
"build": "lb-tsc es2017 --outDir dist",
"clean": "lb-clean loopback-phase*.tgz dist* package api-docs",
"pretest": "npm run build",
"integration": "lb-mocha \"dist/__tests__/integration/**/*.js\"",
"test": "lb-mocha \"dist/__tests__/**/*.js\"",
"unit": "lb-mocha \"dist/__tests__/unit/**/*.js\"",
"verify": "npm pack && tar xf loopback-phase*.tgz && tree package && npm run clean"
},
"author": "IBM",
"copyright.owner": "IBM Corp.",
"license": "MIT",
"dependencies": {
"debug": "^4.1.1"
},
"devDependencies": {
"@loopback/build": "^1.2.1",
"@loopback/testlab": "^1.0.5",
"@loopback/tslint-config": "^2.0.0",
"@types/debug": "^4.1.0",
"@types/node": "^10.9.4"
},
"keywords": [
"LoopBack",
"Phase"
],
"files": [
"README.md",
"index.js",
"index.d.ts",
"dist*/src",
"dist*/index*",
"src"
],
"publishConfig": {
"access": "public"
},
"repository": {
"type": "git",
"url": "https://github.com/strongloop/loopback-next.git"
}
}
147 changes: 147 additions & 0 deletions packages/phase/src/__tests__/unit/handler.unit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
// Copyright IBM Corp. 2018. All Rights Reserved.
// Node module: @loopback/phase
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {expect} from '@loopback/testlab';
import {asRunnable, HandlerChain} from '../..';

describe('Handler', () => {
describe('asRunnable()', () => {
it('should execute phase handlers', async () => {
const context = {
called: [],
};
const run = asRunnable([
async ctx => {
ctx.called.push('foo');
},
async ctx => {
ctx.called.push('bar');
},
]);
await run(context);
expect(context.called).eql(['foo', 'bar']);
});

it('should allow handlerChain.next()', async () => {
const context = {
called: [],
};
const run = asRunnable([
async (ctx, chain) => {
ctx.called.push('foo');
await chain.next();
ctx.called.push('foo-done');
},
async ctx => {
ctx.called.push('bar');
},
]);
await run(context);
expect(context.called).eql(['foo', 'bar', 'foo-done']);
});

it('should allow handlerChain.stop()', async () => {
const context = {
called: [],
};
const run = asRunnable([
async (ctx, chain) => {
ctx.called.push('foo');
chain.stop();
ctx.called.push('foo-done');
},
async ctx => {
ctx.called.push('bar');
},
]);
await run(context);
expect(context.called).eql(['foo', 'foo-done']);
});

it('should allow handlerChain.throw()', async () => {
const context = {
called: [],
};
const run = asRunnable([
async (ctx, chain) => {
ctx.called.push('foo');
chain.throw(new Error('foo fails'));
ctx.called.push('foo-done');
},
async ctx => {
ctx.called.push('bar');
},
]);
await expect(run(context)).to.be.rejectedWith('foo fails');
expect(context.called).eql(['foo']);
});

it('should execute phase handlers in parallel', async () => {
const context = {
called: [],
done: false,
};
const run = asRunnable(
[
async ctx => {
ctx.called.push('foo');
},
async ctx => {
ctx.called.push('bar');
},
],
{parallel: true},
);
await run(context);
expect(context.called).containEql('foo');
expect(context.called).containEql('bar');
expect(context.done).to.be.false();
});
});
});

describe('HandlerChain', () => {
let chain: HandlerChain;
let called: string[] = [];

beforeEach(() => {
called = [];
chain = new HandlerChain({}, [
async ctx => {
called.push('1');
},
async ctx => {
called.push('2');
},
]);
});
it('allows next()', async () => {
await chain.next();
expect(called).to.eql(['2']);
});

it('allows stop()', () => {
chain.stop();
expect(chain.done).to.be.true();
});

it('allows throw()', () => {
expect(() => chain.throw(new Error('err'))).to.throw('err');
});

it('does not allow next() to be called after done', async () => {
await chain.next();
return expect(chain.next()).to.be.rejectedWith(
'The handler chain is done.',
);
});

it('does not allow next() to be called after stop', () => {
chain.stop();
return expect(chain.next()).to.be.rejectedWith(
'The handler chain is done.',
);
});
});
Loading