Introduce expandStringAsync and expandStringSync in url-replacement (#…

…6323)
ampproject · Nov 24, 2016 · c71d9f5 · c71d9f5
1 parent 83416ed
commit c71d9f5
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 7 deletions.
diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js
@@ -610,6 +610,18 @@ var forbiddenTermsSrcInclusive = {
   '\\.getTime\\(\\)': {
     message: 'Unless you do weird date math (whitelist), use Date.now().',
   },
+  '\\.expandStringSync\\(': {
+    message: requiresReviewPrivacy,
+    whitelist: [
+      'src/service/url-replacements-impl.js',
+    ]
+  },
+  '\\.expandStringAsync\\(': {
+    message: requiresReviewPrivacy,
+    whitelist: [
+      'src/service/url-replacements-impl.js',
+    ]
+  },
 };
 
 // Terms that must appear in a source file.

diff --git a/src/service/url-replacements-impl.js b/src/service/url-replacements-impl.js
@@ -524,6 +524,8 @@ export class UrlReplacements {
    * Synchronously expands the provided URL by replacing all known variables with
    * their resolved values. Optional `opt_bindings` can be used to add new
    * variables or override existing ones.  Any async bindings are ignored.
+   *
+   * TODO(mkhatib, #6322): Deprecate and please use expandUrlSync or expandStringSync.
    * @param {string} url
    * @param {!Object<string, (ResolverReturnDef|!SyncResolverDef)>=} opt_bindings
    * @param {!Object<string, ResolverReturnDef>=} opt_collectVars
@@ -532,21 +534,83 @@ export class UrlReplacements {
    * @return {string}
    */
   expandSync(url, opt_bindings, opt_collectVars, opt_whiteList) {
-    return /** @type {string} */(
-        this.expand_(url, opt_bindings, opt_collectVars, /* opt_sync */ true,
-            opt_whiteList));
+    return this.expandUrlSync(
+        url, opt_bindings, opt_collectVars, opt_whiteList);
   }
 
   /**
    * Expands the provided URL by replacing all known variables with their
    * resolved values. Optional `opt_bindings` can be used to add new variables
    * or override existing ones.
+   *
+   * TODO(mkhatib, #6322): Deprecate and please use expandUrlAsync or expandStringAsync.
    * @param {string} url
    * @param {!Object<string, *>=} opt_bindings
    * @return {!Promise<string>}
    */
   expandAsync(url, opt_bindings) {
-    return /** @type {!Promise<string>} */(this.expand_(url, opt_bindings));
+    return this.expandUrlAsync(url, opt_bindings);
+  }
+
+
+  /**
+   * Synchronously expands the provided source by replacing all known variables with
+   * their resolved values. Optional `opt_bindings` can be used to add new
+   * variables or override existing ones.  Any async bindings are ignored.
+   * @param {string} source
+   * @param {!Object<string, (ResolverReturnDef|!SyncResolverDef)>=} opt_bindings
+   * @param {!Object<string, ResolverReturnDef>=} opt_collectVars
+   * @param {!Object<string, boolean>=} opt_whiteList Optional white list of names
+   *     that can be substituted.
+   * @return {string}
+   */
+  expandStringSync(source, opt_bindings, opt_collectVars, opt_whiteList) {
+    return /** @type {string} */ (
+        this.expand_(source, opt_bindings, opt_collectVars, /* opt_sync */ true,
+            opt_whiteList));
+  }
+
+  /**
+   * Expands the provided source by replacing all known variables with their
+   * resolved values. Optional `opt_bindings` can be used to add new variables
+   * or override existing ones.
+   * @param {string} source
+   * @param {!Object<string, *>=} opt_bindings
+   * @return {!Promise<string>}
+   */
+  expandStringAsync(source, opt_bindings) {
+    return /** @type {!Promise<string>} */ (this.expand_(source, opt_bindings));
+  }
+
+  /**
+   * Synchronously expands the provided URL by replacing all known variables with
+   * their resolved values. Optional `opt_bindings` can be used to add new
+   * variables or override existing ones.  Any async bindings are ignored.
+   * @param {string} url
+   * @param {!Object<string, (ResolverReturnDef|!SyncResolverDef)>=} opt_bindings
+   * @param {!Object<string, ResolverReturnDef>=} opt_collectVars
+   * @param {!Object<string, boolean>=} opt_whiteList Optional white list of names
+   *     that can be substituted.
+   * @return {string}
+   */
+  expandUrlSync(url, opt_bindings, opt_collectVars, opt_whiteList) {
+    return this.ensureProtocolMatches_(url, /** @type {string} */ (this.expand_(
+            url, opt_bindings, opt_collectVars, /* opt_sync */ true,
+            opt_whiteList)));
+  }
+
+  /**
+   * Expands the provided URL by replacing all known variables with their
+   * resolved values. Optional `opt_bindings` can be used to add new variables
+   * or override existing ones.
+   * @param {string} url
+   * @param {!Object<string, *>=} opt_bindings
+   * @return {!Promise<string>}
+   */
+  expandUrlAsync(url, opt_bindings) {
+    return /** @type {!Promise<string>} */ (
+        this.expand_(url, opt_bindings).then(
+            replacement => this.ensureProtocolMatches_(url, replacement)));
   }
 
   /**
@@ -687,10 +751,9 @@ export class UrlReplacements {
     }
 
     if (opt_sync) {
-      return this.ensureProtocolMatches_(url, replacement);
+      return replacement;
     }
-    return (replacementPromise || Promise.resolve(replacement))
-        .then(replacement => this.ensureProtocolMatches_(url, replacement));
+    return replacementPromise || Promise.resolve(replacement);
   }
 
   /**

diff --git a/test/functional/test-url-replacements.js b/test/functional/test-url-replacements.js
@@ -1074,4 +1074,32 @@ describe('UrlReplacements', () => {
       });
     });
   });
+
+  describe('Expanding String', () => {
+    it('should not reject protocol changes with expandStringSync', () => {
+      const win = getFakeWindow();
+      const urlReplacements = installUrlReplacementsServiceForDoc(win.ampdoc);
+      let expanded = urlReplacements.expandStringSync(
+          'PROTOCOL://example.com/?r=RANDOM', {
+            'PROTOCOL': 'abc',
+          });
+      expect(expanded).to.match(/abc:\/\/example\.com\/\?r=(\d+(\.\d+)?)$/);
+      expanded = urlReplacements.expandStringSync(
+          'FUNCT://example.com/?r=RANDOM', {
+            'FUNCT': function() { return 'abc'; },
+          });
+      expect(expanded).to.match(/abc:\/\/example\.com\/\?r=(\d+(\.\d+)?)$/);
+    });
+
+    it('should not check protocol changes with expandStringAsync', () => {
+      const win = getFakeWindow();
+      const urlReplacements = installUrlReplacementsServiceForDoc(win.ampdoc);
+      return urlReplacements.expandStringAsync(
+          'RANDOM:X:Y', {
+            'RANDOM': Promise.resolve('abc'),
+          }).then(expanded => {
+            expect(expanded).to.equal('abc:X:Y');
+          });
+    });
+  });
 });