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.

Sign up for GitHub

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

Jump to bottom

Compose middleware #375

Merged
merged 5 commits into from
Feb 6, 2020
Merged

Compose middleware #375

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c6abcd
Convert middleware to be async
Jan 16, 2020
7117f84
Merge branch '@slack/bolt@next' of https://github.com/slackapi/bolt i…
seratch Feb 5, 2020
71e32f6
Fix a few tests incompatible with Bolt v2
seratch Feb 5, 2020
0c300ee
Compose middleware resolve conflicts with #393 (#2)
barlock Feb 6, 2020
4afd362
Fix test fixtures
Feb 6, 2020
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
12 changes: 6 additions & 6 deletions integration-tests/types/action.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,37 +19,37 @@ app.action({ type: 'message_action', action_id: 'dafasf' }, ({ action }) => {

// Action in listner should be - MessageAction
// $ExpectType void
app.action({ type: 'message_action' }, ({
app.action({ type: 'message_action' }, async ({
action, // $ExpectType MessageAction
}) => {
return action;
});

// $ExpectType void
app.action({ type: 'block_actions' }, ({
app.action({ type: 'block_actions' }, async ({
action, // $ExpectType BlockElementAction
}) => {
return action;
});

// $ExpectType void
app.action({ type: 'interactive_message' }, ({
app.action({ type: 'interactive_message' }, async ({
action, // $ExpectType InteractiveAction
}) => {
return action;
});

// $ExpectType void
app.action({ type: 'dialog_submission' }, ({
app.action({ type: 'dialog_submission' }, async ({
action, // $ExpectType DialogSubmitAction
}) => {
return action;
});

// If action is parameterized with MessageAction, action argument in callback should be type MessageAction
// $ExpectType void
app.action<MessageAction>({}, ({
action // $ExpectType MessageAction
app.action<MessageAction>({}, async ({
action, // $ExpectType MessageAction
}) => {
return action;
});
Expand Down
124 changes: 79 additions & 45 deletions src/App.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ErrorCode } from './errors';
import { Receiver, ReceiverEvent, SayFn, NextMiddleware } from './types';
import { ConversationStore } from './conversation-store';
import { LogLevel } from '@slack/logger';
import { ViewConstraints } from './App';
import App, { ViewConstraints } from './App';
import { WebClientOptions, WebClient } from '@slack/web-api';

describe('App', () => {
Expand Down Expand Up @@ -229,6 +229,16 @@ describe('App', () => {
});

describe('event processing', () => {
let fakeReceiver: FakeReceiver;
let fakeErrorHandler: SinonSpy;
let dummyAuthorizationResult: { botToken: string, botId: string };

beforeEach(() => {
fakeReceiver = createFakeReceiver();
fakeErrorHandler = sinon.fake();
dummyAuthorizationResult = { botToken: '', botId: '' };
});

// TODO: verify that authorize callback is called with the correct properties and responds correctly to
// various return values

Expand All @@ -242,7 +252,6 @@ describe('App', () => {

it('should warn and skip when processing a receiver event with unknown type (never crash)', async () => {
// Arrange
const fakeReceiver = createFakeReceiver();
const fakeLogger = createFakeLogger();
const fakeMiddleware = sinon.fake(noopMiddleware);
const invalidReceiverEvents = createInvalidReceiverEvents();
Expand All @@ -256,15 +265,14 @@ describe('App', () => {
}

// Assert
assert(fakeErrorHandler.notCalled);
assert(fakeMiddleware.notCalled);
assert.isAtLeast(fakeLogger.warn.callCount, invalidReceiverEvents.length);
});
it('should warn, send to global error handler, and skip when a receiver event fails authorization', async () => {
// Arrange
const fakeReceiver = createFakeReceiver();
const fakeLogger = createFakeLogger();
const fakeMiddleware = sinon.fake(noopMiddleware);
const fakeErrorHandler = sinon.fake();
const dummyAuthorizationError = new Error();
const dummyReceiverEvent = createDummyReceiverEvent();
const App = await importApp(); // tslint:disable-line:variable-name
Expand All @@ -288,47 +296,67 @@ describe('App', () => {
assert.propertyVal(fakeErrorHandler.firstCall.args[0], 'original', dummyAuthorizationError);
});
describe('global middleware', () => {
it('should process receiver events in order of #use', async () => {
// Arrange
const fakeReceiver = createFakeReceiver();
const fakeFirstMiddleware = sinon.fake(noopMiddleware);
const fakeSecondMiddleware = sinon.fake(noopMiddleware);
const dummyReceiverEvent = createDummyReceiverEvent();
const dummyAuthorizationResult = { botToken: '', botId: '' };
let fakeFirstMiddleware: SinonSpy;
let fakeSecondMiddleware: SinonSpy;
let app: App;
let dummyReceiverEvent: ReceiverEvent;

beforeEach(async () => {
const fakeConversationContext = sinon.fake.returns(noopMiddleware);
const overrides = mergeOverrides(
withNoopAppMetadata(),
withNoopWebClient(),
withMemoryStore(sinon.fake()),
withConversationContext(sinon.fake.returns(noopMiddleware)),
withNoopAppMetadata(),
withNoopWebClient(),
withMemoryStore(sinon.fake()),
withConversationContext(fakeConversationContext),
);
const App = await importApp(overrides); // tslint:disable-line:variable-name

dummyReceiverEvent = createDummyReceiverEvent();
fakeFirstMiddleware = sinon.fake(noopMiddleware);
fakeSecondMiddleware = sinon.fake(noopMiddleware);

app = new App({ receiver: fakeReceiver, authorize: sinon.fake.resolves(dummyAuthorizationResult) });
});

it('should process receiver events in order of #use', async () => {
// Act
const app = new App({ receiver: fakeReceiver, authorize: sinon.fake.resolves(dummyAuthorizationResult) });
app.use(fakeFirstMiddleware);
app.use(fakeSecondMiddleware);
app.error(fakeErrorHandler);
fakeReceiver.emit('message', dummyReceiverEvent);
await delay();

// Assert
assert(fakeErrorHandler.notCalled);
assert(fakeFirstMiddleware.calledOnce);
assert(fakeFirstMiddleware.calledBefore(fakeSecondMiddleware));
assert(fakeSecondMiddleware.calledOnce);
});

it('should error if next called multiple times', async () => {
// Arrange
app.use(fakeFirstMiddleware);
app.use(async ({ next }) => {
await next();
await next();
});
app.use(fakeSecondMiddleware);
app.error(fakeErrorHandler);
fakeReceiver.emit('message', dummyReceiverEvent);
await delay();

// Assert
assert.instanceOf(fakeErrorHandler.firstCall.args[0], Error);
});
});

describe('middleware and listener arguments', () => {

let fakeReceiver: FakeReceiver;
let fakeErrorHandler: SinonSpy;
let dummyAuthorizationResult: { [key: string]: any };
const dummyChannelId = 'CHANNEL_ID';
let overrides: Override;

function buildOverrides(secondOverrides: Override[]): Override {
fakeReceiver = createFakeReceiver();
fakeErrorHandler = sinon.fake();
dummyAuthorizationResult = { botToken: '', botId: '' };
overrides = mergeOverrides(
withNoopAppMetadata(),
...secondOverrides,
Expand All @@ -350,7 +378,7 @@ describe('App', () => {
},
{ // IncomingEventType.Command (app.command)
body: {
command: '/COMMAND_NAME',
command: { command: '/COMMAND_NAME' },
barlock marked this conversation as resolved.
Show resolved Hide resolved
},
ack: noop,
},
Expand Down Expand Up @@ -489,31 +517,34 @@ describe('App', () => {
authorize: sinon.fake.resolves(dummyAuthorizationResult),
});

app.use((_args) => { ackFn(); });
app.action('block_action_id', ({ }) => { actionFn(); });
app.action({ callback_id: 'message_action_callback_id' }, ({ }) => { actionFn(); });
app.use(async ({ next }) => {
await ackFn();
await next();
});
app.action('block_action_id', async ({ }) => { await actionFn(); });
app.action({ callback_id: 'message_action_callback_id' }, async ({ }) => { await actionFn(); });
app.action(
{ type: 'message_action', callback_id: 'another_message_action_callback_id' },
({ }) => { actionFn(); });
app.action({ type: 'message_action', callback_id: 'does_not_exist' }, ({ }) => { actionFn(); });
app.action({ callback_id: 'interactive_message_callback_id' }, ({ }) => { actionFn(); });
app.action({ callback_id: 'dialog_submission_callback_id' }, ({ }) => { actionFn(); });
app.view('view_callback_id', ({ }) => { viewFn(); });
app.view({ callback_id: 'view_callback_id', type: 'view_closed' }, ({ }) => { viewFn(); });
app.options('external_select_action_id', ({ }) => { optionsFn(); });
app.options({ callback_id: 'dialog_suggestion_callback_id' }, ({ }) => { optionsFn(); });

app.event('app_home_opened', ({ }) => { /* noop */ });
app.message('hello', ({ }) => { /* noop */ });
app.command('/echo', ({ }) => { /* noop */ });
async ({ }) => { await actionFn(); });
app.action({ type: 'message_action', callback_id: 'does_not_exist' }, async ({ }) => { await actionFn(); });
app.action({ callback_id: 'interactive_message_callback_id' }, async ({ }) => { await actionFn(); });
app.action({ callback_id: 'dialog_submission_callback_id' }, async ({ }) => { await actionFn(); });
app.view('view_callback_id', async ({ }) => { await viewFn(); });
app.view({ callback_id: 'view_callback_id', type: 'view_closed' }, async ({ }) => { await viewFn(); });
app.options('external_select_action_id', async ({ }) => { await optionsFn(); });
app.options({ callback_id: 'dialog_suggestion_callback_id' }, async ({ }) => { await optionsFn(); });

app.event('app_home_opened', async ({ }) => { /* noop */ });
app.message('hello', async ({ }) => { /* noop */ });
// app.command('/echo', async ({ }) => { /* noop */ });
barlock marked this conversation as resolved.
Show resolved Hide resolved

// invalid view constraints
const invalidViewConstraints1 = {
callback_id: 'foo',
type: 'view_submission',
unknown_key: 'should be detected',
} as any as ViewConstraints;
app.view(invalidViewConstraints1, ({ }) => { /* noop */ });
app.view(invalidViewConstraints1, async ({ }) => { /* noop */ });
assert.isTrue(fakeLogger.error.called);

fakeLogger.error = sinon.fake();
Expand All @@ -523,7 +554,7 @@ describe('App', () => {
type: undefined,
unknown_key: 'should be detected',
} as any as ViewConstraints;
app.view(invalidViewConstraints2, ({ }) => { /* noop */ });
app.view(invalidViewConstraints2, async ({ }) => { /* noop */ });
assert.isTrue(fakeLogger.error.called);

app.error(fakeErrorHandler);
Expand Down Expand Up @@ -571,6 +602,7 @@ describe('App', () => {
await delay();

// Assert
assert(fakeErrorHandler.notCalled);
assert.equal(fakeAxiosPost.callCount, 1);
// Assert that each call to fakeAxiosPost had the right arguments
assert(fakeAxiosPost.calledWith(responseUrl, { text: responseText }));
Expand Down Expand Up @@ -624,11 +656,12 @@ describe('App', () => {
receiver: fakeReceiver,
authorize: sinon.fake.resolves(dummyAuthorizationResult),
});
app.use(({ logger, body }) => {
app.use(async ({ logger, body, next }) => {
logger.info(body);
await next();
});

app.event('app_home_opened', ({ logger, event }) => {
app.event('app_home_opened', async ({ logger, event }) => {
logger.debug(event);
});

Expand Down Expand Up @@ -689,8 +722,9 @@ describe('App', () => {
return Promise.resolve({ userToken: token, botId: 'B123' });
},
});
app.use(async ({ client }) => {
app.use(async ({ client, next }) => {
await client.auth.test();
await next();
});
const clients: WebClient[] = [];
app.event('app_home_opened', async ({ client }) => {
Expand Down Expand Up @@ -763,7 +797,7 @@ describe('App', () => {
// IncomingEventType.Command
{
body: {
command: '/COMMAND_NAME',
command: { command: '/COMMAND_NAME' },
barlock marked this conversation as resolved.
Show resolved Hide resolved
channel_id: channelId,
team_id: 'TEAM_ID',
},
Expand Down Expand Up @@ -924,7 +958,7 @@ describe('App', () => {

// Act
const app = new App({ receiver: fakeReceiver, authorize: sinon.fake.resolves(dummyAuthorizationResult) });
app.use((args) => {
app.use(async (args) => {
assert.isUndefined((args as any).say);
// If the above assertion fails, then it would throw an AssertionError and the following line will not be
// called
Expand Down Expand Up @@ -1061,7 +1095,7 @@ function createDummyReceiverEvent(): ReceiverEvent {

// Utility functions
const noop = (() => Promise.resolve(undefined));
const noopMiddleware = ({ next }: { next: NextMiddleware; }) => { next(); };
const noopMiddleware = async ({ next }: { next: NextMiddleware; }) => { await next(); };
const noopAuthorize = (() => Promise.resolve({}));

// TODO: swap out rewiremock for proxyquire to see if it saves execution time
51 changes: 21 additions & 30 deletions src/App.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
} from './types';
import { IncomingEventType, getTypeAndConversation, assertNever } from './helpers';
import { ErrorCode, CodedError, errorWithCode, asCodedError } from './errors';
import { MiddlewareContext } from './types/middleware';
const packageJson = require('../package.json'); // tslint:disable-line:no-require-imports no-var-requires

/** App initialization options */
Expand Down Expand Up @@ -532,38 +533,28 @@ export default class App {
// Events API requests are acknowledged right away, since there's no data expected
await ack();
}
const middlewareChain = [...this.middleware];

if (this.listeners.length > 0) {
middlewareChain.push(async (ctx) => {
const { next } = ctx;

await Promise.all(this.listeners.map(
listener => processMiddleware(listener, ctx)));

await next();
});
}

// Dispatch event through global middleware
processMiddleware(
listenerArgs as AnyMiddlewareArgs,
this.middleware,
(globalProcessedContext: Context, globalProcessedArgs: AnyMiddlewareArgs, startGlobalBubble) => {
this.listeners.forEach((listenerMiddleware) => {
// Dispatch event through all listeners
processMiddleware(
globalProcessedArgs,
listenerMiddleware,
(_listenerProcessedContext, _listenerProcessedArgs, startListenerBubble) => {
startListenerBubble();
},
(error) => {
startGlobalBubble(error);
},
globalProcessedContext,
this.logger,
listenerArgClient,
);
});
},
(globalError?: CodedError | Error) => {
if (globalError !== undefined) {
this.onGlobalError(globalError);
}
},
context,
this.logger,
listenerArgClient,
);
try {
await processMiddleware(middlewareChain, {
context,
...(listenerArgs as MiddlewareContext<AnyMiddlewareArgs>),
});
} catch (error) {
this.onGlobalError(error);
}
}

/**
Expand Down
Loading