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

Core: Sync between ortb2Imp and mediaTypes #12423

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 21 additions & 0 deletions src/banner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { isArrayOfNums, isInteger, isStr } from './utils.js';

/**
* List of OpenRTB 2.x banner object properties with simple validators.
* Not included: `ext`
* reference: https://github.com/InteractiveAdvertisingBureau/openrtb2.x/blob/main/2.6.md
*/
export const ORTB_BANNER_PARAMS = new Map([
[ 'format', value => Array.isArray(value) && value.length > 0 && value.every(v => typeof v === 'object') ],
[ 'w', isInteger ],
[ 'h', isInteger ],
[ 'btype', isArrayOfNums ],
[ 'battr', isArrayOfNums ],
[ 'pos', isInteger ],
[ 'mimes', value => Array.isArray(value) && value.length > 0 && value.every(v => typeof v === 'string') ],
[ 'topframe', value => [1, 0].includes(value) ],
[ 'expdir', isArrayOfNums ],
[ 'api', isArrayOfNums ],
[ 'id', isStr ],
[ 'vcm', value => [1, 0].includes(value) ]
]);
47 changes: 34 additions & 13 deletions src/prebid.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ import {enrichFPD} from './fpd/enrichment.js';
import {allConsent} from './consentHandler.js';
import {insertLocatorFrame, markBidAsRendered, renderAdDirect, renderIfDeferred} from './adRendering.js';
import {getHighestCpm} from './utils/reducers.js';
import {fillVideoDefaults, validateOrtbVideoFields} from './video.js';
import {ORTB_VIDEO_PARAMS, fillVideoDefaults, validateOrtbVideoFields} from './video.js';
import { ORTB_BANNER_PARAMS } from './banner.js';
import { BANNER, VIDEO } from './mediaTypes.js';

const pbjsInstance = getGlobal();
const { triggerUserSyncs } = userSync;
Expand Down Expand Up @@ -100,20 +102,40 @@ function validateSizes(sizes, targLength) {
return cleanSizes;
}

export function setBattrForAdUnit(adUnit, mediaType) {
const ortb2Imp = adUnit.ortb2Imp || {};
const mediaTypes = adUnit.mediaTypes || {};
// synchronize fields between mediaTypes[mediaType] and ortb2Imp[mediaType]
export function syncOrtb2(adUnit, mediaType) {
const ortb2Imp = deepAccess(adUnit, `ortb2Imp.${mediaType}`);
const mediaTypes = deepAccess(adUnit, `mediaTypes.${mediaType}`);

if (ortb2Imp[mediaType]?.battr && mediaTypes[mediaType]?.battr && (ortb2Imp[mediaType]?.battr !== mediaTypes[mediaType]?.battr)) {
logWarn(`Ad unit ${adUnit.code} specifies conflicting ortb2Imp.${mediaType}.battr and mediaTypes.${mediaType}.battr, the latter will be ignored`, adUnit);
if (!ortb2Imp && !mediaTypes) {
// omitting sync due to not present mediaType
return;
}

const battr = ortb2Imp[mediaType]?.battr || mediaTypes[mediaType]?.battr;
const fields = {
[VIDEO]: ORTB_VIDEO_PARAMS,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
[VIDEO]: ORTB_VIDEO_PARAMS,
[VIDEO]: FEATURES.VIDEO && ORTB_VIDEO_PARAMS

[BANNER]: ORTB_BANNER_PARAMS
}[mediaType];

if (battr != null) {
deepSetValue(adUnit, `ortb2Imp.${mediaType}.battr`, battr);
deepSetValue(adUnit, `mediaTypes.${mediaType}.battr`, battr);
if (!fields) {
return;
}

[...fields].forEach(([key, validator]) => {
const mediaTypesFieldValue = deepAccess(adUnit, `mediaTypes.${mediaType}.${key}`);
const ortbFieldValue = deepAccess(adUnit, `ortb2Imp.${mediaType}.${key}`);

if (mediaTypesFieldValue == undefined && ortbFieldValue == undefined) {
// omitting the params if it's not defined on either of sides
} else if (mediaTypesFieldValue == undefined) {
deepSetValue(adUnit, `mediaTypes.${mediaType}.${key}`, ortbFieldValue);
} else if (ortbFieldValue == undefined) {
deepSetValue(adUnit, `ortb2Imp.${mediaType}.${key}`, mediaTypesFieldValue);
} else {
logWarn(`adUnit ${adUnit.code}: specifies conflicting ortb2Imp.${mediaType}.${key} and mediaTypes.${mediaType}.${key}, the latter will be ignored`, adUnit);
deepSetValue(adUnit, `mediaTypes.${mediaType}.${key}`, ortbFieldValue);
}
});
}

function validateBannerMediaType(adUnit) {
Expand All @@ -128,7 +150,7 @@ function validateBannerMediaType(adUnit) {
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 validatedAdUnit.mediaTypes.banner
}
setBattrForAdUnit(validatedAdUnit, 'banner');
syncOrtb2(validatedAdUnit, 'banner')
return validatedAdUnit;
}

Expand All @@ -152,7 +174,7 @@ function validateVideoMediaType(adUnit) {
}
}
validateOrtbVideoFields(validatedAdUnit);
setBattrForAdUnit(validatedAdUnit, 'video');
syncOrtb2(validatedAdUnit, 'video');
return validatedAdUnit;
}

Expand Down Expand Up @@ -202,7 +224,6 @@ function validateNativeMediaType(adUnit) {
logError('Please use an array of sizes for native.icon.sizes field. Removing invalid mediaTypes.native.icon.sizes property from request.');
delete validatedAdUnit.mediaTypes.native.icon.sizes;
}
setBattrForAdUnit(validatedAdUnit, 'native');
return validatedAdUnit;
}

