From 0376077170efa6b7277236269c873dfaa20c6b71 Mon Sep 17 00:00:00 2001 From: Jimmy Tu Date: Thu, 5 Apr 2018 20:14:11 -0700 Subject: [PATCH 1/2] OpenX Adapter: Support out of order bids. --- modules/openxBidAdapter.js | 38 +- test/spec/modules/openxBidAdapter_spec.js | 549 ++++++++++++++++++---- 2 files changed, 456 insertions(+), 131 deletions(-) diff --git a/modules/openxBidAdapter.js b/modules/openxBidAdapter.js index bc00242b3e3..1cc312da273 100644 --- a/modules/openxBidAdapter.js +++ b/modules/openxBidAdapter.js @@ -70,21 +70,10 @@ function createBannerBidResponses(oxResponseObj, {bids, startTime}) { } for (let i = 0; i < adUnits.length; i++) { let adUnit = adUnits[i]; + let adUnitIdx = parseInt(adUnit.idx, 10); let bidResponse = {}; - if (adUnits.length === bids.length) { - // request and response length match, directly assign the request id based on positioning - bidResponse.requestId = bids[i].bidId; - } else { - for (let j = i; j < bids.length; j++) { - let bid = bids[j]; - if (String(bid.params.unit) === String(adUnit.adunitid) && adUnitHasValidSizeFromBid(adUnit, bid) && !bid.matched) { - // ad unit and size match, this is the correct bid response to bid - bidResponse.requestId = bid.bidId; - bid.matched = true; - break; - } - } - } + + bidResponse.requestId = bids[adUnitIdx].bidId; if (adUnit.pub_rev) { bidResponse.cpm = Number(adUnit.pub_rev) / 1000; @@ -134,27 +123,6 @@ function buildQueryStringFromParams(params) { .join('&'); } -function adUnitHasValidSizeFromBid(adUnit, bid) { - let sizes = utils.parseSizesInput(bid.sizes); - if (!sizes) { - return false; - } - let found = false; - let creative = adUnit.creative && adUnit.creative[0]; - let creative_size = String(creative.width) + 'x' + String(creative.height); - - if (utils.isArray(sizes)) { - for (let i = 0; i < sizes.length; i++) { - let size = sizes[i]; - if (String(size) === String(creative_size)) { - found = true; - break; - } - } - } - return found; -} - function getViewportDimensions(isIfr) { let width; let height; diff --git a/test/spec/modules/openxBidAdapter_spec.js b/test/spec/modules/openxBidAdapter_spec.js index 35146542375..e9a596a75c2 100644 --- a/test/spec/modules/openxBidAdapter_spec.js +++ b/test/spec/modules/openxBidAdapter_spec.js @@ -2,6 +2,7 @@ import {expect} from 'chai'; import {spec} from 'modules/openxBidAdapter'; import {newBidder} from 'src/adapters/bidderFactory'; import {userSync} from 'src/userSync'; +import * as utils from 'src/utils'; const URLBASE = '/w/1.0/arj'; const URLBASEVIDEO = '/v/1.0/avjp'; @@ -9,6 +10,116 @@ const URLBASEVIDEO = '/v/1.0/avjp'; describe('OpenxAdapter', () => { const adapter = newBidder(spec); + /** + * Type Definitions + */ + + /** + * @typedef {{ + * impression: string, + * inview: string, + * click: string + * }} + */ + let OxArjTracking; + /** + * @typedef {{ + * ads: { + * version: number, + * count: number, + * pixels: string, + * ad: Array + * } + * }} + */ + let OxArjResponse; + /** + * @typedef {{ + * adunitid: number, + * adid:number, + * type: string, + * htmlz: string, + * framed: number, + * is_fallback: number, + * ts: string, + * cpipc: number, + * pub_rev: string, + * tbd: ?string, + * adv_id: string, + * deal_id: string, + * auct_win_is_deal: number, + * brand_id: string, + * currency: string, + * idx: string, + * creative: Array + * }} + */ + let OxArjAdUnit; + /** + * @typedef {{ + * id: string, + * width: string, + * height: string, + * target: string, + * mime: string, + * media: string, + * tracking: OxArjTracking + * }} + */ + let OxArjCreative; + + // HELPER METHODS + /** + * @type {OxArjCreative} + */ + const DEFAULT_TEST_ARJ_CREATIVE = { + id: '0', + width: 'test-width', + height: 'test-height', + target: 'test-target', + mime: 'test-mime', + media: 'test-media', + tracking: { + impression: 'test-impression', + inview: 'test-inview', + click: 'test-click' + } + }; + + /** + * @type {OxArjAdUnit} + */ + const DEFAULT_TEST_ARJ_AD_UNIT = { + adunitid: 0, + type: 'test-type', + html: 'test-html', + framed: 0, + is_fallback: 0, + ts: 'test-ts', + tbd: 'NaN', + deal_id: undefined, + auct_win_is_deal: undefined, + cpipc: 0, + pub_rev: 'test-pub_rev', + adv_id: 'test-adv_id', + brand_id: 'test-brand_id', + currency: 'test-currency', + idx: '0', + creative: [DEFAULT_TEST_ARJ_CREATIVE] + }; + + /** + * @type {OxArjResponse} + */ + const DEFAULT_ARJ_RESPONSE = { + ads: { + version: 0, + count: 1, + pixels: 'http://testpixels.net', + ad: [DEFAULT_TEST_ARJ_AD_UNIT] + } + }; + describe('inherited functions', () => { it('exists and is a function', () => { expect(adapter.callBids).to.exist.and.to.be.a('function'); @@ -53,12 +164,14 @@ describe('OpenxAdapter', () => { bidder: 'openx', params: { unit: '12345678', - delDomain: 'test-del-domain', + delDomain: 'test-del-domain' }, adUnitCode: 'adunit-code', - mediaTypes: {video: { - playerSize: [640, 480] - }}, + mediaTypes: { + video: { + playerSize: [640, 480] + } + }, bidId: '30b31c1838de1e', bidderRequestId: '22edbae2733bf6', auctionId: '1d1a030790a475', @@ -302,7 +415,7 @@ describe('OpenxAdapter', () => { }, 'params': { 'unit': '12345678', - 'delDomain': 'test-del-domain', + 'delDomain': 'test-del-domain' }, 'adUnitCode': 'adunit-code', 'bidId': '30b31c1838de1e', @@ -384,101 +497,271 @@ describe('OpenxAdapter', () => { userSync.registerSync.restore(); }); - const bids = [{ - 'bidder': 'openx', - 'params': { - 'unit': '12345678', - 'delDomain': 'test-del-domain' - }, - 'adUnitCode': 'adunit-code', - 'mediaType': 'banner', - 'sizes': [[300, 250], [300, 600]], - 'bidId': '30b31c1838de1e', - 'bidderRequestId': '22edbae2733bf6', - 'auctionId': '1d1a030790a475' - }]; - const bidRequest = { - method: 'GET', - url: '//openx-d.openx.net/v/1.0/arj', - data: {}, - payload: {'bids': bids, 'startTime': new Date()} - }; - - const bidResponse = { - 'ads': - { - 'version': 1, - 'count': 1, - 'pixels': 'http://testpixels.net', - 'ad': [ - { - 'adunitid': 12345678, - 'adid': 5678, - 'type': 'html', - 'html': 'test_html', - 'framed': 1, - 'is_fallback': 0, - 'ts': 'ts', - 'cpipc': 1000, - 'pub_rev': '1000', - 'adv_id': 'adv_id', - 'brand_id': '', - 'creative': [ - { - 'width': '300', - 'height': '250', - 'target': '_blank', - 'mime': 'text/html', - 'media': 'test_media', - 'tracking': { - 'impression': 'http://openx-d.openx.net/v/1.0/ri?ts=ts', - 'inview': 'test_inview', - 'click': 'test_click' - } - } - ] - }] + describe('when there is a standard response', function () { + const creativeOverride = { + id: 234, + width: '300', + height: '250', + tracking: { + impression: 'http://openx-d.openx.net/v/1.0/ri?ts=ts' } + }; - }; - it('should return correct bid response', () => { - const expectedResponse = [ - { - 'requestId': '30b31c1838de1e', - 'cpm': 1, - 'width': '300', - 'height': '250', - 'creativeId': 5678, - 'ad': 'test_html', - 'ttl': 300, - 'netRevenue': true, - 'currency': 'USD', - 'ts': 'ts' - } - ]; + const adUnitOverride = { + ts: 'test-1234567890-ts', + idx: '0', + currency: 'USD', + pub_rev: '10000', + html: '
OpenX Ad
' + }; + const adUnit = mockAdUnit(adUnitOverride, creativeOverride); + const bidResponse = mockArjResponse(undefined, [adUnit]); + + let bid; + let bidRequest; + let bidRequestConfigs; + + beforeEach(function () { + bidRequestConfigs = [{ + 'bidder': 'openx', + 'params': { + 'unit': '12345678', + 'delDomain': 'test-del-domain' + }, + 'adUnitCode': 'adunit-code', + 'mediaType': 'banner', + 'sizes': [[300, 250], [300, 600]], + 'bidId': '30b31c1838de1e', + 'bidderRequestId': '22edbae2733bf6', + 'auctionId': '1d1a030790a475' + }]; + + bidRequest = { + method: 'GET', + url: '//openx-d.openx.net/v/1.0/arj', + data: {}, + payload: {'bids': bidRequestConfigs, 'startTime': new Date()} + }; + + bid = spec.interpretResponse({body: bidResponse}, bidRequest)[0]; + }); + + it('should return a price', function () { + expect(bid.cpm).to.equal(parseInt(adUnitOverride.pub_rev, 10) / 1000); + }); + + it('should return a request id', function () { + expect(bid.requestId).to.equal(bidRequest.payload.bids[0].bidId); + }); + + it('should return width and height for the creative', function () { + expect(bid.width).to.equal(creativeOverride.width); + expect(bid.height).to.equal(creativeOverride.height); + }); + + it('should return a creativeId', function () { + expect(bid.creativeId).to.equal(creativeOverride.id); + }); + + it('should return an ad', function () { + expect(bid.ad).to.equal(adUnitOverride.html); + }); + + it('should have a time-to-live of 5 minutes', function () { + expect(bid.ttl).to.equal(300); + }); - const result = spec.interpretResponse({body: bidResponse}, bidRequest); - expect(Object.keys(result[0])).to.eql(Object.keys(expectedResponse[0])); + it('should always return net revenue', function () { + expect(bid.netRevenue).to.equal(true); + }); + + it('should return a currency', function () { + expect(bid.currency).to.equal(adUnitOverride.currency); + }); + + it('should return a transaction state', function () { + expect(bid.ts).to.equal(adUnitOverride.ts); + }); + + it('should register a beacon', () => { + spec.interpretResponse({body: bidResponse}, bidRequest); + sinon.assert.calledWith(userSync.registerSync, 'image', 'openx', sinon.match(new RegExp(`\/\/openx-d\.openx\.net.*\/bo\?.*ts=${adUnitOverride.ts}`))); + }); }); - it('handles nobid responses', () => { - const bidResponse = { - 'ads': - { - 'version': 1, - 'count': 1, - 'pixels': 'http://testpixels.net', - 'ad': [] - } + describe('when there is a deal', function () { + const adUnitOverride = { + deal_id: 'ox-1000' }; + const adUnit = mockAdUnit(adUnitOverride); + const bidResponse = mockArjResponse(null, [adUnit]); - const result = spec.interpretResponse({body: bidResponse}, bidRequest); - expect(result.length).to.equal(0); + let bid; + let bidRequestConfigs; + let bidRequest; + + beforeEach(function () { + bidRequestConfigs = [{ + 'bidder': 'openx', + 'params': { + 'unit': '12345678', + 'delDomain': 'test-del-domain' + }, + 'adUnitCode': 'adunit-code', + 'mediaType': 'banner', + 'sizes': [[300, 250], [300, 600]], + 'bidId': '30b31c1838de1e', + 'bidderRequestId': '22edbae2733bf6', + 'auctionId': '1d1a030790a475' + }]; + + bidRequest = { + method: 'GET', + url: '//openx-d.openx.net/v/1.0/arj', + data: {}, + payload: {'bids': bidRequestConfigs, 'startTime': new Date()} + }; + bid = spec.interpretResponse({body: bidResponse}, bidRequest)[0]; + mockArjResponse(); + }); + + it('should return a deal id', function () { + expect(bid.dealId).to.equal(adUnitOverride.deal_id); + }); }); - it('should register a beacon', () => { - spec.interpretResponse({body: bidResponse}, bidRequest); - sinon.assert.calledWith(userSync.registerSync, 'image', 'openx', sinon.match(/\/\/openx-d\.openx\.net.*\/bo\?.*ts=ts/)); + describe('when there is no bids in the response', function () { + let bidRequest; + let bidRequestConfigs; + + beforeEach(function () { + bidRequestConfigs = [{ + 'bidder': 'openx', + 'params': { + 'unit': '12345678', + 'delDomain': 'test-del-domain' + }, + 'adUnitCode': 'adunit-code', + 'mediaType': 'banner', + 'sizes': [[300, 250], [300, 600]], + 'bidId': '30b31c1838de1e', + 'bidderRequestId': '22edbae2733bf6', + 'auctionId': '1d1a030790a475' + }]; + + bidRequest = { + method: 'GET', + url: '//openx-d.openx.net/v/1.0/arj', + data: {}, + payload: {'bids': bidRequestConfigs, 'startTime': new Date()} + }; + }); + + it('handles nobid responses', () => { + const bidResponse = { + 'ads': + { + 'version': 1, + 'count': 1, + 'pixels': 'http://testpixels.net', + 'ad': [] + } + }; + + const result = spec.interpretResponse({body: bidResponse}, bidRequest); + expect(result.length).to.equal(0); + }); + }); + + describe('when adunits return out of order', function () { + const bidRequests = [{ + bidder: 'openx', + params: { + unit: '12345678', + delDomain: 'test-del-domain' + }, + adUnitCode: 'adunit-code', + mediaTypes: { + banner: { + sizes: [[100, 111]] + } + }, + bidId: 'test-bid-request-id-1', + bidderRequestId: 'test-request-1', + auctionId: 'test-auction-id-1' + }, { + bidder: 'openx', + params: { + unit: '12345678', + delDomain: 'test-del-domain' + }, + adUnitCode: 'adunit-code', + mediaTypes: { + banner: { + sizes: [[200, 222]] + } + }, + bidId: 'test-bid-request-id-2', + bidderRequestId: 'test-request-1', + auctionId: 'test-auction-id-1' + }, { + bidder: 'openx', + params: { + unit: '12345678', + delDomain: 'test-del-domain' + }, + adUnitCode: 'adunit-code', + mediaTypes: { + banner: { + sizes: [[300, 333]] + } + }, + 'bidId': 'test-bid-request-id-3', + 'bidderRequestId': 'test-request-1', + 'auctionId': 'test-auction-id-1' + }]; + const bidRequest = { + method: 'GET', + url: '//openx-d.openx.net/v/1.0/arj', + data: {}, + payload: {'bids': bidRequests, 'startTime': new Date()} + }; + + let outOfOrderAdunits = [ + mockAdUnit({ + idx: '1' + }, { + width: bidRequests[1].mediaTypes.banner.sizes[0][0], + height: bidRequests[1].mediaTypes.banner.sizes[0][1] + }), + mockAdUnit({ + idx: '2' + }, { + width: bidRequests[2].mediaTypes.banner.sizes[0][0], + height: bidRequests[2].mediaTypes.banner.sizes[0][1] + }), + mockAdUnit({ + idx: '0' + }, { + width: bidRequests[0].mediaTypes.banner.sizes[0][0], + height: bidRequests[0].mediaTypes.banner.sizes[0][1] + }) + ]; + + let bidResponse = mockArjResponse(undefined, outOfOrderAdunits); + + it('should return map adunits back to the proper request', function () { + const bids = spec.interpretResponse({body: bidResponse}, bidRequest); + expect(bids[0].requestId).to.equal(bidRequests[1].bidId); + expect(bids[0].width).to.equal(bidRequests[1].mediaTypes.banner.sizes[0][0]); + expect(bids[0].height).to.equal(bidRequests[1].mediaTypes.banner.sizes[0][1]); + expect(bids[1].requestId).to.equal(bidRequests[2].bidId); + expect(bids[1].width).to.equal(bidRequests[2].mediaTypes.banner.sizes[0][0]); + expect(bids[1].height).to.equal(bidRequests[2].mediaTypes.banner.sizes[0][1]); + expect(bids[2].requestId).to.equal(bidRequests[0].bidId); + expect(bids[2].width).to.equal(bidRequests[0].mediaTypes.banner.sizes[0][0]); + expect(bids[2].height).to.equal(bidRequests[0].mediaTypes.banner.sizes[0][1]); + }); }); }); @@ -615,18 +898,92 @@ describe('OpenxAdapter', () => { it('should register the pixel iframe from video ad response', () => { let syncs = spec.getUserSyncs( - { iframeEnabled: true }, - [{ body: { pixels: syncUrl } }] + {iframeEnabled: true}, + [{body: {pixels: syncUrl}}] ); - expect(syncs).to.deep.equal([{ type: 'iframe', url: syncUrl }]); + expect(syncs).to.deep.equal([{type: 'iframe', url: syncUrl}]); }); it('should register the default iframe if no pixels available', () => { let syncs = spec.getUserSyncs( - { iframeEnabled: true }, + {iframeEnabled: true}, [] ); - expect(syncs).to.deep.equal([{ type: 'iframe', url: '//u.openx.net/w/1.0/pd' }]); + expect(syncs).to.deep.equal([{type: 'iframe', url: '//u.openx.net/w/1.0/pd'}]); }); }); + + /** + * Makes sure the override object does not introduce + * new fields against the contract + * + * This does a shallow check in order to make key checking simple + * with respect to what a helper handles. For helpers that have + * nested fields, either check your design on maybe breaking it up + * to smaller, manageable pieces + * + * OR just call this on your nth level field if necessary. + * + * @param {Object} override Object with keys that overrides the default + * @param {Object} contract Original object contains the default fields + * @param {string} typeName Name of the type we're checking for error messages + * @throws {Error} + */ + function overrideKeyCheck(override, contract, typeName) { + let invalidKey = Object.keys(override) + .find(key => !(key in contract)); + + if (invalidKey) { + throw new Error(`Mock Error: INVALID_KEY. Key:${invalidKey} does not exist in type: ${typeName}`); + } + } + + /** + * Creates a mock ArjResponse + * @param {OxArjResponse=} response + * @param {Array=} adUnits + * @param {Array=} creatives + * @return {OxArjResponse} + */ + function mockArjResponse(response, adUnits = []) { + let mockedArjResponse = utils.deepClone(DEFAULT_ARJ_RESPONSE); + + if (response) { + overrideKeyCheck(response, DEFAULT_ARJ_RESPONSE, 'OxArjResponse'); + overrideKeyCheck(response.ads, DEFAULT_ARJ_RESPONSE.ads, 'OxArjResponse'); + Object.assign(mockedArjResponse, response); + } + + if (adUnits.length) { + mockedArjResponse.ads.count = adUnits.length; + mockedArjResponse.ads.ad = adUnits.map((adUnit, index) => { + overrideKeyCheck(adUnit, DEFAULT_TEST_ARJ_AD_UNIT, 'OxArjAdUnit'); + return Object.assign(utils.deepClone(DEFAULT_TEST_ARJ_AD_UNIT), adUnit); + }); + } + + return mockedArjResponse; + } + + /** + * Creates a mock ArjAdUnit + * @param {OxArjAdUnit=} adUnit + * @param {OxArjCreative=} creative + * @return {OxArjAdUnit} + */ + function mockAdUnit(adUnit, creative) { + overrideKeyCheck(adUnit, DEFAULT_TEST_ARJ_AD_UNIT, 'OxArjAdUnit'); + + let mockedAdUnit = Object.assign(utils.deepClone(DEFAULT_TEST_ARJ_AD_UNIT), adUnit); + + if (creative) { + overrideKeyCheck(creative, DEFAULT_TEST_ARJ_CREATIVE); + if (creative.tracking) { + overrideKeyCheck(creative.tracking, DEFAULT_TEST_ARJ_CREATIVE.tracking, 'OxArjCreative'); + } + Object.assign(mockedAdUnit.creative[0], creative); + } + + return mockedAdUnit; + } }); From 56bd6f4b72c323b7cea2cf53addaa78b4097b0af Mon Sep 17 00:00:00 2001 From: Jimmy Tu Date: Fri, 27 Apr 2018 10:09:30 -0700 Subject: [PATCH 2/2] OpenX Adapter Spec: Updated overrideKeyCheck to use Chai assertion, which throws an AssertionError, so that it will be caught properly by test suite when an invalid override is provided. --- test/spec/modules/openxBidAdapter_spec.js | 24 +++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/spec/modules/openxBidAdapter_spec.js b/test/spec/modules/openxBidAdapter_spec.js index e9a596a75c2..b763c111998 100644 --- a/test/spec/modules/openxBidAdapter_spec.js +++ b/test/spec/modules/openxBidAdapter_spec.js @@ -514,8 +514,8 @@ describe('OpenxAdapter', () => { pub_rev: '10000', html: '
OpenX Ad
' }; - const adUnit = mockAdUnit(adUnitOverride, creativeOverride); - const bidResponse = mockArjResponse(undefined, [adUnit]); + let adUnit; + let bidResponse; let bid; let bidRequest; @@ -543,6 +543,8 @@ describe('OpenxAdapter', () => { payload: {'bids': bidRequestConfigs, 'startTime': new Date()} }; + adUnit = mockAdUnit(adUnitOverride, creativeOverride); + bidResponse = mockArjResponse(undefined, [adUnit]); bid = spec.interpretResponse({body: bidResponse}, bidRequest)[0]; }); @@ -593,8 +595,8 @@ describe('OpenxAdapter', () => { const adUnitOverride = { deal_id: 'ox-1000' }; - const adUnit = mockAdUnit(adUnitOverride); - const bidResponse = mockArjResponse(null, [adUnit]); + let adUnit; + let bidResponse; let bid; let bidRequestConfigs; @@ -621,6 +623,8 @@ describe('OpenxAdapter', () => { data: {}, payload: {'bids': bidRequestConfigs, 'startTime': new Date()} }; + adUnit = mockAdUnit(adUnitOverride); + bidResponse = mockArjResponse(null, [adUnit]); bid = spec.interpretResponse({body: bidResponse}, bidRequest)[0]; mockArjResponse(); }); @@ -927,22 +931,17 @@ describe('OpenxAdapter', () => { * @param {Object} override Object with keys that overrides the default * @param {Object} contract Original object contains the default fields * @param {string} typeName Name of the type we're checking for error messages - * @throws {Error} + * @throws {AssertionError} */ function overrideKeyCheck(override, contract, typeName) { - let invalidKey = Object.keys(override) - .find(key => !(key in contract)); - - if (invalidKey) { - throw new Error(`Mock Error: INVALID_KEY. Key:${invalidKey} does not exist in type: ${typeName}`); - } + expect(contract).to.include.all.keys(Object.keys(override)); } /** * Creates a mock ArjResponse * @param {OxArjResponse=} response * @param {Array=} adUnits - * @param {Array=} creatives + * @throws {AssertionError} * @return {OxArjResponse} */ function mockArjResponse(response, adUnits = []) { @@ -969,6 +968,7 @@ describe('OpenxAdapter', () => { * Creates a mock ArjAdUnit * @param {OxArjAdUnit=} adUnit * @param {OxArjCreative=} creative + * @throws {AssertionError} * @return {OxArjAdUnit} */ function mockAdUnit(adUnit, creative) {