Skip to content

Commit

Permalink
Add logger and client to Middleware args
Browse files Browse the repository at this point in the history
  • Loading branch information
seratch committed Feb 4, 2020
1 parent 8e677ef commit 2901492
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 49 deletions.
20 changes: 14 additions & 6 deletions src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export default class App {

private clientOptions: WebClientOptions;

private clients: {[teamId: string]: WebClientPool} = {};
private clients: { [teamId: string]: WebClientPool } = {};

/** Receiver - ingests events from the Slack platform */
private receiver: Receiver;
Expand Down Expand Up @@ -302,9 +302,9 @@ export default class App {
// NOTE: this is what's called a convenience generic, so that types flow more easily without casting.
// https://basarat.gitbooks.io/typescript/docs/types/generics.html#design-pattern-convenience-generic
public action<Action extends SlackAction = SlackAction>(
actionId: string | RegExp,
...listeners: Middleware<SlackActionMiddlewareArgs<Action>>[]
): void;
actionId: string | RegExp,
...listeners: Middleware<SlackActionMiddlewareArgs<Action>>[]
): void;
public action<Action extends SlackAction = SlackAction,
Constraints extends ActionConstraints<Action> = ActionConstraints<Action>>(
constraints: Constraints,
Expand All @@ -317,7 +317,7 @@ export default class App {
...listeners: Middleware<SlackActionMiddlewareArgs<Extract<Action, { type: Constraints['type'] }>>>[]
): void {
// Normalize Constraints
const constraints: ActionConstraints =
const constraints: ActionConstraints =
(typeof actionIdOrConstraints === 'string' || util.types.isRegExp(actionIdOrConstraints)) ?
{ action_id: actionIdOrConstraints } : actionIdOrConstraints;

Expand Down Expand Up @@ -458,13 +458,17 @@ export default class App {
// Set body and payload (this value will eventually conform to AnyMiddlewareArgs)
// NOTE: the following doesn't work because... distributive?
// const listenerArgs: Partial<AnyMiddlewareArgs> = {
const listenerArgs: Pick<AnyMiddlewareArgs, 'logger' | 'client' | 'body' | 'payload'> & {
const listenerArgs: Pick<AnyMiddlewareArgs, 'body' | 'payload'> & {
/** Say function might be set below */
say?: SayFn
/** Respond function might be set below */
respond?: RespondFn,
/** Ack function might be set below */
ack?: AckFn<any>,
/** The logger for this Bolt app */
logger?: Logger,
/** WebClient with token */
client?: WebClient,
} = {
logger: this.logger,
client: listenerArgClient,
Expand Down Expand Up @@ -546,6 +550,8 @@ export default class App {
startGlobalBubble(error);
},
globalProcessedContext,
this.logger,
listenerArgClient,
);
});
},
Expand All @@ -555,6 +561,8 @@ export default class App {
}
},
context,
this.logger,
listenerArgClient,
);
}

Expand Down
15 changes: 13 additions & 2 deletions src/conversation-store.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { Override, createFakeLogger, delay, wrapToResolveOnFirstCall } from './t
import rewiremock from 'rewiremock';
import { ConversationStore } from './conversation-store';
import { AnyMiddlewareArgs, NextMiddleware, Context } from './types';
import { WebClient } from '@slack/web-api';
import { Logger } from '@slack/logger';

