Skip to content

Commit

Permalink
refactor(ability): removes deprecated types and fields
Browse files Browse the repository at this point in the history
Removes `RuleAnalyzer` and moves its logic under `Rule` and `RuleIndex`
Relates to #355

BREAKING CHANGE: removes deprecated options and types:

* `AbilityOptions['subjectName']` has been removed, use `detectSubjectType` instead
* `LegacyClaimRawRule` and `LegacySubjectRawRule` are both removed, so you are no longer allowed to use `actions` in rule definition, use `action` property instead
* `Ability` throws an error if you specify a rule with property `field` to be an empty array
* `Ability` no longer warns about using only inverted rules. This may be done by intention, so now it's left up to developer to decide whether it's fine or not
  • Loading branch information
stalniy committed Aug 6, 2020
1 parent 74dbe22 commit bf5ef73
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 115 deletions.
20 changes: 2 additions & 18 deletions packages/casl-ability/spec/ability.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,6 @@ describe('Ability', () => {
expect(secondSubscription).to.have.been.called()
})

it('warns if ability contains only inverted rules', () => {
spy.on(console, 'warn', () => {})
ability.update([{ inverted: true, action: 'read', subject: 'Post' }])

// eslint-disable-next-line
expect(console.warn).to.have.been.called()

spy.restore(console, 'warn')
})

