Removed the shifty API. Updated unit tests. Renamed a property becaus…

…e Ive messed it up multiple times myself already during testing, expecting it to have a different name.

integrationExamples/gpt/video_test.html

@@ -49,7 +49,7 @@
         pbjs.requestBids({
           timeout : 700,
           bidsBackHandler : function(bids) {
-            var videoUrl = pbjs.adServer.buildVideoAdUrl({
+            var videoUrl = pbjs.adServers.dfp.buildVideoUrl({
               adUnit: videoAdUnit,
               params: {
                 iu: '/19968336/prebid_cache_video_adunit'

modules/dfpAdServerVideo.js

@@ -83,5 +83,5 @@ export default function buildDfpVideoUrl(options) {
 }
 
 registerVideoSupport('dfp', {
-  buildVideoAdUrl: buildDfpVideoUrl
+  buildVideoUrl: buildDfpVideoUrl
 });

src/adServerManager.js

@@ -1,17 +1,14 @@
 import { getGlobal } from 'src/prebidGlobal';
+import { logWarn } from 'src/utils';
 
 const prebid = getGlobal();
 
 /**
- * This file serves as the plugin point for adserver module functionality.
+ * This file defines the plugin points in prebid-core for AdServer-specific functionality.
  *
- * Ad Server modules should call {@link registerAdserver} to connect into prebid-core.
- *
- * If publishers bundle Prebid with a single Ad Server module, they'll be able to interact with it
- * through the properties and functions on `pbjs.adserver`.
- *
- * If publishers bundle with more than one Ad Server, they'll need to interact with them through
- * `pbjs.adservers.{adServerCode}`, where the adServerCode is defined by the ad server itself.
+ * Its main job is to expose functions for AdServer modules to append functionality to the Prebid public API.
+ * For a given Ad Server with name "adServerName", these functions will only change the API in the
+ * $$PREBID_GLOBAL$$.adServers[adServerName] namespace.
  */
 
 /**
@@ -27,64 +24,31 @@ const prebid = getGlobal();
  * @param {CachedVideoBid} bid The winning Bid which the ad server should show, assuming it beats out
  *   the competition.
  *
- * @param [Object] options Miscellaneous Ad-Server-specific options. This object will have different
- *   properties depending on the specific ad server supported. For more information, see the docs
- *   inside the ad server module you're using.
+ * @param {Object} options Options required by the Ad Server to make a valid AdServer URL.
+ *   This object will have different properties depending on the specific ad server supported.
+ *   For more information, see the docs inside the ad server module you're supporting.
  *
  * @return {string} A URL which can be passed into the Video player to play an ad.
- *   This should call the Ad Server, and return the ad which should be played (letting the
- *   given bid compete alongside the rest of the demand).
  */
 
 /**
  * @typedef {Object} VideoSupport
  *
- * @property {string} name A name which identifies this adserver. Note that this contributes to
- *   the user-facing API. For example, if the adserver name is 'dfp', and the Prebid bundle contains two
- *   or more ad server modules, then publishers will access its functions through 'pbjs.adservers.dfp'.
- *
  * @function {VideoAdUrlBuilder} buildVideoAdUrl
  */
 
-/**
- * Prepares a namespace on object so that utility functions can be added to it.
- *
- * If this is the only name we've seen, attach functionality to $$PREBID_GLOBAL$$.adServer.
- * If we've seen any other names, attach functionality to $$PREBID_GLOBAL$$.adServers[name].
- *
- * @param {object} object The object where this function should manage namespaces.
- * @param {string} name The name for this ad server.
- * @return {object} An object where functions for dealing with this ad server can be added.
- */
-function prepareNamespaceIn(object, name) {
-  if (object.adServer && object.adServer.name !== name) {
-    object.adServers = { };
-    object.adServers[object.adServer.name] = object.adServer;
-    delete object.adServer.name;
-    delete object.adServer;
-  }
-
-  if (object.adServer) {
-    return object.adServer;
-  }
-  if (object.adServers) {
-    if (!object.adServers[name]) {
-      object.adServers[name] = { };
-    }
-    return object.adServers[name];
-  }
-  else {
-    object.adServer = { name };
-    return object.adServer;
-  }
-}
-
 /**
  * Enable video support for the Ad Server.
  *
  * @property {string} name The identifying name for this adserver.
  * @property {VideoSupport} videoSupport An object with the functions needed to support video in Prebid.
  */
 export function registerVideoSupport(name, videoSupport) {
-  prepareNamespaceIn(prebid, name).buildVideoAdUrl = videoSupport.buildVideoAdUrl;
+  prebid.adServers = prebid.adServers || { };
+  prebid.adServers[name] = prebid.adServers[name] || { };
+  if (prebid.adServers[name].buildVideoUrl) {
+    logWarn(`Multiple calls to registerVideoSupport for AdServer ${name}. Expect surprising behavior.`);
+    return;
+  }
+  prebid.adServers[name].buildVideoUrl = videoSupport.buildVideoUrl;
 }

test/spec/unit/adServerManager_spec.js

@@ -6,38 +6,25 @@ const prebid = getGlobal();
 
 describe('The ad server manager', () => {
   beforeEach(() => {
-    delete prebid.adServer;
     delete prebid.adServers;
   });
 
-  it('should register video support to the proper place when one ad server is loaded', () => {
+  it('should register video support to the proper place on the API', () => {
     function videoSupport() { }
-    registerVideoSupport('dfp', { buildVideoAdUrl: videoSupport });
+    registerVideoSupport('dfp', { buildVideoUrl: videoSupport });
 
-    expect(prebid).to.have.property('adServer');
-    expect(prebid.adServer).to.have.property('name', 'dfp');
-
-    expect(prebid.adServer).to.have.property('buildVideoAdUrl');
-    expect(prebid.adServer.buildVideoAdUrl).to.equal(videoSupport);
+    expect(prebid).to.have.property('adServers');
+    expect(prebid.adServers).to.have.property('dfp');
+    expect(prebid.adServers.dfp).to.have.property('buildVideoUrl', videoSupport);
   });
 
-  it('should change the namespaces when two ad servers are loaded', () => {
-    function dfpVideoSupport() { }
-    function astVideoSupport() { }
-    registerVideoSupport('ast', { buildVideoAdUrl: astVideoSupport });
-    registerVideoSupport('dfp', { buildVideoAdUrl: dfpVideoSupport });
+  it('should keep the first function when we try to add a second', () => {
+    function videoSupport() { }
+    registerVideoSupport('dfp', { buildVideoUrl: videoSupport });
+    registerVideoSupport('dfp', { buildVideoUrl: function noop() { } });
 
-    expect(prebid).to.not.have.property('adServer');
     expect(prebid).to.have.property('adServers');
-
-    expect(prebid.adServers).to.have.property('ast');
-    expect(prebid.adServers.ast).to.have.property('buildVideoAdUrl');
     expect(prebid.adServers).to.have.property('dfp');
-    expect(prebid.adServers.dfp).to.have.property('buildVideoAdUrl');
-    expect(prebid.adServers.ast).not.to.have.property('name');
-    expect(prebid.adServers.dfp).not.to.have.property('name');
-
-    expect(prebid.adServers.dfp.buildVideoAdUrl).to.equal(dfpVideoSupport);
-    expect(prebid.adServers.ast.buildVideoAdUrl).to.equal(astVideoSupport);
+    expect(prebid.adServers.dfp).to.have.property('buildVideoUrl', videoSupport);
   });
 });