From 8a63990335a67b4bacaa9b88f9e43b5463de9888 Mon Sep 17 00:00:00 2001
From: Avi Mehta <avimehta@google.com>
Date: Mon, 21 Nov 2016 17:16:24 -0800
Subject: [PATCH] Tests updates and cleanup.

---
 .../amp-analytics/0.1/test/test-variables.js  | 14 ++++-
 extensions/amp-analytics/0.1/variables.js     | 55 ++++++++-----------
 2 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/extensions/amp-analytics/0.1/test/test-variables.js b/extensions/amp-analytics/0.1/test/test-variables.js
index 3e833e771b73b..adfe0a79b83ed 100644
--- a/extensions/amp-analytics/0.1/test/test-variables.js
+++ b/extensions/amp-analytics/0.1/test/test-variables.js
@@ -89,8 +89,9 @@ describe('amp-analytics.VariableService', function() {
       return check('${bar|default:baz}', 'baz');
     });
 
-    it.skip('hash works', () => {
-      return check('${foo|hash}', 'baz');
+    it('hash works', () => {
+      return check('${foo|hash}',
+          '8R9LfzzIKtjQOwqNEUN5Tw3-oUTgU2UvtufGxDh4wRiiacsW5yga9nqHSYBoBkkp');
     });
     it('substr works', () => {
       return check('${foo|substr:2:4}', 'ello');
@@ -117,5 +118,14 @@ describe('amp-analytics.VariableService', function() {
     it('if works', () => {
       return check('${foo|if:yey:boo}', 'yey');
     });
+
+    it('chaining works', () => {
+      return check('${foo|substr:6}', '%20world!%20').then(() =>
+        check('${foo|substr:6|trim}', 'world!')).then(() =>
+        check('${foo|substr:6|trim|toUpperCase}', 'WORLD!')).then(() =>
+        check('${foo|substr:6|trim|toUpperCase|base64}', 'V09STEQh')).then(() =>
+        check('${foo|substr:6|trim|toUpperCase|base64|hash}',
+          'OPTTt2IGW8-R31MrIF_cRUwLTZ9jLDOXEuhNz_QS7Uc5ZmODduHWdplzrZ7Jsnqx'));
+    });
   });
 });
diff --git a/extensions/amp-analytics/0.1/variables.js b/extensions/amp-analytics/0.1/variables.js
index 18c30ef25f48f..c54f8e86a0745 100644
--- a/extensions/amp-analytics/0.1/variables.js
+++ b/extensions/amp-analytics/0.1/variables.js
@@ -21,12 +21,9 @@ import {cryptoFor} from '../../../src/crypto';
 
 const TAG = 'Analytics.Variables';
 
-/** @typedef {function(...?):!Promise<?>} */
+/** @typedef {function(...?):!Promise<?>|string} */
 let VariableFilterDef;
 
-/** @typedef {function(...?):?} */
-let SyncVariableFilterDef;
-
 /**
  * @param {!string} str
  * @param {Number} s
@@ -35,11 +32,14 @@ let SyncVariableFilterDef;
  */
 function substrFilter(str, s, l) {
   const start = Number(s);
-  const length = Number(l);
+  let length = str.length;
   user().assertNumber(start,
-    'Start index ' + start + 'in substr filter should be a number');
-  user().assertNumber(length,
-    'Length ' + length + ' in substr filter should be a number');
+      'Start index ' + start + 'in substr filter should be a number');
+  if (l) {
+    length = user().assertNumber(Number(l),
+        'Length ' + length + ' in substr filter should be a number');
+  }
+
   return str.substr(start, length);
 }
 
@@ -68,16 +68,16 @@ export class VariableService {
       /** @private {./crypto-impl.Crypto} */
     this.crypto_ = null;
 
-    this.registerSync_('default', defaultFilter);
-    this.registerSync_('substr', substrFilter);
-    this.registerSync_('trim', value => user().assertString(value).trim());
-    this.registerSync_('json', value => JSON.stringify(value));
-    this.registerSync_('toLowerCase', value =>
+    this.register_('default', defaultFilter);
+    this.register_('substr', substrFilter);
+    this.register_('trim', value => user().assertString(value).trim());
+    this.register_('json', value => JSON.stringify(value));
+    this.register_('toLowerCase', value =>
         user().assertString(value).toLowerCase());
-    this.registerSync_('toUpperCase', value =>
+    this.register_('toUpperCase', value =>
         user().assertString(value).toUpperCase());
-    this.registerSync_('not', value => String(!value));
-    this.registerSync_('base64', value => btoa(String(value)));
+    this.register_('not', value => String(!value));
+    this.register_('base64', value => btoa(String(value)));
     this.register_('hash', this.hashFilter_.bind(this));
     this.register_('if', (value, thenValue, elseValue) =>
         Promise.resolve(value ? thenValue : elseValue));
@@ -98,18 +98,6 @@ export class VariableService {
     this.filters_[name] = handler;
   }
 
-  /**
-   * @param {string} name
-   * @param {SyncVariableFilterDef} handler
-   */
-  registerSync_(name, handler) {
-    this.register_(name, (...args) =>
-      Promise.resolve(args[0]).then(value => {
-        args.splice(0, 1, value);
-        return handler.apply(null, args);
-      }));
-  }
-
   /**
    * @param {string} filterStr
    * @return {!Object<string, *>}
@@ -135,17 +123,20 @@ export class VariableService {
   /**
    * @param {string} value
    * @param {Array<string>} filters
-   * @return {string}
+   * @return {Promise<string>}
    */
   applyFilters_(value, filters) {
+    let result = Promise.resolve(value);
     for (let i = 0; i < filters.length; i++) {
       const parsedFilter = this.parseFilter_(filters[i].trim());
       if (parsedFilter) {
-        parsedFilter.args.unshift(value);
-        value = parsedFilter.fn.apply(null, parsedFilter.args);
+        result = result.then(value => {
+          parsedFilter.args.unshift(value);
+          return parsedFilter.fn.apply(null, parsedFilter.args);
+        });
       }
     }
-    return value;
+    return result;
   }
 
   /**