Skip to content

Prebid core: fix bug with some native assets being lost from ortb native responses #8785

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 17, 2022
Merged

Prebid core: fix bug with some native assets being lost from ortb native responses #8785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e332251
Prebid core: fix bug with some native assets being lost from ortb nat…
dgirardi Aug 4, 2022
7cf6b86
Do not throw if native response contains assets that were not requested
dgirardi Aug 10, 2022
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
109 changes: 52 additions & 57 deletions src/native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,47 +366,22 @@ export function getNativeTargeting(bid, {index = auctionManager.index} = {}) {
return keyValues;
}

/**
* Constructs a message object containing asset values for each of the
* requested data keys.
*/
export function getAssetMessage(data, adObject) {
const message = {
message: 'assetResponse',
adId: data.adId,
assets: [],
};

if (adObject.native.hasOwnProperty('adTemplate')) {
message.adTemplate = getAssetValue(adObject.native['adTemplate']);
} if (adObject.native.hasOwnProperty('rendererUrl')) {
message.rendererUrl = getAssetValue(adObject.native['rendererUrl']);
}

data.assets.forEach(asset => {
const key = getKeyByValue(CONSTANTS.NATIVE_KEYS, asset);
const value = getAssetValue(adObject.native[key]);

message.assets.push({ key, value });
});

return message;
}
const getNativeRequest = (bidResponse) => auctionManager.index.getAdUnit(bidResponse)?.nativeOrtbRequest;

