Skip to content

Commit

Permalink
Allow vendors to disable key appending on RTC responses merged for Do…
Browse files Browse the repository at this point in the history
…ubleclick (ampproject#12281)

* Allow vendors to disable key appending

* Fix presubmits

* Responded to feedback

* Fix check-types
  • Loading branch information
bradfrizzell authored and gzgogo committed Jan 26, 2018
1 parent 2a2b054 commit a1c2740
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 9 deletions.
27 changes: 23 additions & 4 deletions extensions/amp-a4a/0.1/callout-vendors.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2016 The AMP HTML Authors. All Rights Reserved.
* Copyright 2017 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -13,21 +13,40 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import {getMode} from '../../../src/mode';

//////////////////////////////////////////////////////////////////
// //
// IMPORTANT: All keys in RTC_VENDORS must be lowercase //
// otherwise the vendor endpoint will not be used. //
// //
//////////////////////////////////////////////////////////////////

// Note: disableKeyAppend is an option specifically for DoubleClick's
// implementation of RTC. It prevents the vendor ID from being
// appended onto each key of the RTC response, for each vendor.
// This appending is done to prevent a collision case during merge
// that would cause one RTC response to overwrite another if they
// share key names.
/** @typedef {{
url: string,
macros: Array<string>}} */
macros: Array<string>,
disableKeyAppend: boolean}} */
let RtcVendorDef;

/** @const {!Object<string, RtcVendorDef>} */
export const RTC_VENDORS = {
'fakevendor': {
// Add vendors here
};

// DO NOT MODIFY: Setup for tests
if (getMode().localDev || getMode().test) {
RTC_VENDORS['fakevendor'] = /** @type {RtcVendorDef} */({
url: 'https://localhost:8000/examples/rtcE1.json?slot_id=SLOT_ID&page_id=PAGE_ID&foo_id=FOO_ID',
macros: ['SLOT_ID', 'PAGE_ID', 'FOO_ID'],
},
});
RTC_VENDORS['fakevendor2'] = /** @type {RtcVendorDef} */({
url: 'https://localhost:8000/examples/rtcE1.json?slot_id=SLOT_ID&page_id=PAGE_ID&foo_id=FOO_ID',
disableKeyAppend: true,
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
*/
rewriteRtcKeys_(response, callout) {
// Only perform this substitution for vendor-defined URLs.
if (!RTC_VENDORS[callout]) {
if (!RTC_VENDORS[callout] || RTC_VENDORS[callout].disableKeyAppend) {
return response;
}
const newResponse = {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describes.realWin('DoubleClick Fast Fetch RTC', {amp: true}, env => {
});

it('should properly merge RTC responses from vendors', () => {
RTC_VENDORS['fakevendor2'] = {
RTC_VENDORS['TEMP_VENDOR'] = {
'url': 'https://fakevendor2.biz',
};
const rtcResponseArray = [
Expand All @@ -95,17 +95,17 @@ describes.realWin('DoubleClick Fast Fetch RTC', {amp: true}, env => {
{response: {targeting: {'a': 'foo', 'b': {e: 'f'}}},
callout: 'www.exampleB.com', rtcTime: 500},
{response: {targeting: {'a': 'bar'}},
callout: 'fakevendor2', rtcTime: 100},
callout: 'TEMP_VENDOR', rtcTime: 100},
];
const expectedParams = {
ati: '2,2,2',
artc: '100,500,100',
ard: 'fakevendor,www.exampleB.com,fakevendor2',
ard: 'fakevendor,www.exampleB.com,TEMP_VENDOR',
};
const expectedJsonTargeting = {
targeting: {
'a': 'foo', 'b': {e: 'f'}, 'a_fakevendor': [1,2,3],
'b_fakevendor': {c: 'd'}, 'a_fakevendor2': 'bar'},
'b_fakevendor': {c: 'd'}, 'a_TEMP_VENDOR': 'bar'},
};
testMergeRtcResponses(
rtcResponseArray, expectedParams, expectedJsonTargeting);
Expand Down Expand Up @@ -252,6 +252,16 @@ describes.realWin('DoubleClick Fast Fetch RTC', {amp: true}, env => {
.to.deep.equal(rewrittenResponse);
});

it('should not rewrite key names if vendor has disableKeyAppend', () => {
const response = {
'a': '1',
'b': '2',
};
// fakevendor2 has disableKeyAppend set to true, see callout-vendors.js
expect(impl.rewriteRtcKeys_(response, 'fakevendor2'))
.to.deep.equal(response);
});

it('should not rewrite key names if custom url callout', () => {
const response = {
'a': '1',
Expand Down

0 comments on commit a1c2740

Please sign in to comment.