From 982eddab0165fca2ae16fa20e938c6f593dc973a Mon Sep 17 00:00:00 2001 From: Matthew B White Date: Tue, 24 Jan 2023 14:43:47 +0000 Subject: [PATCH] GetHistoryForKey to return correct data (#375) * GetHistoryForKey to return correct data The 2.5 release used an updated protobuf js library; this has meant changes across the codebase to use this new library, subtly different API.. this function was sadly missed. Signed-off-by: Matthew B White * Fix unit tests Signed-off-by: Matthew B White Signed-off-by: Matthew B White --- libraries/fabric-shim/lib/iterators.js | 87 ++++----- libraries/fabric-shim/test/unit/iterators.js | 195 ++++++------------- test/chaincodes/crud/chaincode.js | 4 + test/fv/utils.js | 4 + 4 files changed, 110 insertions(+), 180 deletions(-) diff --git a/libraries/fabric-shim/lib/iterators.js b/libraries/fabric-shim/lib/iterators.js index fe304d20d..9016c8abb 100644 --- a/libraries/fabric-shim/lib/iterators.js +++ b/libraries/fabric-shim/lib/iterators.js @@ -8,7 +8,7 @@ const logger = require('./logger').getLogger('lib/iterators.js'); -const {ledger} = require('@hyperledger/fabric-protos'); +const { ledger } = require('@hyperledger/fabric-protos'); /** * CommonIterator allows a chaincode to check whether any more result(s) @@ -20,16 +20,16 @@ const {ledger} = require('@hyperledger/fabric-protos'); class CommonIterator { /** - * constructor + * constructor * * Note that the decoded payload will be a protobuf of type * fabprotos.protos.QueryResponse * - * @param {ChaincodeSupportClient} handler client handler - * @param {string} channel_id channel id - * @param {string} txID transaction id - * @param {object} response decoded payload - */ + * @param {ChaincodeSupportClient} handler client handler + * @param {string} channel_id channel id + * @param {string} txID transaction id + * @param {object} response decoded payload + */ constructor(handler, channel_id, txID, response, type) { this.type = type; this.handler = handler; @@ -42,58 +42,57 @@ class CommonIterator { } /** - * close the iterator. - * @async - * @return {promise} A promise that is resolved with the close payload or rejected - * if there is a problem - */ + * close the iterator. + * @async + * @return {promise} A promise that is resolved with the close payload or rejected + * if there is a problem + */ async close() { logger.debug('close called on %s iterator for txid: %s', this.type, this.txID); return await this.handler.handleQueryStateClose(this.response.getId(), this.channel_id, this.txID); } /* - * decode the payload depending on the type of iterator. - * @param {object} bytes - */ - _getResultFromBytes(bytes) { - if (this.type === 'QUERY') { - return ledger.queryresult.KV.deserializeBinary(bytes.getResultbytes()); - } else if (this.type === 'HISTORY') { - return ledger.queryresult.KeyModification.deserializeBinary(bytes.getResultbytes()); - } - throw new Error('Iterator constructed with unknown type: ' + this.type); - } - - - /* - * creates a return value - */ + * creates a return value + */ _createAndEmitResult() { - const queryResult = {}; const resultsList = this.response.getResultsList(); + let queryResult; - const queryResultPb = this._getResultFromBytes(resultsList[this.currentLoc]); - queryResult.value = {value:Buffer.from(queryResultPb.getValue())}; - /* istanbul ignore else*/ - if ('getKey' in queryResultPb) { - queryResult.value.key = Buffer.from(queryResultPb.getKey()).toString(); + // established external API has a very specific structure here + // so need to 'fluff' up this structure to match + // Not all queryResults have the same methods + if (this.type === 'QUERY') { + const queryResultPb = ledger.queryresult.KV.deserializeBinary((resultsList[this.currentLoc]).getResultbytes()); + queryResult = { + key: queryResultPb.getKey(), + value: Buffer.from(queryResultPb.getValue()) + }; + } else if (this.type === 'HISTORY') { + const queryResultPb = ledger.queryresult.KeyModification.deserializeBinary((resultsList[this.currentLoc]).getResultbytes()); + queryResult = { + txId: queryResultPb.getTxId(), + value: Buffer.from(queryResultPb.getValue()), + isDelete: queryResultPb.getIsDelete(), + timestamp: queryResultPb.getTimestamp().toObject() + + }; + } else { + throw new Error('Iterator constructed with unknown type: ' + this.type); } - this.currentLoc++; - queryResult.done = false; - return queryResult; + return {value: queryResult, done: false}; } /** - * Get the next value and return it through a promise. - * @async - * @return {promise} a promise that is fulfilled with an object { value: (next value) }, - * is fulfilled with an object { done: true } if there is no more value, - * or is rejected if any error occurs. - */ + * Get the next value and return it through a promise. + * @async + * @return {promise} a promise that is fulfilled with an object { value: (next value) }, + * is fulfilled with an object { done: true } if there is no more value, + * or is rejected if any error occurs. + */ async next() { // check to see if there are some results left in the current result set const resultsList = this.response.getResultsList(); @@ -112,7 +111,7 @@ class CommonIterator { throw err; } } - return {done: true}; + return { done: true }; } } diff --git a/libraries/fabric-shim/test/unit/iterators.js b/libraries/fabric-shim/test/unit/iterators.js index 2be0c62ba..324fe1fef 100644 --- a/libraries/fabric-shim/test/unit/iterators.js +++ b/libraries/fabric-shim/test/unit/iterators.js @@ -14,10 +14,11 @@ const rewire = require('rewire'); const Iterator = rewire('../../../fabric-shim/lib/iterators.js'); const StateQueryIterator = Iterator.StateQueryIterator; const HistoryQueryIterator = Iterator.HistoryQueryIterator; -const {ChaincodeMessageHandler} = require('../../../fabric-shim/lib/handler.js'); +const { ChaincodeMessageHandler } = require('../../../fabric-shim/lib/handler.js'); +const google_protobuf_timestamp_pb = require('google-protobuf/google/protobuf/timestamp_pb'); +const { ledger } = require('@hyperledger/fabric-protos'); -const {ledger} = require('@hyperledger/fabric-protos'); const channel_id = 'theChannelId'; const txID = 'aTx'; @@ -38,7 +39,7 @@ describe('Iterator', () => { describe('CommonIterator', () => { const CommonIterator = Iterator.__get__('CommonIterator'); - it ('should set the variables using the arguments in the constructor', () => { + it('should set the variables using the arguments in the constructor', () => { const ci = new CommonIterator(mockHandler, channel_id, txID, mockResponse, 'some type'); expect(ci.type).to.deep.equal('some type'); @@ -50,7 +51,7 @@ describe('Iterator', () => { }); describe('close', () => { - it ('should return handler.handleQueryStateClose', async () => { + it('should return handler.handleQueryStateClose', async () => { mockResponse.getId = () => 1; mockHandler.handleQueryStateClose = sinon.stub().resolves('some resolution'); @@ -65,158 +66,80 @@ describe('Iterator', () => { }); }); - describe('_getResultFromBytes', () => { - - it ('should return KV decode on resultbytes for a QUERY type', () => { - const ci = new CommonIterator(mockHandler, channel_id, txID, mockResponse, 'QUERY'); - - const bytes = new ledger.queryresult.KV(); - bytes.setValue('some bytes'); - const result = ci._getResultFromBytes({getResultbytes:() => bytes.serializeBinary()}); - expect(result).is.not.null; - }); - - it ('should return KeyModification decode on resultbytes for a HISTORY type', () => { - const ci = new CommonIterator(mockHandler, channel_id, txID, mockResponse, 'HISTORY'); - - const bytes = new ledger.queryresult.KeyModification(); - bytes.setValue('some bytes'); - - const result = ci._getResultFromBytes({getResultbytes:() => bytes.serializeBinary()}); - expect(result).is.not.null; - }); - - it ('should throw an error for unknown types', () => { - const ci = new CommonIterator(mockHandler, channel_id, txID, mockResponse, 'some type'); - - expect(() => { - ci._getResultFromBytes({resultBytes: 'some bytes'}); - }).to.throw(/Iterator constructed with unknown type: some type/); - }); - }); - describe('_createAndEmitResult', () => { let ci; - let getResultFromBytesStub; + let getResultBytesQuery; + let getResultBytesHistory; + let queryResult; + let historyResult; + let timestampStub; + let getResultbytes; beforeEach(() => { ci = new CommonIterator(mockHandler, channel_id, txID, mockResponse, 'QUERY'); - getResultFromBytesStub = sinon.stub(ci, '_getResultFromBytes').returns('some result'); - }); + queryResult = sandbox.createStubInstance(ledger.queryresult.KV); + historyResult = sandbox.createStubInstance(ledger.queryresult.KeyModification); + timestampStub = sandbox.createStubInstance(google_protobuf_timestamp_pb.Timestamp); + getResultbytes = sinon.stub(); - afterEach(() => { - getResultFromBytesStub.restore(); - }); - it ('should return value of first element of results converted from bytes and done false when hasMore false and results has no more elements after currentLoc', () => { - mockResponse.getResultsList = () => ['some result bytes']; - mockResponse.getHasMore = () => false; - getResultFromBytesStub.returns({getKey:() => 'akey', getValue:() => 'some result'}); - - const result = ci._createAndEmitResult(); - - expect(getResultFromBytesStub.calledOnce).to.be.true; - expect(getResultFromBytesStub.firstCall.args).to.deep.equal(['some result bytes']); - expect(result).to.deep.equal({ - value: {value:Buffer.from('some result'), - key: 'akey'}, - done: false - }); + // queryResult.getTimestamp.returns(timestampStub); + timestampStub.toObject.returns({ seconds: 0, nanos: 0 }); + queryResult.getValue.returns('hello'); + queryResult.getKey.returns('fred'); + historyResult.getValue.returns('hello'); + historyResult.getTxId.returns('0xCAFE'); + historyResult.getIsDelete.returns(true); + historyResult.getTimestamp.returns(timestampStub); + getResultBytesQuery = sinon.stub(ledger.queryresult.KV, 'deserializeBinary').returns(queryResult); + getResultBytesHistory = sinon.stub(ledger.queryresult.KeyModification, 'deserializeBinary').returns(historyResult); }); - it ('should return value of first element of results converted from bytes and done false when hasMore true and results has no more elements after currentLoc', () => { - mockResponse.getResultsList = () => ['some result bytes']; - mockResponse.getHasMore = () => true; - getResultFromBytesStub.returns({getKey:() => 'akey', getValue:() => 'some result'}); - - const result = ci._createAndEmitResult(); - - expect(getResultFromBytesStub.calledOnce).to.be.true; - expect(getResultFromBytesStub.firstCall.args).to.deep.equal(['some result bytes']); - expect(ci.currentLoc).to.deep.equal(1); - expect(result).to.deep.equal({ - value: {value:Buffer.from('some result'), key: 'akey'}, - done: false, - - }); + afterEach(() => { + getResultBytesQuery.restore(); + getResultBytesHistory.restore(); }); - it ('should return value of first element of results converted from bytes and done false when hasMore false and results has elements after currentLoc', () => { - mockResponse.getResultsList = () => ['some result bytes', 'some more result bytes']; - mockResponse.getHasMore = () => false; - getResultFromBytesStub.returns({getKey:() => 'akey', getValue:() => 'some result'}); - + it('should return value from query API', () => { + mockResponse.getResultsList = () => [{ getResultbytes }]; const result = ci._createAndEmitResult(); - expect(getResultFromBytesStub.calledOnce).to.be.true; - expect(getResultFromBytesStub.firstCall.args).to.deep.equal(['some result bytes']); - expect(ci.currentLoc).to.deep.equal(1); + expect(getResultBytesQuery.calledOnce).to.be.true; expect(result).to.deep.equal({ - value: {value:Buffer.from('some result'), - key: 'akey'}, + value: { + key: 'fred', + value: Buffer.from('hello') + }, done: false }); }); - it ('should return value of first element of results converted from bytes and done false when hasMore true and results has elements after currentLoc', () => { - mockResponse.getResultsList = () => ['some result bytes', 'some more result bytes']; - mockResponse.getHasMore = () => true; - - getResultFromBytesStub.returns({getKey:() => 'akey', getValue:() => 'some result'}); - + it('should return value from history API', () => { + mockResponse.getResultsList = () => [{ getResultbytes }]; + ci.type = 'HISTORY'; const result = ci._createAndEmitResult(); - expect(getResultFromBytesStub.calledOnce).to.be.true; - expect(getResultFromBytesStub.firstCall.args).to.deep.equal(['some result bytes']); + expect(getResultBytesHistory.calledOnce).to.be.true; expect(ci.currentLoc).to.deep.equal(1); expect(result).to.deep.equal({ - value: {value: Buffer.from('some result'), - key: 'akey'}, + value: { + value: Buffer.from('hello'), + isDelete: true, + timestamp: {nanos:0, seconds:0}, + txId:'0xCAFE' + }, done: false }); }); - it ('should return as expected with non-zero currentLoc', () => { - mockResponse.getResultsList = () => ['some result bytes', 'some more result bytes']; - mockResponse.getHasMore = () => true; - - ci.currentLoc = 1; - - getResultFromBytesStub.returns({getKey:() => 'akey', getValue:() => 'some result'}); + it('should return error from history API', () => { - - const result = ci._createAndEmitResult(); - - expect(getResultFromBytesStub.calledOnce).to.be.true; - expect(getResultFromBytesStub.firstCall.args).to.deep.equal(['some more result bytes']); - expect(ci.currentLoc).to.deep.equal(2); - expect(result).to.deep.equal({ - value: {value:Buffer.from('some result'), - key: 'akey'}, - done: false - }); + mockResponse.getResultsList = () => [{ getResultbytes }]; + ci.type = 'WIBBLE'; + expect(()=>{ci._createAndEmitResult();}).to.throw(/Iterator constructed with unknown type/); }); - it('should return value of first element of results converted from bytes and done false', () => { - mockResponse.getResultsList = () => ['some result bytes', 'some more result bytes']; - mockResponse.getHasMore = () => false; - - const expectedResult = { - value: {value: Buffer.from('some result'), - key: 'akey'}, - done: false - }; - - getResultFromBytesStub.returns({getKey:() => 'akey', getValue:() => 'some result'}); - - const result = ci._createAndEmitResult(); - - expect(getResultFromBytesStub.calledOnce).to.be.true; - expect(getResultFromBytesStub.firstCall.args).to.deep.equal(['some result bytes']); - expect(ci.currentLoc).to.deep.equal(1); - expect(result).to.deep.equal(expectedResult); - }); }); describe('next', () => { @@ -225,14 +148,14 @@ describe('Iterator', () => { beforeEach(() => { ci = new CommonIterator(mockHandler, channel_id, txID, mockResponse, 'QUERY'); - createAndEmitResultStub = sinon.stub(ci, '_createAndEmitResult').returns('some result'); + createAndEmitResultStub = sinon.stub(ci, '_createAndEmitResult').returns('some result'); }); afterEach(() => { createAndEmitResultStub.restore(); }); - it ('should return _createAndEmitResult when there are elements left in the result set', async () => { + it('should return _createAndEmitResult when there are elements left in the result set', async () => { mockResponse.getResultsList = () => ['some result bytes', 'some more result bytes']; const result = await ci.next(); @@ -240,7 +163,7 @@ describe('Iterator', () => { expect(result).to.deep.equal('some result'); }); - it ('should return _createAndEmitResult when response hasMore and no error occurs', async () => { + it('should return _createAndEmitResult when response hasMore and no error occurs', async () => { mockResponse.getResultsList = () => []; mockResponse.getHasMore = () => true; mockResponse.getId = () => 1; @@ -262,7 +185,7 @@ describe('Iterator', () => { expect(ci.response).to.deep.equal(nextResponse); }); - it ('should throw an error if error occurs when hasMore and listenerCount for data = 0', async () => { + it('should throw an error if error occurs when hasMore and listenerCount for data = 0', async () => { mockResponse.getResultsList = () => []; mockResponse.getHasMore = () => true; @@ -278,30 +201,30 @@ describe('Iterator', () => { expect(createAndEmitResultStub.notCalled).to.be.true; }); - it ('should return done if response does not hasMore and listenerCount for end > 0', async () => { + it('should return done if response does not hasMore and listenerCount for end > 0', async () => { mockResponse.getResultsList = () => []; mockResponse.getHasMore = () => false; const result = await ci.next(); - expect(result).to.deep.equal({done: true}); + expect(result).to.deep.equal({ done: true }); expect(createAndEmitResultStub.notCalled).to.be.true; }); - it ('should return done if response does not hasMore and listenerCount for end = 0', async () => { + it('should return done if response does not hasMore and listenerCount for end = 0', async () => { mockResponse.getResultsList = () => []; mockResponse.getHasMore = () => false; const result = await ci.next(); - expect(result).to.deep.equal({done: true}); + expect(result).to.deep.equal({ done: true }); expect(createAndEmitResultStub.notCalled).to.be.true; }); }); }); describe('StateQueryIterator', () => { - it ('should extend CommonIterator using QUERY for type', () => { + it('should extend CommonIterator using QUERY for type', () => { const sqi = new StateQueryIterator(mockHandler, channel_id, txID, mockResponse); expect(sqi instanceof Iterator.__get__('CommonIterator')).to.be.true; @@ -310,7 +233,7 @@ describe('Iterator', () => { }); describe('HistoryQueryIterator', () => { - it ('should extend CommonIterator using HISTORY for type', () => { + it('should extend CommonIterator using HISTORY for type', () => { const hqi = new HistoryQueryIterator(mockHandler, channel_id, txID, mockResponse); expect(hqi instanceof Iterator.__get__('CommonIterator')).to.be.true; diff --git a/test/chaincodes/crud/chaincode.js b/test/chaincodes/crud/chaincode.js index 8e0c286d3..78eaedb6c 100644 --- a/test/chaincodes/crud/chaincode.js +++ b/test/chaincodes/crud/chaincode.js @@ -14,6 +14,7 @@ async function getAllResults(iterator, getKeys) { const res = await iterator.next(); if (!res.value && res.done) { await iterator.close(); + console.log(`All Result Old Iterator `+allResults) return allResults; } else if (!res.value) { throw new Error('no value and not done (internal error?)'); @@ -23,6 +24,7 @@ async function getAllResults(iterator, getKeys) { if (res.done) { await iterator.close(); loop = false; + console.log(`All Result Old Iterator `+allResults) return allResults; } } @@ -35,6 +37,8 @@ async function getAllResultsUsingAsyncIterator(promiseOfIterator, getKeys) { allResults.push(theVal); } + console.log(`All Result AsyncIterator `+allResults) + // iterator will be automatically closed on exit from the loop // either by reaching the end, or a break or throw terminated the loop return allResults; diff --git a/test/fv/utils.js b/test/fv/utils.js index 8a8dee08e..c88f6adb5 100755 --- a/test/fv/utils.js +++ b/test/fv/utils.js @@ -98,6 +98,8 @@ async function invoke(ccName, func, args, transient) { cmd += `peer chaincode invoke ${getTLSArgs()} -o localhost:7050 -C mychannel -n ${ccName} -c '${printArgs(func, args)}' --waitForEvent --waitForEventTimeout 100s 2>&1`; } const {stderr, stdout} = execSync(cmd); + console.log(stderr); + console.log(stdout) if (stderr) { throw new Error(stderr); } @@ -116,6 +118,8 @@ async function query(ccName, func, args, transient) { cmd += `peer chaincode query ${getTLSArgs()} -C mychannel -n ${ccName} -c '${printArgs(func, args)}' 2>&1`; } const {error, stdout, stderr} = await exec(cmd); + console.log(stderr); + console.log(stdout); if (error) { throw new Error(error, stderr); }