Skip to content

Commit

Permalink
✨ Allow Geo information in url rewriting (#35558)
Browse files Browse the repository at this point in the history
* Looks like this approach works, caching geo proactively

* Move initialization into correct phase per VariableSource definition.

* Type needs to include null

* Test for sync behaviour

* Failure case

* Add unknown test

* Checks

* Comment change

* Don't change amp video tests

* Last two comments on PR

* Cache geo in getGeo_ function
  • Loading branch information
kristoferbaxter committed Aug 10, 2021
1 parent d7a46b7 commit 400d360
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 21 deletions.
4 changes: 3 additions & 1 deletion examples/amp-link-rewriter.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@
<link rel="canonical" href="amps.html">
<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<script async custom-element="amp-geo" src="https://cdn.ampproject.org/v0/amp-geo-0.1.js"></script>
<script async custom-element='amp-link-rewriter' src='https://cdn.ampproject.org/v0/amp-link-rewriter-0.1.js'></script>
<script async src="https://cdn.ampproject.org/v0.js"></script>
</head>
<body>
<amp-geo layout="nodisplay"></amp-geo>
<amp-link-rewriter layout="nodisplay">
<script type="application/json">
{
"output": "https://visit.foo.net/visit?pid=110&url=${href}&cid=${customerId}&mid=${merchantId}&ref=DOCUMENT_REFERRER&location=SOURCE_URL&rel=${rel}",
"output": "https://localhost:8001/visit?c=AMP_GEO(ISOCountry)&r=RANDOM&pid=110&url=${href}&cid=${customerId}&mid=${merchantId}&ref=DOCUMENT_REFERRER&location=SOURCE_URL&rel=${rel}",
"section": [
"#track-section"
],
Expand Down
1 change: 1 addition & 0 deletions extensions/amp-link-rewriter/0.1/link-rewriter.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const REG_DOMAIN_URL = /^(?:https?:)?(?:\/\/)?([^\/?]+)/i;
const PAGE_PROP_ALLOWLIST = {
'SOURCE_URL': true,
'DOCUMENT_REFERRER': true,
'AMP_GEO': true,
};

export class LinkRewriter {
Expand Down
62 changes: 43 additions & 19 deletions src/service/url-replacements-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,39 @@ function screenProperty(screen, property) {
return () => screen[property];
}

/**
*
* @param {Object<string,(string|Array<string>)>|null} geo
* @param {string} geoType
* @return {string}
*/
function geoData(geo, geoType) {
if (geoType) {
userAssert(
geoType === 'ISOCountry',
'The value passed to AMP_GEO() is not valid name:' + geoType
);
return /** @type {string} */ ((geo && geo[geoType]) || 'unknown');
}
return /** @type {string} */ (
geo?.matchedISOCountryGroups.join(GEO_DELIM) || 'unknown'
);
}

/**
* Class to provide variables that pertain to top level AMP window.
*/
export class GlobalVariableSource extends VariableSource {
/**
* @param {!./ampdoc-impl.AmpDoc} ampdoc
*/
constructor(ampdoc) {
super(ampdoc);

/** @private {Object<string,(string|Array<string>)>|null} */
this.cachedGeo_ = null;
}

/**
* Utility function for setting resolver for timing data that supports
* sync and async.
Expand Down Expand Up @@ -112,6 +141,11 @@ export class GlobalVariableSource extends VariableSource {
/** @const {!./viewport/viewport-interface.ViewportInterface} */
const viewport = Services.viewportForDoc(this.ampdoc);

// Greedily cache the geo location if available for synchronous replacements.
Services.geoForDocOrNull(this.ampdoc).then((geo) => {
this.cachedGeo_ = geo;
});

// Returns a random value for cache busters.
this.set('RANDOM', () => Math.random());

Expand Down Expand Up @@ -372,24 +406,10 @@ export class GlobalVariableSource extends VariableSource {
);

// Returns assigned geo value for geoType or all groups.
this.setAsync(
this.setBoth(
'AMP_GEO',
/** @type {AsyncResolverDef} */ (
(geoType) => {
return this.getGeo_((geos) => {
if (geoType) {
userAssert(
geoType === 'ISOCountry',
'The value passed to AMP_GEO() is not valid name:' + geoType
);
return /** @type {string} */ (geos[geoType] || 'unknown');
}
return /** @type {string} */ (
geos.matchedISOCountryGroups.join(GEO_DELIM)
);
}, 'AMP_GEO');
}
)
(geoType) => geoData(this.cachedGeo_, geoType),
(geoType) => this.getGeo_((geo) => geoData(geo, geoType), 'AMP_GEO')
);

// Returns the number of milliseconds since 1 Jan 1970 00:00:00 UTC.
Expand Down Expand Up @@ -766,9 +786,13 @@ export class GlobalVariableSource extends VariableSource {
* @private
*/
getGeo_(getter, expr) {
const element = this.ampdoc.getHeadNode();
return Services.geoForDocOrNull(element).then((geo) => {
if (this.cachedGeo_ !== null) {
return getter(this.cachedGeo_);
}

return Services.geoForDocOrNull(this.ampdoc.getHeadNode()).then((geo) => {
userAssert(geo, 'To use variable %s, amp-geo should be configured', expr);
this.cachedGeo_ = geo;
return getter(geo);
});
}
Expand Down
42 changes: 41 additions & 1 deletion test/unit/test-url-replacements.js
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,7 @@ describes.sandboxed('UrlReplacements', {}, (env) => {
});
});

it('should replace AMP_GEO(ISOCountry) and AMP_GEO', () => {
it('should async replace AMP_GEO(ISOCountry) and AMP_GEO', () => {
env.sandbox.stub(Services, 'geoForDocOrNull').returns(
Promise.resolve({
'ISOCountry': 'unknown',
Expand All @@ -1082,6 +1082,46 @@ describes.sandboxed('UrlReplacements', {}, (env) => {
);
});

it('should sync replace AMP_GEO(ISOCountry) and AMP_GEO', () => {
env.sandbox.stub(Services, 'geoForDocOrNull').returns(
Promise.resolve({
'ISOCountry': 'unknown',
'ISOCountryGroups': ['nafta', 'waldo'],
'nafta': true,
'waldo': true,
'matchedISOCountryGroups': ['nafta', 'waldo'],
})
);
getReplacements().then((replacements) =>
expect(
replacements.expandUrlSync(
'?geo=AMP_GEO,country=AMP_GEO(ISOCountry)'
)
).to.equal('?geo=nafta%2Cwaldo,country=unknown')
);
});

it('should sync replace AMP_GEO(ISOCountry) and AMP_GEO with unknown when geo is not available', () => {
env.sandbox.stub(Services, 'geoForDocOrNull').returns(null);
getReplacements().then((replacements) =>
expect(
replacements.expandUrlSync(
'?geo=AMP_GEO,country=AMP_GEO(ISOCountry)'
)
).to.equal('?geo=unknown,country=unknown')
);
});

it('should sync replace AMP_GEO(ISOCountry) and AMP_GEO with unknown when geo is unknown', () => {
getReplacements().then((replacements) =>
expect(
replacements.expandUrlSync(
'?geo=AMP_GEO,country=AMP_GEO(ISOCountry)'
)
).to.equal('?geo=unknown,country=unknown')
);
});

it.configure()
.skipFirefox()
.run('should accept $expressions', () => {
Expand Down

0 comments on commit 400d360

Please sign in to comment.