Skip to content

Commit

Permalink
deprecate support for adUnit.sizes for pubs (#4458)
Browse files Browse the repository at this point in the history
* deprecate support for adUnit.sizes for pubs

* remove commented code
  • Loading branch information
jsnellbaker authored and jaiminpanchal27 committed Dec 6, 2019
1 parent 843258f commit 6729336
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 41 deletions.
6 changes: 5 additions & 1 deletion integrationExamples/gpt/prebidServer_example.html
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@
pbjs.que.push(function() {
var adUnits = [{
code: 'div-gpt-ad-1460505748561-0',
sizes: [[300, 250]],
mediaTypes: {
banner: {
sizes: [[300, 250]]
}
},
bids: [
{
bidder: 'appnexus',
Expand Down
9 changes: 1 addition & 8 deletions modules/prebidServerBidAdapter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,12 +425,6 @@ const OPEN_RTB_PROTOCOL = {
});

let mediaTypes = {};
// default to banner if mediaTypes isn't defined
if (!(nativeParams || videoParams || bannerParams)) {
const sizeObjects = adUnit.sizes.map(size => ({ w: size[0], h: size[1] }));
mediaTypes['banner'] = {format: sizeObjects};
}

if (bannerParams && bannerParams.sizes) {
const sizes = utils.parseSizesInput(bannerParams.sizes);

Expand Down Expand Up @@ -811,8 +805,7 @@ export function PrebidServer() {

// at this point ad units should have a size array either directly or mapped so filter for that
const validAdUnits = adUnits.filter(unit =>
(unit.sizes && unit.sizes.length) ||
(unit.mediaTypes && unit.mediaTypes.native)
unit.mediaTypes && (unit.mediaTypes.native || (unit.mediaTypes.banner && unit.mediaTypes.banner.sizes) || (unit.mediaTypes.video && unit.mediaTypes.video.playerSize))
);

// in case config.bidders contains invalid bidders, we only process those we sent requests for
Expand Down
56 changes: 33 additions & 23 deletions src/prebid.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,43 +69,53 @@ function setRenderSize(doc, width, height) {
}

export const checkAdUnitSetup = hook('sync', function (adUnits) {
adUnits.forEach((adUnit) => {
function validateSizes(sizes, targLength) {
let cleanSizes = [];
if (utils.isArray(sizes) && ((targLength) ? sizes.length === targLength : sizes.length > 0)) {
// check if an array of arrays or array of numbers
if (sizes.every(sz => isArrayOfNums(sz, 2))) {
cleanSizes = sizes;
} else if (isArrayOfNums(sizes, 2)) {
cleanSizes.push(sizes);
}
}
return cleanSizes;
}

return adUnits.filter(adUnit => {
const mediaTypes = adUnit.mediaTypes;
const normalizedSize = utils.getAdUnitSizes(adUnit);

if (mediaTypes && mediaTypes.banner) {
const banner = mediaTypes.banner;
if (banner.sizes) {
// make sure we always send [[h,w]] format
banner.sizes = normalizedSize;
adUnit.sizes = normalizedSize;
if (!mediaTypes || Object.keys(mediaTypes).length === 0) {
utils.logError(`Detected adUnit.code '${adUnit.code}' did not have a 'mediaTypes' object defined. This is a required field for the auction, so this adUnit has been removed.`);
return false;
}

if (mediaTypes.banner) {
const bannerSizes = validateSizes(mediaTypes.banner.sizes);
if (bannerSizes.length > 0) {
mediaTypes.banner.sizes = bannerSizes;
// TODO eventually remove this internal copy once we're ready to deprecate bidders from reading this adUnit.sizes property
adUnit.sizes = bannerSizes;
} else {
utils.logError('Detected a mediaTypes.banner object did not include sizes. This is a required field for the mediaTypes.banner object. Removing invalid mediaTypes.banner object from request.');
utils.logError('Detected a mediaTypes.banner object without a proper sizes field. Please ensure the sizes are listed like: [[300, 250], ...]. Removing invalid mediaTypes.banner object from request.');
delete adUnit.mediaTypes.banner;
}
} else if (adUnit.sizes) {
utils.logWarn('Usage of adUnits.sizes will eventually be deprecated. Please define size dimensions within the corresponding area of the mediaTypes.<object> (eg mediaTypes.banner.sizes).');
adUnit.sizes = normalizedSize;
}

if (mediaTypes && mediaTypes.video) {
if (mediaTypes.video) {
const video = mediaTypes.video;
if (video.playerSize) {
if (Array.isArray(video.playerSize) && video.playerSize.length === 1 && video.playerSize.every(plySize => isArrayOfNums(plySize, 2))) {
adUnit.sizes = video.playerSize;
} else if (isArrayOfNums(video.playerSize, 2)) {
let newPlayerSize = [];
newPlayerSize.push(video.playerSize);
utils.logInfo(`Transforming video.playerSize from [${video.playerSize}] to [[${newPlayerSize}]] so it's in the proper format.`);
adUnit.sizes = video.playerSize = newPlayerSize;
let tarPlayerSizeLen = (typeof video.playerSize[0] === 'number') ? 2 : 1;
const videoSizes = validateSizes(video.playerSize, tarPlayerSizeLen);
if (videoSizes.length > 0) {
adUnit.sizes = video.playerSize = videoSizes;
} else {
utils.logError('Detected incorrect configuration of mediaTypes.video.playerSize. Please specify only one set of dimensions in a format like: [[640, 480]]. Removing invalid mediaTypes.video.playerSize property from request.');
delete adUnit.mediaTypes.video.playerSize;
}
}
}

if (mediaTypes && mediaTypes.native) {
if (mediaTypes.native) {
const nativeObj = mediaTypes.native;
if (nativeObj.image && nativeObj.image.sizes && !Array.isArray(nativeObj.image.sizes)) {
utils.logError('Please use an array of sizes for native.image.sizes field. Removing invalid mediaTypes.native.image.sizes property from request.');
Expand All @@ -120,8 +130,8 @@ export const checkAdUnitSetup = hook('sync', function (adUnits) {
delete adUnit.mediaTypes.native.icon.sizes;
}
}
return true;
});
return adUnits;
}, 'checkAdUnitSetup');

/// ///////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ export function getAdUnitSizes(adUnit) {
} else {
sizes.push(bannerSizes);
}
// TODO - remove this else block when we're ready to deprecate adUnit.sizes for bidders
} else if (Array.isArray(adUnit.sizes)) {
if (Array.isArray(adUnit.sizes[0])) {
sizes = adUnit.sizes;
Expand Down
1 change: 0 additions & 1 deletion test/spec/modules/prebidServerBidAdapter_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1413,7 +1413,6 @@ describe('S2S Adapter', function () {
server.respondWith(JSON.stringify(cacheResponse));

config.setConfig({ s2sConfig: CONFIG });
debugger; // eslint-disable-line
adapter.callBids(REQUEST, BID_REQUESTS, addBidResponse, done, ajax);
server.respond();
sinon.assert.calledOnce(addBidResponse);
Expand Down
25 changes: 17 additions & 8 deletions test/spec/unit/pbjs_api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,11 @@ describe('Unit: Prebid Module', function () {

adUnits = [{
code: 'adUnit-code',
mediaTypes: {
banner: {
sizes: [[300, 250]]
}
},
bids: [
{bidder: BIDDER_CODE, params: {placementId: 'id'}},
]
Expand Down Expand Up @@ -1314,9 +1319,11 @@ describe('Unit: Prebid Module', function () {
adUnits: [
{
code: 'test1',
mediaTypes: { banner: { sizes: [] } },
bids: [],
}, {
code: 'test2',
mediaTypes: { banner: { sizes: [] } },
bids: [],
}
],
Expand Down Expand Up @@ -1396,9 +1403,11 @@ describe('Unit: Prebid Module', function () {
{
code: 'test1',
transactionId: 'd0676a3c-ff32-45a5-af65-8175a8e7ddca',
mediaTypes: { banner: { sizes: [] } },
bids: []
}, {
code: 'test2',
mediaTypes: { banner: { sizes: [] } },
bids: []
}
]
Expand All @@ -1419,9 +1428,11 @@ describe('Unit: Prebid Module', function () {
adUnits: [
{
code: 'test1',
mediaTypes: { banner: { sizes: [] } },
bids: []
}, {
code: 'test2',
mediaTypes: { banner: { sizes: [] } },
bids: []
}
]
Expand Down Expand Up @@ -1562,7 +1573,6 @@ describe('Unit: Prebid Module', function () {
expect(auctionArgs.adUnits[0].sizes).to.deep.equal([[640, 480]]);
expect(auctionArgs.adUnits[0].mediaTypes.video.playerSize).to.deep.equal([[640, 480]]);
expect(auctionArgs.adUnits[0].mediaTypes.video).to.exist;
assert.ok(logInfoSpy.calledWith('Transforming video.playerSize from [640,480] to [[640,480]] so it\'s in the proper format.'), 'expected message was logged');
});

it('should normalize adUnit.sizes and adUnit.mediaTypes.banner.sizes', function () {
Expand Down Expand Up @@ -1601,7 +1611,7 @@ describe('Unit: Prebid Module', function () {
});
expect(auctionArgs.adUnits[0].sizes).to.deep.equal([[300, 250], [300, 600]]);
expect(auctionArgs.adUnits[0].mediaTypes.banner).to.be.undefined;
assert.ok(logErrorSpy.calledWith('Detected a mediaTypes.banner object did not include sizes. This is a required field for the mediaTypes.banner object. Removing invalid mediaTypes.banner object from request.'));
assert.ok(logErrorSpy.calledWith('Detected a mediaTypes.banner object without a proper sizes field. Please ensure the sizes are listed like: [[300, 250], ...]. Removing invalid mediaTypes.banner object from request.'));

let badVideo1 = [{
code: 'testb2',
Expand Down Expand Up @@ -1763,21 +1773,21 @@ describe('Unit: Prebid Module', function () {
before(function () {
adUnits = [{
code: 'adUnit-code',
sizes: [[300, 250], [300, 600]],
mediaTypes: { banner: { sizes: [[300, 250], [300, 600]] } },
bids: [
{bidder: 'appnexus', params: {placementId: '10433394'}}
]
}];
let adUnitCodes = ['adUnit-code'];
let auction = auctionModule.newAuction({adUnits, adUnitCodes, callback: function() {}, cbTimeout: timeout});

adUnits[0]['mediaType'] = 'native';
adUnits[0]['mediaTypes'] = { native: {} };
adUnitCodes = ['adUnit-code'];
let auction1 = auctionModule.newAuction({adUnits, adUnitCodes, callback: function() {}, cbTimeout: timeout});

adUnits = [{
code: 'adUnit-code',
nativeParams: {type: 'image'},
mediaTypes: { native: { type: 'image' } },
sizes: [[300, 250], [300, 600]],
bids: [
{bidder: 'appnexus', params: {placementId: 'id'}}
Expand Down Expand Up @@ -1811,7 +1821,7 @@ describe('Unit: Prebid Module', function () {
it('should call callBids function on adapterManager', function () {
let adUnits = [{
code: 'adUnit-code',
sizes: [[300, 250], [300, 600]],
mediaTypes: { banner: { sizes: [[300, 250], [300, 600]] } },
bids: [
{bidder: 'appnexus', params: {placementId: '10433394'}}
]
Expand All @@ -1823,8 +1833,7 @@ describe('Unit: Prebid Module', function () {
it('splits native type to individual native assets', function () {
let adUnits = [{
code: 'adUnit-code',
nativeParams: {type: 'image'},
sizes: [[300, 250], [300, 600]],
mediaTypes: { native: { type: 'image' } },
bids: [
{bidder: 'appnexus', params: {placementId: 'id'}}
]
Expand Down

0 comments on commit 6729336

Please sign in to comment.