Compose middleware resolve conflicts with #393

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 App, { ViewConstraints } from './App';
+import { WebClientOptions, WebClient } from '@slack/web-api';
 
 describe('App', () => {
   describe('constructor', () => {
@@ -644,6 +645,129 @@ 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(async ({ logger, body, next }) => {
+            logger.info(body);
+            await next();
+          });
+
+          app.event('app_home_opened', async ({ 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('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 tokens = [
+            'xoxb-123',
+            'xoxp-456',
+            'xoxb-123',
+          ];
+          const app = new App({
+            receiver: fakeReceiver,
+            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, next }) => {
+            await client.auth.test();
+            await next();
+          });
+          const clients: WebClient[] = [];
+          app.event('app_home_opened', async ({ client }) => {
+            clients.push(client);
+            await client.auth.test();
+          });
+
+          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,
+          };
+          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]);
+        });
+      });
+
       describe('say()', () => {
 
         function createChannelContextualReceiverEvents(channelId: string): ReceiverEvent[] {
@@ -881,6 +1005,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 }),
         };

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';
@@ -46,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 */
@@ -110,6 +110,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 +131,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 +175,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(
       {
@@ -284,9 +303,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,
@@ -299,7 +318,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;
 
@@ -418,15 +437,25 @@ 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<SayFn>[0]): Promise<WebAPICallResult> => {
+      const token = selectToken(context);
+      return (message: Parameters<SayFn>[0]) => {
         const postMessageArguments: ChatPostMessageArguments = (typeof message === 'string') ?
           { token, text: message, channel: channelId } : { ...message, token, channel: channelId };
 
         return this.client.chat.postMessage(postMessageArguments);
       };
     };
 
+    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<AnyMiddlewareArgs> = {
@@ -437,7 +466,13 @@ export default class App {
       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,
       body: bodyArg,
       payload:
         (type === IncomingEventType.Event) ?
@@ -515,7 +550,7 @@ export default class App {
     try {
       await processMiddleware(middlewareChain, {
         context,
-        ...(listenerArgs as AnyMiddlewareArgs),
+        ...(listenerArgs as MiddlewareContext<AnyMiddlewareArgs>),
       });
     } catch (error) {
       this.onGlobalError(error);
@@ -607,6 +642,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 });
 

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<ConversationState> {
   conversation?: ConversationState;