Skip to content

Commit

Permalink
FABN-929: Enfore single Transaction invocation
Browse files Browse the repository at this point in the history 
Transaction objects obtained from Contract.createTransaction()
represent a specific transaction with a single transaction ID.
A new Transaction object must be obtained for every transaction
invocation. This change enforces that submit() or evaluate()
can only be called once on a Transaction object.

Change-Id: I909e77d719cabb08c2b5f4f766b8ecd671d6960c
Signed-off-by: Mark S. Lewis <mark_lewis@uk.ibm.com>
  • Loading branch information
@bestbeforetoday @harrisob
bestbeforetoday authored and harrisob committed Nov 6, 2018
1 parent 5fcf519 commit e20c048
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 59 deletions.
10 changes: 10 additions & 0 deletions fabric-network/lib/transaction.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class Transaction {
this._transactionId = contract.createTransactionID();
this._transientMap = null;
this._createTxEventHandler = (() => noOpTxEventHandler);
this._isInvoked = false;
}

/**
Expand Down Expand Up @@ -103,6 +104,7 @@ class Transaction {
*/
async submit(...args) {
verifyArguments(args);
this._setInvokedOrThrow();

const network = this._contract.getNetwork();
const channel = network.getChannel();
Expand Down Expand Up @@ -147,6 +149,13 @@ class Transaction {
return validResponses[0].response.payload || null;
}

_setInvokedOrThrow() {
if (this._isInvoked) {
throw new Error('Transaction has already been invoked');
}
this._isInvoked = true;
}

/**
* Check for proposal response errors.
* @private
Expand Down Expand Up @@ -205,6 +214,7 @@ class Transaction {
*/
async evaluate(...args) {
verifyArguments(args);
this._setInvokedOrThrow();

const queryHandler = this._contract.getQueryHandler();
const chaincodeId = this._contract.getChaincodeId();
Expand Down
126 changes: 67 additions & 59 deletions fabric-network/test/transaction.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,41 @@ const TransactionEventHandler = require('fabric-network/lib/impl/event/transacti
const TransactionID = require('fabric-client/lib/TransactionID');

describe('Transaction', () => {
const transactionName = 'TRANSACTION_NAME';
const expectedResult = Buffer.from('42');

const fakeProposal = {proposal: 'I do'};
const fakeHeader = {header: 'gooooal'};
const validProposalResponse = {
response: {
status: 200,
payload: expectedResult
}
};
const noPayloadProposalResponse = {
response: {
status: 200
}
};
const errorProposalResponse = Object.assign(new Error(), {response: {status: 500, payload: 'error'}});
const emptyStringProposalResponse = {
response: {
status: 200,
payload: Buffer.from('')
}
};

const validProposalResponses = [[validProposalResponse], fakeProposal, fakeHeader];
const noPayloadProposalResponses = [[noPayloadProposalResponse], fakeProposal, fakeHeader];
const noProposalResponses = [[], fakeProposal, fakeHeader];
const errorProposalResponses = [[errorProposalResponse], fakeProposal, fakeHeader];
const mixedProposalResponses = [[validProposalResponse, errorProposalResponse], fakeProposal, fakeHeader];
const emptyStringProposalResponses = [[emptyStringProposalResponse], fakeProposal, fakeHeader];

let stubContract;
let transaction;
let channel;
let stubQueryHandler;

beforeEach(() => {
stubContract = sinon.createStubInstance(Contract);
Expand All @@ -32,11 +66,19 @@ describe('Transaction', () => {
const network = sinon.createStubInstance(Network);
stubContract.getNetwork.returns(network);

const channel = sinon.createStubInstance(Channel);
stubQueryHandler = sinon.createStubInstance(QueryHandler);
stubQueryHandler.queryChaincode.resolves(expectedResult);
stubContract.getQueryHandler.returns(stubQueryHandler);

channel = sinon.createStubInstance(Channel);
channel.sendTransactionProposal.resolves(validProposalResponses);
channel.sendTransaction.resolves({status: 'SUCCESS'});
network.getChannel.returns(channel);

stubContract.getChaincodeId.returns('chaincode-id');
stubContract.getEventHandlerOptions.returns({commitTimeout: 418});

transaction = new Transaction(stubContract, transactionName);
});

afterEach(() => {
Expand All @@ -45,24 +87,20 @@ describe('Transaction', () => {

describe('#getName', () => {
it('return the name', () => {
const name = 'TRANSACTION_NAME';
const transaction = new Transaction(stubContract, name);
const result = transaction.getName();
expect(result).to.equal(name);
expect(result).to.equal(transactionName);
});
});

describe('#getTransactionID', () => {
it('has a default transaction ID', () => {
const transaction = new Transaction(stubContract, 'name');
const result = transaction.getTransactionID();
expect(result).to.be.an.instanceOf(TransactionID);
});
});

describe('#setEventHandlerStrategy', () => {
it('returns this', () => {
const transaction = new Transaction(stubContract, 'name');
const stubEventHandler = sinon.createStubInstance(TransactionEventHandler);
const stubEventHandlerFactoryFn = () => stubEventHandler;

Expand All @@ -74,61 +112,21 @@ describe('Transaction', () => {

describe('#setTransient', () => {
it('returns this', () => {
const transaction = new Transaction(stubContract, 'name');
const result = transaction.setTransient(new Map());
expect(result).to.equal(transaction);
});
});

describe('#submit', () => {
const transactionName = 'TRANSACTION_NAME';
const expectedResult = Buffer.from('42');

const fakeProposal = {proposal: 'I do'};
const fakeHeader = {header: 'gooooal'};
const validProposalResponse = {
response: {
status: 200,
payload: expectedResult
}
};
const noPayloadProposalResponse = {
response: {
status: 200
}
};
const errorProposalResponse = Object.assign(new Error(), {response: {status: 500, payload: 'error'}});
const emptyStringProposalResponse = {
response: {
status: 200,
payload: Buffer.from('')
}
};

const validProposalResponses = [[validProposalResponse], fakeProposal, fakeHeader];
const noPayloadProposalResponses = [[noPayloadProposalResponse], fakeProposal, fakeHeader];
const noProposalResponses = [[], fakeProposal, fakeHeader];
const errorProposalResponses = [[errorProposalResponse], fakeProposal, fakeHeader];
const mixedProposalResponses = [[validProposalResponse, errorProposalResponse], fakeProposal, fakeHeader];
const emptyStringProposalResponses = [[emptyStringProposalResponse], fakeProposal, fakeHeader];

let transaction;
let expectedProposal;
let channel;

beforeEach(() => {
transaction = new Transaction(stubContract, transactionName);

expectedProposal = {
fcn: transactionName,
txId: transaction.getTransactionID(),
chaincodeId: stubContract.getChaincodeId(),
args: []
};

channel = stubContract.getNetwork().getChannel();
channel.sendTransactionProposal.resolves(validProposalResponses);
channel.sendTransaction.resolves({status: 'SUCCESS'});
});

it('rejects for non-string arguments', () => {
Expand Down Expand Up @@ -222,23 +220,21 @@ describe('Transaction', () => {
const result = await transaction.submit();
expect(result.toString()).to.equal('');
});
});

describe('#evaluate', () => {
const transactionName = 'TRANSACTION_NAME';
const expectedResult = Buffer.from('42');

let stubQueryHandler;
let transaction;

beforeEach(() => {
stubQueryHandler = sinon.createStubInstance(QueryHandler);
stubQueryHandler.queryChaincode.resolves(expectedResult);
stubContract.getQueryHandler.returns(stubQueryHandler);
it('throws if called a second time', async () => {
await transaction.submit();
const promise = transaction.submit();
return expect(promise).to.be.rejectedWith('Transaction has already been invoked');
});

transaction = new Transaction(stubContract, transactionName);
it('throws if called after evaluate', async () => {
await transaction.evaluate();
const promise = transaction.submit();
return expect(promise).to.be.rejectedWith('Transaction has already been invoked');
});
});

describe('#evaluate', () => {
it('returns the result from the query handler', async () => {
const result = await transaction.evaluate();
expect(result).to.equal(expectedResult);
Expand Down Expand Up @@ -292,5 +288,17 @@ describe('Transaction', () => {
const result = await transaction.evaluate();
expect(result.toString()).to.equal('');
});

it('throws if called a second time', async () => {
await transaction.evaluate();
const promise = transaction.evaluate();
return expect(promise).to.be.rejectedWith('Transaction has already been invoked');
});

it('throws if called after submit', async () => {
await transaction.submit();
const promise = transaction.evaluate();
return expect(promise).to.be.rejectedWith('Transaction has already been invoked');
});
});
});

0 comments on commit e20c048

Please sign in to comment.