From 290149295750c89551574480b09aa23e990eff1d Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Fri, 31 Jan 2020 18:11:10 +0900 Subject: [PATCH] Add logger and client to Middleware args --- src/App.ts | 20 +++++++---- src/conversation-store.spec.ts | 15 ++++++-- src/middleware/builtin.spec.ts | 66 ++++++++++++++++++++++++---------- src/middleware/process.ts | 8 +++-- src/types/actions/index.ts | 4 --- src/types/command/index.ts | 4 --- src/types/events/index.ts | 4 --- src/types/middleware.ts | 9 ++++- src/types/options/index.ts | 4 --- src/types/view/index.ts | 4 --- 10 files changed, 89 insertions(+), 49 deletions(-) diff --git a/src/App.ts b/src/App.ts index 6dffce385..4d7f80238 100644 --- a/src/App.ts +++ b/src/App.ts @@ -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; @@ -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( - actionId: string | RegExp, - ...listeners: Middleware>[] - ): void; + actionId: string | RegExp, + ...listeners: Middleware>[] + ): void; public action = ActionConstraints>( constraints: Constraints, @@ -317,7 +317,7 @@ export default class App { ...listeners: Middleware>>[] ): void { // Normalize Constraints - const constraints: ActionConstraints = + const constraints: ActionConstraints = (typeof actionIdOrConstraints === 'string' || util.types.isRegExp(actionIdOrConstraints)) ? { action_id: actionIdOrConstraints } : actionIdOrConstraints; @@ -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 = { - const listenerArgs: Pick & { + const listenerArgs: Pick & { /** Say function might be set below */ say?: SayFn /** Respond function might be set below */ respond?: RespondFn, /** Ack function might be set below */ ack?: AckFn, + /** The logger for this Bolt app */ + logger?: Logger, + /** WebClient with token */ + client?: WebClient, } = { logger: this.logger, client: listenerArgClient, @@ -546,6 +550,8 @@ export default class App { startGlobalBubble(error); }, globalProcessedContext, + this.logger, + listenerArgClient, ); }); }, @@ -555,6 +561,8 @@ export default class App { } }, context, + this.logger, + listenerArgClient, ); } diff --git a/src/conversation-store.spec.ts b/src/conversation-store.spec.ts index e166909c6..f8abfe9b8 100644 --- a/src/conversation-store.spec.ts +++ b/src/conversation-store.spec.ts @@ -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 () => { @@ -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); @@ -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 { conversation?: ConversationState; diff --git a/src/middleware/builtin.spec.ts b/src/middleware/builtin.spec.ts index 1353ce8f3..575d95750 100644 --- a/src/middleware/builtin.spec.ts +++ b/src/middleware/builtin.spec.ts @@ -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 { @@ -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, @@ -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 @@ -533,7 +532,13 @@ describe('onlyEvents', () => { }, say: sayNoop, }; - onlyEvents({ next: fakeNext, context: {}, ...args }); + onlyEvents({ + logger, + client, + next: fakeNext, + context: {}, + ...args, + }); assert.isTrue(fakeNext.called); }); @@ -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 @@ -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); }); }); @@ -600,8 +615,6 @@ describe('subtype', () => { function buildArgs(): SlackEventMiddlewareArgs<'message'> & { event?: SlackEvent } { return { - logger, - client, payload: botMessageEvent, event: botMessageEvent, message: botMessageEvent, @@ -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); }); }); @@ -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 = {}, diff --git a/src/middleware/process.ts b/src/middleware/process.ts index 75b97d013..916a61dea 100644 --- a/src/middleware/process.ts +++ b/src/middleware/process.ts @@ -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 @@ -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() @@ -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; @@ -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 diff --git a/src/types/actions/index.ts b/src/types/actions/index.ts index a89b9dee8..0af300fc8 100644 --- a/src/types/actions/index.ts +++ b/src/types/actions/index.ts @@ -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 @@ -36,8 +34,6 @@ export type SlackAction = BlockAction | InteractiveMessage | DialogSubmitAction * this case `ElementAction` must extend `BasicElementAction`. */ export interface SlackActionMiddlewareArgs { - logger: Logger; - client: WebClient; payload: ( Action extends BlockAction ? ElementAction : Action extends InteractiveMessage ? InteractiveAction : diff --git a/src/types/command/index.ts b/src/types/command/index.ts index 9c6345eda..756371e7a 100644 --- a/src/types/command/index.ts +++ b/src/types/command/index.ts @@ -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']; diff --git a/src/types/events/index.ts b/src/types/events/index.ts index 5857fff7a..cff316dbe 100644 --- a/src/types/events/index.ts +++ b/src/types/events/index.ts @@ -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 { - logger: Logger; - client: WebClient; payload: EventFromType; event: this['payload']; message: EventType extends 'message' ? this['payload'] : never; diff --git a/src/types/middleware.ts b/src/types/middleware.ts index f6f195d00..ff7954f38 100644 --- a/src/types/middleware.ts +++ b/src/types/middleware.ts @@ -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 | @@ -21,7 +23,12 @@ export interface Context extends StringIndexed { // constraint would mess up the interface of App#event(), App#message(), etc. export interface Middleware { // 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 { diff --git a/src/types/options/index.ts b/src/types/options/index.ts index 61cde6bc4..25e0503b4 100644 --- a/src/types/options/index.ts +++ b/src/types/options/index.ts @@ -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 { - logger: Logger; - client: WebClient; payload: OptionsRequest; body: this['payload']; options: this['payload']; diff --git a/src/types/view/index.ts b/src/types/view/index.ts index 26ff3ebd3..411df4d51 100644 --- a/src/types/view/index.ts +++ b/src/types/view/index.ts @@ -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 @@ -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 { - logger: Logger; - client: WebClient; payload: ViewOutput; view: this['payload']; body: ViewActionType;