Skip to content

Commit

Permalink
[FABCN-397] State queries limited to 100 results (#141) (#142)
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@mbwhite
mbwhite authored May 11, 2020
1 parent 298687a commit 9d45ba7
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 49 deletions.
9 changes: 6 additions & 3 deletions libraries/fabric-shim/lib/iterators.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down
65 changes: 19 additions & 46 deletions libraries/fabric-shim/test/unit/iterators.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -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;

Expand All @@ -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',
Expand Down Expand Up @@ -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);
Expand All @@ -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');

Expand All @@ -285,19 +258,19 @@ 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();

expect(result).to.deep.equal({done: true});
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();

Expand Down
7 changes: 7 additions & 0 deletions test/chaincodes/crud/chaincode.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}) {
Expand Down
7 changes: 7 additions & 0 deletions test/fv/crud.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down

0 comments on commit 9d45ba7

Please sign in to comment.