From 57de7b2f1d21ceebb7097552c86721d94cac2275 Mon Sep 17 00:00:00 2001 From: Zixuan Liu Date: Sun, 17 May 2020 22:44:46 +0800 Subject: [PATCH] feat: improve effector for improve performance BREAKING CHANGE: - provides a new interface for Effector --- src/coreEnforcer.ts | 51 +++++++++------------- src/effect/defaultEffector.ts | 37 +++------------- src/effect/defaultEffectorStream.ts | 68 +++++++++++++++++++++++++++++ src/effect/effector.ts | 5 ++- src/effect/effectorStream.ts | 22 ++++++++++ src/effect/index.ts | 4 +- 6 files changed, 122 insertions(+), 65 deletions(-) create mode 100644 src/effect/defaultEffectorStream.ts create mode 100644 src/effect/effectorStream.ts diff --git a/src/coreEnforcer.ts b/src/coreEnforcer.ts index 352bdf0..081c156 100644 --- a/src/coreEnforcer.ts +++ b/src/coreEnforcer.ts @@ -12,16 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { compileAsync, compile } from 'expression-eval'; +import { compile, compileAsync } from 'expression-eval'; import { DefaultEffector, Effect, Effector } from './effect'; import { FunctionMap, Model, newModel } from './model'; import { Adapter, Filter, FilteredAdapter, Watcher } from './persist'; import { DefaultRoleManager, RoleManager } from './rbac'; -import { generateGFunction, hasEval, getEvalValue, escapeAssertion, replaceEval } from './util'; +import { escapeAssertion, generateGFunction, getEvalValue, hasEval, replaceEval } from './util'; import { getLogger, logPrint } from './log'; type Matcher = ((context: object) => Promise) | ((context: object) => any); + /** * CoreEnforcer defines the core functionality of an enforcer. */ @@ -286,8 +287,8 @@ export class CoreEnforcer { throw new Error('Unable to find matchers in model'); } - const effect = this.model.model.get('e')?.get('e')?.value; - if (!effect) { + const effectExpr = this.model.model.get('e')?.get('e')?.value; + if (!effectExpr) { throw new Error('Unable to find policy_effect in model'); } @@ -298,19 +299,15 @@ export class CoreEnforcer { expression = this.getExpression(asyncCompile, expString); } - let policyEffects: Effect[]; - let matcherResults: number[]; - const p = this.model.model.get('p')?.get('p'); const policyLen = p?.policy?.length; const rTokens = this.model.model.get('r')?.get('r')?.tokens; const rTokensLen = rTokens?.length; - if (policyLen && policyLen !== 0) { - policyEffects = new Array(policyLen); - matcherResults = new Array(policyLen); + const effectStream = this.eft.newStream(effectExpr); + if (policyLen && policyLen !== 0) { for (let i = 0; i < policyLen; i++) { const parameters: { [key: string]: any } = {}; @@ -347,19 +344,16 @@ export class CoreEnforcer { result = asyncCompile ? await expression(context) : expression(context); } + let eftRes: Effect; switch (typeof result) { case 'boolean': - if (!result) { - policyEffects[i] = Effect.Indeterminate; - continue; - } + eftRes = result ? Effect.Allow : Effect.Indeterminate; break; case 'number': if (result === 0) { - policyEffects[i] = Effect.Indeterminate; - continue; + eftRes = Effect.Indeterminate; } else { - matcherResults[i] = result; + eftRes = result; } break; default: @@ -367,26 +361,23 @@ export class CoreEnforcer { } const eft = parameters['p_eft']; - if (eft) { + if (eft && eftRes === Effect.Allow) { if (eft === 'allow') { - policyEffects[i] = Effect.Allow; + eftRes = Effect.Allow; } else if (eft === 'deny') { - policyEffects[i] = Effect.Deny; + eftRes = Effect.Deny; } else { - policyEffects[i] = Effect.Indeterminate; + eftRes = Effect.Indeterminate; } - } else { - policyEffects[i] = Effect.Allow; } - if (effect === 'priority(p_eft) || deny') { + const [res, done] = effectStream.pushEffect(eftRes); + + if (done) { break; } } } else { - policyEffects = new Array(1); - matcherResults = new Array(1); - const parameters: { [key: string]: any } = {}; rTokens?.forEach((token, j): void => { @@ -405,13 +396,13 @@ export class CoreEnforcer { } if (result) { - policyEffects[0] = Effect.Allow; + effectStream.pushEffect(Effect.Allow); } else { - policyEffects[0] = Effect.Indeterminate; + effectStream.pushEffect(Effect.Indeterminate); } } - const res = this.eft.mergeEffects(effect, policyEffects, matcherResults); + const res = effectStream.current(); // only generate the request --> result string if the message // is going to be logged. diff --git a/src/effect/defaultEffector.ts b/src/effect/defaultEffector.ts index f6cd26f..0ee10bb 100644 --- a/src/effect/defaultEffector.ts +++ b/src/effect/defaultEffector.ts @@ -12,42 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { Effect, Effector } from './effector'; +import { Effector } from './effector'; +import { EffectorStream } from './effectorStream'; +import { DefaultEffectorStream } from './defaultEffectorStream'; /** * DefaultEffector is default effector for Casbin. */ export class DefaultEffector implements Effector { - /** - * mergeEffects merges all matching results collected by the enforcer into a single decision. - * @param {string} expr - * @param {Effect[]} effects - * @param {number[]} results - * @returns {boolean} - */ - public mergeEffects(expr: string, effects: Effect[], results: number[]): boolean { - let result = false; - - if (expr === 'some(where (p_eft == allow))') { - result = effects.some(n => n === Effect.Allow); - } else if (expr === '!some(where (p_eft == deny))') { - result = !effects.some(n => n === Effect.Deny); - } else if (expr === 'some(where (p_eft == allow)) && !some(where (p_eft == deny))') { - result = false; - for (const eft of effects) { - if (eft === Effect.Allow) { - result = true; - } else if (eft === Effect.Deny) { - result = false; - break; - } - } - } else if (expr === 'priority(p_eft) || deny') { - result = effects.some(n => n !== Effect.Indeterminate && n === Effect.Allow); - } else { - throw new Error('unsupported effect'); - } - - return result; + newStream(expr: string): EffectorStream { + return new DefaultEffectorStream(expr); } } diff --git a/src/effect/defaultEffectorStream.ts b/src/effect/defaultEffectorStream.ts new file mode 100644 index 0000000..0df148e --- /dev/null +++ b/src/effect/defaultEffectorStream.ts @@ -0,0 +1,68 @@ +// Copyright 2020 The Casbin Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { EffectorStream } from './effectorStream'; +import { Effect } from './effector'; + +/** + * DefaultEffectorStream is the default implementation of EffectorStream. + */ +export class DefaultEffectorStream implements EffectorStream { + private done = false; + private res = false; + private readonly expr: string; + + constructor(expr: string) { + this.expr = expr; + } + + current(): boolean { + return this.res; + } + + public pushEffect(eft: Effect): [boolean, boolean] { + switch (this.expr) { + case 'some(where (p_eft == allow))': + if (eft == Effect.Allow) { + this.res = true; + this.done = true; + } + break; + case '!some(where (p_eft == deny))': + this.res = true; + if (eft == Effect.Deny) { + this.res = false; + this.done = true; + } + break; + case 'some(where (p_eft == allow)) && !some(where (p_eft == deny))': + if (eft == Effect.Allow) { + this.res = true; + } else if (eft == Effect.Deny) { + this.res = false; + this.done = true; + } + break; + case 'priority(p_eft) || deny': + if (eft !== Effect.Indeterminate) { + this.res = eft === Effect.Allow; + this.done = true; + } + break; + default: + throw new Error('unsupported effect'); + } + return [this.res, this.done]; + } +} diff --git a/src/effect/effector.ts b/src/effect/effector.ts index 05fa2fd..f353b76 100644 --- a/src/effect/effector.ts +++ b/src/effect/effector.ts @@ -14,6 +14,8 @@ // Effect is the result for a policy rule. // Values for policy effect. +import { EffectorStream } from './effectorStream'; + export enum Effect { Allow = 1, Indeterminate, @@ -22,6 +24,5 @@ export enum Effect { // Effector is the interface for Casbin effectors. export interface Effector { - // mergeEffects merges all matching results collected by the enforcer into a single decision. - mergeEffects(expr: string, effects: Effect[], results: number[]): boolean; + newStream(expr: string): EffectorStream; } diff --git a/src/effect/effectorStream.ts b/src/effect/effectorStream.ts new file mode 100644 index 0000000..185734b --- /dev/null +++ b/src/effect/effectorStream.ts @@ -0,0 +1,22 @@ +// Copyright 2020 The Casbin Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { Effect } from './effector'; + +// EffectorStream provides a stream interface +export interface EffectorStream { + current(): boolean; + + pushEffect(eft: Effect): [boolean, boolean]; +} diff --git a/src/effect/index.ts b/src/effect/index.ts index 0b92b92..dfbaf62 100644 --- a/src/effect/index.ts +++ b/src/effect/index.ts @@ -12,5 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -export * from './defaultEffector'; export * from './effector'; +export * from './effectorStream'; +export * from './defaultEffector'; +export * from './defaultEffectorStream';