From da013d438d3a90c91d99fb4422252f2ca3167238 Mon Sep 17 00:00:00 2001 From: Sergii Date: Mon, 8 Jul 2019 21:49:18 +0300 Subject: [PATCH] perf(ability): checks for fields only if fields were specified in rules Also updates docs to potential deprecation of `AbilityBuilder.define` --- packages/casl-ability/README.md | 39 +++++++++++-------- packages/casl-ability/src/ability.js | 57 +++++++++++++++++++--------- 2 files changed, 62 insertions(+), 34 deletions(-) diff --git a/packages/casl-ability/README.md b/packages/casl-ability/README.md index ef0cb420d..96f48bfaa 100644 --- a/packages/casl-ability/README.md +++ b/packages/casl-ability/README.md @@ -17,14 +17,20 @@ CASL concentrates all attention at what a user can actually do and allows to cre `AbilityBuilder` allows to define abilities using DSL: ```js -import { AbilityBuilder } from '@casl/ability' +import { AbilityBuilder, Ability } from '@casl/ability' class Website {} -const ability = AbilityBuilder.define((can, cannot) => { - can('protect', 'Website') - cannot('delete', 'Website') -}) +function defineRules() { + const { can, cannot, rules } = AbilityBuilder.extract() + + can('protect', 'Website') // you can use name of the class + cannot('delete', Website) // or class reference + + return new Ability(rules) +} + +const ability = defineRules() console.log(ability.can('delete', new Website())) // false ``` @@ -190,15 +196,16 @@ This function allows to extract an object of field-value pairs from rule conditi That object later can be passed in a constructor of a class, to provide initial values: ```js +import { AbilityBuilder, Ability } from '@casl/ability' import { rulesToFields } from '@casl/ability/extra' -const ability = AbilityBuilder.define(can => { - can('read', 'Post', { user: 5 }) - can('read', 'Post', { public: true }) -}) +const { can, rules } = AbilityBuilder.extract() + +can('read', 'Post', { user: 5 }) +can('read', 'Post', { public: true }) // { user: 5, public: true } -const initialValues = rulesToFields(ability, 'read', 'Post') +const initialValues = rulesToFields(new Ability(rules), 'read', 'Post') // `Post` may be a mongoose or sequelize record // or any other class which accepts fields as the 1st argument @@ -213,13 +220,13 @@ and collects only those fields which values are non plain objects (i.e., skips m For example: ```js -const ability = AbilityBuilder.define(can => { - can('read', 'Post', { createdAt: { $gte: new Date() } }) - can('read', 'Post', { userId: ObjectId(...) }) - can('read', 'Post', { public: true }) -}) +const { can, rules } = AbilityBuilder.extract() + +can('read', 'Post', { createdAt: { $gte: new Date() } }) +can('read', 'Post', { userId: ObjectId(...) }) +can('read', 'Post', { public: true }) -rulesToFields(ability, 'read', 'Post') // { userId: ObjectId(...), public: true } +rulesToFields(new Ability(rules), 'read', 'Post') // { userId: ObjectId(...), public: true } ``` ## Want to help? diff --git a/packages/casl-ability/src/ability.js b/packages/casl-ability/src/ability.js index 8a8567cc6..97b16ca74 100755 --- a/packages/casl-ability/src/ability.js +++ b/packages/casl-ability/src/ability.js @@ -30,6 +30,7 @@ export class Ability { RuleType, subjectName, originalRules: rules || [], + hasPerFieldRules: false, indexedRules: Object.create(null), mergedRules: Object.create(null), events: {}, @@ -39,16 +40,28 @@ export class Ability { } update(rules) { - if (Array.isArray(rules)) { - const payload = { rules, ability: this }; - - this.emit('update', payload); - this[PRIVATE_FIELD].originalRules = Object.freeze(rules.slice(0)); - this[PRIVATE_FIELD].indexedRules = this.buildIndexFor(rules); - this[PRIVATE_FIELD].mergedRules = Object.create(null); - this.emit('updated', payload); + if (!Array.isArray(rules)) { + return this; } + const payload = { rules, ability: this }; + + this.emit('update', payload); + this[PRIVATE_FIELD].originalRules = rules.slice(0); + this[PRIVATE_FIELD].mergedRules = Object.create(null); + + const index = this.buildIndexFor(rules); + + if (process.env.NODE_ENV !== 'production' && index.isAllInverted && rules.length) { + // eslint-disable-next-line + console.warn('[casl]: Ability contains only inverted rules. That means user will not be able to do any actions. This will be changed to Error throw in the next major version') + } + + this[PRIVATE_FIELD].indexedRules = index.rules; + this[PRIVATE_FIELD].hasPerFieldRules = index.hasPerFieldRules; + + this.emit('updated', payload); + return this; } @@ -56,6 +69,7 @@ export class Ability { const indexedRules = Object.create(null); const { RuleType } = this[PRIVATE_FIELD]; let isAllInverted = true; + let hasPerFieldRules = false; for (let i = 0; i < rules.length; i++) { const rule = new RuleType(rules[i]); @@ -65,6 +79,10 @@ export class Ability { isAllInverted = !!(isAllInverted && rule.inverted); + if (!hasPerFieldRules && rule.fields) { + hasPerFieldRules = true; + } + for (let k = 0; k < subjects.length; k++) { const subject = subjects[k]; indexedRules[subject] = indexedRules[subject] || Object.create(null); @@ -77,12 +95,11 @@ export class Ability { } } - if (process.env.NODE_ENV !== 'production' && isAllInverted && rules.length) { - // eslint-disable-next-line - console.warn('[casl]: Ability contains only inverted rules. That means user will not be able to do any actions. This will be changed to Error throw in the next major version') - } - - return indexedRules; + return { + isAllInverted, + hasPerFieldRules, + rules: indexedRules, + }; } expandActions(rawActions) { @@ -152,15 +169,19 @@ export class Ability { return Object.assign(rules, subjectRules[action], subjectRules.manage); }, []); - // TODO: think whether there is a better to way to prioritize rules + // TODO: think whether there is a better way to prioritize rules // or convert sparse array to regular one return mergedRules.filter(Boolean); } rulesFor(action, subject, field) { - // TODO: skip `isRelevantFor` method calls if there are not fields in rules - return this.possibleRulesFor(action, subject) - .filter(rule => rule.isRelevantFor(subject, field)); + const rules = this.possibleRulesFor(action, subject); + + if (!this[PRIVATE_FIELD].hasPerFieldRules) { + return rules; + } + + return rules.filter(rule => rule.isRelevantFor(subject, field)); } cannot(...args) {