From 914dce13338b1a05fb6e618644e4a0d195cb81eb Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Thu, 30 Jan 2020 17:49:08 +0900 Subject: [PATCH] Add instance pool for #359 --- src/App.spec.ts | 66 +++++++++++++++++++++++++++++++++++-------------- src/App.ts | 41 +++++++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 23 deletions(-) diff --git a/src/App.spec.ts b/src/App.spec.ts index 23c3c0ad6..6c9dcf06d 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', () => { @@ -609,43 +610,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]); }); }); @@ -918,6 +942,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 12346be81..f5c06b1e7 100644 --- a/src/App.ts +++ b/src/App.ts @@ -102,6 +102,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 */ @@ -110,6 +123,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; @@ -148,13 +165,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); if (token !== undefined) { if (authorize !== undefined) { @@ -399,7 +418,7 @@ export default class App { // Factory for say() utility const createSay = (channelId: string): SayFn => { - const token = context.botToken !== undefined ? context.botToken : context.userToken; + const token = selectToken(context); return (message: Parameters[0]) => { const postMessageArguments: ChatPostMessageArguments = (typeof message === 'string') ? { token, text: message, channel: channelId } : { ...message, token, channel: channelId }; @@ -408,6 +427,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 = { @@ -420,7 +449,7 @@ export default class App { ack?: AckFn, } = { logger: this.logger, - client: this.client, + client: listenerArgClient, body: bodyArg, payload: (type === IncomingEventType.Event) ? @@ -591,6 +620,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 });