[FABN-1194] Use connection options for discovery peer

Make Client.buildConnectionOptions public and use Clients's\default connection options for creating discovery peer Change-Id: I497d746cca72b6e7b7064ecc30e8fabf46cfdfae Signed-off-by: LeoLe <[email protected]>
hyperledger · Apr 2, 2019 · 53359f9 · 53359f9
1 parent 1dc025c
commit 53359f9
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 72 deletions.
diff --git a/fabric-client/lib/Channel.js b/fabric-client/lib/Channel.js
@@ -1508,11 +1508,12 @@ const Channel = class {
 		const method = '_buildOptions';
 		logger.debug('%s - start', method);
 		const caroots = this._buildTlsRootCerts(msp);
-		const opts = {
+		let opts = {
 			'pem': caroots,
 			'ssl-target-name-override': host,
 			'name': name
 		};
+		opts = this._clientContext.buildConnectionOptions(opts);
 		this._clientContext.addTlsClientCertAndKey(opts);
 
 		return opts;

diff --git a/fabric-client/lib/Client.js b/fabric-client/lib/Client.js
@@ -187,10 +187,10 @@ const Client = class extends BaseClient {
 		}
 	}
 
-	/*
+	/**
 	 * Utility method to merge connection options into a set of options and
 	 * return a new options object.
-	 * The client's options will not override any existing settings by
+	 * The client's options and default connection options will not override any passed settings by
 	 * the same name, these will only be added as new settings to the
 	 * application's options being passed in. see {@link Client#addConnectionOptions}
 	 * for how this client will have connection options to merge.
@@ -200,7 +200,7 @@ const Client = class extends BaseClient {
 	 * @returns {object} - The object holding both the application's options
 	 *          and this client's options.
 	 */
-	_buildConnectionOptions(options) {
+	buildConnectionOptions(options) {
 		const method = 'getConnectionOptions';
 		logger.debug('%s - start', method);
 		let return_options = Object.assign({}, Client.getConfigSetting('connection-options'));
@@ -256,7 +256,7 @@ const Client = class extends BaseClient {
 	 * methods are called.
 	 * Options will be automatically added when loading a common connection profile
 	 * and the client section has the 'connection' section with an 'options' attribute.
-	 * These options will be initially loaded from the system configuration
+	 * Default connection options will be initially loaded from the system configuration
 	 * 'connection-options' setting.
 	 *
 	 * @param {object} options - The connection options that will be added to
@@ -431,7 +431,7 @@ const Client = class extends BaseClient {
 	 * @returns {Peer} The Peer instance.
 	 */
 	newPeer(url, opts) {
-		return new Peer(url, this._buildConnectionOptions(opts));
+		return new Peer(url, this.buildConnectionOptions(opts));
 	}
 
 	/**
@@ -492,7 +492,7 @@ const Client = class extends BaseClient {
 	 * @returns {Orderer} The Orderer instance.
 	 */
 	newOrderer(url, opts) {
-		return new Orderer(url, this._buildConnectionOptions(opts));
+		return new Orderer(url, this.buildConnectionOptions(opts));
 	}
 
 	/**

diff --git a/fabric-client/test/Channel.js b/fabric-client/test/Channel.js
@@ -1631,6 +1631,21 @@ describe('Channel', () => {
 	describe('#_processPeers', () => {});
 
 	describe('#_buildOrdererName', () => {
+
+		const ordererDefaultConnectionOptions = {
+			'grpc.default_authority': 'orderer.example.com',
+			'grpc.max_receive_message_length': -1,
+			'grpc.max_send_message_length': -1,
+			'grpc.ssl_target_name_override': 'orderer.example.com',
+			name: 'orderer.example.com:7050',
+
+			'grpc.http2.max_pings_without_data': 0,
+			'grpc.http2.min_time_between_pings_ms': 120000,
+			'grpc.keepalive_permit_without_calls': 1,
+			'grpc.keepalive_time_ms': 120000,
+			'grpc.keepalive_timeout_ms': 20000
+		};
+
 		it('should match an existing orderer', () => {
 			const orderer = sinon.createStubInstance(Orderer);
 			orderer.getUrl.returns('grpcs://peer:7051');
@@ -1670,13 +1685,7 @@ describe('Channel', () => {
 			name.should.equal('orderer.example.com:7050');
 			const orderer = channel.getOrderer('orderer.example.com:7050');
 			orderer.getUrl().should.equal('grpc://orderer.example.com:7050');
-			orderer._options.should.deep.equal({
-				'grpc.default_authority': 'orderer.example.com',
-				'grpc.max_receive_message_length': -1,
-				'grpc.max_send_message_length': -1,
-				'grpc.ssl_target_name_override': 'orderer.example.com',
-				name: 'orderer.example.com:7050'
-			});
+			orderer._options.should.deep.equal(ordererDefaultConnectionOptions);
 		});
 
 		it('should add a new orderer if all information is available (non-TLS, localhost)', () => {
@@ -1691,13 +1700,7 @@ describe('Channel', () => {
 			name.should.equal('orderer.example.com:7050');
 			const orderer = channel.getOrderer('orderer.example.com:7050');
 			orderer.getUrl().should.equal('grpc://localhost:7050');
-			orderer._options.should.deep.equal({
-				'grpc.default_authority': 'orderer.example.com',
-				'grpc.max_receive_message_length': -1,
-				'grpc.max_send_message_length': -1,
-				'grpc.ssl_target_name_override': 'orderer.example.com',
-				name: 'orderer.example.com:7050'
-			});
+			orderer._options.should.deep.equal(ordererDefaultConnectionOptions);
 		});
 
 		it('should add a new orderer if all information is available (TLS)', () => {
@@ -1712,13 +1715,7 @@ describe('Channel', () => {
 			name.should.equal('orderer.example.com:7050');
 			const orderer = channel.getOrderer('orderer.example.com:7050');
 			orderer.getUrl().should.equal('grpcs://orderer.example.com:7050');
-			orderer._options.should.deep.equal({
-				'grpc.default_authority': 'orderer.example.com',
-				'grpc.max_receive_message_length': -1,
-				'grpc.max_send_message_length': -1,
-				'grpc.ssl_target_name_override': 'orderer.example.com',
-				name: 'orderer.example.com:7050'
-			});
+			orderer._options.should.deep.equal(ordererDefaultConnectionOptions);
 		});
 
 		it('should add a new orderer if all information is available (TLS, localhost)', () => {
@@ -1733,13 +1730,7 @@ describe('Channel', () => {
 			name.should.equal('orderer.example.com:7050');
 			const orderer = channel.getOrderer('orderer.example.com:7050');
 			orderer.getUrl().should.equal('grpcs://localhost:7050');
-			orderer._options.should.deep.equal({
-				'grpc.default_authority': 'orderer.example.com',
-				'grpc.max_receive_message_length': -1,
-				'grpc.max_send_message_length': -1,
-				'grpc.ssl_target_name_override': 'orderer.example.com',
-				name: 'orderer.example.com:7050'
-			});
+			orderer._options.should.deep.equal(ordererDefaultConnectionOptions);
 		});
 
 		it('should add a new orderer if all information is available (forced TLS)', () => {
@@ -1788,6 +1779,24 @@ describe('Channel', () => {
 	});
 
 	describe('#_buildPeerName', () => {
+
+		const peerDefaultConnectionOptions = { // according to fabric-client/config/default.json
+			'grpc.default_authority': 'peer0.org1.example.com',
+			'grpc.max_receive_message_length': -1,
+			'grpc.max_send_message_length': -1,
+			'grpc.ssl_target_name_override': 'peer0.org1.example.com',
+			name: 'peer0.org1.example.com:7051',
+
+			'grpc.http2.max_pings_without_data': 0,
+			'grpc.http2.min_time_between_pings_ms': 120000,
+			'grpc.keepalive_permit_without_calls': 1,
+			'grpc.keepalive_time_ms': 120000,
+			'grpc.keepalive_timeout_ms': 20000
+		};
+
+		const peerRemoteConnectionOptions = Object.assign({}, peerDefaultConnectionOptions,
+			{'grpc.default_authority': 'peer0.org1.example.com'});
+
 		it('should match an existing peer', () => {
 			const peer = sinon.createStubInstance(Peer);
 			sinon.stub(channel, '_buildUrl').returns('grpcs://peer:7051');
@@ -1822,13 +1831,7 @@ describe('Channel', () => {
 			const channelPeer = channel.getPeer('peer0.org1.example.com:7051');
 			const peer = channelPeer.getPeer();
 			peer.getUrl().should.equal('grpc://peer0.org1.example.com:7051');
-			peer._options.should.deep.equal({
-				'grpc.default_authority': 'peer0.org1.example.com',
-				'grpc.max_receive_message_length': -1,
-				'grpc.max_send_message_length': -1,
-				'grpc.ssl_target_name_override': 'peer0.org1.example.com',
-				name: 'peer0.org1.example.com:7051'
-			});
+			peer._options.should.deep.equal(peerRemoteConnectionOptions);
 		});
 
 		it('should add a new peer if all information is available (non-TLS, localhost)', () => {
@@ -1844,13 +1847,7 @@ describe('Channel', () => {
 			const channelPeer = channel.getPeer('peer0.org1.example.com:7051');
 			const peer = channelPeer.getPeer();
 			peer.getUrl().should.equal('grpc://localhost:7051');
-			peer._options.should.deep.equal({
-				'grpc.default_authority': 'peer0.org1.example.com',
-				'grpc.max_receive_message_length': -1,
-				'grpc.max_send_message_length': -1,
-				'grpc.ssl_target_name_override': 'peer0.org1.example.com',
-				name: 'peer0.org1.example.com:7051'
-			});
+			peer._options.should.deep.equal(peerRemoteConnectionOptions);
 		});
 
 		it('should add a new peer if all information is available (TLS)', () => {
@@ -1866,13 +1863,7 @@ describe('Channel', () => {
 			const channelPeer = channel.getPeer('peer0.org1.example.com:7051');
 			const peer = channelPeer.getPeer();
 			peer.getUrl().should.equal('grpcs://peer0.org1.example.com:7051');
-			peer._options.should.deep.equal({
-				'grpc.default_authority': 'peer0.org1.example.com',
-				'grpc.max_receive_message_length': -1,
-				'grpc.max_send_message_length': -1,
-				'grpc.ssl_target_name_override': 'peer0.org1.example.com',
-				name: 'peer0.org1.example.com:7051'
-			});
+			peer._options.should.deep.equal(Object.assign({}, peerRemoteConnectionOptions));
 		});
 
 		it('should add a new peer if all information is available (TLS, localhost)', () => {
@@ -1888,13 +1879,7 @@ describe('Channel', () => {
 			const channelPeer = channel.getPeer('peer0.org1.example.com:7051');
 			const peer = channelPeer.getPeer();
 			peer.getUrl().should.equal('grpcs://localhost:7051');
-			peer._options.should.deep.equal({
-				'grpc.default_authority': 'peer0.org1.example.com',
-				'grpc.max_receive_message_length': -1,
-				'grpc.max_send_message_length': -1,
-				'grpc.ssl_target_name_override': 'peer0.org1.example.com',
-				name: 'peer0.org1.example.com:7051'
-			});
+			peer._options.should.deep.equal(Object.assign({}, peerRemoteConnectionOptions));
 		});
 
 		it('should add a new peer if all information is available (forced TLS)', () => {
@@ -1947,6 +1932,7 @@ describe('Channel', () => {
 	describe('#_buildOptions', () => {
 		it('should return the build options', () => {
 			client = sinon.createStubInstance(Client);
+			client.buildConnectionOptions.callsFake(opts => opts);
 			channel._clientContext = client;
 			const msp = {tls_root_certs: 'ROOT_CERT'};
 			const pem = 'ROOT_CERT';

diff --git a/fabric-client/test/Client.js b/fabric-client/test/Client.js
@@ -432,7 +432,7 @@ describe('Client', () => {
 			const peerStub = sinon.stub();
 			revert.push(Client.__set__('Peer', peerStub));
 			const client = new Client();
-			client._buildConnectionOptions = (value) => value;
+			client.buildConnectionOptions = (value) => value;
 			const peer = client.newPeer('url', 'opts');
 			sinon.assert.calledWith(peerStub, 'url', 'opts');
 			peer.should.deep.equal(new peerStub());
@@ -517,7 +517,7 @@ describe('Client', () => {
 			const ordererStub = sinon.stub();
 			revert.push(Client.__set__('Orderer', ordererStub));
 			const client = new Client();
-			client._buildConnectionOptions = (value) => value;
+			client.buildConnectionOptions = (value) => value;
 			const peer = client.newOrderer('url', 'opts');
 			sinon.assert.calledWith(ordererStub, 'url', 'opts');
 			peer.should.deep.equal(new ordererStub());
@@ -2963,20 +2963,20 @@ describe('Client', () => {
 		});
 	});
 
-	describe('#_buildConnectionOptions', () => {
+	describe('#buildConnectionOptions', () => {
 		it('should contain option from original option', () => {
 			const opts = {'clientCert': 'thing'};
 			const client = new Client();
 			client.setTlsClientCertAndKey('cert', 'key');
-			const newOpts = client._buildConnectionOptions(opts);
+			const newOpts = client.buildConnectionOptions(opts);
 			newOpts.clientCert.should.equal('thing');
 		});
 
 		it('should contain cert from tls', () => {
 			const opts = {'some': 'thing'};
 			const client = new Client();
 			client.setTlsClientCertAndKey('cert', 'key');
-			const newOpts = client._buildConnectionOptions(opts);
+			const newOpts = client.buildConnectionOptions(opts);
 			newOpts.some.should.equal('thing');
 			newOpts.clientCert.should.equal('cert');
 		});
@@ -2985,15 +2985,15 @@ describe('Client', () => {
 			const opts = {'safe': 'thing'};
 			const client = new Client();
 			client.addConnectionOptions({'safe': 'other', 'hope': 'found'});
-			const newOpts = client._buildConnectionOptions(opts);
+			const newOpts = client.buildConnectionOptions(opts);
 			newOpts.safe.should.equal('thing');
 			newOpts.hope.should.equal('found');
 		});
 
-		it('should call _buildConnectionOptions and return default opts', () => {
+		it('should call buildConnectionOptions and return default opts', () => {
 			const client = new Client();
 			const opts = client.getConfigSetting('connection-options');
-			const newOpts = client._buildConnectionOptions();
+			const newOpts = client.buildConnectionOptions();
 			newOpts.should.deep.equal(opts);
 		});
 	});

diff --git a/test/integration/grpc.js b/test/integration/grpc.js
@@ -231,7 +231,7 @@ function sentConnectionOptionsIfDefined(optionName, optionValue) {
 }
 
 function getClientOptionsDefinedInDefault(client, defaultConnectionOptions) {
-	const clientConnectionOptions = client._buildConnectionOptions();
+	const clientConnectionOptions = client.buildConnectionOptions();
 	const clientOptionsSubset = {};
 	Object.keys(defaultConnectionOptions).forEach(key => clientOptionsSubset[key] = clientConnectionOptions[key]);
 	return clientOptionsSubset;