Add caching to ACL#552
Merged
Add caching to ACL #552
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
RubenVerborgh referenced this pull request Aug 18, 2017
This was referenced Aug 18, 2017
RubenVerborghforce-pushed the feature/cache-acl branch from 94301db to bf912edCompare
RubenVerborghforce-pushed the feature/cache-acl branch from bf912ed to fa23c82CompareRubenVerborghforce-pushed the feature/cache-acl branch from 62cdcbc to 2e81b07Compare
ContributorAuthor
RubenVerborgh commented Aug 18, 2017
@dmitrizagidulin You might want to have this as a merge commit rather than rebase, in order to keep the commits together. |
Contributor
dmitrizagidulin commented Aug 18, 2017
@RubenVerborgh good call, re merge |
lib/acl-checker.js Outdated
| // Check the resource's permissions | ||
| returnthis._permissionSet.then(acls=>this.checkAccess(acls,user,mode)) | ||
| .catch(err=>{ |
Contributor
There was a problem hiding this comment.
I think this should be indented? Something like:
returnthis._permissionSet.then(acls=>this.checkAccess(acls,user,mode)).catch(err=>{})?
ContributorAuthor
There was a problem hiding this comment.
Makes sense, will do. (standard didn't complain; but I'd be the last person to say that it is a good style checker 😉)
ContributorAuthor
There was a problem hiding this comment.
Done in bda7426, let me know if more indentation needs adjustments.
Contributor
dmitrizagidulin commented Aug 18, 2017
Oh man, great work on this! |
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Avoid that the same .acl files are read and parsed multiple times per request.