From d6c9468a1eaa5e00421bdfe966a6b42431f02430 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Tue, 4 Feb 2020 00:13:45 +0900 Subject: [PATCH 1/5] Make invite_type in InviteRequestedEvent more specific --- src/types/events/base-events.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/types/events/base-events.ts b/src/types/events/base-events.ts index a60009495..5ecb9cfd9 100644 --- a/src/types/events/base-events.ts +++ b/src/types/events/base-events.ts @@ -417,6 +417,26 @@ export interface IMOpenEvent extends StringIndexed { channel: string; } +export interface InviteRequestedEvent extends StringIndexed { + type: 'invite_requested'; + invite_request: { + id: string; + email: string; + date_created: number; + requester_ids: string[]; + channel_ids: string[]; + invite_type: 'restricted' | 'ultra_restricted' | 'full_member'; + real_name: string; + date_expire: number; + request_reason: string; + team: { + id: string; + name: string; + domain: string; + } + }; +} + export interface LinkSharedEvent extends StringIndexed { type: 'link_shared'; channel: string; From 6d450c1b54a4ee36df5fed0ff32e8820fc411e41 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Tue, 24 Dec 2019 15:28:00 +0900 Subject: [PATCH 2/5] Fix #168 Give listeners and middleware access to the logger --- src/App.spec.ts | 50 ++++++++++++++++++++++++++++++++++ src/App.ts | 3 +- src/middleware/builtin.spec.ts | 18 +++++++++++- src/types/actions/index.ts | 2 ++ src/types/command/index.ts | 2 ++ src/types/events/index.ts | 2 ++ src/types/options/index.ts | 2 ++ src/types/view/index.ts | 2 ++ 8 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/App.spec.ts b/src/App.spec.ts index cee1049f5..2939d3080 100644 --- a/src/App.spec.ts +++ b/src/App.spec.ts @@ -612,6 +612,56 @@ describe('App', () => { }); }); + describe('logger', () => { + + it('should be available in middleware/listener args', async () => { + // Arrange + const App = await importApp(overrides); // tslint:disable-line:variable-name + const fakeLogger = createFakeLogger(); + const app = new App({ + logger: fakeLogger, + receiver: fakeReceiver, + authorize: sinon.fake.resolves(dummyAuthorizationResult), + }); + app.use(({ logger, body }) => { + logger.info(body); + }); + + app.event('app_home_opened', ({ logger, event }) => { + logger.debug(event); + }); + + const receiverEvents = [ + { + body: { + type: 'event_callback', + token: 'XXYYZZ', + team_id: 'TXXXXXXXX', + api_app_id: 'AXXXXXXXXX', + event: { + type: 'app_home_opened', + event_ts: '1234567890.123456', + user: 'UXXXXXXX1', + text: 'hello friends!', + tab: 'home', + view: {}, + }, + }, + respond: noop, + ack: noop, + }, + ]; + + // Act + receiverEvents.forEach(event => fakeReceiver.emit('message', event)); + await delay(); + + // Assert + assert.isTrue(fakeLogger.info.called); + assert.isTrue(fakeLogger.debug.called); + }); + }); + describe('say()', () => { function createChannelContextualReceiverEvents(channelId: string): ReceiverEvent[] { diff --git a/src/App.ts b/src/App.ts index 63aa79595..1dd4711cb 100644 --- a/src/App.ts +++ b/src/App.ts @@ -430,7 +430,7 @@ 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 */ @@ -438,6 +438,7 @@ export default class App { /** Ack function might be set below */ ack?: AckFn, } = { + logger: this.logger, body: bodyArg, payload: (type === IncomingEventType.Event) ? diff --git a/src/middleware/builtin.spec.ts b/src/middleware/builtin.spec.ts index a8a40ca9d..be473d47c 100644 --- a/src/middleware/builtin.spec.ts +++ b/src/middleware/builtin.spec.ts @@ -3,7 +3,7 @@ import 'mocha'; import { assert } from 'chai'; import sinon from 'sinon'; import { ErrorCode } from '../errors'; -import { Override, delay, wrapToResolveOnFirstCall } from '../test-helpers'; +import { Override, delay, wrapToResolveOnFirstCall, createFakeLogger } from '../test-helpers'; import rewiremock from 'rewiremock'; import { SlackEventMiddlewareArgs, @@ -433,11 +433,13 @@ describe('ignoreSelf()', () => { }); describe('onlyCommands', () => { + const logger = createFakeLogger(); it('should detect valid requests', async () => { const payload: SlashCommand = { ...validCommandPayload }; const fakeNext = sinon.fake(); onlyCommands({ + logger, payload, command: payload, body: payload, @@ -454,6 +456,7 @@ describe('onlyCommands', () => { const payload: any = {}; const fakeNext = sinon.fake(); onlyCommands({ + logger, payload, action: payload, command: undefined, @@ -469,9 +472,12 @@ describe('onlyCommands', () => { }); describe('matchCommandName', () => { + const logger = createFakeLogger(); + function buildArgs(fakeNext: NextMiddleware): SlackCommandMiddlewareArgs & { next: any, context: any } { const payload: SlashCommand = { ...validCommandPayload }; return { + logger, payload, command: payload, body: payload, @@ -498,9 +504,12 @@ describe('matchCommandName', () => { describe('onlyEvents', () => { + const logger = createFakeLogger(); + it('should detect valid requests', async () => { const fakeNext = sinon.fake(); const args: SlackEventMiddlewareArgs<'app_mention'> & { event?: SlackEvent } = { + logger, payload: appMentionEvent, event: appMentionEvent, message: null as never, // a bit hackey to sartisfy TS compiler @@ -524,6 +533,7 @@ describe('onlyEvents', () => { const payload: SlashCommand = { ...validCommandPayload }; const fakeNext = sinon.fake(); onlyEvents({ + logger, payload, command: payload, body: payload, @@ -538,8 +548,11 @@ describe('onlyEvents', () => { }); describe('matchEventType', () => { + const logger = createFakeLogger(); + function buildArgs(): SlackEventMiddlewareArgs<'app_mention'> & { event?: SlackEvent } { return { + logger, payload: appMentionEvent, event: appMentionEvent, message: null as never, // a bit hackey to sartisfy TS compiler @@ -571,8 +584,11 @@ describe('matchEventType', () => { }); describe('subtype', () => { + const logger = createFakeLogger(); + function buildArgs(): SlackEventMiddlewareArgs<'message'> & { event?: SlackEvent } { return { + logger, payload: botMessageEvent, event: botMessageEvent, message: botMessageEvent, diff --git a/src/types/actions/index.ts b/src/types/actions/index.ts index 0af300fc8..8e4016399 100644 --- a/src/types/actions/index.ts +++ b/src/types/actions/index.ts @@ -8,6 +8,7 @@ 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'; /** * All known actions from Slack's Block Kit interactive components, message actions, dialogs, and legacy interactive @@ -34,6 +35,7 @@ export type SlackAction = BlockAction | InteractiveMessage | DialogSubmitAction * this case `ElementAction` must extend `BasicElementAction`. */ export interface SlackActionMiddlewareArgs { + logger: Logger; payload: ( Action extends BlockAction ? ElementAction : Action extends InteractiveMessage ? InteractiveAction : diff --git a/src/types/command/index.ts b/src/types/command/index.ts index 756371e7a..c06a0dfc4 100644 --- a/src/types/command/index.ts +++ b/src/types/command/index.ts @@ -1,10 +1,12 @@ import { StringIndexed } from '../helpers'; import { SayFn, RespondFn, RespondArguments, AckFn } from '../utilities'; +import { Logger } from '@slack/logger'; /** * Arguments which listeners and middleware receive to process a slash command from Slack. */ export interface SlackCommandMiddlewareArgs { + logger: Logger; payload: SlashCommand; command: this['payload']; body: this['payload']; diff --git a/src/types/events/index.ts b/src/types/events/index.ts index cff316dbe..381f64d8f 100644 --- a/src/types/events/index.ts +++ b/src/types/events/index.ts @@ -2,11 +2,13 @@ export * from './base-events'; import { SlackEvent, BasicSlackEvent } from './base-events'; import { StringIndexed } from '../helpers'; import { SayFn } from '../utilities'; +import { Logger } from '@slack/logger'; /** * Arguments which listeners and middleware receive to process an event from Slack's Events API. */ export interface SlackEventMiddlewareArgs { + logger: Logger; payload: EventFromType; event: this['payload']; message: EventType extends 'message' ? this['payload'] : never; diff --git a/src/types/options/index.ts b/src/types/options/index.ts index 25e0503b4..0cf36bacd 100644 --- a/src/types/options/index.ts +++ b/src/types/options/index.ts @@ -1,11 +1,13 @@ import { Option } from '@slack/types'; import { StringIndexed, XOR } from '../helpers'; import { AckFn } from '../utilities'; +import { Logger } from '@slack/logger'; /** * Arguments which listeners and middleware receive to process an options request from Slack */ export interface SlackOptionsMiddlewareArgs { + logger: Logger; payload: OptionsRequest; body: this['payload']; options: this['payload']; diff --git a/src/types/view/index.ts b/src/types/view/index.ts index 411df4d51..49b5ee08c 100644 --- a/src/types/view/index.ts +++ b/src/types/view/index.ts @@ -1,5 +1,6 @@ import { StringIndexed } from '../helpers'; import { RespondArguments, AckFn } from '../utilities'; +import { Logger } from '@slack/logger'; /** * Known view action types @@ -10,6 +11,7 @@ export type SlackViewAction = ViewSubmitAction | ViewClosedAction; * Arguments which listeners and middleware receive to process a view submission event from Slack. */ export interface SlackViewMiddlewareArgs { + logger: Logger; payload: ViewOutput; view: this['payload']; body: ViewActionType; From 7a4281aaa9fdd213ba90427d1582ebbd4474a78f Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Tue, 24 Dec 2019 15:50:44 +0900 Subject: [PATCH 3/5] Fix #354 Add `client` to the list of args send through to listeners --- src/App.spec.ts | 48 ++++++++++++++++++++++++++++++++++ src/App.ts | 3 ++- src/middleware/builtin.spec.ts | 13 +++++++++ src/types/actions/index.ts | 2 ++ src/types/command/index.ts | 2 ++ src/types/events/index.ts | 2 ++ src/types/options/index.ts | 2 ++ src/types/view/index.ts | 2 ++ 8 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/App.spec.ts b/src/App.spec.ts index 2939d3080..5dc5a5fe3 100644 --- a/src/App.spec.ts +++ b/src/App.spec.ts @@ -662,6 +662,54 @@ describe('App', () => { }); }); + describe('client', () => { + + it('should be available in middleware/listener args', async () => { + // Arrange + const App = await importApp(mergeOverrides( // tslint:disable-line:variable-name + withNoopAppMetadata(), + withSuccessfulBotUserFetchingWebClient('B123', 'U123'), + )); + const app = new App({ + receiver: fakeReceiver, + authorize: sinon.fake.resolves(dummyAuthorizationResult), + }); + app.use(async ({ client }) => { + await client.auth.test(); + }); + app.event('app_home_opened', async ({ client }) => { + await client.auth.test(); + }); + + const receiverEvents = [ + { + body: { + type: 'event_callback', + token: 'XXYYZZ', + team_id: 'TXXXXXXXX', + api_app_id: 'AXXXXXXXXX', + event: { + type: 'app_home_opened', + event_ts: '1234567890.123456', + user: 'UXXXXXXX1', + text: 'hello friends!', + tab: 'home', + view: {}, + }, + }, + respond: noop, + ack: noop, + }, + ]; + + // Act + receiverEvents.forEach(event => fakeReceiver.emit('message', event)); + await delay(); + + // Assert + }); + }); + describe('say()', () => { function createChannelContextualReceiverEvents(channelId: string): ReceiverEvent[] { diff --git a/src/App.ts b/src/App.ts index 1dd4711cb..7f8c0eae4 100644 --- a/src/App.ts +++ b/src/App.ts @@ -430,7 +430,7 @@ 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 */ @@ -439,6 +439,7 @@ export default class App { ack?: AckFn, } = { logger: this.logger, + client: this.client, body: bodyArg, payload: (type === IncomingEventType.Event) ? diff --git a/src/middleware/builtin.spec.ts b/src/middleware/builtin.spec.ts index be473d47c..1353ce8f3 100644 --- a/src/middleware/builtin.spec.ts +++ b/src/middleware/builtin.spec.ts @@ -16,6 +16,7 @@ import { import { onlyCommands, onlyEvents, matchCommandName, matchEventType, subtype } from './builtin'; import { SlashCommand } from '../types/command/index'; import { SlackEvent, AppMentionEvent, BotMessageEvent } from '../types/events/base-events'; +import { WebClient } from '@slack/web-api'; describe('matchMessage()', () => { function initializeTestCase(pattern: string | RegExp): Mocha.AsyncFunc { @@ -434,12 +435,14 @@ describe('ignoreSelf()', () => { describe('onlyCommands', () => { const logger = createFakeLogger(); + const client = new WebClient(undefined, { logger, slackApiUrl: undefined }); it('should detect valid requests', async () => { const payload: SlashCommand = { ...validCommandPayload }; const fakeNext = sinon.fake(); onlyCommands({ logger, + client, payload, command: payload, body: payload, @@ -457,6 +460,7 @@ describe('onlyCommands', () => { const fakeNext = sinon.fake(); onlyCommands({ logger, + client, payload, action: payload, command: undefined, @@ -473,11 +477,13 @@ describe('onlyCommands', () => { describe('matchCommandName', () => { const logger = createFakeLogger(); + const client = new WebClient(undefined, { logger, slackApiUrl: undefined }); function buildArgs(fakeNext: NextMiddleware): SlackCommandMiddlewareArgs & { next: any, context: any } { const payload: SlashCommand = { ...validCommandPayload }; return { logger, + client, payload, command: payload, body: payload, @@ -505,11 +511,13 @@ describe('matchCommandName', () => { describe('onlyEvents', () => { const logger = createFakeLogger(); + const client = new WebClient(undefined, { logger, slackApiUrl: undefined }); 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 @@ -534,6 +542,7 @@ describe('onlyEvents', () => { const fakeNext = sinon.fake(); onlyEvents({ logger, + client, payload, command: payload, body: payload, @@ -549,10 +558,12 @@ describe('onlyEvents', () => { describe('matchEventType', () => { const logger = createFakeLogger(); + const client = new WebClient(undefined, { logger, slackApiUrl: undefined }); 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 @@ -585,10 +596,12 @@ describe('matchEventType', () => { describe('subtype', () => { const logger = createFakeLogger(); + const client = new WebClient(undefined, { logger, slackApiUrl: undefined }); function buildArgs(): SlackEventMiddlewareArgs<'message'> & { event?: SlackEvent } { return { logger, + client, payload: botMessageEvent, event: botMessageEvent, message: botMessageEvent, diff --git a/src/types/actions/index.ts b/src/types/actions/index.ts index 8e4016399..a89b9dee8 100644 --- a/src/types/actions/index.ts +++ b/src/types/actions/index.ts @@ -9,6 +9,7 @@ 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,6 +37,7 @@ export type SlackAction = BlockAction | InteractiveMessage | DialogSubmitAction */ 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 c06a0dfc4..9c6345eda 100644 --- a/src/types/command/index.ts +++ b/src/types/command/index.ts @@ -1,12 +1,14 @@ 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 381f64d8f..5857fff7a 100644 --- a/src/types/events/index.ts +++ b/src/types/events/index.ts @@ -3,12 +3,14 @@ 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/options/index.ts b/src/types/options/index.ts index 0cf36bacd..61cde6bc4 100644 --- a/src/types/options/index.ts +++ b/src/types/options/index.ts @@ -2,12 +2,14 @@ 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 49b5ee08c..26ff3ebd3 100644 --- a/src/types/view/index.ts +++ b/src/types/view/index.ts @@ -1,6 +1,7 @@ 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,6 +13,7 @@ export type SlackViewAction = ViewSubmitAction | ViewClosedAction; */ export interface SlackViewMiddlewareArgs { logger: Logger; + client: WebClient; payload: ViewOutput; view: this['payload']; body: ViewActionType; From 8e677ef94c5ac2c895537a79ffbda5460b3e82a7 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Thu, 30 Jan 2020 17:49:08 +0900 Subject: [PATCH 4/5] Add instance pool for #359 --- src/App.spec.ts | 66 +++++++++++++++++++++++++++++++++++-------------- src/App.ts | 44 ++++++++++++++++++++++++++++----- 2 files changed, 85 insertions(+), 25 deletions(-) diff --git a/src/App.spec.ts b/src/App.spec.ts index 5dc5a5fe3..9975b4dea 100644 --- a/src/App.spec.ts +++ b/src/App.spec.ts @@ -10,6 +10,7 @@ import { Receiver, ReceiverEvent, SayFn, NextMiddleware } from './types'; import { ConversationStore } from './conversation-store'; import { LogLevel } from '@slack/logger'; import { ViewConstraints } from './App'; +import { WebClientOptions, WebClient } from '@slack/web-api'; describe('App', () => { describe('constructor', () => { @@ -670,43 +671,66 @@ describe('App', () => { withNoopAppMetadata(), withSuccessfulBotUserFetchingWebClient('B123', 'U123'), )); + const tokens = [ + 'xoxb-123', + 'xoxp-456', + 'xoxb-123', + ]; const app = new App({ receiver: fakeReceiver, - authorize: sinon.fake.resolves(dummyAuthorizationResult), + authorize: () => { + const token = tokens.pop(); + if (typeof token === 'undefined') { + return Promise.resolve({ botId: 'B123' }); + } + if (token.startsWith('xoxb-')) { + return Promise.resolve({ botToken: token, botId: 'B123' }); + } + return Promise.resolve({ userToken: token, botId: 'B123' }); + }, }); app.use(async ({ client }) => { await client.auth.test(); }); + const clients: WebClient[] = []; app.event('app_home_opened', async ({ client }) => { + clients.push(client); await client.auth.test(); }); - const receiverEvents = [ - { - body: { - type: 'event_callback', - token: 'XXYYZZ', - team_id: 'TXXXXXXXX', - api_app_id: 'AXXXXXXXXX', - event: { - type: 'app_home_opened', - event_ts: '1234567890.123456', - user: 'UXXXXXXX1', - text: 'hello friends!', - tab: 'home', - view: {}, - }, + const event = { + body: { + type: 'event_callback', + token: 'legacy', + team_id: 'T123', + api_app_id: 'A123', + event: { + type: 'app_home_opened', + event_ts: '123.123', + user: 'U123', + text: 'Hi there!', + tab: 'home', + view: {}, }, - respond: noop, - ack: noop, }, - ]; + respond: noop, + ack: noop, + }; + const receiverEvents = [event, event, event]; // Act receiverEvents.forEach(event => fakeReceiver.emit('message', event)); await delay(); // Assert + assert.isUndefined(app.client.token); + + assert.equal(clients[0].token, 'xoxb-123'); + assert.equal(clients[1].token, 'xoxp-456'); + assert.equal(clients[2].token, 'xoxb-123'); + + assert.notEqual(clients[0], clients[1]); + assert.strictEqual(clients[0], clients[2]); }); }); @@ -947,6 +971,10 @@ function withSuccessfulBotUserFetchingWebClient(botId: string, botUserId: string return { '@slack/web-api': { WebClient: class { + public token?: string; + constructor(token?: string, _options?: WebClientOptions) { + this.token = token; + } public auth = { test: sinon.fake.resolves({ user_id: botUserId }), }; diff --git a/src/App.ts b/src/App.ts index 7f8c0eae4..6dffce385 100644 --- a/src/App.ts +++ b/src/App.ts @@ -4,7 +4,6 @@ import { ChatPostMessageArguments, addAppMetadata, WebClientOptions, - WebAPICallResult, } from '@slack/web-api'; import { Logger, LogLevel, ConsoleLogger } from '@slack/logger'; import axios, { AxiosInstance } from 'axios'; @@ -110,6 +109,19 @@ export interface ErrorHandler { (error: CodedError): void; } +class WebClientPool { + private pool: { [token: string]: WebClient } = {}; + public getOrCreate(token: string, clientOptions: WebClientOptions): WebClient { + const cachedClient = this.pool[token]; + if (typeof cachedClient !== 'undefined') { + return cachedClient; + } + const client = new WebClient(token, clientOptions); + this.pool[token] = client; + return client; + } +} + /** * A Slack App */ @@ -118,6 +130,10 @@ export default class App { /** Slack Web API client */ public client: WebClient; + private clientOptions: WebClientOptions; + + private clients: {[teamId: string]: WebClientPool} = {}; + /** Receiver - ingests events from the Slack platform */ private receiver: Receiver; @@ -158,13 +174,15 @@ export default class App { this.logger.setLevel(logLevel); this.errorHandler = defaultErrorHandler(this.logger); - this.client = new WebClient(undefined, { + this.clientOptions = { agent, logLevel, logger, tls: clientTls, slackApiUrl: clientOptions !== undefined ? clientOptions.slackApiUrl : undefined, - }); + }; + // the public WebClient instance (app.client) - this one doesn't have a token + this.client = new WebClient(undefined, this.clientOptions); this.axios = axios.create(Object.assign( { @@ -418,8 +436,8 @@ export default class App { // Factory for say() utility const createSay = (channelId: string): SayFn => { - const token = context.botToken !== undefined ? context.botToken : context.userToken; - return (message: Parameters[0]): Promise => { + const token = selectToken(context); + return (message: Parameters[0]) => { const postMessageArguments: ChatPostMessageArguments = (typeof message === 'string') ? { token, text: message, channel: channelId } : { ...message, token, channel: channelId }; @@ -427,6 +445,16 @@ export default class App { }; }; + let listenerArgClient = this.client; + const token = selectToken(context); + if (typeof token !== 'undefined') { + let pool = this.clients[source.teamId]; + if (typeof pool === 'undefined') { + pool = this.clients[source.teamId] = new WebClientPool(); + } + listenerArgClient = pool.getOrCreate(token, this.clientOptions); + } + // Set body and payload (this value will eventually conform to AnyMiddlewareArgs) // NOTE: the following doesn't work because... distributive? // const listenerArgs: Partial = { @@ -439,7 +467,7 @@ export default class App { ack?: AckFn, } = { logger: this.logger, - client: this.client, + client: listenerArgClient, body: bodyArg, payload: (type === IncomingEventType.Event) ? @@ -615,6 +643,10 @@ function singleTeamAuthorization( }; } +function selectToken(context: Context): string | undefined { + return context.botToken !== undefined ? context.botToken : context.userToken; +} + /* Instrumentation */ addAppMetadata({ name: packageJson.name, version: packageJson.version }); From 290149295750c89551574480b09aa23e990eff1d Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Fri, 31 Jan 2020 18:11:10 +0900 Subject: [PATCH 5/5] 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;