Expand Down
183 changes: 183 additions & 0 deletions test/spec/banner_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
import * as utils from '../../src/utils.js';
import { syncOrtb2 } from '../../src/prebid.js';

describe('banner', () => {
describe('syncOrtb2', () => {
let logWarnSpy;

beforeEach(function () {
logWarnSpy = sinon.spy(utils, 'logWarn');
});

afterEach(function () {
utils.logWarn.restore();
});

it('should properly sync fields if both present', () => {
const adUnit = {
mediaTypes: {
banner: {
format: [{w: 100, h: 100}],
btype: [1, 2, 34], // should be overwritten with value from ortb2Imp
battr: [3, 4],
maxduration: 'omitted_value' // should be omitted during copying - not part of banner obj spec
}
},
ortb2Imp: {
banner: {
request: '{payload: true}',
pos: 5,
btype: [999, 999],
vcm: 0,
foobar: 'omitted_value' // should be omitted during copying - not part of banner obj spec
}
}
};

const expected = {
mediaTypes: {
banner: {
format: [{w: 100, h: 100}],
btype: [999, 999],
pos: 5,
battr: [3, 4],
vcm: 0,
maxduration: 'omitted_value'
}
},
ortb2Imp: {
banner: {
format: [{w: 100, h: 100}],
request: '{payload: true}',
pos: 5,
btype: [999, 999],
battr: [3, 4],
vcm: 0,
foobar: 'omitted_value'
}
}
};

syncOrtb2(adUnit, 'banner');
expect(adUnit).to.deep.eql(expected);

assert.ok(logWarnSpy.calledOnce, 'expected warning was logged due to conflicting btype');
});

it('should omit sync if mediaType not present on adUnit', () => {
const adUnit = {
mediaTypes: {
video: {
fieldToOmit: 'omitted_value'
}
},
ortb2Imp: {
video: {
fieldToOmit2: 'omitted_value'
}
}
};

syncOrtb2(adUnit, 'banner');

expect(adUnit.ortb2Imp.banner).to.be.undefined;
expect(adUnit.mediaTypes.banner).to.be.undefined;
});

it('should properly sync if mediaTypes is not present on any of side', () => {
const adUnit = {
mediaTypes: {
banner: {
format: [{w: 100, h: 100}],
btype: [999, 999],
pos: 5,
battr: [3, 4],
vcm: 0,
maxduration: 'omitted_value'
}
},
ortb2Imp: {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ortb2Imp: {}

};

const expected1 = {
mediaTypes: {
banner: {
format: [{w: 100, h: 100}],
btype: [999, 999],
pos: 5,
battr: [3, 4],
vcm: 0,
maxduration: 'omitted_value'
}
},
ortb2Imp: {
banner: {
format: [{w: 100, h: 100}],
btype: [999, 999],
pos: 5,
battr: [3, 4],
vcm: 0,
}
}
};

syncOrtb2(adUnit, 'banner');
expect(adUnit).to.deep.eql(expected1);

const adUnit2 = {
mediaTypes: {},
ortb2Imp: {
banner: {
format: [{w: 100, h: 100}],
btype: [999, 999],
pos: 5,
battr: [3, 4],
vcm: 0,
maxduration: 'omitted_value'
}
}
};

const expected2 = {
mediaTypes: {
banner: {
format: [{w: 100, h: 100}],
btype: [999, 999],
pos: 5,
battr: [3, 4],
vcm: 0,
}
},
ortb2Imp: {
banner: {
format: [{w: 100, h: 100}],
btype: [999, 999],
pos: 5,
battr: [3, 4],
vcm: 0,
maxduration: 'omitted_value'
}
}
};

syncOrtb2(adUnit2, 'banner');
expect(adUnit2).to.deep.eql(expected2);
});

it('should not create empty banner object on ortb2Imp if there was nothing to copy', () => {
const adUnit2 = {
mediaTypes: {
banner: {
noOrtbBannerField1: 'value',
noOrtbBannerField2: 'value'
}
},
ortb2Imp: {
// lack of banner field
}
};
syncOrtb2(adUnit2, 'banner');
expect(adUnit2.ortb2Imp.banner).to.be.undefined
});
});
})
2 changes: 1 addition & 1 deletion test/spec/native_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { convertOrtbRequestToProprietaryNative, fromOrtbNativeRequest } from '..
import {auctionManager} from '../../src/auctionManager.js';
import {getRenderingData} from '../../src/adRendering.js';
import {getCreativeRendererSource} from '../../src/creativeRenderers.js';
import {deepClone, deepSetValue} from '../../src/utils.js';
import {deepSetValue} from '../../src/utils.js';
const utils = require('src/utils');

const bid = {
Expand Down
67 changes: 0 additions & 67 deletions test/spec/unit/pbjs_api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3746,71 +3746,4 @@ describe('Unit: Prebid Module', function () {
expect(auctionManager.getBidsReceived().length).to.equal(0);
});
});