describe('conversationContext middleware', () => {
it('should forward events that have no conversation ID', async () => {
Expand All @@ -19,7 +21,11 @@ describe('conversationContext middleware', () => {
const { conversationContext } = await importConversationStore(
withGetTypeAndConversation(fakeGetTypeAndConversation),
);
const fakeArgs = { body: {}, context: dummyContext, next: fakeNext } as unknown as MiddlewareArgs;
const fakeArgs = {
body: {},
context: dummyContext,
next: fakeNext,
} as unknown as MiddlewareArgs;

// Act
const middleware = conversationContext(fakeStore, fakeLogger);
Expand Down Expand Up @@ -186,7 +192,12 @@ describe('MemoryStore', () => {

/* Testing Harness */

type MiddlewareArgs = AnyMiddlewareArgs & { next: NextMiddleware, context: Context };
type MiddlewareArgs = AnyMiddlewareArgs & {
next: NextMiddleware,
context: Context,
logger: Logger,
client: WebClient,
};

interface DummyContext<ConversationState> {
conversation?: ConversationState;
Expand Down
66 changes: 48 additions & 18 deletions src/middleware/builtin.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { onlyCommands, onlyEvents, matchCommandName, matchEventType, subtype } f
import { SlashCommand } from '../types/command/index';
import { SlackEvent, AppMentionEvent, BotMessageEvent } from '../types/events/base-events';
import { WebClient } from '@slack/web-api';
import { Logger } from '@slack/logger';

describe('matchMessage()', () => {
function initializeTestCase(pattern: string | RegExp): Mocha.AsyncFunc {
Expand Down Expand Up @@ -479,12 +480,12 @@ describe('matchCommandName', () => {
const logger = createFakeLogger();
const client = new WebClient(undefined, { logger, slackApiUrl: undefined });

function buildArgs(fakeNext: NextMiddleware): SlackCommandMiddlewareArgs & { next: any, context: any } {
function buildArgs(fakeNext: NextMiddleware): SlackCommandMiddlewareArgs & MiddlewareCommonArgs {
const payload: SlashCommand = { ...validCommandPayload };
return {
payload,
logger,
client,
payload,
command: payload,
body: payload,
say: sayNoop,
Expand Down Expand Up @@ -516,8 +517,6 @@ describe('onlyEvents', () => {
it('should detect valid requests', async () => {
const fakeNext = sinon.fake();
const args: SlackEventMiddlewareArgs<'app_mention'> & { event?: SlackEvent } = {
logger,
client,
payload: appMentionEvent,
event: appMentionEvent,
message: null as never, // a bit hackey to sartisfy TS compiler
Expand All @@ -533,7 +532,13 @@ describe('onlyEvents', () => {
},
say: sayNoop,
};
onlyEvents({ next: fakeNext, context: {}, ...args });
onlyEvents({
logger,
client,
next: fakeNext,
context: {},
...args,
});
assert.isTrue(fakeNext.called);
});

Expand Down Expand Up @@ -562,8 +567,6 @@ describe('matchEventType', () => {

function buildArgs(): SlackEventMiddlewareArgs<'app_mention'> & { event?: SlackEvent } {
return {
logger,
client,
payload: appMentionEvent,
event: appMentionEvent,
message: null as never, // a bit hackey to sartisfy TS compiler
Expand All @@ -583,13 +586,25 @@ describe('matchEventType', () => {

it('should detect valid requests', async () => {
const fakeNext = sinon.fake();
matchEventType('app_mention')({ next: fakeNext, context: {}, ...buildArgs() });
matchEventType('app_mention')({
logger,
client,
next: fakeNext,
context: {},
...buildArgs(),
});
assert.isTrue(fakeNext.called);
});

it('should skip other requests', async () => {
const fakeNext = sinon.fake();
matchEventType('app_home_opened')({ next: fakeNext, context: {}, ...buildArgs() });
matchEventType('app_home_opened')({
logger,
client,
next: fakeNext,
context: {},
...buildArgs(),
});
assert.isFalse(fakeNext.called);
});
});
Expand All @@ -600,8 +615,6 @@ describe('subtype', () => {

function buildArgs(): SlackEventMiddlewareArgs<'message'> & { event?: SlackEvent } {
return {
logger,
client,
payload: botMessageEvent,
event: botMessageEvent,
message: botMessageEvent,
Expand All @@ -621,13 +634,25 @@ describe('subtype', () => {

it('should detect valid requests', async () => {
const fakeNext = sinon.fake();
subtype('bot_message')({ next: fakeNext, context: {}, ...buildArgs() });
subtype('bot_message')({
logger,
client,
next: fakeNext,
context: {},
...buildArgs(),
});
assert.isTrue(fakeNext.called);
});

it('should skip other requests', async () => {
const fakeNext = sinon.fake();
subtype('me_message')({ next: fakeNext, context: {}, ...buildArgs() });
subtype('me_message')({
logger,
client,
next: fakeNext,
context: {},
...buildArgs(),
});
assert.isFalse(fakeNext.called);
});
});
Expand All @@ -638,14 +663,19 @@ interface DummyContext {
matches?: RegExpExecArray;
}

type MessageMiddlewareArgs = SlackEventMiddlewareArgs<'message'> & { next: NextMiddleware, context: Context };
type TokensRevokedMiddlewareArgs = SlackEventMiddlewareArgs<'tokens_revoked'>
& { next: NextMiddleware, context: Context };
interface MiddlewareCommonArgs {
next: NextMiddleware;
context: Context;
logger: Logger;
client: WebClient;
}
type MessageMiddlewareArgs = SlackEventMiddlewareArgs<'message'> & MiddlewareCommonArgs;
type TokensRevokedMiddlewareArgs = SlackEventMiddlewareArgs<'tokens_revoked'> & MiddlewareCommonArgs;

type MemberJoinedOrLeftChannelMiddlewareArgs = SlackEventMiddlewareArgs<'member_joined_channel' | 'member_left_channel'>
& { next: NextMiddleware, context: Context };
& MiddlewareCommonArgs;

type CommandMiddlewareArgs = SlackCommandMiddlewareArgs & { next: NextMiddleware; context: Context };
type CommandMiddlewareArgs = SlackCommandMiddlewareArgs & MiddlewareCommonArgs;

async function importBuiltin(
overrides: Override = {},
Expand Down
8 changes: 6 additions & 2 deletions src/middleware/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
NextMiddleware,
PostProcessFn,
} from '../types';
import { WebClient } from '@slack/web-api';
import { Logger } from '@slack/logger';

// TODO: what happens if an error is thrown inside a middleware/listener function? it should propagate up and eventually
// be dealt with by the global error handler
Expand All @@ -14,6 +16,8 @@ export function processMiddleware(
afterMiddleware: (context: Context, args: AnyMiddlewareArgs, startBubble: (error?: Error) => void) => void,
afterPostProcess: (error?: Error) => void,
context: Context = {},
logger: Logger,
client: WebClient,
): void {

// Generate next()
Expand All @@ -33,7 +37,7 @@ export function processMiddleware(

// In this condition, errorOrPostProcess will be a postProcess function or undefined
postProcessFns[middlewareIndex - 1] = errorOrPostProcess === undefined ? noopPostProcess : errorOrPostProcess;
thisMiddleware({ context, next: nextWhenNotLast, ...initialArguments });
thisMiddleware({ context, logger, client, next: nextWhenNotLast, ...initialArguments });

if (isLastMiddleware) {
postProcessFns[middlewareIndex] = noopPostProcess;
Expand Down Expand Up @@ -75,7 +79,7 @@ export function processMiddleware(
};

const firstMiddleware = middleware[0];
firstMiddleware({ context, next, ...initialArguments });
firstMiddleware({ context, logger, client, next, ...initialArguments });
}

function noop(): void { } // tslint:disable-line:no-empty
Expand Down
4 changes: 0 additions & 4 deletions src/types/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import { InteractiveMessage } from './interactive-message';
import { DialogSubmitAction, DialogValidation } from './dialog-action';
import { MessageAction } from './message-action';
import { SayFn, SayArguments, RespondFn, AckFn } from '../utilities';
import { Logger } from '@slack/logger';
import { WebClient } from '@slack/web-api';

/**
* All known actions from Slack's Block Kit interactive components, message actions, dialogs, and legacy interactive
Expand All @@ -36,8 +34,6 @@ export type SlackAction = BlockAction | InteractiveMessage | DialogSubmitAction
* this case `ElementAction` must extend `BasicElementAction`.
*/
export interface SlackActionMiddlewareArgs<Action extends SlackAction = SlackAction> {
logger: Logger;
client: WebClient;
payload: (
Action extends BlockAction<infer ElementAction> ? ElementAction :
Action extends InteractiveMessage<infer InteractiveAction> ? InteractiveAction :
Expand Down
4 changes: 0 additions & 4 deletions src/types/command/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
import { StringIndexed } from '../helpers';
import { SayFn, RespondFn, RespondArguments, AckFn } from '../utilities';
import { Logger } from '@slack/logger';
import { WebClient } from '@slack/web-api';

/**
* Arguments which listeners and middleware receive to process a slash command from Slack.
*/
export interface SlackCommandMiddlewareArgs {
logger: Logger;
client: WebClient;
payload: SlashCommand;
command: this['payload'];
body: this['payload'];
Expand Down
4 changes: 0 additions & 4 deletions src/types/events/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,11 @@ export * from './base-events';
import { SlackEvent, BasicSlackEvent } from './base-events';
import { StringIndexed } from '../helpers';
import { SayFn } from '../utilities';
import { Logger } from '@slack/logger';
import { WebClient } from '@slack/web-api';

/**
* Arguments which listeners and middleware receive to process an event from Slack's Events API.
*/
export interface SlackEventMiddlewareArgs<EventType extends string = string> {
logger: Logger;
client: WebClient;
payload: EventFromType<EventType>;
event: this['payload'];
message: EventType extends 'message' ? this['payload'] : never;
Expand Down
9 changes: 8 additions & 1 deletion src/types/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { SlackCommandMiddlewareArgs } from './command';
import { SlackOptionsMiddlewareArgs } from './options';
import { SlackViewMiddlewareArgs } from './view';
import { CodedError, ErrorCode } from '../errors';
import { WebClient } from '@slack/web-api';
import { Logger } from '@slack/logger';

export type AnyMiddlewareArgs =
SlackEventMiddlewareArgs | SlackActionMiddlewareArgs | SlackCommandMiddlewareArgs |
Expand All @@ -21,7 +23,12 @@ export interface Context extends StringIndexed {
// constraint would mess up the interface of App#event(), App#message(), etc.
export interface Middleware<Args> {
// TODO: is there something nice we can do to get context's property types to flow from one middleware to the next?
(args: Args & { next: NextMiddleware, context: Context }): unknown;
(args: Args & {
next: NextMiddleware,
context: Context,
logger: Logger,
client: WebClient,
}): unknown;
}

export interface NextMiddleware {
Expand Down
4 changes: 0 additions & 4 deletions src/types/options/index.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import { Option } from '@slack/types';
import { StringIndexed, XOR } from '../helpers';
import { AckFn } from '../utilities';
import { Logger } from '@slack/logger';
import { WebClient } from '@slack/web-api';

/**
* Arguments which listeners and middleware receive to process an options request from Slack
*/
export interface SlackOptionsMiddlewareArgs<Source extends OptionsSource = OptionsSource> {
logger: Logger;
client: WebClient;
payload: OptionsRequest<Source>;
body: this['payload'];
options: this['payload'];
Expand Down
4 changes: 0 additions & 4 deletions src/types/view/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { StringIndexed } from '../helpers';
import { RespondArguments, AckFn } from '../utilities';
import { Logger } from '@slack/logger';
import { WebClient } from '@slack/web-api';

/**
* Known view action types
Expand All @@ -12,8 +10,6 @@ export type SlackViewAction = ViewSubmitAction | ViewClosedAction;
* Arguments which listeners and middleware receive to process a view submission event from Slack.
*/
export interface SlackViewMiddlewareArgs<ViewActionType extends SlackViewAction = SlackViewAction> {
logger: Logger;
client: WebClient;
payload: ViewOutput;
view: this['payload'];
body: ViewActionType;
Expand Down

0 comments on commit 2901492

Please sign in to comment.