Skip to content

Commit

Permalink
✨ Add requestOrigin field in analytics config (#22454)
Browse files Browse the repository at this point in the history
* add request origin

* expansionOptions fix

* Add changes from comments

* fix tests

* typo

* use URL constructor and move requestOrigin propagation to config.js

* catch URL construct error

* add custom url constructor

* cleanup unused code

* changes from comments

* Don't extract relative path from request and support origin field in request object

* remove unused tests

* add new tests and use URL constructor

* use UrlService

* add comments and tests

* lint
  • Loading branch information
jonathantyng-amp authored and zhouyx committed Jun 21, 2019
1 parent 28bfc51 commit 2888ba7
Show file tree
Hide file tree
Showing 5 changed files with 331 additions and 27 deletions.
24 changes: 23 additions & 1 deletion extensions/amp-analytics/0.1/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,8 @@ export function expandConfigRequest(config) {
config['requests'][k] = expandRequestStr(config['requests'][k]);
}
}
return config;

return handleTopLevelAttributes_(config);
}

/**
Expand All @@ -490,3 +491,24 @@ function expandRequestStr(request) {
'baseUrl': request,
};
}

/**
* Handles top level fields in the given config
* @param {!JsonObject} config
* @return {JsonObject}
*/
function handleTopLevelAttributes_(config) {
// handle a top level requestOrigin
if (hasOwn(config, 'requests') && hasOwn(config, 'requestOrigin')) {
const requestOrigin = config['requestOrigin'];

for (const requestName in config['requests']) {
// only add top level request origin into request if it doesn't have one
if (!hasOwn(config['requests'][requestName], 'origin')) {
config['requests'][requestName]['origin'] = requestOrigin;
}
}
}

return config;
}
112 changes: 88 additions & 24 deletions extensions/amp-analytics/0.1/requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ export class RequestHandler {
/** @const {!Window} */
this.win = this.ampdoc_.win;

/** @const {string} !if specified, all requests are prepended with this */
this.requestOrigin_ = request['origin'];

/** @const {string} */
this.baseUrl = devAssert(request['baseUrl']);

Expand All @@ -66,12 +69,18 @@ export class RequestHandler {
/** @private {!../../../src/service/url-replacements-impl.UrlReplacements} */
this.urlReplacementService_ = Services.urlReplacementsForDoc(element);

/** @private {!../../../src/service/url-impl.Url} */
this.urlService_ = Services.urlForDoc(element);

/** @private {?Promise<string>} */
this.baseUrlPromise_ = null;

/** @private {?Promise<string>} */
this.baseUrlTemplatePromise_ = null;

/** @private {?Promise<string>} */
this.requestOriginPromise_ = null;

/** @private {!Array<!Promise<!BatchSegmentDef>>} */
this.batchSegmentPromises_ = [];

Expand Down Expand Up @@ -135,10 +144,12 @@ export class RequestHandler {

if (!this.baseUrlPromise_) {
expansionOption.freezeVar('extraUrlParams');

this.baseUrlTemplatePromise_ = this.variableService_.expandTemplate(
this.baseUrl,
expansionOption
);

this.baseUrlPromise_ = this.baseUrlTemplatePromise_.then(baseUrl => {
return this.urlReplacementService_.expandUrlAsync(
baseUrl,
Expand All @@ -148,6 +159,29 @@ export class RequestHandler {
});
}

// expand requestOrigin if it is declared
if (!this.requestOriginPromise_ && this.requestOrigin_) {
// do not encode vars in request origin
const requestOriginExpansionOpt = new ExpansionOptions(
expansionOption.vars,
expansionOption.iterations,
true // opt_noEncode
);

this.requestOriginPromise_ = this.variableService_
// expand variables in request origin
.expandTemplate(this.requestOrigin_, requestOriginExpansionOpt)
// substitute in URL values e.g. DOCUMENT_REFERRER -> https://example.com
.then(expandedRequestOrigin => {
return this.urlReplacementService_.expandUrlAsync(
expandedRequestOrigin,
bindings,
this.whiteList_,
true // opt_noEncode
);
});
}

const params = Object.assign({}, configParams, trigger['extraUrlParams']);
const timestamp = this.win.Date.now();
const batchSegmentPromise = expandExtraUrlParams(
Expand Down Expand Up @@ -209,38 +243,48 @@ export class RequestHandler {
*/
fire_() {
const {
requestOriginPromise_: requestOriginPromise,
baseUrlTemplatePromise_: baseUrlTemplatePromise,
baseUrlPromise_: baseUrlPromise,
batchSegmentPromises_: segmentPromises,
} = this;
const trigger = /** @type {!JsonObject} */ (this.lastTrigger_);
this.reset_();

baseUrlTemplatePromise.then(preUrl => {
// preconnect to requestOrigin if available, otherwise baseUrlTemplate
const preconnectPromise = requestOriginPromise
? requestOriginPromise
: baseUrlTemplatePromise;

preconnectPromise.then(preUrl => {
this.preconnect_.url(preUrl, true);
Promise.all([baseUrlPromise, Promise.all(segmentPromises)]).then(
results => {
const baseUrl = results[0];
const batchSegments = results[1];
if (batchSegments.length === 0) {
return;
}
// TODO: iframePing will not work with batch. Add a config validation.
if (trigger['iframePing']) {
userAssert(
trigger['on'] == 'visible',
'iframePing is only available on page view requests.'
);
this.transport_.sendRequestUsingIframe(baseUrl, batchSegments[0]);
} else {
this.transport_.sendRequest(
baseUrl,
batchSegments,
!!this.batchInterval_
);
}
}
);
});

Promise.all([
baseUrlPromise,
Promise.all(segmentPromises),
requestOriginPromise,
]).then(results => {
const requestUrl = this.composeRequestUrl_(results[0], results[2]);

const batchSegments = results[1];
if (batchSegments.length === 0) {
return;
}
// TODO: iframePing will not work with batch. Add a config validation.
if (trigger['iframePing']) {
userAssert(
trigger['on'] == 'visible',
'iframePing is only available on page view requests.'
);
this.transport_.sendRequestUsingIframe(requestUrl, batchSegments[0]);
} else {
this.transport_.sendRequest(
requestUrl,
batchSegments,
!!this.batchInterval_
);
}
});
}

Expand Down Expand Up @@ -327,6 +371,26 @@ export class RequestHandler {
this.refreshBatchInterval_();
}, interval);
}

/**
* Composes a request URL given a base and requestOrigin
* @private
* @param {string} baseUrl
* @param {string=} opt_requestOrigin
* @return {string}
*/
composeRequestUrl_(baseUrl, opt_requestOrigin) {
if (opt_requestOrigin) {
// We expect requestOrigin to always contain the URL origin. In the case
// where requestOrigin has a relative URL, the current page's origin will
// be used. We will simply respect the requestOrigin and baseUrl, we don't
// check if they form a valid URL and request will fail silently
const requestOriginInfo = this.urlService_.parse(opt_requestOrigin);
return requestOriginInfo.origin + baseUrl;
}

return baseUrl;
}
}

/**
Expand Down
91 changes: 91 additions & 0 deletions extensions/amp-analytics/0.1/test/test-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,97 @@ describes.realWin('AnalyticsConfig', {amp: false}, env => {
delete ANALYTICS_CONFIG['-test-venfor'];
});

describe('handles top level fields correctly', () => {
it('propogates requestOrigin into each request object', () => {
ANALYTICS_CONFIG['-test-venfor'] = {
'requestOrigin': 'https://example.com',
'requests': {'test1': '/test1', 'test2': '/test1/test2'},
};

const element = getAnalyticsTag({}, {'type': '-test-venfor'});

return new AnalyticsConfig(element).loadConfig().then(config => {
expect(config['requests']).to.deep.equal({
'test1': {
origin: 'https://example.com',
baseUrl: '/test1',
},
'test2': {
origin: 'https://example.com',
baseUrl: '/test1/test2',
},
});
});
});

it('does not overwrite existing origin in request object', () => {
ANALYTICS_CONFIG['-test-venfor'] = {
'requestOrigin': 'https://toplevel.com',
'requests': {
'test1': {
origin: 'https://nested.com',
baseUrl: '/test1',
},
},
};

const element = getAnalyticsTag({}, {'type': '-test-venfor'});

return new AnalyticsConfig(element).loadConfig().then(config => {
expect(config['requests']).to.deep.equal({
'test1': {
origin: 'https://nested.com',
baseUrl: '/test1',
},
});
});
});

it('handles empty string request origin', () => {
ANALYTICS_CONFIG['-test-venfor'] = {
'requestOrigin': '',
'requests': {
'test1': {
baseUrl: '/test1',
},
},
};

const element = getAnalyticsTag({}, {'type': '-test-venfor'});

return new AnalyticsConfig(element).loadConfig().then(config => {
expect(config['requests']).to.deep.equal({
'test1': {
origin: '',
baseUrl: '/test1',
},
});
});
});

it('handles undefined request origin', () => {
ANALYTICS_CONFIG['-test-venfor'] = {
'requestOrigin': undefined,
'requests': {
'test1': {
baseUrl: '/test1',
},
},
};

const element = getAnalyticsTag({}, {'type': '-test-venfor'});

return new AnalyticsConfig(element).loadConfig().then(config => {
expect(config['requests']).to.deep.equal({
'test1': {
origin: undefined,
baseUrl: '/test1',
},
});
});
});
});

describe('merges requests correctly', () => {
it('inline and vendor both string', () => {
ANALYTICS_CONFIG['-test-venfor'] = {
Expand Down

0 comments on commit 2888ba7

Please sign in to comment.