FABN-985: Use request txId in Channel.queryByChaincode

- In Channel.queryByChaincode(), use a txId property on the supplied
request if present; otherwise create a new transaction ID as before.
- Add some unit tests for queryByChaincode.

Change-Id: I221405b1dfb6f20c25eed8e3533a7105af538e0d
Signed-off-by: Mark S. Lewis <mark_lewis@uk.ibm.com>

Author: bestbeforetoday

fabric-client/lib/Channel.js

@@ -3119,7 +3119,7 @@ const Channel = class {
 	 *           object will be used.
 	 * @property {string} chaincodeId - Required. The id of the chaincode to process
 	 *           the transaction proposal
-	 * @property {object} [transientMap] - Optional. Object with String property names
+	 * @property {object} transientMap - Optional. Object with String property names
 	 *           and Buffer property values that can be used by the chaincode but not
 	 *           saved in the ledger. Data such as cryptographic information for
 	 *           encryption can be passed to the chaincode using this technique. Data
@@ -3131,6 +3131,7 @@ const Channel = class {
 	 * @property {string[]} args - An array of string arguments specific to the
 	 *           chaincode's 'Invoke' method
 	 * @property {integer} request_timeout - The timeout value to use for this request
+	 * @property {TransactionID} txId Optional. Transaction ID to use for the query.
 	 */
 
 	/**
@@ -3142,29 +3143,34 @@ const Channel = class {
 	 * results in the byte array format and the caller will have to be able to decode
 	 * these results.
 	 *
+	 * If the request contains a <code>txId</code> property, that transaction ID will be used, and its administrative
+	 * privileges will apply. In this case the <code>useAdmin</code> parameter to this function will be ignored.
+	 *
 	 * @param {ChaincodeQueryRequest} request
 	 * @param {boolean} useAdmin - Optional. Indicates that the admin credentials should be used in making
-	 *                  this call
+	 *                  this call. Ignored if the <code>request</code> contains a <code>txId</code> property.
 	 * @returns {Promise} A Promise for an array of byte array results returned from the chaincode
 	 *                    on all Endorsing Peers
 	 * @example
 	 * <caption>Get the list of query results returned by the chaincode</caption>
-	 * channel.queryByChaincode(request)
-	 * .then((response_payloads) => {
-	 *		for(let i = 0; i < response_payloads.length; i++) {
-	 *			console.log(util.format('Query result from peer [%s]: %s', i, response_payloads[i].toString('utf8')));
-	 *		}
-	 *	});
+	 * const responsePayloads = await channel.queryByChaincode(request);
+	 * for (let i = 0; i < responsePayloads.length; i++) {
+	 *     console.log(util.format('Query result from peer [%s]: %s', i, responsePayloads[i].toString('utf8')));
+	 * }
 	 */
 	async queryByChaincode(request, useAdmin) {
 		logger.debug('queryByChaincode - start');
 		if (!request) {
 			throw new Error('Missing request object for this queryByChaincode call.');
 		}
 
+		if (request.txId) {
+			useAdmin = request.txId.isAdmin();
+		}
+
 		const targets = this._getTargets(request.targets, Constants.NetworkConfig.CHAINCODE_QUERY_ROLE);
 		const signer = this._clientContext._getSigningIdentity(useAdmin);
-		const txId = new TransactionID(signer, useAdmin);
+		const txId = request.txId || new TransactionID(signer, useAdmin);
 
 		// make a new request object so we can add in the txId and not change the user's
 		const query_request = {
@@ -3173,37 +3179,39 @@ const Channel = class {
 			fcn: request.fcn,
 			args: request.args,
 			transientMap: request.transientMap,
-			txId: txId,
+			txId,
 			signer: signer
 		};
 
-		let results = await Channel.sendTransactionProposal(query_request, this._name, this._clientContext, request.request_timeout);
-		const responses = results[0];
+		const proposalResults = await Channel.sendTransactionProposal(query_request, this._name, this._clientContext, request.request_timeout);
+		const responses = proposalResults[0];
 		logger.debug('queryByChaincode - results received');
-		if (responses && Array.isArray(responses)) {
-			results = [];
-			for (let i = 0; i < responses.length; i++) {
-				const response = responses[i];
-				if (response instanceof Error) {
-					results.push(response);
-				} else if (response.response && response.response.payload) {
-					if (response.response.status === 200) {
-						results.push(response.response.payload);
+
+		if (!responses || !Array.isArray(responses)) {
+			throw new Error('Payload results are missing from the chaincode query');
+		}
+
+		const results = [];
+		responses.forEach((response) => {
+			if (response instanceof Error) {
+				results.push(response);
+			} else if (response.response && response.response.payload) {
+				if (response.response.status === 200) {
+					results.push(response.response.payload);
+				} else {
+					if (response.response.message) {
+						results.push(new Error(response.response.message));
 					} else {
-						if (response.response.message) {
-							results.push(new Error(response.response.message));
-						} else {
-							results.push(new Error(response));
-						}
+						results.push(new Error(response));
 					}
-				} else {
-					logger.error('queryByChaincode - unknown or missing results in query ::' + results);
-					results.push(new Error(response));
 				}
+			} else {
+				logger.error('queryByChaincode - unknown or missing results in query ::' + results);
+				results.push(new Error(response));
 			}
-			return results;
-		}
-		throw new Error('Payload results are missing from the chaincode query');
+		});
+
+		return results;
 	}
 
 	/**

fabric-client/test/Channel.js

@@ -20,6 +20,7 @@ const rewire = require('rewire');
 const chaiAsPromised = require('chai-as-promised');
 chai.use(chaiAsPromised);
 const expect = chai.expect;
+chai.should();
 
 const Channel = require('fabric-client/lib/Channel');
 const ChannelRewire = rewire('fabric-client/lib/Channel');
@@ -89,12 +90,47 @@ describe('Channel', () => {
 		sinon.restore();
 	});
 
+	/**
+	 * Create a transaction success proposal response.
+	 * @param {Buffer} [payload] Transaction return value.
+	 * @returns {ProposalResponse} protobuff
+	 */
+	function createTransactionResponse(payload) {
+		const proposalResponse = createProposalResponse(payload);
+
+		if (payload) {
+			proposalResponse.response.payload = payload;
+		}
+
+		return proposalResponse;
+	}
+
+	/**
+	 * Create a transaction error proposal response.
+	 * @param {String} [message] Error message.
+	 * @returns {ProposalResponse} protobuff
+	 */
+	function createErrorResponse(message) {
+		const proposalResponse = createProposalResponse(message, 500);
+
+		if (typeof message === 'string') {
+			proposalResponse.response.message = Buffer.from(message);
+		}
+
+		return proposalResponse;
+	}
+
 	/**
 	 * Create a skeleton proposal response object.
-	 * @param {String} results value for the payload.extension.results field of the proposal response
+	 * @param {String} results value for the payload.extension.results fields of the proposal response.
+	 * @param {number} [status=200] status code for the response, where 200 is OK and 400+ is an error.
 	 * @returns {ProposalResponse} protobuff
 	 */
-	function createProposalResponse(results) {
+	function createProposalResponse(results, status = 200) {
+		if (typeof results !== 'string') {
+			results = '';
+		}
+
 		const extension = new proposalProto.ChaincodeAction();
 		extension.response = new responseProto.Response();
 		extension.results = Buffer.from(results);
@@ -109,7 +145,7 @@ describe('Channel', () => {
 		endorsement.endorser = identity.toBuffer();
 
 		const response = new responseProto.Response();
-		response.status = 200;
+		response.status = status;
 
 		const proposalResponse = new responseProto.ProposalResponse();
 		proposalResponse.response = response;
@@ -1190,7 +1226,88 @@ describe('Channel', () => {
 
 	describe('#buildEnvelope', () => {});
 
-	describe('#queryByChaincode', () => {});
+	describe('#queryByChaincode', () => {
+		const peer1Result = 'PEER1_RESULT';
+		const peer2Result = 'PEER2_RESULT';
+		let request;
+		let spySendTransactionProposal;
+
+		beforeEach(() => {
+			sinon.stub(peer1, 'sendProposal').resolves(createTransactionResponse(Buffer.from(peer1Result)));
+			sinon.stub(peer2, 'sendProposal').resolves(createTransactionResponse(Buffer.from(peer2Result)));
+
+			spySendTransactionProposal = sinon.spy(Channel, 'sendTransactionProposal');
+
+			request = {
+				targets: [peer1, peer2],
+				chaincodeId: 'chaincodeId',
+				fcn: 'fcn',
+				args: ['arg1', 'arg2']
+			};
+		});
+
+		it('uses supplied transaction ID', async () => {
+			const txId = client.newTransactionID();
+			request.txId = txId;
+
+			await channel.queryByChaincode(request);
+
+			sinon.assert.calledWith(spySendTransactionProposal, sinon.match.has('txId', txId));
+		});
+
+		it('creates a transaction ID if none supplied', async () => {
+			await channel.queryByChaincode(request);
+			sinon.assert.calledWith(spySendTransactionProposal, sinon.match.has('txId', sinon.match.instanceOf(TransactionID)));
+		});
+
+		it('returns valid peer response payloads', async () => {
+			const results = await channel.queryByChaincode(request);
+
+			const resultStrings = results.map((buffer) => buffer.toString());
+			expect(resultStrings).to.have.members([peer1Result, peer2Result]);
+		});
+
+		it('returns error peer response messages', async () => {
+			const errorMessage = 'ALL YOUR BASE ARE BELONG TO ME';
+			peer1.sendProposal.resolves(createErrorResponse(errorMessage));
+			request.targets = [peer1];
+
+			const results = await channel.queryByChaincode(request);
+
+			expect(results).to.have.lengthOf(1);
+			const result = results[0];
+			expect(result).to.be.an.instanceof(Error);
+			expect(result.message).to.equal(errorMessage);
+		});
+
+		it('returns error peer response without message', async () => {
+			peer1.sendProposal.resolves(createErrorResponse());
+			request.targets = [peer1];
+
+			const results = await channel.queryByChaincode(request);
+
+			expect(results).to.have.lengthOf(1);
+			const result = results[0];
+			expect(result).to.be.an.instanceof(Error);
+		});
+
+		it('returns peer invocation failures', async () => {
+			const peerError = new Error('peer invocation error');
+			peer1.sendProposal.rejects(peerError);
+			request.targets = [peer1];
+
+			const results = await channel.queryByChaincode(request);
+
+			expect(results).to.have.lengthOf(1);
+			const result = results[0];
+			expect(result).to.be.an.instanceof(Error);
+			expect(result.message).to.equal(peerError.message);
+		});
+
+		it('throws if no request supplied', async () => {
+			expect(channel.queryByChaincode()).to.be.rejectedWith('Missing request');
+		});
+	});
 
 	describe('#_getTargetForQuery', () => {});
 

fabric-client/types/index.d.ts

@@ -481,6 +481,7 @@ declare namespace Client { // tslint:disable-line:no-namespace
 		transientMap?: TransientMap;
 		fcn?: string;
 		args: string[];
+		txId?: TransactionId;
 	}
 
 	export interface KeyOpts {

test/integration/e2e/e2eUtils.js

@@ -782,7 +782,6 @@ function queryChaincode(org, version, targets, fcn, args, value, chaincodeId, t,
 			// send query
 			const request = {
 				chaincodeId : chaincodeId,
-				txId: tx_id,
 				fcn: fcn,
 				args: args,
 				request_timeout: 3000