Skip to content

Commit

Permalink
access-expr: cache evaluations (#27359)
Browse files Browse the repository at this point in the history
* amp-access: cache evaluations

* create cachekey off based on data

* fix tests

* update cache invalidation strategy

* lint

* move tests to all flow through the class

* eval() is a blacklisted name

* relint

* you missed a spot

* callCount.equal(1) --> calledOnce

* private ivar
  • Loading branch information
samouri committed Mar 26, 2020
1 parent d0632df commit 9bb8244
Show file tree
Hide file tree
Showing 3 changed files with 287 additions and 206 deletions.
51 changes: 50 additions & 1 deletion extensions/amp-access/0.1/access-expr.js
Expand Up @@ -14,10 +14,11 @@
* limitations under the License.
*/

import {hasOwn, map} from '../../../src/utils/object';
import {accessParser as parser} from '../../../build/parsers/access-expr-impl';

/**
* Evaluates access expression.
* Evaluates access expressions.
*
* The grammar is defined in the `access-expr-impl.jison` and compiled using
* (Jison)[https://zaach.github.io/jison/] parser. The compilation steps are
Expand All @@ -44,3 +45,51 @@ export function evaluateAccessExpr(expr, data) {
parser.yy = null;
}
}

/**
* AmpAccessEvaluator evaluates amp-access expressions.
* It uses a cache to speed up repeated evals for the same expression.
*/
export class AmpAccessEvaluator {
/** */
constructor() {
/** @type {Object<string, boolean>} */
this.cache = null;

/** @private @type {JsonObject} */
this.lastData_ = null;
}

/**
* Evaluate access expressions, but turn to a cache first.
* Cache is invalidated when given new data.
*
* @param {string} expr
* @param {!JsonObject} data
* @return {boolean}
*/
evaluate(expr, data) {
if (this.lastData_ !== data) {
this.lastData_ = data;
this.cache = map();
}

if (!hasOwn(this.cache, expr)) {
this.cache[expr] = this.eval_(expr, data);
}

return this.cache[expr];
}

/**
* Evaluates access expressions by proxying calls to evaluateAccessExpr.
* This function only exists to be spied on.
*
* @param {string} expr
* @param {!JsonObject} data
* @return {boolean}
*/
eval_(expr, data) {
return evaluateAccessExpr(expr, data);
}
}
7 changes: 5 additions & 2 deletions extensions/amp-access/0.1/amp-access.js
Expand Up @@ -17,14 +17,14 @@
import {AccessSource, AccessType} from './amp-access-source';
import {AccessVars} from './access-vars';
import {ActionTrust} from '../../../src/action-constants';
import {AmpAccessEvaluator} from './access-expr';
import {AmpEvents} from '../../../src/amp-events';
import {CSS} from '../../../build/amp-access-0.1.css';
import {Observable} from '../../../src/observable';
import {Services} from '../../../src/services';
import {cancellation} from '../../../src/error';
import {dev, user, userAssert} from '../../../src/log';
import {dict} from '../../../src/utils/object';
import {evaluateAccessExpr} from './access-expr';
import {getSourceOrigin} from '../../../src/url';
import {getValueForExpr, tryParseJson} from '../../../src/json';
import {installStylesForDoc} from '../../../src/style-installer';
Expand Down Expand Up @@ -101,6 +101,9 @@ export class AccessService {
/** @private {?Promise<string>} */
this.readerIdPromise_ = null;

/** @private {!./access-expr.AmpAccessEvaluator} */
this.evaluator_ = new AmpAccessEvaluator();

/** @const */
this.sources_ = this.parseConfig_();

Expand Down Expand Up @@ -452,7 +455,7 @@ export class AccessService {
*/
applyAuthorizationToElement_(element, response) {
const expr = element.getAttribute('amp-access');
const on = evaluateAccessExpr(expr, response);
const on = this.evaluator_.evaluate(expr, response);
let renderPromise = null;
if (on) {
renderPromise = this.renderTemplates_(element, response);
Expand Down

0 comments on commit 9bb8244

Please sign in to comment.