Skip to content

Commit

Permalink
Advanced Size Mapping - Code Refactoring. (#5487)
Browse files Browse the repository at this point in the history
* refactor checkAdUnitSetupHook to minimize mutation of adUnit object

* refactor mediatype validation functions to minimize mutation of adunit object

* skip one test which is failing

* add some extra tests and resolve lgtm error¥
  • Loading branch information
Fawke authored Jul 23, 2020
1 parent 6e71525 commit bf5c7c7
Show file tree
Hide file tree
Showing 5 changed files with 327 additions and 178 deletions.
268 changes: 154 additions & 114 deletions modules/sizeMappingV2.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
/**
* This modules adds support for the new Size Mapping spec described here. https://github.com/prebid/Prebid.js/issues/4129
* This implementation replaces global sizeConfig with a adUnit/bidder level sizeConfig with support for labels.
* This module adds support for the new size mapping spec, Advanced Size Mapping. It's documented here. https://github.com/prebid/Prebid.js/issues/4129
* The implementation is an alternative to global sizeConfig. It introduces 'Ad Unit' & 'Bidder' level sizeConfigs and also supports 'labels' for conditional
* rendering. Read full API documentation on Prebid.org, http://prebid.org/dev-docs/modules/sizeMappingV2.html
*/

import * as utils from '../src/utils.js';
import { processNativeAdUnitParams } from '../src/native.js';
import { adunitCounter } from '../src/adUnits.js';
import includes from 'core-js-pure/features/array/includes.js';
import { getHook } from '../src/hook.js';
import {
adUnitSetupChecks
} from '../src/prebid.js';
import { adUnitSetupChecks } from '../src/prebid.js';

// allows for sinon.spy, sinon.stub, etc to unit test calls made to these functions internally
// Allows for stubbing of these functions while writing unit tests.
export const internal = {
checkBidderSizeConfigFormat,
getActiveSizeBucket,
Expand All @@ -22,9 +21,12 @@ export const internal = {
isLabelActivated
};

// 'sizeMappingInternalStore' contains information whether a particular auction is using size mapping V2 (the new size mapping spec),
// and it also contains additional information on each adUnit, as such, mediaTypes, activeViewport, etc.
// This information is required by the 'getBids' function.
/*
'sizeMappingInternalStore' contains information on, whether a particular auction is using size mapping V2 (the new size mapping spec),
and it also contains additional information on each adUnit, such as, mediaTypes, activeViewport, etc. This information is required by
the 'getBids' function.
*/

export const sizeMappingInternalStore = createSizeMappingInternalStore();

function createSizeMappingInternalStore() {
Expand All @@ -46,8 +48,10 @@ function createSizeMappingInternalStore() {
}
}

// returns "true" if atleast one of the adUnit in the adUnits array has declared a Ad Unit or(and) Bidder level sizeConfig
// returns "false" otherwise
/*
Returns "true" if at least one of the adUnits in the adUnits array is using an Ad Unit and/or Bidder level sizeConfig,
otherwise, returns "false."
*/
export function isUsingNewSizeMapping(adUnits) {
let isUsingSizeMappingBool = false;
adUnits.forEach(adUnit => {
Expand All @@ -74,136 +78,172 @@ export function isUsingNewSizeMapping(adUnits) {
return isUsingSizeMappingBool;
}

// returns "adUnits" array which have passed sizeConfig validation checks in addition to mediaTypes checks
// deletes properties from adUnit which fail validation.
/**
This hooked function executes before the function 'checkAdUnitSetup', that is defined in /src/prebid.js. It's necessary to run this funtion before
because it applies a series of checks in order to determine the correctness of the 'sizeConfig' array, which, the original 'checkAdUnitSetup' function
does not recognize.
@params {Array<AdUnits>} adUnits
@returns {Array<AdUnits>} validateAdUnits - Unrecognized properties are deleted.
*/
export function checkAdUnitSetupHook(adUnits) {
return adUnits.filter(adUnit => {
const validateSizeConfig = function (mediaType, sizeConfig, adUnitCode) {
let isValid = true;
const associatedProperty = {
banner: 'sizes',
video: 'playerSize',
native: 'active'
}
const propertyName = associatedProperty[mediaType];
const conditionalLogMessages = {
banner: 'Removing mediaTypes.banner from ad unit.',
video: 'Removing mediaTypes.video.sizeConfig from ad unit.',
native: 'Removing mediaTypes.native.sizeConfig from ad unit.'
}
if (Array.isArray(sizeConfig)) {
sizeConfig.forEach((config, index) => {
const keys = Object.keys(config);
/*
Check #1 (Applies to 'banner', 'video' and 'native' media types.)
Verify that all config objects include 'minViewPort' and 'sizes' property.
If they do not, return 'false'.
*/
if (!(includes(keys, 'minViewPort') && includes(keys, propertyName))) {
utils.logError(`Ad unit ${adUnitCode}: Missing required property 'minViewPort' or 'sizes' from 'mediaTypes.${mediaType}.sizeConfig[${index}]'. ${conditionalLogMessages[mediaType]}`);
isValid = false;
return;
}
/*
Check #2 (Applies to 'banner', 'video' and 'native' media types.)
Verify that 'config.minViewPort' property is in [width, height] format.
If not, return false.
*/
if (!utils.isArrayOfNums(config.minViewPort, 2)) {
utils.logError(`Ad unit ${adUnitCode}: Invalid declaration of 'minViewPort' in 'mediaTypes.${mediaType}.sizeConfig[${index}]'. ${conditionalLogMessages[mediaType]}`);
isValid = false
return;
}
/*
Check #3 (Applies only to 'banner' and 'video' media types.)
Verify that 'config.sizes' (in case of banner) or 'config.playerSize' (in case of video)
property is in [width, height] format. If not, return 'false'.
*/
if (mediaType === 'banner' || mediaType === 'video') {
let showError = false;
if (Array.isArray(config[propertyName])) {
const validatedSizes = adUnitSetupChecks.validateSizes(config[propertyName]);
if (config[propertyName].length > 0 && validatedSizes.length === 0) {
isValid = false;
showError = true;
}
} else {
// Either 'sizes' or 'playerSize' is not declared as an array, which makes it invalid by default.
isValid = false;
showError = true;
}
if (showError) {
utils.logError(`Ad unit ${adUnitCode}: Invalid declaration of '${propertyName}' in 'mediaTypes.${mediaType}.sizeConfig[${index}]'. ${conditionalLogMessages[mediaType]}`);
return;
}
}
/*
Check #4 (Applies only to 'native' media type)
Verify that 'config.active' is a 'boolean'.
If not, return 'false'.
*/
if (mediaType === 'native') {
if (typeof config[propertyName] !== 'boolean') {
utils.logError(`Ad unit ${adUnitCode}: Invalid declaration of 'active' in 'mediaTypes.${mediaType}.sizeConfig[${index}]'. ${conditionalLogMessages[mediaType]}`);
isValid = false;
}
}
});
} else {
utils.logError(`Ad unit ${adUnitCode}: Invalid declaration of 'sizeConfig' in 'mediaTypes.${mediaType}.sizeConfig'. ${conditionalLogMessages[mediaType]}`);
isValid = false;
return isValid;
}

// If all checks have passed, isValid should equal 'true'
return isValid;
}
const validatedAdUnits = [];
adUnits.forEach(adUnit => {
const mediaTypes = adUnit.mediaTypes;
let validatedBanner, validatedVideo, validatedNative;
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;
return;
}

if (mediaTypes.banner) {
const banner = mediaTypes.banner;
if (banner.sizes) {
adUnitSetupChecks.validateBannerMediaType(adUnit);
} else if (banner.sizeConfig) {
if (Array.isArray(banner.sizeConfig)) {
let deleteBannerMediaType = false;
banner.sizeConfig.forEach((config, index) => {
// verify if all config objects include "minViewPort" and "sizes" property.
// if not, remove the mediaTypes.banner object
const keys = Object.keys(config);
if (!(includes(keys, 'minViewPort') && includes(keys, 'sizes'))) {
utils.logError(`Ad Unit: ${adUnit.code}: mediaTypes.banner.sizeConfig[${index}] is missing required property minViewPort or sizes or both.`);
deleteBannerMediaType = true;
return;
}

// check if the config.sizes property is in [w, h] format, if yes, change it to [[w, h]] format.
const bannerSizes = adUnitSetupChecks.validateSizes(config.sizes);
if (utils.isArrayOfNums(config.minViewPort, 2)) {
if (config.sizes.length > 0 && bannerSizes.length > 0) {
config.sizes = bannerSizes;
} else if (config.sizes.length === 0) {
// If a size bucket doesn't have any sizes, sizes is an empty array, i.e. sizes: []. This check takes care of that.
config.sizes = [config.sizes];
} else {
utils.logError(`Ad Unit: ${adUnit.code}: mediaTypes.banner.sizeConfig[${index}] has propery sizes declared with invalid value. Please ensure the sizes are listed like: [[300, 250], ...] or like: [] if no sizes are present for that size bucket.`);
deleteBannerMediaType = true;
}
} else {
utils.logError(`Ad Unit: ${adUnit.code}: mediaTypes.banner.sizeConfig[${index}] has property minViewPort decalared with invalid value. Please ensure minViewPort is an Array and is listed like: [700, 0]. Declaring an empty array is not allowed, instead use: [0, 0].`);
deleteBannerMediaType = true;
if (mediaTypes.banner.sizes) {
// Ad unit is using 'mediaTypes.banner.sizes' instead of the new property 'sizeConfig'. Apply the old checks!
validatedBanner = adUnitSetupChecks.validateBannerMediaType(adUnit);
} else if (mediaTypes.banner.sizeConfig) {
// Ad unit is using the 'sizeConfig' property, 'mediaTypes.banner.sizeConfig'. Apply the new checks!
validatedBanner = utils.deepClone(adUnit);
const isBannerValid = validateSizeConfig('banner', mediaTypes.banner.sizeConfig, adUnit.code);
if (!isBannerValid) {
delete validatedBanner.mediaTypes.banner;
} else {
/*
Make sure 'sizes' field is always an array of arrays. If not, make it so.
For example, [] becomes [[]], and [360, 400] becomes [[360, 400]]
*/
validatedBanner.mediaTypes.banner.sizeConfig.forEach(config => {
if (!Array.isArray(config.sizes[0])) {
config.sizes = [config.sizes];
}
});
if (deleteBannerMediaType) {
utils.logInfo(`Ad Unit: ${adUnit.code}: mediaTypes.banner has been removed due to error in sizeConfig.`);
delete adUnit.mediaTypes.banner;
}
} else {
utils.logError(`Ad Unit: ${adUnit.code}: mediaTypes.banner.sizeConfig is NOT an Array. Removing the invalid object mediaTypes.banner from Ad Unit.`);
delete adUnit.mediaTypes.banner;
}
} else {
utils.logError('Detected a mediaTypes.banner object did not include required property sizes or sizeConfig. Removing invalid mediaTypes.banner object from Ad Unit.');
delete adUnit.mediaTypes.banner;
// Ad unit is invalid since it's mediaType property does not have either 'sizes' or 'sizeConfig' declared.
utils.logError(`Ad unit ${adUnit.code}: 'mediaTypes.banner' does not contain either 'sizes' or 'sizeConfig' property. Removing 'mediaTypes.banner' from ad unit.`);
validatedBanner = utils.deepClone(adUnit);
delete validatedBanner.mediaTypes.banner;
}
}

if (mediaTypes.video) {
const video = mediaTypes.video;
if (video.playerSize) {
adUnitSetupChecks.validateVideoMediaType(adUnit);
} else if (video.sizeConfig) {
if (Array.isArray(video.sizeConfig)) {
let deleteVideoMediaType = false;
video.sizeConfig.forEach((config, index) => {
// verify if all config objects include "minViewPort" and "playerSize" property.
// if not, remove the mediaTypes.video object
const keys = Object.keys(config);
if (!(includes(keys, 'minViewPort') && includes(keys, 'playerSize'))) {
utils.logError(`Ad Unit: ${adUnit.code}: mediaTypes.video.sizeConfig[${index}] is missing required property minViewPort or playerSize or both. Removing the invalid property mediaTypes.video.sizeConfig from Ad Unit.`);
deleteVideoMediaType = true;
return;
}
// check if the config.playerSize property is in [w, h] format, if yes, change it to [[w, h]] format.
let tarPlayerSizeLen = (typeof config.playerSize[0] === 'number') ? 2 : 1;
const videoSizes = adUnitSetupChecks.validateSizes(config.playerSize, tarPlayerSizeLen);
if (utils.isArrayOfNums(config.minViewPort, 2)) {
if (tarPlayerSizeLen === 2) {
utils.logInfo('Transforming video.playerSize from [640,480] to [[640,480]] so it\'s in the proper format.');
}
if (config.playerSize.length > 0 && videoSizes.length > 0) {
config.playerSize = videoSizes;
} else if (config.playerSize.length === 0) {
// If a size bucket doesn't have any playerSize, playerSize is an empty array, i.e. playerSize: []. This check takes care of that.
config.playerSize = [config.playerSize];
} else {
utils.logError(`Ad Unit: ${adUnit.code}: mediaTypes.video.sizeConfig[${index}] has propery playerSize declared with invalid value. Please ensure the playerSize is listed like: [640, 480] or like: [] if no playerSize is present for that size bucket.`);
deleteVideoMediaType = true;
}
} else {
utils.logError(`Ad Unit: ${adUnit.code}: mediaTypes.video.sizeConfig[${index}] has property minViewPort decalared with invalid value. Please ensure minViewPort is an Array and is listed like: [700, 0]. Declaring an empty array is not allowed, instead use: [0, 0].`);
deleteVideoMediaType = true;
if (mediaTypes.video.playerSize) {
// Ad unit is using 'mediaTypes.video.playerSize' instead of the new property 'sizeConfig'. Apply the old checks!
validatedVideo = validatedBanner ? adUnitSetupChecks.validateVideoMediaType(validatedBanner) : adUnitSetupChecks.validateVideoMediaType(adUnit);
} else if (mediaTypes.video.sizeConfig) {
// Ad unit is using the 'sizeConfig' property, 'mediaTypes.video.sizeConfig'. Apply the new checks!
validatedVideo = validatedBanner || utils.deepClone(adUnit);
const isVideoValid = validateSizeConfig('video', mediaTypes.video.sizeConfig, adUnit.code);
if (!isVideoValid) {
delete validatedVideo.mediaTypes.video.sizeConfig;
} else {
/*
Make sure 'playerSize' field is always an array of arrays. If not, make it so.
For example, [] becomes [[]], and [640, 400] becomes [[640, 400]]
*/
validatedVideo.mediaTypes.video.sizeConfig.forEach(config => {
if (!Array.isArray(config.playerSize[0])) {
config.playerSize = [config.playerSize];
}
});
if (deleteVideoMediaType) {
utils.logInfo(`Ad Unit: ${adUnit.code}: mediaTypes.video.sizeConfig has been removed due to error in sizeConfig.`);
delete adUnit.mediaTypes.video.sizeConfig;
}
} else {
utils.logError(`Ad Unit: ${adUnit.code}: mediaTypes.video.sizeConfig is NOT an Array. Removing the invalid property mediaTypes.video.sizeConfig from Ad Unit.`);
return delete adUnit.mediaTypes.video.sizeConfig;
}
}
}

if (mediaTypes.native) {
const native = mediaTypes.native;
adUnitSetupChecks.validateNativeMediaType(adUnit);
// Apply the old native checks
validatedNative = validatedVideo ? adUnitSetupChecks.validateNativeMediaType(validatedVideo) : validatedBanner ? adUnitSetupChecks.validateNativeMediaType(validatedBanner) : adUnitSetupChecks.validateNativeMediaType(adUnit);

// Apply the new checks if 'mediaTypes.native.sizeConfig' detected
if (mediaTypes.native.sizeConfig) {
native.sizeConfig.forEach(config => {
// verify if all config objects include "minViewPort" and "active" property.
// if not, remove the mediaTypes.native object
const keys = Object.keys(config);
if (!(includes(keys, 'minViewPort') && includes(keys, 'active'))) {
utils.logError(`Ad Unit: ${adUnit.code}: mediaTypes.native.sizeConfig is missing required property minViewPort or active or both. Removing the invalid property mediaTypes.native.sizeConfig from Ad Unit.`);
return delete adUnit.mediaTypes.native.sizeConfig;
}

if (!(utils.isArrayOfNums(config.minViewPort, 2) && typeof config.active === 'boolean')) {
utils.logError(`Ad Unit: ${adUnit.code}: mediaTypes.native.sizeConfig has properties minViewPort or active decalared with invalid values. Removing the invalid property mediaTypes.native.sizeConfig from Ad Unit.`);
return delete adUnit.mediaTypes.native.sizeConfig;
}
});
const isNativeValid = validateSizeConfig('native', mediaTypes.native.sizeConfig, adUnit.code);
if (!isNativeValid) {
delete validatedNative.mediaTypes.native.sizeConfig;
}
}
}

return true;
const validatedAdUnit = Object.assign({}, validatedBanner, validatedVideo, validatedNative);
validatedAdUnits.push(validatedAdUnit);
});
return validatedAdUnits;
}

getHook('checkAdUnitSetup').before(function (fn, adUnits) {
Expand Down
Loading

0 comments on commit bf5c7c7

Please sign in to comment.