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

[AD -775][JW8-11235] JWPlayer Targeting module #3

Merged
merged 26 commits into from
Jul 14, 2020
Merged
Show file tree
Hide file tree
Changes from 19 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
3 changes: 2 additions & 1 deletion modules/.submodules.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@
],
"rtdModule": [
"browsiRtdProvider"
]
],
"jwplayerTargeting": []
}
158 changes: 158 additions & 0 deletions modules/jwplayerTargeting.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
import { config } from '../src/config.js';
import { ajaxBuilder } from '../src/ajax.js';
import { logError, isPlainObject } from '../src/utils.js';
import { getGlobal } from '../src/prebidGlobal.js';
import { module } from '../src/hook.js';

const segCache = {};
pajong marked this conversation as resolved.
Show resolved Hide resolved
let requestCount = 0;
let requestTimeout;
let resumeBidRequest;

function setup () {
config.getConfig('jwTargeting', (config) => {
// fetch media ids
fetchTargetingInformation(config.jwTargeting)
});

getGlobal().requestBids.before(onFetchCompletion);
Copy link

Choose a reason for hiding this comment

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

Link to doc on requestBids.before would be great

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's not documented. This was suggested in the ticket https://jwplayer.atlassian.net/browse/JW8-11235 (see section Blocking bid requests).
It allows you to block a bid request. We use this to ensure that a request does not go out until we have finished getting the jwpsegs for the mediaIDs listed in the config.

}

export function fetchTargetingInformation(jwTargeting) {
const mediaIDs = jwTargeting.mediaIDs;
requestCount = mediaIDs.length;
mediaIDs.forEach(mediaID => {
fetchTargetingForMediaId(mediaID);
});
}

export function onFetchCompletion(nextFn, reqBidsConfigObj) {
if (requestCount <= 0) {
nextFn.apply(this, [reqBidsConfigObj]);
return;
}
resumeBidRequest = nextFn.bind(this, reqBidsConfigObj);
requestTimeout = setTimeout(function() {
resumeBidRequest();
resumeBidRequest = null;
requestTimeout = null;
}, 1500);
}

/**
* @param bidRequest {object} - the bid which is passed to a prebid adapter for use in `buildRequests`
* @returns {Array<string>} - an array of jwpseg targeting segments found for the given bidRequest information
*/
export function getTargetingForBid(bidRequest) {
const jwTargeting = bidRequest.jwTargeting;
if (!jwTargeting) {
return [];
}

const playerID = jwTargeting.playerID;
let mediaID = jwTargeting.mediaID;
let segments = segCache[mediaID];
if (segments) {
return segments;
}

const player = getPlayer(playerID);
if (!player) {
return [];
}

let item = mediaID ? player.getPlaylist().find(item => item.mediaid === mediaID) : player.getPlaylistItem();
Copy link

Choose a reason for hiding this comment

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

We can const item

if (!item) {
return [];
}

mediaID = mediaID || item.mediaid;
segments = item.jwpseg;
if (segments && mediaID) {
segCache[mediaID] = segments;
}

return segments || [];
}

function getPlayer(playerID) {
var jwplayer = window.jwplayer;
Copy link

Choose a reason for hiding this comment

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

const

if (!jwplayer) {
logError('jwplayer.js was not found on page');
return;
}

const player = jwplayer(playerID);
if (!player || !player.getPlaylist) {
logError('player ID did not match any players');
return;
}
return player;
}

export function fetchTargetingForMediaId(mediaId) {
const ajax = ajaxBuilder(1500);
pajong marked this conversation as resolved.
Show resolved Hide resolved
ajax(`https://cdn.jwplayer.com/v2/media/${mediaId}`,
{
success: function (response) {
try {
const data = JSON.parse(response);
if (!data) {
pajong marked this conversation as resolved.
Show resolved Hide resolved
return;
}

const playlist = data.playlist;
if (!playlist || !playlist.length) {
return;
}

const jwpseg = playlist[0].jwpseg;
if (jwpseg) {
segCache[mediaId] = jwpseg;
}
} catch (err) {
logError('failed to parse response');
}
onRequestCompleted();
},
error: function () {
logError('failed to retrieve targeting information');
onRequestCompleted();
}
}
);
}

