New hooks API (replaces monkey-patching for currency)

modules/currency.js

@@ -13,9 +13,6 @@ var conversionCache = {};
 var currencyRatesLoaded = false;
 var adServerCurrency = 'USD';
 
-// Used as reference to the original bidmanager.addBidResponse
-var originalBidResponse;
-
 export var currencySupportEnabled = false;
 export var currencyRates = {};
 var bidderCurrencyDefault = {};
@@ -81,12 +78,9 @@ function initCurrency(url) {
   conversionCache = {};
   currencySupportEnabled = true;
 
-  if (!originalBidResponse) {
-    utils.logInfo('Installing addBidResponse decorator for currency module', arguments);
+  utils.logInfo('Installing addBidResponse decorator for currency module', arguments);
 
-    originalBidResponse = bidmanager.addBidResponse;
-    bidmanager.addBidResponse = addBidResponseDecorator(bidmanager.addBidResponse);
-  }
+  bidmanager.addBidResponse.addHook(addBidResponseHook, 100);
 
   if (!currencyRates.conversions) {
     ajax(url, function (response) {
@@ -103,12 +97,9 @@ function initCurrency(url) {
 }
 
 function resetCurrency() {
-  if (originalBidResponse) {
-    utils.logInfo('Uninstalling addBidResponse decorator for currency module', arguments);
+  utils.logInfo('Uninstalling addBidResponse decorator for currency module', arguments);
 
-    bidmanager.addBidResponse = originalBidResponse;
-    originalBidResponse = undefined;
-  }
+  bidmanager.addBidResponse.removeHook(addBidResponseHook);
 
   adServerCurrency = 'USD';
   conversionCache = {};
@@ -118,37 +109,35 @@ function resetCurrency() {
   bidderCurrencyDefault = {};
 }
 
-export function addBidResponseDecorator(fn) {
-  return function(adUnitCode, bid) {
-    if (!bid) {
-      return fn.apply(this, arguments); // if no bid, call original and let it display warnings
-    }
+export function addBidResponseHook(adUnitCode, bid, fn) {
+  if (!bid) {
+    return fn.apply(this, arguments); // if no bid, call original and let it display warnings
+  }
 
-    let bidder = bid.bidderCode || bid.bidder;
-    if (bidderCurrencyDefault[bidder]) {
-      let currencyDefault = bidderCurrencyDefault[bidder];
-      if (bid.currency && currencyDefault !== bid.currency) {
-        utils.logWarn(`Currency default '${bidder}: ${currencyDefault}' ignored. adapter specified '${bid.currency}'`);
-      } else {
-        bid.currency = currencyDefault;
-      }
+  let bidder = bid.bidderCode || bid.bidder;
+  if (bidderCurrencyDefault[bidder]) {
+    let currencyDefault = bidderCurrencyDefault[bidder];
+    if (bid.currency && currencyDefault !== bid.currency) {
+      utils.logWarn(`Currency default '${bidder}: ${currencyDefault}' ignored. adapter specified '${bid.currency}'`);
+    } else {
+      bid.currency = currencyDefault;
     }
+  }
 
-    // default to USD if currency not set
-    if (!bid.currency) {
-      utils.logWarn('Currency not specified on bid.  Defaulted to "USD"');
-      bid.currency = 'USD';
-    }
+  // default to USD if currency not set
+  if (!bid.currency) {
+    utils.logWarn('Currency not specified on bid.  Defaulted to "USD"');
+    bid.currency = 'USD';
+  }
 
-    // execute immediately if the bid is already in the desired currency
-    if (bid.currency === adServerCurrency) {
-      return fn.apply(this, arguments);
-    }
+  // execute immediately if the bid is already in the desired currency
+  if (bid.currency === adServerCurrency) {
+    return fn.apply(this, arguments);
+  }
 
-    bidResponseQueue.push(wrapFunction(fn, this, arguments));
-    if (!currencySupportEnabled || currencyRatesLoaded) {
-      processBidResponseQueue();
-    }
+  bidResponseQueue.push(wrapFunction(fn, this, arguments));
+  if (!currencySupportEnabled || currencyRatesLoaded) {
+    processBidResponseQueue();
   }
 }
 

src/bidmanager.js

@@ -5,6 +5,7 @@ import { isValidVideoBid } from './video';
 import { getCacheUrl, store } from './videoCache';
 import { Renderer } from 'src/Renderer';
 import { config } from 'src/config';
+import { createHook } from 'src/hook';
 
 var CONSTANTS = require('./constants.json');
 var AUCTION_END = CONSTANTS.EVENTS.AUCTION_END;
@@ -82,7 +83,7 @@ exports.bidsBackAll = function () {
 /*
  *   This function should be called to by the bidder adapter to register a bid response
  */
-exports.addBidResponse = function (adUnitCode, bid) {
+exports.addBidResponse = createHook('asyncSeries', function (adUnitCode, bid) {
   if (isValid()) {
     prepareBidForAuction();
 
@@ -250,7 +251,7 @@ exports.addBidResponse = function (adUnitCode, bid) {
       doCallbacksIfNeeded();
     }
   }
-};
+});
 
 function getKeyValueTargetingPairs(bidderCode, custBidObj) {
   var keyValues = {};

src/hook.js

@@ -0,0 +1,78 @@
+
+/**
+ * @typedef {function} HookedFunction
+ * @property {function(function(), [number])} addHook A method that takes a new function to attach as a hook
+ *  to the HookedFunction
+ * @property {function(function())} removeHook A method to remove attached hooks
+ */
+
+/**
+ * A map of global hook methods to allow easy extension of hooked functions that are intended to be extended globally
+ * @type {{}}
+ */
+export const hooks = {};
+
+/**
+ * A utility function for allowing a regular function to be extensible with additional hook functions
+ * @param {string} type The method for applying all attached hooks when this hooked function is called
+ * @param {function()} fn The function to make hookable
+ * @param {string} hookName If provided this allows you to register a name for a global hook to have easy access to
+ *  the addHook and removeHook methods for that hook (which are usually accessed as methods on the function itself)
+ * @returns {HookedFunction} A new function that implements the HookedFunction interface
+ */
+export function createHook(type, fn, hookName) {
+  let _hooks = [{fn, priority: 0}];
+
+  let types = {
+    sync: function(...args) {
+      _hooks.forEach(hook => {
+        hook.fn.apply(this, args);
+      });
+    },
+    asyncSeries: function(...args) {
+      let curr = 0;
+
+      const asyncSeriesNext = (...args) => {
+        let hook = _hooks[++curr];
+        if (typeof hook === 'object' && typeof hook.fn === 'function') {
+          return hook.fn.apply(this, args.concat(asyncSeriesNext))
+        }
+      };
+
+      return _hooks[curr].fn.apply(this, args.concat(asyncSeriesNext));
+    }
+  };
+
+  if (!types[type]) {
+    throw 'invalid hook type';
+  }
+
+  let methods = {
+    addHook: function(fn, priority = 10) {
+      if (typeof fn === 'function') {
+        _hooks.push({
+          fn,
+          priority: priority
+        });
+
+        _hooks.sort((a, b) => b.priority - a.priority);
+      }
+    },
+    removeHook: function(removeFn) {
+      _hooks = _hooks.filter(hook => hook.fn === fn || hook.fn !== removeFn);
+    }
+  };
+
+  if (typeof hookName === 'string') {
+    hooks[hookName] = methods;
+  }
+
+  function hookedFn(...args) {
+    if (_hooks.length === 0) {
+      return fn.apply(this, args);
+    }
+    return types[type].apply(this, args);
+  }
+
+  return Object.assign(hookedFn, methods);
+}

test/spec/hook_spec.js

@@ -0,0 +1,151 @@
+
+import { expect } from 'chai';
+import { createHook, hooks } from 'src/hook';
+
+describe('the hook module', () => {
+  let sandbox;
+
+  beforeEach(() => {
+    sandbox = sinon.sandbox.create();
+  });
+
+  afterEach(() => {
+    sandbox.restore();
+  });
+
+  it('should call all sync hooks attached to a function', () => {
+    let called = [];
+    let calledWith;
+
+    let testFn = () => {
+      called.push(testFn);
+    };
+    let testHook = (...args) => {
+      called.push(testHook);
+      calledWith = args;
+    };
+    let testHook2 = () => {
+      called.push(testHook2);
+    };
+    let testHook3 = () => {
+      called.push(testHook3);
+    };
+
+    let hookedTestFn = createHook('sync', testFn, 'testHook');
+
+    hookedTestFn.addHook(testHook, 50);
+    hookedTestFn.addHook(testHook2, 100);
+
+    // make sure global test hooks work as well (with default priority)
+    hooks['testHook'].addHook(testHook3);
+
+    hookedTestFn(1, 2, 3);
+
+    expect(called).to.deep.equal([
+      testHook2,
+      testHook,
+      testHook3,
+      testFn
+    ]);
+
+    expect(calledWith).to.deep.equal([1, 2, 3]);
+
+    called = [];
+
+    hookedTestFn.removeHook(testHook);
+    hooks['testHook'].removeHook(testHook3);
+
+    hookedTestFn(1, 2, 3);
+
+    expect(called).to.deep.equal([
+      testHook2,
+      testFn
+    ]);
+  });
+
+  it('should allow context to be passed to hooks, but keep bound contexts', () => {
+    let context;
+    let fn = function() {
+      context = this;
+    };
+
+    let boundContext = {};
+    let calledBoundContext;
+    let hook = function() {
+      calledBoundContext = this;
+    }.bind(boundContext);
+
+    let hookFn = createHook('sync', fn);
+    hookFn.addHook(hook);
+
+    let newContext = {};
+    hookFn.bind(newContext)();
+
+    expect(context).to.equal(newContext);
+    expect(calledBoundContext).to.equal(boundContext);
+  });
+
+  describe('asyncSeries', () => {
+    it('should call function as normal if no hooks attached', () => {
+      let fn = sandbox.spy();
+      let hookFn = createHook('asyncSeries', fn);
+
+      hookFn(1);
+
+      expect(fn.calledOnce).to.equal(true);
+      expect(fn.firstCall.args[0]).to.equal(1);
+    });
+
+    it('should call hooks correctly applied in asyncSeries', () => {
+      let called = [];
+
+      let testFn = (called) => {
+        called.push(testFn);
+      };
+      let testHook = (called, next) => {
+        called.push(testHook);
+        next(called);
+      };
+      let testHook2 = (called, next) => {
+        called.push(testHook2);
+        next(called);
+      };
+
+      let hookedTestFn = createHook('asyncSeries', testFn);
+      hookedTestFn.addHook(testHook);
+      hookedTestFn.addHook(testHook2);
+
+      hookedTestFn(called);
+
+      expect(called).to.deep.equal([
+        testHook,
+        testHook2,
+        testFn
+      ]);
+    });
+
+    it('should allow context to be passed to hooks, but keep bound contexts', () => {
+      let context;
+      let fn = function() {
+        context = this;
+      };
+
+      let boundContext1 = {};
+      let calledBoundContext1;
+      let hook1 = function(next) {
+        calledBoundContext1 = this;
+        next()
+      }.bind(boundContext1);
+
+      let hookFn = createHook('asyncSeries', fn);
+      hookFn.addHook(hook1);
+
+      let newContext = {};
+      hookFn = hookFn.bind(newContext);
+      hookFn();
+
+      expect(context).to.equal(newContext);
+      expect(calledBoundContext1).to.equal(boundContext1);
+    });
+  });
+});