Skip to content

Commit

Permalink
perf(ability): checks for fields only if fields were specified in rules
Browse files Browse the repository at this point in the history
Also updates docs to potential deprecation of `AbilityBuilder.define`
  • Loading branch information
stalniy committed Jul 8, 2019
1 parent 4fc1fad commit da013d4
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 34 deletions.
39 changes: 23 additions & 16 deletions packages/casl-ability/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down Expand Up @@ -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
Expand All @@ -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?
Expand Down
57 changes: 39 additions & 18 deletions packages/casl-ability/src/ability.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export class Ability {
RuleType,
subjectName,
originalRules: rules || [],
hasPerFieldRules: false,
indexedRules: Object.create(null),
mergedRules: Object.create(null),
events: {},
Expand All @@ -39,23 +40,36 @@ 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;
}

buildIndexFor(rules) {
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]);
Expand All @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit da013d4

Please sign in to comment.