Skip to content

Commit 3c45bb7

Browse files
parfeonMaxPresman
parfeon
authored and
MaxPresman
committed
Changedagentkeepalive initialization (pubnub#113)
* . changed `agentkeepalive` initialization code to create only one instance for requests over TLS and insecure connections (CE-3104) * . codacy warning fix
1 parent eadb15a commit 3c45bb7

3 files changed

Lines changed: 111 additions & 34 deletions

File tree

‎src/networking/modules/node.js‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,25 @@ import superagent from 'superagent';
44
import superagentProxy from 'superagent-proxy';
55
import AgentKeepAlive from 'agentkeepalive';
66

7+
let keepAliveAgent: (AgentKeepAlive | AgentKeepAlive.HttpsAgent) = null;
8+
let keepAliveSecureAgent: (AgentKeepAlive | AgentKeepAlive.HttpsAgent) = null;
9+
710
superagentProxy(superagent);
811

912
export function proxy(superagentConstruct: superagent) {
1013
return superagentConstruct.proxy(this._config.proxy);
1114
}
1215

1316
export function keepAlive(superagentConstruct: superagent) {
14-
let AgentClass = null;
15-
let agent = null;
16-
17-
if (this._config.secure) {
18-
AgentClass = AgentKeepAlive.HttpsAgent;
19-
} else {
20-
AgentClass = AgentKeepAlive;
21-
}
22-
23-
if (this._config.keepAliveSettings) {
17+
let agent = this._config.secure ? keepAliveSecureAgent : keepAliveAgent;
18+
if (agent === null) {
19+
let AgentClass = this._config.secure ? AgentKeepAlive.HttpsAgent : AgentKeepAlive;
2420
agent = new AgentClass(this._config.keepAliveSettings);
25-
} else {
26-
agent = new AgentClass();
21+
if (this._config.secure) {
22+
keepAliveSecureAgent = agent;
23+
} else {
24+
keepAliveAgent = agent;
25+
}
2726
}
2827

2928
return superagentConstruct.agent(agent);

‎test/integration/components/networking.test.js‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,33 +27,33 @@ describe('#components/networking', () => {
2727
pubnubSDKName = new PubNub({ subscribeKey: 'mySubKey', publishKey: 'myPublishKey', uuid: 'myUUID', sdkName: 'custom-sdk/1.0.0' });
2828
});
2929

30-
describe('supports user-agent generation with partner', () => {
31-
it('returns a correct user-agent object', (done) => {
32-
utils.createNock().get('/time/0')
33-
.query({ uuid: 'myUUID', pnsdk: `PubNub-JS-Nodejs-alligator/${packageJSON.version}` })
34-
.reply(200, [14570763868573725]);
30+
describe('supports user-agent generation with partner', () => {
31+
it('returns a correct user-agent object', (done) => {
32+
utils.createNock().get('/time/0')
33+
.query({ uuid: 'myUUID', pnsdk: `PubNub-JS-Nodejs-alligator/${packageJSON.version}` })
34+
.reply(200, [14570763868573725]);
3535

36-
pubnubPartner.time((status) => {
37-
assert.equal(status.error, false);
38-
assert.equal(status.statusCode, 200);
39-
done();
40-
});
41-
});
36+
pubnubPartner.time((status) => {
37+
assert.equal(status.error, false);
38+
assert.equal(status.statusCode, 200);
39+
done();
40+
});
4241
});
42+
});
4343

44-
describe('supports PNSDK generation with custom SDK name', () => {
45-
it('returns a correct response object', (done) => {
46-
utils.createNock().get('/time/0')
47-
.query({ uuid: 'myUUID', pnsdk: 'custom-sdk/1.0.0' })
48-
.reply(200, [14570763868573725]);
44+
describe('supports PNSDK generation with custom SDK name', () => {
45+
it('returns a correct response object', (done) => {
46+
utils.createNock().get('/time/0')
47+
.query({ uuid: 'myUUID', pnsdk: 'custom-sdk/1.0.0' })
48+
.reply(200, [14570763868573725]);
4949

50-
pubnubSDKName.time((status) => {
51-
assert.equal(status.error, false);
52-
assert.equal(status.statusCode, 200);
53-
done();
54-
});
55-
});
50+
pubnubSDKName.time((status) => {
51+
assert.equal(status.error, false);
52+
assert.equal(status.statusCode, 200);
53+
done();
54+
});
5655
});
56+
});
5757

5858
describe('callback handling', () => {
5959
it('returns a correct status object', (done) => {

‎test/unit/networking.test.js‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/* global describe, it */
2+
import Networking from '../../src/networking';
3+
import superagent from 'superagent';
4+
import sinon from 'sinon';
5+
import nock from 'nock';
6+
import assert from 'assert';
7+
import { del, get, post } from '../../src/networking/modules/web-node';
8+
import { keepAlive, proxy } from '../../src/networking/modules/node';
9+
10+
describe('keep-alive agent', () => {
11+
12+
before(() => nock.disableNetConnect());
13+
after(() => nock.enableNetConnect());
14+
15+
const setupNetwork = (shouldSecure, enableKeepAlive) => {
16+
const config = { origin: 'ps.pndsn.com', secure: shouldSecure, keepAlive: enableKeepAlive, logVerbosity: false };
17+
const networking = new Networking({ keepAlive, del, get, post, proxy });
18+
networking.init(config);
19+
20+
return networking;
21+
}
22+
23+
it('should not create if \'keepAlive\' is \'false\'', () => {
24+
const networking = setupNetwork(false, false);
25+
const superagentGetSpy = sinon.spy(superagent, 'get');
26+
27+
networking.GET({}, { url: '/time/0' }, () => {});
28+
assert(superagentGetSpy.called, 'superagent not called with get');
29+
assert(superagentGetSpy.returnValues[0], 'request instance not created');
30+
assert(!superagentGetSpy.returnValues[0]._agent, 'keep-alive agent should not be created');
31+
32+
superagentGetSpy.restore();
33+
})
34+
35+
it('should create agent for insecure connection', () => {
36+
const networking = setupNetwork(false, true);
37+
const superagentGetSpy = sinon.spy(superagent, 'get');
38+
39+
networking.GET({}, { url: '/time/0' }, () => {});
40+
assert(superagentGetSpy.returnValues[0]._agent, 'keep-alive agent not created');
41+
assert(superagentGetSpy.returnValues[0]._agent.defaultPort !== 443, 'keep-alive agent should access TLS (80) port');
42+
43+
superagentGetSpy.restore();
44+
})
45+
46+
it('should re-use created agent for insecure connection', () => {
47+
const networking = setupNetwork(false, true);
48+
const superagentGetSpy = sinon.spy(superagent, 'get');
49+
50+
networking.GET({}, { url: '/time/0' }, () => {});
51+
networking.GET({}, { url: '/time/0' }, () => {});
52+
assert(superagentGetSpy.returnValues[0]._agent === superagentGetSpy.returnValues[1]._agent, 'same user-agent should be used');
53+
54+
superagentGetSpy.restore();
55+
})
56+
57+
it('should create agent for secure connection', () => {
58+
const networking = setupNetwork(true, true);
59+
const superagentGetSpy = sinon.spy(superagent, 'get');
60+
61+
networking.GET({}, { url: '/time/0' }, () => {});
62+
assert(superagentGetSpy.returnValues[0]._agent, 'keep-alive agent not created');
63+
assert(superagentGetSpy.returnValues[0]._agent.defaultPort === 443, 'keep-alive agent should access SSL (443) port');
64+
65+
superagentGetSpy.restore();
66+
})
67+
68+
it('should re-use created agent for secure connection', () => {
69+
const networking = setupNetwork(true, true);
70+
const superagentGetSpy = sinon.spy(superagent, 'get');
71+
72+
networking.GET({}, { url: '/time/0' }, () => {});
73+
networking.GET({}, { url: '/time/0' }, () => {});
74+
assert(superagentGetSpy.returnValues[0]._agent === superagentGetSpy.returnValues[1]._agent, 'same user-agent should be used');
75+
76+
superagentGetSpy.restore();
77+
})
78+
});

0 commit comments

Comments
 (0)