Skip to content

Commit

Permalink
Add instance pool for slackapi#359
Browse files Browse the repository at this point in the history
  • Loading branch information
seratch committed Jan 30, 2020
1 parent be102b2 commit 914dce1
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 23 deletions.
66 changes: 47 additions & 19 deletions src/App.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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]);
});
});

Expand Down Expand Up @@ -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 }),
};
Expand Down
41 changes: 37 additions & 4 deletions src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<SayFn>[0]) => {
const postMessageArguments: ChatPostMessageArguments = (typeof message === 'string') ?
{ token, text: message, channel: channelId } : { ...message, token, channel: channelId };
Expand All @@ -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<AnyMiddlewareArgs> = {
Expand All @@ -420,7 +449,7 @@ export default class App {
ack?: AckFn<any>,
} = {
logger: this.logger,
client: this.client,
client: listenerArgClient,
body: bodyArg,
payload:
(type === IncomingEventType.Event) ?
Expand Down Expand Up @@ -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 });

Expand Down

0 comments on commit 914dce1

Please sign in to comment.