feat(computed-artifact): support arbitrarily many inputs (#2705)

Author: patrickhulce

lighthouse-core/gather/computed/computed-artifact.js

@@ -17,6 +17,10 @@ class ComputedArtifact {
     this._allComputedArtifacts = allComputedArtifacts;
   }
 
+  get requiredNumberOfArtifacts() {
+    return 1;
+  }
+
   /* eslint-disable no-unused-vars */
 
   /**
@@ -31,21 +35,103 @@ class ComputedArtifact {
     throw new Error('compute_() not implemented for computed artifact ' + this.name);
   }
 
+  /**
+   * This is a helper function for performing cache operations and is responsible for maintaing the
+   * internal cache structure. This function accepts a path of artifacts, used to find the correct
+   * nested cache object, and an operation to perform on that cache with the final key.
+   *
+   * The cache is structured with the first argument occupying the keys of the toplevel cache that point
+   * to the Map of keys for the second argument and so forth until the 2nd to last argument's Map points
+   * to result values rather than further nesting. In the simplest case of a single argument, there
+   * is no nesting and the keys point directly to result values.
+   *
+   *  Map(
+   *    argument1A -> Map(
+   *      argument2A -> result1A2A
+   *      argument2B -> result1A2B
+   *    )
+   *    argument1B -> Map(
+   *      argument2A -> result1B2A
+   *    )
+   *  )
+   *
+   * @param {!Array<*>} artifacts
+   * @param {function(!Map, *)} cacheOperation
+   */
+  _performCacheOperation(artifacts, cacheOperation) {
+    artifacts = artifacts.slice();
+
+    let cache = this._cache;
+    while (artifacts.length > 1) {
+      const nextKey = artifacts.shift();
+      if (cache.has(nextKey)) {
+        cache = cache.get(nextKey);
+      } else {
+        const nextCache = new Map();
+        cache.set(nextKey, nextCache);
+        cache = nextCache;
+      }
+    }
+
+    return cacheOperation(cache, artifacts.shift());
+  }
+
+  /**
+   * Performs a cache.has operation, see _performCacheOperation for more.
+   * @param {!Array<*>} artifacts
+   * @return {boolean}
+   */
+  _cacheHas(artifacts) {
+    return this._performCacheOperation(artifacts, (cache, key) => cache.has(key));
+  }
+
+  /**
+   * Performs a cache.get operation, see _performCacheOperation for more.
+   * @param {!Array<*>} artifacts
+   * @return {*}
+   */
+  _cacheGet(artifacts) {
+    return this._performCacheOperation(artifacts, (cache, key) => cache.get(key));
+  }
+
+  /**
+   * Performs a cache.set operation, see _performCacheOperation for more.
+   * @param {!Array<*>} artifacts
+   * @param {*} result
+   */
+  _cacheSet(artifacts, result) {
+    return this._performCacheOperation(artifacts, (cache, key) => cache.set(key, result));
+  }
+
+  /**
+   * Asserts that the length of the array is the same as the number of inputs the class expects
+   * @param {!Array<*>} artifacts
+   */
+  _assertCorrectNumberOfArtifacts(artifacts) {
+    const actual = artifacts.length;
+    const expected = this.requiredNumberOfArtifacts;
+    if (actual !== expected) {
+      const className = this.constructor.name;
+      throw new Error(`${className} requires ${expected} artifacts but ${actual} were given`);
+    }
+  }
+
   /* eslint-enable no-unused-vars */
 
   /**
    * Request a computed artifact, caching the result on the input artifact.
-   * @param {*} artifact
+   * @param {...*} artifacts
    * @return {!Promise<*>}
    */
-  request(artifact) {
-    if (this._cache.has(artifact)) {
-      return Promise.resolve(this._cache.get(artifact));
+  request(...artifacts) {
+    this._assertCorrectNumberOfArtifacts(artifacts);
+    if (this._cacheHas(artifacts)) {
+      return Promise.resolve(this._cacheGet(artifacts));
     }
 
     const artifactPromise = Promise.resolve()
-      .then(_ => this.compute_(artifact, this._allComputedArtifacts));
-    this._cache.set(artifact, artifactPromise);
+      .then(_ => this.compute_(...artifacts, this._allComputedArtifacts));
+    this._cacheSet(artifacts, artifactPromise);
 
     return artifactPromise;
   }

lighthouse-core/test/gather/computed/computed-artifact-test.js

@@ -12,22 +12,41 @@ const assert = require('assert');
 const ComputedArtifact = require('../../../gather/computed/computed-artifact');
 
 class TestComputedArtifact extends ComputedArtifact {
-  constructor() {
-    super();
+  constructor(...args) {
+    super(...args);
 
+    this.lastArguments = [];
     this.computeCounter = 0;
   }
 
   get name() {
     return 'TestComputedArtifact';
   }
 
-  compute_(_) {
+  compute_(...args) {
+    this.lastArguments = args;
     return this.computeCounter++;
   }
 }
 
+class MultipleInputArtifact extends TestComputedArtifact {
+  get requiredNumberOfArtifacts() {
+    return 2;
+  }
+}
+
 describe('ComputedArtifact base class', () => {
+  it('tests correct number of inputs', () => {
+    const singleInputArtifact = new TestComputedArtifact();
+    const multiInputArtifact = new MultipleInputArtifact();
+
+    return Promise.resolve()
+      .then(_ => singleInputArtifact.request(1))
+      .then(_ => multiInputArtifact.request(1, 2))
+      .then(_ => assert.throws(() => singleInputArtifact.request(1, 2)))
+      .then(_ => assert.throws(() => multiInputArtifact.request(1)));
+  });
+
   it('caches computed artifacts', () => {
     const testComputedArtifact = new TestComputedArtifact();
 
@@ -44,4 +63,23 @@ describe('ComputedArtifact base class', () => {
       assert.equal(result, 1);
     });
   });
+
+  it('caches multiple input arguments', () => {
+    const mockComputed = {computed: true};
+    const computedArtifact = new MultipleInputArtifact(mockComputed);
+
+    const obj0 = {value: 1};
+    const obj1 = {value: 2};
+    const obj2 = {value: 3};
+
+    return computedArtifact.request(obj0, obj1)
+      .then(result => assert.equal(result, 0))
+      .then(_ => assert.deepEqual(computedArtifact.lastArguments, [obj0, obj1, mockComputed]))
+      .then(_ => computedArtifact.request(obj1, obj2))
+      .then(result => assert.equal(result, 1))
+      .then(_ => assert.deepEqual(computedArtifact.lastArguments, [obj1, obj2, mockComputed]))
+      .then(_ => computedArtifact.request(obj0, obj1))
+      .then(result => assert.equal(result, 0))
+      .then(_ => assert.deepEqual(computedArtifact.lastArguments, [obj1, obj2, mockComputed]));
+  });
 });