From 3e2f6f1d9a73fb5dfd27e0c3283f78d8759a9d2e Mon Sep 17 00:00:00 2001 From: Matthew B White Date: Wed, 6 May 2020 11:20:50 +0100 Subject: [PATCH] [FABCN-397] State queries limited to 100 results Errow was the new version of protobufjs used to create the bundle.js gave a different property to the 'has more' field. hasMore replaced has_more As this accessed as JS property it didn't fail, but meant only a limited number could be accessed. corrected, added test for 229 results. bit of dead code pruning Signed-off-by: Matthew B White --- libraries/fabric-shim/lib/iterators.js | 9 ++- libraries/fabric-shim/test/unit/iterators.js | 65 ++++++-------------- test/chaincodes/crud/chaincode.js | 7 +++ test/fv/crud.js | 7 +++ 4 files changed, 39 insertions(+), 49 deletions(-) diff --git a/libraries/fabric-shim/lib/iterators.js b/libraries/fabric-shim/lib/iterators.js index 6e67c613..64ca53cc 100644 --- a/libraries/fabric-shim/lib/iterators.js +++ b/libraries/fabric-shim/lib/iterators.js @@ -20,6 +20,10 @@ class CommonIterator { /** * 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 @@ -66,9 +70,8 @@ class CommonIterator { const queryResult = {}; queryResult.value = this._getResultFromBytes(this.response.results[this.currentLoc]); this.currentLoc++; - // TODO: potential breaking change if it's assumed that if done == true then it has a valid value + queryResult.done = false; - // queryResult.done = !(this.currentLoc < this.response.results.length || this.response.has_more); return queryResult; } @@ -85,7 +88,7 @@ class CommonIterator { return this._createAndEmitResult(); } else { // check to see if there is more and go fetch it - if (this.response.has_more) { + if (this.response.hasMore) { try { const response = await this.handler.handleQueryStateNext(this.response.id, this.channel_id, this.txID); this.currentLoc = 0; diff --git a/libraries/fabric-shim/test/unit/iterators.js b/libraries/fabric-shim/test/unit/iterators.js index 3150c578..5d208108 100644 --- a/libraries/fabric-shim/test/unit/iterators.js +++ b/libraries/fabric-shim/test/unit/iterators.js @@ -107,9 +107,9 @@ describe('Iterator', () => { getResultFromBytesStub.restore(); }); - it ('should return value of first element of results converted from bytes and done false when has_more false and results has no more elements after currentLoc', () => { + 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.results = ['some result bytes']; - mockResponse.has_more = false; + mockResponse.hasMore = false; const result = ci._createAndEmitResult(); @@ -121,9 +121,9 @@ describe('Iterator', () => { }); }); - it ('should return value of first element of results converted from bytes and done false when has_more true and results has no more elements after currentLoc', () => { + 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.results = ['some result bytes']; - mockResponse.has_more = true; + mockResponse.hasMore = true; const result = ci._createAndEmitResult(); @@ -136,9 +136,9 @@ describe('Iterator', () => { }); }); - it ('should return value of first element of results converted from bytes and done false when has_more false and results has elements after currentLoc', () => { + 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.results = ['some result bytes', 'some more result bytes']; - mockResponse.has_more = false; + mockResponse.hasMore = false; const result = ci._createAndEmitResult(); @@ -151,9 +151,9 @@ describe('Iterator', () => { }); }); - it ('should return value of first element of results converted from bytes and done false when has_more true and results has elements after currentLoc', () => { + 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.results = ['some result bytes', 'some more result bytes']; - mockResponse.has_more = true; + mockResponse.hasMore = true; const result = ci._createAndEmitResult(); @@ -168,7 +168,7 @@ describe('Iterator', () => { it ('should return as expected with non-zero currentLoc', () => { mockResponse.results = ['some result bytes', 'some more result bytes']; - mockResponse.has_more = true; + mockResponse.hasMore = true; ci.currentLoc = 1; @@ -185,7 +185,7 @@ describe('Iterator', () => { it ('should return value of first element of results converted from bytes and done false', () => { mockResponse.results = ['some result bytes', 'some more result bytes']; - mockResponse.has_more = false; + mockResponse.hasMore = false; const expectedResult = { value: 'some result', @@ -222,13 +222,13 @@ describe('Iterator', () => { expect(result).to.deep.equal('some result'); }); - it ('should return _createAndEmitResult when response has_more and no error occurs', async () => { + it ('should return _createAndEmitResult when response hasMore and no error occurs', async () => { mockResponse.results = []; - mockResponse.has_more = true; + mockResponse.hasMore = true; const nextResponse = { results: ['some result bytes', 'some more result bytes'], - has_more: false + hasMore: false }; mockHandler.handleQueryStateNext = sinon.stub().resolves(nextResponse); @@ -242,36 +242,9 @@ describe('Iterator', () => { expect(ci.response).to.deep.equal(nextResponse); }); - /* - it ('should emit an error if error occurs when has_more and listenerCount for data > 0', async () => { + it ('should throw an error if error occurs when hasMore and listenerCount for data = 0', async () => { mockResponse.results = []; - mockResponse.has_more = true; - - const err = new Error('some error'); - - mockHandler.handleQueryStateNext = sinon.stub().rejects(err); - const emitStub = sinon.stub(ci, 'emit'); - const listenerCountStub = sinon.stub(ci, 'listenerCount').returns(1); - - ci.currentLoc = 1; - - const result = await ci.next(); - - expect(result).to.be.undefined; - expect(createAndEmitResultStub.notCalled).to.be.ok; - expect(listenerCountStub.calledOnce).to.be.ok; - expect(listenerCountStub.firstCall.args).to.deep.equal(['data']); - expect(emitStub.calledOnce).to.be.ok; - expect(emitStub.firstCall.args).to.deep.equal(['error', ci, err]); - - listenerCountStub.restore(); - emitStub.restore(); - }); - */ - - it ('should throw an error if error occurs when has_more and listenerCount for data = 0', async () => { - mockResponse.results = []; - mockResponse.has_more = true; + mockResponse.hasMore = true; const err = new Error('some error'); @@ -285,9 +258,9 @@ describe('Iterator', () => { expect(createAndEmitResultStub.notCalled).to.be.true; }); - it ('should return done if response does not has_more and listenerCount for end > 0', async () => { + it ('should return done if response does not hasMore and listenerCount for end > 0', async () => { mockResponse.results = []; - mockResponse.has_more = false; + mockResponse.hasMore = false; const result = await ci.next(); @@ -295,9 +268,9 @@ describe('Iterator', () => { expect(createAndEmitResultStub.notCalled).to.be.true; }); - it ('should return done if response does not has_more and listenerCount for end = 0', async () => { + it ('should return done if response does not hasMore and listenerCount for end = 0', async () => { mockResponse.results = []; - mockResponse.has_more = false; + mockResponse.hasMore = false; const result = await ci.next(); diff --git a/test/chaincodes/crud/chaincode.js b/test/chaincodes/crud/chaincode.js index 8e37c5b7..8e0c286d 100644 --- a/test/chaincodes/crud/chaincode.js +++ b/test/chaincodes/crud/chaincode.js @@ -63,6 +63,13 @@ class CrudChaincode extends Contract { await stub.putState(`key${i}`, Buffer.from(`value${i}`)); await stub.putState(`jsonkey${i}`, Buffer.from(JSON.stringify({key: `k${i}`, value: `value${i}`}))); } + + // add a large set of keys for testing pagination and larger data sets + const DATA_SET_SIZE=229; + for (let i = 0; i < DATA_SET_SIZE;i++){ + const compositeKey = stub.createCompositeKey('bulk-data',['bulk',i.toString().padStart(3,'0')]); + await stub.putState(compositeKey, Buffer.from(i.toString().padStart(3,'0'))); + } } async getKey({stub}) { diff --git a/test/fv/crud.js b/test/fv/crud.js index 7e1c42e8..a9bc2ffd 100755 --- a/test/fv/crud.js +++ b/test/fv/crud.js @@ -154,6 +154,13 @@ describe('Chaincode CRUD', () => { expect(JSON.parse(payload)).to.deep.equal(['annblack', 'annred', 'annyellow']); }); + + it('should return the bulk states from a partial composite key using the old iterator style', async function() { + this.timeout(MED_STEP); + const payload = await utils.query(suite, 'org.mynamespace.crud:getStateByPartialCompositeKey', ['bulk-data','bulk']); + expect(JSON.parse(payload)).to.have.lengthOf(229); + }); + it('should return a state from a partial composite key using the new iterator style', async function() { this.timeout(SHORT_STEP); const payload = await utils.query(suite, 'org.mynamespace.crud:getStateByPartialCompositeKeyUsingAsyncIterator', ['name~color', 'ann']);