Skip to content

Commit

Permalink
Merge #3111
Browse files Browse the repository at this point in the history
3111: Perf: implement HttpCache service to cache Accounts.read() call r=jniles a=jniles

**Original Text**
~~This PR introduces a new factory function for debouncing function calls.  The suggested ng-debounce library did not use promises and was cumbersome to use, thus I have implemented my own based on a SO issue.  The `Accounts.read()` method has been debounced by 2000 milliseconds to allow for things like `bhAccountSelects` to benefit from the same HTTP request instead of making one each.~~


**Updated Information**
This Pull request implements a new service called `HttpCache` that should transparently wrap any HTTP requests and ensure that the same function call will return the same result.  This is useful for smaller, more frequently called routes.

_Basic Usage:_
```js
// a GET HTTP request to some url
function read(id, options) {
  return $http.get(/*...*/);
}

// to proxy, simply wrap the function in HttpCache
const proxy = HttpCache((id, options) => read(id, options));

// You can now call proxy as
proxy(1, { limit : 10 })
  .then(() => /*... */);
```

For uncached requests, the behavior is the same a raw HTTP request.  For those that have been called multiple times, the web page no longer needs to send an extra HTTP request.

If you want to skip the cache (and bust it), simply pass `true` as the third parameter.

Closes #2892.

Co-authored-by: Jonathan Niles <jonathanwniles@gmail.com>
Co-authored-by: Jonathan Niles <jonathanwniles@imaworldhealth.org>
  • Loading branch information
3 people committed Sep 11, 2018
2 parents e070e12 + 2ffd167 commit 4643785
Show file tree
Hide file tree
Showing 7 changed files with 223 additions and 38 deletions.
74 changes: 74 additions & 0 deletions client/src/js/services/HttpCacheService.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
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(id, parameters, cacheBust = false) {
const key = serialize(id, parameters);

// if the cache has been populated return the value from memory
if (cache.has(key) && !cacheBust) {
return cache.get(key);
}

// call the callback to get the result and cache it
const promise = callback(id, parameters);
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;
}
22 changes: 11 additions & 11 deletions client/src/modules/accounts/AccountStoreService.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -38,7 +38,7 @@ function AccountStoreService($q, Accounts, AccountTypes, Store) {

function accountStore() {
if (initialLoad) {
return request.then(function () {
return request.then(() => {
return accounts;
});
}
Expand All @@ -48,7 +48,7 @@ function AccountStoreService($q, Accounts, AccountTypes, Store) {

function typeStore() {
if (initTypeLoad) {
return typeRequest.then(function () {
return typeRequest.then(() => {
return accountTypes;
});
}
Expand Down
14 changes: 10 additions & 4 deletions client/src/modules/accounts/accounts.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ angular.module('bhima.services')
.service('AccountService', AccountService);

AccountService.$inject = [
'PrototypeApiService', 'bhConstants',
'PrototypeApiService', 'bhConstants', 'HttpCacheService',
];

/**
* Account Service
*
* A service wrapper for the /accounts HTTP endpoint.
*/
function AccountService(Api, bhConstants) {
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 = read;
service.label = label;

Expand Down Expand Up @@ -41,18 +42,23 @@ function AccountService(Api, bhConstants) {
.then(service.util.unwrapHttpResponse);
}

const callback = (id, options) => Api.read.call(service, id, options);
const fetcher = HttpCache(callback);

/**
* 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
* of objects should be expected.
*
* @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 Api.read.call(this, id, options)
function read(id, options, cacheBust = false) {
return fetcher(id, options, cacheBust)
.then(handleAccounts);
}

Expand Down
45 changes: 24 additions & 21 deletions client/src/modules/cash/modals/transfer-modal.ctrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
];

/**
Expand All @@ -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);
Expand All @@ -46,35 +49,35 @@ 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
vm.voucher.description('CASH.TRANSFER.DESCRIPTION', {
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;
}, {});
Expand All @@ -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);

Expand All @@ -108,19 +111,19 @@ 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;
})
.catch(Notify.handleError);

// look up the cash account
Accounts.read(accounts.account_id)
.then(function (account) {
.then((account) => {
account.hrlabel = Accounts.label(account);
vm.cashAccount = account;
})
Expand Down
1 change: 0 additions & 1 deletion server/config/express.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down

0 comments on commit 4643785

Please sign in to comment.