Skip to content

Commit

Permalink
tests now pass
Browse files Browse the repository at this point in the history
  • Loading branch information
John Rosendahl committed Aug 22, 2018
1 parent 6565d2e commit 7483190
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 32 deletions.
85 changes: 62 additions & 23 deletions src/adapters/bidderFactory.js
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand All @@ -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
}));
});
}
}
Expand All @@ -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])) {
Expand All @@ -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;

This comment has been minimized.

Sign in to view

Copy link
@rachelrj

rachelrj Aug 22, 2018

Does the requestId always equal the bidId on the bid? If not, is it weird to override it? If so, why isn't this set before?
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');
Expand All @@ -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;
Expand All @@ -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)) {
Expand All @@ -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
},
Expand All @@ -251,8 +288,7 @@ export function newBidder(spec) {
break;
case 'POST':
ajax(
request.url,
{
request.url, {
success: onSuccess,
error: onFailure
},
Expand Down Expand Up @@ -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;
Expand All @@ -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]));
}

This comment has been minimized.

Sign in to view

Copy link
@rachelrj

rachelrj Aug 22, 2018

I don't understand exactly what the different keys are used for. Would you mind explaining to me?
return COMMON_BID_RESPONSE_KEYS.every(key => includes(bidKeys, key) && !includes([undefined, null], bid[key]));
}

Expand Down
1 change: 1 addition & 0 deletions src/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ exports.logWarn = function () {
};

exports.logError = function () {
console.log(arguments)
if (debugTurnedOn() && consoleErrorExists) {
console.error.apply(console, decorateLog(arguments, 'ERROR:'));
}
Expand Down
17 changes: 16 additions & 1 deletion test/spec/modules/pubCommonId_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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');
Expand Down
16 changes: 9 additions & 7 deletions test/spec/unit/core/bidderFactory_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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,
Expand All @@ -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);
});
Expand Down Expand Up @@ -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);
Expand All @@ -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);
});

Expand Down
2 changes: 1 addition & 1 deletion test/spec/unit/pbjs_api_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 7483190

Please sign in to comment.