Skip to content

Commit

Permalink
GetHistoryForKey to return correct data
Browse files Browse the repository at this point in the history
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 <whitemat@uk.ibm.com>
  • Loading branch information
mbwhite committed Jan 20, 2023
1 parent 21c4377 commit 9100d47
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 53 deletions.
24 changes: 16 additions & 8 deletions libraries/fabric-shim/lib/iterators.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,29 @@ class CommonIterator {
* creates a return value
*/
_createAndEmitResult() {
const queryResult = {};

const resultsList = this.response.getResultsList();

const queryResultPb = this._getResultFromBytes(resultsList[this.currentLoc]);
queryResult.value = {value:Buffer.from(queryResultPb.getValue())};
/* istanbul ignore else*/

// established external API has a very specific structure here
// so need to 'fluff' up this structure to match
// of possible concern is that the timestamp (google's proto definition)
// may have changed with these new protos
const queryResult = {
value: queryResultPb.getValue_asU8(),
txId: queryResultPb.getTxId(),
isDelete: queryResultPb.getIsDelete(),
timestamp: queryResultPb.getTimestamp().toObject(),
};

if ('getKey' in queryResultPb) {
queryResult.value.key = Buffer.from(queryResultPb.getKey()).toString();
queryResult.key = queryResultPb.getKey();
}


this.currentLoc++;

queryResult.done = false;
return queryResult;
return {value:queryResult, done:false};
}

/**
Expand All @@ -108,7 +116,7 @@ class CommonIterator {
this.response = response;
return this._createAndEmitResult();
} catch (err) {
logger.error('unexpected error received getting next value: %s', err.message);
logger.error('unexpected error received geFtting next value: %s', err.message);
throw err;
}
}
Expand Down
142 changes: 97 additions & 45 deletions libraries/fabric-shim/test/unit/iterators.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ 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 google_protobuf_timestamp_pb = require('google-protobuf/google/protobuf/timestamp_pb');

const {ledger} = require('@hyperledger/fabric-protos');
const { time } = require('console');
const channel_id = 'theChannelId';
const txID = 'aTx';

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

Expand All @@ -67,102 +68,122 @@ describe('Iterator', () => {

describe('_getResultFromBytes', () => {

it ('should return KV decode on resultbytes for a QUERY type', () => {
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()});
const result = ci._getResultFromBytes({ getResultbytes: () => bytes.serializeBinary() });
expect(result).is.not.null;
});

it ('should return KeyModification decode on resultbytes for a HISTORY type', () => {
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()});
const result = ci._getResultFromBytes({ getResultbytes: () => bytes.serializeBinary() });
expect(result).is.not.null;
});

it ('should throw an error for unknown types', () => {
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'});
ci._getResultFromBytes({ resultBytes: 'some bytes' });
}).to.throw(/Iterator constructed with unknown type: some type/);
});
});

describe('_createAndEmitResult', () => {
let ci;
let getResultFromBytesStub;
let queryResult;
let timestampStub;

beforeEach(() => {
ci = new CommonIterator(mockHandler, channel_id, txID, mockResponse, 'QUERY');
getResultFromBytesStub = sinon.stub(ci, '_getResultFromBytes').returns('some result');
queryResult = sandbox.createStubInstance(ledger.queryresult.KeyModification);

timestampStub = sandbox.createStubInstance(google_protobuf_timestamp_pb.Timestamp);

queryResult.getTimestamp.returns(timestampStub);
timestampStub.toObject.returns({seconds:0,nanos:0});
queryResult.getValue_asU8.returns(Buffer.from('hello'));
queryResult.getTxId.returns('0xCAFE');
queryResult.getIsDelete.returns(true);
getResultFromBytesStub = sinon.stub(ci, '_getResultFromBytes').returns(queryResult);
});

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', () => {
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'},
value: {
value: Buffer.from('hello'),
txId:'0xCAFE',
isDelete:true,
timestamp:{seconds:0,nanos:0},
},
done: false
});
});

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', () => {
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,

value: {
value: Buffer.from('hello'),
txId:'0xCAFE',
isDelete:true,
timestamp:{seconds:0,nanos:0},
},
done: false
});
});

it ('should return value of first element of results converted from bytes and done false when hasMore 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.getResultsList = () => ['some result bytes', 'some more 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(ci.currentLoc).to.deep.equal(1);
expect(result).to.deep.equal({
value: {value:Buffer.from('some result'),
key: 'akey'},
value: {
value: Buffer.from('hello'),
txId:'0xCAFE',
isDelete:true,
timestamp:{seconds:0,nanos:0},
},
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', () => {
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'});
// getResultFromBytesStub.returns({ getKey: () => 'akey', getValue: () => 'some result' });


const result = ci._createAndEmitResult();
Expand All @@ -171,29 +192,58 @@ describe('Iterator', () => {
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'},
value: {
value: Buffer.from('hello'),
txId:'0xCAFE',
isDelete:true,
timestamp:{seconds:0,nanos:0},
},
done: false
});
});

it ('should return as expected with non-zero currentLoc', () => {
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'});
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('hello'),
txId:'0xCAFE',
isDelete:true,
timestamp:{seconds:0,nanos:0},
},
done: false
});
});

it('should return as expected with key element', () => {
mockResponse.getResultsList = () => ['some result bytes', 'some more result bytes'];
mockResponse.getHasMore = () => true;

ci.currentLoc = 1;

queryResult.getKey = sinon.stub().returns('IamKey');
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'},
value: {
key:'IamKey',
value: Buffer.from('hello'),
txId:'0xCAFE',
isDelete:true,
timestamp:{seconds:0,nanos:0},
},
done: false
});
});
Expand All @@ -203,13 +253,15 @@ describe('Iterator', () => {
mockResponse.getHasMore = () => false;

const expectedResult = {
value: {value: Buffer.from('some result'),
key: 'akey'},
value: {
value: Buffer.from('hello'),
txId:'0xCAFE',
isDelete:true,
timestamp:{seconds:0,nanos:0},
},
done: false
};

getResultFromBytesStub.returns({getKey:() => 'akey', getValue:() => 'some result'});

const result = ci._createAndEmitResult();

expect(getResultFromBytesStub.calledOnce).to.be.true;
Expand All @@ -225,22 +277,22 @@ 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();

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;
Expand All @@ -262,7 +314,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;

Expand All @@ -278,30 +330,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;
Expand All @@ -310,7 +362,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;
Expand Down

0 comments on commit 9100d47

Please sign in to comment.