Skip to content

Commit

Permalink
Do not load external js if renderer defined on adUnit (prebid#3284)
Browse files Browse the repository at this point in the history
* do not load external js if renderer defined on adUnit

* Added unit test
  • Loading branch information
jaiminpanchal27 authored and Pedro López Jiménez committed Mar 18, 2019
1 parent 82c2b0a commit 1dfe02f
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 79 deletions.
3 changes: 3 additions & 0 deletions modules/appnexusBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ function newRenderer(adUnitCode, rtbBid, rendererOptions = {}) {
url: rtbBid.renderer_url,
config: rendererOptions,
loaded: false,
adUnitCode
});

try {
Expand Down Expand Up @@ -238,6 +239,7 @@ function newRenderer(adUnitCode, rtbBid, rendererOptions = {}) {
* @return Bid
*/
function newBid(serverBid, rtbBid, bidderRequest) {
const bidRequest = utils.getBidRequest(serverBid.uuid, [bidderRequest]);
const bid = {
requestId: serverBid.uuid,
cpm: rtbBid.cpm,
Expand All @@ -246,6 +248,7 @@ function newBid(serverBid, rtbBid, bidderRequest) {
currency: 'USD',
netRevenue: true,
ttl: 300,
adUnitCode: bidRequest.adUnitCode,
appnexus: {
buyerMemberId: rtbBid.buyer_member_id,
dealPriority: rtbBid.deal_priority,
Expand Down
23 changes: 18 additions & 5 deletions src/Renderer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { loadScript } from './adloader';
import * as utils from './utils';
import find from 'core-js/library/fn/array/find';

/**
* @typedef {object} Renderer
Expand All @@ -10,7 +11,7 @@ import * as utils from './utils';
*/

export function Renderer(options) {
const { url, config, id, callback, loaded } = options;
const { url, config, id, callback, loaded, adUnitCode } = options;
this.url = url;
this.config = config;
this.handlers = {};
Expand All @@ -35,12 +36,16 @@ export function Renderer(options) {
this.process();
});

// we expect to load a renderer url once only so cache the request to load script
loadScript(url, this.callback, true);
if (!isRendererDefinedOnAdUnit(adUnitCode)) {
// we expect to load a renderer url once only so cache the request to load script
loadScript(url, this.callback, true);
} else {
utils.logWarn(`External Js not loaded by Renderer since renderer url and callback is already defined on adUnit ${adUnitCode}`);
}
}

Renderer.install = function({ url, config, id, callback, loaded }) {
return new Renderer({ url, config, id, callback, loaded });
Renderer.install = function({ url, config, id, callback, loaded, adUnitCode }) {
return new Renderer({ url, config, id, callback, loaded, adUnitCode });
};

Renderer.prototype.getConfig = function() {
Expand Down Expand Up @@ -94,3 +99,11 @@ export function isRendererRequired(renderer) {
export function executeRenderer(renderer, bid) {
renderer.render(bid);
}

function isRendererDefinedOnAdUnit(adUnitCode) {
const adUnits = $$PREBID_GLOBAL$$.adUnits;
const adUnit = find(adUnits, adUnit => {
return adUnit.code === adUnitCode;
});
return !!(adUnit && adUnit.renderer && adUnit.renderer.url && adUnit.renderer.render);
}
30 changes: 26 additions & 4 deletions test/spec/modules/appnexusBidAdapter_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -463,12 +463,18 @@ describe('AppNexusAdapter', function () {
'currency': 'USD',
'ttl': 300,
'netRevenue': true,
'adUnitCode': 'code',
'appnexus': {
'buyerMemberId': 958
}
}
];
let bidderRequest;
let bidderRequest = {
bids: [{
bidId: '3db3773286ee59',
adUnitCode: 'code'
}]
}
let result = spec.interpretResponse({ body: response }, {bidderRequest});
expect(Object.keys(result[0])).to.have.members(Object.keys(expectedResponse[0]));
});
Expand Down Expand Up @@ -505,7 +511,12 @@ describe('AppNexusAdapter', function () {
}]
}]
};
let bidderRequest;
let bidderRequest = {
bids: [{
bidId: '84ab500420319d',
adUnitCode: 'code'
}]
}

let result = spec.interpretResponse({ body: response }, {bidderRequest});
expect(result[0]).to.have.property('vastUrl');
Expand Down Expand Up @@ -538,7 +549,12 @@ describe('AppNexusAdapter', function () {
},
'impression_trackers': ['http://example.com'],
};
let bidderRequest;
let bidderRequest = {
bids: [{
bidId: '3db3773286ee59',
adUnitCode: 'code'
}]
}

let result = spec.interpretResponse({ body: response1 }, {bidderRequest});
expect(result[0].native.title).to.equal('Native Creative');
Expand All @@ -554,6 +570,7 @@ describe('AppNexusAdapter', function () {

const bidderRequest = {
bids: [{
bidId: '3db3773286ee59',
renderer: {
options: {
adText: 'configured'
Expand All @@ -573,7 +590,12 @@ describe('AppNexusAdapter', function () {
responseWithDeal.tags[0].ads[0].deal_priority = 'high';
responseWithDeal.tags[0].ads[0].deal_code = '123';

let bidderRequest;
let bidderRequest = {
bids: [{
bidId: '3db3773286ee59',
adUnitCode: 'code'
}]
}
let result = spec.interpretResponse({ body: responseWithDeal }, {bidderRequest});
expect(Object.keys(result[0].appnexus)).to.include.members(['buyerMemberId', 'dealPriority', 'dealCode']);
});
Expand Down
175 changes: 105 additions & 70 deletions test/spec/renderer_spec.js
Original file line number Diff line number Diff line change
@@ -1,96 +1,131 @@
import { expect } from 'chai';
import { Renderer } from 'src/Renderer';

describe('Renderer: A renderer installed on a bid response', function () {
let testRenderer1;
let testRenderer2;
let spyRenderFn;
let spyEventHandler;

beforeEach(function () {
testRenderer1 = Renderer.install({
url: 'https://httpbin.org/post',
config: { test: 'config1' },
id: 1
import * as utils from 'src/utils';

describe('Renderer', function () {
describe('Renderer: A renderer installed on a bid response', function () {
let testRenderer1;
let testRenderer2;
let spyRenderFn;
let spyEventHandler;

beforeEach(function () {
testRenderer1 = Renderer.install({
url: 'https://httpbin.org/post',
config: { test: 'config1' },
id: 1
});
testRenderer2 = Renderer.install({
url: 'https://httpbin.org/post',
config: { test: 'config2' },
id: 2
});

spyRenderFn = sinon.spy();
spyEventHandler = sinon.spy();
});
testRenderer2 = Renderer.install({
url: 'https://httpbin.org/post',
config: { test: 'config2' },
id: 2

it('is an instance of Renderer', function () {
expect(testRenderer1 instanceof Renderer).to.equal(true);
});

spyRenderFn = sinon.spy();
spyEventHandler = sinon.spy();
});
it('has expected properties ', function () {
expect(testRenderer1.url).to.equal('https://httpbin.org/post');
expect(testRenderer1.config).to.deep.equal({ test: 'config1' });
expect(testRenderer1.id).to.equal(1);
});

it('is an instance of Renderer', function () {
expect(testRenderer1 instanceof Renderer).to.equal(true);
});
it('returns config from getConfig method', function () {
expect(testRenderer1.getConfig()).to.deep.equal({ test: 'config1' });
expect(testRenderer2.getConfig()).to.deep.equal({ test: 'config2' });
});

it('has expected properties ', function () {
expect(testRenderer1.url).to.equal('https://httpbin.org/post');
expect(testRenderer1.config).to.deep.equal({ test: 'config1' });
expect(testRenderer1.id).to.equal(1);
});
it('sets a render function with setRender method', function () {
testRenderer1.setRender(spyRenderFn);
expect(typeof testRenderer1.render).to.equal('function');
testRenderer1.render();
expect(spyRenderFn.called).to.equal(true);
});

it('returns config from getConfig method', function () {
expect(testRenderer1.getConfig()).to.deep.equal({ test: 'config1' });
expect(testRenderer2.getConfig()).to.deep.equal({ test: 'config2' });
});
it('sets event handlers with setEventHandlers method and handles events with installed handlers', function () {
testRenderer1.setEventHandlers({
testEvent: spyEventHandler
});

it('sets a render function with setRender method', function () {
testRenderer1.setRender(spyRenderFn);
expect(typeof testRenderer1.render).to.equal('function');
testRenderer1.render();
expect(spyRenderFn.called).to.equal(true);
});
expect(testRenderer1.handlers).to.deep.equal({
testEvent: spyEventHandler
});

it('sets event handlers with setEventHandlers method and handles events with installed handlers', function () {
testRenderer1.setEventHandlers({
testEvent: spyEventHandler
testRenderer1.handleVideoEvent({ id: 1, eventName: 'testEvent' });
expect(spyEventHandler.called).to.equal(true);
});

expect(testRenderer1.handlers).to.deep.equal({
testEvent: spyEventHandler
it('pushes commands to queue if renderer is not loaded', function () {
testRenderer1.loaded = false;
testRenderer1.push(spyRenderFn);
expect(testRenderer1.cmd.length).to.equal(1);

// clear queue for next tests
testRenderer1.cmd = [];
});

testRenderer1.handleVideoEvent({ id: 1, eventName: 'testEvent' });
expect(spyEventHandler.called).to.equal(true);
});
it('fires commands immediately if the renderer is loaded', function () {
const func = sinon.spy();

it('pushes commands to queue if renderer is not loaded', function () {
testRenderer1.loaded = false;
testRenderer1.push(spyRenderFn);
expect(testRenderer1.cmd.length).to.equal(1);
testRenderer1.loaded = true;
testRenderer1.push(func);

// clear queue for next tests
testRenderer1.cmd = [];
});
expect(testRenderer1.cmd.length).to.equal(0);

it('fires commands immediately if the renderer is loaded', function () {
const func = sinon.spy();
sinon.assert.calledOnce(func);
});

testRenderer1.loaded = true;
testRenderer1.push(func);
it('processes queue by calling each function in queue', function () {
testRenderer1.loaded = false;
const func1 = sinon.spy();
const func2 = sinon.spy();

expect(testRenderer1.cmd.length).to.equal(0);
testRenderer1.push(func1);
testRenderer1.push(func2);
expect(testRenderer1.cmd.length).to.equal(2);

sinon.assert.calledOnce(func);
});
testRenderer1.process();

it('processes queue by calling each function in queue', function () {
testRenderer1.loaded = false;
const func1 = sinon.spy();
const func2 = sinon.spy();
sinon.assert.calledOnce(func1);
sinon.assert.calledOnce(func2);
expect(testRenderer1.cmd.length).to.equal(0);
});
});

testRenderer1.push(func1);
testRenderer1.push(func2);
expect(testRenderer1.cmd.length).to.equal(2);
describe('3rd party renderer', function () {
let adUnitsOld;
let utilsSpy;
before(function () {
adUnitsOld = $$PREBID_GLOBAL$$.adUnits;
utilsSpy = sinon.spy(utils, 'logWarn');
});

testRenderer1.process();
after(function() {
$$PREBID_GLOBAL$$.adUnits = adUnitsOld;
utilsSpy.restore();
});

sinon.assert.calledOnce(func1);
sinon.assert.calledOnce(func2);
expect(testRenderer1.cmd.length).to.equal(0);
it('should not load renderer and log warn message', function() {
$$PREBID_GLOBAL$$.adUnits = [{
code: 'video1',
renderer: {
url: 'http://cdn.adnxs.com/renderer/video/ANOutstreamVideo.js',
render: sinon.spy()
}
}]

let testRenderer = Renderer.install({
url: 'https://httpbin.org/post',
config: { test: 'config1' },
id: 1,
adUnitCode: 'video1'
});
expect(utilsSpy.callCount).to.equal(1);
});
});
});

0 comments on commit 1dfe02f

Please sign in to comment.