function onRequestCompleted() {
requestCount--;
if (requestCount > 0) {
return;
}
if (requestTimeout) {
clearTimeout(requestTimeout);
requestTimeout = null;
}

if (resumeBidRequest) {
resumeBidRequest();
pajong marked this conversation as resolved.
Show resolved Hide resolved
resumeBidRequest = null;
}
}

setup();
Copy link

Choose a reason for hiding this comment

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

setup is being called as soon as we load this js file, and this is the only place we are calling this function - we can just remove the function


const jwplayerUtilities = {
Copy link

Choose a reason for hiding this comment

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

Is getTargetingForBid the only method in this object?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes!

'getTargetingForBid': getTargetingForBid
Copy link

Choose a reason for hiding this comment

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

You can just do:

const jwplayerUtilities = {
    getTargetingForBid
};

};

module('jwplayerTargeting', function shareJWPlayerUtilities() {
const host = arguments[0];
if (!isPlainObject(host)) {
logError('JW Player module requires plain object to share methods with submodule');
return;
}

for (let method in jwplayerUtilities) {
Copy link

Choose a reason for hiding this comment

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

If jwplayerUtilities is the only method in this jwplayerUtilities, we should simplify this and remove jwplayerUtilities object

host[method] = jwplayerUtilities[method];
}
});
1 change: 0 additions & 1 deletion modules/spotxBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ export const spec = {

const ext = {
sdk_name: 'Prebid 1+',
player_vendor: 'SpotXJW',
Copy link

Choose a reason for hiding this comment

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

We added this a few weeks ago for SpotX to track that the ad is coming from our VPB. Are you sure we want to remove this?
CC: @mamaddox

Choose a reason for hiding this comment

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

Same question. Here is a ticket I created about it: https://jwplayer.atlassian.net/browse/JW8-11226
Now I don't know too much about this parameter, but I know it was added here after the Prebid migration as a workaround for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mamaddox Was this merged upstream ? I saw it was failing SpotX's unit tests. I'll take a look again.

Choose a reason for hiding this comment

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

@karimMourra It's not merged upstream because it doesn't and has never existed there. Probably why the tests are failing.
From what I could gather, it's a parameter that we have always implemented on our side. Before migrating to open source Prebid, we used player_vendor to track the SpotX ads through our own VPB solution. Once we migrated to Prebid, the param was only added back to our fork as a workaround I'm assuming. I didn't see any other Prebid adapters using a param like this and SpotX never used it in their adapter, so I'm assuming that's why the change was never merged upstream. I don't know much about it though tbh.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok now I see. So @mamaddox is VPB using our fork of Prebid.js ?
Adding player_vendor: 'SpotXJW' to spotX's adapter is definitely wrong so i doubt it would be accepted upstream. We probably shouldn't be modifying a vendor's adapter.
If our current build of VPB needs player_vendor: 'SpotXJW' then i'm ok with not merging my branch to our fork, and opening a PR directly with Prebid.js upstream once this PR is approved.

Choose a reason for hiding this comment

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

@karimMourra We do use our fork of Prebid. We are currently pointing to the spotx-min-max-duration branch.
https://github.com/jwplayer/jwplayer-ads-header-bidding/blob/92530d12ca9b0046be466cf1489028421e1a33b9/package.json#L63-L65
You could merge this into our fork since we are using that branch, but I think we would still be in the same situation we've been in regarding this player_vendor param. I tried to spell out everything I knew and could find about the param in that JIRA ticket.
I do agree though that we probably shouldn't just be modifying the adapter on our end.

versionOrtb: ORTB_VERSION
};

Expand Down
Loading