From 5d8f5fd98a998db1c5d6d8da65d8af0028da0b37 Mon Sep 17 00:00:00 2001 From: Jonathan Niles Date: Thu, 30 Aug 2018 15:54:31 +0100 Subject: [PATCH 1/4] test(accounts): initial unit test for read() A test has been added for the read() method to show that the debounce method will actually work. --- .../services/AccountService.spec.js | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/client-unit/services/AccountService.spec.js diff --git a/test/client-unit/services/AccountService.spec.js b/test/client-unit/services/AccountService.spec.js new file mode 100644 index 0000000000..2f23701c57 --- /dev/null +++ b/test/client-unit/services/AccountService.spec.js @@ -0,0 +1,35 @@ +/* global inject expect */ + +describe('AccountService', () => { + + let Accounts; + let $httpBackend; + + beforeEach(module('bhima.services', 'bhima.mocks', 'angularMoment', 'bhima.constants')); + + beforeEach(inject((_$httpBackend_, AccountService, MockDataService) => { + $httpBackend = _$httpBackend_; + Accounts = AccountService; + + $httpBackend.when('GET', '/accounts/') + .respond(200, MockDataService.accounts()); + })); + + // throws if there are more outstanding requests + afterEach(() => { + $httpBackend.verifyNoOutstandingExpectation(); + $httpBackend.verifyNoOutstandingRequest(); + }); + + + it('#read() will fire an HTTP GET request each time it is called', () => { + const NUM_REQUESTS = 10; + let count = NUM_REQUESTS; + while (count--) { + Accounts.read(); + } + + // this would throw if too many requests were called. + expect(() => $httpBackend.flush(NUM_REQUESTS)).not.to.throw(); + }); +}); From 765f5750bab4bb9ecfc0fa8adcd1d2b9787c199b Mon Sep 17 00:00:00 2001 From: Jonathan Niles Date: Fri, 31 Aug 2018 11:17:22 +0100 Subject: [PATCH 2/4] feat(accounts): debounce read request This commit adds a debounce() call to the read request. Since it uses promises and Angular's $timeout(), it will require flushing the $timeouts in tests. Closes #2892. --- client/src/js/services/debounce.js | 41 +++++++++++++++++ .../src/modules/accounts/accounts.service.js | 8 ++-- .../cash/modals/transfer-modal.ctrl.js | 45 ++++++++++--------- server/config/express.js | 1 - .../services/AccountService.spec.js | 15 +++++-- test/client-unit/services/VoucherForm.spec.js | 5 ++- 6 files changed, 86 insertions(+), 29 deletions(-) create mode 100644 client/src/js/services/debounce.js diff --git a/client/src/js/services/debounce.js b/client/src/js/services/debounce.js new file mode 100644 index 0000000000..032266d4dc --- /dev/null +++ b/client/src/js/services/debounce.js @@ -0,0 +1,41 @@ +angular.module('bhima.services') + .factory('debounce', ['$timeout', '$q', ($timeout, $q) => { + + // The service is actually this function, which we call with the func + // that should be debounced and how long to wait in between calls + return function debounce(func, wait, immediate) { + let timeout; + // Create a deferred object that will be resolved when we need to + // actually call the func + let deferred = $q.defer(); + + return function fn() { + const context = this; + + // eslint-disable-next-line prefer-rest-params + const args = arguments; + + const later = function () { + timeout = null; + if (!immediate) { + deferred.resolve(func.apply(context, args)); + deferred = $q.defer(); + } + }; + + const callNow = immediate && !timeout; + if (timeout) { + $timeout.cancel(timeout); + } + + timeout = $timeout(later, wait); + + if (callNow) { + deferred.resolve(func.apply(context, args)); + deferred = $q.defer(); + } + + return deferred.promise; + }; + }; + }]); diff --git a/client/src/modules/accounts/accounts.service.js b/client/src/modules/accounts/accounts.service.js index 80855797bb..fb2cc9149b 100644 --- a/client/src/modules/accounts/accounts.service.js +++ b/client/src/modules/accounts/accounts.service.js @@ -2,7 +2,7 @@ angular.module('bhima.services') .service('AccountService', AccountService); AccountService.$inject = [ - 'PrototypeApiService', 'bhConstants', + 'PrototypeApiService', 'bhConstants', 'debounce', ]; /** @@ -10,11 +10,12 @@ AccountService.$inject = [ * * A service wrapper for the /accounts HTTP endpoint. */ -function AccountService(Api, bhConstants) { +function AccountService(Api, bhConstants, debounce) { const baseUrl = '/accounts/'; const service = new Api(baseUrl); - service.read = read; + // debounce the read() method by 250 milliseconds to avoid needless GET requests + service.read = debounce(read, 150, false); service.label = label; service.getBalance = getBalance; @@ -41,6 +42,7 @@ function AccountService(Api, bhConstants) { .then(service.util.unwrapHttpResponse); } + /** * The read() method loads data from the api endpoint. If an id is provided, * the $http promise is resolved with a single JSON object, otherwise an array diff --git a/client/src/modules/cash/modals/transfer-modal.ctrl.js b/client/src/modules/cash/modals/transfer-modal.ctrl.js index 37f3af7686..3937a4ebe4 100644 --- a/client/src/modules/cash/modals/transfer-modal.ctrl.js +++ b/client/src/modules/cash/modals/transfer-modal.ctrl.js @@ -3,7 +3,7 @@ angular.module('bhima.controllers') CashTransferModalController.$inject = [ 'CurrencyService', 'VoucherService', 'CashboxService', 'AccountService', 'SessionService', - 'CashService', '$state', 'NotifyService', 'ReceiptModal', 'bhConstants', 'VoucherForm' + 'CashService', '$state', 'NotifyService', 'ReceiptModal', 'bhConstants', 'VoucherForm', ]; /** @@ -12,32 +12,35 @@ CashTransferModalController.$inject = [ * @description * This controller is responsible transferring money between a cashbox and a transfer account. */ -function CashTransferModalController(Currencies, Vouchers, Cashboxes, Accounts, Session, Cash, $state, Notify, Receipts, bhConstants, VoucherForm) { - var vm = this; +function CashTransferModalController( + Currencies, Vouchers, Cashboxes, Accounts, Session, Cash, $state, Notify, + Receipts, bhConstants, VoucherForm +) { + const vm = this; vm.voucher = new VoucherForm('CashTransferForm'); - var TRANSFER_TYPE_ID = bhConstants.transactionType.TRANSFER; + const TRANSFER_TYPE_ID = bhConstants.transactionType.TRANSFER; vm.loadAccountDetails = loadAccountDetails; vm.submit = submit; // submit and close the modal function submit(form) { - if (form.$invalid) { return; } + if (form.$invalid) { return 0; } - var record = prepareVoucherRecord(); + const record = prepareVoucherRecord(); // validate - var validation = vm.voucher.validate(); - if (!validation) { return; } + const validation = vm.voucher.validate(); + if (!validation) { return 0; } return Vouchers.create(record) - .then(function (response) { + .then((response) => { Notify.success('CASH.TRANSFER.SUCCESS'); return Receipts.voucher(response.uuid, true); }) - .then(function () { + .then(() => { return $state.go('^.window', { id : $state.params.id }); }) .catch(Notify.handleError); @@ -46,15 +49,15 @@ function CashTransferModalController(Currencies, Vouchers, Cashboxes, Accounts, function prepareVoucherRecord() { // extract the voucher from the VoucherForm - var record = vm.voucher.details; + const record = vm.voucher.details; record.items = vm.voucher.store.data; // configure the debits/credits appropriately - var debit = record.items[0]; + const debit = record.items[0]; debit.configure({ debit : vm.amount, account_id : vm.transferAccount.id }); - var credit = record.items[1]; + const credit = record.items[1]; credit.configure({ credit : vm.amount, account_id : vm.cashAccount.id }); // format voucher description as needed @@ -62,19 +65,19 @@ function CashTransferModalController(Currencies, Vouchers, Cashboxes, Accounts, amount : vm.amount, fromLabel : vm.cashAccount.label, toLabel : vm.transferAccount.label, - userName : Session.user.display_name + userName : Session.user.display_name, }); return record; } // this object retains a mapping of the currency ids to their respective accounts. - var cashCurrencyMap = {}; + let cashCurrencyMap = {}; // this function maps the accounts to their respective currencies. // { currency_id : { currency_id, account_id, transfer_account_id } } function mapCurrenciesToAccounts(currencies) { - return currencies.reduce(function (map, currency) { + return currencies.reduce((map, currency) => { map[currency.currency_id] = currency; return map; }, {}); @@ -88,11 +91,11 @@ function CashTransferModalController(Currencies, Vouchers, Cashboxes, Accounts, // load needed modules Currencies.read() - .then(function (currencies) { + .then((currencies) => { vm.currencies = currencies; return Cashboxes.read($state.params.id); }) - .then(function (cashbox) { + .then((cashbox) => { vm.cashbox = cashbox; vm.disabledCurrencyIds = Cash.calculateDisabledIds(cashbox, vm.currencies); @@ -108,11 +111,11 @@ function CashTransferModalController(Currencies, Vouchers, Cashboxes, Accounts, cashCurrencyMap = mapCurrenciesToAccounts(vm.cashbox.currencies); // pull the accounts from the cashCurrencyMap - var accounts = cashCurrencyMap[selectedCurrencyId]; + const accounts = cashCurrencyMap[selectedCurrencyId]; // look up the transfer account Accounts.read(accounts.transfer_account_id) - .then(function (account) { + .then((account) => { account.hrlabel = Accounts.label(account); vm.transferAccount = account; }) @@ -120,7 +123,7 @@ function CashTransferModalController(Currencies, Vouchers, Cashboxes, Accounts, // look up the cash account Accounts.read(accounts.account_id) - .then(function (account) { + .then((account) => { account.hrlabel = Accounts.label(account); vm.cashAccount = account; }) diff --git a/server/config/express.js b/server/config/express.js index fb6a8f16b0..32e3e50386 100644 --- a/server/config/express.js +++ b/server/config/express.js @@ -97,7 +97,6 @@ exports.configure = function configure(app) { // Only allow routes to use /login, /projects, /logout, and /languages if a // user session does not exists - app.use((req, res, next) => { if (_.isUndefined(req.session.user) && !within(req.path, publicRoutes)) { debug(`Rejecting unauthorized access to ${req.path} from ${req.ip}`); diff --git a/test/client-unit/services/AccountService.spec.js b/test/client-unit/services/AccountService.spec.js index 2f23701c57..f0b4b00aaf 100644 --- a/test/client-unit/services/AccountService.spec.js +++ b/test/client-unit/services/AccountService.spec.js @@ -4,11 +4,16 @@ describe('AccountService', () => { let Accounts; let $httpBackend; + let $timeout; + let $verifyNoPendingTasks; beforeEach(module('bhima.services', 'bhima.mocks', 'angularMoment', 'bhima.constants')); - beforeEach(inject((_$httpBackend_, AccountService, MockDataService) => { + beforeEach(inject((_$httpBackend_, AccountService, MockDataService, _$timeout_, _$verifyNoPendingTasks_) => { $httpBackend = _$httpBackend_; + $timeout = _$timeout_; + $verifyNoPendingTasks = _$verifyNoPendingTasks_; + Accounts = AccountService; $httpBackend.when('GET', '/accounts/') @@ -19,17 +24,21 @@ describe('AccountService', () => { afterEach(() => { $httpBackend.verifyNoOutstandingExpectation(); $httpBackend.verifyNoOutstandingRequest(); + $verifyNoPendingTasks('$timeout'); }); - it('#read() will fire an HTTP GET request each time it is called', () => { + it('#read() will fire only a single HTTP GET request when called mutliple times in a row', () => { const NUM_REQUESTS = 10; let count = NUM_REQUESTS; while (count--) { Accounts.read(); } + $timeout.flush(); + // this would throw if too many requests were called. - expect(() => $httpBackend.flush(NUM_REQUESTS)).not.to.throw(); + expect(() => $httpBackend.flush(1)).not.to.throw(); + expect(() => $httpBackend.flush(1)).to.throw(); }); }); diff --git a/test/client-unit/services/VoucherForm.spec.js b/test/client-unit/services/VoucherForm.spec.js index bb9b537c01..60a83fa073 100644 --- a/test/client-unit/services/VoucherForm.spec.js +++ b/test/client-unit/services/VoucherForm.spec.js @@ -6,6 +6,7 @@ describe('VoucherForm', () => { let Session; let form; let Mocks; + let $timeout; beforeEach(module( 'bhima.services', @@ -19,10 +20,11 @@ describe('VoucherForm', () => { 'bhima.mocks' )); - beforeEach(inject((_VoucherForm_, $httpBackend, _SessionService_, _MockDataService_) => { + beforeEach(inject((_VoucherForm_, $httpBackend, _SessionService_, _MockDataService_, _$timeout_) => { VoucherForm = _VoucherForm_; Session = _SessionService_; Mocks = _MockDataService_; + $timeout = _$timeout_; // set up the required properties for the session Session.create(Mocks.user(), Mocks.enterprise(), Mocks.project()); @@ -37,6 +39,7 @@ describe('VoucherForm', () => { // make sure $http is clean after tests afterEach(() => { + $timeout.flush(); httpBackend.flush(); httpBackend.verifyNoOutstandingExpectation(); httpBackend.verifyNoOutstandingRequest(); From 110204cabda0573a289e6bb8fc838da92fa831c1 Mon Sep 17 00:00:00 2001 From: Jonathan Niles Date: Wed, 5 Sep 2018 08:57:18 +0100 Subject: [PATCH 3/4] feat(HttpCacheService): cache HTTP requests This commit implements the HttpCacheService, a new wrapper for any $http get request that will cache the result for a specified number of seconds to ensure that large payloads are not called too frequently and overload the server. A prototype is implement on the Accounts Service. Closes #2892. --- client/src/js/services/HttpCacheService.js | 75 +++++++++++++++++++ client/src/js/services/debounce.js | 41 ---------- .../src/modules/accounts/accounts.service.js | 13 ++-- .../services/AccountService.spec.js | 54 +++++++++++-- 4 files changed, 132 insertions(+), 51 deletions(-) create mode 100644 client/src/js/services/HttpCacheService.js delete mode 100644 client/src/js/services/debounce.js diff --git a/client/src/js/services/HttpCacheService.js b/client/src/js/services/HttpCacheService.js new file mode 100644 index 0000000000..e9fde9ba5b --- /dev/null +++ b/client/src/js/services/HttpCacheService.js @@ -0,0 +1,75 @@ +angular.module('bhima.services') + .service('HttpCacheService', HttpCacheService); + +HttpCacheService.$inject = ['$interval']; + +/** + * @function HttpCacheService + * + * @description + * The HttpCacheService is a generic wrapper for asynchronous requests to ensure + * they requests with identical parameters are only called once. It works by + * serializing the request's parameters and caching the response to those parameters + * for a short time. If any other requests are made during the cached time, the + * previous response is returned directly. + * + * NOTE(@jniles) - we use $interval here to avoid slowing down protractor tests. + * Thanks to @sfount who debugged this in Dec 2016. + */ +function HttpCacheService($interval) { + + // default cache limit - 15 seconds + const HTTP_CACHE_DEFAULT_TIMEOUT = 15000; + + /** + * @function serialize + * + * @description + * This function serializes the arguments passed in to create a string key + * that can be used to index the callback results. + * + * @returns {String} - string representation of arguments + */ + const serialize = (...args) => JSON.stringify(args); + + /** + * @function HttpCache + * + * @description + * Takes in a callback function to call if the result is not in the cache. The + * response is cached and returned to the caller. + * + * @param {Function} callback - a callback function to call if there is no cached + * value. The result of this function will be cached. + * @param {Number} duration - the duration the result will be cached. + * + * @returns {Function} - a function that wraps the cache query or original + * callback. + */ + function HttpCache(callback, duration = HTTP_CACHE_DEFAULT_TIMEOUT) { + const cache = new Map(); + + + function read(...args) { + const key = serialize(args); + + // if the cache has been populated return the value from memory + if (cache.has(key)) { + return cache.get(key); + } + + // call the callback to get the result and cache it + const promise = callback(...args); + cache.set(key, promise); + + // remove the result from the cache after a duration. Repeated only once + $interval(() => cache.delete(key), duration, 1); + + return promise; + } + + return read; + } + + return HttpCache; +} diff --git a/client/src/js/services/debounce.js b/client/src/js/services/debounce.js deleted file mode 100644 index 032266d4dc..0000000000 --- a/client/src/js/services/debounce.js +++ /dev/null @@ -1,41 +0,0 @@ -angular.module('bhima.services') - .factory('debounce', ['$timeout', '$q', ($timeout, $q) => { - - // The service is actually this function, which we call with the func - // that should be debounced and how long to wait in between calls - return function debounce(func, wait, immediate) { - let timeout; - // Create a deferred object that will be resolved when we need to - // actually call the func - let deferred = $q.defer(); - - return function fn() { - const context = this; - - // eslint-disable-next-line prefer-rest-params - const args = arguments; - - const later = function () { - timeout = null; - if (!immediate) { - deferred.resolve(func.apply(context, args)); - deferred = $q.defer(); - } - }; - - const callNow = immediate && !timeout; - if (timeout) { - $timeout.cancel(timeout); - } - - timeout = $timeout(later, wait); - - if (callNow) { - deferred.resolve(func.apply(context, args)); - deferred = $q.defer(); - } - - return deferred.promise; - }; - }; - }]); diff --git a/client/src/modules/accounts/accounts.service.js b/client/src/modules/accounts/accounts.service.js index fb2cc9149b..84e0f555fd 100644 --- a/client/src/modules/accounts/accounts.service.js +++ b/client/src/modules/accounts/accounts.service.js @@ -2,7 +2,7 @@ angular.module('bhima.services') .service('AccountService', AccountService); AccountService.$inject = [ - 'PrototypeApiService', 'bhConstants', 'debounce', + 'PrototypeApiService', 'bhConstants', 'HttpCacheService', ]; /** @@ -10,12 +10,12 @@ AccountService.$inject = [ * * A service wrapper for the /accounts HTTP endpoint. */ -function AccountService(Api, bhConstants, debounce) { +function AccountService(Api, bhConstants, HttpCache) { const baseUrl = '/accounts/'; const service = new Api(baseUrl); // debounce the read() method by 250 milliseconds to avoid needless GET requests - service.read = debounce(read, 150, false); + service.read = read; service.label = label; service.getBalance = getBalance; @@ -42,6 +42,10 @@ function AccountService(Api, bhConstants, debounce) { .then(service.util.unwrapHttpResponse); } + const callback = (id, options) => Api.read.call(service, id, options) + .then(handleAccounts); + + const fetcher = HttpCache(callback); /** * The read() method loads data from the api endpoint. If an id is provided, @@ -54,8 +58,7 @@ function AccountService(Api, bhConstants, debounce) { * an array of JSONs. */ function read(id, options) { - return Api.read.call(this, id, options) - .then(handleAccounts); + return fetcher(id, options); } function handleAccounts(accounts) { diff --git a/test/client-unit/services/AccountService.spec.js b/test/client-unit/services/AccountService.spec.js index f0b4b00aaf..b56c9a3afc 100644 --- a/test/client-unit/services/AccountService.spec.js +++ b/test/client-unit/services/AccountService.spec.js @@ -4,27 +4,36 @@ describe('AccountService', () => { let Accounts; let $httpBackend; - let $timeout; + let $interval; let $verifyNoPendingTasks; beforeEach(module('bhima.services', 'bhima.mocks', 'angularMoment', 'bhima.constants')); - beforeEach(inject((_$httpBackend_, AccountService, MockDataService, _$timeout_, _$verifyNoPendingTasks_) => { + beforeEach(inject((_$httpBackend_, AccountService, MockDataService, _$interval_, _$verifyNoPendingTasks_) => { $httpBackend = _$httpBackend_; - $timeout = _$timeout_; + $interval = _$interval_; $verifyNoPendingTasks = _$verifyNoPendingTasks_; Accounts = AccountService; $httpBackend.when('GET', '/accounts/') .respond(200, MockDataService.accounts()); + + $httpBackend.when('GET', '/accounts/1') + .respond(200, { id : 1, number : 1, name : 'First Account' }); + + $httpBackend.when('GET', '/accounts/2') + .respond(200, { id : 2, number : 2, name : 'Second Account' }); + + $httpBackend.when('GET', '/accounts/3') + .respond(200, { id : 3, number : 3, name : 'Third Account' }); })); // throws if there are more outstanding requests afterEach(() => { $httpBackend.verifyNoOutstandingExpectation(); $httpBackend.verifyNoOutstandingRequest(); - $verifyNoPendingTasks('$timeout'); + $verifyNoPendingTasks('$interval'); }); @@ -35,10 +44,45 @@ describe('AccountService', () => { Accounts.read(); } - $timeout.flush(); + $interval.flush(); // this would throw if too many requests were called. expect(() => $httpBackend.flush(1)).not.to.throw(); expect(() => $httpBackend.flush(1)).to.throw(); }); + + it('#read() will fire multiple HTTP GET requests for different parameters', () => { + const NUM_REQUESTS = 3; + let count = NUM_REQUESTS; + + do { + Accounts.read(count); + } while (count--); + + $interval.flush(); + + // this would throw if too many requests were called. + expect(() => $httpBackend.flush(4)).not.to.throw(); + expect(() => $httpBackend.flush(1)).to.throw(); + }); + + it('#read() will return different results based on the parameters passed in', () => { + let a; + let b; + + Accounts.read(1) + .then(res => { a = res; }); + + Accounts.read(2) + .then(res => { b = res; }); + + $interval.flush(); + $httpBackend.flush(); + + const resA = { id : 1, number : 1, name : 'First Account' }; + const resB = { id : 2, number : 2, name : 'Second Account' }; + + expect(a).to.deep.equal(resA); + expect(b).to.deep.equal(resB); + }); }); From 2ffd167020c28f054b5b922f0fd33e240edcdc6d Mon Sep 17 00:00:00 2001 From: Jonathan Niles Date: Wed, 5 Sep 2018 17:43:07 +0100 Subject: [PATCH 4/4] feat(HttpCache): add cache bust parameter This commit adds a cache-busting parameter to the HttpCache. This allows a developer to refresh a stale cache for things like updates. --- client/src/js/services/HttpCacheService.js | 9 ++++---- .../modules/accounts/AccountStoreService.js | 22 +++++++++---------- .../src/modules/accounts/accounts.service.js | 11 +++++----- .../services/AccountService.spec.js | 14 +++++++++++- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/client/src/js/services/HttpCacheService.js b/client/src/js/services/HttpCacheService.js index e9fde9ba5b..51b54ed74a 100644 --- a/client/src/js/services/HttpCacheService.js +++ b/client/src/js/services/HttpCacheService.js @@ -49,17 +49,16 @@ function HttpCacheService($interval) { function HttpCache(callback, duration = HTTP_CACHE_DEFAULT_TIMEOUT) { const cache = new Map(); - - function read(...args) { - const key = serialize(args); + function read(id, parameters, cacheBust = false) { + const key = serialize(id, parameters); // if the cache has been populated return the value from memory - if (cache.has(key)) { + if (cache.has(key) && !cacheBust) { return cache.get(key); } // call the callback to get the result and cache it - const promise = callback(...args); + const promise = callback(id, parameters); cache.set(key, promise); // remove the result from the cache after a duration. Repeated only once diff --git a/client/src/modules/accounts/AccountStoreService.js b/client/src/modules/accounts/AccountStoreService.js index 274c3880db..89402b149a 100644 --- a/client/src/modules/accounts/AccountStoreService.js +++ b/client/src/modules/accounts/AccountStoreService.js @@ -12,22 +12,22 @@ AccountStoreService.$inject = [ // Temporary service until caching API services is well designed function AccountStoreService($q, Accounts, AccountTypes, Store) { - var service = this; - var initialLoad = true; - var initTypeLoad = true; + const service = this; + let initialLoad = true; + let initTypeLoad = true; - var accounts = new Store(); - var accountTypes = new Store(); + const accounts = new Store(); + const accountTypes = new Store(); - var request = Accounts.read(null, { detailed : 1 }) - .then(function (result) { + const request = Accounts.read(null, { detailed : 1 }, true) + .then((result) => { accounts.setData(result); initialLoad = false; return accounts.data; }); - var typeRequest = AccountTypes.getAccountType() - .then(function (result) { + const typeRequest = AccountTypes.getAccountType() + .then((result) => { accountTypes.setData(result); initTypeLoad = false; return accountTypes.data; @@ -38,7 +38,7 @@ function AccountStoreService($q, Accounts, AccountTypes, Store) { function accountStore() { if (initialLoad) { - return request.then(function () { + return request.then(() => { return accounts; }); } @@ -48,7 +48,7 @@ function AccountStoreService($q, Accounts, AccountTypes, Store) { function typeStore() { if (initTypeLoad) { - return typeRequest.then(function () { + return typeRequest.then(() => { return accountTypes; }); } diff --git a/client/src/modules/accounts/accounts.service.js b/client/src/modules/accounts/accounts.service.js index 84e0f555fd..7cfa5496dd 100644 --- a/client/src/modules/accounts/accounts.service.js +++ b/client/src/modules/accounts/accounts.service.js @@ -42,9 +42,7 @@ function AccountService(Api, bhConstants, HttpCache) { .then(service.util.unwrapHttpResponse); } - const callback = (id, options) => Api.read.call(service, id, options) - .then(handleAccounts); - + const callback = (id, options) => Api.read.call(service, id, options); const fetcher = HttpCache(callback); /** @@ -54,11 +52,14 @@ function AccountService(Api, bhConstants, HttpCache) { * * @param {Number} id - the id of the account to fetch (optional). * @param {Object} options - options to be passed as query strings (optional). + * @param {Boolean} cacheBust - ignore the cache and send the HTTP request directly + * to the server. * @return {Promise} promise - resolves to either a JSON (if id provided) or * an array of JSONs. */ - function read(id, options) { - return fetcher(id, options); + function read(id, options, cacheBust = false) { + return fetcher(id, options, cacheBust) + .then(handleAccounts); } function handleAccounts(accounts) { diff --git a/test/client-unit/services/AccountService.spec.js b/test/client-unit/services/AccountService.spec.js index b56c9a3afc..d8ade2b098 100644 --- a/test/client-unit/services/AccountService.spec.js +++ b/test/client-unit/services/AccountService.spec.js @@ -1,7 +1,6 @@ /* global inject expect */ describe('AccountService', () => { - let Accounts; let $httpBackend; let $interval; @@ -85,4 +84,17 @@ describe('AccountService', () => { expect(a).to.deep.equal(resA); expect(b).to.deep.equal(resB); }); + + it('#read() will bust cached values with a third parameter', () => { + let count = 10; + + while (count--) { + // third parameter will bust the cache + Accounts.read(1, {}, true); + } + + // this would throw if too many requests were called. + expect(() => $httpBackend.flush(10)).not.to.throw(); + expect(() => $httpBackend.flush(1)).to.throw(); + }); });