From 7483190f9ad7c6f7133c836f83252712d310225d Mon Sep 17 00:00:00 2001 From: John Rosendahl Date: Wed, 22 Aug 2018 10:43:58 -0600 Subject: [PATCH] tests now pass --- src/adapters/bidderFactory.js | 85 +++++++++++++++++------ src/utils.js | 1 + test/spec/modules/pubCommonId_spec.js | 17 ++++- test/spec/unit/core/bidderFactory_spec.js | 16 +++-- test/spec/unit/pbjs_api_spec.js | 2 +- 5 files changed, 89 insertions(+), 32 deletions(-) diff --git a/src/adapters/bidderFactory.js b/src/adapters/bidderFactory.js index b7574f24c08..87399ef368e 100644 --- a/src/adapters/bidderFactory.js +++ b/src/adapters/bidderFactory.js @@ -1,15 +1,30 @@ import Adapter from 'src/adapter'; import adaptermanager from 'src/adaptermanager'; -import { config } from 'src/config'; +import { + config +} from 'src/config'; import bidfactory from 'src/bidfactory'; -import { userSync } from 'src/userSync'; -import { nativeBidIsValid } from 'src/native'; -import { isValidVideoBid } from 'src/video'; +import { + userSync +} from 'src/userSync'; +import { + nativeBidIsValid +} from 'src/native'; +import { + isValidVideoBid +} from 'src/video'; import CONSTANTS from 'src/constants.json'; import events from 'src/events'; import includes from 'core-js/library/fn/array/includes'; -import { logWarn, logError, parseQueryStringParameters, delayExecution, parseSizesInput, getBidderRequest } from 'src/utils'; +import { + logWarn, + logError, + parseQueryStringParameters, + delayExecution, + parseSizesInput, + getBidderRequest +} from 'src/utils'; /** * This file aims to support Adapters during the Prebid 0.x -> 1.x transition. @@ -121,7 +136,8 @@ import { logWarn, logError, parseQueryStringParameters, delayExecution, parseSiz */ // common params for all mediaTypes -const COMMON_BID_RESPONSE_KEYS = ['requestId', 'cpm', 'ttl', 'creativeId', 'netRevenue', 'currency']; +const COMMON_BID_RESPONSE_KEYS = ['requestId']; +const GOOD_BID_RESPONSE_KEYS = ['requestId', 'cpm', 'ttl', 'creativeId', 'netRevenue', 'currency']; /** * Register a bidder with prebid, using the given spec. @@ -131,9 +147,8 @@ const COMMON_BID_RESPONSE_KEYS = ['requestId', 'cpm', 'ttl', 'creativeId', 'netR * @param {BidderSpec} spec An object containing the bare-bones functions we need to make a Bidder. */ export function registerBidder(spec) { - const mediaTypes = Array.isArray(spec.supportedMediaTypes) - ? { supportedMediaTypes: spec.supportedMediaTypes } - : undefined; + const mediaTypes = Array.isArray(spec.supportedMediaTypes) ? {supportedMediaTypes: spec.supportedMediaTypes} : undefined; + function putBidder(spec) { const bidder = newBidder(spec); adaptermanager.registerBidAdapter(bidder, spec.code, mediaTypes); @@ -143,7 +158,9 @@ export function registerBidder(spec) { if (Array.isArray(spec.aliases)) { spec.aliases.forEach(alias => { adaptermanager.aliasRegistry[alias] = spec.code; - putBidder(Object.assign({}, spec, { code: alias })); + putBidder(Object.assign({}, spec, { + code: alias + })); }); } } @@ -156,16 +173,17 @@ export function registerBidder(spec) { */ export function newBidder(spec) { return Object.assign(new Adapter(spec.code), { - getSpec: function() { + getSpec: function () { return Object.freeze(spec); }, registerSyncs, - callBids: function(bidderRequest, addBidResponse, done, ajax) { + callBids: function (bidderRequest, addBidResponse, done, ajax) { if (!Array.isArray(bidderRequest.bids)) { return; } const adUnitCodesHandled = {}; + function addBidWithCode(adUnitCode, bid) { adUnitCodesHandled[adUnitCode] = true; if (isValid(adUnitCode, bid, [bidderRequest])) { @@ -176,8 +194,24 @@ export function newBidder(spec) { // After all the responses have come back, call done() and // register any required usersync pixels. const responses = []; + function afterAllResponses(bids) { const bidsArray = bids ? (bids[0] ? bids : [bids]) : []; + // bidderRequest.end = timestamp(); + let bidIds = bidsArray.map(bid => { + return bid.requestId.toString(); + }); + + let missingBids = bidderRequest.bids.filter(bid => { + return bidIds.indexOf(bid.bidId.toString()) === -1; + }); + + missingBids.forEach(bid => { + const bidRequest = bidRequestMap[bid.bidId]; + bid.requestId = bid.bidId; + let prebidNoBid = Object.assign(bidfactory.createBid(CONSTANTS.STATUS.NO_BID, bidRequest), bid); + addBidWithCode(bid.adUnitCode, prebidNoBid); + }); const videoBid = bidsArray.some(bid => bid.mediaType === 'video'); const cacheEnabled = config.getConfig('cache.url'); @@ -191,16 +225,15 @@ export function newBidder(spec) { // needs to do cleanup before _it_ can be done it should handle that itself in the auction. It should _not_ // require us, the bidders, to conditionally call done. That makes the whole done API very flaky. // As soon as that is refactored, we can move this emit event where it should be, within the done function. - events.emit(CONSTANTS.EVENTS.BIDDER_DONE, bidderRequest); + finishBidder() + } + function finishBidder() { + events.emit(CONSTANTS.EVENTS.BIDDER_DONE, bidderRequest); registerSyncs(responses, bidderRequest.gdprConsent); } const validBidRequests = bidderRequest.bids.filter(filterAndWarn); - if (validBidRequests.length === 0) { - afterAllResponses(); - return; - } const bidRequestMap = {}; validBidRequests.forEach(bid => { bidRequestMap[bid.bidId] = bid; @@ -210,9 +243,14 @@ export function newBidder(spec) { } }); + if (validBidRequests.length === 0) { + afterAllResponses(); + return; + } + let requests = spec.buildRequests(validBidRequests, bidderRequest); if (!requests || requests.length === 0) { - afterAllResponses(); + finishBidder() return; } if (!Array.isArray(requests)) { @@ -237,8 +275,7 @@ export function newBidder(spec) { switch (request.method) { case 'GET': ajax( - `${request.url}${formatGetParameters(request.data)}`, - { + `${request.url}${formatGetParameters(request.data)}`, { success: onSuccess, error: onFailure }, @@ -251,8 +288,7 @@ export function newBidder(spec) { break; case 'POST': ajax( - request.url, - { + request.url, { success: onSuccess, error: onFailure }, @@ -370,7 +406,7 @@ function validBidSize(adUnitCode, bid, bidRequests) { // if a banner impression has one valid size, we assign that size to any bid // response that does not explicitly set width or height if (parsedSizes.length === 1) { - const [ width, height ] = parsedSizes[0].split('x'); + const [width, height] = parsedSizes[0].split('x'); bid.width = width; bid.height = height; return true; @@ -383,6 +419,9 @@ function validBidSize(adUnitCode, bid, bidRequests) { export function isValid(adUnitCode, bid, bidRequests) { function hasValidKeys() { let bidKeys = Object.keys(bid); + if (bid.getStatusCode() === CONSTANTS.STATUS.GOOD) { + return GOOD_BID_RESPONSE_KEYS.every(key => includes(bidKeys, key) && !includes([undefined, null], bid[key])); + } return COMMON_BID_RESPONSE_KEYS.every(key => includes(bidKeys, key) && !includes([undefined, null], bid[key])); } diff --git a/src/utils.js b/src/utils.js index db50745ebcc..2ed548431b2 100644 --- a/src/utils.js +++ b/src/utils.js @@ -280,6 +280,7 @@ exports.logWarn = function () { }; exports.logError = function () { + console.log(arguments) if (debugTurnedOn() && consoleErrorExists) { console.error.apply(console, decorateLog(arguments, 'ERROR:')); } diff --git a/test/spec/modules/pubCommonId_spec.js b/test/spec/modules/pubCommonId_spec.js index bdb9d4f0545..cad78a8c52d 100644 --- a/test/spec/modules/pubCommonId_spec.js +++ b/test/spec/modules/pubCommonId_spec.js @@ -172,7 +172,21 @@ describe('Publisher Common ID', function () { ] }]; adUnitCodes = ['adUnit-code']; - let auction = auctionModule.newAuction({adUnits, adUnitCodes, callback: function() {}, cbTimeout: TIMEOUT}); + let auction = { + addBidReceived: () => {}, + executeCallback: () => {}, + callBids: () => {}, + bidsBackAll: () => {}, + addWinningBid: () => {}, + getWinningBids: () => [], + getTimeout: () => [], + getAuctionId: () => [], + getAuctionStatus: () => [], + getAdUnits: () => [], + getAdUnitCodes: () => [], + getBidRequests: () => [], + getBidsReceived: () => [], + } createAuctionStub = sinon.stub(auctionModule, 'newAuction'); createAuctionStub.returns(auction); initPubcid(); @@ -185,6 +199,7 @@ describe('Publisher Common ID', function () { it('test hook', function() { $$PREBID_GLOBAL$$.requestBids({adUnits}); + adUnits.forEach((unit) => { unit.bids.forEach((bid) => { expect(bid).to.have.deep.property('crumbs.pubcid'); diff --git a/test/spec/unit/core/bidderFactory_spec.js b/test/spec/unit/core/bidderFactory_spec.js index d1422cb1496..f523345b5df 100644 --- a/test/spec/unit/core/bidderFactory_spec.js +++ b/test/spec/unit/core/bidderFactory_spec.js @@ -2,10 +2,9 @@ import { newBidder, registerBidder } from 'src/adapters/bidderFactory'; import adaptermanager from 'src/adaptermanager'; import * as ajax from 'src/ajax'; import { expect } from 'chai'; -import { STATUS } from 'src/constants'; import { userSync } from 'src/userSync' import * as utils from 'src/utils'; -import { config } from 'src/config'; +import CONSTANTS from 'src/constants.json'; const CODE = 'sampleBidder'; const MOCK_BIDS_REQUEST = { @@ -331,12 +330,12 @@ describe('bidders created by newBidder', () => { expect(doneStub.calledOnce).to.equal(true); }); - it('should only add bids for valid adUnit code into the auction, even if the bidder doesn\'t bid on all of them', () => { + it('should only add bids for valid adUnit code into the auction, when a there is no bid it should be recorded as such', () => { const bidder = newBidder(spec); const bid = { creativeId: 'creative-id', - requestId: '1', + requestId: 1, ad: 'ad-url.com', cpm: 0.5, height: 200, @@ -358,8 +357,9 @@ describe('bidders created by newBidder', () => { bidder.callBids(MOCK_BIDS_REQUEST, addBidResponseStub, doneStub, ajaxStub); - expect(addBidResponseStub.calledOnce).to.equal(true); + expect(addBidResponseStub.calledTwice).to.equal(true); expect(addBidResponseStub.firstCall.args[0]).to.equal('mock/placement'); + expect(addBidResponseStub.secondCall.args[1].getStatusCode()).to.equal(CONSTANTS.STATUS.NO_BID) expect(doneStub.calledOnce).to.equal(true); expect(logErrorSpy.callCount).to.equal(0); }); @@ -491,7 +491,7 @@ describe('bidders created by newBidder', () => { expect(doneStub.calledOnce).to.equal(true); }); - it('should not add bids for each adunit code into the auction', () => { + it('should add bids as no_bid for each adunit code into the auction', () => { const bidder = newBidder(spec); spec.isBidRequestValid.returns(true); @@ -505,7 +505,9 @@ describe('bidders created by newBidder', () => { bidder.callBids(MOCK_BIDS_REQUEST, addBidResponseStub, doneStub, ajaxStub); - expect(addBidResponseStub.callCount).to.equal(0); + expect(addBidResponseStub.callCount).to.equal(2); + expect(addBidResponseStub.firstCall.args[1].getStatusCode()).to.equal(CONSTANTS.STATUS.NO_BID) + expect(addBidResponseStub.secondCall.args[1].getStatusCode()).to.equal(CONSTANTS.STATUS.NO_BID) expect(doneStub.calledOnce).to.equal(true); }); diff --git a/test/spec/unit/pbjs_api_spec.js b/test/spec/unit/pbjs_api_spec.js index d46a8d740a5..7171ed4a765 100644 --- a/test/spec/unit/pbjs_api_spec.js +++ b/test/spec/unit/pbjs_api_spec.js @@ -1513,7 +1513,7 @@ describe('Unit: Prebid Module', function () { adUnits: auction2.getAdUnits() }; - assert.equal(auctionManager.getBidsReceived().length, 8, '_bidsReceived contains 8 bids'); + assert.equal(auctionManager.getBidsReceived().length, 9, '_bidsReceived contains 9 bids'); $$PREBID_GLOBAL$$.requestBids(requestObj1); $$PREBID_GLOBAL$$.requestBids(requestObj2);