Skip to content

Commit

Permalink
another approach
Browse files Browse the repository at this point in the history
  • Loading branch information
@zhouyx
zhouyx committed Oct 18, 2016
1 parent 397d7fc commit 366a535
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 128 deletions.
20 changes: 6 additions & 14 deletions build-system/server.js
View file
Expand Up @@ -407,21 +407,13 @@ app.use('/examples/amp-fresh.amp.(min.|max.)?html', function(req, res, next) {

app.use('/impression-proxy/', function(req, res) {
assertCors(req, res, ['GET']);
const fakeHeaders = {
'Host': 'adclick.g.doubleclick.net',
'Origin': 'https://cdn.ampproject.org',
'Cookie': 'Cookie:id=4903ef7b59596f28||t=1451370221|et=730|cs=002213fd48e4405e4579c95dfa',
// TODO(@zhouyx): How to request real ad server response with cookie set
const body = {
'location': 'localhost:8000/examples/#gclid=1234',
'gclid': '1234',
'tracking': 'tracking_url',
};
const options = {
url: 'https://googleads.g.doubleclick.net/pcs/click?alp=1&xai=AKAOjstvASJmd6-NeoO3ner8FBNJW2w8n-sXMo0Nj5YC_LIY2NQjbNs0CoXQtM9tPi8by4H4bHRpMdB14qgRLctKkBKkh3vpR3m8fvPCzcFZ6HrxvxXUqzP17YJsihcINtRniOfmGFkzIolJ3ccPSPq6oYdJpg5lPeufOrLhtWsNspOsRgMSBFP7zH0l8tgtAb665jHEFmdAMH1vl69BxpqU2Q0ZoGDO_SVBMArlL--2nLOVgQt8om6IdzkcodppT9c&sig=Cg0ArKJSzEJGKs3-NmXNEAE&urlfix=1&adurl=https://cdn.ampproject.org/c/www.nbcnews.com/news/us-news/amp/milwaukee-cop-cars-smashed-torched-after-police-kill-suspect-n630236',
headers: fakeHeaders,
};

request(options, (error, response, body) => {
if (!error && response.statusCode == 200) {
res.send(body);
}
});
res.send(body);
});

// Proxy with unminified JS.
Expand Down
69 changes: 33 additions & 36 deletions src/impression.js
View file
Expand Up @@ -14,37 +14,47 @@
* limitations under the License.
*/

import {user} from './log';
import {dev, user} from './log';
import {isExperimentOn} from './experiments';
import {viewerForDoc} from './viewer';
import {xhrFor} from './xhr';
import {getMode} from './mode';
import {openWindowDialog} from './dom';

/** A promise that resolve after getting info from ad server
* {Promise=}
*/
export let trackImpressionPromise;

/**
* Emit a HTTP request to a destination defined on the incoming URL.
* Protected by experiment.
* @param {!Window} win
*/
export function maybeTrackImpression(win) {
let resolveImpression;

trackImpressionPromise = new Promise(resolve => {
resolveImpression = resolve;
});

if (!isExperimentOn(win, 'alp')) {
resolveImpression();
return;
}

const viewer = viewerForDoc(win.document);
/** @const {string|undefined} */
let clickUrl = viewer.getParam('click');
const clickUrl = viewer.getParam('click');

// One test clickUrl
clickUrl = 'https://googleads.g.doubleclick.net/pcs/click?alp=1&xai=AKAOjstvASJmd6-NeoO3ner8FBNJW2w8n-sXMo0Nj5YC_LIY2NQjbNs0CoXQtM9tPi8by4H4bHRpMdB14qgRLctKkBKkh3vpR3m8fvPCzcFZ6HrxvxXUqzP17YJsihcINtRniOfmGFkzIolJ3ccPSPq6oYdJpg5lPeufOrLhtWsNspOsRgMSBFP7zH0l8tgtAb665jHEFmdAMH1vl69BxpqU2Q0ZoGDO_SVBMArlL--2nLOVgQt8om6IdzkcodppT9c&sig=Cg0ArKJSzEJGKs3-NmXNEAE&urlfix=1&adurl=https://cdn.ampproject.org/c/www.nbcnews.com/news/us-news/amp/milwaukee-cop-cars-smashed-torched-after-police-kill-suspect-n630236';
if (!clickUrl) {
resolveImpression();
return;
}
if (clickUrl.indexOf('https://') != 0) {
user().warn('Impression',
'click fragment param should start with https://. Found ',
clickUrl);
resolveImpression();
return;
}
if (win.location.hash) {
Expand All @@ -57,11 +67,9 @@ export function maybeTrackImpression(win) {
viewer.whenFirstVisible().then(() => {
// TODO(@zhouyx) We need a timeout here?
// TODO(@zhouyx) need test with a real response.
invoke(win, clickUrl).then(response => {
if (!response) {
return;
}
invoke(win, dev().assertString(clickUrl)).then(response => {
applyResponse(win, viewer, response);
resolveImpression();
});
});
}
Expand All @@ -70,7 +78,7 @@ export function maybeTrackImpression(win) {
* Send the url to ad server and wait for its response
* @param {!Window} win
* @param {string} clickUrl
* @return {!Promise}
* @return {!Promise<!JSONType>}
*/
function invoke(win, clickUrl) {
if (getMode().localDev && !getMode().test) {
Expand All @@ -80,7 +88,6 @@ function invoke(win, clickUrl) {
credentials: 'include',
requireAmpResponseSourceOrigin: true,
});
// TODO(@cramforce): Do something with the result.
}

/**
Expand All @@ -91,34 +98,24 @@ function invoke(win, clickUrl) {
*/
function applyResponse(win, viewer, response) {
const adLocation = response['location'];
const adTracking = response['tracking'];

// If there's a tracking_url, need to redirect to that url.
const adTrackingUrl = response['tracking_url'];
if (adTrackingUrl) {
// TODO(@zhouyx) Confirm we assume the tracking_url is not
// a cdn.ampproject page, and we don't do viewer redirect
openWindowDialog(win, adTrackingUrl, '_top');
return;
// If there is a tracking_url, need to track it
// Otherwise track the location
const trackUrl = adTracking || adLocation;
if (trackUrl) {
new Image().src = trackUrl;
}

// If adLocation is not an amp page, need to redirect to it.
if (adLocation && adLocation.indexOf('https://cdn.ampproject.org') != 0) {
// TODO(@zhouyx) Confirm we assume the tracking_url is not
// a cdn.ampproject page, and we don't do viewer redirect
openWindowDialog(win, adLocation, '_top');
return;
}

// If have gclid, or location contains gclid. add it to location hash
// Set gclid.
// If provided use it, if not try to find in adLocation from response
let gclid = response['gclid'];
if (!gclid) {
if (adLocation.indexOf('gclid=') > 0) {
gclid = adLocation.substr(adLocation.indexOf('gclid=') + 6);
// If have we have gclid replace the location href with new location we get.
// TODO(@zhouyx) should we replaceState directly w/o checking gclid?
const gclid = response['gclid'];
if (gclid && adLocation) {
if (getMode().localDev && !getMode().test) {
win.history.replaceState(null, '', win.location.href +
'#gclid=' + gclid);
return;
}
}
if (gclid) {
win.location.hash = '#gclid=' + gclid;
win.history.replaceState(null, '', adLocation);
}
}
31 changes: 31 additions & 0 deletions src/service/url-replacements-impl.js
View file
Expand Up @@ -29,6 +29,7 @@ import {viewportForDoc} from '../viewport';
import {userNotificationManagerFor} from '../user-notification';
import {activityFor} from '../activity';
import {isExperimentOn} from '../experiments';
import {trackImpressionPromise} from '../impression.js';


/** @private @const {string} */
Expand Down Expand Up @@ -163,6 +164,18 @@ export class UrlReplacements {
return removeFragment(info.sourceUrl);
}));

this.setAsync_('SOURCE_URL', () => {
let promise = trackImpressionPromise;
if (!promise) {
promise = Promise.resolve();
}
return promise.then(
this.getDocInfoValue_.bind(this, info => {
return removeFragment(info.sourceUrl);
})
);
});

// Returns the host of the Source URL for this AMP document.
this.set_('SOURCE_HOST', this.getDocInfoValue_.bind(this, info => {
return parseUrl(info.sourceUrl).host;
Expand Down Expand Up @@ -197,6 +210,24 @@ export class UrlReplacements {
defaultValue;
});

this.setAsync_('QUERY_PARAM', (param, defaultValue = '') => {
user().assert(param,
'The first argument to QUERY_PARAM, the query string ' +
'param is required');
let promise = trackImpressionPromise;
if (!promise) {
promise = Promise.resolve();
}
return promise.then(() => {
const url = parseUrl(this.ampdoc.win.location.href);
const params = parseQueryString(url.search);

return (typeof params[param] !== 'undefined') ?
params[param] :
defaultValue;
});
});

/**
* Stores client ids that were generated during this page view
* indexed by scope.
Expand Down
87 changes: 10 additions & 77 deletions test/functional/test-impression.js
View file
Expand Up @@ -14,11 +14,13 @@
* limitations under the License.
*/

import {maybeTrackImpression} from '../../src/impression';
import {
trackImpressionPromise,
maybeTrackImpression,
} from '../../src/impression';
import {toggleExperiment} from '../../src/experiments';
import {viewerForDoc} from '../../src/viewer';
import {xhrFor} from '../../src/xhr';
import * as dom from '../../src/dom';
import * as sinon from 'sinon';

describe('impression', () => {
Expand Down Expand Up @@ -49,20 +51,23 @@ describe('impression', () => {
it('should do nothing if the experiment is off', () => {
viewer.getParam.throws(new Error('Should not be called'));
maybeTrackImpression(window);
return trackImpressionPromise.should.be.fulfilled;
});

it('should do nothing if there is no click arg', () => {
toggleExperiment(window, 'alp', true);
viewer.getParam.withArgs('click').returns('');
maybeTrackImpression(window);
expect(xhr.fetchJson.callCount).to.equal(0);
return trackImpressionPromise.should.be.fulfilled;
});

it('should do nothing if there is the click arg is http', () => {
toggleExperiment(window, 'alp', true);
viewer.getParam.withArgs('click').returns('http://www.example.com');
maybeTrackImpression(window);
expect(xhr.fetchJson.callCount).to.equal(0);
return trackImpressionPromise.should.be.fulfilled;
});

it('should invoke URL', () => {
Expand All @@ -82,93 +87,21 @@ describe('impression', () => {
});
});

it('should redirect if xhr return tracking url', () => {
it('should replace location href', () => {
toggleExperiment(window, 'alp', true);
viewer.getParam.withArgs('click').returns('https://www.example.com');

const openWindowDialogSpy = sandbox.stub(dom, 'openWindowDialog',
(win, url, target) => {
expect(url).to.equal('test_tracking_url');
expect(target).to.equal('_top');
});

xhr.fetchJson = () => {
return Promise.resolve({
'location': 'test_location',
'tracking_url': 'test_tracking_url',
});
};

maybeTrackImpression(window);
return Promise.resolve().then(() => {
return Promise.resolve().then(() => {
expect(openWindowDialogSpy).to.be.calledOnce;
});
});
});

it('should redirect if location is some other url', () => {
toggleExperiment(window, 'alp', true);
viewer.getParam.withArgs('click').returns('https://www.example.com');

const openWindowDialogSpy = sandbox.stub(dom, 'openWindowDialog',
(win, url, target) => {
expect(url).to.equal('test_location');
expect(target).to.equal('_top');
});

xhr.fetchJson = () => {
return Promise.resolve({
'location': 'test_location',
});
};

maybeTrackImpression(window);
return Promise.resolve().then(() => {
return Promise.resolve().then(() => {
expect(openWindowDialogSpy).to.be.calledOnce;
});
});
});

it('should not redirect if location is valid', () => {
toggleExperiment(window, 'alp', true);
viewer.getParam.withArgs('click').returns('https://www.example.com');

const openWindowDialogSpy = sandbox.spy(dom, 'openWindowDialog');

xhr.fetchJson = () => {
return Promise.resolve({
'location': 'https://cdn.ampproject.org/c/test/?gclid=654321',
});
};

maybeTrackImpression(window);
return Promise.resolve().then(() => {
return Promise.resolve().then(() => {
expect(openWindowDialogSpy).to.not.be.called;
expect(window.location.hash).to.equal('#gclid=654321');
});
});
});

it('should set gclid to location hash', () => {
toggleExperiment(window, 'alp', true);
viewer.getParam.withArgs('click').returns('https://www.example.com');

const openWindowDialogSpy = sandbox.spy(dom, 'openWindowDialog');

xhr.fetchJson = () => {
return Promise.resolve({
'gclid': '123456',
});
};

maybeTrackImpression(window);
return Promise.resolve().then(() => {
return Promise.resolve().then(() => {
expect(openWindowDialogSpy).to.not.be.called;
expect(window.location.hash).to.be.equal('#gclid=123456');
expect(window.location.href).to.contain('test_location');
return trackImpressionPromise.should.be.fulfilled;
});
});
});
Expand Down

0 comments on commit 366a535

Please sign in to comment.