Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aliasbidder fix #1652

Merged
merged 7 commits into from
Oct 6, 2017
26 changes: 22 additions & 4 deletions src/adaptermanager.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { flatten, getBidderCodes, getDefinedParams, shuffle } from './utils';
import { mapSizes } from './sizeMapping';
import { processNativeAdUnitParams, nativeAdapters } from './native';
import { newBidder } from './adapters/bidderFactory';

var utils = require('./utils.js');
var CONSTANTS = require('./constants.json');
Expand Down Expand Up @@ -194,6 +195,13 @@ function transformHeightWidth(adUnit) {
return sizesObj;
}

function getSupportedMediaTypes(bidderCode) {
let result = [];
if (exports.videoAdapters.includes(bidderCode)) result.push('video');
if (nativeAdapters.includes(bidderCode)) result.push('native');
return result;
}

exports.videoAdapters = []; // added by adapterLoader for now

exports.registerBidAdapter = function (bidAdaptor, bidderCode, {supportedMediaTypes = []} = {}) {
Expand All @@ -220,14 +228,24 @@ exports.aliasBidAdapter = function (bidderCode, alias) {

if (typeof existingAlias === 'undefined') {
var bidAdaptor = _bidderRegistry[bidderCode];

if (typeof bidAdaptor === 'undefined') {
utils.logError('bidderCode "' + bidderCode + '" is not an existing bidder.', 'adaptermanager.aliasBidAdapter');
} else {
try {
let newAdapter = new bidAdaptor.constructor();
newAdapter.setBidderCode(alias);
this.registerBidAdapter(newAdapter, alias);
let newAdapter;
let supportedMediaTypes = getSupportedMediaTypes(bidderCode);
// Have kept old code to support backward compatibilitiy.
// Remove this if loop when all adapters are supporting bidderFactory. i.e When Prebid.js is 1.0
if (bidAdaptor.constructor.prototype != Object.prototype) {
newAdapter = new bidAdaptor.constructor();
newAdapter.setBidderCode(alias);
} else {
let spec = bidAdaptor.getSpec();
newAdapter = newBidder(Object.assign({}, spec, { code: alias }));
}
this.registerBidAdapter(newAdapter, alias, {
supportedMediaTypes
});
} catch (e) {
utils.logError(bidderCode + ' bidder does not currently support aliasing.', 'adaptermanager.aliasBidAdapter');
}
Expand Down
3 changes: 3 additions & 0 deletions src/adapters/bidderFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ export function registerBidder(spec) {
*/
export function newBidder(spec) {
return Object.assign(new Adapter(spec.code), {
getSpec: function() {
Copy link
Contributor

@dbemiller dbemiller Oct 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth doing an Object.freeze on the spec here.

Otherwise someone could call getSpec() and then mutate that object, producing some absolutely horrible bugs. IMO, the burden lies on the file implementer to make sure there's no way to "misuse" the functions they make public.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

return Object.freeze(spec);
},
callBids: function(bidderRequest) {
if (!Array.isArray(bidderRequest.bids)) {
return;
Expand Down
34 changes: 34 additions & 0 deletions test/spec/unit/core/adapterManager_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import AdapterManager from 'src/adaptermanager';
import { getAdUnits } from 'test/fixtures/fixtures';
import CONSTANTS from 'src/constants.json';
import * as utils from 'src/utils';
import { registerBidder } from 'src/adapters/bidderFactory';

require('modules/appnexusAstBidAdapter');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidental import?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not accidental, i took a shortcut to test old way of aliasing adapter. Since we will be only keeping this code for a short time.

But anyway, updated test case to make it as it should be.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok... no worries either way.


const CONFIG = {
enabled: true,
Expand Down Expand Up @@ -81,4 +84,35 @@ describe('adapterManager tests', () => {
expect(spy.called).to.equal(false);
});
})

describe('aliasBidderAdaptor', function() {
let spec;
const CODE = 'sampleBidder';
beforeEach(() => {
spec = {
code: CODE,
isBidRequestValid: () => {},
buildRequests: () => {},
interpretResponse: () => {},
getUserSyncs: () => {}
};
});

// Note remove this test case once Prebid is 1.0
it('should add alias to registry', () => {
const bidderCode = 'appnexusAst';
const alias = 'testalias';
AdapterManager.aliasBidAdapter(bidderCode, alias);
expect(AdapterManager.bidderRegistry).to.have.property(alias);
});

it('should add alias to registry when original adapter is using bidderFactory', function() {
let thisSpec = Object.assign(spec, { supportedMediaTypes: ['video'] });
registerBidder(thisSpec);
const alias = 'aliasBidder';
AdapterManager.aliasBidAdapter(CODE, alias);
expect(AdapterManager.bidderRegistry).to.have.property(alias);
expect(AdapterManager.videoAdapters).to.include(alias);
});
});
});