describe('setBattrForAdUnit', () => {
it('should set copy battr to both places', () => {
const adUnit = {
ortb2Imp: {
video: {
battr: 'banned attribute'
}
},
mediaTypes: {
video: {}
}
}

setBattrForAdUnit(adUnit, 'video');

expect(adUnit.mediaTypes.video.battr).to.deep.equal('banned attribute');
expect(adUnit.ortb2Imp.video.battr).to.deep.equal('banned attribute');

const adUnit2 = {
mediaTypes: {
video: {
battr: 'banned attribute'
}
},
ortb2Imp: {
video: {}
}
}

setBattrForAdUnit(adUnit2, 'video');

expect(adUnit2.ortb2Imp.video.battr).to.deep.equal('banned attribute');
expect(adUnit2.mediaTypes.video.battr).to.deep.equal('banned attribute');
})

it('should log warn if both are specified and differ from eachother', () => {
let spyLogWarn = sinon.spy(utils, 'logWarn');
const adUnit = {
mediaTypes: {
native: {
battr: 'banned attribute'
}
},
ortb2Imp: {
native: {
battr: 'banned attribute 2'
}
}
}
setBattrForAdUnit(adUnit, 'native');
sinon.assert.calledOnce(spyLogWarn);
spyLogWarn.resetHistory();
utils.logWarn.restore();
})

it('should not copy for undefined battr', () => {
const adUnit = {
mediaTypes: {
native: {}
}
}
setBattrForAdUnit(adUnit, 'native');
expect(adUnit.mediaTypes.native).to.deep.equal({});
expect(adUnit.mediaTypes.ortb2Imp).to.not.exist;
})
})
});
Loading