Optimize by filtering grants by action and resource early #33
Conversation
- When checking for granted resources, actions, or attributes, the current code executes all conditions for all grants, and then filters the passed ones by action and/or resource in the query. - This is inefficient, especially if the condition checks are expensive, e.g. if they make database calls. - I've optimized it by first filtering out grants based on the action and resource in the query, and then running the condition checks on the remaining.
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| && this.anyMatch(query.action, grant.action); | ||
| }); | ||
|
|
||
| return (await this.filterGrantsAllowing(matchingGrants, query)) |
There was a problem hiding this comment.
Similar blocks of code found in 3 locations. Consider refactoring.
There was a problem hiding this comment.
I believe this is a false positive. Please manually review.
|
|
||
| const matchingGrants = (await this.getUnionGrantsOfRoles(grants, query)); | ||
|
|
||
| return (await this.filterGrantsAllowing(matchingGrants, query)) |
There was a problem hiding this comment.
Similar blocks of code found in 3 locations. Consider refactoring.
There was a problem hiding this comment.
I believe this is a false positive. Please manually review.
| }).map((grant) => { | ||
| }); | ||
|
|
||
| return (await this.filterGrantsAllowing(matchingGrants, query)) |
There was a problem hiding this comment.
Similar blocks of code found in 3 locations. Consider refactoring.
There was a problem hiding this comment.
I believe this is a false positive. Please manually review.
| }).reduce(Notation.Glob.union, []); | ||
| } | ||
|
|
||
| public static async filterGrantsAllowing(grants: IAccessInfo[], query: IQueryInfo): Promise<IAccessInfo[]> { |
There was a problem hiding this comment.
Function filterGrantsAllowing has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.
There was a problem hiding this comment.
This bit of code is just the latter part of getUnionGrantsOfRoles(...) that has been refactored into it's own function.
|
Code Climate has analyzed commit f0ca088 and detected 4 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (80% is the threshold). This pull request will bring the total coverage in the repository to 90.3% (0.0% change). View more on Code Climate. |
|
Thanks @loban, release this as https://www.npmjs.com/package/role-acl/v/4.1.2 |
executes all conditions for all grants, and then filters the passed ones
by action and/or resource in the query.
if they make database calls.
resource in the query, and then running the condition checks on the remaining.