function setupListenerChangesInListener() {
const unsubscribe = spy(ability.on('update', function listen() {
unsubscribe()
Expand Down Expand Up @@ -442,14 +432,8 @@ describe('Ability', () => {
expect(() => new PureAbility(rules)).to.throw(/"fieldMatcher" option/)
})

it('warns if there is a rule with "fields" property to be an empty array', () => {
spy.on(console, 'warn', () => {})
ability = defineAbility(can => can('read', 'Post', []))

// eslint-disable-next-line
expect(console.warn).to.have.been.called()

spy.restore(console, 'warn')
it('throws if there is a rule with "fields" property to be an empty array', () => {
expect(() => defineAbility(can => can('read', 'Post', []))).to.throw(/`rawRule.fields` cannot be an empty array/)
})

describe('when field patterns', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/casl-ability/spec/builder.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ describe('AbilityBuilder', () => {
})

it('accepts options for `Ability` instance as the 1st parameter', () => {
const subjectName = subject => typeof subject === 'string' ? subject : subject.ModelName
const ability = defineAbility({ subjectName }, (can) => {
const detectSubjectType = subject => typeof subject === 'string' ? subject : subject.ModelName
const ability = defineAbility({ detectSubjectType }, (can) => {
can('read', 'Book')
})

Expand Down
8 changes: 4 additions & 4 deletions packages/casl-ability/src/AbilityBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import {
import { RawRule } from './RawRule';

class RuleBuilder<T extends RawRule<any, any>> {
public rule!: T;
public _rule!: T;

constructor(rule: T) {
this.rule = rule;
this._rule = rule;
}

because(reason: string): this {
this.rule.reason = reason;
this._rule.reason = reason;
return this;
}
}
Expand Down Expand Up @@ -88,7 +88,7 @@ export class AbilityBuilder<T extends AnyAbility = PureAbility> {
conditions?: Generics<T>['conditions'],
): RuleBuilder<RawRuleOf<T>> {
const builder = (this as any).can(action, subject, conditionsOrFields, conditions);
builder.rule.inverted = true;
builder._rule.inverted = true;
return builder;
}

Expand Down
22 changes: 2 additions & 20 deletions packages/casl-ability/src/RawRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,8 @@ export interface SubjectRawRule<A extends string, S extends SubjectType, C> exte
subject: S | S[]
}

export interface LegacyClaimRawRule<A extends string> extends BaseRawRule<undefined> {
/** @deprecated use "action" field instead */
actions: A | A[]
subject?: undefined
}

export interface LegacySubjectRawRule<A extends string, S extends SubjectType, C>
extends BaseRawRule<C> {
/** @deprecated use "action" field instead */
actions: A | A[]
subject: S | S[]
}

type DefineRule<
T extends AbilityTypes = AbilityTupleType,
C = unknown,
Else = ClaimRawRule<Extract<T, string>> | LegacyClaimRawRule<Extract<T, string>>
> = T extends AbilityTupleType
? SubjectRawRule<T[0], T[1], C> | LegacySubjectRawRule<T[0], T[1], C>
: Else;
type DefineRule<T extends AbilityTypes, C, Else = ClaimRawRule<Extract<T, string>>> =
T extends AbilityTupleType ? SubjectRawRule<T[0], T[1], C> : Else;

export type RawRule<T extends AbilityTypes = AbilityTupleType, C = unknown> = DefineRule<T, C>;
export type RawRuleFrom<T extends Abilities, C> = RawRule<ToAbilityTypes<T>, C>;
37 changes: 23 additions & 14 deletions packages/casl-ability/src/Rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,24 @@ import {
ToAbilityTypes,
Normalize
} from './types';
import { RawRule } from './RawRule';
import { RawRule, RawRuleFrom } from './RawRule';

type Tuple<A extends Abilities> = Normalize<ToAbilityTypes<A>>;

function validate<A extends Abilities, C>(rule: RawRuleFrom<A, C>, options: RuleOptions<A, C>) {
if (Array.isArray(rule.fields) && !rule.fields.length) {
throw new Error('`rawRule.fields` cannot be an empty array. https://bit.ly/390miLa');
}

if (rule.fields && !options.fieldMatcher) {
throw new Error('You need to pass "fieldMatcher" option in order to restrict access by fields');
}

if (rule.conditions && !options.conditionsMatcher) {
throw new Error('You need to pass "conditionsMatcher" option in order to restrict access by conditions');
}
}

export class Rule<A extends Abilities, C> {
private readonly _matchConditions: MatchConditions | undefined;
private readonly _matchField: MatchField<string> | undefined;
Expand All @@ -22,26 +36,21 @@ export class Rule<A extends Abilities, C> {
public readonly reason!: string | undefined;

constructor(rule: RawRule<ToAbilityTypes<A>, C>, options: RuleOptions<A, C>) {
this.action = options.resolveAction((rule as any).actions || (rule as any).action);
validate(rule, options);

this.action = options.resolveAction(rule.action);
this.subject = rule.subject;
this.inverted = !!rule.inverted;
this.conditions = rule.conditions;
this.reason = rule.reason;
this.fields = !rule.fields || rule.fields.length === 0
? undefined
: wrapArray(rule.fields);

if ('actions' in rule) {
// eslint-disable-next-line
console.warn('Rule `actions` field is deprecated. Use `action` field instead');
}
this.fields = rule.fields ? wrapArray(rule.fields) : undefined;

if (this.conditions && options.conditionsMatcher) {
this._matchConditions = options.conditionsMatcher(this.conditions);
if (this.conditions) {
this._matchConditions = options.conditionsMatcher!(this.conditions);
}

if (this.fields && options.fieldMatcher) {
this._matchField = options.fieldMatcher(this.fields);
if (this.fields) {
this._matchField = options.fieldMatcher!(this.fields);
}
}

Expand Down
26 changes: 10 additions & 16 deletions packages/casl-ability/src/RuleIndex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,10 @@ import {
RuleOptions,
} from './types';
import { wrapArray, detectSubjectType, identity } from './utils';
import RulesAnalyzer from './RulesAnalyzer';

type AnyRuleIndex = RuleIndex<any, any, any>;

export interface RuleIndexOptions<A extends Abilities, Conditions> {
/** @deprecated use "detectSubjectType" option instead */
subjectName?: this['detectSubjectType']
detectSubjectType?: DetectSubjectType<Normalize<A>[1]>
conditionsMatcher?: ConditionsMatcher<Conditions>
fieldMatcher?: FieldMatcher
Expand Down Expand Up @@ -86,7 +83,7 @@ export class RuleIndex<A extends Abilities, Conditions, BaseEvent extends {} = {
resolveAction: options.resolveAction || identity,
};
Object.defineProperty(this, 'detectSubjectType', {
value: options.detectSubjectType || options.subjectName || detectSubjectType
value: options.detectSubjectType || detectSubjectType
});
Object.defineProperty(this, 'rules', { get: () => this._rules });
this.update(rules);
Expand All @@ -101,31 +98,22 @@ export class RuleIndex<A extends Abilities, Conditions, BaseEvent extends {} = {

this._emit('update', event);
this._mergedRules = Object.create(null);

const analyser = new RulesAnalyzer<A, Conditions>(rules.length > 0);
const indexedRules = this._buildIndexFor(rules, analyser._analyze);

analyser._validate(this._ruleOptions);
this._indexedRules = indexedRules;
this._indexedRules = this._buildIndexFor(rules);
this._rules = rules;
this._hasPerFieldRules = analyser._hasPerFieldRules;
this._emit('updated', event);

return this;
}

private _buildIndexFor(
rawRules: RawRuleFrom<A, Conditions>[],
iterator: RuleIterator<A, Conditions> = identity
) {
private _buildIndexFor(rawRules: RawRuleFrom<A, Conditions>[]) {
const indexedRules: IndexTree<A, Conditions> = Object.create(null);

for (let i = 0; i < rawRules.length; i++) {
iterator(rawRules[i], i);
const rule = new Rule(rawRules[i], this._ruleOptions);
const priority = rawRules.length - i - 1;
const actions = wrapArray(rule.action);
const subjects = wrapArray(rule.subject);
this._analyze(rule);

for (let k = 0; k < subjects.length; k++) {
const subject = this.detectSubjectType(subjects[k]);
Expand All @@ -142,6 +130,12 @@ export class RuleIndex<A extends Abilities, Conditions, BaseEvent extends {} = {
return indexedRules;
}

private _analyze(rule: Rule<A, Conditions>) {
if (!this._hasPerFieldRules && rule.fields) {
this._hasPerFieldRules = true;
}
}

possibleRulesFor(...args: CanParameters<A, false>) {
const [action, subject] = args;
const subjectName = this.detectSubjectType(subject);
Expand Down
41 changes: 0 additions & 41 deletions packages/casl-ability/src/RulesAnalyzer.ts

This file was deleted.

0 comments on commit bf5ef73

Please sign in to comment.