Poor performance of GET /api/delegates/[address]/forging_statistics - Closes #2352

api/controllers/delegates.js

@@ -15,7 +15,6 @@
 'use strict';
 
 var _ = require('lodash');
-var Bignum = require('../../helpers/bignum.js');
 var swaggerHelper = require('../../helpers/swagger');
 
 // Private Fields
@@ -126,11 +125,11 @@ DelegatesController.getForgingStatistics = function(context, next) {
 
 	var filters = {
 		address: params.address.value,
-		start: params.fromTimestamp.value || constants.epochTime.getTime(),
-		end: params.toTimestamp.value || Date.now(),
+		start: params.fromTimestamp.value,
+		end: params.toTimestamp.value,
 	};
 
-	modules.blocks.utils.aggregateBlocksReward(filters, (err, reward) => {
+	modules.delegates.shared.getForgingStatistics(filters, (err, reward) => {
 		if (err) {
 			if (err === 'Account not found' || err === 'Account is not a delegate') {
 				return next(
@@ -140,23 +139,19 @@ DelegatesController.getForgingStatistics = function(context, next) {
 			return next(err);
 		}
 
-		var forged = new Bignum(reward.fees)
-			.plus(new Bignum(reward.rewards))
-			.toString();
-		var response = {
+		return next(null, {
 			data: {
 				fees: reward.fees,
 				rewards: reward.rewards,
-				forged,
+				forged: reward.forged,
 				count: reward.count,
 			},
 			meta: {
-				fromTimestamp: filters.start,
-				toTimestamp: filters.end,
+				fromTimestamp: filters.start || constants.epochTime.getTime(),
+				toTimestamp: filters.end || Date.now(),
 			},
 			links: {},
-		};
-		return next(null, response);
+		});
 	});
 };
 

modules/delegates.js

@@ -24,6 +24,7 @@ const BlockReward = require('../logic/block_reward.js');
 const jobsQueue = require('../helpers/jobs_queue.js');
 const Delegate = require('../logic/delegate.js');
 const slots = require('../helpers/slots.js');
+const Bignum = require('../helpers/bignum.js');
 const transactionTypes = require('../helpers/transaction_types.js');
 
 // Private fields
@@ -1077,6 +1078,62 @@ Delegates.prototype.shared = {
 			return setImmediate(cb, null, delegates);
 		});
 	},
+
+	/**
+	 *
+	 * @param {Object} filters - Filters applied to results
+	 * @param {string} filters.address - Address of the delegate
+	 * @param {string} filters.start - Start time to aggregate
+	 * @param {string} filters.end - End time to aggregate
+	 * @params {function} cb - Callback function
+	 * @param {SetImmediateCallback} cb
+	 */
+	getForgingStatistics(filters, cb) {
+		// If need to aggregate all data then just fetch from the account
+		if (!filters.start && !filters.end) {
+			// TODO: Need to move modules.delegates.getDelegates after adding "fees" in its list
+			modules.accounts.getAccount(
+				{ address: filters.address },
+				['rewards', 'fees', 'producedBlocks', 'isDelegate'],
+				(err, delegate) => {
+					if (err) {
+						return setImmediate(cb, err);
+					}
+
+					if (!delegate) {
+						return setImmediate(cb, 'Account not found');
+					}
+
+					if (!delegate.isDelegate) {
+						return setImmediate(cb, 'Account is not a delegate');
+					}
+
+					return setImmediate(cb, null, {
+						rewards: delegate.rewards,
+						fees: delegate.fees,
+						count: new Bignum(delegate.producedBlocks).toString(),
+						forged: new Bignum(delegate.rewards)
+							.plus(new Bignum(delegate.fees))
+							.toString(),
+					});
+				}
+			);
+
+			// If need to aggregate some period of time
+		} else {
+			modules.blocks.utils.aggregateBlocksReward(filters, (err, reward) => {
+				if (err) {
+					return setImmediate(cb, err);
+				}
+
+				reward.forged = new Bignum(reward.fees)
+					.plus(new Bignum(reward.rewards))
+					.toString();
+
+				return setImmediate(cb, null, reward);
+			});
+		}
+	},
 };
 
 // Export

test/unit/modules/delegates.js

@@ -17,6 +17,9 @@
 var genesisDelegates = require('../../data/genesis_delegates.json');
 var accountFixtures = require('../../fixtures/accounts');
 var application = require('../../common/application');
+const seeder = require('../../common/db_seed');
+
+let db;
 
 describe('delegates', () => {
 	var library;
@@ -26,6 +29,8 @@ describe('delegates', () => {
 			{ sandbox: { name: 'lisk_test_modules_delegates' } },
 			(err, scope) => {
 				library = scope;
+				db = scope.db;
+
 				// Set delegates module as loaded to allow manual forging
 				library.rewiredModules.delegates.__set__('__private.loaded', true);
 				// Load forging delegates
@@ -39,6 +44,10 @@ describe('delegates', () => {
 		application.cleanup(done);
 	});
 
+	afterEach(() => {
+		return sinonSandbox.restore();
+	});
+
 	describe('__private', () => {
 		describe('loadDelegates', () => {
 			var loadDelegates;
@@ -481,4 +490,149 @@ describe('delegates', () => {
 			});
 		});
 	});
+
+	describe('shared', () => {
+		const validDelegate = genesisDelegates.delegates[0];
+
+		describe('getForgingStatistics', () => {
+			it('should fail if invalid address is passed', done => {
+				library.modules.delegates.shared.getForgingStatistics(
+					{ address: 'InvalidAddress' },
+					(err, data) => {
+						expect(err).to.be.eql('Account not found');
+						expect(data).to.be.undefined;
+						done();
+					}
+				);
+			});
+			it('should fail if non-delegate address is passed', done => {
+				// To keep the genesis delegates we are not resetting the seeds
+				seeder
+					.seedAccounts(db)
+					.then(() => {
+						const validAccount = seeder.getAccounts()[0];
+						library.modules.delegates.shared.getForgingStatistics(
+							{ address: validAccount.address },
+							(err, data) => {
+								expect(err).to.be.eql('Account is not a delegate');
+								expect(data).to.be.undefined;
+								done();
+							}
+						);
+					})
+					.catch(done);
+			});
+			it('should be ok if a valid delegate address is passed', done => {
+				library.modules.delegates.shared.getForgingStatistics(
+					{ address: validDelegate.address },
+					(err, data) => {
+						expect(err).to.be.null;
+						expect(data).to.have.keys('count', 'rewards', 'fees', 'forged');
+						done();
+					}
+				);
+			});
+			it('should aggregate the data runtime if start and end is provided', done => {
+				sinonSandbox.spy(library.modules.blocks.utils, 'aggregateBlocksReward');
+				sinonSandbox.spy(library.modules.accounts, 'getAccount');
+
+				const params = {
+					address: validDelegate.address,
+					end: Date.now(),
+					start: Date.now() - 7,
+				};
+
+				library.modules.delegates.shared.getForgingStatistics(
+					params,
+					(err, data) => {
+						expect(err).to.be.null;
+						expect(data).to.have.keys('count', 'rewards', 'fees', 'forged');
+						expect(library.modules.blocks.utils.aggregateBlocksReward).to.be
+							.calledOnce;
+						expect(
+							library.modules.blocks.utils.aggregateBlocksReward.firstCall
+								.args[0]
+						).to.be.eql(params);
+						expect(library.modules.accounts.getAccount).to.not.been.called;
+						done();
+					}
+				);
+			});
+			it('should aggregate the data runtime if start is omitted', done => {
+				sinonSandbox.spy(library.modules.blocks.utils, 'aggregateBlocksReward');
+				sinonSandbox.spy(library.modules.accounts, 'getAccount');
+
+				const params = {
+					address: validDelegate.address,
+					end: Date.now(),
+				};
+
+				library.modules.delegates.shared.getForgingStatistics(
+					params,
+					(err, data) => {
+						expect(err).to.be.null;
+						expect(data).to.have.keys('count', 'rewards', 'fees', 'forged');
+						expect(library.modules.blocks.utils.aggregateBlocksReward).to.be
+							.calledOnce;
+						expect(
+							library.modules.blocks.utils.aggregateBlocksReward.firstCall
+								.args[0]
+						).to.be.eql(params);
+						expect(library.modules.accounts.getAccount).to.not.been.called;
+						done();
+					}
+				);
+			});
+
+			it('should aggregate the data runtime if end is omitted', done => {
+				sinonSandbox.spy(library.modules.blocks.utils, 'aggregateBlocksReward');
+				sinonSandbox.spy(library.modules.accounts, 'getAccount');
+
+				const params = {
+					address: validDelegate.address,
+					start: Date.now() - 7,
+				};
+
+				library.modules.delegates.shared.getForgingStatistics(
+					params,
+					(err, data) => {
+						expect(err).to.be.null;
+						expect(data).to.have.keys('count', 'rewards', 'fees', 'forged');
+						expect(library.modules.blocks.utils.aggregateBlocksReward).to.be
+							.calledOnce;
+						expect(
+							library.modules.blocks.utils.aggregateBlocksReward.firstCall
+								.args[0]
+						).to.be.eql(params);
+						expect(library.modules.accounts.getAccount).to.not.been.called;
+						done();
+					}
+				);
+			});
+
+			it('should fetch data from accounts if both start and end is omitted', done => {
+				sinonSandbox.spy(library.modules.blocks.utils, 'aggregateBlocksReward');
+				sinonSandbox.spy(library.modules.accounts, 'getAccount');
+
+				const params = {
+					address: validDelegate.address,
+				};
+
+				library.modules.delegates.shared.getForgingStatistics(
+					params,
+					(err, data) => {
+						expect(err).to.be.null;
+						expect(data).to.have.keys('count', 'rewards', 'fees', 'forged');
+						expect(library.modules.blocks.utils.aggregateBlocksReward).to.not
+							.been.called;
+						expect(
+							library.modules.accounts.getAccount.firstCall.args[0]
+						).to.be.eql(params);
+						expect(library.modules.accounts.getAccount).to.be.calledOnce;
+						done();
+					}
+				);
+			});
+		});
+	});
 });