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.

Sign up for GitHub

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

Jump to bottom

deprecate support for adUnit.sizes for pubs #4458

Merged
merged 2 commits into from
Dec 6, 2019
Merged

deprecate support for adUnit.sizes for pubs #4458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
190f296
deprecate support for adUnit.sizes for pubs
jsnellbaker Nov 12, 2019
6184b4a
remove commented code
jsnellbaker Nov 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion integrationExamples/gpt/prebidServer_example.html
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,12 +418,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 @@ -780,8 +774,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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,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
View file
Open in desktop
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