export function getAllAssetsMessage(data, adObject, {getNativeReq = (bidResponse) => auctionManager.index.getAdUnit(bidResponse).nativeOrtbRequest} = {}) {
function assetsMessage(data, adObject, keys, {getNativeReq = getNativeRequest} = {}) {
const message = {
message: 'assetResponse',
adId: data.adId,
};

// Pass to Prebid Universal Creative all assets, the legacy ones + the ortb ones (under ortb property)
const ortbRequest = getNativeReq(adObject);
let nativeReq = adObject.native;
let nativeResp = adObject.native;
const ortbResponse = adObject.native?.ortb;
let legacyResponse = {};
if (ortbRequest && ortbResponse) {
legacyResponse = toLegacyResponse(ortbResponse, ortbRequest);
nativeReq = {
nativeResp = {
...adObject.native,
...legacyResponse
};
Expand All @@ -416,27 +391,40 @@ export function getAllAssetsMessage(data, adObject, {getNativeReq = (bidResponse
}
message.assets = [];

Object.keys(nativeReq).forEach(function(key) {
if (key === 'adTemplate' && nativeReq[key]) {
message.adTemplate = getAssetValue(nativeReq[key]);
} else if (key === 'rendererUrl' && nativeReq[key]) {
message.rendererUrl = getAssetValue(nativeReq[key]);
(keys == null ? Object.keys(nativeResp) : keys).forEach(function(key) {
if (key === 'adTemplate' && nativeResp[key]) {
message.adTemplate = getAssetValue(nativeResp[key]);
} else if (key === 'rendererUrl' && nativeResp[key]) {
message.rendererUrl = getAssetValue(nativeResp[key]);
} else if (key === 'ext') {
Object.keys(nativeReq[key]).forEach(extKey => {
if (nativeReq[key][extKey]) {
const value = getAssetValue(nativeReq[key][extKey]);
Object.keys(nativeResp[key]).forEach(extKey => {
if (nativeResp[key][extKey]) {
const value = getAssetValue(nativeResp[key][extKey]);
message.assets.push({ key: extKey, value });
}
})
} else if (nativeReq[key] && CONSTANTS.NATIVE_KEYS.hasOwnProperty(key)) {
const value = getAssetValue(nativeReq[key]);
} else if (nativeResp[key] && CONSTANTS.NATIVE_KEYS.hasOwnProperty(key)) {
const value = getAssetValue(nativeResp[key]);

message.assets.push({ key, value });
}
});
return message;
}

/**
* Constructs a message object containing asset values for each of the
* requested data keys.
*/
export function getAssetMessage(data, adObject, {getNativeReq = getNativeRequest} = {}) {
const keys = data.assets.map((k) => getKeyByValue(CONSTANTS.NATIVE_KEYS, k));
return assetsMessage(data, adObject, keys, {getNativeReq});
}

export function getAllAssetsMessage(data, adObject, {getNativeReq = getNativeRequest} = {}) {
return assetsMessage(data, adObject, null, {getNativeReq});
}

/**
* Native assets can be a string or an object with a url prop. Returns the value
* appropriate for sending in adserver targeting or placeholder replacement.
Expand Down Expand Up @@ -689,40 +677,47 @@ export function legacyPropertiesToOrtbNative(legacyNative) {
}

export function toOrtbNativeResponse(legacyResponse, ortbRequest) {
// copy the request, so we don't pollute it with response data below
ortbRequest = deepClone(ortbRequest);

const ortbResponse = {
...legacyPropertiesToOrtbNative(legacyResponse),
assets: []
};

function useRequestAsset(predicate, fn) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is minor, the function name could be clearer to note it applies a request asset to a response.

let asset = ortbRequest.assets.find(predicate);
if (asset != null) {
asset = deepClone(asset);
fn(asset);
ortbResponse.assets.push(asset);
}
}

Object.keys(legacyResponse).filter(key => !!legacyResponse[key]).forEach(key => {
const value = legacyResponse[key];
switch (key) {
// process titles
case 'title':
const titleAsset = ortbRequest.assets.find(asset => asset.title != null);
titleAsset.title = {
text: value
};
ortbResponse.assets.push(titleAsset);
useRequestAsset(asset => asset.title != null, titleAsset => {
titleAsset.title = {
text: value
};
})
break;
case 'image':
case 'icon':
const imageType = key === 'image' ? NATIVE_IMAGE_TYPES.MAIN : NATIVE_IMAGE_TYPES.ICON;
const imageAsset = ortbRequest.assets.find(asset => asset.img != null && asset.img.type == imageType);
imageAsset.img = {
url: value
};
ortbResponse.assets.push(imageAsset);
useRequestAsset(asset => asset.img != null && asset.img.type === imageType, imageAsset => {
imageAsset.img = {
url: value
};
})
break;
default:
if (key in PREBID_NATIVE_DATA_KEYS_TO_ORTB) {
const dataAsset = ortbRequest.assets.find(asset => asset.data != null && asset.data.type === NATIVE_ASSET_TYPES[PREBID_NATIVE_DATA_KEYS_TO_ORTB[key]]);
dataAsset.data = {
value
};
ortbResponse.assets.push(dataAsset);
useRequestAsset(asset => asset.data != null && asset.data.type === NATIVE_ASSET_TYPES[PREBID_NATIVE_DATA_KEYS_TO_ORTB[key]], dataAsset => {
dataAsset.data = {
value
};
})
}
break;
}
Expand Down
96 changes: 96 additions & 0 deletions test/spec/native_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,66 @@ describe('native.js', function () {
value: bid.native.ext.foo,
});
});

const SAMPLE_ORTB_REQUEST = toOrtbNativeRequest({
title: 'vtitle',
body: 'vbody'
});
const SAMPLE_ORTB_RESPONSE = {
native: {
ortb: {
link: {
url: 'url'
},
assets: [
{
id: 0,
title: {
text: 'vtitle'
}
},
{
id: 1,
data: {
value: 'vbody'
}
}
]
}
}
}
describe('getAllAssetsMessage', () => {
it('returns assets in legacy format for ortb responses', () => {
const actual = getAllAssetsMessage({}, SAMPLE_ORTB_RESPONSE, {getNativeReq: () => SAMPLE_ORTB_REQUEST});
expect(actual.assets).to.eql([
{
key: 'clickUrl',
value: 'url'
},
{
key: 'title',
value: 'vtitle'
},
{
key: 'body',
value: 'vbody'
},
])
});
});
describe('getAssetsMessage', () => {
Object.entries({
'hb_native_title': {key: 'title', value: 'vtitle'},
'hb_native_body': {key: 'body', value: 'vbody'}
}).forEach(([tkey, assetVal]) => {
it(`returns ${tkey} asset in legacy format for ortb responses`, () => {
const actual = getAssetMessage({
assets: [tkey]
}, SAMPLE_ORTB_RESPONSE, {getNativeReq: () => SAMPLE_ORTB_REQUEST})
expect(actual.assets).to.eql([assetVal])
})
})
})
});

describe('validate native openRTB', function () {
Expand Down Expand Up @@ -1022,3 +1082,39 @@ describe('fireClickTrackers', () => {
urls.forEach(url => sinon.assert.calledWith(fetchURL, url));
})
})

describe('toOrtbNativeResponse', () => {
it('should work when there are unrequested assets in the response', () => {
const legacyResponse = {
'title': 'vtitle',
'body': 'vbody'
}
const request = toOrtbNativeRequest({
title: {
required: 'true'
},

});
const ortbResponse = toOrtbNativeResponse(legacyResponse, request);
expect(ortbResponse.assets.length).to.eql(1);
});

it('should not modify the request', () => {
const legacyResponse = {
title: 'vtitle'
}
const request = toOrtbNativeRequest({
title: {
required: true
}
});
const requestCopy = JSON.parse(JSON.stringify(request));
const response = toOrtbNativeResponse(legacyResponse, request);
expect(request).to.eql(requestCopy);
sinon.assert.match(response.assets[0], {
title: {
text: 'vtitle'
}
})
})
})