Skip to content

Commit

Permalink
Make a smaller param for ard, and fix order of params (#14019)
Browse files Browse the repository at this point in the history 
* Make a smaller param for ard, and fix order of params

* Fix lint

* Respond to feedback

* responding to feedback

* fix lint
  • Loading branch information
@bradfrizzell @keithwrightbos
bradfrizzell authored and keithwrightbos committed Mar 20, 2018
1 parent 4623c1f commit 613d8c4
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 26 deletions.
40 changes: 28 additions & 12 deletions extensions/amp-a4a/0.1/real-time-config-manager.js
View file
Expand Up @@ -17,7 +17,10 @@ import {RTC_VENDORS} from './callout-vendors.js';
import {Services} from '../../../src/services';
import {dev, user} from '../../../src/log';
import {isArray, isObject} from '../../../src/types';
import {isSecureUrl} from '../../../src/url';
import {
isSecureUrl,
parseUrl,
} from '../../../src/url';
import {tryParseJson} from '../../../src/json';

/** @type {string} */
Expand Down Expand Up @@ -61,18 +64,30 @@ export const RTC_ERROR_ENUM = {
* @param {string} error
* @param {string} callout
* @param {number=} opt_rtcTime
* @param {boolean=} opt_log
* @return {!Promise<!rtcResponseDef>}
* @private
*/
function buildErrorResponse_(error, callout, opt_rtcTime, opt_log) {
if (opt_log) {
dev().warn(TAG, `Dropping RTC Callout to ${callout} due to ${error}`);
}
function buildErrorResponse_(error, callout, opt_rtcTime) {
dev().warn(TAG, `RTC callout to ${callout} caused ${error}`);
return Promise.resolve(/**@type {rtcResponseDef} */(
{error, callout, rtcTime: opt_rtcTime || 0}));
}

/**
* Converts a URL into its corresponding shortened callout string.
* We also truncate to a maximum length of 50 characters.
* For instance, if we are passed
* "https://example.com/example.php?foo=a&bar=b, then we return
* example.com/example.php
* @param {string} url
* @return {string}
* @visibleForTesting
*/
export function getCalloutParam_(url) {
const parsedUrl = parseUrl(url);
return (parsedUrl.hostname + parsedUrl.pathname).substr(0, 50);
}

/**
* For a given A4A Element, sends out Real Time Config requests to
* any urls or vendors specified by the publisher.
Expand Down Expand Up @@ -106,7 +121,7 @@ export function maybeExecuteRealTimeConfig_(a4aElement, customMacros) {
if (!url) {
return promiseArray.push(
buildErrorResponse_(
RTC_ERROR_ENUM.UNKNOWN_VENDOR, vendor, undefined, true));
RTC_ERROR_ENUM.UNKNOWN_VENDOR, vendor));
}
const validVendorMacros = {};
Object.keys(rtcConfig['vendors'][vendor]).forEach(macro => {
Expand Down Expand Up @@ -142,6 +157,7 @@ export function inflateAndSendRtc_(a4aElement, url, seenUrls, promiseArray,
rtcStartTime, macros, timeoutMillis, opt_vendor) {
const win = a4aElement.win;
const ampDoc = a4aElement.getAmpDoc();
const callout = opt_vendor || getCalloutParam_(url);
/**
* The time that it takes to substitute the macros into the URL can vary
* depending on what the url requires to be substituted, i.e. a long
Expand All @@ -152,22 +168,22 @@ export function inflateAndSendRtc_(a4aElement, url, seenUrls, promiseArray,
if (Object.keys(seenUrls).length == MAX_RTC_CALLOUTS) {
return buildErrorResponse_(
RTC_ERROR_ENUM.MAX_CALLOUTS_EXCEEDED,
opt_vendor || url, undefined, true);
callout);
}
if (!isSecureUrl(url)) {
return buildErrorResponse_(RTC_ERROR_ENUM.INSECURE_URL,
opt_vendor || url, undefined, true);
callout);
}
if (seenUrls[url]) {
return buildErrorResponse_(RTC_ERROR_ENUM.DUPLICATE_URL,
opt_vendor || url, undefined, true);
callout);
}
seenUrls[url] = true;
if (url.length > MAX_URL_LENGTH) {
url = truncUrl_(url);
}
return sendRtcCallout_(
url, rtcStartTime, win, timeoutMillis, opt_vendor || url);
url, rtcStartTime, win, timeoutMillis, callout);
};

const urlReplacements = Services.urlReplacementsForDoc(ampDoc);
Expand All @@ -181,7 +197,7 @@ export function inflateAndSendRtc_(a4aElement, url, seenUrls, promiseArray,
return send(url);
}).catch(unused => {
return buildErrorResponse_(RTC_ERROR_ENUM.MACRO_EXPAND_TIMEOUT,
opt_vendor || url, undefined, true);
callout);
}));
}

Expand Down
50 changes: 38 additions & 12 deletions extensions/amp-a4a/0.1/test/test-real-time-config-manager.js
View file
Expand Up @@ -22,6 +22,7 @@ import '../../../amp-ad/0.1/amp-ad';
import {AmpA4A} from '../amp-a4a';
import {
RTC_ERROR_ENUM,
getCalloutParam_,
inflateAndSendRtc_,
maybeExecuteRealTimeConfig_,
truncUrl_,
Expand Down Expand Up @@ -90,6 +91,22 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
});
});

describe('#getCalloutParam_', () => {
it('should convert url to callout param when parseable', () => {
const url = 'https://www.example.com/endpoint.php?unincluded';
const ard = getCalloutParam_(url);
expect(ard).to.equal('www.example.com/endpoint.php');
});

it('should convert & trunc url when parseable', () => {
const url = 'https://www.example.com/thisIsTooMany' +
'Characters1234567891011121314.php';
const ard = getCalloutParam_(url);
expect(ard).to.equal(
'www.example.com/thisIsTooManyCharacters12345678910');
});
});

describe('#maybeExecuteRealTimeConfig_', () => {
function executeTest(args) {
const {urls, vendors, timeoutMillis, rtcCalloutResponses,
Expand Down Expand Up @@ -137,7 +154,10 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
return urls;
}

function rtcEntry(response, callout, error) {
function rtcEntry(response, url, error, isVendor) {
// If this is an entry for a vendor, then the callout is just
// the vendor name passed in to url here.
const callout = !!isVendor ? url : getCalloutParam_(url);
return response ? {response, callout, rtcTime: 10} :
{callout, error, rtcTime: 10};
}
Expand Down Expand Up @@ -168,7 +188,8 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
const expectedRtcArray = [];
urls.forEach((url, i) => {
expectedRtcArray.push({
callout: url, rtcTime: 10, response: rtcCalloutResponses[i]});
callout: getCalloutParam_(url), rtcTime: 10,
response: rtcCalloutResponses[i]});
});
return executeTest({
urls, inflatedUrls: urls, rtcCalloutResponses, calloutCount,
Expand Down Expand Up @@ -231,7 +252,7 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
const calloutCount = 1;
const expectedRtcArray = [];
expectedRtcArray.push(rtcEntry(rtcCalloutResponses[0],
Object.keys(vendors)[0].toLowerCase()));
Object.keys(vendors)[0].toLowerCase(), undefined, true));
return executeTest({
vendors, customMacros, inflatedUrls, rtcCalloutResponses,
calloutCount, expectedCalloutUrls: inflatedUrls, expectedRtcArray});
Expand All @@ -254,7 +275,7 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
const calloutCount = 1;
const expectedRtcArray = [];
expectedRtcArray.push(rtcEntry(rtcCalloutResponses[0],
Object.keys(vendors)[0].toLowerCase()));
Object.keys(vendors)[0].toLowerCase(), undefined, true));
return executeTest({
vendors, inflatedUrls, rtcCalloutResponses,
calloutCount, expectedCalloutUrls: inflatedUrls, expectedRtcArray});
Expand Down Expand Up @@ -283,7 +304,7 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
rtcEntry(rtcCalloutResponses[i], inflatedUrls[i]));
}
expectedRtcArray.push(rtcEntry(rtcCalloutResponses[4],
Object.keys(vendors)[0].toLowerCase()));
Object.keys(vendors)[0].toLowerCase(), undefined, true));
const calloutCount = 5;
return executeTest({
urls, vendors, customMacros, inflatedUrls, rtcCalloutResponses,
Expand All @@ -301,7 +322,7 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
for (let i = 0; i < 1; i++) {
expectedRtcArray.push(
rtcEntry(rtcCalloutResponses[i],
Object.keys(vendors)[0].toLowerCase()));
Object.keys(vendors)[0].toLowerCase(), undefined, true));
}
const calloutCount = 1;
return executeTest({
Expand Down Expand Up @@ -352,8 +373,10 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
'https://www.0.com/',
];
const expectedRtcArray = [
{response: rtcCalloutResponses[0], callout: urls[0], rtcTime: 10},
{callout: urls[1], error: RTC_ERROR_ENUM.DUPLICATE_URL, rtcTime: 10},
{response: rtcCalloutResponses[0],
callout: getCalloutParam_(urls[0]), rtcTime: 10},
{callout: getCalloutParam_(urls[1]),
error: RTC_ERROR_ENUM.DUPLICATE_URL, rtcTime: 10},
];
return executeTest({
urls, inflatedUrls: urls, rtcCalloutResponses, calloutCount,
Expand All @@ -376,9 +399,12 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
'https://www.2.com',
];
const expectedRtcArray = [
{response: rtcCalloutResponses[0], callout: urls[0], rtcTime: 10},
{response: rtcCalloutResponses[1], callout: urls[1], rtcTime: 10},
{callout: urls[2], error: RTC_ERROR_ENUM.INSECURE_URL, rtcTime: 10},
{response: rtcCalloutResponses[0],
callout: getCalloutParam_(urls[0]), rtcTime: 10},
{response: rtcCalloutResponses[1],
callout: getCalloutParam_(urls[1]), rtcTime: 10},
{callout: getCalloutParam_(urls[2]),
error: RTC_ERROR_ENUM.INSECURE_URL, rtcTime: 10},
];
return executeTest({
urls, inflatedUrls: urls, rtcCalloutResponses, calloutCount,
Expand All @@ -392,7 +418,7 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
const expectedRtcArray = [];
expectedRtcArray.push(
rtcEntry(null, Object.keys(vendors)[0].toLowerCase(),
RTC_ERROR_ENUM.UNKNOWN_VENDOR));
RTC_ERROR_ENUM.UNKNOWN_VENDOR, true));
return executeTest({vendors, calloutCount, expectedRtcArray});
});
it('should handle bad JSON response', () => {
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
View file
Expand Up @@ -613,8 +613,8 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
this.identityToken = results[1];
return googleAdUrl(
this, DOUBLECLICK_BASE_URL, startTime, Object.assign(
this.getBlockParameters_(), rtcParams,
this.buildIdentityParams_(), this.getPageParameters_()));
this.getBlockParameters_(), this.buildIdentityParams_(),
this.getPageParameters_(), rtcParams));
});
this.troubleshootData_.adUrl = urlPromise;
return urlPromise;
Expand Down

0 comments on commit 613d8c4

Please sign in to comment.